Add RemoveFromCache and GetCachedCount methods to IScrobbler and SQLite implementation

- change scrobbler behaviour to remove sucessfully scrobbled tracks from the cache
- change return value of Scrobble methods - will always be either Successful, Cached. Actual failure reason may be stored by Scrobbler cache impl
- Remove CacheEnabled property from IScrobbler. Just use new MemoryScrobbler class if needed
- Remove Scrobbler class, add MemoryScrobbler. No migration path because the behaviour is different - and the old behaviour not that useful

src/IF.Lastfm.Core.Tests.Integration/Commands/TrackScrobbleCommandTests.cs

@@ -71,7 +71,7 @@ public async Task ScrobblesMultiple()
 
             var countingHandler = new CountingHttpClientHandler();
             var httpClient = new HttpClient(countingHandler);
-            var scrobbler = new Scrobbler(Lastfm.Auth, httpClient)
+            var scrobbler = new MemoryScrobbler(Lastfm.Auth, httpClient)
             {
                 MaxBatchSize = 2
             };

src/IF.Lastfm.Core.Tests/Scrobblers/ScrobblerTests.cs

@@ -1,6 +1,5 @@
 using IF.Lastfm.Core.Scrobblers;
 using System.Net.Http;
-using Scrobbler = IF.Lastfm.Core.Scrobblers.Scrobbler;
 
 namespace IF.Lastfm.Core.Tests.Scrobblers
 {
@@ -9,7 +8,7 @@ public class ScrobblerTests : ScrobblerTestsBase
         protected override ScrobblerBase GetScrobbler()
         {
             var httpClient = new HttpClient(FakeResponseHandler);
-            return new Scrobbler(MockAuth.Object, httpClient);
+            return new MemoryScrobbler(MockAuth.Object, httpClient);
         }
     }
 }

src/IF.Lastfm.Core.Tests/Scrobblers/ScrobblerTestsBase.cs

@@ -163,12 +163,6 @@ public async Task ScrobblesExistingCachedTracks()
 
             var scrobbleResponse1 = await ExecuteTestInternal(scrobblesToCache, responseMessage1);
             
-            if (!Scrobbler.CacheEnabled)
-            {
-                Assert.AreEqual(LastResponseStatus.BadAuth, scrobbleResponse1.Status);
-                return;
-            }
-
             Assert.AreEqual(LastResponseStatus.Cached, scrobbleResponse1.Status);
 
             var scrobblesToSend = testScrobbles.Skip(1).Take(1);
@@ -189,18 +183,11 @@ public async Task CorrectResponseWithBadAuth()
             var responseMessage = TestHelper.CreateResponseMessage(HttpStatusCode.Forbidden, TrackApiResponses.TrackScrobbleBadAuthError);
             var scrobbleResponse = await ExecuteTestInternal(testScrobbles, responseMessage, requestMessage);
 
-            if (Scrobbler.CacheEnabled)
+            Assert.AreEqual(LastResponseStatus.Cached, scrobbleResponse.Status);
-            {
-                Assert.AreEqual(LastResponseStatus.Cached, scrobbleResponse.Status);
 
-                // check actually cached
+            // check actually cached
-                var cached = await Scrobbler.GetCachedAsync();
+            var cached = await Scrobbler.GetCachedAsync();
-                TestHelper.AssertSerialiseEqual(testScrobbles, cached);
+            TestHelper.AssertSerialiseEqual(testScrobbles, cached);
-            }
-            else
-            {
-                Assert.AreEqual(LastResponseStatus.BadAuth, scrobbleResponse.Status);
-            }
         }
 
         [Test]
@@ -212,18 +199,11 @@ public async Task CorrectResponseWhenRequestFailed()
             var responseMessage = TestHelper.CreateResponseMessage(HttpStatusCode.RequestTimeout, new byte[0]);
             var scrobbleResponse = await ExecuteTestInternal(testScrobbles, responseMessage, requestMessage);
 
