feat: replace AddUserLibrary and RemoveUserLibrary with SetUserLibraries for better library management

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-06-28 13:22:47 -04:00
commit fbde103499
6 changed files with 13 additions and 104 deletions

View file

@ -75,8 +75,4 @@ type UserRepository interface {
// Library association methods
GetUserLibraries(userID string) (Libraries, error)
SetUserLibraries(userID string, libraryIDs []int) error
// TODO Remove these methods in favor of SetUserLibraries
AddUserLibrary(userID string, libraryID int) error
RemoveUserLibrary(userID string, libraryID int) error
}

View file

@ -363,9 +363,9 @@ var _ = Describe("ArtistRepository", func() {
// Try to put the user (will fail silently if already exists)
_ = ur.Put(&testUser)
// Add library association
err = ur.AddUserLibrary(currentUser.ID, 1)
// Ignore error if user-library association already exists
// Add library association using SetUserLibraries
err = ur.SetUserLibraries(currentUser.ID, []int{1})
// Ignore error if user already has these libraries or other conflicts
if err != nil && !strings.Contains(err.Error(), "UNIQUE constraint failed") && !strings.Contains(err.Error(), "duplicate key") {
Expect(err).ToNot(HaveOccurred())
}
@ -633,9 +633,9 @@ var _ = Describe("ArtistRepository", func() {
err := ur.Put(&unauthorizedUser)
Expect(err).ToNot(HaveOccurred())
// Then add library access
err = ur.AddUserLibrary(unauthorizedUser.ID, 1)
Expect(err).ToNot(HaveOccurred())
// Then add library access
err = ur.SetUserLibraries(unauthorizedUser.ID, []int{1})
Expect(err).ToNot(HaveOccurred())
// Update the user object with the libraries to simulate middleware behavior
libraries, err := ur.GetUserLibraries(unauthorizedUser.ID)
@ -648,11 +648,11 @@ var _ = Describe("ArtistRepository", func() {
restrictedRepo = NewArtistRepository(ctx, GetDBXBuilder())
})
AfterEach(func() {
// Clean up: remove the user's library access
ur := NewUserRepository(request.WithUser(log.NewContext(context.TODO()), adminUser), GetDBXBuilder())
_ = ur.RemoveUserLibrary(unauthorizedUser.ID, 1)
})
AfterEach(func() {
// Clean up: remove the user's library access
ur := NewUserRepository(request.WithUser(log.NewContext(context.TODO()), adminUser), GetDBXBuilder())
_ = ur.SetUserLibraries(unauthorizedUser.ID, []int{})
})
It("CountAll returns correct count after gaining access", func() {
count, err := restrictedRepo.CountAll()

View file

@ -143,7 +143,7 @@ var _ = BeforeSuite(func() {
// Associate users with library 1 (default test library)
for i := range testUsers {
err := ur.AddUserLibrary(testUsers[i].ID, 1)
err := ur.SetUserLibraries(testUsers[i].ID, []int{1})
if err != nil {
panic(err)
}

View file

@ -458,24 +458,6 @@ func (r *userRepository) SetUserLibraries(userID string, libraryIDs []int) error
return nil
}
func (r *userRepository) AddUserLibrary(userID string, libraryID int) error {
insert := Insert("user_library").
Columns("user_id", "library_id").
Values(userID, libraryID).
Suffix("ON CONFLICT (user_id, library_id) DO NOTHING")
_, err := r.executeSQL(insert)
return err
}
func (r *userRepository) RemoveUserLibrary(userID string, libraryID int) error {
del := Delete("user_library").Where(And{
Eq{"user_id": userID},
Eq{"library_id": libraryID},
})
_, err := r.executeSQL(del)
return err
}
var _ model.UserRepository = (*userRepository)(nil)
var _ rest.Repository = (*userRepository)(nil)
var _ rest.Persistable = (*userRepository)(nil)

View file

@ -272,31 +272,6 @@ var _ = Describe("UserRepository", func() {
_ = libRepo.(*libraryRepository).delete(squirrel.Eq{"id": []int{library1.ID, library2.ID}})
})
Describe("AddUserLibrary", func() {
It("adds a library association for a user", func() {
err := repo.AddUserLibrary(userID, library1.ID)
Expect(err).To(BeNil())
libraries, err := repo.GetUserLibraries(userID)
Expect(err).To(BeNil())
Expect(libraries).To(HaveLen(1))
Expect(libraries[0].ID).To(Equal(library1.ID))
})
It("does not create duplicate associations", func() {
err := repo.AddUserLibrary(userID, library1.ID)
Expect(err).To(BeNil())
// Add the same association again
err = repo.AddUserLibrary(userID, library1.ID)
Expect(err).To(BeNil())
libraries, err := repo.GetUserLibraries(userID)
Expect(err).To(BeNil())
Expect(libraries).To(HaveLen(1))
})
})
Describe("GetUserLibraries", func() {
It("returns empty list when user has no library associations", func() {
libraries, err := repo.GetUserLibraries("non-existent-user")
@ -305,9 +280,7 @@ var _ = Describe("UserRepository", func() {
})
It("returns user's associated libraries", func() {
err := repo.AddUserLibrary(userID, library1.ID)
Expect(err).To(BeNil())
err = repo.AddUserLibrary(userID, library2.ID)
err := repo.SetUserLibraries(userID, []int{library1.ID, library2.ID})
Expect(err).To(BeNil())
libraries, err := repo.GetUserLibraries(userID)
@ -359,32 +332,6 @@ var _ = Describe("UserRepository", func() {
Expect(libraries).To(HaveLen(0))
})
})
Describe("RemoveUserLibrary", func() {
It("removes a library association", func() {
err := repo.AddUserLibrary(userID, library1.ID)
Expect(err).To(BeNil())
err = repo.AddUserLibrary(userID, library2.ID)
Expect(err).To(BeNil())
err = repo.RemoveUserLibrary(userID, library1.ID)
Expect(err).To(BeNil())
libraries, err := repo.GetUserLibraries(userID)
Expect(err).To(BeNil())
Expect(libraries).To(HaveLen(1))
Expect(libraries[0].ID).To(Equal(library2.ID))
})
It("does nothing when association doesn't exist", func() {
err := repo.RemoveUserLibrary(userID, library1.ID)
Expect(err).To(BeNil())
libraries, err := repo.GetUserLibraries(userID)
Expect(err).To(BeNil())
Expect(libraries).To(HaveLen(0))
})
})
})
Describe("Admin User Auto-Assignment", func() {

View file

@ -123,19 +123,3 @@ func (u *MockedUserRepo) SetUserLibraries(userID string, libraryIDs []int) error
u.UserLibraries[userID] = libraryIDs
return nil
}
func (u *MockedUserRepo) AddUserLibrary(userID string, libraryID int) error {
if u.Error != nil {
return u.Error
}
// Mock: do nothing for now
return nil
}
func (u *MockedUserRepo) RemoveUserLibrary(userID string, libraryID int) error {
if u.Error != nil {
return u.Error
}
// Mock: do nothing for now
return nil
}