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

property name with `type` is not working correctly with loopback complex model #4131

Closed
PRAKASH-SIVAGURUNATHAN opened this Issue Feb 7, 2019 · 7 comments

Comments

Projects
None yet
5 participants
@PRAKASH-SIVAGURUNATHAN
Copy link

PRAKASH-SIVAGURUNATHAN commented Feb 7, 2019

We have a model definition as follows

{
  "name": "accounts",
  "plural": "accounts",
  "base": "PersistedModel",
  "idInjection": true,
  "options": {
    "validateUpsert": true
  },
  "properties": {
    "name": {
      "type": "string"
    },
    "items": {
      "type" : {
          "itemname": { "type": "string"},
          "type": { "type": "string" }
      }
    }
  },
  "validations": [],
  "relations": {},
  "acls": [],
  "methods": {}
}

In explorer the structure of the items is coming as shown below

screen shot 2019-02-07 at 12 23 09 pm

When we test the model by providing the values for items property, the response does not have a valid structure for items
screen shot 2019-02-07 at 12 29 30 pm

@PRAKASH-SIVAGURUNATHAN

This comment has been minimized.

Copy link
Author

PRAKASH-SIVAGURUNATHAN commented Feb 7, 2019

NOTE: If we have the property name as type in the top-level it works fine. Loopback is behaving differently only when the type model property is defined within a complex structure.

@bajtos bajtos added the customer label Feb 7, 2019

@jannyHou

This comment has been minimized.

Copy link
Contributor

jannyHou commented Feb 7, 2019

Hi @PRAKASH-SIVAGURUNATHAN could you fork https://github.com/strongloop/loopback-sandbox and provide some example code to reproduce your error? Thanks. I can help you investigate.

From your description, a property named "type" has conflict with some code in https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/model-builder.js, it will be quicker for us to debug with a sample app.

@PRAKASH-SIVAGURUNATHAN

This comment has been minimized.

Copy link
Author

PRAKASH-SIVAGURUNATHAN commented Feb 12, 2019

@jannyHou : I have created an forked branch of loopback-sandbox and pushed the model which our product is using. Please refer to the forked branch of
https://github.com/PRAKASH-SIVAGURUNATHAN/loopback-sandbox

@hacksparrow hacksparrow self-assigned this Feb 13, 2019

@hacksparrow hacksparrow added the bug label Feb 13, 2019

@hacksparrow

This comment has been minimized.

Copy link
Member

hacksparrow commented Feb 13, 2019

Thanks for the sample app @PRAKASH-SIVAGURUNATHAN. I can confirm it as a bug. Taking a look at the cause and trying to find a fix.

@hacksparrow

This comment has been minimized.

Copy link
Member

hacksparrow commented Feb 13, 2019

@PRAKASH-SIVAGURUNATHAN which connector are you using, by the way?

@hacksparrow

This comment has been minimized.

Copy link
Member

hacksparrow commented Feb 19, 2019

Bug is caused by the sub-property name type conflicting with internal Model properties. Relevant files - https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/model-definition.js#L237, https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/model-builder.js#L886.

We are already not allowing constructor as a top level key. type works fine as a top level key, but causes conflict when used as a sub-key.

I am working on a solution.

@hacksparrow hacksparrow referenced this issue Feb 20, 2019

Merged

feat: Support "type" key in sub-properties #1693

2 of 2 tasks complete

@dhmlau dhmlau added this to the February 2019 milestone milestone Feb 25, 2019

@hacksparrow

This comment has been minimized.

Copy link
Member

hacksparrow commented Feb 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.