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

feat: add EntityEdgeDeletionBehavior.SET_NULL_INVALIDATE_CACHE_ONLY #178

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

wschurman
Copy link
Member

@wschurman wschurman commented Jul 14, 2022

Why

Previously it was recommended to use EntityEdgeDeletionBehavior.CASCADE_DELETE_INVALIDATE_CACHE when the database was set up to do ON DELETE SET NULL, but this had the bug where the deletion triggers would be run for the entity that was not actually being deleted by the DBMS (it was just being updated).

The correct solution before this PR was to use EntityEdgeDeletionBehavior.SET_NULL and rely upon entity to do the right thing.

This PR introduces a new EntityEdgeDeletionBehavior for the ON DELETE SET NULL case, EntityEdgeDeletionBehavior.SET_NULL_INVALIDATE_CACHE_ONLY, which runs the update triggers (and update cache invalidations) but rely upon the dbms for the actual underlying data update.

How

Add new mode and tests.

Notes:

  • This corrects the odd cascading info passing to the privacy policies, which now is controlled entirely internally to the mutator which is correct (no more control of it via the public API).

Test Plan

Run tests.

@wschurman wschurman force-pushed the @wschurman/update-invalidate-only branch 3 times, most recently from 910e244 to 22f9f4a Compare July 14, 2022 18:51
@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Merging #178 (c7b52e2) into main (25d14bf) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
+ Coverage   96.17%   96.23%   +0.06%     
==========================================
  Files          80       80              
  Lines        1985     1991       +6     
  Branches      257      263       +6     
==========================================
+ Hits         1909     1916       +7     
+ Misses         71       70       -1     
  Partials        5        5              
Flag Coverage Δ
integration 96.23% <100.00%> (+0.06%) ⬆️
unittest 96.23% <100.00%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../src/__integration-tests__/entities/ChildEntity.ts 100.00% <ø> (ø)
packages/entity/src/Entity.ts 100.00% <ø> (ø)
packages/entity/src/EntityMutationInfo.ts 100.00% <ø> (ø)
packages/entity/src/EntityFieldDefinition.ts 100.00% <100.00%> (ø)
packages/entity/src/EntityMutator.ts 98.85% <100.00%> (+0.62%) ⬆️
packages/entity/src/EntityMutatorFactory.ts 100.00% <100.00%> (ø)
...ges/entity/src/ViewerScopedEntityMutatorFactory.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25d14bf...c7b52e2. Read the comment docs.

@wschurman wschurman force-pushed the @wschurman/update-invalidate-only branch from 22f9f4a to 25cf6bb Compare July 14, 2022 19:16
@wschurman wschurman marked this pull request as ready for review July 14, 2022 19:20
@wschurman wschurman requested review from ide and dsokal July 14, 2022 19:20
packages/entity/src/EntityFieldDefinition.ts Outdated Show resolved Hide resolved
packages/entity/src/EntityMutator.ts Show resolved Hide resolved
packages/entity/src/EntityMutatorFactory.ts Outdated Show resolved Hide resolved
packages/entity/src/EntityMutatorFactory.ts Outdated Show resolved Hide resolved
packages/entity/src/ViewerScopedEntityMutatorFactory.ts Outdated Show resolved Hide resolved
mutatorFactory
.forUpdate(inboundReferenceEntity, queryContext)
.setField(fieldName, null)
['updateInTransactionAsync'](
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does type checking still work with the indexed accessor? Might be better to cast the type of setField's return value to an UpdateMutator.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, typechecking still works.

@wschurman wschurman requested a review from ide July 14, 2022 20:33
@wschurman wschurman merged commit a0c35dd into main Jul 14, 2022
@wschurman wschurman deleted the @wschurman/update-invalidate-only branch July 14, 2022 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants