Loopback: White/Black listing methods generated by relations

[gunjpan]

Bug or feature request

• [x] Bug
• [ ] Feature request

Description of feature (or steps to reproduce if bug)

Continuing from #651, the feature is a limited one. It does not allow to white/black list methods imposed on a model by relations.

Expected result

white/black listing of methods should be applied to related model method endpoints along with model's own methods

Actual result (if bug)

For example: student.__get__projects, a related model method for endpoint:/api/students/:id/projects, is not affected when following is set in model-config.json:

"MyModel": {
    "dataSource": "db",
    "public": true,
    "options":{
      "remoting": {
      "sharedMethods": {
        "*": false, // this is not effective
        "create": true,
        "__get__projects": false //neither this for related model methods
      }
     }
    }
  }

Additional information (Node.js version, LoopBack version, etc)

[gunjpan]

cc/@bajtos

[gunjpan]

Another flaw, reported by @ahmed-abdulmoniem :
@gunjpan Another issue ..

Can we hide PATCH end points?

I see for example PATCH and PUT has the same method names but with index?! like upsert_0 and upsert?

When I hide upsert it hides both PATCH and PUT .. can we keep one of them and hide the others?

[superkhau]

@ritch I think we should get @jannyHou to work on this feature as she is more familiar in this area.

[jannyHou]

@superkhau
According to the bug described in the first comment:

Continuing from #651, the feature is a limited one. It does not allow to white/black list methods imposed on a model by relations.

It seems more like a loopback-model/strong-remoting issue, I am not familiar with how the white/black list work :(

relation relevant part:
loopback/model.js resolves relation functions in model.

[superkhau]

It seems more like a loopback-model/strong-remoting issue, I am not familiar with how the white/black list work :(

NP, I'm not sure either, I believe @richardpringle worked this feature awhile back. It'll probably touch both areas: remoting + the relation part you mentioned above. Anyways, let's leave it in the backlog and figure out when it actually gets prioritized. TY for chiming in. ;)

[richardpringle]

@jannyHou @superkhau, it wasn't actually me that worked on it. I'm not sure if nested relations were ever tested though.

[superkhau]

@richardpringle TY for chiming in. Guess we'll have to dig into it when it gets prioritized. :bowing_man:

[bajtos]

I am guessing that the code handling blacklist/whitelist is run in the first turn of the event loop, when a model is created/configured, but is not attached to a datasource yet. The relation methods are not defined at this time yet. These methods are defined in a later tick, when the model gets attached to a datasource.

If my guess is correct, then I think we should listen for events like modelRemoted, remoteMethodDisabled (see loopback-component-explorer) and/or create a new event for this purpose, e.g. remoteMethodDefined. I think a new event is the most robust solution.

Also while we are fixing relation methods, it would be good to fix any remote method defined in later turns of event loop. See e.g. how our SOAP (and maybe Swagger) connector work - they fetch WSDL/Swagger via HTTP first, and then eventually they define remote methods using the fetched metadata.

A generic test case that I think we should support:

var MyModel = ...;
// configure { "sharedMethods": {"*": false } }

setTimeout(() => {
  MyModel.remoteMethod('methodAddedLater', {http: {path: '/later', verb: 'GET' }});
  MyModel.methodAddedLater = function(cb) { cb(); };

  // verify that GET /mymodels/later returns 404
}, 50);

[davide-talesco]

Also I have noticed that if I selectively enable some methods of my model (which extends PersistedModel), the method on the prototype don't get actually exposed.
Not too sure this is related...

"MyModel": {
    "dataSource": "db",
    "public": true,
    "options":{
      "remoting": {
      "sharedMethods": {
        "*": false, 
        "create": true,
        "prototype.patchAttributes": true, //this method does not get exposed
      }
     }
    }
  }

Any ideas?

[bajtos]

I suspect the code handling sharedMethod configuration does not correctly support prototype methods. Here is the relevant code:

https://github.com/strongloop/loopback/blob/75fdd3cf327052495fd7ebb010704eb952e33562/lib/application.js#L570-L575

  var sharedMethods = model.sharedClass.methods({includeDisabled: true});
  sharedMethods.forEach(function(sharedMethod) {
    // use the specific setting if it exists
    var hasSpecificSetting = settings.hasOwnProperty(sharedMethod.name);
    if (hasSpecificSetting) {
      sharedMethod.shared = settings[sharedMethod.name];
    } // ...

sharedClass.methods() is defined here.

I have two hypothesis:

1. sharedMethod.name does not include prototype. prefix for prototype methods
2. Some of the shared methods returned by methods() call are temporary, therefore sharedMethod.shared changes will be discarded. I think the solution is to call sharedClass. disableMethodByName instead.

[luncht1me]

Just another note to add,
I have a custom model which I've disabled relation remote methods for.
But I have another model which uses the prior as a base, and the remote methods are NOT disabled on it. So I have to manually disable them on it.

Just thought I'd mention it.

[rashmihunt]

@jannyHou @kjdelisle FYI.. As per @ritch and @raymondfeng, part of this sprint I am going through loopback repository and triaging some issues as P1 based on severity and community interest in getting the problem resolved. I am also adding team-apex label based on the conversation in the issue and area of expertise of the team.

[mrbatista]

Same problem with remote method created with scopes.

[devpascoe]

I had to resort to adding it in the model.js for now
eg:
module.exports = function (Profile) {
Profile.disableRemoteMethodByName('prototype.__get__students');
}

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

[estebanDarriens]

can we reopen this issue?
I have a model with relations and I need to hide some relation's method, there is an existent way to do that?

I'm trying some like this in model-config.json: (_but it seems not to work_)

"MyModel": {
    "dataSource": "MySQLMaster",
    "public": true,
    "options": {
       "remoting": {
         "sharedMethods": {
            "*":false,
            "find": true,
            "findById": true,
            "findOne": true,
            "exists": true,
            "count": true,
            "create": true,
            "prototype.patchAttributes": true,
            "deleteById": true,

            "prototype.__get__domains": true,
            "prototype.__create__domains": true,
            "prototype.__delete__domains": false,
            "prototype.__findById__domains": true,
            "prototype.__destroyById__domains": false,
            "prototype.__count__domains": true,
        }

[Overdrivr]

You can hide them by writing the following code in MyModel.js

module.exports = function(MyModel) {
// second parameter is false for prototype functions, true for static methods like findById, findOne, etc.
MyModel.disableRemoteMethod('__get__domains', false);
// etc

[estebanDarriens]

@Overdrivr you're so right! I had forgot that possibility, and works perfectly, also it is possible hide with:

MyModel.disableRemoteMethodByName('prototype.__updateById__domains');

Maybe this restriction (hiding relations methods) should be added as a note in the documentation.

[richardpringle]

@estebanDarriens, can you open a new ticket proposing the addition to the documentation and mention @crandmck?

[virkt25]

Mention @bschrammIBM instead in the new ticket.

[richardpringle]

@virkt25, is @crandmck no longer maintaining the LoopBack documentation?

[virkt25]

Nope ... @bschrammIBM is maintaining the documentation now.