-            if (Scrobbler.CacheEnabled)
+            Assert.AreEqual(LastResponseStatus.Cached, scrobbleResponse.Status);
-            {
-                Assert.AreEqual(LastResponseStatus.Cached, scrobbleResponse.Status);
 
-                // check actually cached
+            // check actually cached
-                var cached = await Scrobbler.GetCachedAsync();
+            var cached = await Scrobbler.GetCachedAsync();
-                TestHelper.AssertSerialiseEqual(testScrobbles, cached);
+            TestHelper.AssertSerialiseEqual(testScrobbles, cached);
-            }
-            else
-            {
-                Assert.AreEqual(LastResponseStatus.RequestFailed, scrobbleResponse.Status);
-            }
         }
     }
 }

src/IF.Lastfm.Core/Api/LastfmClient.cs

@@ -25,7 +25,7 @@ public class LastfmClient : ApiBase
 
         public ScrobblerBase Scrobbler
         {
-            get { return _scrobbler ?? (_scrobbler = new Scrobbler(Auth, HttpClient)); }
+            get { return _scrobbler ?? (_scrobbler = new MemoryScrobbler(Auth, HttpClient)); }
             set { _scrobbler = value; }
         }
 

src/IF.Lastfm.Core/Objects/Scrobble.cs

@@ -1,11 +1,19 @@
 using System;
 using IF.Lastfm.Core.Api.Helpers;
 using Newtonsoft.Json.Linq;
+using System.Collections.Generic;
 
 namespace IF.Lastfm.Core.Objects
 {
-    public class Scrobble
+    public class Scrobble : IEquatable<Scrobble>
     {
+        /// <summary>
+        /// Not part of the Last.fm API. This is a convenience property allowing Scrobbles to have a unique ID.
+        /// IF.Lastfm.SQLite uses this field to store a primary key, if this Scrobble was cached.
+        /// Not used in Equals or GetHashCode implementations.
+        /// </summary>
+        public int Id { get; set; }
+
         public string IgnoredReason { get; private set; }
 
         public string Artist { get; private set; }
@@ -49,5 +57,37 @@ internal static Scrobble ParseJToken(JToken token)
                 IgnoredReason = ignoredMessage
             };
         }
+
+        public override bool Equals(object obj)
+        {
+            return Equals(obj as Scrobble);
+        }
+
+        public bool Equals(Scrobble other)
+        {
+            return other != null &&
+                   IgnoredReason == other.IgnoredReason &&
+                   Artist == other.Artist &&
+                   AlbumArtist == other.AlbumArtist &&
+                   Album == other.Album &&
+                   Track == other.Track &&
+                   TimePlayed.Equals(other.TimePlayed) &&
+                   ChosenByUser == other.ChosenByUser &&
+                   EqualityComparer<TimeSpan?>.Default.Equals(Duration, other.Duration);
+        }
+
+        public override int GetHashCode()
+        {
+            var hashCode = 417801827;
+            hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(IgnoredReason);
+            hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(Artist);
+            hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(AlbumArtist);
+            hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(Album);
+            hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(Track);
+            hashCode = hashCode * -1521134295 + EqualityComparer<DateTimeOffset>.Default.GetHashCode(TimePlayed);
+            hashCode = hashCode * -1521134295 + ChosenByUser.GetHashCode();
+            hashCode = hashCode * -1521134295 + EqualityComparer<TimeSpan?>.Default.GetHashCode(Duration);
+            return hashCode;
+        }
     }
 }

src/IF.Lastfm.Core/Scrobblers/IScrobbler.cs

