Loopback-datasource-juggler: Embedded object prevents further container validations to pass

[raulmt]

Bug or feature request

• [x] Bug
• [ ] Feature request

Description of feature (or steps to reproduce if bug)

1. Clone https://github.com/strongloop/loopback-example-relations
2. Add a file named zz-customer-address.js to server/boot directory so it's the last boot script to be executed, with the following content:

module.exports = function(app, cb) {
  var Customer = app.models.Customer;
  Customer.findOne({where: {name: 'John Smith'}}, function(err, customer) {
    customer.save(cb);
  });
};

1. Run the app with node .

Link to sample repo to reproduce issue (if bug)

https://github.com/strongloop/loopback-example-relations : this loopback example repo has the problem; with the steps mentioned above you can reproduce it.

Expected result

No error

Actual result (if bug)

The added boot script will make loopback throw this error:

ValidationError: The `Customer` instance is not valid. Details: `address` is invalid: `id` can't be set (value: undefined).

Getting the customer which has an address and trying to save it it's just one of the actions that will trigger this same error. The real problem is the embedded Address is not valid because it has an id set, even when forceId was specified as false in the relation. So since it's invalid, the entire customer cannot be saved/updated in any way.

I'm not sure why, but explicitly setting forceId to false in the Address JSON model definition seems to fix this problem. If this is intended, then it should probably be mentioned in the documentation for embedded relations and this official example mentioned above should be updated with that. But it might actually be a bug in datasource juggler, and it's probably related to the issue 1 listed here #727 since forceId at embedded relation level is set to false, but this is still always generating an id for the embedded object… so maybe if the id is not generated then the issue would not happen.

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

Node 6.9.2, latest loopback downloaded with npm install in the example project (currently 3.1.1).

[jannyHou]

@raulmt In LB3.x we change forceId: true by default due to security issue. Please check doc https://loopback.io/doc/en/lb3/Migrating-to-3.0.html#use-forceid-to-explicitly-set-model-ids for details.

And in your case, if you want to save a customer with an embedded address instance which has id value, you need to add forceId: false in common/models/address.json.
So the fix depends on your requirement:

• if you have to embed the instance with id provided, then do the change I described above.
• if id is not necessary, you can remove the id property from customer.address then save.

[jannyHou]

I am closing this issue since the answer figured out. Please feel free to reopen it if you have other questions :) Thanks for understanding.

[raulmt]

@jannyHou setting forceId: false inside common/models/address.json is the same "solution" I found, described in the original message. But the thing is the example (and myself in my project) was already specifying forceId: false in the relationship options: https://github.com/strongloop/loopback-example-relations/blob/master/common/models/customer.json#L21

That forceId inside options is a documented property, and my expectation was that would make this Address model, inside this relationship context, to not generate an id. Because setting forceId to false at Address model level will make that behaviour happen in every context, and this model could very well be also used outside of a relationship, where you could want to have the auto-generated id.

Am I explaining better why I still see a problem here? Because, then what is options.forceId supposed to do then? It's even described with a different default, false, which for embedded objects actually makes sense… but instead is using the default true at the related model definition level.

[jannyHou]

@raulmt True it sounds like a bug of the option. We will take a look asap.

[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.

[raulmt]

@jannyHou Hi, this was automatically closed but you agreed it is a bug. Should this at least be left as open?

[dhmlau]

Reopening this as requested.

[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.