Optimized RemoveFromToManyRelationship endpoint #1039
Merged
+590
−237
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1039 +/- ##
==========================================
- Coverage 91.63% 89.95% -1.69%
==========================================
Files 252 257 +5
Lines 6777 7026 +249
==========================================
+ Hits 6210 6320 +110
- Misses 567 706 +139
Continue to review full report at Codecov.
|
bart-degreed
force-pushed
the
optimized-remove-to-many
branch
5 times, most recently
from
5943ccb to
861dc80
Compare
bart-degreed
force-pushed
the
optimized-remove-to-many
branch
from
861dc80 to
c529811
Compare
maurei
approved these changes
Aug 13, 2021
added 3 commits
August 13, 2021 14:29
…the requested set of attribute.
…ship) Before we'd fetch the full left resource including attributes, along with the full relationship (all related resources with all of their attributes). This commit changes that to only fetch the left ID and the subset of right IDs to be removed, then makes the EF Core change tracker believe the full relationship was loaded, so it can perform change detection and generate SQL for the diff. Replaced PlaceholderResourceCollector usage with resetting the change tracker on CreateResource/UpdateResource and failure in SaveChanges. This fixes broken JSON:API change tracking, where a side effect in database during save goes unnoticed. The refetch-after-save would execute the query, but did not refresh tracked entities. This is by design, see https://stackoverflow.com/questions/46205114/how-to-refresh-an-entity-framework-core-dbcontext. Resetting the full change tracker made this bug surface.
bart-degreed
force-pushed
the
optimized-remove-to-many
branch
from
c529811 to
cde3954
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Optimized DELETE Relationship endpoints (remove from to-many relationship)
Before we'd fetch the full left resource including attributes, along with the full relationship (all related resources with all of their attributes). This commit changes that to only fetch the left ID and the subset of right IDs to be removed, then makes the EF Core change tracker believe the full relationship was loaded, so it can perform change detection and generate SQL for the diff.
Replaced
PlaceholderResourceCollectorusage with resetting the change tracker on CreateResource/UpdateResource and failure inSaveChanges. This fixes broken JSON:API change tracking, where a side effect in database during save goes unnoticed. The re-fetch-after-save would execute the query, but did not refresh tracked entities. This is by design, see https://stackoverflow.com/questions/46205114/how-to-refresh-an-entity-framework-core-dbcontext. Resetting the full change tracker made this bug surface.Breaking change: we don't call
IResourceDefinition.OnPrepareWriteAsyncanymore from RemoveFromToMany endpoint.Fixes #1030.
QUALITY CHECKLIST