diff --git a/changelog.d/2900-single-file-audiobook-template.md b/changelog.d/2900-single-file-audiobook-template.md new file mode 100644 index 000000000..08cce3104 --- /dev/null +++ b/changelog.d/2900-single-file-audiobook-template.md @@ -0,0 +1,2 @@ +### Fixed +- **A single-file audiobook is now named from the audiobook file template** (#2900): with `naming.audiobook_file_template` set, a folder of tracks was renamed but a lone `.m4b` kept its download name. It now gets the same name a track would, and so does a folder holding just one track. Put `{Part}` in a group with its own text, like `{Title}{ - Pt. Part:3}.{ext}`, and the group is left out when there is only one file, giving `The Shining.m4b`; written bare, as in `{Title} - Part {Part:3}.{ext}`, the single file is numbered as part 1. Settings now accepts `{Part}` inside such a group, which it used to reject. **Rename files** and the Fix Match preview propose the same name for a single-file audiobook already in the library. With no template set nothing changes. Thanks zikeji for the report. diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 0cb358f77..47b5d09b3 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -276,10 +276,23 @@ Two settings opt out of the merge: **Flatten multi-disc audiobooks** and a per-file audiobook naming template both keep the historical behaviour and place the audiobook in a sibling `Title (2)` folder. So does a download whose audiobook files do not share a folder of their own, which Bindery places file -by file. +by file. A lone audiobook file such as a single `.m4b` still merges with a +naming template set; it is named from the template, and a file already there +under that name is skipped the same way. (For handing files to Storyteller's *watch folder* instead, see [Handing off to another library tool](#handing-off-to-another-library-tool-cwa-calibre-storyteller).) +The per-file audiobook naming template (`naming.audiobook_file_template`) +renames a single-file audiobook too, whether it arrives as a lone `.m4b` or as +a folder holding one track, and **Rename files** proposes the same name for one +already in the library. `{Part}` decides how it reads. In a group with its own +text, such as `{Title}{ - Pt. Part:3}.{ext}`, the group is left out for a single +file (`The Shining.m4b`) and numbered for several (`Doctor Sleep - Pt. 001.m4b`). +Written bare, as in the default `{Title} - Part {Part:3}.{ext}`, the single file +is numbered as part 1 (`The Shining - Part 001.m4b`). Inside a group every word +that is a token name is read as the token, so write `Pt.` rather than `Part` for +the label there. + ### `BINDERY_DOWNLOAD_DIR` is not a watch folder A common misreading: `BINDERY_DOWNLOAD_DIR` does **not** mean "all completed diff --git a/internal/api/settings_handler.go b/internal/api/settings_handler.go index bc9ce6171..e3769cb99 100644 --- a/internal/api/settings_handler.go +++ b/internal/api/settings_handler.go @@ -17,6 +17,7 @@ import ( "github.com/vavallee/bindery/internal/calibre" "github.com/vavallee/bindery/internal/db" "github.com/vavallee/bindery/internal/httpsec" + "github.com/vavallee/bindery/internal/importer" "github.com/vavallee/bindery/internal/metadata/hardcover" "github.com/vavallee/bindery/internal/models" "github.com/vavallee/bindery/internal/pathmap" @@ -669,11 +670,14 @@ func validateSettingValue(key, value string) error { case SettingNamingAudiobookFileTemplate: // Empty disables per-file audiobook renaming (#1126). A non-empty // template MUST carry a {Part} token, otherwise every track flattens to - // the same filename and all but the last are dropped. + // the same filename and all but the last are dropped. The token may + // sit in a conditional group ("{Title}{ - Pt. Part:3}.{ext}"), which a + // single-file audiobook drops entirely (#2900), so this asks the + // renderer rather than looking for the substring "{Part". if value == "" { return nil } - if !strings.Contains(value, "{Part") { + if !importer.AudiobookTemplateHasPart(value) { return fmt.Errorf("naming.audiobook_file_template must include a {Part} token so each track gets a unique name") } case SettingImportDropLinkMode: diff --git a/internal/api/settings_handler_test.go b/internal/api/settings_handler_test.go index 9cb03ef0b..0c527d2c8 100644 --- a/internal/api/settings_handler_test.go +++ b/internal/api/settings_handler_test.go @@ -778,6 +778,9 @@ func TestValidateSettingValue_AudiobookFileTemplate(t *testing.T) { "{Title} - Part {Part:3}.{ext}", "{Part}.{ext}", "{Author} - {Title} - {Part:2}.{ext}", + // A conditional group, which a single-file audiobook drops (#2900). + "{Title}{ - Pt. Part:3}.{ext}", + "{Title}{ - Part:3}.{ext}", } { if err := validateSettingValue(SettingNamingAudiobookFileTemplate, v); err != nil { t.Errorf("%q should be accepted: %v", v, err) diff --git a/internal/importer/audiobook_naming_test.go b/internal/importer/audiobook_naming_test.go index c79d10650..40306fb11 100644 --- a/internal/importer/audiobook_naming_test.go +++ b/internal/importer/audiobook_naming_test.go @@ -55,7 +55,7 @@ func TestFlattenAudiobookDirNamed_Ordering(t *testing.T) { r := NewRenamer("") book := &models.Book{Title: "Recursion"} author := &models.Author{Name: "Blake Crouch"} - namer := func(index int, ext string) string { + namer := func(index, _ int, ext string) string { return r.AudiobookFileName("{Title} - Part {Part:3}.{ext}", author, book, "", "", trimDot(ext), index+1) } @@ -86,7 +86,7 @@ func TestFlattenAudiobookDirNamed_Ordering(t *testing.T) { func TestFlattenAudiobookDirNamed_DuplicateNameFails(t *testing.T) { src := buildMultiDiscTree(t) dst := filepath.Join(t.TempDir(), "out") - namer := func(index int, ext string) string { return "same" + ext } // no {Part} + namer := func(_, _ int, ext string) string { return "same" + ext } // no {Part} if err := flattenAudiobookDirNamed(context.Background(), "copy", src, dst, namer); err == nil { t.Error("expected a duplicate-name error when the namer omits Part") } diff --git a/internal/importer/flatten.go b/internal/importer/flatten.go index 12fe0f6bd..021bce25a 100644 --- a/internal/importer/flatten.go +++ b/internal/importer/flatten.go @@ -168,7 +168,7 @@ func isMultiDiscAudiobook(srcRoot string) bool { // no separators), so book-derived strings cannot escape the library. func flattenAudiobookDir(ctx context.Context, mode, srcRoot, destDir string) error { // Default "Part NNN.ext" naming: mirrors the historical multi-disc flatten. - return flattenAudiobookDirNamed(ctx, mode, srcRoot, destDir, func(index int, ext string) string { + return flattenAudiobookDirNamed(ctx, mode, srcRoot, destDir, func(index, _ int, ext string) string { return fmt.Sprintf("Part %03d%s", index+1, ext) }) } @@ -176,9 +176,11 @@ func flattenAudiobookDir(ctx context.Context, mode, srcRoot, destDir string) err // flattenAudiobookDirNamed is flattenAudiobookDir with a caller-supplied namer, // so both the built-in "Part NNN" multi-disc flatten and the opt-in per-file // audiobook naming template (#1126) share one deterministic placement path. -// nameFor receives the 0-based playback index and the lowercased source -// extension (with leading dot) and returns the destination basename. -func flattenAudiobookDirNamed(ctx context.Context, mode, srcRoot, destDir string, nameFor func(index int, ext string) string) error { +// nameFor receives the 0-based playback index, the number of tracks, and the +// lowercased source extension (with leading dot) and returns the destination +// basename. The count lets the template namer give a folder holding a single +// track the same name as a lone audiobook file (#2900). +func flattenAudiobookDirNamed(ctx context.Context, mode, srcRoot, destDir string, nameFor func(index, count int, ext string) string) error { if mode != "copy" && mode != "hardlink" { return fmt.Errorf("flatten supports copy/hardlink only, got %q", mode) } @@ -205,7 +207,7 @@ func flattenAudiobookDirNamed(ctx context.Context, mode, srcRoot, destDir string return err } ext := strings.ToLower(filepath.Ext(tr.src)) - name := filepath.Base(nameFor(i, ext)) + name := filepath.Base(nameFor(i, len(tracks), ext)) if _, clash := reserved[name]; clash { // A template that omits {Part} would collapse every track to one // name and silently drop all but the last. Fail loudly instead. diff --git a/internal/importer/renamer.go b/internal/importer/renamer.go index 9954e7904..6add41dd8 100644 --- a/internal/importer/renamer.go +++ b/internal/importer/renamer.go @@ -166,17 +166,81 @@ func (r *Renamer) DestPath(rootFolder string, author *models.Author, book *model // ext is the source extension WITHOUT a leading dot (the template supplies the // dot before {ext}, matching the ebook template convention). func (r *Renamer) AudiobookFileName(template string, author *models.Author, book *models.Book, series, seriesNumber, ext string, part int) string { + return r.audiobookFileName(template, author, book, series, seriesNumber, ext, strconv.Itoa(part)) +} + +// AudiobookSingleFileName renders the per-file audiobook naming template for +// an audiobook that is one audio file (#2900): a lone .m4b, or a folder holding +// a single track. It is the same render AudiobookFileName does for a track of a +// multi-file book, so the two differ only in the part. +// +// {Part} renders empty when every {Part} in the template sits in a conditional +// group with its own text, so the #1127 collapse drops the whole group: +// "{Title}{ - Pt. Part:3}.{ext}" gives "Title.m4b" here and "Title - Pt. 001.m4b" +// for each track of a multi-file book. A bare "{Part}" or "{Part:3}" has its +// glue outside the braces, and an empty value there leaves it dangling +// ("Title - Part .m4b" from the default template), so such a template numbers +// the lone file as part 1, exactly as it would the first of several. +func (r *Renamer) AudiobookSingleFileName(template string, author *models.Author, book *models.Book, series, seriesNumber, ext string) string { + if template == "" { + template = defaultAudiobookFileTemplate + } + part := "" + if !audiobookPartIsConditional(template) { + part = "1" + } + return r.audiobookFileName(template, author, book, series, seriesNumber, ext, part) +} + +func (r *Renamer) audiobookFileName(template string, author *models.Author, book *models.Book, series, seriesNumber, ext, part string) string { if template == "" { template = defaultAudiobookFileTemplate } name := r.applyWithExtra(template, author, book, series, seriesNumber, ext, map[string]string{ - "Part": strconv.Itoa(part), + "Part": part, }) // Defensive: collapse any separator a hand-edited template might introduce // so a track can never escape the destination directory. return filepath.Base(name) } +// audiobookPartIsConditional reports whether every {Part} in template is in a +// conditional group, one renderGroup collapses to "" when the part is empty. +// It is the negation of renderGroup's simple-form test: a group whose content +// is "Part" with an optional default or width modifier renders the bare value, +// so an empty part there cannot take its surrounding text with it. A template +// with no {Part} at all is trivially conditional. +func audiobookPartIsConditional(template string) bool { + for _, g := range templateGroupRe.FindAllStringSubmatch(template, -1) { + if m := simpleGroupRe.FindStringSubmatch(g[1]); m != nil && !widthThenLiteralRe.MatchString(m[2]) && m[1] == "Part" { + return false + } + } + return true +} + +// AudiobookTemplateHasPart reports whether template carries a {Part} token the +// renderer substitutes, either as a group of its own ("{Part:3}") or inside a +// conditional group with literal text ("{ - Pt. Part:3}"). The settings +// validator uses it so the conditional form a single-file audiobook collapses +// (#2900) can be saved; a plain substring test for "{Part" rejected it. +func AudiobookTemplateHasPart(template string) bool { + for _, g := range templateGroupRe.FindAllStringSubmatch(template, -1) { + if m := simpleGroupRe.FindStringSubmatch(g[1]); m != nil && !widthThenLiteralRe.MatchString(m[2]) { + if m[1] == "Part" { + return true + } + continue + } + for _, w := range groupWordRe.FindAllStringSubmatch(g[1], -1) { + if w[1] == "Part" { + return true + } + } + } + return false +} + // AudiobookDestDir computes the destination directory into which an audiobook // download folder should be moved. The download's internal file structure is // preserved inside (multi-part m4b/mp3 + cover + cue sheet stay together). diff --git a/internal/importer/reorganize.go b/internal/importer/reorganize.go index edb986264..023308a7b 100644 --- a/internal/importer/reorganize.go +++ b/internal/importer/reorganize.go @@ -164,7 +164,7 @@ func (s *Scanner) proposedPathFor(ctx context.Context, book *models.Book, author if flattenedOntoFolderPath(f.Path, dest) { return dest, ReorgStatusError, flattenedOntoFolderMsg } - dest = s.singleFileAudiobookDest(ctx, book, dest, f.Path) + dest = s.singleFileAudiobookDest(ctx, book, author, seriesTitle, seriesNum, dest, f.Path) } else { dest = uniqueDirExcluding(dest, f.Path) } @@ -214,12 +214,14 @@ func isSingleFile(path string) bool { // 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 { +// folder treated as available so a file already in place stays a noop. The +// file is named the way the import names it (#2900): from the audiobook file +// template when one is set, otherwise its own name. +func (s *Scanner) singleFileAudiobookDest(ctx context.Context, book *models.Book, author *models.Author, seriesTitle, seriesNum, 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)) + return filepath.Join(destDir, s.singleAudiobookFileName(ctx, author, book, seriesTitle, seriesNum, current)) } const flattenedOntoFolderMsg = "this audiobook file sits where its folder belongs and has lost its extension (#2894). " + diff --git a/internal/importer/scanner.go b/internal/importer/scanner.go index 54e401360..2997f3c0b 100644 --- a/internal/importer/scanner.go +++ b/internal/importer/scanner.go @@ -1789,7 +1789,13 @@ func (s *Scanner) tryImportInternal(ctx context.Context, dl *models.Download, do if mode == "move" { flattenMode = "copy" } - namer := func(index int, ext string) string { + namer := func(index, count int, ext string) string { + // A folder holding one track is a single-file + // audiobook too, so it gets the same name a lone + // .m4b does on the branch below (#2900). + if count == 1 { + return s.renamer.AudiobookSingleFileName(tmpl, author, book, seriesTitle, seriesNum, strings.TrimPrefix(ext, ".")) + } return s.renamer.AudiobookFileName(tmpl, author, book, seriesTitle, seriesNum, strings.TrimPrefix(ext, "."), index+1) } slog.Info("renaming audiobook files per template", "src", audiobookSource, "dst", destDir, "mode", flattenMode, "template", tmpl) @@ -1892,7 +1898,10 @@ func (s *Scanner) tryImportInternal(ctx context.Context, dl *models.Download, do if err := os.MkdirAll(destDir, 0o750); err != nil { dirErr = fmt.Errorf("create audiobook dest dir: %w", err) } else { - name := filepath.Base(audiobookSource) + name := s.singleAudiobookFileName(ctx, author, book, seriesTitle, seriesNum, audiobookSource) + if name != filepath.Base(audiobookSource) { + slog.Info("renaming audiobook file per template", "src", audiobookSource, "dst", destDir, "name", name, "mode", mode) + } dstFile := filepath.Join(destDir, name) // A merge never overwrites what is already in the book's // folder, the same contract CopyDirMergeCtx and friends diff --git a/internal/importer/scanner_handoff.go b/internal/importer/scanner_handoff.go index 7129ef3fc..6daf357ed 100644 --- a/internal/importer/scanner_handoff.go +++ b/internal/importer/scanner_handoff.go @@ -163,6 +163,29 @@ func (s *Scanner) audiobookFileTemplate(ctx context.Context) string { return strings.TrimSpace(setting.Value) } +// singleAudiobookFileName is the name a single-file audiobook takes inside its +// folder: the source file's own name, or, when naming.audiobook_file_template +// is set, that template rendered the way the folder branch renders a track, +// with {Part} left out as AudiobookSingleFileName describes (#2900). The +// import's single-file branch and Rename files both call it, so a reorganized +// file lands where a fresh import would put it. The extension is lowercased +// for the template, the same as flattenAudiobookDirNamed does for each track. +// A template that renders to nothing usable keeps the source name rather than +// placing a file called "." or with no name at all. +func (s *Scanner) singleAudiobookFileName(ctx context.Context, author *models.Author, book *models.Book, seriesTitle, seriesNum, src string) string { + name := filepath.Base(src) + tmpl := s.audiobookFileTemplate(ctx) + if tmpl == "" { + return name + } + ext := strings.TrimPrefix(strings.ToLower(filepath.Ext(src)), ".") + rendered := s.renamer.AudiobookSingleFileName(tmpl, author, book, seriesTitle, seriesNum, ext) + if rendered == "" || rendered == "." || rendered == ".." || rendered == string(filepath.Separator) { + return name + } + return rendered +} + // pushToCWA copies the just-imported file into the directory watched by a // sibling Calibre-Web-Automated container, when the cwa.ingest_path setting // is configured. CWA's auto-ingest deletes whatever lands in that folder diff --git a/internal/importer/single_file_audiobook_template_test.go b/internal/importer/single_file_audiobook_template_test.go new file mode 100644 index 000000000..be978034b --- /dev/null +++ b/internal/importer/single_file_audiobook_template_test.go @@ -0,0 +1,319 @@ +package importer + +import ( + "os" + "path/filepath" + "sort" + "testing" + + "github.com/vavallee/bindery/internal/models" +) + +// Tests for #2900: a single-file audiobook is named from +// naming.audiobook_file_template, the same as each track of a multi-file one. + +// conditionalPartTemplate puts {Part} in a #1127 conditional group, so the +// group, dash included, collapses when there is no part to number. "Pt." is +// used rather than "Part" because inside a group every word that is a token +// name is read as that token. +const conditionalPartTemplate = "{Author} - {Title}{ - Pt. Part:3}.{ext}" + +func TestAudiobookSingleFileName(t *testing.T) { + r := NewRenamer("") + author := &models.Author{Name: "Stephen King"} + book := &models.Book{Title: "The Shining"} + + cases := []struct { + name string + template string + want string + }{ + {"conditional part group collapses", conditionalPartTemplate, "Stephen King - The Shining.m4b"}, + {"conditional group without literal word", "{Title}{ - Part:3}.{ext}", "The Shining.m4b"}, + {"leading width then literal collapses", "{Part:3 - }{Title}.{ext}", "The Shining.m4b"}, + {"template without part tokens renders as written", "{Author} - {Title}.{ext}", "Stephen King - The Shining.m4b"}, + // A bare {Part} keeps its glue outside the braces. Rendering it empty + // would give "The Shining - Part .m4b", so the lone file is part 1. + {"unconditional part numbers the file as part one", "{Title} - Part {Part:3}.{ext}", "The Shining - Part 001.m4b"}, + {"empty template uses the default", "", "The Shining - Part 001.m4b"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := r.AudiobookSingleFileName(tc.template, author, book, "", "", "m4b"); got != tc.want { + t.Errorf("AudiobookSingleFileName(%q) = %q, want %q", tc.template, got, tc.want) + } + }) + } + + // The multi-file render of the same template differs only in the part. + if got, want := r.AudiobookFileName(conditionalPartTemplate, author, book, "", "", "m4b", 2), "Stephen King - The Shining - Pt. 002.m4b"; got != want { + t.Errorf("AudiobookFileName part 2 = %q, want %q", got, want) + } +} + +func TestAudiobookTemplateHasPart(t *testing.T) { + for tmpl, want := range map[string]bool{ + "{Title} - Part {Part:3}.{ext}": true, + "{Part}.{ext}": true, + conditionalPartTemplate: true, + "{Title}{ - Part:3}.{ext}": true, + "{Part:3 - }{Title}.{ext}": true, + "{Title}.{ext}": false, + "{Title} - Part.{ext}": false, // literal text outside a group is not a token + "{Title}{ - Parts Series}.{ext}": false, + } { + if got := AudiobookTemplateHasPart(tmpl); got != want { + t.Errorf("AudiobookTemplateHasPart(%q) = %v, want %v", tmpl, got, want) + } + } +} + +// importLoneAudiobook imports one audiobook whose source is the FILE itself +// (what a manual import of a lone .m4b resolves to) and returns the names in +// the folder the book records. A template of "" leaves the setting unset. +func importLoneAudiobook(t *testing.T, mode, template, srcName string) (folder string, names []string) { + t.Helper() + s, book, dlRepo, bookRepo, settingsRepo, _, ctx := sharedFormatFixture(t, t.TempDir()) + if err := settingsRepo.Set(ctx, "import.mode", mode); err != nil { + t.Fatal(err) + } + if template != "" { + if err := settingsRepo.Set(ctx, "naming.audiobook_file_template", template); err != nil { + t.Fatal(err) + } + } + src := filepath.Join(t.TempDir(), srcName) + if err := os.WriteFile(src, []byte("m4b-bytes"), 0o644); err != nil { + t.Fatal(err) + } + dl := &models.Download{GUID: "guid-2900-" + t.Name(), Title: "Jordan B. Peterson - We Who Wrestle with God [M4B]", BookID: &book.ID, Status: models.StateCompleted} + if err := dlRepo.Create(ctx, dl); err != nil { + t.Fatal(err) + } + s.ImportFromPath(ctx, dl, src, models.MediaTypeAudiobook) + got, err := dlRepo.GetByGUID(ctx, dl.GUID) + if err != nil { + t.Fatal(err) + } + if got.Status != models.StateImported { + t.Fatalf("import status = %q, want imported (error: %s)", got.Status, got.ErrorMessage) + } + files, err := bookRepo.ListFiles(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + for _, f := range files { + if f.Format == models.MediaTypeAudiobook { + folder = f.Path + } + } + return folder, dirNames(t, folder) +} + +func dirNames(t *testing.T, dir string) []string { + t.Helper() + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + var names []string + for _, e := range entries { + if filepath.Ext(e.Name()) != ".opf" { + names = append(names, e.Name()) + } + } + sort.Strings(names) + return names +} + +func TestImport_SingleFileAudiobookUsesFileTemplate(t *testing.T) { + for _, mode := range []string{"hardlink", "copy", "move"} { + t.Run(mode, func(t *testing.T) { + _, names := importLoneAudiobook(t, mode, conditionalPartTemplate, "wwwwg_cd-rip.M4B") + want := "Jordan B. Peterson - We Who Wrestle with God.m4b" + if len(names) != 1 || names[0] != want { + t.Errorf("folder holds %v, want [%s]", names, want) + } + }) + } +} + +func TestImport_SingleFileAudiobookUnconditionalPartIsPartOne(t *testing.T) { + _, names := importLoneAudiobook(t, "copy", "{Title} - Part {Part:3}.{ext}", "source.m4b") + if want := "We Who Wrestle with God - Part 001.m4b"; len(names) != 1 || names[0] != want { + t.Errorf("folder holds %v, want [%s]", names, want) + } +} + +func TestImport_SingleFileAudiobookWithoutTemplateKeepsSourceName(t *testing.T) { + _, names := importLoneAudiobook(t, "copy", "", "wwwwg_cd-rip.M4B") + if len(names) != 1 || names[0] != "wwwwg_cd-rip.M4B" { + t.Errorf("folder holds %v, want the source name kept", names) + } +} + +// importAudiobookFolder imports a download folder holding the given tracks. +func importAudiobookFolder(t *testing.T, template string, tracks ...string) []string { + t.Helper() + s, book, dlRepo, bookRepo, settingsRepo, _, ctx := sharedFormatFixture(t, t.TempDir()) + if err := settingsRepo.Set(ctx, "import.mode", "copy"); err != nil { + t.Fatal(err) + } + if err := settingsRepo.Set(ctx, "naming.audiobook_file_template", template); err != nil { + t.Fatal(err) + } + src := t.TempDir() + for _, name := range tracks { + if err := os.WriteFile(filepath.Join(src, name), []byte(name), 0o644); err != nil { + t.Fatal(err) + } + } + dl := &models.Download{GUID: "guid-2900-dir-" + t.Name(), Title: "Jordan B. Peterson - We Who Wrestle with God", BookID: &book.ID, Status: models.StateCompleted} + if err := dlRepo.Create(ctx, dl); err != nil { + t.Fatal(err) + } + s.ImportFromPath(ctx, dl, src, models.MediaTypeAudiobook) + if got, _ := dlRepo.GetByGUID(ctx, dl.GUID); got == nil || got.Status != models.StateImported { + t.Fatalf("folder import did not complete: %+v", got) + } + files, err := bookRepo.ListFiles(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + for _, f := range files { + if f.Format == models.MediaTypeAudiobook { + return dirNames(t, f.Path) + } + } + t.Fatal("no audiobook recorded") + return nil +} + +// A folder holding one track is the same audiobook as the lone file, so it +// must get the same name rather than depending on how the release was packed. +func TestImport_OneTrackFolderMatchesSingleFileName(t *testing.T) { + names := importAudiobookFolder(t, conditionalPartTemplate, "only.m4b") + if want := "Jordan B. Peterson - We Who Wrestle with God.m4b"; len(names) != 1 || names[0] != want { + t.Errorf("folder holds %v, want [%s]", names, want) + } +} + +// Multi-file naming is unchanged: every track is numbered, conditional or not. +func TestImport_MultiTrackFolderStillNumbersEveryTrack(t *testing.T) { + for tmpl, want := range map[string][]string{ + conditionalPartTemplate: { + "Jordan B. Peterson - We Who Wrestle with God - Pt. 001.m4b", + "Jordan B. Peterson - We Who Wrestle with God - Pt. 002.m4b", + }, + "{Title} - Part {Part:3}.{ext}": { + "We Who Wrestle with God - Part 001.m4b", + "We Who Wrestle with God - Part 002.m4b", + }, + } { + t.Run(tmpl, func(t *testing.T) { + names := importAudiobookFolder(t, tmpl, "01.m4b", "02.m4b") + if len(names) != len(want) || names[0] != want[0] || names[1] != want[1] { + t.Errorf("folder holds %v, want %v", names, want) + } + }) + } +} + +// A merge into the book's shared folder never overwrites a file already there +// (#2686). With a template that check is made against the templated name. +func TestImport_SingleFileAudiobookTemplatedNameCollisionIsSkipped(t *testing.T) { + for _, mode := range []string{"hardlink", "copy", "move"} { + t.Run(mode, func(t *testing.T) { + s, book, dlRepo, _, ctx, ebookDir := importSharedEbook(t, mode) + if err := s.settings.Set(ctx, "naming.audiobook_file_template", conditionalPartTemplate); err != nil { + t.Fatal(err) + } + existing := []byte("an m4b that was already here") + seeded := filepath.Join(ebookDir, "Jordan B. Peterson - We Who Wrestle with God.m4b") + if err := os.WriteFile(seeded, existing, 0o644); err != nil { + t.Fatal(err) + } + src := filepath.Join(t.TempDir(), "source.m4b") + if err := os.WriteFile(src, []byte("m4b-bytes"), 0o644); err != nil { + t.Fatal(err) + } + dl := &models.Download{GUID: "guid-2900-collide-" + t.Name(), Title: "We Who Wrestle with God [M4B]", BookID: &book.ID, Status: models.StateCompleted} + if err := dlRepo.Create(ctx, dl); err != nil { + t.Fatal(err) + } + s.ImportFromPath(ctx, dl, src, models.MediaTypeAudiobook) + if got, _ := dlRepo.GetByGUID(ctx, dl.GUID); got == nil || got.Status != models.StateImported { + t.Fatalf("import did not complete: %+v", got) + } + if got, err := os.ReadFile(seeded); err != nil || string(got) != string(existing) { + t.Errorf("the file already at the templated name was overwritten or removed (err %v)", err) + } + if _, err := os.Stat(filepath.Join(ebookDir, "source.m4b")); !os.IsNotExist(err) { + t.Errorf("the skipped file was placed under its source name instead: %v", err) + } + if _, err := os.Stat(src); err != nil { + t.Errorf("mode=%s: the source of a skipped file is gone: %v", mode, err) + } + }) + } +} + +// Rename files proposes the name an import would give, and apply puts the file +// exactly where the preview said. +func TestReorganize_SingleFileAudiobookUsesFileTemplate(t *testing.T) { + env, _, audiobookDir, ctx := reorgFixture(t) + if err := env.s.settings.Set(ctx, "naming.audiobook_file_template", conditionalPartTemplate); err != nil { + t.Fatal(err) + } + book := env.seed(t, ctx, "Jane Doe", "My Book") + oldPath := filepath.Join(audiobookDir, "Jane Doe", "my_book_rip.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)", "Jane Doe - 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 || results[0].Proposed != moves[0].Proposed { + t.Fatalf("apply = %+v, want moved to the previewed %q", results[0], moves[0].Proposed) + } + if _, err := os.Stat(want); err != nil { + t.Errorf("file not at the templated name: %v", err) + } + + // Once there it stays there. + again, err := env.s.PreviewReorganizeBook(ctx, book.ID) + if err != nil { + t.Fatal(err) + } + if len(again) != 1 || again[0].Status != ReorgStatusNoop { + t.Errorf("second preview = %+v, want noop", again) + } +} + +// Fix Match's destination preview routes through the same code, so it names +// the templated file too. +func TestPreviewImportDestination_SingleFileAudiobookUsesFileTemplate(t *testing.T) { + env, _, audiobookDir, ctx := reorgFixture(t) + if err := env.s.settings.Set(ctx, "naming.audiobook_file_template", conditionalPartTemplate); err != nil { + t.Fatal(err) + } + target := env.seed(t, ctx, "Jane Doe", "Right Book") + src := filepath.Join(t.TempDir(), "rip.m4b") + writeFileAt(t, src) + + got, err := env.s.PreviewImportDestination(ctx, target.ID, src, "") + if err != nil { + t.Fatal(err) + } + if want := filepath.Join(audiobookDir, "Jane Doe", "Right Book (2020)", "Jane Doe - Right Book.m4b"); got.Destination != want { + t.Errorf("destination = %q, want %q", got.Destination, want) + } +} diff --git a/web/src/pages/settings/GeneralTab.tsx b/web/src/pages/settings/GeneralTab.tsx index d06d82a7e..5f4ce1e81 100644 --- a/web/src/pages/settings/GeneralTab.tsx +++ b/web/src/pages/settings/GeneralTab.tsx @@ -393,7 +393,7 @@ export default function GeneralTab({ onNavigate }: GeneralTabProps = {}) { {t('settings.general.audiobookFileTemplate', 'Audiobook file naming (per track)')}

- {t('settings.general.audiobookFileTemplateHint', 'Leave empty to keep the download’s original file layout. Set a template to rename every audiobook track in playback order — it must include {Part}.')} + {t('settings.general.audiobookFileTemplateHint', 'Leave empty to keep the download’s original file layout. Set a template to rename every audiobook track in playback order; it must include {Part}. A single-file audiobook is renamed too, as part 1, unless {Part} sits in a group with its own text, such as {Title}{ - Pt. Part:3}.{ext}, which is left out when there is only one file.')}