Configure remoting options via app.set and config.json

[bajtos]

#433 adds a remoting option normalizeHttpPath. At the moment, the only way how to set this options is to modify server/server.js and change the line calling loopback.rest().

We should modify loopback.rest() to read default options via app.get(). Since app.get() options are already populated from config.json by loopback-boot, this change will allow developers to keep the configuration in json files.

Example configuration in config.json:

{
  "port": 3000,
  "restApiRoot": "/api",
  "remoting": {
    "normalizeHttpPath": true
  }
}

Implementation in app.handler():

options = extend(this.get('remoting') || {}, options);
var remotes = this.remotes();
var handler = this._handlers[type] = remotes.handler(type, options);

/cc @fabien @ritch

[ritch]

👍

[fabien]

@bajtos @raymondfeng unfortunately, normalizeHttpPath: true from server/config.json doesn't seem to work as expected. The option is never made available to https://github.com/strongloop/loopback/blob/master/lib/model.js#L120 which means the only way to currently do this, is using Model.settings.remoting.normalizeHttpPath = true, which defeats the purpose. It's usually something you'd switch on for your whole app.

[bajtos]

If I am reading #164 correctly, then you need to set the flag inside rest section:

{
  "remoting": {
      "rest": {"normalizeHttpPath": true}
  }
}

I think it's not necessary to pass the option to https://github.com/strongloop/loopback/blob/master/lib/model.js#L120, because the app-wide setting is IMO used as the default value - see your PR strongloop/strong-remoting#90.

[fabien]

@bajtos it is already in the rest section (by default, in a new LB project), yet I don't see how the app-wide setting is used. I tried, but it's never passed to that particular point in the code, and unfortunately I don't see a way to get the app-wide setting there (app is not available in model.js I think).

[bajtos]

I see, I guess it is a bug then. I am afraid I don't have enough time to investigate this now myself, sorry for that.

I quickly looked at https://github.com/fabien/strong-remoting/blob/7a46b050b8156d1d6ae10e1283f1444da0c52b4d/lib/remote-objects.js#L135-L145, it seems that global options are applied only to classes gathered via exports, not to classes already registered via _classes. Perhaps that's the root of the problem?

[kevinji]

@fabien Since the option is currently broken, is there a workaround that can force the enabling of normalizeHttpPath? Your suggestion of Model.settings.remoting.normalizeHttpPath = true doesn't seem to quite work.

[fabien]

@mc10 it should work, but I guess it depends on where you declare this setting.

Have you tried adding this in your model.json?

properties: { ... },
remoting: {
  normalizeHttpPath: true
}

[fabien]

@bajtos yes, this is the issue indeed I think.

[kevinji]

Do I need to add this to each individual model's .json files? That seems quite unwieldy, and you can't change the built-in models either.

[fabien]

@mc10 I'm afraid this is the only option right now, we need to look in to the issue. You might be able to iterate over all models in a boot script, and set normalizeHttpPath on each model. For this to work, the bootscript should run before the point where the api is initialized on boot.

[kevinji]

I'm not sure how I'd go about iterating through the models individually--would you mind explaining? As a stopgap measure I've used a remoting option in each individual model's JSON file.

[bajtos]

We should rethink what needs to be done here. The current project template loads loopback.rest via server/middleware.json, where it is possible to specify the options object passed to loopback.rest(options). In a discussion elsewhere, we came to the conclusion that remoting config should be in fact moved away from server/config.json and kept in server/middleware-config.json.

[sinedied]

Is there any news regarding the the normalizeHttpPaths options?
I just download the latest strongloop tools and created the "notes" example project with Loopback 3.0.0, and this options is there in config.json.

