From 6d0640064c081b56fa38a55f02a2b8ad90b54e60 Mon Sep 17 00:00:00 2001 From: Dennis Daume Date: Sat, 12 Mar 2016 16:12:07 +0100 Subject: [PATCH 1/6] Initial hack on migrating the local song filtering to Dynamic Data --- Espera.Core/Song.cs | 13 +++-- Espera.View/Espera.View.csproj | 12 +++++ Espera.View/SearchEngine.cs | 65 +++++++++++----------- Espera.View/ViewModels/ArtistViewModel.cs | 66 +++++++++++------------ Espera.View/ViewModels/LocalViewModel.cs | 61 +++++++++++++++++++-- Espera.View/packages.config | 2 + 6 files changed, 146 insertions(+), 73 deletions(-) diff --git a/Espera.Core/Song.cs b/Espera.Core/Song.cs index 1bb87023..aad6c42b 100644 --- a/Espera.Core/Song.cs +++ b/Espera.Core/Song.cs @@ -1,10 +1,10 @@ -using Espera.Network; -using Rareform.Validation; -using System; +using System; using System.ComponentModel; using System.Diagnostics; using System.Runtime.CompilerServices; using System.Threading.Tasks; +using Espera.Network; +using Rareform.Validation; namespace Espera.Core { @@ -14,7 +14,7 @@ public abstract class Song : IEquatable, INotifyPropertyChanged private bool isCorrupted; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The path of the song. /// The duration of the song. @@ -45,7 +45,9 @@ protected Song(string path, TimeSpan duration) public string Genre { get; set; } /// - /// A runtime identifier for interaction with the mobile API. + /// A runtime identifier that uniquely identifies this songs. + /// + /// This identifier changes at each startup, but is stable in a running instance of the application. /// public Guid Guid { get; private set; } @@ -56,6 +58,7 @@ protected Song(string path, TimeSpan duration) public bool IsCorrupted { get { return this.isCorrupted; } + set { if (this.isCorrupted != value) diff --git a/Espera.View/Espera.View.csproj b/Espera.View/Espera.View.csproj index 1352ac76..1a899490 100644 --- a/Espera.View/Espera.View.csproj +++ b/Espera.View/Espera.View.csproj @@ -74,6 +74,18 @@ ..\packages\DeltaCompressionDotNet.1.0.0\lib\net45\DeltaCompressionDotNet.PatchApi.dll + + ..\packages\DynamicData.4.3.1.1090\lib\net45\DynamicData.dll + True + + + ..\packages\DynamicData.4.3.1.1090\lib\net45\DynamicData.Plinq.dll + True + + + ..\packages\DynamicData.ReactiveUI.2.2.0.2007\lib\portable-net45+win+wpa81+wp80\DynamicData.ReactiveUI.dll + True + False ..\packages\Espera-Network.1.0.36\lib\portable-net45+monoandroid+wpa81\Espera.Network.dll diff --git a/Espera.View/SearchEngine.cs b/Espera.View/SearchEngine.cs index fe521454..8877f0c0 100644 --- a/Espera.View/SearchEngine.cs +++ b/Espera.View/SearchEngine.cs @@ -1,47 +1,48 @@ -using Espera.Core; -using Rareform.Validation; -using System; -using System.Collections.Generic; +using System; using System.Linq; +using Espera.Core; namespace Espera.View { - public static class SearchEngine + public static class StringExtensions { - /// - /// Filters the source by the specified search text. - /// - /// The songs to search. - /// The search text. - /// The filtered sequence of songs. - public static IEnumerable FilterSongs(this IEnumerable source, string searchText) + public static bool ContainsIgnoreCase(this string value, string other) { - if (searchText == null) - Throw.ArgumentNullException(() => searchText); + return value.IndexOf(other, StringComparison.InvariantCultureIgnoreCase) >= 0; + } + } - if (String.IsNullOrWhiteSpace(searchText)) - return source; + public class SearchEngine + { + private readonly string[] keywords; + private readonly bool passThrough; - IEnumerable keyWords = searchText.Split(' '); + public SearchEngine(string searchText) + { + if (String.IsNullOrWhiteSpace(searchText)) + { + this.passThrough = true; + return; + } - return source - .AsParallel() - .Where - ( - song => keyWords.All - ( - keyword => - song.Artist.ContainsIgnoreCase(keyword) || - song.Album.ContainsIgnoreCase(keyword) || - song.Genre.ContainsIgnoreCase(keyword) || - song.Title.ContainsIgnoreCase(keyword) - ) - ); + this.keywords = searchText.Split(' '); } - private static bool ContainsIgnoreCase(this string value, string other) + public bool Filter(Song song) { - return value.IndexOf(other, StringComparison.InvariantCultureIgnoreCase) >= 0; + if (this.passThrough) + { + return true; + } + + return this.keywords.All + ( + keyword => + song.Artist.ContainsIgnoreCase(keyword) || + song.Album.ContainsIgnoreCase(keyword) || + song.Genre.ContainsIgnoreCase(keyword) || + song.Title.ContainsIgnoreCase(keyword) + ); } } } \ No newline at end of file diff --git a/Espera.View/ViewModels/ArtistViewModel.cs b/Espera.View/ViewModels/ArtistViewModel.cs index 673305dc..6e03627c 100644 --- a/Espera.View/ViewModels/ArtistViewModel.cs +++ b/Espera.View/ViewModels/ArtistViewModel.cs @@ -1,17 +1,16 @@ -using Espera.Core; -using ReactiveUI; -using Splat; -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Reactive.Linq; -using System.Reactive.Subjects; using System.Threading.Tasks; using System.Windows.Media.Imaging; +using Espera.Core; +using ReactiveUI; +using Splat; namespace Espera.View.ViewModels { - public sealed class ArtistViewModel : ReactiveObject, IComparable, IEquatable, IDisposable + public sealed class ArtistViewModel : ReactiveObject, IEquatable, IDisposable { private readonly ObservableAsPropertyHelper cover; private readonly int orderHint; @@ -21,23 +20,21 @@ public sealed class ArtistViewModel : ReactiveObject, IComparable /// - /// + /// /// /// A hint that tells this instance which position it has in the artist list. This helps for /// priorizing the album cover loading. The higher the number, the earlier it is in the list /// (Think of a reversed sorted list). /// - public ArtistViewModel(string artistName, IEnumerable songs, int orderHint = 1) + public ArtistViewModel(string artistName, IObservable artworkKeys, int orderHint = 1) { this.songs = new ReactiveList(); this.orderHint = orderHint; - this.cover = this.songs.ItemsAdded.Select(x => x.WhenAnyValue(y => y.ArtworkKey)) - .Merge() - .Where(x => x != null) + artworkKeys .Distinct() // Ignore duplicate artworks - .Select(LoadArtworkAsync) + .Select(key => Observable.FromAsync(() => this.LoadArtworkAsync(key))) .Concat() .FirstOrDefaultAsync(pic => pic != null) .ToProperty(this, x => x.Cover); @@ -64,29 +61,9 @@ public BitmapSource Cover public string Name { get; private set; } - public int CompareTo(ArtistViewModel other) - { - if (this.IsAllArtists && other.IsAllArtists) - { - return 0; - } - - if (this.IsAllArtists) - { - return -1; - } - - if (other.IsAllArtists) - { - return 1; - } - - return String.Compare(SortHelpers.RemoveArtistPrefixes(this.Name), SortHelpers.RemoveArtistPrefixes(other.Name), StringComparison.InvariantCultureIgnoreCase); - } - public void Dispose() { - this.cover.Dispose(); + this.cover?.Dispose(); } public bool Equals(ArtistViewModel other) @@ -136,5 +113,28 @@ private async Task LoadArtworkAsync(string key) return null; } } + + public class Comparer : IComparer + { + public int Compare(ArtistViewModel x, ArtistViewModel y) + { + if (x.IsAllArtists && y.IsAllArtists) + { + return 0; + } + + if (x.IsAllArtists) + { + return -1; + } + + if (y.IsAllArtists) + { + return 1; + } + + return String.Compare(SortHelpers.RemoveArtistPrefixes(x.Name), SortHelpers.RemoveArtistPrefixes(y.Name), StringComparison.InvariantCultureIgnoreCase); + } + } } } \ No newline at end of file diff --git a/Espera.View/ViewModels/LocalViewModel.cs b/Espera.View/ViewModels/LocalViewModel.cs index c8c5174b..626cb9c1 100644 --- a/Espera.View/ViewModels/LocalViewModel.cs +++ b/Espera.View/ViewModels/LocalViewModel.cs @@ -4,10 +4,14 @@ using System.Reactive; using System.Reactive.Linq; using System.Reactive.Subjects; +using DynamicData; +using DynamicData.Controllers; +using DynamicData.ReactiveUI; using Espera.Core; using Espera.Core.Management; using Espera.Core.Settings; using Rareform.Validation; +using ReactiveMarrow; using ReactiveUI; namespace Espera.View.ViewModels @@ -17,12 +21,13 @@ public class LocalViewModel : SongSourceViewModel private readonly ReactiveList allArtists; private readonly ArtistViewModel allArtistsViewModel; private readonly SortOrder artistOrder; + private readonly ReactiveList artists; private readonly Subject artistUpdateSignal; private readonly ObservableAsPropertyHelper isUpdating; private readonly ReactiveCommand playNowCommand; private readonly ObservableAsPropertyHelper showAddSongsHelperMessage; + private readonly ReactiveList songs; private readonly ViewSettings viewSettings; - private ILookup filteredSongs; private ArtistViewModel selectedArtist; public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings coreSettings, Guid accessToken) @@ -38,14 +43,61 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c this.allArtistsViewModel = new ArtistViewModel("All Artists"); this.allArtists = new ReactiveList { this.allArtistsViewModel }; + /* this.Artists = this.allArtists.CreateDerivedCollection(x => x, x => x.IsAllArtists || this.filteredSongs.Contains(x.Name), (x, y) => x.CompareTo(y), this.artistUpdateSignal); + */ // We need a default sorting order this.ApplyOrder(SortHelpers.GetOrderByArtist, ref this.artistOrder); this.SelectedArtist = this.allArtistsViewModel; + this.songs = new ReactiveList(); + this.SelectableSongs = this.songs; + this.artists = new ReactiveList(); + var source = new SourceCache(x => x.Guid); + + IObservable> searchEngine = this.WhenAnyValue(x => x.SearchText) + .Select(searchText => new SearchEngine(searchText)) + .Select(engine => new Func(song => engine.Filter(song))); + + IObservable> artistFilter = this.WhenAnyValue(x => x.SelectedArtist) + .Select(artist => new Func(song => artist.IsAllArtists || song.Artist.Equals(artist.Name, StringComparison.InvariantCultureIgnoreCase))); + + var filteredSource = source.Connect() + .Filter(searchEngine) + .Publish() + .RefCount(); + + filteredSource + .Filter(artistFilter) + .Transform(x => new LocalSongViewModel(x)) + .Bind(this.songs) + .DisposeMany() + .Subscribe(); + + filteredSource + .Group(x => x.Artist) + .Transform(x => new ArtistViewModel(x.Key, Observable.Never())) + .Sort(new ArtistViewModel.Comparer()) + .Bind(this.artists) + .DisposeMany() + .Subscribe(); + + this.Library.SongsUpdated + .Buffer(TimeSpan.FromSeconds(1), RxApp.TaskpoolScheduler) + .Where(x => x.Any()) + .ToUnit() + .StartWith(Unit.Default) + .Select(_ => this.Library.Songs) + .Subscribe(x => source.Edit(update => + { + update.Clear(); + update.AddOrUpdate(x); + })); + + /* var gate = new object(); this.Library.SongsUpdated .Buffer(TimeSpan.FromSeconds(1), RxApp.TaskpoolScheduler) @@ -65,6 +117,7 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c .Skip(1) .Synchronize(gate) .Subscribe(_ => this.UpdateSelectableSongs()); + */ this.playNowCommand = ReactiveCommand.CreateAsyncTask(this.Library.LocalAccessControl.ObserveAccessPermission(accessToken) .Select(x => x == AccessPermission.Admin || !coreSettings.LockPlayPause), _ => @@ -87,7 +140,7 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c this.OpenTagEditor = ReactiveCommand.Create(this.WhenAnyValue(x => x.SelectedSongs, x => x.Any())); } - public IReactiveDerivedList Artists { get; private set; } + public IReadOnlyReactiveList Artists => this.artists; public override DefaultPlaybackAction DefaultPlaybackAction { @@ -121,6 +174,7 @@ public override ReactiveCommand PlayNowCommand public ArtistViewModel SelectedArtist { get { return this.selectedArtist; } + set { // We don't ever want the selected artist to be null @@ -139,6 +193,7 @@ public int TitleColumnWidth set { this.viewSettings.LocalTitleColumnWidth = value; } } + /* private void UpdateArtists() { var groupedByArtist = this.Library.Songs @@ -224,6 +279,6 @@ private void UpdateSelectableSongs() { this.SelectedSongs = this.SelectableSongs.Take(1).ToList(); } - } + }*/ } } \ No newline at end of file diff --git a/Espera.View/packages.config b/Espera.View/packages.config index ecf1c242..89b32f26 100644 --- a/Espera.View/packages.config +++ b/Espera.View/packages.config @@ -6,6 +6,8 @@ + + From aa15befa02644f70531849118553d99791612b88 Mon Sep 17 00:00:00 2001 From: Dennis Daume Date: Sun, 13 Mar 2016 19:28:51 +0100 Subject: [PATCH 2/6] More work on the transition to Dynamic Data --- Espera.View/ViewModels/ArtistViewModel.cs | 66 +++++++++++- Espera.View/ViewModels/LocalViewModel.cs | 123 ++-------------------- 2 files changed, 71 insertions(+), 118 deletions(-) diff --git a/Espera.View/ViewModels/ArtistViewModel.cs b/Espera.View/ViewModels/ArtistViewModel.cs index 6e03627c..672447a8 100644 --- a/Espera.View/ViewModels/ArtistViewModel.cs +++ b/Espera.View/ViewModels/ArtistViewModel.cs @@ -68,7 +68,32 @@ public void Dispose() public bool Equals(ArtistViewModel other) { - return this.Name == other.Name; + if (Object.ReferenceEquals(other, null)) + { + return false; + } + + if (this.IsAllArtists && other.IsAllArtists) + { + return true; + } + + if (this.IsAllArtists || other.IsAllArtists) + { + return false; + } + + return this.Name.Equals(other.Name, StringComparison.InvariantCultureIgnoreCase); + } + + public override bool Equals(object obj) + { + return base.Equals(obj as ArtistViewModel); + } + + public override int GetHashCode() + { + return new { A = this.IsAllArtists, B = this.Name }.GetHashCode(); } public void UpdateSongs(IEnumerable songs) @@ -114,6 +139,45 @@ private async Task LoadArtworkAsync(string key) } } + /// + /// A custom equality class for the artist grouping, until + /// https://github.com/RolandPheasant/DynamicData/issues/31 is resolved + /// + public class ArtistString : IEquatable + { + private readonly string artistName; + + public ArtistString(string artistName) + { + this.artistName = artistName; + } + + public static implicit operator ArtistString(string source) + { + return new ArtistString(source); + } + + public static implicit operator string(ArtistString source) + { + return source.artistName; + } + + public bool Equals(ArtistString other) + { + return StringComparer.InvariantCultureIgnoreCase.Equals(this.artistName, other.artistName); + } + + public override bool Equals(object obj) + { + return base.Equals(obj as ArtistString); + } + + public override int GetHashCode() + { + return StringComparer.InvariantCultureIgnoreCase.GetHashCode(this.artistName); + } + } + public class Comparer : IComparer { public int Compare(ArtistViewModel x, ArtistViewModel y) diff --git a/Espera.View/ViewModels/LocalViewModel.cs b/Espera.View/ViewModels/LocalViewModel.cs index 626cb9c1..6f6f2af1 100644 --- a/Espera.View/ViewModels/LocalViewModel.cs +++ b/Espera.View/ViewModels/LocalViewModel.cs @@ -6,6 +6,7 @@ using System.Reactive.Subjects; using DynamicData; using DynamicData.Controllers; +using DynamicData.Operators; using DynamicData.ReactiveUI; using Espera.Core; using Espera.Core.Management; @@ -43,11 +44,6 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c this.allArtistsViewModel = new ArtistViewModel("All Artists"); this.allArtists = new ReactiveList { this.allArtistsViewModel }; - /* - this.Artists = this.allArtists.CreateDerivedCollection(x => x, - x => x.IsAllArtists || this.filteredSongs.Contains(x.Name), (x, y) => x.CompareTo(y), this.artistUpdateSignal); - */ - // We need a default sorting order this.ApplyOrder(SortHelpers.GetOrderByArtist, ref this.artistOrder); @@ -73,14 +69,17 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c filteredSource .Filter(artistFilter) .Transform(x => new LocalSongViewModel(x)) + .ObserveOn(RxApp.MainThreadScheduler) .Bind(this.songs) .DisposeMany() .Subscribe(); filteredSource - .Group(x => x.Artist) + .Group(x => (ArtistViewModel.ArtistString) x.Artist) .Transform(x => new ArtistViewModel(x.Key, Observable.Never())) - .Sort(new ArtistViewModel.Comparer()) + .Or(Observable.Return(new ChangeSet(ChangeReason.Add, Guid.NewGuid().ToString(), this.allArtistsViewModel))) + .Sort(new ArtistViewModel.Comparer(), SortOptimisations.ComparesImmutableValuesOnly) + .ObserveOn(RxApp.MainThreadScheduler) .Bind(this.artists) .DisposeMany() .Subscribe(); @@ -97,28 +96,6 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c update.AddOrUpdate(x); })); - /* - var gate = new object(); - this.Library.SongsUpdated - .Buffer(TimeSpan.FromSeconds(1), RxApp.TaskpoolScheduler) - .Where(x => x.Any()) - .Select(_ => Unit.Default) - .Merge(this.WhenAny(x => x.SearchText, _ => Unit.Default) - .Do(_ => this.SelectedArtist = this.allArtistsViewModel)) - .Synchronize(gate) - .ObserveOn(RxApp.MainThreadScheduler) - .Subscribe(_ => - { - this.UpdateSelectableSongs(); - this.UpdateArtists(); - }); - - this.WhenAnyValue(x => x.SelectedArtist) - .Skip(1) - .Synchronize(gate) - .Subscribe(_ => this.UpdateSelectableSongs()); - */ - this.playNowCommand = ReactiveCommand.CreateAsyncTask(this.Library.LocalAccessControl.ObserveAccessPermission(accessToken) .Select(x => x == AccessPermission.Admin || !coreSettings.LockPlayPause), _ => { @@ -192,93 +169,5 @@ public int TitleColumnWidth get { return this.viewSettings.LocalTitleColumnWidth; } set { this.viewSettings.LocalTitleColumnWidth = value; } } - - /* - private void UpdateArtists() - { - var groupedByArtist = this.Library.Songs - .ToLookup(x => x.Artist, StringComparer.InvariantCultureIgnoreCase); - - List artistsToRemove = this.allArtists.Where(x => !groupedByArtist.Contains(x.Name)).ToList(); - artistsToRemove.Remove(this.allArtistsViewModel); - - this.allArtists.RemoveAll(artistsToRemove); - - foreach (ArtistViewModel artistViewModel in artistsToRemove) - { - artistViewModel.Dispose(); - } - - // We use this reverse ordered list of artists so we can priorize the loading of album - // covers of artists that we display first in the artist list. This way we can "fake" a - // fast loading of all covers, as the user doesn't see most of the artists down the - // list. The higher the number, the higher the prioritization. - List orderedArtists = groupedByArtist.Select(x => x.Key) - .OrderByDescending(SortHelpers.RemoveArtistPrefixes) - .ToList(); - - foreach (var songs in groupedByArtist) - { - ArtistViewModel model = this.allArtists.FirstOrDefault(x => x.Name.Equals(songs.Key, StringComparison.InvariantCultureIgnoreCase)); - - if (model == null) - { - int priority = orderedArtists.IndexOf(songs.Key) + 1; - this.allArtists.Add(new ArtistViewModel(songs.Key, songs, priority)); - } - - else - { - model.UpdateSongs(songs); - } - } - } - - private void UpdateSelectableSongs() - { - this.filteredSongs = this.Library.Songs.FilterSongs(this.SearchText) - .ToLookup(x => x.Artist, StringComparer.InvariantCultureIgnoreCase); - - var newArtists = new HashSet(this.filteredSongs.Select(x => x.Key)); - var oldArtists = this.Artists.Where(x => !x.IsAllArtists).Select(x => x.Name); - - if (!newArtists.SetEquals(oldArtists)) - { - this.artistUpdateSignal.OnNext(Unit.Default); - } - - List selectableSongs = this.filteredSongs - .Where(group => this.SelectedArtist.IsAllArtists || @group.Key.Equals(this.SelectedArtist.Name, StringComparison.InvariantCultureIgnoreCase)) - .SelectMany(x => x) - .Select(song => new LocalSongViewModel(song)) - .OrderBy(this.SongOrderFunc) - .ToList(); - - // Ignore redundant song updates. - if (!selectableSongs.SequenceEqual(this.SelectableSongs)) - { - // Scratch the old viewmodels - foreach (var viewModel in this.SelectableSongs) - { - viewModel.Dispose(); - } - - this.SelectableSongs = selectableSongs; - } - - else - { - // We don't have to update the selectable songs, get rid of the redundant ones we've created - foreach (LocalSongViewModel viewModel in selectableSongs) - { - viewModel.Dispose(); - } - } - - if (this.SelectedSongs == null) - { - this.SelectedSongs = this.SelectableSongs.Take(1).ToList(); - } - }*/ } } \ No newline at end of file From afee20332ac7c62151c1e36a7f92bede37d92065 Mon Sep 17 00:00:00 2001 From: Dennis Daume Date: Mon, 14 Mar 2016 01:25:18 +0100 Subject: [PATCH 3/6] Fix an equality check --- Espera.View/ViewModels/ArtistViewModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Espera.View/ViewModels/ArtistViewModel.cs b/Espera.View/ViewModels/ArtistViewModel.cs index 672447a8..5ca176c5 100644 --- a/Espera.View/ViewModels/ArtistViewModel.cs +++ b/Espera.View/ViewModels/ArtistViewModel.cs @@ -169,7 +169,7 @@ public bool Equals(ArtistString other) public override bool Equals(object obj) { - return base.Equals(obj as ArtistString); + return this.Equals(obj as ArtistString); } public override int GetHashCode() From fe36d885b978a1a7a6501f52b20918ead784569a Mon Sep 17 00:00:00 2001 From: Dennis Daume Date: Sun, 27 Mar 2016 20:21:40 +0200 Subject: [PATCH 4/6] Tune the filtering performance --- Espera.View/ViewModels/ArtistViewModel.cs | 20 ++--------- Espera.View/ViewModels/LocalViewModel.cs | 42 +++++++++++++++-------- 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/Espera.View/ViewModels/ArtistViewModel.cs b/Espera.View/ViewModels/ArtistViewModel.cs index 5ca176c5..bdb617e5 100644 --- a/Espera.View/ViewModels/ArtistViewModel.cs +++ b/Espera.View/ViewModels/ArtistViewModel.cs @@ -14,7 +14,6 @@ public sealed class ArtistViewModel : ReactiveObject, IEquatable cover; private readonly int orderHint; - private readonly ReactiveList songs; /// /// The constructor. @@ -28,11 +27,10 @@ public sealed class ArtistViewModel : ReactiveObject, IEquatable public ArtistViewModel(string artistName, IObservable artworkKeys, int orderHint = 1) { - this.songs = new ReactiveList(); - this.orderHint = orderHint; - artworkKeys + this.cover = artworkKeys + .Where(x => x != null) .Distinct() // Ignore duplicate artworks .Select(key => Observable.FromAsync(() => this.LoadArtworkAsync(key))) .Concat() @@ -40,8 +38,6 @@ public ArtistViewModel(string artistName, IObservable artworkKeys, int o .ToProperty(this, x => x.Cover); var connect = this.Cover; // Connect the property to the source observable immediately - this.UpdateSongs(songs); - this.Name = artistName; this.IsAllArtists = false; } @@ -96,18 +92,6 @@ public override int GetHashCode() return new { A = this.IsAllArtists, B = this.Name }.GetHashCode(); } - public void UpdateSongs(IEnumerable songs) - { - var songsToAdd = songs.Where(x => !this.songs.Contains(x)).ToList(); - - // Can't use AddRange here, ReactiveList resets the list on big changes and we don't get - // the add notification - foreach (LocalSong song in songsToAdd) - { - this.songs.Add(song); - } - } - private async Task LoadArtworkAsync(string key) { try diff --git a/Espera.View/ViewModels/LocalViewModel.cs b/Espera.View/ViewModels/LocalViewModel.cs index 6f6f2af1..fbc380cc 100644 --- a/Espera.View/ViewModels/LocalViewModel.cs +++ b/Espera.View/ViewModels/LocalViewModel.cs @@ -19,7 +19,6 @@ namespace Espera.View.ViewModels { public class LocalViewModel : SongSourceViewModel { - private readonly ReactiveList allArtists; private readonly ArtistViewModel allArtistsViewModel; private readonly SortOrder artistOrder; private readonly ReactiveList artists; @@ -42,7 +41,6 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c this.artistUpdateSignal = new Subject(); this.allArtistsViewModel = new ArtistViewModel("All Artists"); - this.allArtists = new ReactiveList { this.allArtistsViewModel }; // We need a default sorting order this.ApplyOrder(SortHelpers.GetOrderByArtist, ref this.artistOrder); @@ -52,32 +50,48 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c this.songs = new ReactiveList(); this.SelectableSongs = this.songs; this.artists = new ReactiveList(); - var source = new SourceCache(x => x.Guid); + var songSource = new SourceCache(x => x.Guid); - IObservable> searchEngine = this.WhenAnyValue(x => x.SearchText) + IObservableCache songsCache = songSource.Connect() + .Transform(x => new LocalSongViewModel(x)) + .DisposeMany() + .AsObservableCache(); + + IObservableCache artistsCache = songSource.Connect() + .Group(x => (ArtistViewModel.ArtistString)x.Artist) + .Transform(x => new ArtistViewModel(x.Key, x.Cache.Connect().WhereReasonsAre(ChangeReason.Add).Flatten().Select(y => y.Current.ArtworkKey))) + .DisposeMany() + .AsObservableCache(); + + IObservable> searchEngine = this.WhenAnyValue(x => x.SearchText) .Select(searchText => new SearchEngine(searchText)) - .Select(engine => new Func(song => engine.Filter(song))); + .Select(engine => new Func(song => engine.Filter(song.Model))); - IObservable> artistFilter = this.WhenAnyValue(x => x.SelectedArtist) - .Select(artist => new Func(song => artist.IsAllArtists || song.Artist.Equals(artist.Name, StringComparison.InvariantCultureIgnoreCase))); + IObservable> artistFilter = this.WhenAnyValue(x => x.SelectedArtist) + .Select(artist => new Func(song => artist.IsAllArtists || song.Artist.Equals(artist.Name, StringComparison.InvariantCultureIgnoreCase))); - var filteredSource = source.Connect() + var filteredSource = songsCache.Connect() .Filter(searchEngine) .Publish() .RefCount(); filteredSource .Filter(artistFilter) - .Transform(x => new LocalSongViewModel(x)) .ObserveOn(RxApp.MainThreadScheduler) .Bind(this.songs) .DisposeMany() .Subscribe(); - filteredSource - .Group(x => (ArtistViewModel.ArtistString) x.Artist) - .Transform(x => new ArtistViewModel(x.Key, Observable.Never())) - .Or(Observable.Return(new ChangeSet(ChangeReason.Add, Guid.NewGuid().ToString(), this.allArtistsViewModel))) + var filteredArtistGrouping = filteredSource + .Group(x => x.Artist) + .Convert(x => x.Key) + .ToCollection() + .Select(x => new HashSet(x, StringComparer.InvariantCultureIgnoreCase)) + .Select(artists => new Func(artistViewModel => artists.Contains(artistViewModel.Name))); + + artistsCache.Connect() + .Filter(filteredArtistGrouping) + .StartWithItem(this.allArtistsViewModel, Guid.NewGuid().ToString()) .Sort(new ArtistViewModel.Comparer(), SortOptimisations.ComparesImmutableValuesOnly) .ObserveOn(RxApp.MainThreadScheduler) .Bind(this.artists) @@ -90,7 +104,7 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c .ToUnit() .StartWith(Unit.Default) .Select(_ => this.Library.Songs) - .Subscribe(x => source.Edit(update => + .Subscribe(x => songSource.Edit(update => { update.Clear(); update.AddOrUpdate(x); From 13e27d18f0af0b7e901dd9cbac6646736b1e828a Mon Sep 17 00:00:00 2001 From: Dennis Daume Date: Sun, 27 Mar 2016 21:11:15 +0200 Subject: [PATCH 5/6] Apply sorting to the song list --- Espera.View/ViewModels/LocalViewModel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Espera.View/ViewModels/LocalViewModel.cs b/Espera.View/ViewModels/LocalViewModel.cs index fbc380cc..a59739c5 100644 --- a/Espera.View/ViewModels/LocalViewModel.cs +++ b/Espera.View/ViewModels/LocalViewModel.cs @@ -5,6 +5,7 @@ using System.Reactive.Linq; using System.Reactive.Subjects; using DynamicData; +using DynamicData.Binding; using DynamicData.Controllers; using DynamicData.Operators; using DynamicData.ReactiveUI; @@ -77,6 +78,7 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c filteredSource .Filter(artistFilter) + .Sort(SortExpressionComparer.Ascending(x => SortHelpers.RemoveArtistPrefixes(x.Artist)).ThenByAscending(x => x.Album).ThenByAscending(x => x.TrackNumber), SortOptimisations.ComparesImmutableValuesOnly) .ObserveOn(RxApp.MainThreadScheduler) .Bind(this.songs) .DisposeMany() From 66ff6c48eaaf64d5527d9481372661d6e99fbf9f Mon Sep 17 00:00:00 2001 From: Dennis Daume Date: Sun, 27 Mar 2016 21:41:50 +0200 Subject: [PATCH 6/6] Remove a sort optimisation, since the artist value can change when the user edits the metadata --- Espera.View/ViewModels/LocalViewModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Espera.View/ViewModels/LocalViewModel.cs b/Espera.View/ViewModels/LocalViewModel.cs index a59739c5..19c1ac62 100644 --- a/Espera.View/ViewModels/LocalViewModel.cs +++ b/Espera.View/ViewModels/LocalViewModel.cs @@ -78,7 +78,7 @@ public LocalViewModel(Library library, ViewSettings viewSettings, CoreSettings c filteredSource .Filter(artistFilter) - .Sort(SortExpressionComparer.Ascending(x => SortHelpers.RemoveArtistPrefixes(x.Artist)).ThenByAscending(x => x.Album).ThenByAscending(x => x.TrackNumber), SortOptimisations.ComparesImmutableValuesOnly) + .Sort(SortExpressionComparer.Ascending(x => SortHelpers.RemoveArtistPrefixes(x.Artist)).ThenByAscending(x => x.Album).ThenByAscending(x => x.TrackNumber)) .ObserveOn(RxApp.MainThreadScheduler) .Bind(this.songs) .DisposeMany()