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

lb4 relations referential integrity for hasMany/belongsTo #1718

Open
virkt25 opened this Issue Sep 17, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@virkt25
Copy link
Contributor

virkt25 commented Sep 17, 2018

Description / Steps to reproduce / Feature proposal

As I was working on the user order history component for https://github.com/strongloop/loopback4-example-shopping I ran into an issue I thought LoopBack would've taken care of for me -- referential integrity with the @hasMany relation.

Current Behavior

I am able to create an order using /post/{userId}/orders for a userId which is not that of a user. I would've expected some sort of a check that would've ensure that the userId exists.

Expected Behavior

Throw an error for an userId not in the database.

Acceptance Criteria

For longer term, we can look into a better way to handle referential integrity. For now:

  • check the database whether the userId exist and then make the post call

See Reporting Issues for more tips on writing good issues

@dhmlau

This comment has been minimized.

Copy link
Contributor

dhmlau commented Nov 27, 2018

In discussion with @raymondfeng @bajtos:
Proposed solution: check the database whether the userId exist and then make the post call
In the future, we can look into a better way to handle it.

@dhmlau dhmlau removed the needs-grooming label Nov 27, 2018

@dhmlau dhmlau referenced this issue Nov 27, 2018

Closed

Monthly Milestone - December 2018 ⛄️ 🎄 #2084

11 of 22 tasks complete

@bajtos bajtos changed the title lb4 relations referential integrity lb4 relations referential integrity for hasMany/belongsTo Nov 29, 2018

@jannyHou jannyHou added this to the December 2018 Milestone milestone Nov 30, 2018

@dhmlau dhmlau removed this from the December 2018 Milestone milestone Dec 2, 2018

@dhmlau dhmlau added the TOB label Dec 2, 2018

@jannyHou

This comment has been minimized.

Copy link
Contributor

jannyHou commented Dec 11, 2018

Defer the estimation until we figure out at which level are we going to apply the constraint.
There are different approaches:

  • repository level: the framework handles it for users, enforce on the database side
  • controller level: fix the shopping example, enforce on the client side
@bajtos

This comment has been minimized.

Copy link
Member

bajtos commented Dec 13, 2018

In our recent discussion about HasOne's unique constraint for the foreign key, @raymondfeng @b-admike and me discovered a new way how to look at relations and constraints. Essentially, we see two levels of relations:

  • Weak relations, where the developer primarily wants to navigate related models (think of GraphQL) and referential integrity is either not important or already enforced by the (SQL) database.
  • Strong relations, where the referential integrity is guaranteed by the framework together with the database.

I think this issue (#1718) is applicable to Strong relations only.

IMO, referential integrity must be guaranteed at repository level (or lower). For example, when a test helper invokes repository APIs to seed the database with sample data, then we want the referential integrity to be preserved. If we implemented referential integrity at controller level, then consumers of Repository API would be bypassing these checks and would be able to created corrupted data.

Implementation wise, it's important to implement referential integrity in way that does not create any race conditions. For example, a naive implementation calling findById on the source model before creating a new instance of the target model creates a race condition - if another request deletes the source model instance after findById received database response but before create command is set, then we end up with corrupted data.

In SQL databases, referential integrity is typically implemented by defining FOREIGN KEY constraints. In the context of LB4, I think this means that lb4 migrate must create such constraints based on the relations configured by the app.

For NoSQL databases, we can use optimistic approach:

  • Create the target model first, assume the source model exists
  • Check if the source model really exists (call findById)
  • If the source model does not exist, then delete the newly created target model and return an error.

Alternatively, we can also tell users to use a different relation type when using a particular NoSQL database, e.g. use ReferencesOne instead of HasOne for databases that don't support UNIQUE constraint.

The difficult part is how to decide which approach to use for each database, and make it easy for users to know when they are using a relation that's not supported by their database. For example, a connector can print a warning when it detects that a model is trying to setup a foreign key or unique constraint.

For the scope of this particular GitHub issue, I am proposing:

  • Update the docs, clearly describe two flavors of relations (weak vs. strong)
  • Make it clear that at the moment, LB4 implements weak relations only and does not enforce referential integrity.
  • Create a follow-up Epic "Strong relations with referential integrity". We will need to do some research & spiking for the epic first, but let's not worry about that until the epic is on our short-term backlog. Add the following existing issue(s) to this new epic: #2127
@dhmlau

This comment has been minimized.

Copy link
Contributor

dhmlau commented Jan 22, 2019

@bajtos @b-admike , could you please add the acceptance criteria? Thanks.

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