From a220243279b02177dad68327b21a03268d8e54a9 Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Mon, 11 Mar 2024 00:44:20 -0300 Subject: [PATCH 1/3] misc: Decouple Grid view from Albums The Grid view and related components were highly coupled to Albums, including the filter button that is currently used only for album filtering. As part of the migration of Artists to Grid view, this is the smallest possible change to convert many of the existing code to generics/interfaces that will allow different media to be displayed and filtered by using Grid views. The main changes in this diff are: * Introduction of generics for Media types (`M`) and Filter options (`F`), which can be later extended to other entities besides Albums. * Complete decoupling of `GridViewPage` and related components from Albums (making these pages also generic). * Refactoring of Subsonic/Jellyfin specific code to understand these new generics when dealing with filtering logic. --- backend/mediaprovider/helpers/iterators.go | 43 +++---- backend/mediaprovider/jellyfin/iterators.go | 36 +++--- backend/mediaprovider/mediaprovider.go | 69 +++++++---- .../mediaprovider/subsonic/albumiterator.go | 40 ++++--- .../mediaprovider/subsonic/trackiterator.go | 7 +- ui/browsing/albumspage.go | 33 +++++- ui/browsing/favoritespage.go | 18 +-- ui/browsing/genrepage.go | 28 ++++- ui/browsing/gridviewpage.go | 107 ++++++++---------- ui/widgets/albumfilterbutton.go | 54 ++++++--- ui/widgets/filterbutton.go | 13 +++ ui/widgets/gridview.go | 20 ++-- 12 files changed, 281 insertions(+), 187 deletions(-) create mode 100644 ui/widgets/filterbutton.go diff --git a/backend/mediaprovider/helpers/iterators.go b/backend/mediaprovider/helpers/iterators.go index 86c8d95..03ddc1f 100644 --- a/backend/mediaprovider/helpers/iterators.go +++ b/backend/mediaprovider/helpers/iterators.go @@ -7,17 +7,12 @@ import ( "github.com/dweymouth/supersonic/sharedutil" ) -type Filter[T any] interface { - IsNil() bool - Matches(*T) bool -} - -type baseIter[T any] struct { - filter Filter[T] - prefetchCB func(*T) +type baseIter[M, F any] struct { + filter mediaprovider.MediaFilter[M, F] + prefetchCB func(*M) serverPos int - fetcher func(offset, limit int) ([]*T, error) - prefetched []*T + fetcher func(offset, limit int) ([]*M, error) + prefetched []*M prefetchedPos int done bool } @@ -25,7 +20,7 @@ type baseIter[T any] struct { type AlbumFetchFn func(offset, limit int) ([]*mediaprovider.Album, error) func NewAlbumIterator(fetchFn AlbumFetchFn, filter mediaprovider.AlbumFilter, cb func(string)) mediaprovider.AlbumIterator { - return &baseIter[mediaprovider.Album]{ + return &baseIter[mediaprovider.Album, mediaprovider.AlbumFilterOptions]{ prefetchCB: func(a *mediaprovider.Album) { cb(a.CoverArtID) }, filter: filter, fetcher: fetchFn, @@ -35,7 +30,7 @@ func NewAlbumIterator(fetchFn AlbumFetchFn, filter mediaprovider.AlbumFilter, cb type ArtistFetchFn func(offset, limit int) ([]*mediaprovider.Artist, error) func NewArtistIterator(fetchFn ArtistFetchFn) mediaprovider.ArtistIterator { - return &baseIter[mediaprovider.Artist]{ + return &baseIter[mediaprovider.Artist, nilFilterOptions]{ fetcher: fetchFn, filter: nilFilter[mediaprovider.Artist]{}, } @@ -44,14 +39,14 @@ func NewArtistIterator(fetchFn ArtistFetchFn) mediaprovider.ArtistIterator { type TrackFetchFn func(offset, limit int) ([]*mediaprovider.Track, error) func NewTrackIterator(fetchFn TrackFetchFn, cb func(string)) mediaprovider.TrackIterator { - return &baseIter[mediaprovider.Track]{ + return &baseIter[mediaprovider.Track, nilFilterOptions]{ prefetchCB: func(a *mediaprovider.Track) { cb(a.CoverArtID) }, filter: nilFilter[mediaprovider.Track]{}, fetcher: fetchFn, } } -func (r *baseIter[T]) Next() *T { +func (r *baseIter[M, F]) Next() *M { if r.done { return nil } @@ -89,7 +84,7 @@ func (r *baseIter[T]) Next() *T { return r.prefetched[0] } -type randomIter struct { +type randomAlbumIter struct { filter mediaprovider.AlbumFilter prefetchCB func(coverArtID string) albumIDSet map[string]bool @@ -108,8 +103,8 @@ type randomIter struct { done bool } -func NewRandomAlbumIter(deterministicFetcher, randomFetcher AlbumFetchFn, filter mediaprovider.AlbumFilter, prefetchCoverCB func(string)) *randomIter { - return &randomIter{ +func NewRandomAlbumIter(deterministicFetcher, randomFetcher AlbumFetchFn, filter mediaprovider.AlbumFilter, prefetchCoverCB func(string)) *randomAlbumIter { + return &randomAlbumIter{ filter: filter, prefetchCB: prefetchCoverCB, deterministicFetcher: deterministicFetcher, @@ -118,7 +113,7 @@ func NewRandomAlbumIter(deterministicFetcher, randomFetcher AlbumFetchFn, filter } } -func (r *randomIter) Next() *mediaprovider.Album { +func (r *randomAlbumIter) Next() *mediaprovider.Album { if r.done { return nil } @@ -192,8 +187,14 @@ func (r *randomIter) Next() *mediaprovider.Album { return nil } -type nilFilter[T any] struct{} +type nilFilterOptions struct{} -func (n nilFilter[T]) IsNil() bool { return true } +type nilFilter[M any] struct{} -func (n nilFilter[T]) Matches(*T) bool { return true } +func (n nilFilter[M]) IsNil() bool { return true } + +func (n nilFilter[M]) Matches(*M) bool { return true } + +func (n nilFilter[M]) Options() nilFilterOptions { return nilFilterOptions{} } + +func (n nilFilter[M]) SetOptions(o nilFilterOptions) {} diff --git a/backend/mediaprovider/jellyfin/iterators.go b/backend/mediaprovider/jellyfin/iterators.go index 311a9eb..6a630d5 100644 --- a/backend/mediaprovider/jellyfin/iterators.go +++ b/backend/mediaprovider/jellyfin/iterators.go @@ -58,7 +58,7 @@ func (j *jellyfinMediaProvider) IterateAlbums(sortOrder string, filter mediaprov jfSort.Field = jellyfin.SortByYear jfSort.Mode = jellyfin.SortDesc } - jfFilt := jfFilterFromFilter(&filter) + jfFilt := jfFilterFromFilter(filter) fetcher := func(offs, limit int) ([]*mediaprovider.Album, error) { al, err := j.client.GetAlbums(jellyfin.QueryOpts{ @@ -152,23 +152,29 @@ func (j *jellyfinMediaProvider) IterateArtists(sortOrder string) mediaprovider.A // Creates the Jellyfin filter to implement the given mediaprovider filter, // and zeros out the now-unneeded fields in the mediaprovider filter. -func jfFilterFromFilter(filter *mediaprovider.AlbumFilter) jellyfin.Filter { +func jfFilterFromFilter(filter mediaprovider.AlbumFilter) jellyfin.Filter { var jfFilt jellyfin.Filter - if filter.ExcludeUnfavorited { + + filterOptions := filter.Options() + + if filterOptions.ExcludeUnfavorited { jfFilt.Favorite = true - filter.ExcludeUnfavorited = false // Jellyfin will handle this filter + filterOptions.ExcludeUnfavorited = false // Jellyfin will handle this filter } - if filter.MinYear > 0 && filter.MaxYear > 0 { - jfFilt.YearRange = [2]int{filter.MinYear, filter.MaxYear} - filter.MinYear, filter.MaxYear = 0, 0 - } else if filter.MinYear > 0 { - jfFilt.YearRange = [2]int{filter.MinYear, time.Now().Year()} - filter.MinYear, filter.MaxYear = 0, 0 - } else if filter.MaxYear > 0 { - jfFilt.YearRange = [2]int{1900, filter.MaxYear} - filter.MinYear, filter.MaxYear = 0, 0 + if filterOptions.MinYear > 0 && filterOptions.MaxYear > 0 { + jfFilt.YearRange = [2]int{filterOptions.MinYear, filterOptions.MaxYear} + filterOptions.MinYear, filterOptions.MaxYear = 0, 0 + } else if filterOptions.MinYear > 0 { + jfFilt.YearRange = [2]int{filterOptions.MinYear, time.Now().Year()} + filterOptions.MinYear, filterOptions.MaxYear = 0, 0 + } else if filterOptions.MaxYear > 0 { + jfFilt.YearRange = [2]int{1900, filterOptions.MaxYear} + filterOptions.MinYear, filterOptions.MaxYear = 0, 0 } - jfFilt.Genres = filter.Genres - filter.Genres = nil + jfFilt.Genres = filterOptions.Genres + filterOptions.Genres = nil + + filter.SetOptions(filterOptions) + return jfFilt } diff --git a/backend/mediaprovider/mediaprovider.go b/backend/mediaprovider/mediaprovider.go index 52d455e..5627956 100644 --- a/backend/mediaprovider/mediaprovider.go +++ b/backend/mediaprovider/mediaprovider.go @@ -7,7 +7,24 @@ import ( "strings" ) -type AlbumFilter struct { +type MediaIterator[M any] interface { + Next() *M +} + +type ArtistIterator = MediaIterator[Artist] +type AlbumIterator = MediaIterator[Album] +type TrackIterator = MediaIterator[Track] + +type MediaFilter[M, F any] interface { + Options() F + SetOptions(F) + IsNil() bool + Matches(*M) bool +} + +type AlbumFilter = MediaFilter[Album, AlbumFilterOptions] + +type AlbumFilterOptions struct { MinYear int MaxYear int // 0 == unset/match any Genres []string // len(0) == unset/match any @@ -16,42 +33,46 @@ type AlbumFilter struct { ExcludeUnfavorited bool // mut. exc. with ExcludeFavorited } -// Returns true if the filter is the nil filter - i.e. matches everything -func (a AlbumFilter) IsNil() bool { - return a.MinYear == 0 && a.MaxYear == 0 && - len(a.Genres) == 0 && - !a.ExcludeFavorited && !a.ExcludeUnfavorited +type albumFilter struct { + options AlbumFilterOptions } -func (f AlbumFilter) Matches(album *Album) bool { +func NewAlbumFilter(options AlbumFilterOptions) *albumFilter { + return &albumFilter{options} +} + +func (a albumFilter) Options() AlbumFilterOptions { + return a.options +} + +func (a *albumFilter) SetOptions(o AlbumFilterOptions) { + a.options = o +} + +// Returns true if the filter is the nil filter - i.e. matches everything +func (a albumFilter) IsNil() bool { + return a.options.MinYear == 0 && a.options.MaxYear == 0 && + len(a.options.Genres) == 0 && + !a.options.ExcludeFavorited && !a.options.ExcludeUnfavorited +} + +func (f albumFilter) Matches(album *Album) bool { if album == nil { return false } - if f.ExcludeFavorited && album.Favorite { + if f.options.ExcludeFavorited && album.Favorite { return false } - if f.ExcludeUnfavorited && !album.Favorite { + if f.options.ExcludeUnfavorited && !album.Favorite { return false } - if y := album.Year; y < f.MinYear || (f.MaxYear > 0 && y > f.MaxYear) { + if y := album.Year; y < f.options.MinYear || (f.options.MaxYear > 0 && y > f.options.MaxYear) { return false } - if len(f.Genres) == 0 { + if len(f.options.Genres) == 0 { return true } - return genresMatch(f.Genres, album.Genres) -} - -type ArtistIterator interface { - Next() *Artist -} - -type AlbumIterator interface { - Next() *Album -} - -type TrackIterator interface { - Next() *Track + return genresMatch(f.options.Genres, album.Genres) } type RatingFavoriteParameters struct { diff --git a/backend/mediaprovider/subsonic/albumiterator.go b/backend/mediaprovider/subsonic/albumiterator.go index 6818972..b16b731 100644 --- a/backend/mediaprovider/subsonic/albumiterator.go +++ b/backend/mediaprovider/subsonic/albumiterator.go @@ -35,23 +35,24 @@ func (s *subsonicMediaProvider) AlbumSortOrders() []string { } } -func filterMatches(f mediaprovider.AlbumFilter, album *subsonic.AlbumID3, ignoreGenre bool) bool { +func filterAlbumMatches(f mediaprovider.AlbumFilter, album *subsonic.AlbumID3, ignoreGenre bool) bool { + filterOptions := f.Options() if album == nil { return false } - if f.ExcludeFavorited && !album.Starred.IsZero() { + if filterOptions.ExcludeFavorited && !album.Starred.IsZero() { return false } - if f.ExcludeUnfavorited && album.Starred.IsZero() { + if filterOptions.ExcludeUnfavorited && album.Starred.IsZero() { return false } - if y := album.Year; y < f.MinYear || (f.MaxYear > 0 && y > f.MaxYear) { + if y := album.Year; y < filterOptions.MinYear || (filterOptions.MaxYear > 0 && y > filterOptions.MaxYear) { return false } - if ignoreGenre || len(f.Genres) == 0 { + if ignoreGenre || len(filterOptions.Genres) == 0 { return true } - for _, g := range f.Genres { + for _, g := range filterOptions.Genres { if strings.EqualFold(g, album.Genre) { return true } @@ -60,21 +61,24 @@ func filterMatches(f mediaprovider.AlbumFilter, album *subsonic.AlbumID3, ignore } func (s *subsonicMediaProvider) IterateAlbums(sortOrder string, filter mediaprovider.AlbumFilter) mediaprovider.AlbumIterator { - if sortOrder == "" && len(filter.Genres) == 1 { - genre := filter.Genres[0] + filterOptions := filter.Options() + if sortOrder == "" && len(filterOptions.Genres) == 1 { + genre := filterOptions.Genres[0] // The Subsonic API (non-OpenSubsonic) returns only the first genre for multi-genre albums, // but servers do internally match against all the genres the album is categorized with. // So we must not additionally filter by genre to avoid excluding results where // the single genre returned by Subsonic isn't the one we're iterating on. - filter.Genres = nil + filterOptions.Genres = nil fetchFn := func(offset, limit int) ([]*subsonic.AlbumID3, error) { return s.client.GetAlbumList2("byGenre", map[string]string{"genre": genre, "offset": strconv.Itoa(offset), "limit": strconv.Itoa(limit)}) } + filter.SetOptions(filterOptions) return helpers.NewAlbumIterator(makeFetchFn(fetchFn), filter, s.prefetchCoverCB) } - if sortOrder == "" && filter.ExcludeUnfavorited { - filter.ExcludeUnfavorited = false // we're already filtering by this + if sortOrder == "" && filterOptions.ExcludeUnfavorited { + filterOptions.ExcludeUnfavorited = false // we're already filtering by this + filter.SetOptions(filterOptions) return s.baseIterFromSimpleSortOrder("starred", filter) } if sortOrder == "" { @@ -112,10 +116,10 @@ func (s *subsonicMediaProvider) IterateAlbums(sortOrder string, filter mediaprov } func (s *subsonicMediaProvider) SearchAlbums(searchQuery string, filter mediaprovider.AlbumFilter) mediaprovider.AlbumIterator { - return s.newSearchIter(searchQuery, filter, s.prefetchCoverCB) + return s.newSearchAlbumIter(searchQuery, filter, s.prefetchCoverCB) } -type searchIter struct { +type searchAlbumIter struct { searchIterBase prefetchCB func(string) @@ -126,8 +130,8 @@ type searchIter struct { done bool } -func (s *subsonicMediaProvider) newSearchIter(query string, filter mediaprovider.AlbumFilter, cb func(string)) *searchIter { - return &searchIter{ +func (s *subsonicMediaProvider) newSearchAlbumIter(query string, filter mediaprovider.AlbumFilter, cb func(string)) *searchAlbumIter { + return &searchAlbumIter{ searchIterBase: searchIterBase{ query: query, s: s.client, @@ -138,7 +142,7 @@ func (s *subsonicMediaProvider) newSearchIter(query string, filter mediaprovider } } -func (s *searchIter) Next() *mediaprovider.Album { +func (s *searchAlbumIter) Next() *mediaprovider.Album { if s.done { return nil } @@ -197,12 +201,12 @@ func (s *searchIter) Next() *mediaprovider.Album { return nil } -func (s *searchIter) addNewAlbums(al []*subsonic.AlbumID3) { +func (s *searchAlbumIter) addNewAlbums(al []*subsonic.AlbumID3) { for _, album := range al { if _, have := s.albumIDset[album.ID]; have { continue } - if !filterMatches(s.filter, album, false) { + if !filterAlbumMatches(s.filter, album, false) { continue } s.prefetched = append(s.prefetched, album) diff --git a/backend/mediaprovider/subsonic/trackiterator.go b/backend/mediaprovider/subsonic/trackiterator.go index 3758368..1a7ec72 100644 --- a/backend/mediaprovider/subsonic/trackiterator.go +++ b/backend/mediaprovider/subsonic/trackiterator.go @@ -10,8 +10,11 @@ import ( func (s *subsonicMediaProvider) IterateTracks(searchQuery string) mediaprovider.TrackIterator { if searchQuery == "" { return &allTracksIterator{ - s: s, - albumIter: s.IterateAlbums(AlbumSortArtistAZ, mediaprovider.AlbumFilter{}), + s: s, + albumIter: s.IterateAlbums( + AlbumSortArtistAZ, + mediaprovider.NewAlbumFilter(mediaprovider.AlbumFilterOptions{}), + ), } } return &searchTracksIterator{ diff --git a/ui/browsing/albumspage.go b/ui/browsing/albumspage.go index cf4d0c4..8fafe68 100644 --- a/ui/browsing/albumspage.go +++ b/ui/browsing/albumspage.go @@ -14,10 +14,12 @@ import ( ) type albumsPageAdapter struct { - cfg *backend.AlbumsPageConfig - contr *controller.Controller - mp mediaprovider.MediaProvider - pm *backend.PlaybackManager + cfg *backend.AlbumsPageConfig + contr *controller.Controller + mp mediaprovider.MediaProvider + pm *backend.PlaybackManager + filter mediaprovider.AlbumFilter + filterBtn *widgets.AlbumFilterButton } func NewAlbumsPage(cfg *backend.AlbumsPageConfig, pool *util.WidgetPool, contr *controller.Controller, pm *backend.PlaybackManager, mp mediaprovider.MediaProvider, im *backend.ImageManager) Page { @@ -27,8 +29,27 @@ func NewAlbumsPage(cfg *backend.AlbumsPageConfig, pool *util.WidgetPool, contr * func (a *albumsPageAdapter) Title() string { return "Albums" } -func (a *albumsPageAdapter) Filter() *mediaprovider.AlbumFilter { - return &mediaprovider.AlbumFilter{} +func (a *albumsPageAdapter) Filter() mediaprovider.AlbumFilter { + if a.filter == nil { + a.filter = mediaprovider.NewAlbumFilter( + mediaprovider.AlbumFilterOptions{}, + ) + } + return a.filter +} + +func (a *albumsPageAdapter) FilterButton() widgets.FilterButton[mediaprovider.Album, mediaprovider.AlbumFilterOptions] { + if a.filterBtn == nil { + disableGenres := len(a.Filter().Options().Genres) > 0 + genreFn := a.mp.GetGenres + if disableGenres { + // genre filter is disabled for this page, so no need to actually call genre list fetching function + genreFn = func() ([]*mediaprovider.Genre, error) { return nil, nil } + } + a.filterBtn = widgets.NewAlbumFilterButton(a.Filter(), genreFn) + a.filterBtn.GenreDisabled = disableGenres + } + return a.filterBtn } func (a *albumsPageAdapter) PlaceholderResource() fyne.Resource { return myTheme.AlbumIcon } diff --git a/ui/browsing/favoritespage.go b/ui/browsing/favoritespage.go index 50a0195..9e78fae 100644 --- a/ui/browsing/favoritespage.go +++ b/ui/browsing/favoritespage.go @@ -51,13 +51,15 @@ type FavoritesPage struct { func NewFavoritesPage(cfg *backend.FavoritesPageConfig, pool *util.WidgetPool, contr *controller.Controller, mp mediaprovider.MediaProvider, pm *backend.PlaybackManager, im *backend.ImageManager) *FavoritesPage { a := &FavoritesPage{ - filter: mediaprovider.AlbumFilter{ExcludeUnfavorited: true}, - cfg: cfg, - pool: pool, - contr: contr, - pm: pm, - mp: mp, - im: im, + filter: mediaprovider.NewAlbumFilter(mediaprovider.AlbumFilterOptions{ + ExcludeUnfavorited: true, + }), + cfg: cfg, + pool: pool, + contr: contr, + pm: pm, + mp: mp, + im: im, } a.ExtendBaseWidget(a) a.createHeader(0) @@ -95,7 +97,7 @@ func (a *FavoritesPage) createHeader(activeBtnIdx int) { a.searcher.PlaceHolder = "Search page" a.searcher.OnSearched = a.OnSearched a.searcher.Entry.Text = a.searchText - a.filterBtn = widgets.NewAlbumFilterButton(&a.filter, a.mp.GetGenres) + a.filterBtn = widgets.NewAlbumFilterButton(a.filter, a.mp.GetGenres) a.filterBtn.FavoriteDisabled = true a.filterBtn.OnChanged = a.Reload } diff --git a/ui/browsing/genrepage.go b/ui/browsing/genrepage.go index 9ac4873..e330fed 100644 --- a/ui/browsing/genrepage.go +++ b/ui/browsing/genrepage.go @@ -13,10 +13,12 @@ import ( ) type genrePageAdapter struct { - genre string - contr *controller.Controller - mp mediaprovider.MediaProvider - pm *backend.PlaybackManager + genre string + contr *controller.Controller + mp mediaprovider.MediaProvider + pm *backend.PlaybackManager + filter mediaprovider.AlbumFilter + filterBtn *widgets.AlbumFilterButton } func NewGenrePage(genre string, pool *util.WidgetPool, contr *controller.Controller, pm *backend.PlaybackManager, mp mediaprovider.MediaProvider, im *backend.ImageManager) Page { @@ -26,8 +28,22 @@ func NewGenrePage(genre string, pool *util.WidgetPool, contr *controller.Control func (g *genrePageAdapter) Title() string { return g.genre } -func (g *genrePageAdapter) Filter() *mediaprovider.AlbumFilter { - return &mediaprovider.AlbumFilter{Genres: []string{g.genre}} +func (g *genrePageAdapter) Filter() mediaprovider.AlbumFilter { + if g.filter == nil { + g.filter = mediaprovider.NewAlbumFilter( + mediaprovider.AlbumFilterOptions{ + Genres: []string{g.genre}, + }, + ) + } + return g.filter +} + +func (g *genrePageAdapter) FilterButton() widgets.FilterButton[mediaprovider.Album, mediaprovider.AlbumFilterOptions] { + if g.filterBtn == nil { + g.filterBtn = widgets.NewAlbumFilterButton(g.Filter(), g.mp.GetGenres) + } + return g.filterBtn } func (g *genrePageAdapter) PlaceholderResource() fyne.Resource { diff --git a/ui/browsing/gridviewpage.go b/ui/browsing/gridviewpage.go index 7815765..576e8d1 100644 --- a/ui/browsing/gridviewpage.go +++ b/ui/browsing/gridviewpage.go @@ -14,13 +14,11 @@ import ( "fyne.io/fyne/v2/widget" ) -var _ Page = (*GridViewPage)(nil) - // Base widget for grid view pages -type GridViewPage struct { +type GridViewPage[M, F any] struct { widget.BaseWidget - adapter GridViewPageAdapter + adapter GridViewPageAdapter[M, F] pool *util.WidgetPool mp mediaprovider.MediaProvider im *backend.ImageManager @@ -31,8 +29,8 @@ type GridViewPage struct { title *widget.RichText sortOrder *sortOrderSelect - filterBtn *widgets.AlbumFilterButton - filter *mediaprovider.AlbumFilter + filterBtn widgets.FilterButton[M, F] + filter mediaprovider.MediaFilter[M, F] searcher *widgets.SearchEntry searchText string @@ -40,14 +38,17 @@ type GridViewPage struct { } // Base type for pages that show an iterable GridView -type GridViewPageAdapter interface { +type GridViewPageAdapter[M, F any] interface { // Returns the title for the page Title() string - // Returns the base album filter for this page, if any. + // Returns the base media filter for this page, if any. // A filterable page with no base filters applied should return a zero-valued - // *AlbumFilter, *not* nil. (Nil means unfilterable and no filter button created.) - Filter() *mediaprovider.AlbumFilter + // filter pointer, *not* nil. (Nil means unfilterable and no filter button created.) + Filter() mediaprovider.MediaFilter[M, F] + + // Returns the filter button for the page, if any. + FilterButton() widgets.FilterButton[M, F] // Returns the cover placeholder resource for the page PlaceholderResource() fyne.Resource @@ -59,11 +60,11 @@ type GridViewPageAdapter interface { ActionButton() *widget.Button // Returns the iterator for the given sortOrder and filter. - // (Non-album pages can ignore the filter argument) - Iter(sortOrder string, filter mediaprovider.AlbumFilter) widgets.GridViewIterator + // (Non-media pages can ignore the filter argument) + Iter(sortOrder string, filter mediaprovider.MediaFilter[M, F]) widgets.GridViewIterator // Returns the iterator for the given search query and filter. - SearchIter(query string, filter mediaprovider.AlbumFilter) widgets.GridViewIterator + SearchIter(query string, filter mediaprovider.MediaFilter[M, F]) widgets.GridViewIterator // Function that connects the GridView callbacks to the appropriate action handlers. ConnectGridActions(*widgets.GridView) @@ -96,18 +97,19 @@ func (s *sortOrderSelect) MinSize() fyne.Size { return fyne.NewSize(170, s.Select.MinSize().Height) } -func NewGridViewPage( - adapter GridViewPageAdapter, +func NewGridViewPage[M, F any]( + adapter GridViewPageAdapter[M, F], pool *util.WidgetPool, mp mediaprovider.MediaProvider, im *backend.ImageManager, -) *GridViewPage { - gp := &GridViewPage{ - adapter: adapter, - pool: pool, - mp: mp, - im: im, - filter: adapter.Filter(), +) *GridViewPage[M, F] { + gp := &GridViewPage[M, F]{ + adapter: adapter, + pool: pool, + mp: mp, + im: im, + filter: adapter.Filter(), + filterBtn: adapter.FilterButton(), } gp.ExtendBaseWidget(gp) gp.createTitleAndSort() @@ -128,7 +130,7 @@ func NewGridViewPage( return gp } -func (g *GridViewPage) createTitleAndSort() { +func (g *GridViewPage[M, F]) createTitleAndSort() { g.title = widget.NewRichText(&widget.TextSegment{ Text: g.adapter.Title(), Style: widget.RichTextStyle{SizeName: theme.SizeNameHeadingText}, @@ -140,25 +142,17 @@ func (g *GridViewPage) createTitleAndSort() { } } -func (g *GridViewPage) createSearchAndFilter() { +func (g *GridViewPage[M, F]) createSearchAndFilter() { g.searcher = widgets.NewSearchEntry() g.searcher.PlaceHolder = "Search page" g.searcher.Text = g.searchText g.searcher.OnSearched = g.OnSearched - if g.filter != nil { - disableGenres := len(g.filter.Genres) > 0 - genreFn := g.mp.GetGenres - if disableGenres { - // genre filter is disabled for this page, so no need to actually call genre list fetching function - genreFn = func() ([]*mediaprovider.Genre, error) { return nil, nil } - } - g.filterBtn = widgets.NewAlbumFilterButton(g.filter, genreFn) - g.filterBtn.GenreDisabled = disableGenres - g.filterBtn.OnChanged = g.Reload + if g.filterBtn != nil { + g.filterBtn.SetOnChanged(g.Reload) } } -func (g *GridViewPage) createContainer() { +func (g *GridViewPage[M, F]) createContainer() { header := container.NewHBox(util.NewHSpace(6), g.title) if g.sortOrder != nil { header.Add(container.NewCenter(g.sortOrder)) @@ -175,7 +169,7 @@ func (g *GridViewPage) createContainer() { g.container = container.NewBorder(header, nil, nil, nil, g.grid) } -func (g *GridViewPage) Reload() { +func (g *GridViewPage[M, F]) Reload() { if g.searchText != "" { g.doSearch(g.searchText) } else { @@ -183,23 +177,19 @@ func (g *GridViewPage) Reload() { } } -func (g *GridViewPage) Route() controller.Route { +func (g *GridViewPage[M, F]) Route() controller.Route { return g.adapter.Route() } -var _ Searchable = (*GridViewPage)(nil) - -func (g *GridViewPage) SearchWidget() fyne.Focusable { +func (g *GridViewPage[M, F]) SearchWidget() fyne.Focusable { return g.searcher } -var _ Scrollable = (*GridViewPage)(nil) - -func (g *GridViewPage) Scroll(scrollAmt float32) { +func (g *GridViewPage[M, F]) Scroll(scrollAmt float32) { g.grid.ScrollToOffset(g.grid.GetScrollOffset() + scrollAmt) } -func (g *GridViewPage) OnSearched(query string) { +func (g *GridViewPage[M, F]) OnSearched(query string) { if query == "" { if g.sortOrder != nil { g.sortOrder.Enable() @@ -215,50 +205,47 @@ func (g *GridViewPage) OnSearched(query string) { g.searchText = query } -func (g *GridViewPage) doSearch(query string) { +func (g *GridViewPage[M, F]) doSearch(query string) { if g.searchText == "" { g.gridState = g.grid.SaveToState() } g.grid.Reset(g.adapter.SearchIter(query, g.getFilter())) } -func (g *GridViewPage) onSortOrderChanged(order string) { +func (g *GridViewPage[M, F]) onSortOrderChanged(order string) { g.adapter.(SortableGridViewPageAdapter).SaveSortOrder(g.getSortOrder()) g.grid.Reset(g.adapter.Iter(g.getSortOrder(), g.getFilter())) } -func (g *GridViewPage) getFilter() mediaprovider.AlbumFilter { - if g.filter != nil { - return *g.filter - } - return mediaprovider.AlbumFilter{} +func (g *GridViewPage[M, F]) getFilter() mediaprovider.MediaFilter[M, F] { + return g.filter } -func (g *GridViewPage) getSortOrder() string { +func (g *GridViewPage[M, F]) getSortOrder() string { if g.sortOrder != nil { return g.sortOrder.Selected } return "" } -func (g *GridViewPage) CreateRenderer() fyne.WidgetRenderer { +func (g *GridViewPage[M, F]) CreateRenderer() fyne.WidgetRenderer { return widget.NewSimpleRenderer(g.container) } -type savedGridViewPage struct { - adapter GridViewPageAdapter +type savedGridViewPage[M, F any] struct { + adapter GridViewPageAdapter[M, F] im *backend.ImageManager mp mediaprovider.MediaProvider searchText string - filter *mediaprovider.AlbumFilter + filter mediaprovider.MediaFilter[M, F] pool *util.WidgetPool sortOrder string gridState *widgets.GridViewState searchGridState *widgets.GridViewState } -func (g *GridViewPage) Save() SavedPage { - sa := &savedGridViewPage{ +func (g *GridViewPage[M, F]) Save() SavedPage { + sa := &savedGridViewPage[M, F]{ adapter: g.adapter, pool: g.pool, mp: g.mp, @@ -279,8 +266,8 @@ func (g *GridViewPage) Save() SavedPage { return sa } -func (s *savedGridViewPage) Restore() Page { - gp := &GridViewPage{ +func (s *savedGridViewPage[M, F]) Restore() Page { + gp := &GridViewPage[M, F]{ adapter: s.adapter, pool: s.pool, mp: s.mp, diff --git a/ui/widgets/albumfilterbutton.go b/ui/widgets/albumfilterbutton.go index 9a6b546..9299b5c 100644 --- a/ui/widgets/albumfilterbutton.go +++ b/ui/widgets/albumfilterbutton.go @@ -30,11 +30,11 @@ type AlbumFilterButton struct { genreListChan chan []string - filter *mediaprovider.AlbumFilter + filter mediaprovider.AlbumFilter dialog *widget.PopUp } -func NewAlbumFilterButton(filter *mediaprovider.AlbumFilter, fetchGenresFunc func() ([]*mediaprovider.Genre, error)) *AlbumFilterButton { +func NewAlbumFilterButton(filter mediaprovider.AlbumFilter, fetchGenresFunc func() ([]*mediaprovider.Genre, error)) *AlbumFilterButton { a := &AlbumFilterButton{ filter: filter, Button: widget.Button{ @@ -66,10 +66,19 @@ func (a *AlbumFilterButton) Refresh() { a.Button.Refresh() } +func (a *AlbumFilterButton) Filter() mediaprovider.AlbumFilter { + return a.filter +} + +func (a *AlbumFilterButton) SetOnChanged(fn func()) { + a.OnChanged = fn +} + func (a *AlbumFilterButton) filterEmpty() bool { - return a.filter.MinYear == 0 && a.filter.MaxYear == 0 && - (a.FavoriteDisabled || !a.filter.ExcludeFavorited && !a.filter.ExcludeUnfavorited) && - (a.GenreDisabled || len(a.filter.Genres) == 0) + filterOptions := a.filter.Options() + return filterOptions.MinYear == 0 && filterOptions.MaxYear == 0 && + (a.FavoriteDisabled || !filterOptions.ExcludeFavorited && !filterOptions.ExcludeUnfavorited) && + (a.GenreDisabled || len(filterOptions.Genres) == 0) } func (a *AlbumFilterButton) onFilterChanged() { @@ -115,53 +124,64 @@ func NewAlbumFilterPopup(filter *AlbumFilterButton) *AlbumFilterPopup { minYear := NewTextRestrictedEntry(yearValidator) minYear.SetMinCharWidth(4) minYear.OnChanged = func(yearStr string) { + filterOptions := a.filterBtn.filter.Options() if yearStr == "" { - a.filterBtn.filter.MinYear = 0 + filterOptions.MinYear = 0 } else if i, err := strconv.Atoi(yearStr); err == nil { - a.filterBtn.filter.MinYear = i + filterOptions.MinYear = i } + a.filterBtn.filter.SetOptions(filterOptions) debounceOnChanged() } - if a.filterBtn.filter.MinYear > 0 { - minYear.Text = strconv.Itoa(a.filterBtn.filter.MinYear) + filterOptions := a.filterBtn.filter.Options() + if filterOptions.MinYear > 0 { + minYear.Text = strconv.Itoa(filterOptions.MinYear) } maxYear := NewTextRestrictedEntry(yearValidator) maxYear.SetMinCharWidth(4) maxYear.OnChanged = func(yearStr string) { + filterOptions := a.filterBtn.filter.Options() if yearStr == "" { - a.filterBtn.filter.MaxYear = 0 + filterOptions.MaxYear = 0 } else if i, err := strconv.Atoi(yearStr); err == nil { - a.filterBtn.filter.MaxYear = i + filterOptions.MaxYear = i } + a.filterBtn.filter.SetOptions(filterOptions) debounceOnChanged() } - if a.filterBtn.filter.MaxYear > 0 { - maxYear.Text = strconv.Itoa(a.filterBtn.filter.MaxYear) + if filterOptions.MaxYear > 0 { + maxYear.Text = strconv.Itoa(filterOptions.MaxYear) } // setup is favorite/not favorite filters a.isFavorite = widget.NewCheck("Is favorite", func(fav bool) { + filterOptions := a.filterBtn.filter.Options() if fav { a.isNotFavorite.SetChecked(false) } - a.filterBtn.filter.ExcludeUnfavorited = fav + filterOptions.ExcludeUnfavorited = fav + a.filterBtn.filter.SetOptions(filterOptions) debounceOnChanged() }) a.isFavorite.Hidden = a.filterBtn.FavoriteDisabled a.isNotFavorite = widget.NewCheck("Is not favorite", func(fav bool) { + filterOptions := a.filterBtn.filter.Options() if fav { a.isFavorite.SetChecked(false) } - a.filterBtn.filter.ExcludeFavorited = fav + filterOptions.ExcludeFavorited = fav + a.filterBtn.filter.SetOptions(filterOptions) debounceOnChanged() }) a.isNotFavorite.Hidden = a.filterBtn.FavoriteDisabled // create genre filter subsection a.genreFilter = NewGenreFilterSubsection(func(selectedGenres []string) { - a.filterBtn.filter.Genres = selectedGenres + filterOptions := a.filterBtn.filter.Options() + filterOptions.Genres = selectedGenres + a.filterBtn.filter.SetOptions(filterOptions) debounceOnChanged() - }, a.filterBtn.filter.Genres) + }, filterOptions.Genres) a.genreFilter.Hidden = a.filterBtn.GenreDisabled // setup container diff --git a/ui/widgets/filterbutton.go b/ui/widgets/filterbutton.go new file mode 100644 index 0000000..1c44277 --- /dev/null +++ b/ui/widgets/filterbutton.go @@ -0,0 +1,13 @@ +package widgets + +import ( + "fyne.io/fyne/v2" + "github.com/dweymouth/supersonic/backend/mediaprovider" +) + +type FilterButton[M, F any] interface { + fyne.CanvasObject + + Filter() mediaprovider.MediaFilter[M, F] + SetOnChanged(func()) +} diff --git a/ui/widgets/gridview.go b/ui/widgets/gridview.go index ec88fbf..fbe4b25 100644 --- a/ui/widgets/gridview.go +++ b/ui/widgets/gridview.go @@ -18,23 +18,23 @@ import ( const batchFetchSize = 6 -type BatchingIterator struct { - iter mediaprovider.AlbumIterator +type BatchingIterator[M any] struct { + iter mediaprovider.MediaIterator[M] } -func NewBatchingIterator(iter mediaprovider.AlbumIterator) BatchingIterator { - return BatchingIterator{iter} +func NewBatchingIterator[M any](iter mediaprovider.MediaIterator[M]) BatchingIterator[M] { + return BatchingIterator[M]{iter} } -func (b *BatchingIterator) NextN(n int) []*mediaprovider.Album { - results := make([]*mediaprovider.Album, 0, n) +func (b *BatchingIterator[M]) NextN(n int) []*M { + results := make([]*M, 0, n) i := 0 for i < n { - album := b.iter.Next() - if album == nil { + value := b.iter.Next() + if value == nil { break } - results = append(results, album) + results = append(results, value) i++ } return results @@ -45,7 +45,7 @@ type GridViewIterator interface { } type gridViewAlbumIterator struct { - iter BatchingIterator + iter BatchingIterator[mediaprovider.Album] } func (g gridViewAlbumIterator) NextN(n int) []GridViewItemModel { From ab2ac363b0153dbd99dbc2f5c99ab8c7479e19b2 Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Mon, 11 Mar 2024 23:59:53 -0300 Subject: [PATCH 2/3] fix: Disable genre filter only on Genre page --- ui/browsing/albumspage.go | 9 +-------- ui/browsing/genrepage.go | 3 ++- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/ui/browsing/albumspage.go b/ui/browsing/albumspage.go index 8fafe68..c5f2252 100644 --- a/ui/browsing/albumspage.go +++ b/ui/browsing/albumspage.go @@ -40,14 +40,7 @@ func (a *albumsPageAdapter) Filter() mediaprovider.AlbumFilter { func (a *albumsPageAdapter) FilterButton() widgets.FilterButton[mediaprovider.Album, mediaprovider.AlbumFilterOptions] { if a.filterBtn == nil { - disableGenres := len(a.Filter().Options().Genres) > 0 - genreFn := a.mp.GetGenres - if disableGenres { - // genre filter is disabled for this page, so no need to actually call genre list fetching function - genreFn = func() ([]*mediaprovider.Genre, error) { return nil, nil } - } - a.filterBtn = widgets.NewAlbumFilterButton(a.Filter(), genreFn) - a.filterBtn.GenreDisabled = disableGenres + a.filterBtn = widgets.NewAlbumFilterButton(a.Filter(), a.mp.GetGenres) } return a.filterBtn } diff --git a/ui/browsing/genrepage.go b/ui/browsing/genrepage.go index e330fed..279d1e2 100644 --- a/ui/browsing/genrepage.go +++ b/ui/browsing/genrepage.go @@ -41,7 +41,8 @@ func (g *genrePageAdapter) Filter() mediaprovider.AlbumFilter { func (g *genrePageAdapter) FilterButton() widgets.FilterButton[mediaprovider.Album, mediaprovider.AlbumFilterOptions] { if g.filterBtn == nil { - g.filterBtn = widgets.NewAlbumFilterButton(g.Filter(), g.mp.GetGenres) + g.filterBtn = widgets.NewAlbumFilterButton(g.Filter(), func() ([]*mediaprovider.Genre, error) { return nil, nil }) + g.filterBtn.GenreDisabled = true } return g.filterBtn } From b8cd79e6defc68ca33e5808ef23aa2007825dfe7 Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Sun, 17 Mar 2024 00:56:49 -0300 Subject: [PATCH 3/3] fix: Add cloning logic to filters, make providers not update original filter --- backend/mediaprovider/helpers/iterators.go | 4 +++- backend/mediaprovider/jellyfin/iterators.go | 24 +++++++++++-------- backend/mediaprovider/mediaprovider.go | 23 ++++++++++++++++-- .../mediaprovider/subsonic/albumiterator.go | 16 ++++++++----- 4 files changed, 48 insertions(+), 19 deletions(-) diff --git a/backend/mediaprovider/helpers/iterators.go b/backend/mediaprovider/helpers/iterators.go index 03ddc1f..a7e9778 100644 --- a/backend/mediaprovider/helpers/iterators.go +++ b/backend/mediaprovider/helpers/iterators.go @@ -195,6 +195,8 @@ func (n nilFilter[M]) IsNil() bool { return true } func (n nilFilter[M]) Matches(*M) bool { return true } +func (n nilFilter[M]) Clone() mediaprovider.MediaFilter[M, nilFilterOptions] { return n } + func (n nilFilter[M]) Options() nilFilterOptions { return nilFilterOptions{} } -func (n nilFilter[M]) SetOptions(o nilFilterOptions) {} +func (n nilFilter[M]) SetOptions(options nilFilterOptions) {} diff --git a/backend/mediaprovider/jellyfin/iterators.go b/backend/mediaprovider/jellyfin/iterators.go index 6a630d5..c6c58a9 100644 --- a/backend/mediaprovider/jellyfin/iterators.go +++ b/backend/mediaprovider/jellyfin/iterators.go @@ -58,7 +58,7 @@ func (j *jellyfinMediaProvider) IterateAlbums(sortOrder string, filter mediaprov jfSort.Field = jellyfin.SortByYear jfSort.Mode = jellyfin.SortDesc } - jfFilt := jfFilterFromFilter(filter) + jfFilt, modifiedFilter := jfFilterFromFilter(filter) fetcher := func(offs, limit int) ([]*mediaprovider.Album, error) { al, err := j.client.GetAlbums(jellyfin.QueryOpts{ @@ -84,9 +84,9 @@ func (j *jellyfinMediaProvider) IterateAlbums(sortOrder string, filter mediaprov } return sharedutil.MapSlice(al, toAlbum), nil } - return helpers.NewRandomAlbumIter(determFetcher, fetcher, filter, j.prefetchCoverCB) + return helpers.NewRandomAlbumIter(determFetcher, fetcher, modifiedFilter, j.prefetchCoverCB) } - return helpers.NewAlbumIterator(fetcher, filter, j.prefetchCoverCB) + return helpers.NewAlbumIterator(fetcher, modifiedFilter, j.prefetchCoverCB) } func (j *jellyfinMediaProvider) SearchAlbums(searchQuery string, filter mediaprovider.AlbumFilter) mediaprovider.AlbumIterator { @@ -151,15 +151,20 @@ func (j *jellyfinMediaProvider) IterateArtists(sortOrder string) mediaprovider.A } // Creates the Jellyfin filter to implement the given mediaprovider filter, -// and zeros out the now-unneeded fields in the mediaprovider filter. -func jfFilterFromFilter(filter mediaprovider.AlbumFilter) jellyfin.Filter { +// and returns a modified mediaprovider filter, with now-unneeded fields zeroed out. +func jfFilterFromFilter(filter mediaprovider.AlbumFilter) (jellyfin.Filter, mediaprovider.AlbumFilter) { var jfFilt jellyfin.Filter - filterOptions := filter.Options() + // Clone the original filter to not modify its options. + // Set filters must be maintained in the original filter, as they are used for the UI. + // Modified filter options are used to ignore further filtering that was already handled by the + // Jellyfin API. + modifiedFilter := filter.Clone() + filterOptions := modifiedFilter.Options() if filterOptions.ExcludeUnfavorited { jfFilt.Favorite = true - filterOptions.ExcludeUnfavorited = false // Jellyfin will handle this filter + filterOptions.ExcludeUnfavorited = false } if filterOptions.MinYear > 0 && filterOptions.MaxYear > 0 { jfFilt.YearRange = [2]int{filterOptions.MinYear, filterOptions.MaxYear} @@ -174,7 +179,6 @@ func jfFilterFromFilter(filter mediaprovider.AlbumFilter) jellyfin.Filter { jfFilt.Genres = filterOptions.Genres filterOptions.Genres = nil - filter.SetOptions(filterOptions) - - return jfFilt + modifiedFilter.SetOptions(filterOptions) + return jfFilt, modifiedFilter } diff --git a/backend/mediaprovider/mediaprovider.go b/backend/mediaprovider/mediaprovider.go index 5627956..2bdcccb 100644 --- a/backend/mediaprovider/mediaprovider.go +++ b/backend/mediaprovider/mediaprovider.go @@ -18,6 +18,7 @@ type TrackIterator = MediaIterator[Track] type MediaFilter[M, F any] interface { Options() F SetOptions(F) + Clone() MediaFilter[M, F] IsNil() bool Matches(*M) bool } @@ -33,6 +34,19 @@ type AlbumFilterOptions struct { ExcludeUnfavorited bool // mut. exc. with ExcludeFavorited } +// Clone returns a deep copy of the filter options +func (o AlbumFilterOptions) Clone() AlbumFilterOptions { + genres := make([]string, len(o.Genres)) + copy(genres, o.Genres) + return AlbumFilterOptions{ + MinYear: o.MinYear, + MaxYear: o.MaxYear, + Genres: genres, + ExcludeFavorited: o.ExcludeFavorited, + ExcludeUnfavorited: o.ExcludeUnfavorited, + } +} + type albumFilter struct { options AlbumFilterOptions } @@ -45,8 +59,13 @@ func (a albumFilter) Options() AlbumFilterOptions { return a.options } -func (a *albumFilter) SetOptions(o AlbumFilterOptions) { - a.options = o +func (a *albumFilter) SetOptions(options AlbumFilterOptions) { + a.options = options +} + +// Clone returns a deep copy of the filter +func (a albumFilter) Clone() AlbumFilter { + return NewAlbumFilter(a.options.Clone()) } // Returns true if the filter is the nil filter - i.e. matches everything diff --git a/backend/mediaprovider/subsonic/albumiterator.go b/backend/mediaprovider/subsonic/albumiterator.go index b16b731..5637460 100644 --- a/backend/mediaprovider/subsonic/albumiterator.go +++ b/backend/mediaprovider/subsonic/albumiterator.go @@ -68,18 +68,22 @@ func (s *subsonicMediaProvider) IterateAlbums(sortOrder string, filter mediaprov // but servers do internally match against all the genres the album is categorized with. // So we must not additionally filter by genre to avoid excluding results where // the single genre returned by Subsonic isn't the one we're iterating on. - filterOptions.Genres = nil + modifiedFilter := filter.Clone() + modifiedOptions := modifiedFilter.Options() + modifiedOptions.Genres = nil + modifiedFilter.SetOptions(modifiedOptions) fetchFn := func(offset, limit int) ([]*subsonic.AlbumID3, error) { return s.client.GetAlbumList2("byGenre", map[string]string{"genre": genre, "offset": strconv.Itoa(offset), "limit": strconv.Itoa(limit)}) } - filter.SetOptions(filterOptions) - return helpers.NewAlbumIterator(makeFetchFn(fetchFn), filter, s.prefetchCoverCB) + return helpers.NewAlbumIterator(makeFetchFn(fetchFn), modifiedFilter, s.prefetchCoverCB) } if sortOrder == "" && filterOptions.ExcludeUnfavorited { - filterOptions.ExcludeUnfavorited = false // we're already filtering by this - filter.SetOptions(filterOptions) - return s.baseIterFromSimpleSortOrder("starred", filter) + modifiedFilter := filter.Clone() + modifiedOptions := modifiedFilter.Options() + modifiedOptions.ExcludeUnfavorited = false // we're already filtering by this + modifiedFilter.SetOptions(modifiedOptions) + return s.baseIterFromSimpleSortOrder("starred", modifiedFilter) } if sortOrder == "" { sortOrder = AlbumSortRecentlyAdded // default