Scrobbling rewrite - abstract K+ API requests and more tests

API.Tests/Services/ScrobblingServiceTests.cs

@@ -1,11 +1,15 @@
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using API.DTOs.Scrobbling;
+using API.Entities;
 using API.Entities.Enums;
+using API.Entities.Scrobble;
 using API.Helpers.Builders;
 using API.Services;
 using API.Services.Plus;
 using API.SignalR;
+using Kavita.Common;
 using Microsoft.Extensions.Logging;
 using NSubstitute;
 using Xunit;
@@ -15,11 +19,15 @@ namespace API.Tests.Services;
 
 public class ScrobblingServiceTests : AbstractDbTest
 {
+    private const int ChapterPages = 100;
+
     private readonly ScrobblingService _service;
     private readonly ILicenseService _licenseService;
     private readonly ILocalizationService _localizationService;
     private readonly ILogger<ScrobblingService> _logger;
     private readonly IEmailService _emailService;
+    private readonly IKavitaPlusApiService _kavitaPlusApiService;
+    private readonly IReaderService _readerService;
 
     public ScrobblingServiceTests()
     {
@@ -27,8 +35,17 @@ public class ScrobblingServiceTests : AbstractDbTest
         _localizationService = Substitute.For<ILocalizationService>();
         _logger = Substitute.For<ILogger<ScrobblingService>>();
         _emailService = Substitute.For<IEmailService>();
+        _kavitaPlusApiService = Substitute.For<IKavitaPlusApiService>();
 
-        _service = new ScrobblingService(UnitOfWork, Substitute.For<IEventHub>(), _logger,  _licenseService, _localizationService, _emailService);
+        _service = new ScrobblingService(UnitOfWork, Substitute.For<IEventHub>(), _logger,  _licenseService,
+            _localizationService, _emailService, _kavitaPlusApiService);
+
+        _readerService = new ReaderService(UnitOfWork,
+            Substitute.For<ILogger<ReaderService>>(),
+            Substitute.For<IEventHub>(),
+            Substitute.For<IImageService>(),
+            Substitute.For<IDirectoryService>(),
+            Substitute.For<IScrobblingService>()); // Do not use the actual one
     }
 
     protected override async Task ResetDb()
@@ -46,6 +63,18 @@ public class ScrobblingServiceTests : AbstractDbTest
         var series = new SeriesBuilder("Test Series")
             .WithFormat(MangaFormat.Archive)
             .WithMetadata(new SeriesMetadataBuilder().Build())
+            .WithVolume(new VolumeBuilder("Volume 1")
+                .WithChapters([
+                    new ChapterBuilder("1")
+                        .WithPages(ChapterPages)
+                        .Build(),
+                    new ChapterBuilder("2")
+                        .WithPages(ChapterPages)
+                        .Build(),
+                    new ChapterBuilder("3")
+                        .WithPages(ChapterPages)
+                        .Build()])
+                .Build())
             .Build();
 
         var library = new LibraryBuilder("Test Library", LibraryType.Manga)
@@ -67,6 +96,176 @@ public class ScrobblingServiceTests : AbstractDbTest
         await UnitOfWork.CommitAsync();
     }
 
