From 349645bf32a2954609e8ec2f8a8fcb2ec63b1a9c Mon Sep 17 00:00:00 2001 From: Joe Milazzo Date: Thu, 21 Nov 2024 06:51:24 -0600 Subject: [PATCH] Small Logging Improvement (#3389) --- API/Controllers/ReaderController.cs | 52 ++++++++++++-------------- API/Services/CacheService.cs | 2 - API/Services/Plus/ScrobblingService.cs | 2 +- API/Services/ReaderService.cs | 1 + 4 files changed, 26 insertions(+), 31 deletions(-) diff --git a/API/Controllers/ReaderController.cs b/API/Controllers/ReaderController.cs index 6efeb04de..b17afa814 100644 --- a/API/Controllers/ReaderController.cs +++ b/API/Controllers/ReaderController.cs @@ -21,7 +21,6 @@ using Kavita.Common; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; -using Microsoft.IdentityModel.Tokens; using MimeTypes; namespace API.Controllers; @@ -90,7 +89,7 @@ public class ReaderController : BaseApiController } catch (Exception) { - _cacheService.CleanupChapters(new []{ chapterId }); + _cacheService.CleanupChapters([chapterId]); throw; } } @@ -105,7 +104,8 @@ public class ReaderController : BaseApiController /// Should Kavita extract pdf into images. Defaults to false. /// [HttpGet("image")] - [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = new []{"chapterId", "page", "extractPdf", "apiKey"})] + [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = ["chapterId", "page", "extractPdf", "apiKey" + ])] [AllowAnonymous] public async Task GetImage(int chapterId, int page, string apiKey, bool extractPdf = false) { @@ -117,7 +117,7 @@ public class ReaderController : BaseApiController { var chapter = await _cacheService.Ensure(chapterId, extractPdf); if (chapter == null) return NoContent(); - _logger.LogInformation("Fetching Page {PageNum} on Chapter {ChapterId}", page, chapterId); + var path = _cacheService.GetCachedPagePath(chapter.Id, page); if (string.IsNullOrEmpty(path) || !System.IO.File.Exists(path)) return BadRequest(await _localizationService.Translate(userId, "no-image-for-page", page)); @@ -127,7 +127,7 @@ public class ReaderController : BaseApiController } catch (Exception) { - _cacheService.CleanupChapters(new []{ chapterId }); + _cacheService.CleanupChapters([chapterId]); throw; } } @@ -140,7 +140,7 @@ public class ReaderController : BaseApiController /// /// [HttpGet("thumbnail")] - [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = new []{"chapterId", "pageNum", "apiKey"})] + [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = ["chapterId", "pageNum", "apiKey"])] [AllowAnonymous] public async Task GetThumbnail(int chapterId, int pageNum, string apiKey) { @@ -164,14 +164,14 @@ public class ReaderController : BaseApiController /// We must use api key as bookmarks could be leaked to other users via the API /// [HttpGet("bookmark-image")] - [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = new []{"seriesId", "page", "apiKey"})] + [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = ["seriesId", "page", "apiKey"])] [AllowAnonymous] public async Task GetBookmarkImage(int seriesId, string apiKey, int page) { - if (page < 0) page = 0; var userId = await _unitOfWork.UserRepository.GetUserIdByApiKeyAsync(apiKey); if (userId == 0) return Unauthorized(); + if (page < 0) page = 0; var totalPages = await _cacheService.CacheBookmarkForSeries(userId, seriesId); if (page > totalPages) { @@ -188,7 +188,7 @@ public class ReaderController : BaseApiController } catch (Exception) { - _cacheService.CleanupBookmarks(new []{ seriesId }); + _cacheService.CleanupBookmarks([seriesId]); throw; } } @@ -202,12 +202,13 @@ public class ReaderController : BaseApiController /// /// [HttpGet("file-dimensions")] - [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = new []{"chapterId", "extractPdf"})] + [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = ["chapterId", "extractPdf"])] public async Task>> GetFileDimensions(int chapterId, bool extractPdf = false) { if (chapterId <= 0) return ArraySegment.Empty; var chapter = await _cacheService.Ensure(chapterId, extractPdf); if (chapter == null) return NoContent(); + return Ok(_cacheService.GetCachedFileDimensions(_cacheService.GetCachePath(chapterId))); } @@ -220,7 +221,8 @@ public class ReaderController : BaseApiController /// Include file dimensions. Only useful for image based reading /// [HttpGet("chapter-info")] - [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = new []{"chapterId", "extractPdf", "includeDimensions"})] + [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = ["chapterId", "extractPdf", "includeDimensions" + ])] public async Task> GetChapterInfo(int chapterId, bool extractPdf = false, bool includeDimensions = false) { if (chapterId <= 0) return Ok(null); // This can happen occasionally from UI, we should just ignore @@ -260,6 +262,7 @@ public class ReaderController : BaseApiController } if (info.ChapterTitle is {Length: > 0}) { + // TODO: Can we rework the logic of generating titles for the UI and instead calculate that in the DB? info.Title += " - " + info.ChapterTitle; } @@ -291,7 +294,7 @@ public class ReaderController : BaseApiController /// Include file dimensions (extra I/O). Defaults to true. /// [HttpGet("bookmark-info")] - [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = new []{"seriesId", "includeDimensions"})] + [ResponseCache(CacheProfileName = ResponseCacheProfiles.Hour, VaryByQueryKeys = ["seriesId", "includeDimensions"])] public async Task> GetBookmarkInfo(int seriesId, bool includeDimensions = true) { var totalPages = await _cacheService.CacheBookmarkForSeries(User.GetUserId(), seriesId); @@ -375,13 +378,10 @@ public class ReaderController : BaseApiController var chapters = await _unitOfWork.ChapterRepository.GetChaptersAsync(markVolumeReadDto.VolumeId); await _readerService.MarkChaptersAsUnread(user, markVolumeReadDto.SeriesId, chapters); - if (await _unitOfWork.CommitAsync()) - { - BackgroundJob.Enqueue(() => _scrobblingService.ScrobbleReadingUpdate(user.Id, markVolumeReadDto.SeriesId)); - return Ok(); - } + if (!await _unitOfWork.CommitAsync()) return BadRequest(await _localizationService.Translate(User.GetUserId(), "generic-read-progress")); - return BadRequest(await _localizationService.Translate(User.GetUserId(), "generic-read-progress")); + BackgroundJob.Enqueue(() => _scrobblingService.ScrobbleReadingUpdate(user.Id, markVolumeReadDto.SeriesId)); + return Ok(); } /// @@ -551,7 +551,7 @@ public class ReaderController : BaseApiController } /// - /// Save page against Chapter for logged in user + /// Save page against Chapter for authenticated user /// /// /// @@ -769,7 +769,7 @@ public class ReaderController : BaseApiController /// /// /// chapter id for next manga - [ResponseCache(CacheProfileName = "Hour", VaryByQueryKeys = new [] { "seriesId", "volumeId", "currentChapterId"})] + [ResponseCache(CacheProfileName = "Hour", VaryByQueryKeys = ["seriesId", "volumeId", "currentChapterId"])] [HttpGet("next-chapter")] public async Task> GetNextChapter(int seriesId, int volumeId, int currentChapterId) { @@ -787,7 +787,7 @@ public class ReaderController : BaseApiController /// /// /// chapter id for next manga - [ResponseCache(CacheProfileName = "Hour", VaryByQueryKeys = new [] { "seriesId", "volumeId", "currentChapterId"})] + [ResponseCache(CacheProfileName = "Hour", VaryByQueryKeys = ["seriesId", "volumeId", "currentChapterId"])] [HttpGet("prev-chapter")] public async Task> GetPreviousChapter(int seriesId, int volumeId, int currentChapterId) { @@ -801,7 +801,7 @@ public class ReaderController : BaseApiController /// /// [HttpGet("time-left")] - [ResponseCache(CacheProfileName = "Hour", VaryByQueryKeys = new [] { "seriesId"})] + [ResponseCache(CacheProfileName = "Hour", VaryByQueryKeys = ["seriesId"])] public async Task> GetEstimateToCompletion(int seriesId) { var userId = User.GetUserId(); @@ -899,12 +899,8 @@ public class ReaderController : BaseApiController [HttpGet("all-chapter-progress")] public async Task>> GetProgressForChapter(int chapterId) { - if (User.IsInRole(PolicyConstants.AdminRole)) - { - return Ok(await _unitOfWork.AppUserProgressRepository.GetUserProgressForChapter(chapterId)); - } - - return Ok(await _unitOfWork.AppUserProgressRepository.GetUserProgressForChapter(chapterId, User.GetUserId())); + var userId = User.IsInRole(PolicyConstants.AdminRole) ? 0 : User.GetUserId(); + return Ok(await _unitOfWork.AppUserProgressRepository.GetUserProgressForChapter(chapterId, userId)); } } diff --git a/API/Services/CacheService.cs b/API/Services/CacheService.cs index 241fd68ca..9a8ef64ce 100644 --- a/API/Services/CacheService.cs +++ b/API/Services/CacheService.cs @@ -118,8 +118,6 @@ public class CacheService : ICacheService Cache.MaxFiles = originalCacheSize; } - _logger.LogDebug("File Dimensions call for {Length} images took {Time}ms", dimensions.Count, sw.ElapsedMilliseconds); - return dimensions; } diff --git a/API/Services/Plus/ScrobblingService.cs b/API/Services/Plus/ScrobblingService.cs index 51939198b..5dba6f56e 100644 --- a/API/Services/Plus/ScrobblingService.cs +++ b/API/Services/Plus/ScrobblingService.cs @@ -345,7 +345,7 @@ public class ScrobblingService : IScrobblingService _unitOfWork.ScrobbleRepository.Attach(evt); await _unitOfWork.CommitAsync(); - _logger.LogDebug("Added Scrobbling Read update on {SeriesName} with Userid {UserId} ", series.Name, userId); + _logger.LogDebug("Added Scrobbling Read update on {SeriesName} - Volume: {VolumeNumber} Chapter: {ChapterNumber} for User: {UserId}", series.Name, evt.VolumeNumber, evt.ChapterNumber, userId); } catch (Exception ex) { diff --git a/API/Services/ReaderService.cs b/API/Services/ReaderService.cs index ca081bc9d..dd4b824b8 100644 --- a/API/Services/ReaderService.cs +++ b/API/Services/ReaderService.cs @@ -291,6 +291,7 @@ public class ReaderService : IReaderService _unitOfWork.AppUserProgressRepository.Update(userProgress); } + _logger.LogDebug("Saving Progress on Chapter {ChapterId} from Series {SeriesId} to {PageNum}", progressDto.ChapterId, progressDto.SeriesId, progressDto.PageNum); userProgress?.MarkModified(); if (!_unitOfWork.HasChanges() || await _unitOfWork.CommitAsync())