diff --git a/core/playlists.go b/core/playlists.go index d712aed25..4ec5d376a 100644 --- a/core/playlists.go +++ b/core/playlists.go @@ -195,12 +195,13 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m } filteredLines = append(filteredLines, line) } - pathsWithLibrary, err := s.normalizePaths(ctx, pls, folder, filteredLines) + resolvedPaths, err := s.resolvePaths(ctx, folder, filteredLines) if err != nil { - log.Warn(ctx, "Error normalizing paths in playlist", "playlist", pls.Name, err) + log.Warn(ctx, "Error resolving paths in playlist", "playlist", pls.Name, err) continue } - found, err := mediaFileRepository.FindByPaths(pathsWithLibrary) + + found, err := mediaFileRepository.FindByPaths(resolvedPaths) if err != nil { log.Warn(ctx, "Error reading files from DB", "playlist", pls.Name, err) continue @@ -212,7 +213,7 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m key := fmt.Sprintf("%d:%s", found[idx].LibraryID, normalizePathForComparison(found[idx].Path)) existing[key] = idx } - for _, path := range pathsWithLibrary { + for _, path := range resolvedPaths { // Parse the library-qualified path parts := strings.SplitN(path, ":", 2) // Path is already qualified: "libraryID:path" @@ -250,8 +251,9 @@ type pathResolution struct { valid bool } -// toLibraryQualifiedPath converts the absolute path to a library-qualified path format: "libraryID:relativePath". -func (r pathResolution) toLibraryQualifiedPath() (string, error) { +// ToQualifiedString converts the path resolution to a library-qualified string with forward slashes. +// Format: "libraryID:relativePath" with forward slashes for path separators. +func (r pathResolution) ToQualifiedString() (string, error) { if !r.valid { return "", fmt.Errorf("invalid path resolution") } @@ -259,17 +261,8 @@ func (r pathResolution) toLibraryQualifiedPath() (string, error) { 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, libraryID int) pathResolution { - return pathResolution{ - absolutePath: absolutePath, - libraryPath: libraryPath, - libraryID: libraryID, - valid: true, - } + // Convert path separators to forward slashes + return fmt.Sprintf("%d:%s", r.libraryID, filepath.ToSlash(relativePath)), nil } // libraryMatcher holds sorted libraries with cleaned paths for efficient path matching. @@ -315,99 +308,121 @@ func newLibraryMatcher(libs model.Libraries) *libraryMatcher { } } -// 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) { - matcher, err := s.buildLibraryMatcher(ctx) +// pathResolver handles path resolution logic for playlist imports. +type pathResolver struct { + matcher *libraryMatcher +} + +// newPathResolver creates a pathResolver with libraries loaded from the datastore. +func newPathResolver(ctx context.Context, ds model.DataStore) (*pathResolver, error) { + matcher, err := buildLibraryMatcher(ctx, ds) if err != nil { return nil, err } + return &pathResolver{matcher: matcher}, nil +} - res := make([]string, 0, len(lines)) - for idx, line := range lines { - 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 - } - - qualifiedPath, err := resolution.toLibraryQualifiedPath() - if err != nil { - log.Debug(ctx, "Error getting library-qualified path", "playlist", pls.Name, "path", line, - "libPath", resolution.libraryPath, "filePath", resolution.absolutePath, err) - continue - } - - res = append(res, qualifiedPath) +// buildLibraryMatcher creates a libraryMatcher with libraries sorted by path length (longest first). +func buildLibraryMatcher(ctx context.Context, ds model.DataStore) (*libraryMatcher, error) { + libs, err := ds.Library(ctx).GetAll() + if err != nil { + return nil, err } - // 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 + return newLibraryMatcher(libs), nil } // resolvePath determines the absolute path and library path for a playlist entry. -func (s *playlists) resolvePath(line string, folder *model.Folder, matcher *libraryMatcher) pathResolution { +func (r *pathResolver) resolvePath(line string, folder *model.Folder) pathResolution { if folder != nil && !filepath.IsAbs(line) { - return s.resolveRelativePath(line, folder, matcher) + return r.resolveRelativePath(line, folder) } - return s.resolveAbsolutePath(line, matcher) + return r.resolveAbsolutePath(line) } // 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, matcher *libraryMatcher) pathResolution { +func (r *pathResolver) resolveRelativePath(line string, folder *model.Folder) 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 - libID, libPath := matcher.findLibraryForPath(absolutePath) + libID, libPath := r.matcher.findLibraryForPath(absolutePath) if libID != 0 { - return newValidResolution(absolutePath, libPath, libID) + return pathResolution{ + absolutePath: absolutePath, + libraryPath: libPath, + libraryID: libID, + valid: true, + } } // Fallback: Check if it's in the playlist's own library - return s.validatePathInLibrary(absolutePath, folder.LibraryPath, folder.LibraryID) + return r.validatePathInLibrary(absolutePath, folder.LibraryPath, folder.LibraryID) } // resolveAbsolutePath handles absolute paths by matching them against library paths. -func (s *playlists) resolveAbsolutePath(line string, matcher *libraryMatcher) pathResolution { +func (r *pathResolver) resolveAbsolutePath(line string) pathResolution { cleanPath := filepath.Clean(line) - libID, libPath := matcher.findLibraryForPath(cleanPath) + libID, libPath := r.matcher.findLibraryForPath(cleanPath) if libID == 0 { return pathResolution{valid: false} } - return newValidResolution(cleanPath, libPath, libID) + return pathResolution{ + absolutePath: cleanPath, + libraryPath: libPath, + libraryID: libID, + valid: true, + } } // 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, libraryID int) pathResolution { +func (r *pathResolver) 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, libraryID) + return pathResolution{ + absolutePath: absolutePath, + libraryPath: libraryPath, + libraryID: libraryID, + valid: true, + } } -// 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() +// resolvePaths 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) resolvePaths(ctx context.Context, folder *model.Folder, lines []string) ([]string, error) { + resolver, err := newPathResolver(ctx, s.ds) if err != nil { return nil, err } - return newLibraryMatcher(libs), nil + results := make([]string, 0, len(lines)) + for idx, line := range lines { + resolution := resolver.resolvePath(line, folder) + + if !resolution.valid { + log.Warn(ctx, "Path in playlist not found in any library", "path", line, "line", idx) + continue + } + + qualifiedPath, err := resolution.ToQualifiedString() + if err != nil { + log.Debug(ctx, "Error getting library-qualified path", "path", line, + "libPath", resolution.libraryPath, "filePath", resolution.absolutePath, err) + continue + } + + results = append(results, qualifiedPath) + } + + return results, nil } func (s *playlists) updatePlaylist(ctx context.Context, newPls *model.Playlist) error { diff --git a/core/playlists_internal_test.go b/core/playlists_internal_test.go index 6ed57a525..67abd0f8b 100644 --- a/core/playlists_internal_test.go +++ b/core/playlists_internal_test.go @@ -13,7 +13,6 @@ import ( var _ = Describe("buildLibraryMatcher", func() { var ds *tests.MockDataStore var mockLibRepo *tests.MockLibraryRepo - var ps *playlists ctx := context.Background() BeforeEach(func() { @@ -21,7 +20,6 @@ var _ = Describe("buildLibraryMatcher", func() { ds = &tests.MockDataStore{ MockedLibrary: mockLibRepo, } - ps = &playlists{ds: ds} }) Describe("Longest library path matching", func() { @@ -33,7 +31,7 @@ var _ = Describe("buildLibraryMatcher", func() { {ID: 3, Path: "/music-classical/opera"}, }) - matcher, err := ps.buildLibraryMatcher(ctx) + matcher, err := buildLibraryMatcher(ctx, ds) Expect(err).ToNot(HaveOccurred()) // Test that longest path matches first and returns correct library ID @@ -61,7 +59,7 @@ var _ = Describe("buildLibraryMatcher", func() { {ID: 2, Path: "/home/user/music-backup"}, }) - matcher, err := ps.buildLibraryMatcher(ctx) + matcher, err := buildLibraryMatcher(ctx, ds) Expect(err).ToNot(HaveOccurred()) // Test that music-backup library is matched correctly @@ -81,7 +79,7 @@ var _ = Describe("buildLibraryMatcher", func() { {ID: 2, Path: "/music-classical"}, }) - matcher, err := ps.buildLibraryMatcher(ctx) + matcher, err := buildLibraryMatcher(ctx, ds) Expect(err).ToNot(HaveOccurred()) // Exact library path should match @@ -98,7 +96,7 @@ var _ = Describe("buildLibraryMatcher", func() { {ID: 4, Path: "/media/audio/classical/baroque"}, }) - matcher, err := ps.buildLibraryMatcher(ctx) + matcher, err := buildLibraryMatcher(ctx, ds) Expect(err).ToNot(HaveOccurred()) testCases := []struct { @@ -124,7 +122,7 @@ var _ = Describe("buildLibraryMatcher", func() { It("handles empty library list", func() { mockLibRepo.SetData([]model.Library{}) - matcher, err := ps.buildLibraryMatcher(ctx) + matcher, err := buildLibraryMatcher(ctx, ds) Expect(err).ToNot(HaveOccurred()) Expect(matcher).ToNot(BeNil()) @@ -139,7 +137,7 @@ var _ = Describe("buildLibraryMatcher", func() { {ID: 1, Path: "/music"}, }) - matcher, err := ps.buildLibraryMatcher(ctx) + matcher, err := buildLibraryMatcher(ctx, ds) Expect(err).ToNot(HaveOccurred()) libID, libPath := matcher.findLibraryForPath("/music/track.mp3") @@ -153,7 +151,7 @@ var _ = Describe("buildLibraryMatcher", func() { {ID: 2, Path: "/music(backup)"}, }) - matcher, err := ps.buildLibraryMatcher(ctx) + matcher, err := buildLibraryMatcher(ctx, ds) Expect(err).ToNot(HaveOccurred()) Expect(matcher).ToNot(BeNil()) @@ -172,7 +170,7 @@ var _ = Describe("buildLibraryMatcher", func() { {ID: 3, Path: "/abc"}, }) - matcher, err := ps.buildLibraryMatcher(ctx) + matcher, err := buildLibraryMatcher(ctx, ds) Expect(err).ToNot(HaveOccurred()) // Verify that longer paths match correctly (not cut off by shorter prefix) @@ -193,6 +191,248 @@ var _ = Describe("buildLibraryMatcher", func() { }) }) +var _ = Describe("pathResolver", func() { + var ds *tests.MockDataStore + var mockLibRepo *tests.MockLibraryRepo + var resolver *pathResolver + ctx := context.Background() + + BeforeEach(func() { + mockLibRepo = &tests.MockLibraryRepo{} + ds = &tests.MockDataStore{ + MockedLibrary: mockLibRepo, + } + + // Setup test libraries + mockLibRepo.SetData([]model.Library{ + {ID: 1, Path: "/music"}, + {ID: 2, Path: "/music-classical"}, + {ID: 3, Path: "/podcasts"}, + }) + + var err error + resolver, err = newPathResolver(ctx, ds) + Expect(err).ToNot(HaveOccurred()) + }) + + Describe("resolvePath", func() { + It("resolves absolute paths", func() { + resolution := resolver.resolvePath("/music/artist/album/track.mp3", nil) + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(1)) + Expect(resolution.libraryPath).To(Equal("/music")) + Expect(resolution.absolutePath).To(Equal("/music/artist/album/track.mp3")) + }) + + It("resolves relative paths when folder is provided", func() { + folder := &model.Folder{ + Path: "playlists", + LibraryPath: "/music", + LibraryID: 1, + } + + resolution := resolver.resolvePath("../artist/album/track.mp3", folder) + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(1)) + Expect(resolution.absolutePath).To(Equal("/music/artist/album/track.mp3")) + }) + + It("returns invalid resolution for paths outside any library", func() { + resolution := resolver.resolvePath("/outside/library/track.mp3", nil) + + Expect(resolution.valid).To(BeFalse()) + }) + }) + + Describe("resolveAbsolutePath", func() { + It("resolves path within a library", func() { + resolution := resolver.resolveAbsolutePath("/music/track.mp3") + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(1)) + Expect(resolution.libraryPath).To(Equal("/music")) + Expect(resolution.absolutePath).To(Equal("/music/track.mp3")) + }) + + It("resolves path to the longest matching library", func() { + resolution := resolver.resolveAbsolutePath("/music-classical/track.mp3") + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(2)) + Expect(resolution.libraryPath).To(Equal("/music-classical")) + }) + + It("returns invalid resolution for path outside libraries", func() { + resolution := resolver.resolveAbsolutePath("/videos/movie.mp4") + + Expect(resolution.valid).To(BeFalse()) + }) + + It("cleans the path before matching", func() { + resolution := resolver.resolveAbsolutePath("/music//artist/../artist/track.mp3") + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.absolutePath).To(Equal("/music/artist/track.mp3")) + }) + }) + + Describe("resolveRelativePath", func() { + It("resolves relative path within same library", func() { + folder := &model.Folder{ + Path: "playlists", + LibraryPath: "/music", + LibraryID: 1, + } + + resolution := resolver.resolveRelativePath("../songs/track.mp3", folder) + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(1)) + Expect(resolution.absolutePath).To(Equal("/music/songs/track.mp3")) + }) + + It("resolves relative path to different library", func() { + folder := &model.Folder{ + Path: "playlists", + LibraryPath: "/music", + LibraryID: 1, + } + + // Path goes up and into a different library + resolution := resolver.resolveRelativePath("../../podcasts/episode.mp3", folder) + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(3)) + Expect(resolution.libraryPath).To(Equal("/podcasts")) + }) + + It("uses matcher to find correct library for resolved path", func() { + folder := &model.Folder{ + Path: "playlists", + LibraryPath: "/music", + LibraryID: 1, + } + + // This relative path resolves to music-classical library + resolution := resolver.resolveRelativePath("../../music-classical/track.mp3", folder) + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(2)) + Expect(resolution.libraryPath).To(Equal("/music-classical")) + }) + + It("returns invalid for relative paths escaping all libraries", func() { + folder := &model.Folder{ + Path: "playlists", + LibraryPath: "/music", + LibraryID: 1, + } + + resolution := resolver.resolveRelativePath("../../../../etc/passwd", folder) + + Expect(resolution.valid).To(BeFalse()) + }) + }) + + Describe("validatePathInLibrary", func() { + It("validates path within library", func() { + resolution := resolver.validatePathInLibrary("/music/artist/track.mp3", "/music", 1) + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(1)) + Expect(resolution.libraryPath).To(Equal("/music")) + }) + + It("rejects path that escapes library using ..", func() { + resolution := resolver.validatePathInLibrary("/music/../etc/passwd", "/music", 1) + + Expect(resolution.valid).To(BeFalse()) + }) + + It("rejects path outside library", func() { + resolution := resolver.validatePathInLibrary("/podcasts/track.mp3", "/music", 1) + + Expect(resolution.valid).To(BeFalse()) + }) + + It("accepts path exactly at library root", func() { + resolution := resolver.validatePathInLibrary("/music", "/music", 1) + + Expect(resolution.valid).To(BeTrue()) + }) + }) + + Describe("Cross-library resolution scenarios", func() { + It("handles playlist in library A referencing file in library B", func() { + // Playlist is in /music/playlists + folder := &model.Folder{ + Path: "playlists", + LibraryPath: "/music", + LibraryID: 1, + } + + // Relative path that goes to /podcasts library + resolution := resolver.resolvePath("../../podcasts/show/episode.mp3", folder) + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(3), "Should resolve to podcasts library") + Expect(resolution.libraryPath).To(Equal("/podcasts")) + }) + + It("prefers longer library paths when resolving", func() { + // Ensure /music-classical is matched instead of /music + resolution := resolver.resolvePath("/music-classical/baroque/track.mp3", nil) + + Expect(resolution.valid).To(BeTrue()) + Expect(resolution.libraryID).To(Equal(2), "Should match /music-classical, not /music") + }) + }) +}) + +var _ = Describe("pathResolution", func() { + Describe("ToQualifiedString", func() { + It("converts valid resolution to qualified string with forward slashes", func() { + resolution := pathResolution{ + absolutePath: "/music/artist/album/track.mp3", + libraryPath: "/music", + libraryID: 1, + valid: true, + } + + qualifiedStr, err := resolution.ToQualifiedString() + + Expect(err).ToNot(HaveOccurred()) + Expect(qualifiedStr).To(Equal("1:artist/album/track.mp3")) + }) + + It("handles Windows-style paths by converting to forward slashes", func() { + resolution := pathResolution{ + absolutePath: "/music/artist/album/track.mp3", + libraryPath: "/music", + libraryID: 2, + valid: true, + } + + qualifiedStr, err := resolution.ToQualifiedString() + + Expect(err).ToNot(HaveOccurred()) + // Should always use forward slashes regardless of OS + Expect(qualifiedStr).To(ContainSubstring("2:")) + Expect(qualifiedStr).ToNot(ContainSubstring("\\")) + }) + + It("returns error for invalid resolution", func() { + resolution := pathResolution{valid: false} + + _, err := resolution.ToQualifiedString() + + Expect(err).To(HaveOccurred()) + }) + }) +}) + var _ = Describe("normalizePathForComparison", func() { It("normalizes Unicode characters to NFC form and converts to lowercase", func() { // Test with NFD (decomposed) input - as would come from macOS filesystem