I changed the normalizeHttpPaths to true:

  "remoting": {
    "context": false,
    "rest": {
      "handleErrors": false,
      "normalizeHttpPath": true,
      "xml": false
    }

But it has absolutely no effect on the PersistedModel methods, nor the model name so it is basically useless and does not what the docs says it should :/

Am I missing something? The first time I reported issues with this was more than a year ago (#1785), was there any progress?

[bajtos]

@sinedied I did a bit of research and found the following:

You can enable normalizeHttpPaths at model level, for example by adding the following lines to your model JSON file:

{
  "name": "CartItem",
  "base": "PersistedModel",
  // ...
  "remoting": {
    "normalizeHttpPath": true
  }
}

2. AFAICT, there is no code in loopback and strong-remoting that would allow app-wide configuration of normalizeHttpPath. Our project template in loopback-workspace is wrong :( Would you mind to submit a pull request fixing it? See
   templates/projects/empty-server/data.js#L71
   and Add remoting options to server/config.json loopback-workspace#164 (cc @raymondfeng).

3. While we are talking about templates, I think it would be nice to modify the model template to include normalizeHttpPath setting, see common/models/model-definition.json#L38. I am thinking about the following:

{
  // ...
    "scopes": "object",
    "indexes": "object",
    "options": {
      "type": "object",
      "default": {
        //---begin addition
        "remoting": {
          "normalizeHttpPath": false
        }
        //---end
        "validateUpsert": true
      }

cc @crandmck is the option normalizeHttpPath documented anywhere? Do we need to fix the docs too?

[sinedied]

@bajtos Yes, I noticed the only place this flag works was at the module level, but even though it only affect user-added remoting methods, but not methods inherited from PersistedModel.

The doc is actually misleading on this option, but it would really be useful to have this option works as described, application-wide to enforce consistency on REST endpoints.

As a side note, I find it very ugly that default remoting methods from PersistedModel use a mix of cameCase and kebab-case, that's not very professional when you want to expose a public API :/

[bajtos]

@sinedied I don't remember all the details about this flag. Could you please create a small app reproducing the issue you are facing per our bug reporting instructions and open a new issue to discuss what's missing in the current implementation of normalizeHttpPath? Please mention my GitHub handle (@bajtos) in the issue description, I am not following all new LoopBack issues.

Let's keep this issue #608 focused on what the title says - "Configure remoting options via app.set and config.json".

[Freundschaft]

I also think its highly confusing that the documentation states https://loopback.io/doc/en/lb3/config.json.html#top-level-properties

If true, in HTTP paths, converts:
Uppercase letters to lowercase.
Underscores (_) to dashes (-).
CamelCase to dash-delimited.
Does not affect placeholders (for example ":id"). For example, "MyClass" or "My_class" becomes "my-class".

but it doesnt acutally work...

[crandmck]

Yes, if it doesn't actually work as documented, then we should remove doc for remoting.rest. normalizeHttpPath from config.json and instead document the property in the model definition JSON file.

@Freundschaft Can you confirm that this is correct?

Of course, if/when this is fixed/changed then we should change the docs back accordingly.

[Freundschaft]

@crandmck at least for me the global settings defintively dont work, i didnt check in the loopback sourcecode though.

[crandmck]

OK, I changed the docs, PTAL ^

[Freundschaft]

how can I see the preview of the docs you changed?

[crandmck]

@Freundschaft You can see the changes to the source markdown files in the PR: https://github.com/strongloop/loopback.io/pull/296/files

If you want to preview how they will look on when published to the site, follow instructions in http://loopback.io/doc/en/contrib/jekyll_getting_started.html.

[lehni]

I have managed to address this issue in https://github.com/lehni/strong-remoting/commit/f8ec056509f2fb816e81a00dde440c70fe4e6996

I will create PR for it now on https://github.com/strongloop/strong-remoting

[lehni]

This is the PR for it: strongloop/strong-remoting#410
And here the changes: https://github.com/lehni/strong-remoting/commit/b7908a55fb1d68fd5af68062329e0905f174a108

[crandmck]

@lehni Will the above PR require any changes to docs (once it's merged)?
If so, could you summarize please?

[lehni]

@crandmck I will look into documentation requirements once the PR landed. I believe that what I found online is actually vague enough that it is not wrong for the new behavior.

Bu I just realized that while normalizeHttpPath is mentioned in the documentation of config.json for LB 2, it is gone from LB 3's docs:

https://loopback.io/doc/en/lb2/config.json.html
https://loopback.io/doc/en/lb3/config.json.html

Perhaps it's then simply a question of bringing it back?

[crandmck]

I'm sure it was removed for a reason, but if this PR resurrects it, then it's easy enough to reinstate it in the v3 docs. Please advise!

[bajtos]

IIRC, rest. normalizeHttpPath path does not work and the feature should be configured in server/middleware.json instead. I am not entirely sure about this, it would be best to add a unit-test to loopback project to verify which approach actually works: remoting options in config.json or loopback#rest parameters in server/middleware.json or perhaps both.

[lehni]

rest.normalizeHttpPath works for me with the above PR, no need for middleware. I am happy to look into unit tests for the various scenarios. Into which module would they go? strong-remoting also?

[bajtos]

I am happy to look into unit tests for the various scenarios. Into which module would they go? strong-remoting also?

That's great to hear! These tests should go to strongloop/loopback, to verify our assumptions about how strong-remoting works inside LoopBack apps. You will need to install your development version of strong-remoting into your loopback project, I personally use npm link.

We already have a test for model-level normalization here, so just add a new test for application-level normalization.

[lehni]

I think I need some help to get this particular test running. I thought I tracked it down to:

grunt karma:unit-once

...or this, if I wanted to run the tests repeatedly while editing them:

grunt karma:unit

To test things, I then changed this line:

 request(app).get('/user-accounts').expect(200, done);

...to:

 request(app).get('/UserAccounts').expect(200, done);

...hoping, it would trigger an error when running the tests again, but it did not.

What am I doing wrong?

[lehni]

Ok, my bad... Turns out it's on the mocha side of things:

grunt mochaTest:unit

[lehni]

@bajtos I've created a PR for the tests now: #3527

[bajtos]

Closing as done by strongloop/strong-remoting#410 and #3527. Thank you @lehni for this valuable contribution! 👏

[crandmck]

So, should I put rest.normalizeHttpPath back into the v3 docs?

[bajtos]

So, should I put rest.normalizeHttpPath back into the v3 docs?

@crandmck yes please!

[crandmck]

Done with loopbackio/loopback.io@657d537.

See https://loopback.io/doc/en/lb3/config.json.html#remoting-properties

[lehni]

@bajtos great news, thank you!