Expose Replace* methods

[Amir-61]

Description:

• Re-mapping updateAttributes and updateOrCreate endpoints to use PATCH verb by default for 3.X.
• Exposing replaceById and replaceORCreate via PUT verbs by default for 3.X.
• Add a flag model.settings.options.updateOnPUT for 3.X.

Note: This patch is for 3.X

Related to #2330 (Implementation for 2.x)

Connect to #1979

[bajtos]

When updateOnPUT is set, then you need to change the HTTP path too, not only the verb.

updateOnPUT=false

  replaceById     => PUT /:id
  replaceOrCreate => PUT /

updateOnPUT=true

  replaceById     => POST /:id/replaceById
  replaceOrCreate => POST /replaceOrCreate

[bajtos]

I don't think it's correct to call setRemoting twice for the same method, expecting it to create two HTTP endpoints. The correct way is to provide an array of values in the http property. See how exists defines two HTTP endpoints:

       http: [
         { verb: 'get', path: '/:id/exists' },
         { verb: 'head', path: '/:id' },
       ],

We should do the same here. For example:

var upsertEndpoints = [{ verb: 'patch', path: '/' }];
if (!options.replaceOnPUT) {
  upsertEndpoints.push( {
    verb: 'put',
    patch: '/',
  });
}

setRemoting('PersistedModel', 'upsert', {
  // ...
  http: upsertEndpoints
});

Or you can use the approach you have in place for replaceOrCreateOptions below, both ways are fine.

[Amir-61]

Hey @bajtos

Please note strong-remoting does not support re-registering/re-exposing endpoints for models; that means once the models get created, endpoints get registered/exposed so then later on you cannot pass something like the following and expect the new endpoints get registered for store model:

app.models.store.settings.replaceOnPUT = true;
app.models.store.setup();

In other words strong-remoting does not have this capability; the only way you can pass these settings is when you create the models. Hence, I changed the test suits to have two versions of models (let's say for store model); one with replaceOnPUT set to true and one for replaceOnPUT set to false in JSON file fixtures.

This patch is almost done; I would say 90% is done; could you please confirm you agree with my approach?

Thanks Miroslav!

[bajtos]

Perhaps this could be rewritten as follows?

options.replaceOnPUT = options.replaceOnPUT !== false;

[bajtos]

What is the difference between this test and it('has expected remote methods with model.settings.replaceOnPUT set to true' above? I think it's enough to keep only one of these tests, isn't it?

[bajtos]

@slnode test please

[Amir-61]

The CI failures would be passed once this PR#2375 lands. Please see this comment

@bajtos I addressed all of your concerns. Could you PTAL again?

Also I believe still another PR with some changes needs to be submitted for 2.x and a review may be needed for that as well. There would be some changes between these two patches (3.x Vs. 2.x)

Thanks :-)

[Amir-61]

Also please note I have changed the logic for formatMethod, getFormattedMethodsExcludingRelations, getFormattedScopeMethods,... based on the changes in strong-remoting...

[bajtos]

I think the second item/alias should have prototype. prefix too.

 'prototype.patchAttributes', 'prototype.updateAttributes'

[bajtos]

I think we either need to fix L627 to prepend prototype. prefix to isStatic:false methods, or change remoting definition of prototype.patchAttributes to specify method alias as prototype.updateAttributes. This depends on how strong-remoting treats aliases of prototype methods, which is something I am not sure about. Could you please check? It is also worth checking how loopback-connector-remote handles these aliases, to ensure code calling myModelInstance.updateAttributes keeps working (assuming the model is attached to a remote-backed datasource).

[Amir-61]

@bajtos I addressed your feedback... Please review :-)

PS: CI failures do not seem relevant.

[Amir-61]

@slnode test please

[Amir-61]

@slnode test please

[bajtos]

LGTM 👏