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

Projects
None yet
5 participants
@b-admike
Copy link
Member

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 requested review from bajtos and raymondfeng as code owners Dec 12, 2018

@b-admike b-admike force-pushed the fix/hasone-uniqueness branch 2 times, most recently from caa876b to cb55408 Dec 12, 2018

@b-admike b-admike force-pushed the fix/hasone-uniqueness branch 3 times, most recently from 6d53e9d to 3b3975f Dec 12, 2018

fix(repository): revert hasOne target FK as PK implementation
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 Dec 12, 2018

@b-admike b-admike merged commit fcc76df into master Dec 12, 2018

5 checks passed

clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.08%) to 90.258%
Details
security/snyk - package.json (dhmlau) No new issues
Details

@b-admike b-admike deleted the fix/hasone-uniqueness branch Dec 12, 2018

@RaphaelDrai

This comment has been minimized.

Copy link

RaphaelDrai commented Dec 13, 2018

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

This comment has been minimized.

Copy link
Member

b-admike commented Dec 19, 2018

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

This comment has been minimized.

Copy link

gczobel-f5 commented Jan 3, 2019

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

@RaphaelDrai

This comment has been minimized.

Copy link

RaphaelDrai commented Jan 10, 2019

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

@bajtos

This comment has been minimized.

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.

@strongloop strongloop 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.