Repository navigation
fix(importer): small follow ups to #3000, #3003 and #2988 - #3031
Conversation
…ne (#2998) The language gate counted an EPUB declaring no specific language (none, und, mul) as neither allowed nor disallowed, so a release holding one plus a Swedish EPUB under an English only profile was rejected whole, although the untagged file alone imports. It now counts as acceptable: it is imported and the Swedish one skipped, the shape of the format gate. NormalizeLanguageCode also folds the withdrawn ISO 639-1 codes iw, in and ji onto he, id and yi. They used to pass through raw, read as no language, and relabel the book to "iw". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
…2937) PathOwnedByOtherBook counts a book_files row whose book no longer exists as owned. That is right for the delete guards, but adoption's pre-check and the existing file bind on add author used it too, so adoption gave a 409 naming no book and add author searched for a file it already had, while the write behind both (Track) would take the orphan row over. Add PathOwnedByLiveOtherBook, which joins books and ignores an orphan row, and use it only in AdoptionHandler.checkOwnership and existingFileOwnedByOtherBook. The delete guards (path_safety.go), reassign cleanup (manual_import.go) and the reorganize collision check keep the orphan counts as owned behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
…credits (#2942) isPlaceholderAuthor only trimmed outer brackets and folded case, so "Various Artists.", "Various Authors;", "Unknown Author(s)", "Full-Cast" and "Anon." read as authors, and publisher credits such as "Brilliance Audio", "Recorded Books", "Tantor Audio" or "HarperAudio" were not on the list at all. Each raised an author conflict with the folder and an Add author for nobody. Fold the tag before comparing (brackets, dots, slashes and apostrophes dropped, other punctuation read as a space) and extend the single table with "Author Unknown" and the common audiobook publishers and imprints. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ne (#2998) Review of the first commit: counting an undeclared (none, und, mul) EPUB as acceptable let a release of a Swedish edition plus the same edition with its metadata stripped import under an English only profile, be marked done and never searched again, the #2998 symptom. An undeclared EPUB now imports on its own or beside an EPUB in an allowed language, as on main, and no longer rescues a release whose only declared language is disallowed. The allowed plus disallowed skip path from #3000 is unchanged. The iw/in/ji normalisation stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Three independent fixes, all look correct.
Legacy ISO codes (internal/models/language.go:186): lookup sits correctly inside the len(code) == 2 block that runs after subtag stripping, so iw-IL strips to iw then maps iw -> he -> heb. ji -> yi -> yid confirmed in the table.
PathOwnedByLiveOtherBook (internal/db/book_files.go:388): SQL JOIN correctly excludes orphan rows. The pre-check/write pairing is sound: Track atomically reclaims an orphan when !ownerTitle.Valid (line 164), matching the pre-check returning false. Conservative callers (delete guards in path_safety.go:289,322, manual_import.go:459, reorganize.go:202) are untouched. The TOCTOU window between the second ownership check (line 346) and registerFile is covered by catching PathOwnedError at line 352.
isPlaceholderAuthor / placeholderKey (internal/importer/unmatched_evidence.go:110): key transformations are consistent (v.a. folds to va, n/a to na, semicolons become a space that Fields collapses). Test covers false-positive risk for real names (Penguin, Harper Lee, Cast, Unknown Soldier) -- all pass through.
Language enforcement: comment-only cleanup after the revert, no logic change.
One note, non-blocking: bookFileOwner interface in path_safety.go:362 declares only PathOwnedByOtherBook. The new method is used through the concrete db.BookRepo in both new call sites, so there is no compilation issue; the interface covers only the delete-guard path and does not need extending.
--- Bindery triage bot (automated). Reply to correct me; a human will see it.
Summary
Three small follow ups left by #3000, #3003 and #2988, one commit each.
Refs #2998, #2937, #2942.
Per item notes
8d0ad7ce). The first commit counted an undeclared EPUB (none,und,mul) as acceptable, which let a Swedish edition plus the same edition with stripped metadata import under anengprofile, the #2998 symptom. Net effect is main's rule, now pinned by tests and spelled out in the guide: an undeclared EPUB imports alone or beside an allowed one, and does not rescue a release whose only declared language is disallowed.NormalizeLanguageCodefoldsiw,in,jiontohe,id,yi(soheb,ind,yid). They used to pass through raw, count as no language, and relabel the book toiw. Every caller passes a language field value, never a title word, soincannot be read off a release name.PathOwnedByLiveOtherBook(joinsbooks, ignores a row whose book is gone). Used only inAdoptionHandler.checkOwnershipandexistingFileOwnedByOtherBook(the add author existing file bind), both guarding a write throughTrack, which already reclaims orphan rows. The delete guards inpath_safety.go, reassign cleanup inmanual_import.goand the reorganize collision check keepPathOwnedByOtherBook, where an orphan still counts as owned. The user guide line saying adoption still refuses such a file is updated. The #3003 line in the released v1.40.1 CHANGELOG section is left alone; the new fragment covers the change.isPlaceholderAuthorfolds the tag first (brackets, dots, slashes, apostrophes dropped, other punctuation read as a space) and the single table gains "Author Unknown", "Unknown Artists" and the common audiobook publishers (Brilliance, Recorded Books, Tantor, Blackstone, Penguin, HarperAudio, Macmillan, Random House, Podium). The oldv.a.,v/a,n/akeys are nowva,na.Known, not changed here: under a restricted profile an imported
undEPUB still relabels the book tound(relabelLanguagefalls back tometa.Language), the same as a loneundEPUB does today. #3000 already lists the any language variant of this as a follow up.Checklist
docs/User-Guide-Wiki.md, godoc on the new symbol)Test plan
Fail before (new tests against
origin/mainsources):TestLanguageEnforcement_UndeclaredFileBesideDisallowedBlocks(none,und,mul), against the first commit:download status = "imported", want "importBlocked". Passes on main and after8d0ad7ce.TestLanguageEnforcement_UndeclaredFileBesideAllowedImports(undeclared +en+sv): passes before and after, a guard. One EPUB is placed per book, so the undeclared file is named first to show it is taken, not skipped.TestLanguageEnforcement_TwoLetterCodesOutsideTheOldTablenewiw/in/jicases:"imported", want "importBlocked"TestNormalizeLanguageCode:NormalizeLanguageCode("iw") = "iw", want "heb", and thein/jicasesTestAdopt_FileLeftByADeletedBookIsAdopted:adopt = 409 ... already belongs to a book in your libraryTestHandleNewWantedBook_BindsAFileLeftByADeletedBook: declined the bindTestIsPlaceholderAuthor: 17 spellings returned false, and"Brilliance Audio"raised a conflictGuards that pass before and after:
TestAdopt_FileOfALiveBookIsStill409,TestHandleNewWantedBook_DoesNotBindAnotherBooksFile, the real name half ofTestIsPlaceholderAuthor.TestBookFileRepo_PathOwnedVariantsOnAnOrphanRowpins both variants (new symbol, so it cannot run on main).Ran:
gofmt -l,go vet ./...golangci-lint runv2.11.4 oninternal/importer,internal/api,internal/db,internal/models: 0 issuesgo test -timeout 40m ./internal/importer/ ./internal/api/ ./internal/db/ ./internal/models/: all ok (before8d0ad7ce)8d0ad7ce:go test -timeout 40m ./internal/importer/ -run TestLanguageEnforcement,./internal/models/, golangci-lint oninternal/importer: ok, 0 issuesgo test ./cmd/... ./internal/...: one run under load average 40 to 60 from other sessions. Everything passed except timing failures in code this PR does not touch, each passing when rerun alone:internal/apihit the 10m timeout (plusTestAddBook_OrphanAuthorDeletedOnTimeoutgot a 500), andinternal/duplicatesTestScanScaletook 160ms against a 100ms budget. Not rerun in full on an idle machine.go test -racemake smoke🤖 Generated with Claude Code
https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9