mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-29 06:37:17 +01:00
96b051ffa7* fix(nativeapi): stop partial PUTs from clearing untouched columns The REST layer parses the request body's top-level JSON keys and passes them to Repository.Update as colsToUpdate. The radio and library repositories discarded that list and issued a full-row UPDATE, so any field absent from the body was written as its zero value. For radio this wiped uploaded_image, deleting the station's cover on every partial update (the Web UI is unaffected because its form submits the whole record). For library it silently cleared remote_path and default_new_users. Thread the column list through to Put in both repositories, and extract the column-selection half of filterUpdateValues into selectUpdateColumns so library, which hand-builds its update map, shares the same rule instead of copying it. Fixes #6057 * refactor(persistence): drop pluginRepository's dead rest.Persistable methods Save and Update had no callers: PUT /api/plugin/{id} is served by the hand-written updatePlugin handler over a typed request struct, and the route only wires rest.GetAll and rest.Get. Both methods delegated to Put, which upserts all twelve columns, so wiring rest.Put to this repository would have reintroduced the partial-update clobbering fixed in the previous commit. Removing them, along with the rest.Persistable assertion, makes that a compile error instead of a silent data loss. Put itself is unchanged and still backs plugin discovery.
275 lines
9.8 KiB
Go
275 lines
9.8 KiB
Go
package persistence
|
|
|
|
import (
|
|
"context"
|
|
"time"
|
|
|
|
"github.com/Masterminds/squirrel"
|
|
"github.com/navidrome/navidrome/log"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/model/request"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
"github.com/pocketbase/dbx"
|
|
)
|
|
|
|
var _ = Describe("LibraryRepository", func() {
|
|
var repo model.LibraryRepository
|
|
var ctx context.Context
|
|
var conn *dbx.DB
|
|
|
|
BeforeEach(func() {
|
|
ctx = request.WithUser(log.NewContext(context.TODO()), model.User{ID: "userid"})
|
|
conn = GetDBXBuilder()
|
|
repo = NewLibraryRepository(ctx, conn)
|
|
})
|
|
|
|
AfterEach(func() {
|
|
// Clean up test libraries (keep ID 1 which is the default library)
|
|
_, _ = conn.NewQuery("DELETE FROM library WHERE id > 1").Execute()
|
|
})
|
|
|
|
Describe("Put", func() {
|
|
Context("when ID is 0", func() {
|
|
It("inserts a new library with autoassigned ID", func() {
|
|
lib := &model.Library{
|
|
ID: 0,
|
|
Name: "Test Library",
|
|
Path: "/music/test",
|
|
}
|
|
|
|
err := repo.Put(lib)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(lib.ID).To(BeNumerically(">", 0))
|
|
Expect(lib.CreatedAt).ToNot(BeZero())
|
|
Expect(lib.UpdatedAt).ToNot(BeZero())
|
|
|
|
// Verify it was inserted
|
|
savedLib, err := repo.Get(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(savedLib.Name).To(Equal("Test Library"))
|
|
Expect(savedLib.Path).To(Equal("/music/test"))
|
|
})
|
|
})
|
|
|
|
Context("when colsToUpdate is specified", func() {
|
|
It("only writes the requested columns", func() {
|
|
lib := &model.Library{
|
|
Name: "Original Library",
|
|
Path: "/music/original",
|
|
RemotePath: "/remote/original",
|
|
DefaultNewUsers: true,
|
|
}
|
|
Expect(repo.Put(lib)).To(Succeed())
|
|
|
|
Expect(repo.Put(&model.Library{ID: lib.ID, Name: "Renamed", Path: lib.Path}, "name", "path")).To(Succeed())
|
|
|
|
saved, err := repo.Get(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(saved.Name).To(Equal("Renamed"))
|
|
Expect(saved.RemotePath).To(Equal("/remote/original"))
|
|
Expect(saved.DefaultNewUsers).To(BeTrue())
|
|
})
|
|
})
|
|
|
|
Context("when ID is non-zero and record exists", func() {
|
|
It("updates the existing record", func() {
|
|
// First create a library
|
|
lib := &model.Library{
|
|
ID: 0,
|
|
Name: "Original Library",
|
|
Path: "/music/original",
|
|
}
|
|
err := repo.Put(lib)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
originalID := lib.ID
|
|
originalCreatedAt := lib.CreatedAt
|
|
|
|
// Ensure the update's timestamp is strictly greater than the
|
|
// create's timestamp on platforms with coarse clock resolution
|
|
// (Windows' time.Now() is millisecond-granular).
|
|
time.Sleep(2 * time.Millisecond)
|
|
|
|
// Now update it
|
|
lib.Name = "Updated Library"
|
|
lib.Path = "/music/updated"
|
|
err = repo.Put(lib)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
// Verify it was updated, not inserted
|
|
Expect(lib.ID).To(Equal(originalID))
|
|
Expect(lib.CreatedAt).To(Equal(originalCreatedAt))
|
|
Expect(lib.UpdatedAt).To(BeTemporally(">", originalCreatedAt))
|
|
|
|
// Verify the changes were saved
|
|
savedLib, err := repo.Get(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(savedLib.Name).To(Equal("Updated Library"))
|
|
Expect(savedLib.Path).To(Equal("/music/updated"))
|
|
})
|
|
})
|
|
|
|
Context("when ID is non-zero but record doesn't exist", func() {
|
|
It("inserts a new record with the specified ID", func() {
|
|
lib := &model.Library{
|
|
ID: 999,
|
|
Name: "New Library with ID",
|
|
Path: "/music/new",
|
|
}
|
|
|
|
// Ensure the record doesn't exist
|
|
_, err := repo.Get(999)
|
|
Expect(err).To(HaveOccurred())
|
|
|
|
// Put should insert it
|
|
err = repo.Put(lib)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(lib.ID).To(Equal(999))
|
|
Expect(lib.CreatedAt).ToNot(BeZero())
|
|
Expect(lib.UpdatedAt).ToNot(BeZero())
|
|
|
|
// Verify it was inserted with the correct ID
|
|
savedLib, err := repo.Get(999)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(savedLib.ID).To(Equal(999))
|
|
Expect(savedLib.Name).To(Equal("New Library with ID"))
|
|
Expect(savedLib.Path).To(Equal("/music/new"))
|
|
})
|
|
})
|
|
})
|
|
|
|
It("refreshes stats", func() {
|
|
libBefore, err := repo.Get(1)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(repo.RefreshStats(1)).To(Succeed())
|
|
libAfter, err := repo.Get(1)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(libAfter.UpdatedAt).To(BeTemporally(">", libBefore.UpdatedAt))
|
|
|
|
var songsRes, albumsRes, artistsRes, foldersRes, filesRes, missingRes struct{ Count int64 }
|
|
var sizeRes struct{ Sum int64 }
|
|
var durationRes struct{ Sum float64 }
|
|
|
|
Expect(conn.NewQuery("select count(*) as count from media_file where library_id = {:id} and missing = 0").Bind(dbx.Params{"id": 1}).One(&songsRes)).To(Succeed())
|
|
Expect(conn.NewQuery("select count(*) as count from album where library_id = {:id} and missing = 0").Bind(dbx.Params{"id": 1}).One(&albumsRes)).To(Succeed())
|
|
Expect(conn.NewQuery("select count(*) as count from library_artist la join artist a on la.artist_id = a.id where la.library_id = {:id} and a.missing = 0").Bind(dbx.Params{"id": 1}).One(&artistsRes)).To(Succeed())
|
|
Expect(conn.NewQuery("select count(*) as count from folder where library_id = {:id} and missing = 0").Bind(dbx.Params{"id": 1}).One(&foldersRes)).To(Succeed())
|
|
Expect(conn.NewQuery("select ifnull(sum(num_audio_files + num_playlists + json_array_length(image_files)),0) as count from folder where library_id = {:id} and missing = 0").Bind(dbx.Params{"id": 1}).One(&filesRes)).To(Succeed())
|
|
Expect(conn.NewQuery("select count(*) as count from media_file where library_id = {:id} and missing = 1").Bind(dbx.Params{"id": 1}).One(&missingRes)).To(Succeed())
|
|
Expect(conn.NewQuery("select ifnull(sum(size),0) as sum from album where library_id = {:id} and missing = 0").Bind(dbx.Params{"id": 1}).One(&sizeRes)).To(Succeed())
|
|
Expect(conn.NewQuery("select ifnull(sum(duration),0) as sum from album where library_id = {:id} and missing = 0").Bind(dbx.Params{"id": 1}).One(&durationRes)).To(Succeed())
|
|
|
|
Expect(libAfter.TotalSongs).To(Equal(int(songsRes.Count)))
|
|
Expect(libAfter.TotalAlbums).To(Equal(int(albumsRes.Count)))
|
|
Expect(libAfter.TotalArtists).To(Equal(int(artistsRes.Count)))
|
|
Expect(libAfter.TotalFolders).To(Equal(int(foldersRes.Count)))
|
|
Expect(libAfter.TotalFiles).To(Equal(int(filesRes.Count)))
|
|
Expect(libAfter.TotalMissingFiles).To(Equal(int(missingRes.Count)))
|
|
Expect(libAfter.TotalSize).To(Equal(sizeRes.Sum))
|
|
Expect(libAfter.TotalDuration).To(Equal(durationRes.Sum))
|
|
})
|
|
|
|
Describe("ScanBegin and ScanEnd", func() {
|
|
var lib *model.Library
|
|
|
|
BeforeEach(func() {
|
|
lib = &model.Library{
|
|
ID: 0,
|
|
Name: "Test Scan Library",
|
|
Path: "/music/test-scan",
|
|
}
|
|
err := repo.Put(lib)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
})
|
|
|
|
DescribeTable("ScanBegin",
|
|
func(fullScan bool, expectedFullScanInProgress bool) {
|
|
err := repo.ScanBegin(lib.ID, fullScan)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
updatedLib, err := repo.Get(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(updatedLib.LastScanStartedAt).ToNot(BeZero())
|
|
Expect(updatedLib.FullScanInProgress).To(Equal(expectedFullScanInProgress))
|
|
},
|
|
Entry("sets FullScanInProgress to true for full scan", true, true),
|
|
Entry("sets FullScanInProgress to false for quick scan", false, false),
|
|
)
|
|
|
|
Context("ScanEnd", func() {
|
|
BeforeEach(func() {
|
|
err := repo.ScanBegin(lib.ID, true)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
})
|
|
|
|
It("sets LastScanAt and clears FullScanInProgress and LastScanStartedAt", func() {
|
|
err := repo.ScanEnd(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
updatedLib, err := repo.Get(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(updatedLib.LastScanAt).ToNot(BeZero())
|
|
Expect(updatedLib.FullScanInProgress).To(BeFalse())
|
|
Expect(updatedLib.LastScanStartedAt).To(BeZero())
|
|
})
|
|
|
|
It("sets LastScanAt to be after LastScanStartedAt", func() {
|
|
libBefore, err := repo.Get(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
err = repo.ScanEnd(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
libAfter, err := repo.Get(lib.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(libAfter.LastScanAt).To(BeTemporally(">=", libBefore.LastScanStartedAt))
|
|
})
|
|
})
|
|
})
|
|
|
|
Describe("Delete", func() {
|
|
var adminRepo model.LibraryRepository
|
|
var artistRepo model.ArtistRepository
|
|
|
|
artistMissing := func(id string) bool {
|
|
var missing bool
|
|
err := conn.NewQuery("SELECT missing FROM artist WHERE id = {:id}").
|
|
Bind(dbx.Params{"id": id}).Row(&missing)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
return missing
|
|
}
|
|
|
|
BeforeEach(func() {
|
|
adminCtx := request.WithUser(log.NewContext(context.TODO()), adminUser)
|
|
adminRepo = NewLibraryRepository(adminCtx, conn)
|
|
artistRepo = NewArtistRepository(adminCtx, conn)
|
|
})
|
|
|
|
It("marks artists orphaned by the delete as missing", func() {
|
|
lib := model.Library{Name: "Doomed Library", Path: "/doomed"}
|
|
Expect(adminRepo.Put(&lib)).To(Succeed())
|
|
|
|
orphanArtist := model.Artist{ID: "delete-orphan", Name: "Orphan To Be"}
|
|
sharedArtist := model.Artist{ID: "delete-shared", Name: "Shared Artist"}
|
|
Expect(artistRepo.Put(&orphanArtist)).To(Succeed())
|
|
Expect(artistRepo.Put(&sharedArtist)).To(Succeed())
|
|
Expect(adminRepo.AddArtist(lib.ID, orphanArtist.ID)).To(Succeed())
|
|
Expect(adminRepo.AddArtist(lib.ID, sharedArtist.ID)).To(Succeed())
|
|
Expect(adminRepo.AddArtist(1, sharedArtist.ID)).To(Succeed())
|
|
DeferCleanup(func() {
|
|
if raw, ok := artistRepo.(*artistRepository); ok {
|
|
_, _ = raw.executeSQL(squirrel.Delete("artist").
|
|
Where(squirrel.Eq{"id": []string{orphanArtist.ID, sharedArtist.ID}}))
|
|
}
|
|
})
|
|
|
|
Expect(artistMissing(orphanArtist.ID)).To(BeFalse())
|
|
|
|
Expect(adminRepo.Delete(lib.ID)).To(Succeed())
|
|
|
|
Expect(artistMissing(orphanArtist.ID)).To(BeTrue(), "orphaned artist should be marked missing")
|
|
Expect(artistMissing(sharedArtist.ID)).To(BeFalse(), "artist still in another library must stay visible")
|
|
})
|
|
})
|
|
})
|