refactor podcast schema and generate unique episode paths (#373)

closes #350

db/migrations.go

@@ -5,11 +5,13 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"os"
 	"path/filepath"
 	"strings"
 	"time"
 
 	"github.com/jinzhu/gorm"
+	"go.senan.xyz/gonic/fileutil"
 	"go.senan.xyz/gonic/playlist"
 	"go.senan.xyz/gonic/server/ctrlsubsonic/specid"
 	"gopkg.in/gormigrate.v1"
@@ -59,6 +61,7 @@ func (db *DB) Migrate(ctx MigrationContext) error {
 		construct(ctx, "202307281628", migrateAlbumArtistsMany2Many),
 		construct(ctx, "202309070009", migrateDeleteArtistCoverField),
 		construct(ctx, "202309131743", migrateArtistInfo),
+		construct(ctx, "202309161411", migratePlaylistsPaths),
 	}
 
 	return gormigrate.
@@ -478,11 +481,11 @@ func migratePlaylistsToM3U(tx *gorm.DB, ctx MigrationContext) error {
 			return track.AbsPath()
 		case specid.PodcastEpisode:
 			var pe PodcastEpisode
-			tx.Where("id=?", id.Value).Find(&pe)
+			tx.Where("id=?", id.Value).Preload("Podcast").Find(&pe)
-			if pe.Path == "" {
+			if pe.Filename == "" {
 				return ""
 			}
-			return filepath.Join(ctx.PodcastsPath, pe.Path)
+			return filepath.Join(ctx.PodcastsPath, fileutil.Safe(pe.Podcast.Title), pe.Filename)
 		}
 		return ""
 	}
@@ -613,3 +616,97 @@ func migrateArtistInfo(tx *gorm.DB, _ MigrationContext) error {
 	).
 		Error
 }
+
+func migratePlaylistsPaths(tx *gorm.DB, ctx MigrationContext) error {
+	if !tx.Dialect().HasColumn("podcast_episodes", "path") {
+		return nil
+	}
+	if !tx.Dialect().HasColumn("podcasts", "image_path") {
+		return nil
+	}
+
+	step := tx.Exec(`
+			ALTER TABLE podcasts RENAME COLUMN image_path TO image
+	`)
+	if err := step.Error; err != nil {
+		return fmt.Errorf("step drop podcast_episodes path: %w", err)
+	}
+
+	step = tx.AutoMigrate(
+		Podcast{},
+		PodcastEpisode{},
+	)
+	if err := step.Error; err != nil {
+		return fmt.Errorf("step auto migrate: %w", err)
+	}
+
+	var podcasts []*Podcast
+	if err := tx.Find(&podcasts).Error; err != nil {
+		return fmt.Errorf("step load: %w", err)
+	}
+
+	for _, p := range podcasts {
+		p.Image = filepath.Base(p.Image)
+		if err := tx.Save(p).Error; err != nil {
+			return fmt.Errorf("saving podcast for cover %d: %w", p.ID, err)
+		}
+
+		oldPath, err := fileutil.First(
+			filepath.Join(ctx.PodcastsPath, fileutil.Safe(p.Title)),
+			filepath.Join(ctx.PodcastsPath, strings.ReplaceAll(p.Title, string(filepath.Separator), "_")), // old safe func
+			filepath.Join(ctx.PodcastsPath, p.Title),
+		)
+		if err != nil {
+			return fmt.Errorf("find old podcast path: %w", err)
+		}
+		newPath := filepath.Join(ctx.PodcastsPath, fileutil.Safe(p.Title))
+		p.RootDir = newPath
+		if err := tx.Save(p).Error; err != nil {
+			return fmt.Errorf("saving podcast %d: %w", p.ID, err)
+		}
+		if oldPath == newPath {
+			continue
+		}
+		if err := os.Rename(oldPath, newPath); err != nil {
+			return fmt.Errorf("rename podcast path: %w", err)
+		}
+	}
+
+	var podcastEpisodes []*PodcastEpisode
+	if err := tx.Preload("Podcast").Find(&podcastEpisodes, "status=? OR status=?", PodcastEpisodeStatusCompleted, PodcastEpisodeStatusDownloading).Error; err != nil {
+		return fmt.Errorf("step load: %w", err)
+	}
+	for _, pe := range podcastEpisodes {
+		if pe.Filename == "" {
+			continue
+		}
+		oldPath, err := fileutil.First(
+			filepath.Join(pe.Podcast.RootDir, fileutil.Safe(pe.Filename)),
+			filepath.Join(pe.Podcast.RootDir, strings.ReplaceAll(pe.Filename, string(filepath.Separator), "_")), // old safe func
+			filepath.Join(pe.Podcast.RootDir, pe.Filename),
+		)
+		if err != nil {
+			return fmt.Errorf("find old podcast episode path: %w", err)
+		}
+		newName := fileutil.Safe(filepath.Base(oldPath))
+		pe.Filename = newName
+		if err := tx.Save(pe).Error; err != nil {
+			return fmt.Errorf("saving podcast episode %d: %w", pe.ID, err)
+		}
+		newPath := filepath.Join(pe.Podcast.RootDir, newName)
+		if oldPath == newPath {
+			continue
+		}
+		if err := os.Rename(oldPath, newPath); err != nil {
+			return fmt.Errorf("rename podcast episode path: %w", err)
+		}
+	}
+
+	step = tx.Exec(`
+			ALTER TABLE podcast_episodes DROP COLUMN path
+	`)
+	if err := step.Error; err != nil {
+		return fmt.Errorf("step drop podcast_episodes path: %w", err)
+	}
+	return nil
+}

db/model.go

