Skip to content

Implemented normalizeHttpPath option - #90

Merged
raymondfeng merged 1 commit into
strongloop:masterfrom
fabien:feature/normalize-url
Sep 13, 2014
Merged

Implemented normalizeHttpPath option#90
raymondfeng merged 1 commit into
strongloop:masterfrom
fabien:feature/normalize-url

Conversation

@fabien

@fabien fabien commented Aug 4, 2014

Copy link
Copy Markdown
Contributor

@slnode

slnode commented Aug 4, 2014

Copy link
Copy Markdown

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@fabien

fabien commented Aug 4, 2014

Copy link
Copy Markdown
Contributor Author

See: strongloop/loopback#433 for LB integration

@raymondfeng

Copy link
Copy Markdown
Member

@ritch Can you help @fabien here?

@fabien

fabien commented Aug 8, 2014

Copy link
Copy Markdown
Contributor Author

@ritch @raymondfeng this is currently a blocking issue for me - AFAIK there's no good way to change the remoting path of the base models, and frankly having non-normalized paths in urls is just unacceptable to me; it should never have slipped into LB 2.0 in the first place IMHO: /Users ???

@ritch

ritch commented Aug 20, 2014

Copy link
Copy Markdown
Member

I think we need to support url formats instead of a flag to normalize URLs this way. This is what I am thinking...

function RemoteObjects() {
  // ...
  this.addFormat({
    for: 'url',
    name: 'dashed'
  }, function(original) {
    return dasherize(lowercase(original));
  });
  // ...
}

// usage (eg in a loopback app)
app.remotes().format('url', 'dashed');

// custom style
app.remotes().addFormat('url', 'my style', myFormatFn);

// we can also make other things format-able in the same way (note: different arg style)
app.remotes().addFormat('response body', 'namespaced', function(body) {
  return {
    myNamespace: body
  };
});

@fabien

fabien commented Aug 20, 2014

Copy link
Copy Markdown
Contributor Author

@ritch +1 although I would be just as happy with:

// usage (eg in a loopback app)
app.remotes({ formatting: { url: 'dashed' } });

@bajtos

bajtos commented Sep 6, 2014

Copy link
Copy Markdown
Member

Since none of us have time to implement the version with all bells and whistles, I am proposing to go with the solution proposed by @fabien.

It is flexible enough in the sense that it can be easily extended to support a function value of normalizePath and thus allowing the user to apply whatever custom normalization he wants.

// use the default implementation as provided by this pull request.
normalizePath: true` 

// the same configuration expressed as a custom function
// can be implemented later
normalizePath: function(fragment) { 
  return inflection.transform(fragment, ['underscore', 'dasherize']); 
}

Another pro of this patch is that the rest transport will finally have an options object, which we need for other features anyway, e.g. to add an option for disabling error stack traces.

@bajtos bajtos added the #review label Sep 6, 2014
@raymondfeng

Copy link
Copy Markdown
Member

@fabien Can you rebase?

@fabien

fabien commented Sep 12, 2014

Copy link
Copy Markdown
Contributor Author

@raymondfeng rebased (sorry for the double commit there)

@fabien
fabien force-pushed the feature/normalize-url branch from 4d752bd to 7a46b05 Compare September 12, 2014 10:47
@raymondfeng

Copy link
Copy Markdown
Member

Any objections if I merge it?

@bajtos

bajtos commented Sep 12, 2014

Copy link
Copy Markdown
Member

Any objections if I merge it?

I don't have any objections.

raymondfeng added a commit that referenced this pull request Sep 13, 2014
Implemented normalizeHttpPath option
@raymondfeng
raymondfeng merged commit 33e8240 into strongloop:master Sep 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants