mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
refactor: consolidate maintenance operations into unified service
Consolidate MissingFiles and RefreshAlbums functionality into a new Maintenance service. This refactoring: - Creates core.Maintenance interface combining DeleteMissingFiles, DeleteAllMissingFiles, and RefreshAlbums methods - Moves RefreshAlbums logic from AlbumRepository persistence layer to core Maintenance service - Removes MissingFiles interface and moves its implementation to maintenanceService - Updates all references in wire providers, native API router, and handlers - Removes RefreshAlbums interface method from AlbumRepository model - Improves separation of concerns by centralizing maintenance operations in the core domain This change provides a cleaner API and better organization of maintenance-related database operations.
This commit is contained in:
Deluan
parent
593b5db8e9
commit
319651d7d1
220
core/maintenance.go
Normal file
220
core/maintenance.go
Normal file
@ -0,0 +1,220 @@
|
||||
package core
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"time"
|
||||
|
||||
"github.com/Masterminds/squirrel"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
)
|
||||
|
||||
type Maintenance 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
|
||||
// RefreshAlbums recalculates album attributes from media files
|
||||
RefreshAlbums(ctx context.Context, albumIDs []string) error
|
||||
}
|
||||
|
||||
type maintenanceService struct {
|
||||
ds model.DataStore
|
||||
}
|
||||
|
||||
func NewMaintenance(ds model.DataStore) Maintenance {
|
||||
return &maintenanceService{
|
||||
ds: ds,
|
||||
}
|
||||
}
|
||||
|
||||
func (s *maintenanceService) DeleteMissingFiles(ctx context.Context, ids []string) error {
|
||||
return s.deleteMissing(ctx, ids)
|
||||
}
|
||||
|
||||
func (s *maintenanceService) DeleteAllMissingFiles(ctx context.Context) error {
|
||||
return s.deleteMissing(ctx, nil)
|
||||
}
|
||||
|
||||
// deleteMissing handles the deletion of missing files and triggers necessary cleanup operations
|
||||
func (s *maintenanceService) 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
|
||||
}
|
||||
|
||||
// RefreshAlbums recalculates album attributes (size, duration, song count, etc.) from media files.
|
||||
// It uses batch queries to minimize database round-trips for efficiency.
|
||||
func (s *maintenanceService) RefreshAlbums(ctx context.Context, albumIDs []string) error {
|
||||
if len(albumIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
log.Debug(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 := s.refreshAlbumChunk(ctx, chunk); err != nil {
|
||||
return fmt.Errorf("refreshing album chunk: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
log.Debug(ctx, "Successfully refreshed albums", "count", len(albumIDs))
|
||||
return nil
|
||||
}
|
||||
|
||||
// refreshAlbumChunk processes a single chunk of album IDs
|
||||
func (s *maintenanceService) refreshAlbumChunk(ctx context.Context, albumIDs []string) error {
|
||||
albumRepo := s.ds.Album(ctx)
|
||||
mfRepo := s.ds.MediaFile(ctx)
|
||||
|
||||
// Batch load existing albums
|
||||
albums, err := albumRepo.GetAll(model.QueryOptions{
|
||||
Filters: squirrel.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
|
||||
mediaFiles, err := mfRepo.GetAll(model.QueryOptions{
|
||||
Filters: squirrel.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(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 := albumRepo.Put(&newAlbum); err != nil {
|
||||
log.Error(ctx, "Error updating album during refresh", "albumID", albumID, err)
|
||||
// Continue with other albums instead of failing entirely
|
||||
continue
|
||||
}
|
||||
log.Trace(ctx, "Refreshed album", "albumID", albumID, "name", newAlbum.Name)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// getAffectedAlbumIDs returns distinct album IDs from missing media files
|
||||
func (s *maintenanceService) 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 *maintenanceService) 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.RefreshAlbums(bgCtx, 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))
|
||||
}
|
||||
}
|
||||
}()
|
||||
}
|
||||
253
core/maintenance_test.go
Normal file
253
core/maintenance_test.go
Normal file
@ -0,0 +1,253 @@
|
||||
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("Maintenance", func() {
|
||||
var ds *testDataStore
|
||||
var service Maintenance
|
||||
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 = NewMaintenance(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
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user