How to replaceById/replaceOrCreate with HasManyRepository

[ludohenin]

Description / Steps to reproduce / Feature proposal

While trying to implement an array inclusion in model property defined as HasMany I realized that HasManyRepository is missing a put method. The patch is not enough as it does not allow to have the foreign key set in the model (throwing an error Property myModelId cannot be changed!).

Current Behavior

HasManyRepository implements:

• create
• delete
• find
• patch

Expected Behavior

HasManyRepository should implement:

• create
• delete
• find
• patch
• put

[bajtos]

From what I remember, we have intentionally left out put (replaceById) method from HasManyRepository interface with the following reasoning:

In order to do a full replace of all properties, the caller must provide both the foreignKey value and the primary key (id). Since the caller already has access to the primary key of the target model, there is no need to go through the relation repository and the operation can be performed directly on DefaultCrudRepository for the target model.

Having said that, it's possible we are missing something and there are reasons why our proposal does not work well in practice. Let's discuss!

Also, there are two possible PUT endpoints we may want to expose:

• replaceById exposed at PUT /source-model/{foreign-key}/target-model/{primary-key}
• replaceOrCreate exposed at PUT /source-model/{foreign-key}/target-model
  @ludohenin which operation are you interested in?

[ludohenin]

@bajtos thanks for your detailled answer.

In order to do a full replace of all properties, the caller must provide both the foreignKey value and the primary key (id). Since the caller already has access to the primary key of the target model, there is no need to go through the relation repository and the operation can be performed directly on DefaultCrudRepository for the target model.

This make a lot of sense. I've adjusted my code to use the right repository and it works just perfectly.

Side note, in my usecase (mimicing an "include") I don't have (and I don't want) an extra endpoint to update a "subresource".

closing

[bajtos]

This make a lot of sense. I've adjusted my code to use the right repository and it works just perfectly.

Awesome! I am glad my advice worked for you.

I think you are not the last person encountering this question. Could you advise us how to improve the documentation so that the next person will find the information right in the docs and don't have to go to our issue tracker? Where would you (did you?) look?

Would you mind to contribute this improvement yourself? See https://loopback.io/doc/en/contrib/doc-contrib.html to get started.

I am re-opening this issue and labelling it as "docs".

[ludohenin]

@bajtos second thought on that one. I still thing the doc for updating is fine in some cases but then came one of the cases you mentioned:

• replaceById exposed at PUT /source-model/{foreign-key}/target-model/{primary-key}

I think this one makes a lot of sense as well (and there's a case in the todo-list example package).
All the thing is about validating the foreign-key on the target model, I don't think we should leave this up to the client, the relation must make sure the foreign-key is set to the right value. And IMO the URI is he source of truth not the body object (explicitly declared in the replaceById method.

// something like
replaceById(pk, fk, model) 

If I'm using the above mentioned end-point using the target model repository to do an update, it's super easy to mess up with the foreign key

thoughts ?

[bajtos]

@ludohenin sorry for a late response, I have too many GitHub notifications to keep up with them :(

• replaceById exposed at PUT /source-model/{foreign-key}/target-model/{primary-key}

I think this one makes a lot of sense as well (and there's a case in the todo-list example package).

I was not able to find the place in our todo-list example using this endpoint, could you please post a link to the code you are referring to?

All the thing is about validating the foreign-key on the target model, I don't think we should leave this up to the client, the relation must make sure the foreign-key is set to the right value. And IMO the URI is he source of truth not the body object (explicitly declared in the replaceById method.

I am not sure if all LB4 users will see it the same way and consider the FK value provided in the URI as the source of truth. I think this can create a confusing situation for callers, when they edit the FK in the request body and but don't see the changes being made in the database.

I guess it would make sense to me if the operation required both FK values to be identical, and aborting the request with a descriptive error otherwise.

If I'm using the above mentioned end-point using the target model repository to do an update, it's super easy to mess up with the foreign key

It's not clear to me why it's super easy to mess up the foreign key, could you please post your code so that I can better understand what you have tried so far?

To avoid confusion: I am not entirely against implementing replaceById functionality for relations. I just want to avoid design & implementation that will be confusing to LoopBack users. So far, it seems to me that the complexity of a good implementation outweighs benefits. I am happy to get convinced otherwise.

[ludohenin]

@bajtos Sorry (as well) for the very late reply

I guess it would make sense to me if the operation required both FK values to be identical, and aborting the request with a descriptive error otherwise.

You got it all here. Would be really nice to have this consistency check.