@@ -8,7 +8,6 @@ package db
 
 import (
 	"fmt"
-	"path"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -134,7 +133,7 @@ func (t *Track) AbsPath() string {
 	if t.Album == nil {
 		return ""
 	}
-	return path.Join(
+	return filepath.Join(
 		t.Album.RootDir,
 		t.Album.LeftPath,
 		t.Album.RightPath,
@@ -146,7 +145,7 @@ func (t *Track) RelPath() string {
 	if t.Album == nil {
 		return ""
 	}
-	return path.Join(
+	return filepath.Join(
 		t.Album.LeftPath,
 		t.Album.RightPath,
 		t.Filename,
@@ -354,10 +353,11 @@ type Podcast struct {
 	Title        string
 	Description  string
 	ImageURL     string
-	ImagePath    string
+	Image        string
 	Error        string
 	Episodes     []*PodcastEpisode
 	AutoDownload PodcastAutoDownload
+	RootDir      string
 }
 
 func (p *Podcast) SID() *specid.ID {
@@ -387,11 +387,10 @@ type PodcastEpisode struct {
 	Bitrate     int
 	Length      int
 	Size        int
-	Path        string
 	Filename    string
 	Status      PodcastEpisodeStatus
 	Error       string
-	AbsP        string `gorm:"-"` // TODO: not this. instead we need some consistent way to get the AbsPath for both tracks and podcast episodes. or just files in general
+	Podcast     *Podcast
 }
 
 func (pe *PodcastEpisode) AudioLength() int  { return pe.Length }
@@ -418,7 +417,10 @@ func (pe *PodcastEpisode) MIME() string {
 }
 
 func (pe *PodcastEpisode) AbsPath() string {
-	return pe.AbsP
+	if pe.Podcast == nil {
+		return ""
+	}
+	return filepath.Join(pe.Podcast.RootDir, pe.Filename)
 }
 
 type Bookmark struct {

fileutil/fileutil.go

@@ -0,0 +1,56 @@
+// TODO: this package shouldn't really exist. we can usually just attempt our normal filesystem operations
+// and handle errors atomically. eg.
+// - Safe could instead be try create file, handle error
+// - Unique could be try create file, on err create file (1), etc
+package fileutil
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+)
+
+var nonAlphaNumExpr = regexp.MustCompile("[^a-zA-Z0-9_.]+")
+
+func Safe(filename string) string {
+	filename = nonAlphaNumExpr.ReplaceAllString(filename, "")
+	return filename
+}
+
+// try to find a unqiue file (or dir) name. incrementing like "example (1)"
+func Unique(base, filename string) (string, error) {
+	return unique(base, filename, 0)
+}
+
+func unique(base, filename string, count uint) (string, error) {
+	var suffix string
+	if count > 0 {
+		suffix = fmt.Sprintf(" (%d)", count)
+	}
+	path := base + suffix
+	if filename != "" {
+		noExt := strings.TrimSuffix(filename, filepath.Ext(filename))
+		path = filepath.Join(base, noExt+suffix+filepath.Ext(filename))
+	}
+	_, err := os.Stat(path)
+	if os.IsNotExist(err) {
+		return path, nil
+	}
+	if err != nil {
+		return "", err
+	}
+	return unique(base, filename, count+1)
+}
+
+func First(path ...string) (string, error) {
+	var err error
+	for _, p := range path {
+		_, err = os.Stat(p)
+		if err == nil {
+			return p, nil
+		}
+	}
+	return "", err
+}

fileutil/fileutil_test.go

@@ -0,0 +1,56 @@
+package fileutil
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestUniquePath(t *testing.T) {
+	unq := func(base, filename string, count uint) string {
+		r, err := unique(base, filename, count)
+		require.NoError(t, err)
+		return r
+	}
+
+	require.Equal(t, "test/wow.mp3", unq("test", "wow.mp3", 0))
+	require.Equal(t, "test/wow (1).mp3", unq("test", "wow.mp3", 1))
+	require.Equal(t, "test/wow (2).mp3", unq("test", "wow.mp3", 2))
+
+	require.Equal(t, "test", unq("test", "", 0))
+	require.Equal(t, "test (1)", unq("test", "", 1))
+
+	base := filepath.Join(t.TempDir(), "a")
+
+	require.NoError(t, os.MkdirAll(base, os.ModePerm))
+
+	next := base + " (1)"
+	require.Equal(t, next, unq(base, "", 0))
+
+	require.NoError(t, os.MkdirAll(next, os.ModePerm))
+
+	next = base + " (2)"
+	require.Equal(t, next, unq(base, "", 0))
+
+	_, err := os.Create(filepath.Join(base, "test.mp3"))
+	require.NoError(t, err)
+	require.Equal(t, filepath.Join(base, "test (1).mp3"), unq(base, "test.mp3", 0))
+}
+
+func TestFirst(t *testing.T) {
+	base := t.TempDir()
+	name := filepath.Join(base, "test")
+	_, err := os.Create(name)
+	require.NoError(t, err)
+
+	p := func(name string) string {
+		return filepath.Join(base, name)
+	}
+
+	r, err := First(p("one"), p("two"), p("test"), p("four"))
+	require.NoError(t, err)
+	require.Equal(t, p("test"), r)
+
+}

podcasts/podcasts.go

@@ -10,7 +10,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -19,6 +18,7 @@ import (
 	"github.com/mmcdole/gofeed"
 
 	"go.senan.xyz/gonic/db"
+	"go.senan.xyz/gonic/fileutil"
 	"go.senan.xyz/gonic/mime"
 	"go.senan.xyz/gonic/scanner/tags"
 )
@@ -35,45 +35,6 @@ type Podcasts struct {
 }
 
 func New(db *db.DB, base string, tagger tags.Reader) *Podcasts {
-	// Walk podcast path making filenames safe. Phase 1: Files
-	err := filepath.WalkDir(base, func(path string, d os.DirEntry, err error) error {
-		if (path == base) || d.IsDir() {
-			return nil
-		}
-		if err != nil {
-			return err
-		}
-		localBase := d.Name()
-		dir := filepath.Dir(path)
-		safeBase := safeFilename(localBase)
-		if localBase == safeBase {
-			return nil
-		}
-		return os.Rename(strings.Join([]string{dir, localBase}, "/"), strings.Join([]string{dir, safeBase}, "/"))
-	})
-	if err != nil {
-		log.Fatalf("error making podcast episode filenames safe: %v\n", err)
-	}
-	// Phase 2: Directories
-	err = filepath.WalkDir(base, func(path string, d os.DirEntry, err error) error {
-		var pathError *os.PathError
-		if (path == base) || !d.IsDir() || errors.As(err, &pathError) { // Spurious path errors after move
-			return nil
-		}
-		if err != nil {
-			return err
-		}
-		localBase := d.Name()
-		dir := filepath.Dir(path)
-		safeBase := safeFilename(localBase)
-		if localBase == safeBase {
-			return nil
-		}
-		return os.Rename(strings.Join([]string{dir, localBase}, "/"), strings.Join([]string{dir, safeBase}, "/"))
-	})
-	if err != nil {
-		log.Fatalf("error making podcast directory names safe: %v\n", err)
-	}
 	return &Podcasts{
 		db:      db,
 		baseDir: base,
@@ -132,15 +93,19 @@ func (p *Podcasts) GetNewestPodcastEpisodes(count int) ([]*db.PodcastEpisode, er
 }
 
 func (p *Podcasts) AddNewPodcast(rssURL string, feed *gofeed.Feed) (*db.Podcast, error) {
+	rootDir, err := fileutil.Unique(filepath.Join(p.baseDir, fileutil.Safe(feed.Title)), "")
+	if err != nil {
+		return nil, fmt.Errorf("find unique podcast dir: %w", err)
+
+	}
 	podcast := db.Podcast{
 		Description: feed.Description,
 		ImageURL:    feed.Image.URL,
 		Title:       feed.Title,
 		URL:         rssURL,
+		RootDir:     rootDir,
 	}
-	podPath := absPath(p.baseDir, &podcast)
+	if err := os.Mkdir(podcast.RootDir, 0755); err != nil && !os.IsExist(err) {
-	err := os.Mkdir(podPath, 0755)
-	if err != nil && !os.IsExist(err) {
 		return nil, err
 	}
 	if err := p.db.Save(&podcast).Error; err != nil {
@@ -150,7 +115,7 @@ func (p *Podcasts) AddNewPodcast(rssURL string, feed *gofeed.Feed) (*db.Podcast,
 		return nil, err
 	}
 	go func() {
-		if err := p.downloadPodcastCover(podPath, &podcast); err != nil {
+		if err := p.downloadPodcastCover(&podcast); err != nil {
 			log.Printf("error downloading podcast cover: %v", err)
 		}
 	}()
@@ -379,6 +344,7 @@ func (p *Podcasts) DownloadEpisode(episodeID int) error {
 	podcastEpisode := db.PodcastEpisode{}
 	podcast := db.Podcast{}
 	err := p.db.
+		Preload("Podcast").
 		Where("id=?", episodeID).
 		First(&podcastEpisode).
 		Error
@@ -417,13 +383,16 @@ func (p *Podcasts) DownloadEpisode(episodeID int) error {
 		}
 		filename = path.Base(audioURL.Path)
 	}
-	filename = p.findUniqueEpisodeName(&podcast, &podcastEpisode, safeFilename(filename))
+	path, err := fileutil.Unique(podcast.RootDir, fileutil.Safe(filename))
-	audioFile, err := os.Create(path.Join(absPath(p.baseDir, &podcast), filename))
+	if err != nil {
+		return fmt.Errorf("find unique path: %w", err)
+	}
+	_, filename = filepath.Split(path)
+	audioFile, err := os.Create(filepath.Join(podcast.RootDir, filename))
 	if err != nil {
 		return fmt.Errorf("create audio file: %w", err)
 	}
 	podcastEpisode.Filename = filename
-	podcastEpisode.Path = path.Join(safeFilename(podcast.Title), filename)
 	p.db.Save(&podcastEpisode)
 	go func() {
 		if err := p.doPodcastDownload(&podcastEpisode, audioFile, resp.Body); err != nil {
@@ -433,37 +402,13 @@ func (p *Podcasts) DownloadEpisode(episodeID int) error {
 	return nil
 }
 
-func (p *Podcasts) findUniqueEpisodeName(podcast *db.Podcast, podcastEpisode *db.PodcastEpisode, filename string) string {
-	podcastPath := path.Join(absPath(p.baseDir, podcast), filename)
-	if _, err := os.Stat(podcastPath); os.IsNotExist(err) {
-		return filename
-	}
-	titlePath := fmt.Sprintf("%s%s", safeFilename(podcastEpisode.Title), filepath.Ext(filename))
-	podcastPath = path.Join(absPath(p.baseDir, podcast), titlePath)
-	if _, err := os.Stat(podcastPath); os.IsNotExist(err) {
-		return titlePath
-	}
-	// try to find a filename like FILENAME (1).mp3 incrementing
-	return findEpisode(absPath(p.baseDir, podcast), filename, 1)
-}
-
-func findEpisode(base, filename string, count int) string {
-	noExt := strings.TrimSuffix(filename, filepath.Ext(filename))
-	testFile := fmt.Sprintf("%s (%d)%s", noExt, count, filepath.Ext(filename))
-	podcastPath := path.Join(base, testFile)
-	if _, err := os.Stat(podcastPath); os.IsNotExist(err) {
-		return testFile
-	}
-	return findEpisode(base, filename, count+1)
-}
-
 func getContentDispositionFilename(header string) (string, bool) {
 	_, params, _ := mime.ParseMediaType(header)
 	filename, ok := params["filename"]
 	return filename, ok
 }
 
-func (p *Podcasts) downloadPodcastCover(podPath string, podcast *db.Podcast) error {
+func (p *Podcasts) downloadPodcastCover(podcast *db.Podcast) error {
 	imageURL, err := url.Parse(podcast.ImageURL)
 	if err != nil {
 		return fmt.Errorf("parse image url: %w", err)
@@ -483,10 +428,10 @@ func (p *Podcasts) downloadPodcastCover(podPath string, podcast *db.Podcast) err
 	if ext == "" {
 		contentHeader := resp.Header.Get("content-disposition")
 		filename, _ := getContentDispositionFilename(contentHeader)
-		ext = path.Ext(filename)
+		ext = filepath.Ext(filename)
 	}
-	coverPath := path.Join(podPath, "cover"+ext)
+	cover := "cover" + ext
-	coverFile, err := os.Create(coverPath)
+	coverFile, err := os.Create(filepath.Join(podcast.RootDir, cover))
 	if err != nil {
 		return fmt.Errorf("creating podcast cover: %w", err)
 	}
@@ -494,7 +439,7 @@ func (p *Podcasts) downloadPodcastCover(podPath string, podcast *db.Podcast) err
 	if _, err := io.Copy(coverFile, resp.Body); err != nil {
 		return fmt.Errorf("writing podcast cover: %w", err)
 	}
-	podcast.ImagePath = path.Join(safeFilename(podcast.Title), fmt.Sprintf("cover%s", ext))
+	podcast.Image = fmt.Sprintf("cover%s", ext)
 	if err := p.db.Save(podcast).Error; err != nil {
 		return fmt.Errorf("save podcast: %w", err)
 	}
@@ -507,8 +452,7 @@ func (p *Podcasts) doPodcastDownload(podcastEpisode *db.PodcastEpisode, file *os
 	}
 	defer file.Close()
 	stat, _ := file.Stat()
-	podcastPath := path.Join(p.baseDir, podcastEpisode.Path)
+	podcastTags, err := p.tagger.Read(podcastEpisode.AbsPath())
-	podcastTags, err := p.tagger.Read(podcastPath)
 	if err != nil {
 		log.Printf("error parsing podcast audio: %e", err)
 		podcastEpisode.Status = db.PodcastEpisodeStatusError
@@ -531,7 +475,11 @@ func (p *Podcasts) DeletePodcast(podcastID int) error {
 	if err != nil {
 		return err
 	}
-	if err := os.RemoveAll(absPath(p.baseDir, &podcast)); err != nil {
+	if podcast.RootDir == "" {
+		return fmt.Errorf("podcast has no root dir")
+	}
+
+	if err := os.RemoveAll(podcast.RootDir); err != nil {
 		return fmt.Errorf("delete podcast directory: %w", err)
 	}
 	err = p.db.
@@ -546,13 +494,13 @@ func (p *Podcasts) DeletePodcast(podcastID int) error {
 
 func (p *Podcasts) DeletePodcastEpisode(podcastEpisodeID int) error {
 	episode := db.PodcastEpisode{}
-	err := p.db.First(&episode, podcastEpisodeID).Error
+	err := p.db.Preload("Podcast").First(&episode, podcastEpisodeID).Error
 	if err != nil {
 		return err
 	}
 	episode.Status = db.PodcastEpisodeStatusDeleted
 	p.db.Save(&episode)
-	if err := os.Remove(filepath.Join(p.baseDir, episode.Path)); err != nil {
+	if err := os.Remove(episode.AbsPath()); err != nil {
 		return err
 	}
 	return err
@@ -566,6 +514,7 @@ func (p *Podcasts) PurgeOldPodcasts(maxAge time.Duration) error {
 		Where("created_at < ?", expDate).
 		Where("updated_at < ?", expDate).
 		Where("modified_at < ?", expDate).
+		Preload("Podcast").
 		Find(&episodes).
 		Error
 	if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
@@ -576,20 +525,12 @@ func (p *Podcasts) PurgeOldPodcasts(maxAge time.Duration) error {
 		if err := p.db.Save(episode).Error; err != nil {
 			return fmt.Errorf("save new podcast status: %w", err)
 		}
-		if err := os.Remove(filepath.Join(p.baseDir, episode.Path)); err != nil {
+		if episode.Podcast == nil {
+			return fmt.Errorf("episode %d has no podcast", episode.ID)
+		}
+		if err := os.Remove(episode.AbsPath()); err != nil {
 			return fmt.Errorf("remove podcast path: %w", err)
 		}
 	}
 	return nil
 }
-
-var nonAlphaNum = regexp.MustCompile("[^a-zA-Z0-9_.]+")
-
-func safeFilename(filename string) string {
-	filename = nonAlphaNum.ReplaceAllString(filename, "")
-	return filename
-}
-
-func absPath(base string, p *db.Podcast) string {
-	return filepath.Join(base, safeFilename(p.Title))
-}

podcasts/podcasts_test.go

@@ -1,20 +1,69 @@
 package podcasts
 
 import (
-	"os"
+	"bytes"
+	_ "embed"
+	"path/filepath"
 	"testing"
 	"time"
 
 	"github.com/mmcdole/gofeed"
+	"github.com/stretchr/testify/require"
+	"go.senan.xyz/gonic/db"
+	"go.senan.xyz/gonic/mockfs"
 )
 
+//go:embed testdata/rss.new
+var testRSS []byte
+
+func TestPodcastsAndEpisodesWithSameName(t *testing.T) {
+	t.Skip("requires network access")
+
+	m := mockfs.New(t)
+	require := require.New(t)
+
+	base := t.TempDir()
+	podcasts := New(m.DB(), base, m.TagReader())
+
+	fp := gofeed.NewParser()
+	newFeed, err := fp.Parse(bytes.NewReader(testRSS))
+	if err != nil {
+		t.Fatalf("parse test data: %v", err)
+	}
+
+	podcast, err := podcasts.AddNewPodcast("file://testdata/rss.new", newFeed)
+	require.NoError(err)
+
+	require.Equal(podcast.RootDir, filepath.Join(base, "InternetBox"))
+
+	podcast, err = podcasts.AddNewPodcast("file://testdata/rss.new", newFeed)
+	require.NoError(err)
+
+	// check we made a unique podcast name
+	require.Equal(podcast.RootDir, filepath.Join(base, "InternetBox (1)"))
+
+	podcastEpisodes, err := podcasts.GetNewestPodcastEpisodes(10)
+	require.NoError(err)
+	require.Greater(len(podcastEpisodes), 0)
+
+	var pe []*db.PodcastEpisode
+	require.NoError(m.DB().Order("id").Find(&pe, "podcast_id=? AND title=?", podcast.ID, "Episode 126").Error)
+	require.Len(pe, 2)
+
+	require.NoError(podcasts.DownloadEpisode(pe[0].ID))
+	require.NoError(podcasts.DownloadEpisode(pe[1].ID))
+
+	require.NoError(m.DB().Order("id").Preload("Podcast").Find(&pe, "podcast_id=? AND title=?", podcast.ID, "Episode 126").Error)
+	require.Len(pe, 2)
+
+	// check we made a unique podcast episode names
+	require.Equal("InternetBoxEpisode126.mp3", pe[0].Filename)
+	require.Equal("InternetBoxEpisode126 (1).mp3", pe[1].Filename)
+}
+
 func TestGetMoreRecentEpisodes(t *testing.T) {
 	fp := gofeed.NewParser()
-	newFile, err := os.Open("testdata/rss.new")
+	newFeed, err := fp.Parse(bytes.NewReader(testRSS))
-	if err != nil {
-		t.Fatalf("open test data: %v", err)
-	}
-	newFeed, err := fp.Parse(newFile)
 	if err != nil {
 		t.Fatalf("parse test data: %v", err)
 	}

podcasts/testdata/rss.new

@@ -2,7 +2,7 @@
 <rss version="2.0" xmlns:atom="http://www.w3.org/2005/Atom" xmlns:cc="http://web.resource.org/cc/" xmlns:itunes="http://www.itunes.com/dtds/podcast-1.0.dtd" xmlns:media="http://search.yahoo.com/mrss/" xmlns:content="http://purl.org/rss/1.0/modules/content/" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
 	<channel>
 		<atom:link href="https://internetbox.libsyn.com/rss" rel="self" type="application/rss+xml"/>
-		<title>Internet Box</title>
+		<title>Internet Box!</title>
 		<pubDate>Mon, 01 Apr 2019 04:35:43 +0000</pubDate>
 		<lastBuildDate>Sun, 10 Jan 2021 00:07:33 +0000</lastBuildDate>
 		<generator>Libsyn WebEngine 2.0</generator>
@@ -70,7 +70,23 @@
 			<pubDate>Mon, 27 Jun 2016 06:33:43 +0000</pubDate>
 			<guid isPermaLink="false"><![CDATA[d5113fc98b7baf005bf66b225b166ee0]]></guid>
 			<link><![CDATA[http://internetboxpodcast.com/episode-126/]]></link>
-			<itunes:image href="https://ssl-static.libsyn.com/p/assets/d/d/3/3/dd338b309838f617/iTunes.png" />
+			<itunes:image href="file://testdata/cover.png" />
+			<description><![CDATA[<p>The Internet Box is clicking this week!</p>]]></description>
+			<content:encoded><![CDATA[<p>The Internet Box is clicking this week!</p>]]></content:encoded>
+			<enclosure length="66139165" type="audio/mpeg" url="https://traffic.libsyn.com/secure/internetbox/InternetBoxEpisode126.mp3?dest-id=79492" />
+			<itunes:duration>01:20:42</itunes:duration>
+			<itunes:explicit>yes</itunes:explicit>
+			<itunes:keywords>box,2,factory,internet,pizza,future,robots,season,farts,cake,via,reddit,313,vorarephilia</itunes:keywords>
+			<itunes:subtitle><![CDATA[The Internet Box is clicking this week!]]></itunes:subtitle>
+		</item>
+
+		<!-- dupe episode name -->
+		<item>
+			<title>Episode 126</title>
+			<pubDate>Mon, 27 Jun 2016 06:33:43 +0000</pubDate>
+			<guid isPermaLink="false"><![CDATA[d5113fc98b7baf005bf66b225b166ee0]]></guid>
+			<link><![CDATA[http://internetboxpodcast.com/episode-126/]]></link>
+			<itunes:image href="file://testdata/cover.png" />
 			<description><![CDATA[<p>The Internet Box is clicking this week!</p>]]></description>
 			<content:encoded><![CDATA[<p>The Internet Box is clicking this week!</p>]]></content:encoded>
 			<enclosure length="66139165" type="audio/mpeg" url="https://traffic.libsyn.com/secure/internetbox/InternetBoxEpisode126.mp3?dest-id=79492" />

server/ctrlsubsonic/ctrl_test.go

@@ -8,7 +8,6 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"os"
-	"path"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -48,7 +47,7 @@ func makeGoldenPath(test string) string {
 	snake := testCamelExpr.ReplaceAllString(test, "${1}_${2}")
 	lower := strings.ToLower(snake)
 	relPath := strings.ReplaceAll(lower, "/", "_")
-	return path.Join("testdata", relPath)
+	return filepath.Join("testdata", relPath)
 }
 
 func makeHTTPMock(query url.Values) (*httptest.ResponseRecorder, *http.Request) {

server/ctrlsubsonic/handlers_common.go

@@ -184,11 +184,7 @@ func (c *Controller) ServeGetPlayQueue(r *http.Request) *spec.Response {
 			c.DB.
 				Where("id=?", id.Value).
 				Find(&pe)
-			p := db.Podcast{}
+			sub.PlayQueue.List[i] = spec.NewTCPodcastEpisode(&pe)
-			c.DB.
-				Where("id=?", pe.PodcastID).
-				Find(&p)
-			sub.PlayQueue.List[i] = spec.NewTCPodcastEpisode(&pe, &p)
 			sub.PlayQueue.List[i].TranscodeMeta = transcodeMeta
 		}
 	}
@@ -300,7 +296,7 @@ func (c *Controller) ServeJukebox(r *http.Request) *spec.Response { // nolint:go
 	trackPaths := func(ids []specid.ID) ([]string, error) {
 		var paths []string
 		for _, id := range ids {
-			r, err := specidpaths.Locate(c.DB, c.PodcastsPath, id)
+			r, err := specidpaths.Locate(c.DB, id)
 			if err != nil {
 				return nil, fmt.Errorf("find track by id: %w", err)
 			}

server/ctrlsubsonic/handlers_playlist.go

@@ -94,7 +94,7 @@ func (c *Controller) ServeCreatePlaylist(r *http.Request) *spec.Response {
 	playlist.Items = nil
 	ids := params.GetOrIDList("songId", nil)
 	for _, id := range ids {
-		r, err := specidpaths.Locate(c.DB, c.PodcastsPath, id)
+		r, err := specidpaths.Locate(c.DB, id)
 		if err != nil {
 			return spec.NewError(0, "lookup id %v: %v", id, err)
 		}
@@ -154,7 +154,7 @@ func (c *Controller) ServeUpdatePlaylist(r *http.Request) *spec.Response {
 	// add items
 	if ids, err := params.GetIDList("songIdToAdd"); err == nil {
 		for _, id := range ids {
-			item, err := specidpaths.Locate(c.DB, c.PodcastsPath, id)
+			item, err := specidpaths.Locate(c.DB, id)
 			if err != nil {
 				return spec.NewError(0, "locate id %q: %v", id, err)
 			}
@@ -224,14 +224,10 @@ func playlistRender(c *Controller, params params.Params, playlistID string, play
 			resp.Duration += track.Length
 		case specid.PodcastEpisode:
 			var pe db.PodcastEpisode
-			if err := c.DB.Where("id=?", id.Value).Find(&pe).Error; errors.Is(err, gorm.ErrRecordNotFound) {
+			if err := c.DB.Preload("Podcast").Where("id=?", id.Value).Find(&pe).Error; errors.Is(err, gorm.ErrRecordNotFound) {
 				return nil, fmt.Errorf("load podcast episode by id: %w", err)
 			}
-			var p db.Podcast
+			trch = spec.NewTCPodcastEpisode(&pe)
-			if err := c.DB.Where("id=?", pe.PodcastID).Find(&p).Error; errors.Is(err, gorm.ErrRecordNotFound) {
-				return nil, fmt.Errorf("load podcast by id: %w", err)
-			}
-			trch = spec.NewTCPodcastEpisode(&pe, &p)
 			resp.Duration += pe.Length
 		default:
 			continue

server/ctrlsubsonic/handlers_raw.go

@@ -9,7 +9,7 @@ import (
 	"log"
 	"net/http"
 	"os"
-	"path"
+	"path/filepath"
 	"time"
 
 	"github.com/disintegration/imaging"
@@ -117,26 +117,26 @@ var (
 )
 
 // TODO: can we use specidpaths.Locate here?
-func coverFor(dbc *db.DB, artistInfoCache *artistinfocache.ArtistInfoCache, podcastPath string, id specid.ID) (io.ReadCloser, error) {
+func coverFor(dbc *db.DB, artistInfoCache *artistinfocache.ArtistInfoCache, id specid.ID) (io.ReadCloser, error) {
 	switch id.Type {
 	case specid.Album:
 		return coverForAlbum(dbc, id.Value)
 	case specid.Artist:
 		return coverForArtist(artistInfoCache, id.Value)
 	case specid.Podcast:
-		return coverForPodcast(dbc, podcastPath, id.Value)
+		return coverForPodcast(dbc, id.Value)
 	case specid.PodcastEpisode:
-		return coverGetPathPodcastEpisode(dbc, podcastPath, id.Value)
+		return coverGetPathPodcastEpisode(dbc, id.Value)
 	default:
 		return nil, errCoverNotFound
 	}
 }
 
 func coverForAlbum(dbc *db.DB, id int) (*os.File, error) {
-	folder := &db.Album{}
+	var folder db.Album
 	err := dbc.DB.
 		Select("id, root_dir, left_path, right_path, cover").
-		First(folder, id).
+		First(&folder, id).
 		Error
 	if err != nil {
 		return nil, fmt.Errorf("select album: %w", err)
@@ -144,7 +144,7 @@ func coverForAlbum(dbc *db.DB, id int) (*os.File, error) {
 	if folder.Cover == "" {
 		return nil, errCoverEmpty
 	}
-	return os.Open(path.Join(folder.RootDir, folder.LeftPath, folder.RightPath, folder.Cover))
+	return os.Open(filepath.Join(folder.RootDir, folder.LeftPath, folder.RightPath, folder.Cover))
 }
 
 func coverForArtist(artistInfoCache *artistinfocache.ArtistInfoCache, id int) (io.ReadCloser, error) {
@@ -162,39 +162,33 @@ func coverForArtist(artistInfoCache *artistinfocache.ArtistInfoCache, id int) (i
 	return resp.Body, nil
 }
 
-func coverForPodcast(dbc *db.DB, podcastPath string, id int) (*os.File, error) {
+func coverForPodcast(dbc *db.DB, id int) (*os.File, error) {
-	podcast := &db.Podcast{}
+	var podcast db.Podcast
 	err := dbc.
-		First(podcast, id).
+		First(&podcast, id).
 		Error
 	if err != nil {
 		return nil, fmt.Errorf("select podcast: %w", err)
 	}
-	if podcast.ImagePath == "" {
+	if podcast.Image == "" {
 		return nil, errCoverEmpty
 	}
-	return os.Open(path.Join(podcastPath, podcast.ImagePath))
+	return os.Open(filepath.Join(podcast.RootDir, podcast.Image))
 }
 
-func coverGetPathPodcastEpisode(dbc *db.DB, podcastPath string, id int) (*os.File, error) {
+func coverGetPathPodcastEpisode(dbc *db.DB, id int) (*os.File, error) {
-	episode := &db.PodcastEpisode{}
+	var pe db.PodcastEpisode
 	err := dbc.
-		First(episode, id).
+		Preload("Podcast").
+		First(&pe, id).
 		Error
 	if err != nil {
 		return nil, fmt.Errorf("select episode: %w", err)
 	}
-	podcast := &db.Podcast{}
+	if pe.Podcast == nil || pe.Podcast.Image == "" {
-	err = dbc.
-		First(podcast, episode.PodcastID).
-		Error
-	if err != nil {
-		return nil, fmt.Errorf("select podcast: %w", err)
-	}
-	if podcast.ImagePath == "" {
 		return nil, errCoverEmpty
 	}
-	return os.Open(path.Join(podcastPath, podcast.ImagePath))
+	return os.Open(filepath.Join(pe.Podcast.RootDir, pe.Podcast.Image))
 }
 
 func coverScaleAndSave(reader io.Reader, cachePath string, size int) error {
@@ -220,14 +214,14 @@ func (c *Controller) ServeGetCoverArt(w http.ResponseWriter, r *http.Request) *s
 		return spec.NewError(10, "please provide an `id` parameter")
 	}
 	size := params.GetOrInt("size", coverDefaultSize)
-	cachePath := path.Join(
+	cachePath := filepath.Join(
 		c.CacheCoverPath,
 		fmt.Sprintf("%s-%d.%s", id.String(), size, coverCacheFormat),
 	)
 	_, err = os.Stat(cachePath)
 	switch {
 	case os.IsNotExist(err):
-		reader, err := coverFor(c.DB, c.ArtistInfoCache, c.PodcastsPath, id)
+		reader, err := coverFor(c.DB, c.ArtistInfoCache, id)
 		if err != nil {
 			return spec.NewError(10, "couldn't find cover `%s`: %v", id, err)
 		}
@@ -256,7 +250,7 @@ func (c *Controller) ServeStream(w http.ResponseWriter, r *http.Request) *spec.R
 		return spec.NewError(10, "please provide an `id` parameter")
 	}
 
-	file, err := specidpaths.Locate(c.DB, c.PodcastsPath, id)
+	file, err := specidpaths.Locate(c.DB, id)
 	if err != nil {
 		return spec.NewError(0, "error looking up id %s: %v", id, err)
 	}

server/ctrlsubsonic/spec/construct_by_folder.go

@@ -1,7 +1,7 @@
 package spec
 
 import (
-	"path"
+	"path/filepath"
 	"strings"
 
 	"go.senan.xyz/gonic/db"
@@ -62,7 +62,7 @@ func NewTCTrackByFolder(t *db.Track, parent *db.Album) *TrackChild {
 		Title:       t.TagTitle,
 		TrackNumber: t.TagTrackNumber,
 		DiscNumber:  t.TagDiscNumber,
-		Path: path.Join(
+		Path: filepath.Join(
 			parent.LeftPath,
 			parent.RightPath,
 			t.Filename,
@@ -95,21 +95,24 @@ func NewTCTrackByFolder(t *db.Track, parent *db.Album) *TrackChild {
 	return trCh
 }
 
-func NewTCPodcastEpisode(pe *db.PodcastEpisode, parent *db.Podcast) *TrackChild {
+func NewTCPodcastEpisode(pe *db.PodcastEpisode) *TrackChild {
 	trCh := &TrackChild{
 		ID:          pe.SID(),
 		ContentType: pe.MIME(),
 		Suffix:      pe.Ext(),
 		Size:        pe.Size,
 		Title:       pe.Title,
-		Path:        pe.Path,
+		ParentID:    pe.SID(),
-		ParentID:    parent.SID(),
 		Duration:    pe.Length,
 		Bitrate:     pe.Bitrate,
 		IsDir:       false,
 		Type:        "podcastepisode",
 		CreatedAt:   pe.CreatedAt,
 	}
+	if pe.Podcast != nil {
+		trCh.ParentID = pe.Podcast.SID()
+		trCh.Path = pe.AbsPath()
+	}
 	return trCh
 }
 

server/ctrlsubsonic/spec/construct_by_tags.go

@@ -1,7 +1,7 @@
 package spec
 
 import (
-	"path"
+	"path/filepath"
 	"sort"
 	"strings"
 
@@ -61,7 +61,7 @@ func NewTrackByTags(t *db.Track, album *db.Album) *TrackChild {
 		Artist:      t.TagTrackArtist,
 		TrackNumber: t.TagTrackNumber,
 		DiscNumber:  t.TagDiscNumber,
-		Path: path.Join(
+		Path: filepath.Join(
 			album.LeftPath,
 			album.RightPath,
 			t.Filename,

server/ctrlsubsonic/spec/construct_podcast.go

@@ -1,13 +1,13 @@
 package spec
 
 import (
-	"jaytaylor.com/html2text"
 	"go.senan.xyz/gonic/db"
+	"jaytaylor.com/html2text"
 )
 
 func NewPodcastChannel(p *db.Podcast) *PodcastChannel {
 	desc, err := html2text.FromString(p.Description, html2text.Options{TextOnly: true})
-	if (err != nil) {
+	if err != nil {
 		desc = ""
 	}
 	ret := &PodcastChannel{
@@ -26,31 +26,34 @@ func NewPodcastChannel(p *db.Podcast) *PodcastChannel {
 	return ret
 }
 
-func NewPodcastEpisode(e *db.PodcastEpisode) *PodcastEpisode {
+func NewPodcastEpisode(pe *db.PodcastEpisode) *PodcastEpisode {
-	if e == nil {
+	if pe == nil {
 		return nil
 	}
-	desc, err := html2text.FromString(e.Description, html2text.Options{TextOnly: true})
+	desc, err := html2text.FromString(pe.Description, html2text.Options{TextOnly: true})
-	if (err != nil) {
+	if err != nil {
 		desc = ""
 	}
-	return &PodcastEpisode{
+	r := &PodcastEpisode{
-		ID:          e.SID(),
+		ID:          pe.SID(),
-		StreamID:    e.SID(),
+		StreamID:    pe.SID(),
-		ContentType: e.MIME(),
+		ContentType: pe.MIME(),
-		ChannelID:   e.PodcastSID(),
+		ChannelID:   pe.PodcastSID(),
-		Title:       e.Title,
+		Title:       pe.Title,
 		Description: desc,
-		Status:      string(e.Status),
+		Status:      string(pe.Status),
-		CoverArt:    e.PodcastSID(),
+		CoverArt:    pe.PodcastSID(),
-		PublishDate: *e.PublishDate,
+		PublishDate: *pe.PublishDate,
 		Genre:       "Podcast",
-		Duration:    e.Length,
+		Duration:    pe.Length,
-		Year:        e.PublishDate.Year(),
+		Year:        pe.PublishDate.Year(),
-		Suffix:      formatExt(e.Ext()),
+		Suffix:      formatExt(pe.Ext()),
-		BitRate:     e.Bitrate,
+		BitRate:     pe.Bitrate,
 		IsDir:       false,
-		Path:        e.Path,
+		Size:        pe.Size,
-		Size:        e.Size,
 	}
+	if pe.Podcast != nil {
+		r.Path = pe.AbsPath()
+	}
+	return r
 }

server/ctrlsubsonic/specidpaths/specidpaths.go

@@ -18,22 +18,18 @@ type Result interface {
 }
 
 // Locate maps a specid to its location on the filesystem
-func Locate(dbc *db.DB, podcastsPath string, id specid.ID) (Result, error) {
+func Locate(dbc *db.DB, id specid.ID) (Result, error) {
 	switch id.Type {
 	case specid.Track:
 		var track db.Track
-		if err := dbc.Preload("Album").Where("id=?", id.Value).Find(&track).Error; err == nil {
+		return &track, dbc.Preload("Album").Where("id=?", id.Value).Find(&track).Error
-			return &track, nil
-		}
 	case specid.PodcastEpisode:
 		var pe db.PodcastEpisode
-		if err := dbc.Where("id=? AND status=?", id.Value, db.PodcastEpisodeStatusCompleted).Find(&pe).Error; err == nil {
+		return &pe, dbc.Preload("Podcast").Where("id=? AND status=?", id.Value, db.PodcastEpisodeStatusCompleted).Find(&pe).Error
-			pe.AbsP = filepath.Join(podcastsPath, pe.Path)
+	default:
-			return &pe, err
-		}
-	}
 		return nil, ErrNotFound
 	}
+}
 
 // Locate maps a location on the filesystem to a specid
 func Lookup(dbc *db.DB, musicPaths []string, podcastsPath string, path string) (Result, error) {
@@ -42,9 +38,13 @@ func Lookup(dbc *db.DB, musicPaths []string, podcastsPath string, path string) (
 	}
 
 	if strings.HasPrefix(path, podcastsPath) {
-		path, _ = filepath.Rel(podcastsPath, path)
+		podcastPath, episodeFilename := filepath.Split(path)
+		q := dbc.
+			Joins(`JOIN podcasts ON podcasts.id=podcast_episodes.podcast_id`).
+			Where(`podcasts.root_dir=? AND podcast_episodes.filename=?`, filepath.Clean(podcastPath), filepath.Clean(episodeFilename))
+
 		var pe db.PodcastEpisode
-		if err := dbc.Where(`path=?`, path).First(&pe).Error; err == nil {
+		if err := q.First(&pe).Error; err == nil {
 			return &pe, nil
 		}
 		return nil, ErrNotFound