+    private async Task<ScrobbleEvent> CreateScrobbleEvent(int? seriesId = null)
+    {
+        var evt = new ScrobbleEvent
+        {
+            ScrobbleEventType = ScrobbleEventType.ChapterRead,
+            Format = PlusMediaFormat.Manga,
+            SeriesId = seriesId ?? 0,
+            LibraryId = 0,
+            AppUserId = 0,
+        };
+
+        if (seriesId != null)
+        {
+            var series = await UnitOfWork.SeriesRepository.GetSeriesByIdAsync(seriesId.Value);
+            if (series != null) evt.Series = series;
+        }
+
+        return evt;
+    }
+
+
+    #region K+ API Request Tests
+
+    [Fact]
+    public async Task PostScrobbleUpdate_AuthErrors()
+    {
+        _kavitaPlusApiService.PostScrobbleUpdate(null!, "")
+            .ReturnsForAnyArgs(new ScrobbleResponseDto()
+            {
+                ErrorMessage = "Unauthorized"
+            });
+
+        var evt = await CreateScrobbleEvent();
+        await Assert.ThrowsAsync<KavitaException>(async () =>
+        {
+            await _service.PostScrobbleUpdate(new ScrobbleDto(), "", evt);
+        });
+        Assert.True(evt.IsErrored);
+        Assert.Equal("Kavita+ subscription no longer active", evt.ErrorDetails);
+    }
+
+    [Fact]
+    public async Task PostScrobbleUpdate_UnknownSeriesLoggedAsError()
+    {
+        _kavitaPlusApiService.PostScrobbleUpdate(null!, "")
+            .ReturnsForAnyArgs(new ScrobbleResponseDto()
+            {
+                ErrorMessage = "Unknown Series"
+            });
+
+        await SeedData();
+        var evt = await CreateScrobbleEvent(1);
+
+        await _service.PostScrobbleUpdate(new ScrobbleDto(), "", evt);
+        await UnitOfWork.CommitAsync();
+        Assert.True(evt.IsErrored);
+
+        var series = await UnitOfWork.SeriesRepository.GetSeriesByIdAsync(1);
+        Assert.NotNull(series);
+        Assert.True(series.IsBlacklisted);
+
+        var errors = await UnitOfWork.ScrobbleRepository.GetAllScrobbleErrorsForSeries(1);
+        Assert.Single(errors);
+        Assert.Equal("Series cannot be matched for Scrobbling", errors.First().Comment);
+        Assert.Equal(series.Id, errors.First().SeriesId);
+    }
+
+    [Fact]
+    public async Task PostScrobbleUpdate_InvalidAccessToken()
+    {
+        _kavitaPlusApiService.PostScrobbleUpdate(null!, "")
+            .ReturnsForAnyArgs(new ScrobbleResponseDto()
+            {
+                ErrorMessage = "Access token is invalid"
+            });
+
+        var evt = await CreateScrobbleEvent();
+
+        await Assert.ThrowsAsync<KavitaException>(async () =>
+        {
+            await _service.PostScrobbleUpdate(new ScrobbleDto(), "", evt);
+        });
+
+        Assert.True(evt.IsErrored);
+        Assert.Equal("Access Token needs to be rotated to continue scrobbling", evt.ErrorDetails);
+    }
+
+    #endregion
+
+    #region Scrobble Reading Update Tests
+
+    [Fact]
+    public async Task ScrobbleReadingUpdate_IgnoreNoLicense()
+    {
+        await ResetDb();
+        await SeedData();
+
+        _licenseService.HasActiveLicense().Returns(false);
+
+        await _service.ScrobbleReadingUpdate(1, 1);
+        var events = await UnitOfWork.ScrobbleRepository.GetAllEventsForSeries(1);
+        Assert.Empty(events);
+    }
+
+    [Fact]
+    public async Task ScrobbleReadingUpdate_UpdateExistingNotIsProcessed()
+    {
+        await ResetDb();
+        await SeedData();
+
+        var user = await UnitOfWork.UserRepository.GetUserByIdAsync(1);
+        Assert.NotNull(user);
+
+        var chapter1 = await UnitOfWork.ChapterRepository.GetChapterAsync(1);
+        var chapter2 = await UnitOfWork.ChapterRepository.GetChapterAsync(2);
+        var chapter3 = await UnitOfWork.ChapterRepository.GetChapterAsync(3);
+        Assert.NotNull(chapter1);
+        Assert.NotNull(chapter2);
+        Assert.NotNull(chapter3);
+
+        _licenseService.HasActiveLicense().Returns(true);
+
+        await _service.ScrobbleReadingUpdate(1, 1);
+        var events = await UnitOfWork.ScrobbleRepository.GetAllEventsForSeries(1);
+        Assert.Single(events);
+
+        var readEvent = events.First();
+        Assert.False(readEvent.IsProcessed);
+
+        await _readerService.MarkChaptersAsRead(user, 1, [chapter1]);
+        await UnitOfWork.CommitAsync();
+
+        // Scrobble update
+        await _service.ScrobbleReadingUpdate(1, 1);
+        events = await UnitOfWork.ScrobbleRepository.GetAllEventsForSeries(1);
+        Assert.Single(events);
+
+        readEvent = events.First();
+        Assert.False(readEvent.IsProcessed);
+        Assert.Equal(1, readEvent.ChapterNumber);
+
+        // Mark as processed
+        readEvent.IsProcessed = true;
+        await UnitOfWork.CommitAsync();
+
+        await _readerService.MarkChaptersAsRead(user, 1, [chapter2]);
+        await UnitOfWork.CommitAsync();
+
+        // Scrobble update
+        await _service.ScrobbleReadingUpdate(1, 1);
+        events = await UnitOfWork.ScrobbleRepository.GetAllEventsForSeries(1);
+        Assert.Equal(2, events.Count);
+        Assert.Single(events.Where(e => e.IsProcessed).ToList());
+        Assert.Single(events.Where(e => !e.IsProcessed).ToList());
+
+        // Should update the existing non processed event
+        await _readerService.MarkChaptersAsRead(user, 1, [chapter3]);
+        await UnitOfWork.CommitAsync();
+
+        // Scrobble update
+        await _service.ScrobbleReadingUpdate(1, 1);
+        events = await UnitOfWork.ScrobbleRepository.GetAllEventsForSeries(1);
+        Assert.Equal(2, events.Count);
+        Assert.Single(events.Where(e => e.IsProcessed).ToList());
+        Assert.Single(events.Where(e => !e.IsProcessed).ToList());
+    }
+
+    #endregion
+
+
     #region ScrobbleWantToReadUpdate Tests
 
     [Fact]

