Loopback: UpdateAttributes on embedsMany updates whole object

[kussberg]

Hello,
we have an embedsMany relation and would like to update the specific attribute of the relation object. Update in the REST just replaces/updates the whole object. Is it possible to do the updateAttributes on exactly this specific object?
Have a good time!

[loay]

Can you provide more info please?
Let's use this example in the tutorial:
https://docs.strongloop.com/display/public/LB/Embedded+models+and+relations#Embeddedmodelsandrelations-EmbedsMany
so customer has multiple emails. Which object are you trying to update its attribute? and when you say it updates the whole object instead of the specific attribute .. can you provide an example as well?
Thanks.

[mrbatista]

@loay same problem with embedOne relation. See this sandbox repository that reproduce the issue (Run npm test after install dependencies and see that second test fail).
The problem for mongoDB connector is that $set operator patch single field and replace object, for patch object need to use dot notation see this for more details.

[mrbatista]

@raymondfeng This is a critical bug: update attributes of model with embed object replace whole instead update only specified attributes. Any workaround for fix this issue?

[mrbatista]

Is it possible to have any feedback from a strongloop team member? If it's known bug, I should review my implementation logic. Thanks a lot.

[loay]

@mrbatista Looking into it .. Will get back to you shortly.

[mrbatista]

@loay Thanks!

[loay]

@ekussberg @mrbatista
i did few testings in order to ensure that the issue could be reproduced in the correct manner.

You are right .. when you update one attribute, it does not update all the other attributes .. so if we take the branch created by @mrbatista as an example.
when you try to update
setting.one
then it will only update one, but two and three recently inserted values will be erased and you then two will get the defaulted value, which is 0 and three will be blank as it is not require.d

However, from what I have seen, I do not think it is a bug but more as a feature of the embedded relations. The embedded relation is meant to be called as one entity.

workaround:
In your file: test.test.js, I replaced line 79 with this ;
.send({setting: {one: 9, two:res.body.setting.two, three:res.body.setting.three}})

That will inherit the recently inserted values for the non-updated attributes and won't make it default to the default value in JSON and test will pass .

@bajtos @raymondfeng Can you confirm if it is a bug or feature?
Thanks.

[fabien]

Looking at @mrbatista 's failing test case (without running it right now), I can see you were updating the 'parent' model's attributes, and then expect the related settings to be handled correctly. However, this is not how it works (you could call it a feature, but not a bug). Instead, you should perform the update on the relation.

So instead of:

PUT /api/tests/:id
{ setting: ... }

You should actually call:

PUT /api/tests/:id/_setting
{ ... }

Or, in code: testInstance._setting.update({ ... }, cb)

Here are some examples from the unit-tests:

https://github.com/strongloop/loopback-datasource-juggler/blob/master/test/relations.test.js#L3611
https://github.com/strongloop/loopback/blob/master/test/relations.integration.js#L787

Hope this helps.

[mrbatista]

@loay your workaround works, but there is a security issue: any client can override property/object of specific Model.
@fabien Ok for updateAttributes for embedsOne relation I need to call:

PUT /api/tests/:id/_setting
{ ... }

I expect, however, that

PUT /api/tests/:id
{ setting: ... }

skip relation (in this case embedsOne model), otherwise any client can compromise the integrity of the data. This API is required to updateAttributes of father model.
What do you think about it?

[fabien]

@mrbatista I agree, we should make sure that such embedded properties are excluded from 'parent' updates - @raymondfeng thoughts?

[loay]

@mrbatista
we can use proper ACL for giving proper access for the CRUD operations so we can avoid any security issues.

[mrbatista]

@loay I have set proper ACL with static role $owner. Rather than being a security problem, it is a data integrity one.

[loay]

@mrbatista
One more thing that might clear things ... which is somehow related to what @fabien was saying earlier

if I post the following Test instance:

[
  {
    "type": 1,
    "flag": 1,
    "email": "[email protected]",
    "setting": {
      "one": 10,
      "two": 20,
      "three": 30,
      "id": "string"
    },
    "profile": {},
    "id": 10
  }
]

