nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-18 07:30:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(persistence): unexport getAllIDs

Browse Source 
Removing the artwork backfill left GetAllIDs with no caller outside the
persistence package: each repository's own GetCursor is the only one, and
radio's — which existed solely for that backfill — went with it.

Off the model interfaces and unexported, which also drops four mock
implementations that existed only to satisfy them.
This commit is contained in:
Deluan 2026-09-01 17:28:20 -04:00
parent 296068a233
commit 8dd778239b
16 changed files with 19 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
model/album.go
View File

@ -143,7 +143,6 @@ type AlbumRepository interface {
UpdateExternalInfo(*Album) error
Get(id string) (*Album, error)
GetAll(...QueryOptions) (Albums, error)
GetAllIDs(...QueryOptions) ([]string, error)
// GetSoleAlbumArtistIDsInSubtrees returns the sole album artists of the albums with folders in
// any of the given library-relative subtrees.
GetSoleAlbumArtistIDsInSubtrees(lib Library, paths ...string) ([]string, error)

1
model/artist.go
View File

@ -90,7 +90,6 @@ type ArtistRepository interface {
UpdateExternalInfo(a *Artist) error
Get(id string) (*Artist, error)
GetAll(options ...QueryOptions) (Artists, error)
GetAllIDs(options ...QueryOptions) ([]string, error)
GetCursor(options ...QueryOptions) (ArtistCursor, error)
GetIndex(includeMissing bool, libraryIds []int, roles ...Role) (ArtistIndexes, error)

2
model/mediafile.go
View File

@ -553,8 +553,6 @@ type MediaFileRepository interface {
// expression, using the logged user's annotations. Limit and offset are ignored.
MatchesCriteria(id string, c criteria.Criteria) (bool, error)
GetCursor(options ...QueryOptions) (MediaFileCursor, error)
// GetAllIDs returns just the media_file IDs for the same row set as GetAll.
GetAllIDs(options ...QueryOptions) ([]string, error)
// GetAlbumIDsByFolder returns the distinct IDs of albums with non-missing tracks in the given
// folders or their direct children.
GetAlbumIDsByFolder(lib Library, folderIDs ...string) ([]string, error)

1
model/playlist.go
View File

@ -150,7 +150,6 @@ type PlaylistRepository interface {
Get(id string) (*Playlist, error)
GetWithTracks(id string, refreshSmartPlaylist, includeMissing bool) (*Playlist, error)
GetAll(options ...QueryOptions) (Playlists, error)
GetAllIDs(options ...QueryOptions) ([]string, error)
GetCursor(options ...QueryOptions) (PlaylistCursor, error)
FindByPath(path string) (*Playlist, error)
Delete(id string) error

6
persistence/album_repository.go
View File

@ -259,8 +259,8 @@ func (r *albumRepository) hydrateArtwork(albums model.Albums) {
func(a *model.Album) (string, *model.ItemImage) { return a.ID, &a.ItemImage })
}
// GetAllIDs returns the IDs of GetAll's row set, skipping its column projection and JSON decoding.
func (r *albumRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
// getAllIDs returns the IDs of GetAll's row set, skipping its column projection and JSON decoding.
func (r *albumRepository) getAllIDs(options ...model.QueryOptions) ([]string, error) {
sq := r.applyLibraryFilter(r.newSelect(options...).Columns("album.id"))
if filtersNeedAnnotation(sq) {
sq = r.withAnnotation(sq, "album.id")
@ -304,7 +304,7 @@ func (r *albumRepository) GetSoleAlbumArtistIDsInSubtrees(lib model.Library, pat
}
func (r *albumRepository) GetCursor(options ...model.QueryOptions) (model.AlbumCursor, error) {
ids, err := r.GetAllIDs(options...)
ids, err := r.getAllIDs(options...)
if err != nil {
return nil, err
}

4
persistence/album_repository_test.go
View File

@ -152,12 +152,12 @@ var _ = Describe("AlbumRepository", func() {
})
})
Describe("GetAllIDs", func() {
Describe("getAllIDs", func() {
It("returns the same id set as GetAll", func() {
want, err := albumRepo.GetAll()
Expect(err).ToNot(HaveOccurred())
Expect(want).ToNot(BeEmpty())
ids, err := albumRepo.GetAllIDs()
ids, err := albumRepo.getAllIDs()
Expect(err).ToNot(HaveOccurred())
Expect(ids).To(ConsistOf(slice.Map(want, func(a model.Album) string { return a.ID })))
})

6
persistence/artist_repository.go
View File

@ -265,9 +265,9 @@ func (r *artistRepository) GetAll(options ...model.QueryOptions) (model.Artists,
return res, err
}
// GetAllIDs returns just the artist IDs for the same row set as GetAll, skipping the
// getAllIDs returns just the artist IDs for the same row set as GetAll, skipping the
// heavy stats columns and JSON post-processing.
func (r *artistRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
func (r *artistRepository) getAllIDs(options ...model.QueryOptions) ([]string, error) {
sq := r.applyLibraryFilterToArtistQuery(r.newSelect(options...).Columns("artist.id")).GroupBy("artist.id")
if filtersNeedAnnotation(sq) {
sq = r.withAnnotation(sq, "artist.id")
@ -284,7 +284,7 @@ func (r *artistRepository) hydrateArtwork(artists model.Artists) {
}
func (r *artistRepository) GetCursor(options ...model.QueryOptions) (model.ArtistCursor, error) {
ids, err := r.GetAllIDs(options...)
ids, err := r.getAllIDs(options...)
if err != nil {
return nil, err
}

4
persistence/artist_repository_test.go
View File

@ -285,12 +285,12 @@ var _ = Describe("ArtistRepository", func() {
})
})
Describe("GetAllIDs", func() {
Describe("getAllIDs", func() {
It("returns the same id set as GetAll", func() {
want, err := repo.GetAll()
Expect(err).ToNot(HaveOccurred())
Expect(want).ToNot(BeEmpty())
ids, err := repo.GetAllIDs()
ids, err := repo.(*artistRepository).getAllIDs()
Expect(err).ToNot(HaveOccurred())
Expect(ids).To(ConsistOf(slice.Map(want, func(a model.Artist) string { return a.ID })))
})

2
persistence/artwork_hydration_test.go
View File

@ -578,7 +578,7 @@ var _ = Describe("Artwork hydration", func() {
opts := model.QueryOptions{Sort: "name", Filters: onlyPlaylists}
// Both phases must filter on their own: the id pre-pass and the chunk fetch.
Expect(repo.GetAllIDs(opts)).To(ConsistOf(plsBest.ID))
Expect(repo.(*playlistRepository).getAllIDs(opts)).To(ConsistOf(plsBest.ID))
all, err := repo.GetAll(model.QueryOptions{Filters: onlyPlaylists})
Expect(err).ToNot(HaveOccurred())
Expect(slice.Map(all, func(p model.Playlist) string { return p.ID })).To(ConsistOf(plsBest.ID))

6
persistence/mediafile_repository.go
View File

@ -308,8 +308,8 @@ func (r *mediaFileRepository) GetCursor(options ...model.QueryOptions) (model.Me
return wrapMediaFileCursor(cursor), nil
}
// GetAllIDs returns the IDs of GetAll's row set, skipping its wide column projection.
func (r *mediaFileRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
// getAllIDs returns the IDs of GetAll's row set, skipping its wide column projection.
func (r *mediaFileRepository) getAllIDs(options ...model.QueryOptions) ([]string, error) {
sq := r.applyLibraryFilter(r.newSelect(options...).Columns("media_file.id"))
if filtersNeedAnnotation(sq) {
sq = r.withAnnotation(sq, "media_file.id")
@ -341,7 +341,7 @@ func (r *mediaFileRepository) GetAlbumIDsByFolder(lib model.Library, folderIDs .
// GetCursorWithArtwork streams the same rows as GetCursor, hydrated, via an id pre-pass.
func (r *mediaFileRepository) GetCursorWithArtwork(options ...model.QueryOptions) (model.MediaFileCursor, error) {
ids, err := r.GetAllIDs(options...)
ids, err := r.getAllIDs(options...)
if err != nil {
return nil, err
}

6
persistence/playlist_repository.go
View File

@ -208,8 +208,8 @@ func (r *playlistRepository) GetAll(options ...model.QueryOptions) (model.Playli
return playlists, err
}
// GetAllIDs returns the IDs of GetAll's row set, skipping its per-row processing.
func (r *playlistRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
// getAllIDs returns the IDs of GetAll's row set, skipping its per-row processing.
func (r *playlistRepository) getAllIDs(options ...model.QueryOptions) ([]string, error) {
// Joins a projection of user, not the table: its name/created_at columns would make an ORDER BY
// on the playlist's own ambiguous.
sq := r.newSelect(options...).Columns("playlist.id", "user.user_name as owner_name").
@ -224,7 +224,7 @@ func (r *playlistRepository) GetAllIDs(options ...model.QueryOptions) ([]string,
func (r *playlistRepository) GetCursor(options ...model.QueryOptions) (model.PlaylistCursor, error) {
// Both passes apply userFilter, so a visibility change between them cannot widen the cursor.
ids, err := r.GetAllIDs(options...)
ids, err := r.getAllIDs(options...)
if err != nil {
return nil, err
}

4
persistence/playlist_repository_test.go
View File

@ -74,12 +74,12 @@ var _ = Describe("PlaylistRepository", func() {
})
})
Describe("GetAllIDs", func() {
Describe("getAllIDs", func() {
It("returns the same id set as GetAll", func() {
want, err := repo.GetAll()
Expect(err).ToNot(HaveOccurred())
Expect(want).ToNot(BeEmpty())
ids, err := repo.GetAllIDs()
ids, err := repo.(*playlistRepository).getAllIDs()
Expect(err).ToNot(HaveOccurred())
Expect(ids).To(ConsistOf(slice.Map(want, func(p model.Playlist) string { return p.ID })))
})

9
tests/mock_album_repo.go
View File

@ -7,7 +7,6 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/slice"
)
func CreateMockAlbumRepo() *MockAlbumRepo {
@ -85,14 +84,6 @@ func (m *MockAlbumRepo) GetAll(qo ...model.QueryOptions) (model.Albums, error) {
return m.All, nil
}
func (m *MockAlbumRepo) GetAllIDs(qo ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(qo...)
if err != nil {
return nil, err
}
return slice.Map(all, func(a model.Album) string { return a.ID }), nil
}
func (m *MockAlbumRepo) GetCursor(qo ...model.QueryOptions) (model.AlbumCursor, error) {
res, err := m.GetAll(qo...)
if err != nil {

9
tests/mock_artist_repo.go
View File

@ -6,7 +6,6 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/slice"
)
func CreateMockArtistRepo() *MockArtistRepo {
@ -117,14 +116,6 @@ func (m *MockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, e
return allArtists, nil
}
func (m *MockArtistRepo) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(options...)
if err != nil {
return nil, err
}
return slice.Map(all, func(a model.Artist) string { return a.ID }), nil
}
func (m *MockArtistRepo) GetCursor(options ...model.QueryOptions) (model.ArtistCursor, error) {
res, err := m.GetAll(options...)
if err != nil {

8
tests/mock_mediafile_repo.go
View File

@ -130,14 +130,6 @@ func (m *MockMediaFileRepo) GetCursorWithArtwork(qo ...model.QueryOptions) (mode
return m.GetCursor(qo...)
}
func (m *MockMediaFileRepo) GetAllIDs(qo ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(qo...)
if err != nil {
return nil, err
}
return slice.Map(all, func(mf model.MediaFile) string { return mf.ID }), nil
}
func (m *MockMediaFileRepo) Put(mf *model.MediaFile) error {
if m.Err {
return errors.New("error")

9
tests/mock_playlist_repo.go
View File

@ -7,7 +7,6 @@ import (
"github.com/deluan/rest"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/slice"
)
func CreateMockPlaylistRepo() *MockPlaylistRepo {
@ -54,14 +53,6 @@ func (m *MockPlaylistRepo) GetAll(options ...model.QueryOptions) (model.Playlist
return m.All, nil
}
func (m *MockPlaylistRepo) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(options...)
if err != nil {
return nil, err
}
return slice.Map(all, func(p model.Playlist) string { return p.ID }), nil
}
func (m *MockPlaylistRepo) GetCursor(options ...model.QueryOptions) (model.PlaylistCursor, error) {
res, err := m.GetAll(options...)
if err != nil {