API/Data/Repositories/ScrobbleEventRepository.cs

@@ -29,7 +29,15 @@ public interface IScrobbleRepository
     Task<IList<ScrobbleError>> GetAllScrobbleErrorsForSeries(int seriesId);
     Task ClearScrobbleErrors();
     Task<bool> HasErrorForSeries(int seriesId);
-    Task<ScrobbleEvent?> GetEvent(int userId, int seriesId, ScrobbleEventType eventType);
+    /// <summary>
+    /// Get all events for a specific user and type
+    /// </summary>
+    /// <param name="userId"></param>
+    /// <param name="seriesId"></param>
+    /// <param name="eventType"></param>
+    /// <param name="isNotProcessed">If true, only returned not processed events</param>
+    /// <returns></returns>
+    Task<ScrobbleEvent?> GetEvent(int userId, int seriesId, ScrobbleEventType eventType, bool isNotProcessed = false);
     Task<IEnumerable<ScrobbleEvent>> GetUserEventsForSeries(int userId, int seriesId);
     Task<PagedList<ScrobbleEventDto>> GetUserEvents(int userId, ScrobbleEventFilter filter, UserParams pagination);
     Task<IList<ScrobbleEvent>> GetAllEventsForSeries(int seriesId);
@@ -146,10 +154,13 @@ public class ScrobbleRepository : IScrobbleRepository
         return await _context.ScrobbleError.AnyAsync(n => n.SeriesId == seriesId);
     }
 
-    public async Task<ScrobbleEvent?> GetEvent(int userId, int seriesId, ScrobbleEventType eventType)
+    public async Task<ScrobbleEvent?> GetEvent(int userId, int seriesId, ScrobbleEventType eventType, bool isNotProcessed = false)
     {
-        return await _context.ScrobbleEvent.FirstOrDefaultAsync(e =>
-            e.AppUserId == userId && e.SeriesId == seriesId && e.ScrobbleEventType == eventType);
+        return await _context.ScrobbleEvent
+            .Where(e => e.AppUserId == userId && e.SeriesId == seriesId && e.ScrobbleEventType == eventType)
+            .WhereIf(isNotProcessed, e => !e.IsProcessed)
+            .OrderBy(e => e.LastModifiedUtc)
+            .FirstOrDefaultAsync();
     }
 
     public async Task<IEnumerable<ScrobbleEvent>> GetUserEventsForSeries(int userId, int seriesId)

