mirror of
https://github.com/navidrome/navidrome.git
synced 2026-05-03 06:51:16 +00:00
fix: enhance handling of library-qualified paths and improve cross-library playlist support
Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
parent
6a2851ef92
commit
ae04256030
@ -10,7 +10,6 @@ import (
|
||||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strings"
|
||||
"time"
|
||||
@ -206,12 +205,25 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m
|
||||
log.Warn(ctx, "Error reading files from DB", "playlist", pls.Name, err)
|
||||
continue
|
||||
}
|
||||
// Build lookup map with library-qualified keys
|
||||
existing := make(map[string]int, len(found))
|
||||
for idx := range found {
|
||||
existing[normalizePathForComparison(found[idx].Path)] = idx
|
||||
// Key format: "libraryID:path" (normalized)
|
||||
key := fmt.Sprintf("%d:%s", found[idx].LibraryID, normalizePathForComparison(found[idx].Path))
|
||||
existing[key] = idx
|
||||
}
|
||||
for _, path := range paths {
|
||||
idx, ok := existing[normalizePathForComparison(path)]
|
||||
// Parse the library-qualified path
|
||||
normalizedPath := path
|
||||
if parts := strings.SplitN(path, ":", 2); len(parts) == 2 {
|
||||
// Path is already qualified: "libraryID:path"
|
||||
normalizedPath = parts[0] + ":" + normalizePathForComparison(parts[1])
|
||||
} else {
|
||||
// Unqualified path (backward compatibility)
|
||||
normalizedPath = normalizePathForComparison(path)
|
||||
}
|
||||
|
||||
idx, ok := existing[normalizedPath]
|
||||
if ok {
|
||||
mfs = append(mfs, found[idx])
|
||||
} else {
|
||||
@ -239,131 +251,168 @@ func normalizePathForComparison(path string) string {
|
||||
type pathResolution struct {
|
||||
absolutePath string
|
||||
libraryPath string
|
||||
libraryID int
|
||||
valid bool
|
||||
}
|
||||
|
||||
// toRelativePath converts the absolute path to a library-relative path.
|
||||
func (r pathResolution) toRelativePath() (string, error) {
|
||||
// toLibraryQualifiedPath converts the absolute path to a library-qualified path format: "libraryID:relativePath".
|
||||
func (r pathResolution) toLibraryQualifiedPath() (string, error) {
|
||||
if !r.valid {
|
||||
return "", fmt.Errorf("invalid path resolution")
|
||||
}
|
||||
return filepath.Rel(r.libraryPath, r.absolutePath)
|
||||
relativePath, err := filepath.Rel(r.libraryPath, r.absolutePath)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return fmt.Sprintf("%d:%s", r.libraryID, relativePath), nil
|
||||
}
|
||||
|
||||
// newValidResolution creates a valid path resolution.
|
||||
func newValidResolution(absolutePath, libraryPath string) pathResolution {
|
||||
func newValidResolution(absolutePath, libraryPath string, libraryID int) pathResolution {
|
||||
return pathResolution{
|
||||
absolutePath: absolutePath,
|
||||
libraryPath: libraryPath,
|
||||
libraryID: libraryID,
|
||||
valid: true,
|
||||
}
|
||||
}
|
||||
|
||||
// normalizePaths converts playlist file paths to library-relative paths.
|
||||
// libraryMatcher holds sorted libraries with cleaned paths for efficient path matching.
|
||||
type libraryMatcher struct {
|
||||
libraries model.Libraries
|
||||
cleanedPaths []string
|
||||
}
|
||||
|
||||
// findLibraryForPath finds which library contains the given absolute path.
|
||||
// Returns library ID and path, or 0 and empty string if not found.
|
||||
func (lm *libraryMatcher) findLibraryForPath(absolutePath string) (int, string) {
|
||||
// Check sorted libraries (longest path first) to find the best match
|
||||
for i, cleanLibPath := range lm.cleanedPaths {
|
||||
// Check if absolutePath is under this library path
|
||||
if strings.HasPrefix(absolutePath, cleanLibPath) {
|
||||
// Ensure it's a proper path boundary (not just a prefix)
|
||||
if len(absolutePath) == len(cleanLibPath) || absolutePath[len(cleanLibPath)] == filepath.Separator {
|
||||
return lm.libraries[i].ID, cleanLibPath
|
||||
}
|
||||
}
|
||||
}
|
||||
return 0, ""
|
||||
}
|
||||
|
||||
// newLibraryMatcher creates a libraryMatcher with libraries sorted by path length (longest first).
|
||||
// This ensures correct matching when library paths are prefixes of each other.
|
||||
// Example: /music-classical must be checked before /music
|
||||
// Otherwise, /music-classical/track.mp3 would match /music instead of /music-classical
|
||||
func newLibraryMatcher(libs model.Libraries) *libraryMatcher {
|
||||
// Sort libraries by path length (descending) to ensure longest paths match first.
|
||||
slices.SortFunc(libs, func(i, j model.Library) int {
|
||||
return cmp.Compare(len(j.Path), len(i.Path)) // Reverse order for descending
|
||||
})
|
||||
|
||||
// Pre-clean all library paths once for efficient matching
|
||||
cleanedPaths := make([]string, len(libs))
|
||||
for i, lib := range libs {
|
||||
cleanedPaths[i] = filepath.Clean(lib.Path)
|
||||
}
|
||||
return &libraryMatcher{
|
||||
libraries: libs,
|
||||
cleanedPaths: cleanedPaths,
|
||||
}
|
||||
}
|
||||
|
||||
// normalizePaths converts playlist file paths to library-qualified paths (format: "libraryID:relativePath").
|
||||
// For relative paths, it resolves them to absolute paths first, then determines which
|
||||
// library they belong to. This allows playlists to reference files across library boundaries.
|
||||
func (s *playlists) normalizePaths(ctx context.Context, pls *model.Playlist, folder *model.Folder, lines []string) ([]string, error) {
|
||||
libRegex, err := s.compileLibraryPaths(ctx)
|
||||
matcher, err := s.buildLibraryMatcher(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
res := make([]string, 0, len(lines))
|
||||
for idx, line := range lines {
|
||||
resolution := s.resolvePath(line, folder, libRegex)
|
||||
resolution := s.resolvePath(line, folder, matcher)
|
||||
|
||||
if !resolution.valid {
|
||||
log.Warn(ctx, "Path in playlist not found in any library", "path", line, "line", idx)
|
||||
continue
|
||||
}
|
||||
|
||||
relativePath, err := resolution.toRelativePath()
|
||||
qualifiedPath, err := resolution.toLibraryQualifiedPath()
|
||||
if err != nil {
|
||||
log.Debug(ctx, "Error getting relative path", "playlist", pls.Name, "path", line,
|
||||
log.Debug(ctx, "Error getting library-qualified path", "playlist", pls.Name, "path", line,
|
||||
"libPath", resolution.libraryPath, "filePath", resolution.absolutePath, err)
|
||||
continue
|
||||
}
|
||||
|
||||
res = append(res, relativePath)
|
||||
res = append(res, qualifiedPath)
|
||||
}
|
||||
return slice.Map(res, filepath.ToSlash), nil
|
||||
// Convert path separators to forward slashes (only the path part, not the library ID prefix)
|
||||
return slice.Map(res, func(qp string) string {
|
||||
parts := strings.SplitN(qp, ":", 2)
|
||||
if len(parts) == 2 {
|
||||
return parts[0] + ":" + filepath.ToSlash(parts[1])
|
||||
}
|
||||
return filepath.ToSlash(qp)
|
||||
}), nil
|
||||
}
|
||||
|
||||
// resolvePath determines the absolute path and library path for a playlist entry.
|
||||
func (s *playlists) resolvePath(line string, folder *model.Folder, libRegex *regexp.Regexp) pathResolution {
|
||||
func (s *playlists) resolvePath(line string, folder *model.Folder, matcher *libraryMatcher) pathResolution {
|
||||
if folder != nil && !filepath.IsAbs(line) {
|
||||
return s.resolveRelativePath(line, folder, libRegex)
|
||||
return s.resolveRelativePath(line, folder, matcher)
|
||||
}
|
||||
return s.resolveAbsolutePath(line, libRegex)
|
||||
return s.resolveAbsolutePath(line, matcher)
|
||||
}
|
||||
|
||||
// resolveRelativePath handles relative paths by converting them to absolute paths
|
||||
// and finding their library location. This enables cross-library playlist references.
|
||||
func (s *playlists) resolveRelativePath(line string, folder *model.Folder, libRegex *regexp.Regexp) pathResolution {
|
||||
func (s *playlists) resolveRelativePath(line string, folder *model.Folder, matcher *libraryMatcher) pathResolution {
|
||||
// Step 1: Resolve relative path to absolute path based on playlist location
|
||||
// Example: playlist at /music/playlists/test.m3u with line "../songs/abc.mp3"
|
||||
// resolves to /music/songs/abc.mp3
|
||||
absolutePath := filepath.Clean(filepath.Join(folder.AbsolutePath(), line))
|
||||
|
||||
// Step 2: Determine which library this absolute path belongs to using regex matching
|
||||
if libPath := libRegex.FindString(absolutePath); libPath != "" {
|
||||
return newValidResolution(absolutePath, libPath)
|
||||
// Step 2: Determine which library this absolute path belongs to
|
||||
libID, libPath := matcher.findLibraryForPath(absolutePath)
|
||||
if libID != 0 {
|
||||
return newValidResolution(absolutePath, libPath, libID)
|
||||
}
|
||||
|
||||
// Fallback: Check if it's in the playlist's own library
|
||||
return s.validatePathInLibrary(absolutePath, folder.LibraryPath)
|
||||
return s.validatePathInLibrary(absolutePath, folder.LibraryPath, folder.LibraryID)
|
||||
}
|
||||
|
||||
// resolveAbsolutePath handles absolute paths by matching them against library paths.
|
||||
func (s *playlists) resolveAbsolutePath(line string, libRegex *regexp.Regexp) pathResolution {
|
||||
func (s *playlists) resolveAbsolutePath(line string, matcher *libraryMatcher) pathResolution {
|
||||
cleanPath := filepath.Clean(line)
|
||||
libPath := libRegex.FindString(cleanPath)
|
||||
libID, libPath := matcher.findLibraryForPath(cleanPath)
|
||||
|
||||
if libPath == "" {
|
||||
if libID == 0 {
|
||||
return pathResolution{valid: false}
|
||||
}
|
||||
return newValidResolution(cleanPath, libPath)
|
||||
return newValidResolution(cleanPath, libPath, libID)
|
||||
}
|
||||
|
||||
// validatePathInLibrary verifies that an absolute path belongs to the specified library.
|
||||
// It rejects paths that escape the library using ".." segments.
|
||||
func (s *playlists) validatePathInLibrary(absolutePath, libraryPath string) pathResolution {
|
||||
func (s *playlists) validatePathInLibrary(absolutePath, libraryPath string, libraryID int) pathResolution {
|
||||
rel, err := filepath.Rel(libraryPath, absolutePath)
|
||||
if err != nil || strings.HasPrefix(rel, "..") {
|
||||
return pathResolution{valid: false}
|
||||
}
|
||||
|
||||
return newValidResolution(absolutePath, libraryPath)
|
||||
return newValidResolution(absolutePath, libraryPath, libraryID)
|
||||
}
|
||||
|
||||
func (s *playlists) compileLibraryPaths(ctx context.Context) (*regexp.Regexp, error) {
|
||||
// buildLibraryMatcher creates a libraryMatcher with libraries sorted by path length (longest first).
|
||||
func (s *playlists) buildLibraryMatcher(ctx context.Context) (*libraryMatcher, error) {
|
||||
libs, err := s.ds.Library(ctx).GetAll()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Sort libraries by path length (descending) to ensure longest paths match first.
|
||||
// This prevents shorter paths that are prefixes from incorrectly matching.
|
||||
// Example: /music-classical must be checked before /music
|
||||
// Otherwise, /music-classical/track.mp3 would match /music instead of /music-classical
|
||||
slices.SortFunc(libs, func(i, j model.Library) int {
|
||||
return cmp.Compare(len(j.Path), len(i.Path)) // Reverse order for descending
|
||||
})
|
||||
|
||||
// Create regex patterns for each library path
|
||||
patterns := make([]string, len(libs))
|
||||
for i, lib := range libs {
|
||||
cleanPath := filepath.Clean(lib.Path)
|
||||
escapedPath := regexp.QuoteMeta(cleanPath)
|
||||
patterns[i] = fmt.Sprintf("^%s(?:/|$)", escapedPath)
|
||||
}
|
||||
// Combine all patterns into a single regex - order matters due to alternation
|
||||
combinedPattern := strings.Join(patterns, "|")
|
||||
re, err := regexp.Compile(combinedPattern)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("compiling library paths `%s`: %w", combinedPattern, err)
|
||||
}
|
||||
return re, nil
|
||||
return newLibraryMatcher(libs), nil
|
||||
}
|
||||
|
||||
func (s *playlists) updatePlaylist(ctx context.Context, newPls *model.Playlist) error {
|
||||
|
||||
@ -10,7 +10,7 @@ import (
|
||||
"golang.org/x/text/unicode/norm"
|
||||
)
|
||||
|
||||
var _ = Describe("compileLibraryPaths", func() {
|
||||
var _ = Describe("buildLibraryMatcher", func() {
|
||||
var ds *tests.MockDataStore
|
||||
var mockLibRepo *tests.MockLibraryRepo
|
||||
var ps *playlists
|
||||
@ -33,25 +33,25 @@ var _ = Describe("compileLibraryPaths", func() {
|
||||
{ID: 3, Path: "/music-classical/opera"},
|
||||
})
|
||||
|
||||
libRegex, err := ps.compileLibraryPaths(ctx)
|
||||
matcher, err := ps.buildLibraryMatcher(ctx)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
// Test that longest path matches first
|
||||
// Note: The regex pattern ^path(?:/|$) will match the path plus the trailing /
|
||||
// Test that longest path matches first and returns correct library ID
|
||||
testCases := []struct {
|
||||
path string
|
||||
expected string
|
||||
path string
|
||||
expectedLibID int
|
||||
expectedLibPath string
|
||||
}{
|
||||
{"/music-classical/opera/track.mp3", "/music-classical/opera/"},
|
||||
{"/music-classical/track.mp3", "/music-classical/"},
|
||||
{"/music/track.mp3", "/music/"},
|
||||
{"/music-classical/opera/", "/music-classical/opera/"}, // Trailing slash
|
||||
{"/music-classical/opera", "/music-classical/opera"}, // Exact match (no trailing /)
|
||||
{"/music-classical/opera/track.mp3", 3, "/music-classical/opera"},
|
||||
{"/music-classical/track.mp3", 2, "/music-classical"},
|
||||
{"/music/track.mp3", 1, "/music"},
|
||||
{"/music-classical/opera/subdir/file.mp3", 3, "/music-classical/opera"},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
matched := libRegex.FindString(tc.path)
|
||||
Expect(matched).To(Equal(tc.expected), "Path %s should match %s, but got %s", tc.path, tc.expected, matched)
|
||||
libID, libPath := matcher.findLibraryForPath(tc.path)
|
||||
Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d, but got %d", tc.path, tc.expectedLibID, libID)
|
||||
Expect(libPath).To(Equal(tc.expectedLibPath), "Path %s should match library path %s, but got %s", tc.path, tc.expectedLibPath, libPath)
|
||||
}
|
||||
})
|
||||
|
||||
@ -61,16 +61,18 @@ var _ = Describe("compileLibraryPaths", func() {
|
||||
{ID: 2, Path: "/home/user/music-backup"},
|
||||
})
|
||||
|
||||
libRegex, err := ps.compileLibraryPaths(ctx)
|
||||
matcher, err := ps.buildLibraryMatcher(ctx)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
// Test that music-backup library is matched correctly
|
||||
matched := libRegex.FindString("/home/user/music-backup/track.mp3")
|
||||
Expect(matched).To(Equal("/home/user/music-backup/"))
|
||||
libID, libPath := matcher.findLibraryForPath("/home/user/music-backup/track.mp3")
|
||||
Expect(libID).To(Equal(2))
|
||||
Expect(libPath).To(Equal("/home/user/music-backup"))
|
||||
|
||||
// Test that music library is still matched correctly
|
||||
matched = libRegex.FindString("/home/user/music/track.mp3")
|
||||
Expect(matched).To(Equal("/home/user/music/"))
|
||||
libID, libPath = matcher.findLibraryForPath("/home/user/music/track.mp3")
|
||||
Expect(libID).To(Equal(1))
|
||||
Expect(libPath).To(Equal("/home/user/music"))
|
||||
})
|
||||
|
||||
It("matches path that is exactly the library root", func() {
|
||||
@ -79,12 +81,13 @@ var _ = Describe("compileLibraryPaths", func() {
|
||||
{ID: 2, Path: "/music-classical"},
|
||||
})
|
||||
|
||||
libRegex, err := ps.compileLibraryPaths(ctx)
|
||||
matcher, err := ps.buildLibraryMatcher(ctx)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
// Exact library path should match (no trailing /)
|
||||
matched := libRegex.FindString("/music-classical")
|
||||
Expect(matched).To(Equal("/music-classical"))
|
||||
// Exact library path should match
|
||||
libID, libPath := matcher.findLibraryForPath("/music-classical")
|
||||
Expect(libID).To(Equal(2))
|
||||
Expect(libPath).To(Equal("/music-classical"))
|
||||
})
|
||||
|
||||
It("handles complex nested library structures", func() {
|
||||
@ -95,22 +98,24 @@ var _ = Describe("compileLibraryPaths", func() {
|
||||
{ID: 4, Path: "/media/audio/classical/baroque"},
|
||||
})
|
||||
|
||||
libRegex, err := ps.compileLibraryPaths(ctx)
|
||||
matcher, err := ps.buildLibraryMatcher(ctx)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
testCases := []struct {
|
||||
path string
|
||||
expected string
|
||||
path string
|
||||
expectedLibID int
|
||||
expectedLibPath string
|
||||
}{
|
||||
{"/media/audio/classical/baroque/bach/track.mp3", "/media/audio/classical/baroque/"},
|
||||
{"/media/audio/classical/mozart/track.mp3", "/media/audio/classical/"},
|
||||
{"/media/audio/rock/track.mp3", "/media/audio/"},
|
||||
{"/media/video/movie.mp4", "/media/"},
|
||||
{"/media/audio/classical/baroque/bach/track.mp3", 4, "/media/audio/classical/baroque"},
|
||||
{"/media/audio/classical/mozart/track.mp3", 3, "/media/audio/classical"},
|
||||
{"/media/audio/rock/track.mp3", 2, "/media/audio"},
|
||||
{"/media/video/movie.mp4", 1, "/media"},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
matched := libRegex.FindString(tc.path)
|
||||
Expect(matched).To(Equal(tc.expected), "Path %s should match %s, but got %s", tc.path, tc.expected, matched)
|
||||
libID, libPath := matcher.findLibraryForPath(tc.path)
|
||||
Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d", tc.path, tc.expectedLibID)
|
||||
Expect(libPath).To(Equal(tc.expectedLibPath), "Path %s should match library path %s", tc.path, tc.expectedLibPath)
|
||||
}
|
||||
})
|
||||
})
|
||||
@ -119,13 +124,14 @@ var _ = Describe("compileLibraryPaths", func() {
|
||||
It("handles empty library list", func() {
|
||||
mockLibRepo.SetData([]model.Library{})
|
||||
|
||||
libRegex, err := ps.compileLibraryPaths(ctx)
|
||||
matcher, err := ps.buildLibraryMatcher(ctx)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(libRegex).ToNot(BeNil())
|
||||
Expect(matcher).ToNot(BeNil())
|
||||
|
||||
// Should not match anything
|
||||
matched := libRegex.FindString("/music/track.mp3")
|
||||
Expect(matched).To(BeEmpty())
|
||||
libID, libPath := matcher.findLibraryForPath("/music/track.mp3")
|
||||
Expect(libID).To(Equal(0))
|
||||
Expect(libPath).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("handles single library", func() {
|
||||
@ -133,54 +139,55 @@ var _ = Describe("compileLibraryPaths", func() {
|
||||
{ID: 1, Path: "/music"},
|
||||
})
|
||||
|
||||
libRegex, err := ps.compileLibraryPaths(ctx)
|
||||
matcher, err := ps.buildLibraryMatcher(ctx)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
matched := libRegex.FindString("/music/track.mp3")
|
||||
Expect(matched).To(Equal("/music/"))
|
||||
libID, libPath := matcher.findLibraryForPath("/music/track.mp3")
|
||||
Expect(libID).To(Equal(1))
|
||||
Expect(libPath).To(Equal("/music"))
|
||||
})
|
||||
|
||||
It("handles libraries with special regex characters", func() {
|
||||
It("handles libraries with special characters in paths", func() {
|
||||
mockLibRepo.SetData([]model.Library{
|
||||
{ID: 1, Path: "/music[test]"},
|
||||
{ID: 2, Path: "/music(backup)"},
|
||||
})
|
||||
|
||||
libRegex, err := ps.compileLibraryPaths(ctx)
|
||||
matcher, err := ps.buildLibraryMatcher(ctx)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(libRegex).ToNot(BeNil())
|
||||
Expect(matcher).ToNot(BeNil())
|
||||
|
||||
// Special characters should be escaped and match literally
|
||||
matched := libRegex.FindString("/music[test]/track.mp3")
|
||||
Expect(matched).To(Equal("/music[test]/"))
|
||||
// Special characters should match literally
|
||||
libID, libPath := matcher.findLibraryForPath("/music[test]/track.mp3")
|
||||
Expect(libID).To(Equal(1))
|
||||
Expect(libPath).To(Equal("/music[test]"))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Regex pattern validation", func() {
|
||||
It("ensures regex alternation respects order by testing actual matching behavior", func() {
|
||||
Describe("Path matching order", func() {
|
||||
It("ensures longest paths match first", func() {
|
||||
mockLibRepo.SetData([]model.Library{
|
||||
{ID: 1, Path: "/a"},
|
||||
{ID: 2, Path: "/ab"},
|
||||
{ID: 3, Path: "/abc"},
|
||||
})
|
||||
|
||||
libRegex, err := ps.compileLibraryPaths(ctx)
|
||||
matcher, err := ps.buildLibraryMatcher(ctx)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
// Verify that longer paths match correctly (not cut off by shorter prefix)
|
||||
// If ordering is wrong, /ab would match before /abc for path "/abc/file"
|
||||
testCases := []struct {
|
||||
path string
|
||||
expected string
|
||||
path string
|
||||
expectedLibID int
|
||||
}{
|
||||
{"/abc/file.mp3", "/abc/"},
|
||||
{"/ab/file.mp3", "/ab/"},
|
||||
{"/a/file.mp3", "/a/"},
|
||||
{"/abc/file.mp3", 3},
|
||||
{"/ab/file.mp3", 2},
|
||||
{"/a/file.mp3", 1},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
matched := libRegex.FindString(tc.path)
|
||||
Expect(matched).To(Equal(tc.expected), "Path %s should match %s", tc.path, tc.expected)
|
||||
libID, _ := matcher.findLibraryForPath(tc.path)
|
||||
Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d", tc.path, tc.expectedLibID)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@ -265,6 +265,71 @@ var _ = Describe("Playlists", func() {
|
||||
Expect(pls.Tracks[0].Path).To(Equal("rock.mp3")) // From music library
|
||||
Expect(pls.Tracks[1].Path).To(Equal("bach.mp3")) // From music-classical library (not music!)
|
||||
})
|
||||
|
||||
It("correctly handles identical relative paths from different libraries", func() {
|
||||
// This tests the bug where two libraries have files at the same relative path
|
||||
// and only one appears in the playlist
|
||||
tmpDir := GinkgoT().TempDir()
|
||||
musicDir := tmpDir + "/music"
|
||||
classicalDir := tmpDir + "/classical"
|
||||
Expect(os.Mkdir(musicDir, 0755)).To(Succeed())
|
||||
Expect(os.Mkdir(classicalDir, 0755)).To(Succeed())
|
||||
Expect(os.MkdirAll(musicDir+"/album", 0755)).To(Succeed())
|
||||
Expect(os.MkdirAll(classicalDir+"/album", 0755)).To(Succeed())
|
||||
// Create placeholder files so paths resolve correctly
|
||||
Expect(os.WriteFile(musicDir+"/album/track.mp3", []byte{}, 0600)).To(Succeed())
|
||||
Expect(os.WriteFile(classicalDir+"/album/track.mp3", []byte{}, 0600)).To(Succeed())
|
||||
|
||||
// Both libraries have a file at "album/track.mp3"
|
||||
mockLibRepo.SetData([]model.Library{
|
||||
{ID: 1, Path: musicDir},
|
||||
{ID: 2, Path: classicalDir},
|
||||
})
|
||||
|
||||
// Mock returns files with same relative path but different IDs and library IDs
|
||||
// Keys use the library-qualified format: "libraryID:path"
|
||||
ds.MockedMediaFile = &mockedMediaFileRepo{
|
||||
data: map[string]model.MediaFile{
|
||||
"1:album/track.mp3": {ID: "music-track", Path: "album/track.mp3", LibraryID: 1, Title: "Rock Song"},
|
||||
"2:album/track.mp3": {ID: "classical-track", Path: "album/track.mp3", LibraryID: 2, Title: "Classical Piece"},
|
||||
},
|
||||
}
|
||||
// Recreate playlists service to pick up new mock
|
||||
ps = core.NewPlaylists(ds)
|
||||
|
||||
// Create playlist in music library that references both tracks
|
||||
plsContent := "#PLAYLIST:Same Path Test\nalbum/track.mp3\n../classical/album/track.mp3"
|
||||
plsFile := musicDir + "/test.m3u"
|
||||
Expect(os.WriteFile(plsFile, []byte(plsContent), 0600)).To(Succeed())
|
||||
|
||||
plsFolder := &model.Folder{
|
||||
ID: "1",
|
||||
LibraryID: 1,
|
||||
LibraryPath: musicDir,
|
||||
Path: "",
|
||||
Name: "",
|
||||
}
|
||||
|
||||
pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
// Should have BOTH tracks, not just one
|
||||
Expect(pls.Tracks).To(HaveLen(2), "Playlist should contain both tracks with same relative path")
|
||||
|
||||
// Verify we got tracks from DIFFERENT libraries (the key fix!)
|
||||
// Collect the library IDs
|
||||
libIDs := make(map[int]bool)
|
||||
for _, track := range pls.Tracks {
|
||||
libIDs[track.LibraryID] = true
|
||||
}
|
||||
Expect(libIDs).To(HaveLen(2), "Tracks should come from two different libraries")
|
||||
Expect(libIDs[1]).To(BeTrue(), "Should have track from library 1")
|
||||
Expect(libIDs[2]).To(BeTrue(), "Should have track from library 2")
|
||||
|
||||
// Both tracks should have the same relative path
|
||||
Expect(pls.Tracks[0].Path).To(Equal("album/track.mp3"))
|
||||
Expect(pls.Tracks[1].Path).To(Equal("album/track.mp3"))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -431,17 +496,43 @@ var _ = Describe("Playlists", func() {
|
||||
})
|
||||
})
|
||||
|
||||
// mockedMediaFileRepo's FindByPaths method returns a list of MediaFiles with the same paths as the input
|
||||
// mockedMediaFileRepo's FindByPaths method mimics the real implementation.
|
||||
// If data map is provided, it looks up files using the path as the key.
|
||||
// Otherwise, it creates MediaFile entries from the input paths.
|
||||
type mockedMediaFileRepo struct {
|
||||
model.MediaFileRepository
|
||||
data map[string]model.MediaFile
|
||||
}
|
||||
|
||||
func (r *mockedMediaFileRepo) FindByPaths(paths []string) (model.MediaFiles, error) {
|
||||
var mfs model.MediaFiles
|
||||
|
||||
// If data map is provided, use it to look up files
|
||||
if r.data != nil {
|
||||
for _, path := range paths {
|
||||
// Look up by the path key (test should use "path:libraryID" format for keys)
|
||||
if mf, ok := r.data[path]; ok {
|
||||
mfs = append(mfs, mf)
|
||||
}
|
||||
}
|
||||
return mfs, nil
|
||||
}
|
||||
|
||||
// Fallback: create MediaFile entries from paths
|
||||
// Parse library-qualified format: "libraryID:path"
|
||||
for idx, path := range paths {
|
||||
libraryID := 1 // Default library ID
|
||||
actualPath := path
|
||||
if parts := strings.SplitN(path, ":", 2); len(parts) == 2 {
|
||||
if libID, err := strconv.Atoi(parts[0]); err == nil {
|
||||
libraryID = libID
|
||||
actualPath = parts[1]
|
||||
}
|
||||
}
|
||||
mfs = append(mfs, model.MediaFile{
|
||||
ID: strconv.Itoa(idx),
|
||||
Path: path,
|
||||
ID: strconv.Itoa(idx),
|
||||
Path: actualPath,
|
||||
LibraryID: libraryID,
|
||||
})
|
||||
}
|
||||
return mfs, nil
|
||||
@ -453,13 +544,46 @@ type mockedMediaFileFromListRepo struct {
|
||||
data []string
|
||||
}
|
||||
|
||||
func (r *mockedMediaFileFromListRepo) FindByPaths([]string) (model.MediaFiles, error) {
|
||||
func (r *mockedMediaFileFromListRepo) FindByPaths(paths []string) (model.MediaFiles, error) {
|
||||
var mfs model.MediaFiles
|
||||
|
||||
// Build a map of requested paths with their library IDs (handle both qualified and unqualified)
|
||||
type pathRequest struct {
|
||||
path string
|
||||
libraryID int
|
||||
qualified bool
|
||||
}
|
||||
requests := make([]pathRequest, 0, len(paths))
|
||||
for _, path := range paths {
|
||||
req := pathRequest{path: path, libraryID: 1, qualified: false}
|
||||
if parts := strings.SplitN(path, ":", 2); len(parts) == 2 {
|
||||
if libID, err := strconv.Atoi(parts[0]); err == nil {
|
||||
req.libraryID = libID
|
||||
req.path = parts[1]
|
||||
req.qualified = true
|
||||
}
|
||||
}
|
||||
requests = append(requests, req)
|
||||
}
|
||||
|
||||
// Return files that match the requests
|
||||
// Need to use normalized comparison to handle Unicode normalization differences
|
||||
normalizeForComparison := func(s string) string {
|
||||
return strings.ToLower(norm.NFC.String(s))
|
||||
}
|
||||
|
||||
for idx, path := range r.data {
|
||||
mfs = append(mfs, model.MediaFile{
|
||||
ID: strconv.Itoa(idx),
|
||||
Path: path,
|
||||
})
|
||||
normalizedPath := normalizeForComparison(path)
|
||||
for _, req := range requests {
|
||||
if normalizeForComparison(req.path) == normalizedPath {
|
||||
mfs = append(mfs, model.MediaFile{
|
||||
ID: strconv.Itoa(idx),
|
||||
Path: path,
|
||||
LibraryID: req.libraryID,
|
||||
})
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
return mfs, nil
|
||||
}
|
||||
|
||||
@ -4,6 +4,8 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
@ -193,11 +195,40 @@ func (r *mediaFileRepository) GetCursor(options ...model.QueryOptions) (model.Me
|
||||
}
|
||||
|
||||
func (r *mediaFileRepository) FindByPaths(paths []string) (model.MediaFiles, error) {
|
||||
sel := r.newSelect().Columns("*").Where(Eq{"path collate nocase": paths})
|
||||
// Parse library-qualified paths and build query
|
||||
// Format: "libraryID:path" or just "path" (backward compatibility)
|
||||
query := Or{}
|
||||
|
||||
for _, path := range paths {
|
||||
parts := strings.SplitN(path, ":", 2)
|
||||
if len(parts) == 2 {
|
||||
// Library-qualified path: "libraryID:path"
|
||||
libraryID, err := strconv.Atoi(parts[0])
|
||||
if err != nil {
|
||||
// Invalid format, skip
|
||||
continue
|
||||
}
|
||||
relativePath := parts[1]
|
||||
query = append(query, And{
|
||||
Eq{"path collate nocase": relativePath},
|
||||
Eq{"library_id": libraryID},
|
||||
})
|
||||
} else {
|
||||
// Unqualified path: search across all libraries
|
||||
query = append(query, Eq{"path collate nocase": path})
|
||||
}
|
||||
}
|
||||
|
||||
if len(query) == 0 {
|
||||
return model.MediaFiles{}, nil
|
||||
}
|
||||
|
||||
sel := r.newSelect().Columns("*").Where(query)
|
||||
var res dbMediaFiles
|
||||
if err := r.queryAll(sel, &res); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return res.toModels(), nil
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user