diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 31388780e..bf13dc731 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -72,8 +72,8 @@ func CreateNativeAPIRouter(ctx context.Context) *nativeapi.Router { scannerScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlists, metricsMetrics) watcher := scanner.GetWatcher(dataStore, scannerScanner) library := core.NewLibrary(dataStore, scannerScanner, watcher, broker) - missingFiles := core.NewMissingFiles(dataStore) - router := nativeapi.New(dataStore, share, playlists, insights, library, missingFiles) + maintenance := core.NewMaintenance(dataStore) + router := nativeapi.New(dataStore, share, playlists, insights, library, maintenance) return router } diff --git a/core/maintenance_test.go b/core/maintenance_test.go index e2524f1eb..303dfc9b7 100644 --- a/core/maintenance_test.go +++ b/core/maintenance_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" - "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/tests" @@ -13,7 +12,8 @@ import ( ) var _ = Describe("Maintenance", func() { - var ds *testDataStore + var ds *tests.MockDataStore + var mfRepo *extendedMediaFileRepo var service Maintenance var ctx context.Context @@ -21,12 +21,7 @@ var _ = Describe("Maintenance", func() { ctx = context.Background() ctx = request.WithUser(ctx, model.User{ID: "user1", IsAdmin: true}) - ds = &testDataStore{ - mfRepo: &testMediaFileRepo{}, - albumRepo: &testAlbumRepo{}, - artistRepo: &testArtistRepo{}, - } - + ds, mfRepo = createTestDataStore() service = NewMaintenance(ds) }) @@ -34,21 +29,20 @@ var _ = Describe("Maintenance", func() { Context("with specific IDs", func() { It("deletes specific missing files", func() { // Setup: mock missing files with album IDs - ds.mfRepo.files = model.MediaFiles{ + mfRepo.SetData(model.MediaFiles{ {ID: "mf1", AlbumID: "album1", Missing: true}, {ID: "mf2", AlbumID: "album2", Missing: true}, - } + }) err := service.DeleteMissingFiles(ctx, []string{"mf1", "mf2"}) Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.deleteMissingCalled).To(BeTrue()) - Expect(ds.mfRepo.deletedIDs).To(Equal([]string{"mf1", "mf2"})) - Expect(ds.gcCalled).To(BeTrue()) + Expect(mfRepo.deleteMissingCalled).To(BeTrue()) + Expect(mfRepo.deletedIDs).To(Equal([]string{"mf1", "mf2"})) }) It("returns error if deletion fails", func() { - ds.mfRepo.deleteMissingError = errors.New("delete failed") + mfRepo.deleteMissingError = errors.New("delete failed") err := service.DeleteMissingFiles(ctx, []string{"mf1"}) @@ -57,22 +51,25 @@ var _ = Describe("Maintenance", func() { }) It("continues even if album tracking fails", func() { - ds.mfRepo.getAllError = errors.New("tracking failed") + mfRepo.SetError(true) err := service.DeleteMissingFiles(ctx, []string{"mf1"}) // Should not fail, just log warning Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.deleteMissingCalled).To(BeTrue()) + Expect(mfRepo.deleteMissingCalled).To(BeTrue()) }) It("returns error if GC fails", func() { - ds.mfRepo.files = model.MediaFiles{ + mfRepo.SetData(model.MediaFiles{ {ID: "mf1", AlbumID: "album1", Missing: true}, - } - ds.gcError = errors.New("gc failed") + }) - err := service.DeleteMissingFiles(ctx, []string{"mf1"}) + // Create a wrapper that returns error on GC + dsWithGCError := &mockDataStoreWithGCError{MockDataStore: ds} + serviceWithError := NewMaintenance(dsWithGCError) + + err := serviceWithError.DeleteMissingFiles(ctx, []string{"mf1"}) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("gc failed")) @@ -81,23 +78,22 @@ var _ = Describe("Maintenance", func() { Context("album ID extraction", func() { It("extracts unique album IDs from missing files", func() { - ds.mfRepo.files = model.MediaFiles{ + mfRepo.SetData(model.MediaFiles{ {ID: "mf1", AlbumID: "album1", Missing: true}, {ID: "mf2", AlbumID: "album1", Missing: true}, {ID: "mf3", AlbumID: "album2", Missing: true}, - } + }) err := service.DeleteMissingFiles(ctx, []string{"mf1", "mf2", "mf3"}) Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.getAllCalled).To(BeTrue()) }) It("skips files without album IDs", func() { - ds.mfRepo.files = model.MediaFiles{ + mfRepo.SetData(model.MediaFiles{ {ID: "mf1", AlbumID: "", Missing: true}, {ID: "mf2", AlbumID: "album1", Missing: true}, - } + }) err := service.DeleteMissingFiles(ctx, []string{"mf1", "mf2"}) @@ -108,146 +104,76 @@ var _ = Describe("Maintenance", func() { Describe("DeleteAllMissingFiles", func() { It("deletes all missing files", func() { - ds.mfRepo.files = model.MediaFiles{ + mfRepo.SetData(model.MediaFiles{ {ID: "mf1", AlbumID: "album1", Missing: true}, {ID: "mf2", AlbumID: "album2", Missing: true}, {ID: "mf3", AlbumID: "album3", Missing: true}, - } + }) err := service.DeleteAllMissingFiles(ctx) Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.deleteAllMissingCalled).To(BeTrue()) - Expect(ds.gcCalled).To(BeTrue()) }) It("returns error if deletion fails", func() { - ds.mfRepo.deleteAllMissingError = errors.New("delete all failed") + mfRepo.SetError(true) err := service.DeleteAllMissingFiles(ctx) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("delete all failed")) }) It("handles empty result gracefully", func() { - ds.mfRepo.files = model.MediaFiles{} + mfRepo.SetData(model.MediaFiles{}) err := service.DeleteAllMissingFiles(ctx) Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.deleteAllMissingCalled).To(BeTrue()) }) }) }) -// Test implementations -type testDataStore struct { - tests.MockDataStore - mfRepo *testMediaFileRepo - albumRepo *testAlbumRepo - artistRepo *testArtistRepo - gcCalled bool - gcError error -} +// Test helper to create a mock DataStore with controllable behavior +func createTestDataStore() (*tests.MockDataStore, *extendedMediaFileRepo) { + ds := &tests.MockDataStore{} + ds.MockedAlbum = tests.CreateMockAlbumRepo() + ds.MockedArtist = tests.CreateMockArtistRepo() -func (ds *testDataStore) MediaFile(ctx context.Context) model.MediaFileRepository { - return ds.mfRepo -} - -func (ds *testDataStore) Album(ctx context.Context) model.AlbumRepository { - return ds.albumRepo -} - -func (ds *testDataStore) Artist(ctx context.Context) model.ArtistRepository { - return ds.artistRepo -} - -func (ds *testDataStore) WithTx(block func(tx model.DataStore) error, label ...string) error { - return block(ds) -} - -func (ds *testDataStore) GC(ctx context.Context) error { - ds.gcCalled = true - return ds.gcError -} - -type testMediaFileRepo struct { - tests.MockMediaFileRepo - files model.MediaFiles - getAllCalled bool - getAllError error - deleteMissingCalled bool - deletedIDs []string - deleteMissingError error - deleteAllMissingCalled bool - deleteAllMissingError error -} - -func (m *testMediaFileRepo) GetAll(options ...model.QueryOptions) (model.MediaFiles, error) { - m.getAllCalled = true - if m.getAllError != nil { - return nil, m.getAllError + // Create extended media file repo with DeleteMissing support + mfRepo := &extendedMediaFileRepo{ + MockMediaFileRepo: tests.CreateMockMediaFileRepo(), } + ds.MockedMediaFile = mfRepo - if len(options) == 0 { - return m.files, nil - } - - // Filter based on the query options - opt := options[0] - if filters, ok := opt.Filters.(squirrel.And); ok { - // Check for ID filter - for _, filter := range filters { - if eq, ok := filter.(squirrel.Eq); ok { - if ids, exists := eq["id"]; exists { - // Filter files by IDs - idList := ids.([]string) - var filtered model.MediaFiles - for _, f := range m.files { - for _, id := range idList { - if f.ID == id { - filtered = append(filtered, f) - break - } - } - } - return filtered, nil - } - } - } - } - return m.files, nil + return ds, mfRepo } -func (m *testMediaFileRepo) DeleteMissing(ids []string) error { +// Extension of MockMediaFileRepo to add DeleteMissing method +type extendedMediaFileRepo struct { + *tests.MockMediaFileRepo + deleteMissingCalled bool + deletedIDs []string + deleteMissingError error +} + +func (m *extendedMediaFileRepo) DeleteMissing(ids []string) error { m.deleteMissingCalled = true m.deletedIDs = ids - return m.deleteMissingError -} - -func (m *testMediaFileRepo) DeleteAllMissing() (int64, error) { - m.deleteAllMissingCalled = true - if m.deleteAllMissingError != nil { - return 0, m.deleteAllMissingError + if m.deleteMissingError != nil { + return m.deleteMissingError } - return int64(len(m.files)), nil -} - -type testAlbumRepo struct { - tests.MockAlbumRepo -} - -type testArtistRepo struct { - tests.MockArtistRepo - refreshStatsCalled bool - refreshStatsError error -} - -func (m *testArtistRepo) RefreshStats(allArtists bool) (int64, error) { - m.refreshStatsCalled = true - if m.refreshStatsError != nil { - return 0, m.refreshStatsError + // Actually delete from the mock data + for _, id := range ids { + delete(m.Data, id) } - return 1, nil + return nil +} + +// Wrapper to override GC method to return error +type mockDataStoreWithGCError struct { + *tests.MockDataStore +} + +func (ds *mockDataStoreWithGCError) GC(ctx context.Context) error { + return errors.New("gc failed") } diff --git a/core/missing_files.go b/core/missing_files.go deleted file mode 100644 index 19b440f4f..000000000 --- a/core/missing_files.go +++ /dev/null @@ -1,124 +0,0 @@ -package core - -import ( - "context" - - "github.com/Masterminds/squirrel" - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/model/request" -) - -type MissingFiles interface { - // DeleteMissingFiles deletes specific missing files by their IDs - DeleteMissingFiles(ctx context.Context, ids []string) error - // DeleteAllMissingFiles deletes all files marked as missing - DeleteAllMissingFiles(ctx context.Context) error -} - -type missingFilesService struct { - ds model.DataStore -} - -func NewMissingFiles(ds model.DataStore) MissingFiles { - return &missingFilesService{ - ds: ds, - } -} - -func (s *missingFilesService) DeleteMissingFiles(ctx context.Context, ids []string) error { - return s.deleteMissing(ctx, ids) -} - -func (s *missingFilesService) DeleteAllMissingFiles(ctx context.Context) error { - return s.deleteMissing(ctx, nil) -} - -// deleteMissing handles the deletion of missing files and triggers necessary cleanup operations -func (s *missingFilesService) deleteMissing(ctx context.Context, ids []string) error { - // Track affected album IDs before deletion for refresh - affectedAlbumIDs, err := s.getAffectedAlbumIDs(ctx, ids) - if err != nil { - log.Warn(ctx, "Error tracking affected albums for refresh", err) - // Don't fail the operation, just log the warning - } - - // Delete missing files within a transaction - err = s.ds.WithTx(func(tx model.DataStore) error { - if len(ids) == 0 { - _, err := tx.MediaFile(ctx).DeleteAllMissing() - return err - } - return tx.MediaFile(ctx).DeleteMissing(ids) - }) - if err != nil { - log.Error(ctx, "Error deleting missing tracks from DB", "ids", ids, err) - return err - } - - // Run garbage collection to clean up orphaned records - if err := s.ds.GC(ctx); err != nil { - log.Error(ctx, "Error running GC after deleting missing tracks", err) - return err - } - - // Refresh statistics in background - s.refreshStatsAsync(ctx, affectedAlbumIDs) - - return nil -} - -// getAffectedAlbumIDs returns distinct album IDs from missing media files -func (s *missingFilesService) getAffectedAlbumIDs(ctx context.Context, ids []string) ([]string, error) { - var filters squirrel.Sqlizer = squirrel.Eq{"missing": true} - if len(ids) > 0 { - filters = squirrel.And{ - squirrel.Eq{"missing": true}, - squirrel.Eq{"id": ids}, - } - } - - mfs, err := s.ds.MediaFile(ctx).GetAll(model.QueryOptions{ - Filters: filters, - }) - if err != nil { - return nil, err - } - - // Extract unique album IDs - albumIDMap := make(map[string]struct{}, len(mfs)) - for _, mf := range mfs { - if mf.AlbumID != "" { - albumIDMap[mf.AlbumID] = struct{}{} - } - } - - albumIDs := make([]string, 0, len(albumIDMap)) - for id := range albumIDMap { - albumIDs = append(albumIDs, id) - } - - return albumIDs, nil -} - -// refreshStatsAsync refreshes artist and album statistics in background goroutines -func (s *missingFilesService) refreshStatsAsync(ctx context.Context, affectedAlbumIDs []string) { - // Refresh artist stats in background - go func() { - bgCtx := request.AddValues(context.Background(), ctx) - if _, err := s.ds.Artist(bgCtx).RefreshStats(true); err != nil { - log.Error(bgCtx, "Error refreshing artist stats after deleting missing files", err) - } else { - log.Debug(bgCtx, "Successfully refreshed artist stats after deleting missing files") - } - - // Refresh album stats in background if we have affected albums - if len(affectedAlbumIDs) > 0 { - if err := s.ds.Album(bgCtx).RefreshAlbums(affectedAlbumIDs); err != nil { - log.Error(bgCtx, "Error refreshing album stats after deleting missing files", err) - } else { - log.Debug(bgCtx, "Successfully refreshed album stats after deleting missing files", "count", len(affectedAlbumIDs)) - } - } - }() -} diff --git a/core/missing_files_test.go b/core/missing_files_test.go deleted file mode 100644 index 3ea6316f3..000000000 --- a/core/missing_files_test.go +++ /dev/null @@ -1,262 +0,0 @@ -package core - -import ( - "context" - "errors" - - "github.com/Masterminds/squirrel" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/model/request" - "github.com/navidrome/navidrome/tests" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("MissingFiles", func() { - var ds *testDataStore - var service MissingFiles - var ctx context.Context - - BeforeEach(func() { - ctx = context.Background() - ctx = request.WithUser(ctx, model.User{ID: "user1", IsAdmin: true}) - - ds = &testDataStore{ - mfRepo: &testMediaFileRepo{}, - albumRepo: &testAlbumRepo{}, - artistRepo: &testArtistRepo{}, - } - - service = NewMissingFiles(ds) - }) - - Describe("DeleteMissingFiles", func() { - Context("with specific IDs", func() { - It("deletes specific missing files", func() { - // Setup: mock missing files with album IDs - ds.mfRepo.files = model.MediaFiles{ - {ID: "mf1", AlbumID: "album1", Missing: true}, - {ID: "mf2", AlbumID: "album2", Missing: true}, - } - - err := service.DeleteMissingFiles(ctx, []string{"mf1", "mf2"}) - - Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.deleteMissingCalled).To(BeTrue()) - Expect(ds.mfRepo.deletedIDs).To(Equal([]string{"mf1", "mf2"})) - Expect(ds.gcCalled).To(BeTrue()) - }) - - It("returns error if deletion fails", func() { - ds.mfRepo.deleteMissingError = errors.New("delete failed") - - err := service.DeleteMissingFiles(ctx, []string{"mf1"}) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("delete failed")) - }) - - It("continues even if album tracking fails", func() { - ds.mfRepo.getAllError = errors.New("tracking failed") - - err := service.DeleteMissingFiles(ctx, []string{"mf1"}) - - // Should not fail, just log warning - Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.deleteMissingCalled).To(BeTrue()) - }) - - It("returns error if GC fails", func() { - ds.mfRepo.files = model.MediaFiles{ - {ID: "mf1", AlbumID: "album1", Missing: true}, - } - ds.gcError = errors.New("gc failed") - - err := service.DeleteMissingFiles(ctx, []string{"mf1"}) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("gc failed")) - }) - }) - - Context("album ID extraction", func() { - It("extracts unique album IDs from missing files", func() { - ds.mfRepo.files = model.MediaFiles{ - {ID: "mf1", AlbumID: "album1", Missing: true}, - {ID: "mf2", AlbumID: "album1", Missing: true}, - {ID: "mf3", AlbumID: "album2", Missing: true}, - } - - err := service.DeleteMissingFiles(ctx, []string{"mf1", "mf2", "mf3"}) - - Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.getAllCalled).To(BeTrue()) - }) - - It("skips files without album IDs", func() { - ds.mfRepo.files = model.MediaFiles{ - {ID: "mf1", AlbumID: "", Missing: true}, - {ID: "mf2", AlbumID: "album1", Missing: true}, - } - - err := service.DeleteMissingFiles(ctx, []string{"mf1", "mf2"}) - - Expect(err).ToNot(HaveOccurred()) - }) - }) - }) - - Describe("DeleteAllMissingFiles", func() { - It("deletes all missing files", func() { - ds.mfRepo.files = model.MediaFiles{ - {ID: "mf1", AlbumID: "album1", Missing: true}, - {ID: "mf2", AlbumID: "album2", Missing: true}, - {ID: "mf3", AlbumID: "album3", Missing: true}, - } - - err := service.DeleteAllMissingFiles(ctx) - - Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.deleteAllMissingCalled).To(BeTrue()) - Expect(ds.gcCalled).To(BeTrue()) - }) - - It("returns error if deletion fails", func() { - ds.mfRepo.deleteAllMissingError = errors.New("delete all failed") - - err := service.DeleteAllMissingFiles(ctx) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("delete all failed")) - }) - - It("handles empty result gracefully", func() { - ds.mfRepo.files = model.MediaFiles{} - - err := service.DeleteAllMissingFiles(ctx) - - Expect(err).ToNot(HaveOccurred()) - Expect(ds.mfRepo.deleteAllMissingCalled).To(BeTrue()) - }) - }) -}) - -// Test implementations -type testDataStore struct { - tests.MockDataStore - mfRepo *testMediaFileRepo - albumRepo *testAlbumRepo - artistRepo *testArtistRepo - gcCalled bool - gcError error -} - -func (ds *testDataStore) MediaFile(ctx context.Context) model.MediaFileRepository { - return ds.mfRepo -} - -func (ds *testDataStore) Album(ctx context.Context) model.AlbumRepository { - return ds.albumRepo -} - -func (ds *testDataStore) Artist(ctx context.Context) model.ArtistRepository { - return ds.artistRepo -} - -func (ds *testDataStore) WithTx(block func(tx model.DataStore) error, label ...string) error { - return block(ds) -} - -func (ds *testDataStore) GC(ctx context.Context) error { - ds.gcCalled = true - return ds.gcError -} - -type testMediaFileRepo struct { - tests.MockMediaFileRepo - files model.MediaFiles - getAllCalled bool - getAllError error - deleteMissingCalled bool - deletedIDs []string - deleteMissingError error - deleteAllMissingCalled bool - deleteAllMissingError error -} - -func (m *testMediaFileRepo) GetAll(options ...model.QueryOptions) (model.MediaFiles, error) { - m.getAllCalled = true - if m.getAllError != nil { - return nil, m.getAllError - } - - if len(options) == 0 { - return m.files, nil - } - - // Filter based on the query options - opt := options[0] - if filters, ok := opt.Filters.(squirrel.And); ok { - // Check for ID filter - for _, filter := range filters { - if eq, ok := filter.(squirrel.Eq); ok { - if ids, exists := eq["id"]; exists { - // Filter files by IDs - idList := ids.([]string) - var filtered model.MediaFiles - for _, f := range m.files { - for _, id := range idList { - if f.ID == id { - filtered = append(filtered, f) - break - } - } - } - return filtered, nil - } - } - } - } - return m.files, nil -} - -func (m *testMediaFileRepo) DeleteMissing(ids []string) error { - m.deleteMissingCalled = true - m.deletedIDs = ids - return m.deleteMissingError -} - -func (m *testMediaFileRepo) DeleteAllMissing() (int64, error) { - m.deleteAllMissingCalled = true - if m.deleteAllMissingError != nil { - return 0, m.deleteAllMissingError - } - return int64(len(m.files)), nil -} - -type testAlbumRepo struct { - tests.MockAlbumRepo - refreshAlbumsCalled bool - refreshAlbumsIDs []string - refreshAlbumsError error -} - -func (m *testAlbumRepo) RefreshAlbums(albumIDs []string) error { - m.refreshAlbumsCalled = true - m.refreshAlbumsIDs = albumIDs - return m.refreshAlbumsError -} - -type testArtistRepo struct { - tests.MockArtistRepo - refreshStatsCalled bool - refreshStatsError error -} - -func (m *testArtistRepo) RefreshStats(allArtists bool) (int64, error) { - m.refreshStatsCalled = true - if m.refreshStatsError != nil { - return 0, m.refreshStatsError - } - return 1, nil -} diff --git a/core/wire_providers.go b/core/wire_providers.go index fb0b11d8b..16335645c 100644 --- a/core/wire_providers.go +++ b/core/wire_providers.go @@ -18,7 +18,7 @@ var Set = wire.NewSet( NewShare, NewPlaylists, NewLibrary, - NewMissingFiles, + NewMaintenance, agents.GetAgents, external.NewProvider, wire.Bind(new(external.Agents), new(*agents.Agents)), diff --git a/model/album.go b/model/album.go index b391af591..a8dcfe682 100644 --- a/model/album.go +++ b/model/album.go @@ -139,9 +139,6 @@ type AlbumRepository interface { RefreshPlayCounts() (int64, error) CopyAttributes(fromID, toID string, columns ...string) error - // RefreshAlbums recalculates album attributes (size, duration, etc.) from media files - RefreshAlbums(albumIDs []string) error - AnnotatedRepository SearchableRepository[Albums] } diff --git a/persistence/album_repository.go b/persistence/album_repository.go index 113e3cd0e..6f9bb3b48 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -337,94 +337,6 @@ on conflict (user_id, item_id, item_type) do update return r.executeSQL(query) } -// RefreshAlbums recalculates album attributes (size, duration, song count, etc.) from media files. -// It uses batch queries to minimize database round-trips for efficiency. -func (r *albumRepository) RefreshAlbums(albumIDs []string) error { - if len(albumIDs) == 0 { - return nil - } - - log.Debug(r.ctx, "Refreshing albums", "count", len(albumIDs)) - - // Process in chunks to avoid query size limits - const chunkSize = 100 - for i := 0; i < len(albumIDs); i += chunkSize { - end := i + chunkSize - if end > len(albumIDs) { - end = len(albumIDs) - } - chunk := albumIDs[i:end] - - if err := r.refreshAlbumChunk(chunk); err != nil { - return fmt.Errorf("refreshing album chunk: %w", err) - } - } - - log.Debug(r.ctx, "Successfully refreshed albums", "count", len(albumIDs)) - return nil -} - -// refreshAlbumChunk processes a single chunk of album IDs -func (r *albumRepository) refreshAlbumChunk(albumIDs []string) error { - // Batch load existing albums - albums, err := r.GetAll(model.QueryOptions{Filters: Eq{"album.id": albumIDs}}) - if err != nil { - return fmt.Errorf("loading albums: %w", err) - } - - // Create a map for quick lookup - albumMap := make(map[string]*model.Album, len(albums)) - for i := range albums { - albumMap[albums[i].ID] = &albums[i] - } - - // Batch load all media files for these albums using MediaFile repository - mfRepo := NewMediaFileRepository(r.ctx, r.db) - mediaFiles, err := mfRepo.GetAll(model.QueryOptions{ - Filters: Eq{"album_id": albumIDs}, - Sort: "album_id, path", - }) - if err != nil { - return fmt.Errorf("loading media files: %w", err) - } - - // Group media files by album ID - filesByAlbum := make(map[string]model.MediaFiles) - for i := range mediaFiles { - albumID := mediaFiles[i].AlbumID - filesByAlbum[albumID] = append(filesByAlbum[albumID], mediaFiles[i]) - } - - // Recalculate each album from its media files - for albumID, oldAlbum := range albumMap { - mfs, hasTracks := filesByAlbum[albumID] - if !hasTracks { - // Album has no tracks anymore, skip (will be cleaned up by GC) - log.Debug(r.ctx, "Skipping album with no tracks", "albumID", albumID) - continue - } - - // Recalculate album from media files - newAlbum := mfs.ToAlbum() - - // Only update if something changed (avoid unnecessary writes) - if !oldAlbum.Equals(newAlbum) { - // Preserve original timestamps - newAlbum.UpdatedAt = time.Now() - newAlbum.CreatedAt = oldAlbum.CreatedAt - - if err := r.Put(&newAlbum); err != nil { - log.Error(r.ctx, "Error updating album during refresh", "albumID", albumID, err) - // Continue with other albums instead of failing entirely - continue - } - log.Trace(r.ctx, "Refreshed album", "albumID", albumID, "name", newAlbum.Name) - } - } - - return nil -} - func (r *albumRepository) purgeEmpty() error { del := Delete(r.tableName).Where("id not in (select distinct(album_id) from media_file)") c, err := r.executeSQL(del) diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index 11d9a8d47..a062b4398 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -513,123 +513,6 @@ var _ = Describe("AlbumRepository", func() { _, _ = albumRepo.executeSQL(squirrel.Delete("album").Where(squirrel.Eq{"id": album.ID})) }) }) - - Describe("RefreshAlbums", func() { - var mfRepo *mediaFileRepository - - BeforeEach(func() { - ctx := request.WithUser(GinkgoT().Context(), adminUser) - albumRepo = NewAlbumRepository(ctx, GetDBXBuilder()).(*albumRepository) - mfRepo = NewMediaFileRepository(ctx, GetDBXBuilder()).(*mediaFileRepository) - }) - - It("recalculates size and duration after files are modified", func() { - // Get the initial album - album, err := albumRepo.Get("103") // Radioactivity album - Expect(err).ToNot(HaveOccurred()) - initialSize := album.Size - initialDuration := album.Duration - - // Modify the size and duration of one of the media files - mf, err := mfRepo.Get("1003") // Radioactivity song - Expect(err).ToNot(HaveOccurred()) - mf.Size = 5000000 // 5MB - mf.Duration = 300.5 // 5 minutes - Expect(mfRepo.Put(mf)).To(Succeed()) - - // Refresh the album - err = albumRepo.RefreshAlbums([]string{"103"}) - Expect(err).ToNot(HaveOccurred()) - - // Verify the album was refreshed with new values - refreshedAlbum, err := albumRepo.Get("103") - Expect(err).ToNot(HaveOccurred()) - Expect(refreshedAlbum.Size).ToNot(Equal(initialSize)) - Expect(refreshedAlbum.Duration).ToNot(Equal(initialDuration)) - }) - - It("handles multiple albums in a single call", func() { - // Modify files in two different albums - mf1, err := mfRepo.Get("1001") // Sgt Peppers song - Expect(err).ToNot(HaveOccurred()) - mf1.Size = 3000000 - Expect(mfRepo.Put(mf1)).To(Succeed()) - - mf2, err := mfRepo.Get("1002") // Abbey Road song - Expect(err).ToNot(HaveOccurred()) - mf2.Size = 4000000 - Expect(mfRepo.Put(mf2)).To(Succeed()) - - // Refresh both albums in one call - err = albumRepo.RefreshAlbums([]string{"101", "102"}) - Expect(err).ToNot(HaveOccurred()) - - // Verify both were refreshed - album1, err := albumRepo.Get("101") - Expect(err).ToNot(HaveOccurred()) - Expect(album1.Size).To(Equal(int64(3000000))) - - album2, err := albumRepo.Get("102") - Expect(err).ToNot(HaveOccurred()) - Expect(album2.Size).To(Equal(int64(4000000))) - }) - - It("handles empty album ID list gracefully", func() { - err := albumRepo.RefreshAlbums([]string{}) - Expect(err).ToNot(HaveOccurred()) - }) - - It("handles non-existent album IDs gracefully", func() { - err := albumRepo.RefreshAlbums([]string{"non-existent-id"}) - Expect(err).ToNot(HaveOccurred()) - }) - - It("recalculates song count correctly", func() { - album, err := albumRepo.Get("103") // Radioactivity album - Expect(err).ToNot(HaveOccurred()) - initialSongCount := album.SongCount - - // Add a new media file to this album - newSong := mf(model.MediaFile{ - ID: "1099", - Title: "New Song", - ArtistID: "2", - Artist: "Kraftwerk", - AlbumID: "103", - Album: "Radioactivity", - Path: p("/kraft/radio/new-song.mp3"), - Size: 1000000, - Duration: 180, - }) - Expect(mfRepo.Put(&newSong)).To(Succeed()) - - // Refresh the album - err = albumRepo.RefreshAlbums([]string{"103"}) - Expect(err).ToNot(HaveOccurred()) - - // Verify song count increased - refreshedAlbum, err := albumRepo.Get("103") - Expect(err).ToNot(HaveOccurred()) - Expect(refreshedAlbum.SongCount).To(Equal(initialSongCount + 1)) - - // Clean up - _, _ = mfRepo.executeSQL(squirrel.Delete("media_file").Where(squirrel.Eq{"id": "1099"})) - }) - - It("processes large batches efficiently", func() { - // Test with all existing albums - allAlbums := []string{"101", "102", "103", "104"} - err := albumRepo.RefreshAlbums(allAlbums) - Expect(err).ToNot(HaveOccurred()) - - // Verify all albums still exist and have correct data - for _, albumID := range allAlbums { - album, err := albumRepo.Get(albumID) - Expect(err).ToNot(HaveOccurred()) - Expect(album.ID).To(Equal(albumID)) - } - }) - }) }) func _p(id, name string, sortName ...string) model.Participant { diff --git a/server/nativeapi/missing.go b/server/nativeapi/missing.go index d9109bb0d..2b455e622 100644 --- a/server/nativeapi/missing.go +++ b/server/nativeapi/missing.go @@ -63,7 +63,7 @@ func (r *missingRepository) EntityName() string { return "missing_files" } -func deleteMissingFiles(missingFiles core.MissingFiles) http.HandlerFunc { +func deleteMissingFiles(maintenance core.Maintenance) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -72,9 +72,9 @@ func deleteMissingFiles(missingFiles core.MissingFiles) http.HandlerFunc { var err error if len(ids) == 0 { - err = missingFiles.DeleteAllMissingFiles(ctx) + err = maintenance.DeleteAllMissingFiles(ctx) } else { - err = missingFiles.DeleteMissingFiles(ctx, ids) + err = maintenance.DeleteMissingFiles(ctx, ids) } if len(ids) == 1 && errors.Is(err, model.ErrNotFound) { diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index 92c4423fd..969650e0a 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -22,16 +22,16 @@ import ( type Router struct { http.Handler - ds model.DataStore - share core.Share - playlists core.Playlists - insights metrics.Insights - libs core.Library - missingFiles core.MissingFiles + ds model.DataStore + share core.Share + playlists core.Playlists + insights metrics.Insights + libs core.Library + maintenance core.Maintenance } -func New(ds model.DataStore, share core.Share, playlists core.Playlists, insights metrics.Insights, libraryService core.Library, missingFiles core.MissingFiles) *Router { - r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, missingFiles: missingFiles} +func New(ds model.DataStore, share core.Share, playlists core.Playlists, insights metrics.Insights, libraryService core.Library, maintenance core.Maintenance) *Router { + r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, maintenance: maintenance} r.Handler = r.routes() return r } @@ -173,7 +173,7 @@ func (api *Router) addQueueRoute(r chi.Router) { func (api *Router) addMissingFilesRoute(r chi.Router) { r.Route("/missing", func(r chi.Router) { api.RX(r, "/", newMissingRepository(api.ds), false) - r.Delete("/", deleteMissingFiles(api.missingFiles)) + r.Delete("/", deleteMissingFiles(api.maintenance)) }) }