Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lb-ng: Issue when working with related models #221

Open
markusiseli opened this issue May 3, 2016 · 11 comments
Open

lb-ng: Issue when working with related models #221

markusiseli opened this issue May 3, 2016 · 11 comments

Comments

@markusiseli
Copy link

Given two simple related models: user and answer, defined as follows:
user.json:

...
"relations": {
    "answers": {
    "type": "hasMany",
    "model": "answer",
}
...

and answer.json:

...
"relations": {
    "user": {
    "type": "belongsTo",
    "model": "user"
}
...

The client-side code runs fine, does all the user authentication etc. but creating an answer instance, given the user model using User.answers.create({"id": some_user_id}, {some_answer_json}); throws an Uncaught TypeError: Cannot set property 'create' of undefined.

I tracked the error back to lb-ng: lb-ng creates the following code (lb-services.js) for creation of an answer instance, given the user (R in this case is the user resource):

...
R.answers.create = function() {
    var TargetResource = $injector.get("Answer");
    var action = TargetResource["::create::user::answers"];
    return action.apply(R, arguments);
};
...

The issue with this code is that the property answers of R - as in R.answers - was not defined before defining R.answers.create, hence the error. This error occurs with all similar methods on related models, where the relation name is inserted: e.g. R.answers.createMany, etc.

Here is my very temporary fix:
In lb-services.js, manually initialize R.answers = {}; before the line R.anwers.create = function() { works and the answer instance gets created with the creatorId in the DB.

If this is a bug, I'd be glad to help fix it, but would need instructions. If this is not a bug, please let me know what is wrong with my understanding.

@dancingshell
Copy link

@markusiseli not sure if this is the fix, but you may need to explicitly allow methods for these related models in the user ACL (user.json). See https://docs.strongloop.com/display/public/LB/Accessing+related+models#Accessingrelatedmodels-Restrictingaccesstorelatedmodels

example:

"acls": [
    {
      "principalType": "ROLE",
      "principalId": "$owner",
      "permission": "ALLOW",
      "property": [
        "__create__answers"
      ]
    }
  ]

@markusiseli
Copy link
Author

@dancingshell thanks for this, it pointed me to the solution.
For security reasons I had disabled all methods by default using a mixin and had only activated the __create_answers but not the__get__answers method.

Here is an excerpt from my lb-services.js file with my comments:

// If __get__answers method enabled, the code below will be generated
// If not enabled the code is missing, but should be R.answers = {}, since other methods will depend on this, see next code segment!
R.answers = function() {  
    var TargetResource = $injector.get("Answer");
    var action = TargetResource["::get::user::answers"];
    return action.apply(R, arguments);
};

// If __get__answers method enabled, the code below will be generated
// If not enabled the code is missing
// Note that this code depends on the definition of the code above!
R.answers.create = function() {  
      var TargetResource = $injector.get("Answer");
      var action = TargetResource["::create::user::answers"];
      return action.apply(R, arguments);
};

Thus, for my current models, the fix was to expose the __get__answers method.
I would say that this is a BUG - probably not too high priority though.

@davidcheung
Copy link
Contributor

Hi @markusiseli, thanks for bringing this up, trying to understand the issue, so basically the bug is:
when in relations a get method isnt enabled, the whole Resource isnt defined?


this following line might provide some insight to whomever fixing issue:

// sort the names to make sure the get method creating the scope

// sort the names to make sure the get method creating the scope

@davidcheung davidcheung self-assigned this May 10, 2016
@markusiseli
Copy link
Author

@davidcheung yes. I used Model.disableRemoteMethod(methodName, method.isStatic); in a mixin to disable all methods by default and the __get__* method is part of these methods (see: strongloop/loopback#651). Then I enable only the methods I'd like to expose, like __create__*. I can generate a gist, please let me know.

@davidcheung
Copy link
Contributor

@markusiseli thanks! that would be great,
I think the main goal of fix should be making sure the behaviour is the same as exposed from strong-remoting, hoping to pin point exactly what triggers the relation resource to be undefined so whomever fixing this bug will have an easier time.

@markusiseli
Copy link
Author

@davidcheung please see following repository (did not succeed in generating gist):
https://github.com/markusiseli/loopback-angular-sdk-test
I hope this is ok...

@richardpringle
Copy link

richardpringle commented May 11, 2016

@davidcheung, threw a waiting label on here even though in this case we are waiting for you! 😉

@markusiseli, we generally prefer a repo over a gist! Makes our lives easier when digging for the root cause of an issue, thanks.

@davidcheung
Copy link
Contributor

Thanks @markusiseli! 😄 I probably won't be able to get to it today, but I think it should be enough for me to reproduce the issue

@superkhau superkhau removed the waiting label May 11, 2016
@davidcheung
Copy link
Contributor

Thanks @markusiseli, I can reproduce this problem,
since explorer can handle this, meaning its bug to the angular-sdk (not issue with strong-remoting)
might need to add a logic in buildScopeMethod() to handle this situation since the code is relying on get to handle this

@davidcheung davidcheung added bug and removed triage labels May 19, 2016
@DecentGradient
Copy link

I have this problem as well without any acls setup

@bajtos
Copy link
Member

bajtos commented Jun 7, 2017

I think this is the relevant code that's assuming a get method will be always present in a scope: lib/services.template.ejs#L277-L279:

      // sort the names to make sure the get method creating the scope
      // is emitted first (R.categories before R.categories.create)
      Object.keys(scopeMethods).sort().forEach(function(methodName) {

The fix should be relatively straightforward - check whether scopeMethods contain a top-level get method and if it does not, then emit an empty scope object.

Are there any volunteers for contributing this fix? I am happy to help you along the way, just mention my GH handle in the pull request description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants