fix(metadata): m4b text-track fallback; in-app confirm; save-first write
Three defects from real-world use: - The m4b rewrite assumed every stream survives -map 0 -c copy; files carrying text-ish tracks (the movenc 'text' fourcc) fail the ipod muxer with "Tag text incompatible with output codec id" — exactly the error a write attempt hit in the field. The rewrite now attempts full fidelity first, then retries mapping audio + attached pictures only (-dn -sn): chapters live in the container header (-map_chapters 0) so they survive; every attempt still ffprobe-validates before the atomic rename and the original is never touched on total failure. - The browser confirm() is replaced by an in-app dialog (WriteToFileConfirmDialog, Alpine + templ — no native dialogs). - "Write to file" now SAVES the pending editor edits first (persisted through the same PUT the Save button uses), then writes the file from those DB values, and closes the editor + refreshes the page exactly like Save — previously the endpoint wrote the LAST-SAVED DB values, so edits sitting in the modal never reached the file, and the page didn't visibly update. Test coverage per the owner's bar: EVERY single-file container (MP3, FLAC, M4A, M4B) gets its own runtime-generated fixture, gets written, and the FILE's metadata is then verified independently — ffprobe reads the rewritten container and every mapped key is asserted per format, including the subtitle round-trip split (MP3/FLAC yes, M4A/B no) — and the EPUB/CBZ/sidecar assertions now PARSE the written XML (dc titles/ creators, ComicInfo fields, calibre meta) instead of substring matching. A smart-genre unit test also pins the mixed-blob contract: a recognized shelf wins the genre column even when demoted format words ride along in the same blob.
This commit is contained in:
John O'Keefe committed
1 parent
96c1228805
commit
fb1645c90c
8 files changed
+396
-152
No files matched your search
@@ -5,6 +5,7 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"encoding/xml"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
@@ -114,33 +115,57 @@ func TestMetadataWriteToFile(t *testing.T) {
|
||||
writeEPUB(abs, "Original EPUB Title")
|
||||
itemID := insertItem(rel, "Original EPUB Title")
|
||||
|
||||
editItem(itemID, map[string]interface{}{"title": "Embedded New Title", "series": "Embedded Series", "series_number": 2})
|
||||
editItem(itemID, map[string]interface{}{
|
||||
"title": "Embedded New Title",
|
||||
"authors": []string{"Embedded Author"},
|
||||
"series": "Embedded Series",
|
||||
"series_number": 2,
|
||||
})
|
||||
|
||||
code, body := callWrite(setup.Token, itemID)
|
||||
require.Equal(t, http.StatusOK, code)
|
||||
assert.Equal(t, "embedded-opf", body["metadata_target"])
|
||||
|
||||
// The zip on disk now carries the new values, and the ORIGINAL
|
||||
// unique-identifier survives the splice.
|
||||
// The zip on disk now carries the new values — verified by
|
||||
// PARSING the rewritten OPF (not string matching), with the
|
||||
// ORIGINAL unique-identifier surviving the splice.
|
||||
reader, err := zip.OpenReader(abs)
|
||||
require.NoError(t, err)
|
||||
defer reader.Close()
|
||||
var opfContent string
|
||||
var opfContent []byte
|
||||
for _, f := range reader.File {
|
||||
if f.Name == "OEBPS/content.opf" {
|
||||
rc, err := f.Open()
|
||||
require.NoError(t, err)
|
||||
buf := new(strings.Builder)
|
||||
_, err = io.Copy(buf, rc)
|
||||
opfContent, err = io.ReadAll(rc)
|
||||
require.NoError(t, rc.Close())
|
||||
require.NoError(t, err)
|
||||
opfContent = buf.String()
|
||||
}
|
||||
}
|
||||
assert.Contains(t, opfContent, "Embedded New Title")
|
||||
assert.Contains(t, opfContent, "Embedded Series")
|
||||
assert.NotContains(t, opfContent, "Original EPUB Title")
|
||||
assert.Contains(t, opfContent, "urn:uuid:abcd", "original identifier preserved for unique-identifier")
|
||||
reader.Close()
|
||||
|
||||
var opf struct {
|
||||
Metadata struct {
|
||||
Titles []string `xml:"http://purl.org/dc/elements/1.1/ title"`
|
||||
Creators []string `xml:"http://purl.org/dc/elements/1.1/ creator"`
|
||||
Metas []struct {
|
||||
Name string `xml:"name,attr"`
|
||||
Content string `xml:"content,attr"`
|
||||
} `xml:"meta"`
|
||||
} `xml:"metadata"`
|
||||
}
|
||||
require.NoError(t, xml.Unmarshal(opfContent, &opf))
|
||||
require.NotEmpty(t, opf.Metadata.Titles, "dc:title present in written OPF")
|
||||
assert.Equal(t, "Embedded New Title", opf.Metadata.Titles[0])
|
||||
assert.NotContains(t, opf.Metadata.Titles, "Original EPUB Title", "old title replaced")
|
||||
require.NotEmpty(t, opf.Metadata.Creators)
|
||||
var seriesMeta string
|
||||
for _, m := range opf.Metadata.Metas {
|
||||
if m.Name == "calibre:series" {
|
||||
seriesMeta = m.Content
|
||||
}
|
||||
}
|
||||
assert.Equal(t, "Embedded Series", seriesMeta)
|
||||
assert.Contains(t, string(opfContent), "urn:uuid:abcd", "original identifier preserved for unique-identifier")
|
||||
|
||||
// The inline rescan re-extracted from the file: the DB title
|
||||
// still reads the written value.
|
||||
@@ -172,7 +197,17 @@ func TestMetadataWriteToFile(t *testing.T) {
|
||||
sidecar := filepath.Join(filepath.Dir(abs), "metadata.opf")
|
||||
data, err := os.ReadFile(sidecar)
|
||||
require.NoError(t, err, "metadata.opf must exist next to a lone book")
|
||||
assert.Contains(t, string(data), "Sidecar New Title")
|
||||
|
||||
// Parse the sidecar like the scanner does: title must be present
|
||||
// as a dc:title ELEMENT, not just a substring.
|
||||
var opf struct {
|
||||
Metadata struct {
|
||||
Titles []string `xml:"http://purl.org/dc/elements/1.1/ title"`
|
||||
} `xml:"metadata"`
|
||||
}
|
||||
require.NoError(t, xml.Unmarshal(data, &opf))
|
||||
require.NotEmpty(t, opf.Metadata.Titles)
|
||||
assert.Equal(t, "Sidecar New Title", opf.Metadata.Titles[0])
|
||||
})
|
||||
|
||||
t.Run("shared dir falls back to <basename>.opf", func(t *testing.T) {
|
||||
@@ -221,93 +256,115 @@ func TestMetadataWriteToFile(t *testing.T) {
|
||||
_ = itemB
|
||||
})
|
||||
|
||||
t.Run("m4b tags are rewritten via ffmpeg and survive rescan", func(t *testing.T) {
|
||||
rel := fmt.Sprintf("test-metadata-write-%d/audio/book.m4b", os.Getpid())
|
||||
abs := filepath.Join(uploadsDir, rel)
|
||||
require.NoError(t, os.MkdirAll(filepath.Dir(abs), 0o755))
|
||||
// Generate the fixture at test runtime (AGENTS: no binary media
|
||||
// in git): 2s of silence through the native AAC encoder, with
|
||||
// an original title tag to prove the rewrite replaces it.
|
||||
gen := exec.Command("ffmpeg", "-hide_banner", "-loglevel", "error", "-y",
|
||||
"-f", "lavfi", "-i", "anullsrc=r=44100:cl=mono",
|
||||
"-t", "2", "-c:a", "aac",
|
||||
"-metadata", "title=Generated Fixture",
|
||||
"-metadata", "album=Generated Fixture",
|
||||
abs)
|
||||
out, err := gen.CombinedOutput()
|
||||
require.NoError(t, err, "ffmpeg fixture generation failed: %s", string(out))
|
||||
|
||||
var itemID string
|
||||
err = setup.DBPool.QueryRow(ctx,
|
||||
`INSERT INTO media_items (library_id, title, file_path, format_group, mime_type, file_size, created_at, imported_at, updated_at)
|
||||
VALUES ($1, 'Generated Fixture', $2, 'audiobook', 'audio/mp4', 100, NOW(), NOW(), NOW()) RETURNING id`,
|
||||
libID, rel,
|
||||
).Scan(&itemID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Follow the real editor flow: PUT first (records
|
||||
// metadata_overrides, so the inline rescan keeps the edited
|
||||
// values instead of folder-structure derivations).
|
||||
editItem(itemID, map[string]interface{}{
|
||||
"title": "Audio New Title",
|
||||
"subtitle": "Audio Sub",
|
||||
"authors": []string{"Audio Author"},
|
||||
"narrators": []string{"Narrator Person"},
|
||||
"series": "Audio Series",
|
||||
"series_number": 1.5,
|
||||
"date_published": "1953-04-16",
|
||||
"tags": []string{"Science"},
|
||||
})
|
||||
|
||||
code, body := callWrite(setup.Token, itemID)
|
||||
require.Equal(t, http.StatusOK, code)
|
||||
assert.Equal(t, "audio-tags", body["metadata_target"])
|
||||
|
||||
// The container now carries the round-trip keys (probe with the
|
||||
// same tool the scanner uses).
|
||||
probe := func() map[string]string {
|
||||
out, err := exec.Command("ffprobe", "-v", "error", "-show_entries",
|
||||
"format_tags", "-of", "json", abs).Output()
|
||||
require.NoError(t, err)
|
||||
var parsed struct {
|
||||
Format struct {
|
||||
Tags map[string]string `json:"tags"`
|
||||
} `json:"format"`
|
||||
}
|
||||
require.NoError(t, json.Unmarshal(out, &parsed))
|
||||
return parsed.Format.Tags
|
||||
t.Run("audio tags rewrite round-trips for every container", func(t *testing.T) {
|
||||
// Each single-file container gets its own generated fixture
|
||||
// (ffmpeg at test runtime — no binary media in git), its own
|
||||
// write, and an INDEPENDENT verification pass: ffprobe the
|
||||
// rewritten file and assert every mapped key landed. This is the
|
||||
// "check the file's metadata yourself" contract per format:
|
||||
//
|
||||
// album/artist/composer/grouping/date/genre → all four
|
||||
// subtitle → mp3 + flac only
|
||||
// (the mp4 muxer
|
||||
// drops it)
|
||||
cases := []struct {
|
||||
ext string
|
||||
audioCodec string
|
||||
wantSubtitle bool
|
||||
}{
|
||||
{ext: ".mp3", audioCodec: "libmp3lame", wantSubtitle: true},
|
||||
{ext: ".flac", audioCodec: "flac", wantSubtitle: true},
|
||||
{ext: ".m4a", audioCodec: "aac", wantSubtitle: false},
|
||||
{ext: ".m4b", audioCodec: "aac", wantSubtitle: false},
|
||||
}
|
||||
tags := probe()
|
||||
assert.Equal(t, "Audio New Title", tags["album"], "the scanner's album rung")
|
||||
assert.Equal(t, "Audio New Title", tags["title"])
|
||||
// This ffmpeg's mp4 muxer drops subtitle keys — nothing that
|
||||
// can't be read back may be written (fields_written omits it).
|
||||
assert.Empty(t, tags["subtitle"], "subtitle must not be written where it can't round-trip")
|
||||
assert.Equal(t, "Audio Author", tags["artist"])
|
||||
assert.Equal(t, "Narrator Person", tags["composer"])
|
||||
assert.Equal(t, "Audio Series", tags["grouping"])
|
||||
assert.Equal(t, "Science", tags["genre"])
|
||||
assert.NotContains(t, tags["album"], "Generated Fixture", "old tag replaced")
|
||||
|
||||
writeMap := body["write"].(map[string]interface{})
|
||||
assert.NotContains(t, writeMap["fields_written"], "subtitle", "honest fields_written")
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.ext, func(t *testing.T) {
|
||||
rel := fmt.Sprintf("test-metadata-write-%d/audio-%s/book%s", os.Getpid(), strings.TrimPrefix(tc.ext, "."), tc.ext)
|
||||
abs := filepath.Join(uploadsDir, rel)
|
||||
require.NoError(t, os.MkdirAll(filepath.Dir(abs), 0o755))
|
||||
gen := exec.Command("ffmpeg", "-hide_banner", "-loglevel", "error", "-y",
|
||||
"-f", "lavfi", "-i", "anullsrc=r=44100:cl=mono",
|
||||
"-t", "2", "-c:a", tc.audioCodec,
|
||||
"-metadata", "title=Generated Fixture",
|
||||
"-metadata", "album=Generated Fixture",
|
||||
abs)
|
||||
if out, err := gen.CombinedOutput(); err != nil {
|
||||
t.Skipf("fixture generation failed for %s (%v): %s", tc.ext, err, string(out))
|
||||
}
|
||||
|
||||
// Still a valid, same-length audio file after the rewrite.
|
||||
dur := func() float64 {
|
||||
out, err := exec.Command("ffprobe", "-v", "error", "-show_entries",
|
||||
"format=duration", "-of", "default=noprint_wrappers=1:nokey=1", abs).Output()
|
||||
require.NoError(t, err)
|
||||
v, err := strconv.ParseFloat(strings.TrimSpace(string(out)), 64)
|
||||
require.NoError(t, err)
|
||||
return v
|
||||
var itemID string
|
||||
err := setup.DBPool.QueryRow(ctx,
|
||||
`INSERT INTO media_items (library_id, title, file_path, format_group, mime_type, file_size, created_at, imported_at, updated_at)
|
||||
VALUES ($1, 'Generated Fixture', $2, 'audiobook', $3, 100, NOW(), NOW(), NOW()) RETURNING id`,
|
||||
libID, rel, "audio/"+strings.TrimPrefix(tc.ext, "."),
|
||||
).Scan(&itemID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Follow the real editor flow: PUT first (records
|
||||
// metadata_overrides, so the inline rescan keeps the
|
||||
// edited values instead of folder-structure derivations).
|
||||
editItem(itemID, map[string]interface{}{
|
||||
"title": "Audio New Title",
|
||||
"subtitle": "Audio Sub",
|
||||
"authors": []string{"Audio Author"},
|
||||
"narrators": []string{"Narrator Person"},
|
||||
"series": "Audio Series",
|
||||
"series_number": 1.5,
|
||||
"date_published": "1953-04-16",
|
||||
"tags": []string{"Science"},
|
||||
})
|
||||
|
||||
code, body := callWrite(setup.Token, itemID)
|
||||
require.Equal(t, http.StatusOK, code, "%s write", tc.ext)
|
||||
assert.Equal(t, "audio-tags", body["metadata_target"], tc.ext)
|
||||
|
||||
// ---- verify the FILE's metadata independently ----
|
||||
probe := func() map[string]string {
|
||||
out, err := exec.Command("ffprobe", "-v", "error", "-show_entries",
|
||||
"format_tags", "-of", "json", abs).Output()
|
||||
require.NoError(t, err)
|
||||
var parsed struct {
|
||||
Format struct {
|
||||
Tags map[string]string `json:"tags"`
|
||||
} `json:"format"`
|
||||
}
|
||||
require.NoError(t, json.Unmarshal(out, &parsed))
|
||||
return parsed.Format.Tags
|
||||
}
|
||||
tags := probe()
|
||||
assert.Equal(t, "Audio New Title", tags["album"], "%s: the scanner's album rung", tc.ext)
|
||||
assert.Equal(t, "Audio New Title", tags["title"], tc.ext)
|
||||
assert.Equal(t, "Audio Author", tags["artist"], tc.ext)
|
||||
assert.Equal(t, "Narrator Person", tags["composer"], "%s: narrators ride composer", tc.ext)
|
||||
assert.Equal(t, "Audio Series", tags["grouping"], tc.ext)
|
||||
assert.Equal(t, "Science", tags["genre"], tc.ext)
|
||||
assert.Equal(t, "1953-04-16", tags["date"], tc.ext)
|
||||
assert.NotContains(t, tags["album"], "Generated Fixture", "%s: old tag replaced", tc.ext)
|
||||
if tc.wantSubtitle {
|
||||
assert.Equal(t, "Audio Sub", tags["subtitle"], "%s: subtitle round-trips", tc.ext)
|
||||
} else {
|
||||
assert.Empty(t, tags["subtitle"], "%s: subtitle must not be written where it can't round-trip", tc.ext)
|
||||
writeMap := body["write"].(map[string]interface{})
|
||||
assert.NotContains(t, writeMap["fields_written"], "subtitle", "%s: honest fields_written", tc.ext)
|
||||
}
|
||||
|
||||
// Still valid, same-length audio after the rewrite.
|
||||
durOut, err := exec.Command("ffprobe", "-v", "error", "-show_entries",
|
||||
"format=duration", "-of", "default=noprint_wrappers=1:nokey=1", abs).Output()
|
||||
require.NoError(t, err)
|
||||
dur, err := strconv.ParseFloat(strings.TrimSpace(string(durOut)), 64)
|
||||
require.NoError(t, err)
|
||||
assert.Greater(t, dur, 1.5, "%s: audio survived the rewrite", tc.ext)
|
||||
|
||||
// The inline rescan re-read the tags: the DB title stays
|
||||
// the written one.
|
||||
var title string
|
||||
require.NoError(t, setup.DBPool.QueryRow(ctx,
|
||||
`SELECT title FROM media_items WHERE id = $1`, itemID).Scan(&title))
|
||||
assert.Equal(t, "Audio New Title", title, tc.ext)
|
||||
})
|
||||
}
|
||||
assert.Greater(t, dur(), 1.5, "audio survived the rewrite")
|
||||
|
||||
// The inline rescan re-read the tags: DB title stays the written one.
|
||||
var title string
|
||||
require.NoError(t, setup.DBPool.QueryRow(ctx,
|
||||
`SELECT title FROM media_items WHERE id = $1`, itemID).Scan(&title))
|
||||
assert.Equal(t, "Audio New Title", title)
|
||||
})
|
||||
|
||||
t.Run("CBZ gets ComicInfo.xml injected and rescan keeps it", func(t *testing.T) {
|
||||
@@ -344,24 +401,33 @@ func TestMetadataWriteToFile(t *testing.T) {
|
||||
require.Equal(t, http.StatusOK, code)
|
||||
assert.Equal(t, "comicinfo", body["metadata_target"])
|
||||
|
||||
// ---- verify the FILE's ComicInfo.xml by parsing it ----
|
||||
reader, err := zip.OpenReader(abs)
|
||||
require.NoError(t, err)
|
||||
var infoXML string
|
||||
var infoData []byte
|
||||
for _, f := range reader.File {
|
||||
if f.Name == "ComicInfo.xml" {
|
||||
rc, err := f.Open()
|
||||
require.NoError(t, err)
|
||||
data, err := io.ReadAll(rc)
|
||||
infoData, err = io.ReadAll(rc)
|
||||
require.NoError(t, rc.Close())
|
||||
require.NoError(t, err)
|
||||
infoXML = string(data)
|
||||
}
|
||||
}
|
||||
reader.Close()
|
||||
assert.Contains(t, infoXML, "<Title>Comic New Title</Title>")
|
||||
assert.Contains(t, infoXML, "<Series>Comic Series</Series>")
|
||||
assert.Contains(t, infoXML, "<Number>4</Number>")
|
||||
assert.Contains(t, infoXML, "<Writer>Comic Writer</Writer>")
|
||||
require.NotEmpty(t, infoData, "ComicInfo.xml must exist inside the CBZ")
|
||||
|
||||
var info struct {
|
||||
Title string `xml:"Title"`
|
||||
Series string `xml:"Series"`
|
||||
Number int `xml:"Number"`
|
||||
Writer string `xml:"Writer"`
|
||||
}
|
||||
require.NoError(t, xml.Unmarshal(infoData, &info))
|
||||
assert.Equal(t, "Comic New Title", info.Title)
|
||||
assert.Equal(t, "Comic Series", info.Series)
|
||||
assert.Equal(t, 4, info.Number)
|
||||
assert.Equal(t, "Comic Writer", info.Writer)
|
||||
|
||||
// The inline rescan reads the ComicInfo back: the DB title and
|
||||
// series stay the written values.
|
||||
|
||||
Reference in new issue
Block a user