diff --git a/API.Tests/Services/ScrobblingServiceTests.cs b/API.Tests/Services/ScrobblingServiceTests.cs index 50398a146..a9b2e1e3d 100644 --- a/API.Tests/Services/ScrobblingServiceTests.cs +++ b/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 _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(); _logger = Substitute.For>(); _emailService = Substitute.For(); + _kavitaPlusApiService = Substitute.For(); - _service = new ScrobblingService(UnitOfWork, Substitute.For(), _logger, _licenseService, _localizationService, _emailService); + _service = new ScrobblingService(UnitOfWork, Substitute.For(), _logger, _licenseService, + _localizationService, _emailService, _kavitaPlusApiService); + + _readerService = new ReaderService(UnitOfWork, + Substitute.For>(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For()); // 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 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(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(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] diff --git a/API/Data/Repositories/ScrobbleEventRepository.cs b/API/Data/Repositories/ScrobbleEventRepository.cs index c5f30c2ec..26496e8f4 100644 --- a/API/Data/Repositories/ScrobbleEventRepository.cs +++ b/API/Data/Repositories/ScrobbleEventRepository.cs @@ -29,7 +29,15 @@ public interface IScrobbleRepository Task> GetAllScrobbleErrorsForSeries(int seriesId); Task ClearScrobbleErrors(); Task HasErrorForSeries(int seriesId); - Task GetEvent(int userId, int seriesId, ScrobbleEventType eventType); + /// + /// Get all events for a specific user and type + /// + /// + /// + /// + /// If true, only returned not processed events + /// + Task GetEvent(int userId, int seriesId, ScrobbleEventType eventType, bool isNotProcessed = false); Task> GetUserEventsForSeries(int userId, int seriesId); Task> GetUserEvents(int userId, ScrobbleEventFilter filter, UserParams pagination); Task> GetAllEventsForSeries(int seriesId); @@ -146,10 +154,13 @@ public class ScrobbleRepository : IScrobbleRepository return await _context.ScrobbleError.AnyAsync(n => n.SeriesId == seriesId); } - public async Task GetEvent(int userId, int seriesId, ScrobbleEventType eventType) + public async Task 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> GetUserEventsForSeries(int userId, int seriesId) diff --git a/API/Extensions/ApplicationServiceExtensions.cs b/API/Extensions/ApplicationServiceExtensions.cs index 10f285669..851731753 100644 --- a/API/Extensions/ApplicationServiceExtensions.cs +++ b/API/Extensions/ApplicationServiceExtensions.cs @@ -75,6 +75,7 @@ public static class ApplicationServiceExtensions services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/API/Services/Plus/KavitaPlusApiService.cs b/API/Services/Plus/KavitaPlusApiService.cs new file mode 100644 index 000000000..cdf9471f8 --- /dev/null +++ b/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; + +/// +/// All Http requests to K+ should be contained in this service, the service will not handle any errors. +/// This is expected from the caller. +/// +public interface IKavitaPlusApiService +{ + Task HasTokenExpired(string license, string token, ScrobbleProvider provider); + Task GetRateLimit(string license, string token); + Task PostScrobbleUpdate(ScrobbleDto data, string license); +} + +public class KavitaPlusApiService(ILogger logger): IKavitaPlusApiService +{ + private const string ScrobblingPath = "/api/scrobbling/"; + + public async Task 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 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 PostScrobbleUpdate(ScrobbleDto data, string license) + { + return await PostAndReceive(ScrobblingPath + "update", data, license); + } + + /// + /// Send a GET request to K+ + /// + /// only path of the uri, the host is added + /// + /// + /// + private static async Task Get(string url, string license, string? aniListToken = null) + { + return await (Configuration.KavitaPlusApiUrl + url) + .WithKavitaPlusHeaders(license, aniListToken) + .GetAsync(); + } + + /// + /// Send a POST request to K+ + /// + /// only path of the uri, the host is added + /// + /// + /// + /// Return type + /// + private static async Task PostAndReceive(string url, object body, string license, string? aniListToken = null) + { + return await (Configuration.KavitaPlusApiUrl + url) + .WithKavitaPlusHeaders(license, aniListToken) + .PostJsonAsync(body) + .ReceiveJson(); + } +} diff --git a/API/Services/Plus/ScrobblingService.cs b/API/Services/Plus/ScrobblingService.cs index 85814dcd9..c89097221 100644 --- a/API/Services/Plus/ScrobblingService.cs +++ b/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 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,81 +500,77 @@ public class ScrobblingService : IScrobblingService return 0; } - private async Task PostScrobbleUpdate(ScrobbleDto data, string license, ScrobbleEvent evt) + public async Task PostScrobbleUpdate(ScrobbleDto data, string license, ScrobbleEvent evt) { try { - var response = await (Configuration.KavitaPlusApiUrl + "/api/scrobbling/update") - .WithKavitaPlusHeaders(license) - .PostJsonAsync(data) - .ReceiveJson(); + var response = await _kavitaPlusApiService.PostScrobbleUpdate(data, license); - if (!response.Successful) + if (response.Successful || response.ErrorMessage == null) return response.RateLeft; + + // Might want to log this under ScrobbleError + if (response.ErrorMessage.Contains("Too Many Requests")) { - // Might want to log this under ScrobbleError - if (response.ErrorMessage != null && 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.Contains("Unauthorized")) + { + _logger.LogCritical("Kavita+ responded with Unauthorized. Please check your subscription"); + await _licenseService.HasActiveLicense(true); + evt.IsErrored = true; + evt.ErrorDetails = "Kavita+ subscription no longer active"; + throw new KavitaException("Kavita+ responded with Unauthorized. Please check your subscription"); + } + if (response.ErrorMessage.Contains("Access token is invalid")) + { + evt.IsErrored = true; + evt.ErrorDetails = AccessTokenErrorMessage; + throw new KavitaException("Access token is invalid"); + } + 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); + if (!await _unitOfWork.ScrobbleRepository.HasErrorForSeries(evt.SeriesId)) { - _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")) - { - _logger.LogCritical("Kavita+ responded with Unauthorized. Please check your subscription"); - await _licenseService.HasActiveLicense(true); - evt.IsErrored = true; - 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")) - { - evt.IsErrored = true; - evt.ErrorDetails = AccessTokenErrorMessage; - throw new KavitaException("Access token is invalid"); - } - if (response.ErrorMessage != null && 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); - if (!await _unitOfWork.ScrobbleRepository.HasErrorForSeries(evt.SeriesId)) + // Create a new ExternalMetadata entry to indicate that this is not matchable + var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(evt.SeriesId, SeriesIncludes.ExternalMetadata); + if (series == null) return 0; + + series.ExternalSeriesMetadata ??= new ExternalSeriesMetadata() {SeriesId = evt.SeriesId}; + series.IsBlacklisted = true; + _unitOfWork.SeriesRepository.Update(series); + + _unitOfWork.ScrobbleRepository.Attach(new ScrobbleError() { - // Create a new ExternalMetadata entry to indicate that this is not matchable - var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(evt.SeriesId, SeriesIncludes.ExternalMetadata); - if (series == null) return 0; + Comment = UnknownSeriesErrorMessage, + Details = data.SeriesName, + LibraryId = evt.LibraryId, + SeriesId = evt.SeriesId + }); - series.ExternalSeriesMetadata ??= new ExternalSeriesMetadata() {SeriesId = evt.SeriesId}; - series.IsBlacklisted = true; - _unitOfWork.SeriesRepository.Update(series); - - _unitOfWork.ScrobbleRepository.Attach(new ScrobbleError() - { - Comment = UnknownSeriesErrorMessage, - Details = data.SeriesName, - LibraryId = evt.LibraryId, - SeriesId = evt.SeriesId - }); - - } - - evt.IsErrored = true; - evt.ErrorDetails = UnknownSeriesErrorMessage; - } else if (response.ErrorMessage != null && response.ErrorMessage.StartsWith("Review")) - { - // Log the Series name and Id in ScrobbleErrors - _logger.LogInformation("Kavita+ was unable to save the review"); - if (!await _unitOfWork.ScrobbleRepository.HasErrorForSeries(evt.SeriesId)) - { - _unitOfWork.ScrobbleRepository.Attach(new ScrobbleError() - { - Comment = response.ErrorMessage, - Details = data.SeriesName, - LibraryId = evt.LibraryId, - SeriesId = evt.SeriesId - }); - } - evt.IsErrored = true; - evt.ErrorDetails = "Review was unable to be saved due to upstream requirements"; } + + evt.IsErrored = true; + evt.ErrorDetails = UnknownSeriesErrorMessage; + } else if (response.ErrorMessage.StartsWith("Review")) + { + // Log the Series name and Id in ScrobbleErrors + _logger.LogInformation("Kavita+ was unable to save the review"); + if (!await _unitOfWork.ScrobbleRepository.HasErrorForSeries(evt.SeriesId)) + { + _unitOfWork.ScrobbleRepository.Attach(new ScrobbleError() + { + Comment = response.ErrorMessage, + Details = data.SeriesName, + LibraryId = evt.LibraryId, + SeriesId = evt.SeriesId + }); + } + 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)) { diff --git a/Kavita.Common/Configuration.cs b/Kavita.Common/Configuration.cs index 3450cdce3..f2d64cde6 100644 --- a/Kavita.Common/Configuration.cs +++ b/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