now, I want to only update the email in the parent "Test" without touching anything else ..

PUT /api/Tests/{id}
{
  "email": "[email protected]"
}

Now that will give the following where the parent is updated separately while keeping the setting attributes untouched.

{
  "type": 1,
  "flag": 1,
  "email": "[email protected]",
  "setting": {
    "one": 10,
    "two": 20,
    "three": 30,
    "id": "string"
  },
  "profile": {},
  "id": 10
}

I will wait for the thoughts of @raymondfeng then maybe we can create a separate feature request for it.

[mrbatista]

@loay your example is right. Now if I want to update only field email and setting.one:

PUT /api/Tests/{id}
{
  "email": "[email protected]",
  "setting": {"one": 1}
}

the result is:

{
  "type": 1,
  "flag": 1,
  "email": "[email protected]",
  "setting": {
    "one": 1,
    "two": 0
  },
  "profile": {},
  "id": 10
}

embed model setting is completely overridden. Field setting.two is zero for default value in setting.json.
My consideration is that if I call this crud

PUT /api/tests/:id/_setting
{ ... }

for update attributes in embed model, I expect that

PUT /api/Tests/{id}
{
  "email": "[email protected]",
  "setting": {"one": 1}
}

skip setting.one field (and other fields if present) for integrity of data.
Do you think the same way?
For me it's a bug not feature.

[loay]

@mrbatista
Thanks for clarifying. We will put that on the list to get it sorted out ASAP.

[Bilbaukay]

+1 for bug vs feature
Spent some time wondering why my data were wiped out

[bajtos]

FWIW, we have a feature request https://github.com/strongloop/loopback/issues/1196 where the user is asking for "create" operation to create embedded models too. This may be an exact opposite of what is being discussed/requested here.

It makes me wonder: if create/update called on a parent model should not update related models, then what's the point of using an embedded relation instead of a regular one? If the app was using "hasMany" instead of "embedsMany", this whole issue would not exist. What other features of embedded relations are you depending on?

I am trying to build a better understanding of real-world usages of embedded relations, so that we can keep the right direction when making fixes and improvements.

[mitsos1os]

@ekussberg @loay @bajtos I too believe that if someone is posting on the specific embeded model endpoint, a PUT should only update the attributes mentioned and not the whole embedded model.
ex:
Given this model:

{
  "type": 1,
  "flag": 1,
  "email": "[email protected]",
  "setting": {
    "one": 10,
    "two": 20,
    "three": 30,
    "id": "string"
  },
  "profile": {},
  "id": 10
}

and issuing a PUT /api/Test/setting with body of {"two":200} should only update the property two of the setting embedded model, leaving the rest intact:

{
  "type": 1,
  "flag": 1,
  "email": "[email protected]",
  "setting": {
    "one": 10,
    "two": 200,
    "three": 30,
    "id": "string"
  },
  "profile": {},
  "id": 10
}

Besides this, the original problem from @ekussberg was not discussed or resolved, which is more clearly a bug.
When we issue an update to an embedded model of embedsMany relationship, it _still_ replaces the whole embedded model... Which I believe is wrong according to the way the rest API is modeled.
As per the example pointed by @loay , when issuing a PUT /api/customers/7/emails/e05bdbbc7d220a6228b5fe48 {label:personal}(id was an example of email model id) I expect it to only change the label property of the embedded email model,(as would happen if email was a root model) and not replace the whole model, which is what is happening right now...

[kussberg]

@mitsos1os yes you pointed in clear way! It should be possible to update specific attributes of embedded/relation models.

[mitsos1os]

@ekussberg Thanks! I just hope that a solution is provided soon so that we can use all this good stuff!

[mitsos1os]

I have created a pull request for this:
https://github.com/strongloop/loopback-datasource-juggler/pull/961

[kjdelisle]

Closing this issue due to age. Feel free to comment here if you still have interest in fixing this.