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

fix(repository): revert hasOne target FK as PK implementation #2147

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

b-admike
Copy link
Contributor

@b-admike b-admike commented Dec 12, 2018

Remove attempt at guaranteeing the foreign key of a has one relation to be unique by using it
as the primary key of the target model. This allow users to set up their own unique contstraints. @RaphaelDrai FYI (what I mentioned in #2005 (comment))

Motivation:

@bajtos @raymondfeng and I had a discussion and since the memory connector and cloudant connector do not support unique indexes, we've decided to have the first release of hasOne provide "weak" relations i.e. ease of navigation and aggregation instead of enforcement of referential integrity and document it so users are aware of the limitations.

TODO: docs for the hasOne relation (most likely in another PR, so we can do a release).

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@b-admike b-admike self-assigned this Dec 12, 2018
@b-admike b-admike force-pushed the fix/hasone-uniqueness branch 2 times, most recently from caa876b to cb55408 Compare December 12, 2018 13:24
@b-admike b-admike force-pushed the fix/hasone-uniqueness branch 3 times, most recently from 6d53e9d to 3b3975f Compare December 12, 2018 18:02
Remove attempt at guaranteeing the foreign key of a has one relation to be unique by using it
as the primary key of the target model. This allow users to set up their own unique contstraints.
@b-admike b-admike force-pushed the fix/hasone-uniqueness branch from 3b3975f to 37ef168 Compare December 12, 2018 18:23
@b-admike b-admike merged commit fcc76df into master Dec 12, 2018
@b-admike b-admike deleted the fix/hasone-uniqueness branch December 12, 2018 18:58
@RaphaelDrai
Copy link

Hello @b-admike,
I understand that the change will guarantee the uniqueness relation from the source to the target but, the uniqueness of the relation in the opposite direction (from the target to the source) is upon the user decision, this is right?
If the answer is positive, can you please elaborate the two way configurations in the @model() for the user to guarantee or ignore the uniqueness from the target to the source direction?
I mean how to configure the @hasone and @belongsTo decorators in the @model().
Thanks.

@b-admike
Copy link
Contributor Author

Hello @b-admike,
I understand that the change will guarantee the uniqueness relation from the source to the target but, the uniqueness of the relation in the opposite direction (from the target to the source) is upon the user decision, this is right?
If the answer is positive, can you please elaborate the two way configurations in the @model() for the user to guarantee or ignore the uniqueness from the target to the source direction?
I mean how to configure the @hasone and @belongsTo decorators in the @model().
Thanks.

Actually the changes do not guarantee uniqueness of the relation in any way, it is up to our users to set up the underlying database to be used instead of LoopBack doing so since we couldn't find a generic approach for it and need to investigate how to deliver a robust solution. You can find more info at #1718 (comment) and #2161.

@gczobel-f5
Copy link
Contributor

But what about in-memory? In this case the is no DB...

@RaphaelDrai
Copy link

Hello,
What is the status of the missing implementation of DELETE and PATCH CRUD operations?
Thanks,
Raphael

@bajtos
Copy link
Member

bajtos commented Jan 10, 2019

@RaphaelDrai Good catch! I think those two operations should have been implemented as part of #1422 but somehow that task slipped out.

Please create a new GitHub issue for this missing feature.

I am going to lock this pull request to collaborators only, please open new issues for any new discussions.

@loopbackio loopbackio locked as resolved and limited conversation to collaborators Jan 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants