From 293a627358553c9f61802caf9d4bc76af9c2e239 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Mon, 21 Sep 2026 06:50:42 +0000 Subject: podkit: playlist URL to private podcast feed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One Go binary with fetch / tag / feed / serve / all. The stages run separately on purpose: the download is the slow part and should not be repeated while adjusting titles or the base URL. Built from a shell + Python + Go sketch. The pieces that needed fixing on the way, all of which produce a feed that loads wrong or not at all: - Enclosure URLs were unescaped. Track titles routinely contain spaces and ampersands, which make both the URL and the XML invalid — the likeliest way for a feed to fail to load entirely. Only the final path segment is escaped; escaping the base would mangle scheme and host. - Ordering came from filename sort, which is correct only while zero-padding is applied consistently. One unpadded file puts lecture 10 after lecture 1. Order now comes from the parsed track number. - No pubDate and no itunes:type. Podcast clients default to newest-first, which starts a lecture course at the final lecture. Items now carry distinct ascending dates and the feed is marked serial. - Missing channel description and link, which several clients reject outright. - A struct field with no xml tag emitted a stray element. - guid had no isPermaLink="false", so a reader may treat it as a fetchable URL. - Errors from ReadDir, Info and Marshal were discarded, so a missing directory produced an empty feed that looked like a working setup with nothing in it. - os.Args indexed without a length check. Beyond the sketch: a download archive so an interrupted series resumes, an up-front check for yt-dlp and ffmpeg before the long download rather than a post-processing failure after the bytes are on disk, non-audio files excluded by extension so cover art and .part files cannot become episodes, and a serve command with token gating that 404s rather than 401s. Found by the first live run against a LibriVox collection: sources whose own titles begin with a track number came out as "01 - 01 - April.mp3", because the output template prepends the playlist index regardless. Collapsed, but only when the two numbers agree — "01 - 1964 - Lecture" is a year and "05 - 12 Angry Men" is a title, and neither is a repeat. 47 tests. Verified end to end against public-domain audio: downloaded, converted, tagged, served, and the delivered file is a playable 128kbps MP3 with correct ID3 title/artist/album/track frames. --- internal/tag/tag_test.go | 158 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 internal/tag/tag_test.go (limited to 'internal/tag/tag_test.go') diff --git a/internal/tag/tag_test.go b/internal/tag/tag_test.go new file mode 100644 index 0000000..f722cb6 --- /dev/null +++ b/internal/tag/tag_test.go @@ -0,0 +1,158 @@ +package tag + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// A minimal but genuinely valid MP3: one silent MPEG-1 Layer III frame. +// id3v2 needs a real file to append a tag to, and a zero-byte file is not one. +func writeMP3(t *testing.T, dir, name string) string { + t.Helper() + // 0xFF 0xFB = MPEG-1 Layer III, no CRC; 0x90 = 128kbps 44.1kHz; then padding. + frame := []byte{0xFF, 0xFB, 0x90, 0x00} + body := append(frame, make([]byte, 413)...) + p := filepath.Join(dir, name) + if err := os.WriteFile(p, body, 0o644); err != nil { + t.Fatalf("writing %s: %v", name, err) + } + return p +} + +func TestWriteDir_TagsEachFile(t *testing.T) { + dir := t.TempDir() + writeMP3(t, dir, "01 - The Law of Gravitation.mp3") + writeMP3(t, dir, "02 - The Relation of Mathematics to Physics.mp3") + + results, err := WriteDir(dir, Metadata{ + Album: "Character of Physical Law", + Artist: "Richard Feynman", + }) + if err != nil { + t.Fatalf("WriteDir: %v", err) + } + if len(results) != 2 { + t.Fatalf("got %d results, want 2", len(results)) + } + for _, r := range results { + if r.Err != nil { + t.Errorf("%s: %v", r.File, r.Err) + } + } + + title, track, ok := Read(filepath.Join(dir, "01 - The Law of Gravitation.mp3")) + if !ok { + t.Fatal("tags were not readable after writing") + } + if title != "The Law of Gravitation" { + t.Errorf("title = %q", title) + } + if track != 1 { + t.Errorf("track = %d, want 1", track) + } +} + +// Read is handed to the feed generator, which relies on ok=false to fall back +// to the filename. An untagged file is the normal state of a fresh download, +// not an error. +func TestRead_UntaggedFileReportsNotOK(t *testing.T) { + dir := t.TempDir() + p := writeMP3(t, dir, "01 - Untagged.mp3") + if _, _, ok := Read(p); ok { + t.Error("an untagged file should report ok=false") + } +} + +func TestRead_MissingFileReportsNotOK(t *testing.T) { + if _, _, ok := Read(filepath.Join(t.TempDir(), "nope.mp3")); ok { + t.Error("a missing file should report ok=false, not panic") + } +} + +// Track frames are commonly "3/7". Taking the whole string would make the +// number unparseable and lose the ordering. +func TestRead_ParsesTrackOfTotal(t *testing.T) { + dir := t.TempDir() + writeMP3(t, dir, "03 - Third.mp3") + writeMP3(t, dir, "01 - First.mp3") + writeMP3(t, dir, "02 - Second.mp3") + + if _, err := WriteDir(dir, Metadata{Album: "A"}); err != nil { + t.Fatalf("WriteDir: %v", err) + } + _, track, ok := Read(filepath.Join(dir, "03 - Third.mp3")) + if !ok { + t.Fatal("tags not readable") + } + if track != 3 { + t.Errorf("track = %d, want 3 (from a \"3/3\" frame)", track) + } +} + +// ID3v2 is an MP3 convention; writing it into other containers produces +// something no player reads, so other formats are skipped rather than corrupted. +func TestWriteDir_SkipsNonMP3(t *testing.T) { + dir := t.TempDir() + writeMP3(t, dir, "01 - One.mp3") + if err := os.WriteFile(filepath.Join(dir, "02 - Two.opus"), []byte("not an mp3"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("jpeg"), 0o644); err != nil { + t.Fatal(err) + } + + results, err := WriteDir(dir, Metadata{Album: "A"}) + if err != nil { + t.Fatalf("WriteDir: %v", err) + } + if len(results) != 1 || !strings.HasSuffix(results[0].File, ".mp3") { + t.Errorf("got %+v, want only the mp3", results) + } +} + +func TestWriteDir_MissingDirectoryIsAnError(t *testing.T) { + if _, err := WriteDir(filepath.Join(t.TempDir(), "nope"), Metadata{}); err == nil { + t.Error("expected an error for a missing directory") + } +} + +// A file with no track prefix should still get its metadata; only the track +// number is absent. +func TestWriteDir_HandlesUnnumberedFiles(t *testing.T) { + dir := t.TempDir() + writeMP3(t, dir, "Introduction.mp3") + + results, err := WriteDir(dir, Metadata{Album: "Course", Artist: "Someone"}) + if err != nil { + t.Fatalf("WriteDir: %v", err) + } + if results[0].Err != nil { + t.Fatalf("tagging failed: %v", results[0].Err) + } + title, track, ok := Read(filepath.Join(dir, "Introduction.mp3")) + if !ok || title != "Introduction" { + t.Errorf("title = %q ok = %v, want the filename stem", title, ok) + } + if track != 0 { + t.Errorf("track = %d, want 0 for an unnumbered file", track) + } +} + +// Re-running the tagger must be safe; the pipeline is meant to be resumable. +func TestWriteDir_IsIdempotent(t *testing.T) { + dir := t.TempDir() + writeMP3(t, dir, "01 - One.mp3") + md := Metadata{Album: "A", Artist: "B"} + + for i := 0; i < 3; i++ { + if _, err := WriteDir(dir, md); err != nil { + t.Fatalf("pass %d: %v", i, err) + } + } + title, track, ok := Read(filepath.Join(dir, "01 - One.mp3")) + if !ok || title != "One" || track != 1 { + t.Errorf("after 3 passes: title=%q track=%d ok=%v", title, track, ok) + } +} -- cgit v1.2.3