fix(library): let distinct books sharing an identifier both into the library - #41
Merged
Merged
Conversation
…library An add was refused whenever any book already held carried the same ASIN or first ISBN, whatever else differed. Audible publishes one collection ASIN for every novella inside it, so B002V8MRS2 answers for "Dilation Sleep", "Nightingale" and "Grafenwalder's Bestiary" alike: importing one of them locked the other two out of the library for good. Two narrations of a book matched to the same product hit the same wall. An identifier now has to agree with the title and the narrators before the incoming book counts as one already held. That still refuses a genuine re-import, where all three match, and a book with no narrator recorded on either side compares equal on that count rather than manufacturing a difference out of missing data. Every book carrying the identifier is now considered rather than whichever one the database returned first, which is what GetAllByAsinAsync and GetAllByIsbnAsync are for - an identifier is no longer unique here, so the first row back is not necessarily the namesake worth comparing against. The rule lives in AudiobookEditionIdentity because both add paths need it: the application service behind manual import, and the API workflow behind Add New. Two implementations of "is this the same book" would eventually disagree, and a library that accepts a book down one path and refuses it down the other is worse than either answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQvEWJmh71bNydZiKtDqfo
…ale usings Adding the two "all matches" lookups pushed AudiobookRepository.cs to 530 lines, past the 500 the architecture test holds production sources to. The identifier lookups move to their own partial, which is a real seam rather than a split for the line count: they are the only members that resolve a book from an external identifier, and the reason they exist in "all" form is a property of identifiers rather than of the repository. dotnet format also removed two using directives the global usings already cover. Verified in a container this time, which is what should have happened before the first push: Release build clean, the eight new tests pass, and the architecture test is green again. Two LibraryController_AddToLibraryTests fail here, and fail identically on unmodified canary in the same container, so they are the Linux environment rather than this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQvEWJmh71bNydZiKtDqfo
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.
The bug
An add was refused whenever any book already in the library carried the same ASIN or first ISBN, whatever else differed — title, author and narrator were never consulted (
LibraryAddService.cs:105).Audible publishes one collection ASIN for every novella inside it. On my NAS,
B002V8MRS2answers for Dilation Sleep, Nightingale and Grafenwalder's Bestiary, so importing one locked the other two out permanently. Five import attempts in one session failed this way:Two narrations of one book matched to the same Audible product hit the same wall.
The rule now
An identifier has to agree with the title and the narrators before the incoming book counts as one already held. A genuine re-import — all three matching — is still refused. A book with no narrator recorded on either side compares equal on that count, so the title carries the distinction rather than a difference being manufactured out of missing data.
GetAllByAsinAsync/GetAllByIsbnAsyncwere added because an identifier is no longer unique here: the first row the database returns is not necessarily the namesake worth comparing against.One rule, both paths
AudiobookEditionIdentityholds it, and both add paths call it — the application service behind manual import and the API workflow behind Add New. They previously carried separate copies of the check. Two implementations of "is this the same book" would eventually disagree, and a library that accepts a book down one path and refuses it down the other is worse than either answer.Testing
Eight new unit tests covering the collection-ASIN case, the two-narrations case, a true re-import, narrator order, missing narrators on both sides, the legacy single-narrator field, and that every namesake is checked rather than the first.
I could not build or run tests locally — dotnet isn't installed on this machine (macOS isn't a supported dev target for this repo), so the pre-commit
dotnet formatand pre-push checks were bypassed. CI is the first real compile of this change; treat a red build here as expected feedback rather than a surprise.Worth sending upstream — the collection-ASIN problem is not fork-specific.
🤖 Generated with Claude Code
https://claude.ai/code/session_01TQvEWJmh71bNydZiKtDqfo