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

lb4 relations referential integrity for hasMany/belongsTo #1718

Closed
1 task
virkt25 opened this issue Sep 17, 2018 · 8 comments
Closed
1 task

lb4 relations referential integrity for hasMany/belongsTo #1718

virkt25 opened this issue Sep 17, 2018 · 8 comments
Labels
feature needs grooming Relations Model relations (has many, etc.) stale

Comments

@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
Copy link
Member

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.

@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
Copy link
Contributor

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
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: Relations and referential integrity in NoSQL databases #2127

@dhmlau
Copy link
Member

dhmlau commented Jan 22, 2019

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

@dhmlau
Copy link
Member

dhmlau commented Sep 11, 2019

The Strong relation epic won't be in the Q4 roadmap. Postponing this.

@odykyi

This comment has been minimized.

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been marked stale because it has not seen activity within six months. 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. This issue will be closed within 30 days of being stale.

@stale stale bot added the stale label Dec 25, 2020
@stale
Copy link

stale bot commented Jul 14, 2021

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.

@stale stale bot closed this as completed Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature needs grooming Relations Model relations (has many, etc.) stale
Projects
None yet
Development

No branches or pull requests

5 participants