Fix #49: author merge trips the unique provider-id indexes while the source still owns them - #66
Open
sebclark wants to merge 1 commit into
Open
Conversation
…till owns them Persist the survivor without the contested provider ids first (preserving the abort-safety the method is named for), then hand the ids over inside the merge transaction: release them on the source row, apply them to the survivor, and delete the source atomically.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #49
The bug
Exactly as diagnosed in the issue:
TryMergeAuthorsWithoutDataLosscopies the source's provider ids onto the survivor and callsUpdateAuthor(target)before the transaction deletes the source row — so the source still owns the same values and the save trips the unique index. All five provider-id columns are covered by unique indexes (UX_Authors_HardcoverAuthorId,GoodreadsAuthorId,AudnexusAuthorId,OpenLibraryAuthorId,GoogleBooksAuthorId), so any of them can block the merge, and every bulk sync retries and fails the same way.Fix
The existing early
UpdateAuthor(target)isn't accidental — it's the method's abort-safety (a failure leaves the source intact), so the fix keeps it rather than reordering it away:MergeIntoSurvivorcomputes the merged state as beforeReleaseAndTransferUniqueProviderIdsclears the ids on the source row and applies the merged ids to the survivor via SQL, then the children are reassigned and the source deleted — all committed atomically, so the unique indexes never see two rows holding the same value and any failure rolls the whole handoff backTests
New
AuthorMergeUniqueProviderIdFixturerunning against a real SQLite database with the real five unique indexes and the issue's exact data shape (source416owningaz:B000APO0PQ, survivor1577):saving_the_survivor_while_the_source_still_owns_the_id_reproduces_issue_49— pins the failure mode:UNIQUE constraint failed: Authors.AudnexusAuthorIdhandoff_should_transfer_all_unique_provider_ids_without_violating_the_indexes— clean transfer + source deletionrollback_should_leave_the_source_untouched— abort-safety preservedBooks suite: 568/568 passing.