@@ -6,8 +6,6 @@ namespace IF.Lastfm.Core.Scrobblers
 {
     public interface IScrobbler
     {
-        bool CacheEnabled { get; }
-            
         Task<IEnumerable<Scrobble>> GetCachedAsync();
         
         Task<ScrobbleResponse> ScrobbleAsync(Scrobble scrobble);

src/IF.Lastfm.Core/Scrobblers/MemoryScrobbler.cs

@@ -0,0 +1,49 @@
+using System.Collections.Generic;
+using System.Linq;
+using System.Net.Http;
+using System.Threading.Tasks;
+using IF.Lastfm.Core.Api;
+using IF.Lastfm.Core.Api.Enums;
+using IF.Lastfm.Core.Objects;
+
+namespace IF.Lastfm.Core.Scrobblers
+{
+    public class MemoryScrobbler : ScrobblerBase
+    {
+        private readonly HashSet<Scrobble> _scrobbles;
+
+        public MemoryScrobbler(ILastAuth auth, HttpClient httpClient = null) : base(auth, httpClient)
+        {
+            _scrobbles = new HashSet<Scrobble>();
+        }
+
+        public override Task<IEnumerable<Scrobble>> GetCachedAsync()
+        {
+            return Task.FromResult(_scrobbles.AsEnumerable());
+        }
+
+        public override Task RemoveFromCacheAsync(ICollection<Scrobble> scrobbles)
+        {
+            foreach (var scrobble in scrobbles)
+            {
+                _scrobbles.Remove(scrobble);
+            }
+
+            return Task.FromResult(0);
+        }
+
+        public override Task<int> GetCachedCountAsync()
+        {
+            return Task.FromResult(_scrobbles.Count);
+        }
+
+        protected override Task<LastResponseStatus> CacheAsync(IEnumerable<Scrobble> scrobbles, LastResponseStatus reason)
+        {
+            foreach (var scrobble in scrobbles)
+            {
+                _scrobbles.Add(scrobble);
+            }
+            return Task.FromResult(LastResponseStatus.Cached);
+        }
+    }
+}

src/IF.Lastfm.Core/Scrobblers/Scrobbler.cs

@@ -1,28 +0,0 @@
-using System.Collections.Generic;
-using System.Linq;
-using System.Net.Http;
-using System.Threading.Tasks;
-using IF.Lastfm.Core.Api;
-using IF.Lastfm.Core.Api.Enums;
-using IF.Lastfm.Core.Objects;
-
-namespace IF.Lastfm.Core.Scrobblers
-{
-    public class Scrobbler : ScrobblerBase
-    {
-        public Scrobbler(ILastAuth auth, HttpClient httpClient = null) : base(auth, httpClient)
-        {
-            CacheEnabled = false;
-        }
-
-        public override Task<IEnumerable<Scrobble>> GetCachedAsync()
-        {
-            return Task.FromResult(Enumerable.Empty<Scrobble>());
-        }
-
-        protected override Task<LastResponseStatus> CacheAsync(IEnumerable<Scrobble> scrobble, LastResponseStatus originalResponseStatus)
-        {
-            return Task.FromResult(originalResponseStatus);
-        }
-    }
-}

src/IF.Lastfm.Core/Scrobblers/ScrobblerBase.cs

@@ -4,6 +4,7 @@
 using IF.Lastfm.Core.Helpers;
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Linq;
 using System.Net.Http;
 using System.Threading.Tasks;
@@ -15,8 +16,6 @@ public abstract class ScrobblerBase : ApiBase, IScrobbler
     {
         public event EventHandler<ScrobbleResponse> AfterSend;
 
-        public bool CacheEnabled { get; protected set; }
-
         internal int MaxBatchSize { get; set; }
 
         protected ScrobblerBase(ILastAuth auth, HttpClient httpClient = null) : base(httpClient)
@@ -43,7 +42,7 @@ public Task<ScrobbleResponse> SendCachedScrobblesAsync()
 
         public async Task<ScrobbleResponse> ScrobbleAsyncInternal(IEnumerable<Scrobble> scrobbles)
         {
-            var scrobblesList = scrobbles.ToList();
+            var scrobblesList = new ReadOnlyCollection<Scrobble>(scrobbles.ToList());
             var cached = await GetCachedAsync();
             var pending = scrobblesList.Concat(cached).OrderBy(s => s.TimePlayed).ToList();
             if (!pending.Any())
@@ -65,6 +64,14 @@ public async Task<ScrobbleResponse> ScrobbleAsyncInternal(IEnumerable<Scrobble>
                 {
                     var response = await command.ExecuteAsync();
                     
+                    var acceptedMap = new HashSet<Scrobble>(scrobblesList);
+                    foreach (var ignored in response.Ignored)
+                    {
+                        acceptedMap.Remove(ignored);
+                    }
+                    
+                    await RemoveFromCacheAsync(acceptedMap);
+                    
                     responses.Add(response);
                 }
                 catch (HttpRequestException httpEx)
@@ -80,28 +87,15 @@ public async Task<ScrobbleResponse> ScrobbleAsyncInternal(IEnumerable<Scrobble>
             }
             else
             {
-                try
+                var firstBadResponse = responses.FirstOrDefault(r => !r.Success && r.Status != LastResponseStatus.Unknown);
-                {
+                var originalResponseStatus = firstBadResponse?.Status ?? LastResponseStatus.RequestFailed; // TODO check httpEx
-                    var firstBadResponse = responses.FirstOrDefault(r => !r.Success && r.Status != LastResponseStatus.Unknown);
-                    var originalResponseStatus = firstBadResponse != null
-                        ? firstBadResponse.Status
-                        : LastResponseStatus.RequestFailed; // TODO check httpEx
 
-                    var cacheStatus = await CacheAsync(scrobblesList, originalResponseStatus);
+                var cacheStatus = await CacheAsync(scrobblesList, originalResponseStatus);
 
-                    scrobblerResponse = new ScrobbleResponse(cacheStatus);
+                scrobblerResponse = new ScrobbleResponse(cacheStatus);
-                }
-                catch (Exception e)
-                {
-                    scrobblerResponse = new ScrobbleResponse(LastResponseStatus.CacheFailed)
-                    {
-                        Exception = e
-                    };
-                }
             }
             
-            var ignoredScrobbles = responses.SelectMany(r => r.Ignored);
+            scrobblerResponse.Ignored = responses.SelectMany(r => r.Ignored);
-            scrobblerResponse.Ignored = ignoredScrobbles;
 
             AfterSend?.Invoke(this, scrobblerResponse);
 
@@ -110,6 +104,10 @@ public async Task<ScrobbleResponse> ScrobbleAsyncInternal(IEnumerable<Scrobble>
 
         public abstract Task<IEnumerable<Scrobble>> GetCachedAsync();
 
-        protected abstract Task<LastResponseStatus> CacheAsync(IEnumerable<Scrobble> scrobble, LastResponseStatus originalResponseStatus);
+        public abstract Task RemoveFromCacheAsync(ICollection<Scrobble> scrobbles);
+
+        public abstract Task<int> GetCachedCountAsync();
+
+        protected abstract Task<LastResponseStatus> CacheAsync(IEnumerable<Scrobble> scrobble, LastResponseStatus reason);
     }
 }

src/IF.Lastfm.SQLite.Tests.Integration/SQLiteScrobblerTests.cs

@@ -3,6 +3,7 @@
 using System.Net.Http;
 using IF.Lastfm.Core.Scrobblers;
 using IF.Lastfm.Core.Tests.Scrobblers;
+using SQLite;
 
 namespace IF.Lastfm.SQLite.Tests.Integration
 {
@@ -12,7 +13,7 @@ public class SQLiteScrobblerTests : ScrobblerTestsBase
 
         public override void Initialise()
         {
-            var dbPath = Path.GetFullPath("test.db");
+            var dbPath = Path.GetFullPath($"test-{DateTime.UtcNow.ToFileTimeUtc()}.db");
             File.Delete(dbPath);
             using (File.Create(dbPath))
             {
@@ -25,6 +26,7 @@ public override void Initialise()
 
         public override void Cleanup()
         {
+            SQLiteAsyncConnection.ResetPool();
             GC.Collect();
             GC.WaitForPendingFinalizers();
             File.Delete(_dbPath);

src/IF.Lastfm.SQLite/SQLiteScrobbler.cs

@@ -6,67 +6,63 @@
 using IF.Lastfm.Core.Api.Enums;
 using IF.Lastfm.Core.Objects;
 using IF.Lastfm.Core.Scrobblers;
+using Newtonsoft.Json.Converters;
 using SQLite;
 
 namespace IF.Lastfm.SQLite
 {
     public class SQLiteScrobbler : ScrobblerBase
     {
-        public string DatabasePath { get; private set; }
+        public string DatabasePath { get; }
 
         public SQLiteScrobbler(ILastAuth auth, string databasePath, HttpClient client = null) : base(auth, client)
         {
             DatabasePath = databasePath;
-
-            CacheEnabled = true;
         }
 
-        public override Task<IEnumerable<Scrobble>> GetCachedAsync()
+        public override async Task<IEnumerable<Scrobble>> GetCachedAsync()
         {
-            using (var db = new SQLiteConnection(DatabasePath, SQLiteOpenFlags.ReadOnly))
+            var db = GetConnection();
-            {
+            var tableInfo = db.Table<Scrobble>();
-                var tableInfo = db.GetTableInfo(typeof (Scrobble).Name);
+            var cached = await tableInfo.ToListAsync();
-                if (!tableInfo.Any())
+            return cached;
-                {
-                    return Task.FromResult(Enumerable.Empty<Scrobble>());
-                }
-
-                var cached = db.Query<Scrobble>("SELECT * FROM Scrobble");
-                db.Close();
-
-                return Task.FromResult(cached.AsEnumerable());
-            }
         }
 
-        protected override Task<LastResponseStatus> CacheAsync(IEnumerable<Scrobble> scrobbles, LastResponseStatus originalResponseStatus)
+        public override async Task RemoveFromCacheAsync(ICollection<Scrobble> scrobbles)
         {
-            // TODO cache originalResponse - reason to cache
+            var db = GetConnection();
-            return Task.Run(() =>
+            await db.RunInTransactionAsync(connection =>
             {
-                Cache(scrobbles);
-                return LastResponseStatus.Cached;
-            });
-        }
-
-        private void Cache(IEnumerable<Scrobble> scrobbles)
-        {
-            using (var db = new SQLiteConnection(DatabasePath, SQLiteOpenFlags.ReadWrite))
-            {
-                var tableInfo = db.GetTableInfo(typeof (Scrobble).Name);
-                if (!tableInfo.Any())
-                {
-                    db.CreateTable<Scrobble>();
-                }
-
-                db.BeginTransaction();
                 foreach (var scrobble in scrobbles)
                 {
-                    db.Insert(scrobble);
+                    connection.Delete(scrobble);
                 }
-                db.Commit();
+            });
 
-                db.Close();
+            await Task.WhenAll(scrobbles.Select(s => db.DeleteAsync(s)).ToArray());
-            }
+        }
+
+        public override async Task<int> GetCachedCountAsync()
+        {
+            var db = GetConnection();
+            var tableInfo = db.Table<Scrobble>();
+            var count = await tableInfo.CountAsync();
+            return count;
+        }
+
+        protected override async Task<LastResponseStatus> CacheAsync(IEnumerable<Scrobble> scrobbles, LastResponseStatus reason)
+        {
+            // TODO cache reason
+            var db = GetConnection();
+            await db.InsertAllAsync(scrobbles);
+            return LastResponseStatus.Cached;
+        }
+        
+        private SQLiteAsyncConnection GetConnection()
+        {
+            var db = new SQLiteAsyncConnection(DatabasePath, SQLiteOpenFlags.ReadWrite);
+            db.GetConnection().CreateTable<Scrobble>(CreateFlags.AutoIncPK | CreateFlags.AllImplicit);
+            return db;
         }
     }
 }