API/Extensions/ApplicationServiceExtensions.cs

@@ -75,6 +75,7 @@ public static class ApplicationServiceExtensions
         services.AddScoped<ISettingsService, SettingsService>();
 
 
+        services.AddScoped<IKavitaPlusApiService, KavitaPlusApiService>();
         services.AddScoped<IScrobblingService, ScrobblingService>();
         services.AddScoped<ILicenseService, LicenseService>();
         services.AddScoped<IExternalMetadataService, ExternalMetadataService>();

API/Services/Plus/KavitaPlusApiService.cs

@@ -0,0 +1,75 @@
+#nullable enable
+using System.Threading.Tasks;
+using API.DTOs.Scrobbling;
+using API.Extensions;
+using Flurl.Http;
+using Kavita.Common;
+using Microsoft.Extensions.Logging;
+
+namespace API.Services.Plus;
+
+/// <summary>
+/// All Http requests to K+ should be contained in this service, the service will not handle any errors.
+/// This is expected from the caller.
+/// </summary>
+public interface IKavitaPlusApiService
+{
+    Task<bool> HasTokenExpired(string license, string token, ScrobbleProvider provider);
+    Task<int> GetRateLimit(string license, string token);
+    Task<ScrobbleResponseDto> PostScrobbleUpdate(ScrobbleDto data, string license);
+}
+
+public class KavitaPlusApiService(ILogger<KavitaPlusApiService> logger): IKavitaPlusApiService
+{
+    private const string ScrobblingPath = "/api/scrobbling/";
+
+    public async Task<bool> HasTokenExpired(string license, string token, ScrobbleProvider provider)
+    {
+        var res = await Get(ScrobblingPath + "valid-key?provider=" + provider + "&key=" + token, license, token);
+        var str = await res.GetStringAsync();
+        return bool.Parse(str);
+    }
+
+    public async Task<int> GetRateLimit(string license, string token)
+    {
+        var res = await Get(ScrobblingPath + "rate-limit?accessToken=" + token, license, token);
+        var str = await res.GetStringAsync();
+        return int.Parse(str);
+    }
+
+    public async Task<ScrobbleResponseDto> PostScrobbleUpdate(ScrobbleDto data, string license)
+    {
+        return await PostAndReceive<ScrobbleResponseDto>(ScrobblingPath + "update", data, license);
+    }
+
+    /// <summary>
+    /// Send a GET request to K+
+    /// </summary>
+    /// <param name="url">only path of the uri, the host is added</param>
+    /// <param name="license"></param>
+    /// <param name="aniListToken"></param>
+    /// <returns></returns>
+    private static async Task<IFlurlResponse> Get(string url, string license, string? aniListToken = null)
+    {
+        return await (Configuration.KavitaPlusApiUrl + url)
+            .WithKavitaPlusHeaders(license, aniListToken)
+            .GetAsync();
+    }
+
+    /// <summary>
+    /// Send a POST request to K+
+    /// </summary>
+    /// <param name="url">only path of the uri, the host is added</param>
+    /// <param name="body"></param>
+    /// <param name="license"></param>
+    /// <param name="aniListToken"></param>
+    /// <typeparam name="T">Return type</typeparam>
+    /// <returns></returns>
+    private static async Task<T> PostAndReceive<T>(string url, object body, string license, string? aniListToken = null)
+    {
+        return await (Configuration.KavitaPlusApiUrl + url)
+            .WithKavitaPlusHeaders(license, aniListToken)
+            .PostJsonAsync(body)
+            .ReceiveJson<T>();
+    }
+}

API/Services/Plus/ScrobblingService.cs

@@ -71,6 +71,7 @@ public class ScrobblingService : IScrobblingService
     private readonly ILicenseService _licenseService;
     private readonly ILocalizationService _localizationService;
     private readonly IEmailService _emailService;
