From 141d10e6da87af7bbe6177e8b87502e34ccd1bf7 Mon Sep 17 00:00:00 2001 From: Joseph Milazzo Date: Wed, 13 Jul 2022 15:19:00 -0400 Subject: [PATCH] Misc Bugfixes (#1373) * Fixed an issue where signalr cover update events would fire before the covers were updated in db and hence UI would show as if no cover for quite some time. * Refactored MetadataService to GenerateCovers, as that is what this service does. * Fixed a bug where list item for books that have 0.X series index wouldn't render on series detail. Added Name updates on volume on scan * Removed some debug code --- API/DTOs/VolumeDto.cs | 3 + API/Services/MetadataService.cs | 73 ++++++++++++------- API/Services/TaskScheduler.cs | 4 +- API/Services/Tasks/ScannerService.cs | 6 +- .../series-detail.component.html | 10 ++- .../app/shared/_services/download.service.ts | 6 +- 6 files changed, 66 insertions(+), 36 deletions(-) diff --git a/API/DTOs/VolumeDto.cs b/API/DTOs/VolumeDto.cs index 4136cf70c..5a20e61a5 100644 --- a/API/DTOs/VolumeDto.cs +++ b/API/DTOs/VolumeDto.cs @@ -1,6 +1,7 @@  using System; using System.Collections.Generic; +using API.Entities; using API.Entities.Interfaces; namespace API.DTOs @@ -8,7 +9,9 @@ namespace API.DTOs public class VolumeDto : IHasReadTimeEstimate { public int Id { get; set; } + /// public int Number { get; set; } + /// public string Name { get; set; } public int Pages { get; set; } public int PagesRead { get; set; } diff --git a/API/Services/MetadataService.cs b/API/Services/MetadataService.cs index 5f3f37d8b..6e943fc01 100644 --- a/API/Services/MetadataService.cs +++ b/API/Services/MetadataService.cs @@ -23,20 +23,20 @@ namespace API.Services; public interface IMetadataService { /// - /// Recalculates metadata for all entities in a library. + /// Recalculates cover images for all entities in a library. /// /// /// [DisableConcurrentExecution(timeoutInSeconds: 60 * 60 * 60)] - [AutomaticRetry(Attempts = 0, OnAttemptsExceeded = AttemptsExceededAction.Delete)] - Task RefreshMetadata(int libraryId, bool forceUpdate = false); + [AutomaticRetry(Attempts = 3, OnAttemptsExceeded = AttemptsExceededAction.Delete)] + Task GenerateCoversForLibrary(int libraryId, bool forceUpdate = false); /// - /// Performs a forced refresh of metadata just for a series and it's nested entities + /// Performs a forced refresh of cover images just for a series and it's nested entities /// /// /// /// Overrides any cache logic and forces execution - Task RefreshMetadataForSeries(int libraryId, int seriesId, bool forceUpdate = true); + Task GenerateCoversForSeries(int libraryId, int seriesId, bool forceUpdate = true); } public class MetadataService : IMetadataService @@ -48,6 +48,7 @@ public class MetadataService : IMetadataService private readonly IReadingItemService _readingItemService; private readonly IDirectoryService _directoryService; private readonly ChapterSortComparerZeroFirst _chapterSortComparerForInChapterSorting = new ChapterSortComparerZeroFirst(); + private IList _updateEvents = new List(); public MetadataService(IUnitOfWork unitOfWork, ILogger logger, IEventHub eventHub, ICacheHelper cacheHelper, IReadingItemService readingItemService, IDirectoryService directoryService) @@ -65,25 +66,27 @@ public class MetadataService : IMetadataService /// /// /// Force updating cover image even if underlying file has not been modified or chapter already has a cover image - private async Task UpdateChapterCoverImage(Chapter chapter, bool forceUpdate) + private Task UpdateChapterCoverImage(Chapter chapter, bool forceUpdate) { var firstFile = chapter.Files.MinBy(x => x.Chapter); if (!_cacheHelper.ShouldUpdateCoverImage(_directoryService.FileSystem.Path.Join(_directoryService.CoverImageDirectory, chapter.CoverImage), firstFile, chapter.Created, forceUpdate, chapter.CoverImageLocked)) - return false; + return Task.FromResult(false); - if (firstFile == null) return false; + if (firstFile == null) return Task.FromResult(false); _logger.LogDebug("[MetadataService] Generating cover image for {File}", firstFile.FilePath); chapter.CoverImage = _readingItemService.GetCoverImage(firstFile.FilePath, ImageService.GetChapterFormat(chapter.Id, chapter.VolumeId), firstFile.Format); - await _eventHub.SendMessageAsync(MessageFactory.CoverUpdate, - MessageFactory.CoverUpdateEvent(chapter.Id, MessageFactoryEntityTypes.Chapter), false); - return true; + + // await _eventHub.SendMessageAsync(MessageFactory.CoverUpdate, + // MessageFactory.CoverUpdateEvent(chapter.Id, MessageFactoryEntityTypes.Chapter), false); + _updateEvents.Add(MessageFactory.CoverUpdateEvent(chapter.Id, MessageFactoryEntityTypes.Chapter)); + return Task.FromResult(true); } private void UpdateChapterLastModified(Chapter chapter, bool forceUpdate) { - var firstFile = chapter.Files.OrderBy(x => x.Chapter).FirstOrDefault(); + var firstFile = chapter.Files.MinBy(x => x.Chapter); if (firstFile == null || _cacheHelper.HasFileNotChangedSinceCreationOrLastScan(chapter, forceUpdate, firstFile)) return; firstFile.UpdateLastModified(); @@ -94,22 +97,23 @@ public class MetadataService : IMetadataService /// /// /// Force updating cover image even if underlying file has not been modified or chapter already has a cover image - private async Task UpdateVolumeCoverImage(Volume volume, bool forceUpdate) + private Task UpdateVolumeCoverImage(Volume volume, bool forceUpdate) { // We need to check if Volume coverImage matches first chapters if forceUpdate is false if (volume == null || !_cacheHelper.ShouldUpdateCoverImage( _directoryService.FileSystem.Path.Join(_directoryService.CoverImageDirectory, volume.CoverImage), - null, volume.Created, forceUpdate)) return false; + null, volume.Created, forceUpdate)) return Task.FromResult(false); + volume.Chapters ??= new List(); var firstChapter = volume.Chapters.MinBy(x => double.Parse(x.Number), _chapterSortComparerForInChapterSorting); - if (firstChapter == null) return false; + if (firstChapter == null) return Task.FromResult(false); volume.CoverImage = firstChapter.CoverImage; - await _eventHub.SendMessageAsync(MessageFactory.CoverUpdate, MessageFactory.CoverUpdateEvent(volume.Id, MessageFactoryEntityTypes.Volume), false); + //await _eventHub.SendMessageAsync(MessageFactory.CoverUpdate, MessageFactory.CoverUpdateEvent(volume.Id, MessageFactoryEntityTypes.Volume), false); + _updateEvents.Add(MessageFactory.CoverUpdateEvent(volume.Id, MessageFactoryEntityTypes.Volume)); - - return true; + return Task.FromResult(true); } /// @@ -117,13 +121,13 @@ public class MetadataService : IMetadataService /// /// /// Force updating cover image even if underlying file has not been modified or chapter already has a cover image - private async Task UpdateSeriesCoverImage(Series series, bool forceUpdate) + private Task UpdateSeriesCoverImage(Series series, bool forceUpdate) { - if (series == null) return; + if (series == null) return Task.CompletedTask; if (!_cacheHelper.ShouldUpdateCoverImage(_directoryService.FileSystem.Path.Join(_directoryService.CoverImageDirectory, series.CoverImage), null, series.Created, forceUpdate, series.CoverImageLocked)) - return; + return Task.CompletedTask; series.Volumes ??= new List(); var firstCover = series.Volumes.GetCoverImage(series.Format); @@ -143,7 +147,9 @@ public class MetadataService : IMetadataService } } series.CoverImage = firstCover?.CoverImage ?? coverImage; - await _eventHub.SendMessageAsync(MessageFactory.CoverUpdate, MessageFactory.CoverUpdateEvent(series.Id, MessageFactoryEntityTypes.Series), false); + //await _eventHub.SendMessageAsync(MessageFactory.CoverUpdate, MessageFactory.CoverUpdateEvent(series.Id, MessageFactoryEntityTypes.Series), false); + _updateEvents.Add(MessageFactory.CoverUpdateEvent(series.Id, MessageFactoryEntityTypes.Series)); + return Task.CompletedTask; } @@ -194,18 +200,20 @@ public class MetadataService : IMetadataService /// - /// Refreshes Metadata for a whole library + /// Refreshes Cover Images for a whole library /// /// This can be heavy on memory first run /// /// Force updating cover image even if underlying file has not been modified or chapter already has a cover image [DisableConcurrentExecution(timeoutInSeconds: 60 * 60 * 60)] - [AutomaticRetry(Attempts = 0, OnAttemptsExceeded = AttemptsExceededAction.Delete)] - public async Task RefreshMetadata(int libraryId, bool forceUpdate = false) + [AutomaticRetry(Attempts = 3, OnAttemptsExceeded = AttemptsExceededAction.Delete)] + public async Task GenerateCoversForLibrary(int libraryId, bool forceUpdate = false) { var library = await _unitOfWork.LibraryRepository.GetLibraryForIdAsync(libraryId, LibraryIncludes.None); _logger.LogInformation("[MetadataService] Beginning metadata refresh of {LibraryName}", library.Name); + _updateEvents.Clear(); + var chunkInfo = await _unitOfWork.SeriesRepository.GetChunkInfo(library.Id); var stopwatch = Stopwatch.StartNew(); var totalTime = 0L; @@ -253,6 +261,8 @@ public class MetadataService : IMetadataService await _unitOfWork.CommitAsync(); + await FlushEvents(); + _logger.LogInformation( "[MetadataService] Processed {SeriesStart} - {SeriesEnd} out of {TotalSeries} series in {ElapsedScanTime} milliseconds for {LibraryName}", chunk * chunkInfo.ChunkSize, (chunk * chunkInfo.ChunkSize) + nonLibrarySeries.Count, chunkInfo.TotalSize, stopwatch.ElapsedMilliseconds, library.Name); @@ -280,7 +290,7 @@ public class MetadataService : IMetadataService /// /// /// Overrides any cache logic and forces execution - public async Task RefreshMetadataForSeries(int libraryId, int seriesId, bool forceUpdate = true) + public async Task GenerateCoversForSeries(int libraryId, int seriesId, bool forceUpdate = true) { var sw = Stopwatch.StartNew(); var series = await _unitOfWork.SeriesRepository.GetFullSeriesForSeriesIdAsync(seriesId); @@ -309,8 +319,19 @@ public class MetadataService : IMetadataService if (_unitOfWork.HasChanges() && await _unitOfWork.CommitAsync()) { await _eventHub.SendMessageAsync(MessageFactory.CoverUpdate, MessageFactory.CoverUpdateEvent(series.Id, MessageFactoryEntityTypes.Series), false); + await FlushEvents(); } _logger.LogInformation("[MetadataService] Updated metadata for {SeriesName} in {ElapsedMilliseconds} milliseconds", series.Name, sw.ElapsedMilliseconds); } + + private async Task FlushEvents() + { + // Send all events out now that entities are saved + foreach (var updateEvent in _updateEvents) + { + await _eventHub.SendMessageAsync(MessageFactory.CoverUpdate, updateEvent, false); + } + _updateEvents.Clear(); + } } diff --git a/API/Services/TaskScheduler.cs b/API/Services/TaskScheduler.cs index 3a3498a40..e9030b969 100644 --- a/API/Services/TaskScheduler.cs +++ b/API/Services/TaskScheduler.cs @@ -188,7 +188,7 @@ public class TaskScheduler : ITaskScheduler } _logger.LogInformation("Enqueuing library metadata refresh for: {LibraryId}", libraryId); - BackgroundJob.Enqueue(() => _metadataService.RefreshMetadata(libraryId, forceUpdate)); + BackgroundJob.Enqueue(() => _metadataService.GenerateCoversForLibrary(libraryId, forceUpdate)); } public void RefreshSeriesMetadata(int libraryId, int seriesId, bool forceUpdate = false) @@ -200,7 +200,7 @@ public class TaskScheduler : ITaskScheduler } _logger.LogInformation("Enqueuing series metadata refresh for: {SeriesId}", seriesId); - BackgroundJob.Enqueue(() => _metadataService.RefreshMetadataForSeries(libraryId, seriesId, forceUpdate)); + BackgroundJob.Enqueue(() => _metadataService.GenerateCoversForSeries(libraryId, seriesId, forceUpdate)); } public void ScanSeries(int libraryId, int seriesId, bool forceUpdate = false) diff --git a/API/Services/Tasks/ScannerService.cs b/API/Services/Tasks/ScannerService.cs index 31f07fff8..abcb8e6b9 100644 --- a/API/Services/Tasks/ScannerService.cs +++ b/API/Services/Tasks/ScannerService.cs @@ -192,7 +192,7 @@ public class ScannerService : IScannerService await CleanupDbEntities(); BackgroundJob.Enqueue(() => _cacheService.CleanupChapters(chapterIds)); BackgroundJob.Enqueue(() => _directoryService.ClearDirectory(_directoryService.TempDirectory)); - BackgroundJob.Enqueue(() => _metadataService.RefreshMetadataForSeries(libraryId, series.Id, false)); + BackgroundJob.Enqueue(() => _metadataService.GenerateCoversForSeries(libraryId, series.Id, false)); BackgroundJob.Enqueue(() => _wordCountAnalyzerService.ScanSeries(libraryId, series.Id, false)); } @@ -327,7 +327,7 @@ public class ScannerService : IScannerService await CleanupDbEntities(); - BackgroundJob.Enqueue(() => _metadataService.RefreshMetadata(libraryId, false)); + BackgroundJob.Enqueue(() => _metadataService.GenerateCoversForLibrary(libraryId, false)); BackgroundJob.Enqueue(() => _wordCountAnalyzerService.ScanLibrary(libraryId, false)); BackgroundJob.Enqueue(() => _directoryService.ClearDirectory(_directoryService.TempDirectory)); } @@ -804,6 +804,8 @@ public class ScannerService : IScannerService _unitOfWork.VolumeRepository.Add(volume); } + volume.Name = volumeNumber; + _logger.LogDebug("[ScannerService] Parsing {SeriesName} - Volume {VolumeNumber}", series.Name, volume.Name); var infos = parsedInfos.Where(p => p.Volumes == volumeNumber).ToArray(); UpdateChapters(series, volume, infos); diff --git a/UI/Web/src/app/series-detail/series-detail.component.html b/UI/Web/src/app/series-detail/series-detail.component.html index 85203f0a3..2a8dfd361 100644 --- a/UI/Web/src/app/series-detail/series-detail.component.html +++ b/UI/Web/src/app/series-detail/series-detail.component.html @@ -185,7 +185,7 @@ @@ -194,7 +194,6 @@ - @@ -203,7 +202,7 @@
  • {{utilityService.formatChapterName(libraryType) + 's'}} - +
    @@ -227,7 +226,7 @@ [pagesRead]="chapter.pagesRead" [totalPages]="chapter.pages" (read)="openChapter(chapter)" [includeVolume]="true" [blur]="user?.preferences?.blurUnreadSummaries || false"> - +
    @@ -262,6 +261,9 @@ {{chapter.title || chapter.range}} + diff --git a/UI/Web/src/app/shared/_services/download.service.ts b/UI/Web/src/app/shared/_services/download.service.ts index f0d6ffcd1..a6e26dc5f 100644 --- a/UI/Web/src/app/shared/_services/download.service.ts +++ b/UI/Web/src/app/shared/_services/download.service.ts @@ -120,9 +120,11 @@ export class DownloadService { } return of(0); }), switchMap(async (size) => { - return await this.confirmSize(size, entityType); + return await this.confirmSize(size, entityType); }) - ).pipe(filter(wantsToDownload => wantsToDownload), switchMap(() => { + ).pipe(filter(wantsToDownload => { + return wantsToDownload; + }), switchMap(() => { return downloadCall.pipe( tap((d) => { if (callback) callback(d);