From 702542891a3b94389c0d228fa3e48cafb126f3a2 Mon Sep 17 00:00:00 2001 From: Zikeji Date: Thu, 1 Oct 2026 02:33:53 -0400 Subject: [PATCH 1/2] fix(importer): move single-file audiobooks into their folder on reorganize (#2894) A single-file audiobook tracked as the .m4b itself was given the templated folder path as its own destination, so Rename files turned Author/Title.m4b into a file named "Title (Year)" with no extension. Resolve a file row to /, mirroring the single-file import branch: join the book's own ebook folder (#2686), otherwise uniquify with the file's current folder treated as available so a file already in place stays a noop instead of moving to "Title (Year) (2)". The sidecar cleanup and write follow file vs folder rather than format. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Zikeji --- .../2894-reorganize-single-file-audiobook.md | 2 + internal/importer/destination_preview_test.go | 4 +- internal/importer/reorganize.go | 35 ++++- internal/importer/reorganize_test.go | 144 ++++++++++++++++++ 4 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 changelog.d/2894-reorganize-single-file-audiobook.md diff --git a/changelog.d/2894-reorganize-single-file-audiobook.md b/changelog.d/2894-reorganize-single-file-audiobook.md new file mode 100644 index 000000000..499e8a7a1 --- /dev/null +++ b/changelog.d/2894-reorganize-single-file-audiobook.md @@ -0,0 +1,2 @@ +### Fixed +- **Rename files no longer turns a single-file audiobook into a file with no extension** (#2894): an audiobook tracked as the `.m4b` itself, which is what a library scan records for a flat `Author/Title.m4b` layout, was moved onto its templated folder path, so `Stephen King/The Shining.m4b` became a file named `Stephen King/The Shining (1977)`. Rename then reported it as already correct, and an ebook import for the same book failed with `not a directory`. The file now moves inside the templated folder under its own name, its `metadata.opf` goes in that folder, and a file already in its folder stays put instead of being moved to `Title (Year) (2)`. With a shared library root it joins the book's ebook folder, the same as an import. A file already flattened by the old behaviour still needs its extension restored by hand. diff --git a/internal/importer/destination_preview_test.go b/internal/importer/destination_preview_test.go index 6efdd725e..ad70089b2 100644 --- a/internal/importer/destination_preview_test.go +++ b/internal/importer/destination_preview_test.go @@ -80,9 +80,9 @@ func TestPreviewImportDestination_FormatHintWins(t *testing.T) { if err != nil { t.Fatalf("PreviewImportDestination: %v", err) } - want := filepath.Join(audiobookDir, "Jane Doe", "Right Book (2020)") + want := filepath.Join(audiobookDir, "Jane Doe", "Right Book (2020)", "mislabelled.epub") if got.Destination != want { - t.Errorf("destination = %q, want the audiobook root %q", got.Destination, want) + t.Errorf("destination = %q, want inside the audiobook root %q", got.Destination, want) } } diff --git a/internal/importer/reorganize.go b/internal/importer/reorganize.go index 2eb15ad5e..4bad6e9d1 100644 --- a/internal/importer/reorganize.go +++ b/internal/importer/reorganize.go @@ -160,7 +160,11 @@ func (s *Scanner) proposedPathFor(ctx context.Context, book *models.Book, author // than a false collision (or an endless (N)→(N+1) churn). Ebooks are single // files and are not uniquified at import, so they skip this. if audiobook { - dest = uniqueDirExcluding(dest, f.Path) + if isSingleFile(f.Path) { + dest = s.singleFileAudiobookDest(ctx, book, dest, f.Path) + } else { + dest = uniqueDirExcluding(dest, f.Path) + } } if filepath.Clean(dest) == filepath.Clean(f.Path) { @@ -200,6 +204,21 @@ func (s *Scanner) proposedPathFor(ctx context.Context, book *models.Book, author return dest, ReorgStatusMove, "" } +func isSingleFile(path string) bool { + info, err := os.Stat(path) + return err == nil && !info.IsDir() +} + +// singleFileAudiobookDest mirrors the single-file import branch: join the +// book's own ebook folder (#2686), otherwise uniquify with the file's current +// folder treated as available so a file already in place stays a noop. +func (s *Scanner) singleFileAudiobookDest(ctx context.Context, book *models.Book, destDir, current string) string { + if existing, merging := s.existingEbookDir(ctx, book); !merging || filepath.Clean(destDir) != existing { + destDir = uniqueDirExcluding(destDir, filepath.Dir(current)) + } + return filepath.Join(destDir, filepath.Base(current)) +} + // uniqueDirExcluding mirrors UniqueDir but treats keep as if it were absent, so // resolving the destination for a file already parked at a uniquified location // returns that same location (a noop) instead of bumping to the next suffix. @@ -271,6 +290,7 @@ func (s *Scanner) applyOne(ctx context.Context, fileID int64) ReorganizeMove { return m } + singleFile := isSingleFile(file.Path) if err := s.moveTrackedFile(ctx, file, proposed); err != nil { m.Status = ReorgStatusFailed m.Message = err.Error() @@ -288,11 +308,12 @@ func (s *Scanner) applyOne(ctx context.Context, fileID int64) ReorganizeMove { return m } - // A single-file ebook leaves its metadata.opf behind when it moves out of a - // folder, which both strands a stale sidecar and blocks the prune below - // from reclaiming the now-empty folder. An audiobook moves as a whole - // directory, so its sidecar travels with it and there is nothing to clean. - if file.Format != models.MediaTypeAudiobook { + // A single file (an ebook, or a lone audiobook file) leaves its + // metadata.opf behind when it moves out of a folder, which both strands a + // stale sidecar and blocks the prune below from reclaiming the now-empty + // folder. An audiobook folder moves whole, so its sidecar travels with it + // and there is nothing to clean. + if singleFile { removeOrphanedSidecar(filepath.Dir(file.Path), filepath.Dir(proposed)) } pruneEmptyParents(filepath.Dir(file.Path), s.rootsForFormat(ctx, author, file.Format)) @@ -315,7 +336,7 @@ func (s *Scanner) applyOne(ctx context.Context, fileID int64) ReorganizeMove { // feature that is off by default. if s.opfSidecarEnabled(ctx) { sidecarDir := proposed - if file.Format != models.MediaTypeAudiobook { + if singleFile { sidecarDir = filepath.Dir(proposed) } edition := s.resolveCalibreEdition(ctx, nil, book) diff --git a/internal/importer/reorganize_test.go b/internal/importer/reorganize_test.go index e13ff5710..c979f42b1 100644 --- a/internal/importer/reorganize_test.go +++ b/internal/importer/reorganize_test.go @@ -595,3 +595,147 @@ func TestReorganize_NoSidecarWhenDisabled(t *testing.T) { t.Errorf("no sidecar may be written while the setting is off, stat err = %v", err) } } + +// TestReorganize_SingleFileAudiobookMovesIntoFolder is the regression test for +// #2894. An audiobook tracked as a lone .m4b used to be given the templated +// folder path as its own destination, so the move renamed the file to +// "My Book (2020)" with no extension and no folder. +func TestReorganize_SingleFileAudiobookMovesIntoFolder(t *testing.T) { + env, _, audiobookDir, ctx := reorgFixture(t) + book := env.seed(t, ctx, "Jane Doe", "My Book") + + oldPath := filepath.Join(audiobookDir, "Jane Doe", "My Book.m4b") + writeFileAt(t, oldPath) + if err := env.books.AddBookFile(ctx, book.ID, models.MediaTypeAudiobook, oldPath); err != nil { + t.Fatal(err) + } + + want := filepath.Join(audiobookDir, "Jane Doe", "My Book (2020)", "My Book.m4b") + moves, err := env.s.PreviewReorganizeBook(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + if len(moves) != 1 || moves[0].Status != ReorgStatusMove || moves[0].Proposed != want { + t.Fatalf("preview = %+v, want move to %q", moves, want) + } + + results := env.s.ApplyReorganize(ctx, []int64{moves[0].FileID}) + if results[0].Status != ReorgStatusMoved { + t.Fatalf("apply = %+v, want moved", results[0]) + } + info, err := os.Stat(want) + if err != nil { + t.Fatalf("file not at templated location: %v", err) + } + if !info.Mode().IsRegular() { + t.Errorf("%q is %v, want a regular file", want, info.Mode()) + } + files, err := env.books.ListBookFiles(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + if len(files) != 1 || files[0].Path != want { + t.Errorf("book_files path = %v, want %q", files, want) + } +} + +// A single-file audiobook already inside its templated folder is a noop. The +// folder exists because the file is in it, so it must not read as a collision +// and push the destination to "My Book (2020) (2)". +func TestReorganize_SingleFileAudiobookInPlaceIsNoop(t *testing.T) { + env, _, audiobookDir, ctx := reorgFixture(t) + book := env.seed(t, ctx, "Jane Doe", "My Book") + + path := filepath.Join(audiobookDir, "Jane Doe", "My Book (2020)", "My Book.m4b") + writeFileAt(t, path) + if err := env.books.AddBookFile(ctx, book.ID, models.MediaTypeAudiobook, path); err != nil { + t.Fatal(err) + } + + moves, err := env.s.PreviewReorganizeBook(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + if len(moves) != 1 || moves[0].Status != ReorgStatusNoop || moves[0].Proposed != path { + t.Fatalf("preview = %+v, want noop at %q", moves, path) + } +} + +func TestReorganize_SingleFileAudiobookSidecarGoesInsideTheFolder(t *testing.T) { + env, _, audiobookDir, ctx := reorgFixture(t) + env.enableOPFSidecar(t, ctx) + book := env.seed(t, ctx, "Jane Doe", "My Book") + + oldPath := filepath.Join(audiobookDir, "Jane Doe", "My Book.m4b") + writeFileAt(t, oldPath) + if err := env.books.AddBookFile(ctx, book.ID, models.MediaTypeAudiobook, oldPath); err != nil { + t.Fatal(err) + } + + moves, err := env.s.PreviewReorganizeBook(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + if results := env.s.ApplyReorganize(ctx, []int64{moves[0].FileID}); results[0].Status != ReorgStatusMoved { + t.Fatalf("apply = %+v, want moved", results[0]) + } + + folder := filepath.Join(audiobookDir, "Jane Doe", "My Book (2020)") + if _, err := os.Stat(filepath.Join(folder, "metadata.opf")); err != nil { + t.Errorf("sidecar should be inside the audiobook folder: %v", err) + } + if _, err := os.Stat(filepath.Join(folder, "My Book.m4b")); err != nil { + t.Errorf("audiobook should be inside its folder: %v", err) + } +} + +// With a shared root the templated audiobook folder is the ebook's folder. The +// single-file import merges into it (#2686), so reorganize must place the file +// there too rather than in "My Book (2020) (2)". +func TestReorganize_SingleFileAudiobookJoinsItsEbookFolder(t *testing.T) { + database, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { database.Close() }) + ctx := context.Background() + root := t.TempDir() + books := db.NewBookRepo(database) + authors := db.NewAuthorRepo(database) + s := NewScanner(db.NewDownloadRepo(database), db.NewDownloadClientRepo(database), + books, authors, db.NewHistoryRepo(database), root, root, "", "", "") + s.WithSettings(db.NewSettingsRepo(database)) + s.WithRootFolders(db.NewRootFolderRepo(database)) + s.WithSeriesRepo(db.NewSeriesRepo(database)) + env := reorgEnv{s: s, books: books, authors: authors} + + book := env.seed(t, ctx, "Jane Doe", "My Book") + ebook := filepath.Join(root, "Jane Doe", "My Book (2020)", "My Book - Jane Doe.epub") + writeFileAt(t, ebook) + if err := books.AddBookFile(ctx, book.ID, models.MediaTypeEbook, ebook); err != nil { + t.Fatal(err) + } + audio := filepath.Join(root, "Jane Doe", "My Book.m4b") + writeFileAt(t, audio) + if err := books.AddBookFile(ctx, book.ID, models.MediaTypeAudiobook, audio); err != nil { + t.Fatal(err) + } + + want := filepath.Join(root, "Jane Doe", "My Book (2020)", "My Book.m4b") + moves, err := s.PreviewReorganizeBook(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + for _, m := range moves { + switch m.Format { + case models.MediaTypeEbook: + if m.Status != ReorgStatusNoop { + t.Errorf("ebook: status = %q (%s), want noop", m.Status, m.Message) + } + case models.MediaTypeAudiobook: + if m.Status != ReorgStatusMove || m.Proposed != want { + t.Errorf("audiobook: %+v, want move to %q", m, want) + } + } + } +} From 3129bab32a55ea6520c557c0ae69b97cb9d0e65f Mon Sep 17 00:00:00 2001 From: vavallee Date: Thu, 1 Oct 2026 14:25:45 -0300 Subject: [PATCH 2/2] fix(importer): report audiobooks already flattened by #2894 instead of moving them again A file the old Rename files left on its folder path (named "Title (Year)" with no extension) used to read as a noop. With the file row fix it would instead be proposed as a move to "Title (Year) (2)/Title (Year)", burying the extensionless file in a new folder. Report it as an error naming the manual recovery and leave it in place. Also cover the stale sidecar cleanup for a lone audiobook file, apply the shared folder case end to end, and spell out the recovery steps and credit in the changelog fragment. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee --- .../2894-reorganize-single-file-audiobook.md | 2 +- internal/importer/reorganize.go | 21 ++++++ internal/importer/reorganize_test.go | 68 +++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/changelog.d/2894-reorganize-single-file-audiobook.md b/changelog.d/2894-reorganize-single-file-audiobook.md index 499e8a7a1..f92a2f10d 100644 --- a/changelog.d/2894-reorganize-single-file-audiobook.md +++ b/changelog.d/2894-reorganize-single-file-audiobook.md @@ -1,2 +1,2 @@ ### Fixed -- **Rename files no longer turns a single-file audiobook into a file with no extension** (#2894): an audiobook tracked as the `.m4b` itself, which is what a library scan records for a flat `Author/Title.m4b` layout, was moved onto its templated folder path, so `Stephen King/The Shining.m4b` became a file named `Stephen King/The Shining (1977)`. Rename then reported it as already correct, and an ebook import for the same book failed with `not a directory`. The file now moves inside the templated folder under its own name, its `metadata.opf` goes in that folder, and a file already in its folder stays put instead of being moved to `Title (Year) (2)`. With a shared library root it joins the book's ebook folder, the same as an import. A file already flattened by the old behaviour still needs its extension restored by hand. +- **Rename files no longer turns a single-file audiobook into a file with no extension** (#2894): an audiobook tracked as the `.m4b` itself, which is what a library scan records for a flat `Author/Title.m4b` layout, was moved onto its templated folder path, so `Stephen King/The Shining.m4b` became a file named `Stephen King/The Shining (1977)`. Rename then reported it as already correct, and an ebook import for the same book failed with `not a directory`. The file now moves inside the templated folder under its own name, its `metadata.opf` goes in that folder, and a file already in its folder stays put instead of being moved to `Title (Year) (2)`. With a shared library root it joins the book's ebook folder, the same as an import. A file already flattened by the old behaviour is now shown as an error in Rename files instead of being moved again, and needs fixing by hand: move it into a folder and give it back its extension (for example `The Shining (1977)/The Shining.m4b`), use **Forget this file** on the book for the old path (not Delete, which removes it from disk), then run a library scan so Bindery picks up the folder. Thanks zikeji for the report and the fix. diff --git a/internal/importer/reorganize.go b/internal/importer/reorganize.go index 4bad6e9d1..edb986264 100644 --- a/internal/importer/reorganize.go +++ b/internal/importer/reorganize.go @@ -161,6 +161,9 @@ func (s *Scanner) proposedPathFor(ctx context.Context, book *models.Book, author // files and are not uniquified at import, so they skip this. if audiobook { if isSingleFile(f.Path) { + if flattenedOntoFolderPath(f.Path, dest) { + return dest, ReorgStatusError, flattenedOntoFolderMsg + } dest = s.singleFileAudiobookDest(ctx, book, dest, f.Path) } else { dest = uniqueDirExcluding(dest, f.Path) @@ -219,6 +222,24 @@ func (s *Scanner) singleFileAudiobookDest(ctx context.Context, book *models.Book return filepath.Join(destDir, filepath.Base(current)) } +const flattenedOntoFolderMsg = "this audiobook file sits where its folder belongs and has lost its extension (#2894). " + + "Move it into a folder and restore its extension, use Forget this file on the book for the old path, then run a library scan" + +// flattenedOntoFolderPath reports whether a tracked single audiobook file is +// what Rename files left behind before #2894 was fixed: the file was moved onto +// its templated folder path itself (or the " (N)" variant it uniquified to), so +// it has the folder's name and no audio extension. Older releases read this as +// a noop. Proposing a move from here would bury the extensionless file one +// level down in a " (2)" folder, so it is reported instead and left for the +// manual recovery the message describes. +func flattenedOntoFolderPath(path, destDir string) bool { + if IsAudioFile(path) { + return false + } + p, d := filepath.Clean(path), filepath.Clean(destDir) + return p == d || strings.HasPrefix(p, d+" (") +} + // uniqueDirExcluding mirrors UniqueDir but treats keep as if it were absent, so // resolving the destination for a file already parked at a uniquified location // returns that same location (a noop) instead of bumping to the next suffix. diff --git a/internal/importer/reorganize_test.go b/internal/importer/reorganize_test.go index c979f42b1..9dd9207c0 100644 --- a/internal/importer/reorganize_test.go +++ b/internal/importer/reorganize_test.go @@ -736,6 +736,74 @@ func TestReorganize_SingleFileAudiobookJoinsItsEbookFolder(t *testing.T) { if m.Status != ReorgStatusMove || m.Proposed != want { t.Errorf("audiobook: %+v, want move to %q", m, want) } + if results := s.ApplyReorganize(ctx, []int64{m.FileID}); results[0].Status != ReorgStatusMoved { + t.Fatalf("apply = %+v, want moved", results[0]) + } + } + } + for _, p := range []string{ebook, want} { + if _, err := os.Stat(p); err != nil { + t.Errorf("%q should be in the shared folder after apply: %v", p, err) } } } + +// A file already flattened by the old #2894 behaviour sits on the folder path +// itself (or its " (N)" variant) with no extension. Proposing a move from there +// would push it, still extensionless, into "My Book (2020) (2)/My Book (2020)". +// It must be reported as an error with the recovery steps and left untouched. +func TestReorganize_SingleFileAudiobookFlattenedOntoFolderIsReported(t *testing.T) { + for _, name := range []string{"My Book (2020)", "My Book (2020) (2)"} { + t.Run(name, func(t *testing.T) { + env, _, audiobookDir, ctx := reorgFixture(t) + book := env.seed(t, ctx, "Jane Doe", "My Book") + + path := filepath.Join(audiobookDir, "Jane Doe", name) + writeFileAt(t, path) + if err := env.books.AddBookFile(ctx, book.ID, models.MediaTypeAudiobook, path); err != nil { + t.Fatal(err) + } + + moves, err := env.s.PreviewReorganizeBook(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + if len(moves) != 1 || moves[0].Status != ReorgStatusError || !strings.Contains(moves[0].Message, "#2894") { + t.Fatalf("preview = %+v, want an error naming #2894", moves) + } + if results := env.s.ApplyReorganize(ctx, []int64{moves[0].FileID}); results[0].Status != ReorgStatusError { + t.Fatalf("apply = %+v, want it left alone", results[0]) + } + if info, err := os.Stat(path); err != nil || !info.Mode().IsRegular() { + t.Errorf("flattened file must stay where it is: %v", err) + } + }) + } +} + +// A lone audiobook file moving out of an old folder leaves that folder's +// metadata.opf behind, the same as an ebook. It must be cleaned up so the old +// folder can be pruned. +func TestReorganize_SingleFileAudiobookLeavesNoOrphanSidecar(t *testing.T) { + env, _, audiobookDir, ctx := reorgFixture(t) + book := env.seed(t, ctx, "Jane Doe", "My Book") + + oldDir := filepath.Join(audiobookDir, "Jane Doe", "Old Title (2020)") + oldPath := filepath.Join(oldDir, "My Book.m4b") + writeFileAt(t, oldPath) + writeFileAt(t, filepath.Join(oldDir, "metadata.opf")) + if err := env.books.AddBookFile(ctx, book.ID, models.MediaTypeAudiobook, oldPath); err != nil { + t.Fatal(err) + } + + moves, err := env.s.PreviewReorganizeBook(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + if results := env.s.ApplyReorganize(ctx, []int64{moves[0].FileID}); results[0].Status != ReorgStatusMoved { + t.Fatalf("apply = %+v, want moved", results[0]) + } + if _, err := os.Stat(oldDir); !os.IsNotExist(err) { + t.Errorf("old folder should be pruned once its stale sidecar is removed, stat err = %v", err) + } +}