+    private readonly IKavitaPlusApiService _kavitaPlusApiService;
 
     public const string AniListWeblinkWebsite = "https://anilist.co/manga/";
     public const string MalWeblinkWebsite = "https://myanimelist.net/manga/";
@@ -107,7 +108,8 @@ public class ScrobblingService : IScrobblingService
 
 
     public ScrobblingService(IUnitOfWork unitOfWork, IEventHub eventHub, ILogger<ScrobblingService> logger,
-        ILicenseService licenseService, ILocalizationService localizationService, IEmailService emailService)
+        ILicenseService licenseService, ILocalizationService localizationService, IEmailService emailService,
+        IKavitaPlusApiService kavitaPlusApiService)
     {
         _unitOfWork = unitOfWork;
         _eventHub = eventHub;
@@ -115,6 +117,7 @@ public class ScrobblingService : IScrobblingService
         _licenseService = licenseService;
         _localizationService = localizationService;
         _emailService = emailService;
+        _kavitaPlusApiService = kavitaPlusApiService;
 
         FlurlConfiguration.ConfigureClientForUrl(Configuration.KavitaPlusApiUrl);
     }
@@ -222,11 +225,7 @@ public class ScrobblingService : IScrobblingService
 
         try
         {
-            var response = await (Configuration.KavitaPlusApiUrl + "/api/scrobbling/valid-key?provider=" + provider + "&key=" + token)
-                .WithKavitaPlusHeaders(license.Value, token)
-                .GetStringAsync();
-
-            return bool.Parse(response);
+            return await _kavitaPlusApiService.HasTokenExpired(license.Value, token, provider);
         }
         catch (HttpRequestException e)
         {
@@ -374,8 +373,9 @@ public class ScrobblingService : IScrobblingService
         _logger.LogInformation("Processing Scrobbling reading event for {AppUserId} on {SeriesName}", userId, series.Name);
         if (await CheckIfCannotScrobble(userId, seriesId, series)) return;
 
+        // Check if there is an existing not yet processed event, if so update it
         var existingEvt = await _unitOfWork.ScrobbleRepository.GetEvent(userId, series.Id,
-            ScrobbleEventType.ChapterRead);
+            ScrobbleEventType.ChapterRead, true);
         if (existingEvt is {IsProcessed: false})
         {
             // We need to just update Volume/Chapter number
@@ -386,8 +386,10 @@ public class ScrobblingService : IScrobblingService
                 (int) await _unitOfWork.AppUserProgressRepository.GetHighestFullyReadVolumeForSeries(seriesId, userId);
             existingEvt.ChapterNumber =
                 await _unitOfWork.AppUserProgressRepository.GetHighestFullyReadChapterForSeries(seriesId, userId);
+
             _unitOfWork.ScrobbleRepository.Update(existingEvt);
             await _unitOfWork.CommitAsync();
+
             _logger.LogDebug("Overriding scrobble event for {Series} from vol {PrevVol} ch {PrevChap} -> vol {UpdatedVol} ch {UpdatedChap}",
                 existingEvt.Series.Name, prevVol, prevChapter, existingEvt.VolumeNumber, existingEvt.ChapterNumber);
             return;
@@ -488,11 +490,7 @@ public class ScrobblingService : IScrobblingService
         if (string.IsNullOrWhiteSpace(aniListToken)) return 0;
         try
         {
-            var response = await (Configuration.KavitaPlusApiUrl + "/api/scrobbling/rate-limit?accessToken=" + aniListToken)
-                .WithKavitaPlusHeaders(license, aniListToken)
-                .GetStringAsync();
-
-            return int.Parse(response);
+            return await _kavitaPlusApiService.GetRateLimit(license, aniListToken);
         }
         catch (Exception e)
         {
@@ -502,25 +500,22 @@ public class ScrobblingService : IScrobblingService
         return 0;
     }
 
-    private async Task<int> PostScrobbleUpdate(ScrobbleDto data, string license, ScrobbleEvent evt)
+    public async Task<int> PostScrobbleUpdate(ScrobbleDto data, string license, ScrobbleEvent evt)
     {
         try
         {
-            var response = await (Configuration.KavitaPlusApiUrl + "/api/scrobbling/update")
-                .WithKavitaPlusHeaders(license)
-                .PostJsonAsync(data)
-                .ReceiveJson<ScrobbleResponseDto>();
+            var response = await _kavitaPlusApiService.PostScrobbleUpdate(data, license);
+
+            if (response.Successful || response.ErrorMessage == null) return response.RateLeft;
 
-            if (!response.Successful)
-            {
             // Might want to log this under ScrobbleError
-                if (response.ErrorMessage != null && response.ErrorMessage.Contains("Too Many Requests"))
+            if (response.ErrorMessage.Contains("Too Many Requests"))
             {
                 _logger.LogInformation("Hit Too many requests, sleeping to regain requests and retrying");
                 await Task.Delay(TimeSpan.FromMinutes(10));
                 return await PostScrobbleUpdate(data, license, evt);
             }
-                if (response.ErrorMessage != null && response.ErrorMessage.Contains("Unauthorized"))
+            if (response.ErrorMessage.Contains("Unauthorized"))
             {
                 _logger.LogCritical("Kavita+ responded with Unauthorized. Please check your subscription");
                 await _licenseService.HasActiveLicense(true);
@@ -528,13 +523,13 @@ public class ScrobblingService : IScrobblingService
                 evt.ErrorDetails = "Kavita+ subscription no longer active";
                 throw new KavitaException("Kavita+ responded with Unauthorized. Please check your subscription");
             }
-                if (response.ErrorMessage != null && response.ErrorMessage.Contains("Access token is invalid"))
+            if (response.ErrorMessage.Contains("Access token is invalid"))
             {
                 evt.IsErrored = true;
                 evt.ErrorDetails = AccessTokenErrorMessage;
                 throw new KavitaException("Access token is invalid");
             }
-                if (response.ErrorMessage != null && response.ErrorMessage.Contains("Unknown Series"))
+            if (response.ErrorMessage.Contains("Unknown Series"))
             {
                 // Log the Series name and Id in ScrobbleErrors
                 _logger.LogInformation("Kavita+ was unable to match the series: {SeriesName}", evt.Series.Name);
@@ -560,7 +555,7 @@ public class ScrobblingService : IScrobblingService
 
                 evt.IsErrored = true;
                 evt.ErrorDetails = UnknownSeriesErrorMessage;
-                } else if (response.ErrorMessage != null && response.ErrorMessage.StartsWith("Review"))
+            } else if (response.ErrorMessage.StartsWith("Review"))
             {
                 // Log the Series name and Id in ScrobbleErrors
                 _logger.LogInformation("Kavita+ was unable to save the review");
@@ -577,7 +572,6 @@ public class ScrobblingService : IScrobblingService
                 evt.IsErrored = true;
                 evt.ErrorDetails = "Review was unable to be saved due to upstream requirements";
             }
-            }
 
             return response.RateLeft;
         }
@@ -595,7 +589,7 @@ public class ScrobblingService : IScrobblingService
             }
 
             _logger.LogError(ex, "Scrobbling to Kavita+ API failed due to error: {ErrorMessage}", ex.Message);
-            if (ex.Message.Contains("Call failed with status code 500 (Internal Server Error)"))
+            if (ex.StatusCode == 500 || ex.Message.Contains("Call failed with status code 500 (Internal Server Error)"))
             {
                 if (!await _unitOfWork.ScrobbleRepository.HasErrorForSeries(evt.SeriesId))
                 {

Kavita.Common/Configuration.cs

@@ -17,7 +17,7 @@ public static class Configuration
     private static readonly string AppSettingsFilename = Path.Join("config", GetAppSettingFilename());
 
     public static readonly string KavitaPlusApiUrl = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") == Environments.Development
-        ?  "https://plus.kavitareader.com" : "https://plus.kavitareader.com";
+        ?  "http://localhost:5020" : "https://plus.kavitareader.com";
     public static readonly string StatsApiUrl = "https://stats.kavitareader.com";
 
     public static int Port