From 67ba5e302f3b59b39813862950647e033559d46b Mon Sep 17 00:00:00 2001 From: Joseph Milazzo Date: Mon, 7 Feb 2022 17:44:06 -0800 Subject: [PATCH] Refactored the way cover images are updated from SignalR to use an explicit event that is sent at a granular level for a given type of entity. (#1046) Fixed a bad event listener for RefreshMetadata (now removed) to update metadata on Series Detail. Now uses ScanService, which indicates a series has completed a scan. --- API/Controllers/CollectionController.cs | 7 +- API/Services/MetadataService.cs | 99 +++++-------------- API/Services/Tasks/ScannerService.cs | 5 + API/SignalR/MessageFactory.cs | 28 +++--- API/SignalR/SignalREvents.cs | 15 ++- .../app/_models/events/cover-update-event.ts | 7 ++ UI/Web/src/app/_services/image.service.ts | 2 +- .../src/app/_services/message-hub.service.ts | 21 ++-- .../series-card/series-card.component.ts | 8 +- UI/Web/src/app/library/library.component.ts | 1 + .../series-detail/series-detail.component.ts | 9 +- .../src/app/shared/image/image.component.ts | 23 ++++- 12 files changed, 108 insertions(+), 117 deletions(-) create mode 100644 UI/Web/src/app/_models/events/cover-update-event.ts diff --git a/API/Controllers/CollectionController.cs b/API/Controllers/CollectionController.cs index 883c92bef..0c8738229 100644 --- a/API/Controllers/CollectionController.cs +++ b/API/Controllers/CollectionController.cs @@ -6,8 +6,10 @@ using API.Data; using API.DTOs.CollectionTags; using API.Entities.Metadata; using API.Extensions; +using API.SignalR; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.SignalR; namespace API.Controllers { @@ -17,11 +19,13 @@ namespace API.Controllers public class CollectionController : BaseApiController { private readonly IUnitOfWork _unitOfWork; + private readonly IHubContext _messageHub; /// - public CollectionController(IUnitOfWork unitOfWork) + public CollectionController(IUnitOfWork unitOfWork, IHubContext messageHub) { _unitOfWork = unitOfWork; + _messageHub = messageHub; } /// @@ -152,6 +156,7 @@ namespace API.Controllers { tag.CoverImageLocked = false; tag.CoverImage = string.Empty; + await _messageHub.Clients.All.SendAsync(SignalREvents.CoverUpdate, MessageFactory.CoverUpdateEvent(tag.Id, "collection")); _unitOfWork.CollectionTagRepository.Update(tag); } diff --git a/API/Services/MetadataService.cs b/API/Services/MetadataService.cs index ed446ec19..29bdd7ca3 100644 --- a/API/Services/MetadataService.cs +++ b/API/Services/MetadataService.cs @@ -58,7 +58,7 @@ public class MetadataService : IMetadataService /// /// /// Force updating cover image even if underlying file has not been modified or chapter already has a cover image - private bool UpdateChapterCoverImage(Chapter chapter, bool forceUpdate) + private async Task UpdateChapterCoverImage(Chapter chapter, bool forceUpdate) { var firstFile = chapter.Files.OrderBy(x => x.Chapter).FirstOrDefault(); @@ -69,6 +69,7 @@ public class MetadataService : IMetadataService _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 _messageHub.Clients.All.SendAsync(SignalREvents.CoverUpdate, MessageFactory.CoverUpdateEvent(chapter.Id, "chapter")); return true; } @@ -86,7 +87,7 @@ public class MetadataService : IMetadataService /// /// /// Force updating cover image even if underlying file has not been modified or chapter already has a cover image - private bool UpdateVolumeCoverImage(Volume volume, bool forceUpdate) + private async 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( @@ -98,6 +99,8 @@ public class MetadataService : IMetadataService if (firstChapter == null) return false; volume.CoverImage = firstChapter.CoverImage; + await _messageHub.Clients.All.SendAsync(SignalREvents.CoverUpdate, MessageFactory.CoverUpdateEvent(volume.Id, "volume")); + return true; } @@ -106,7 +109,7 @@ public class MetadataService : IMetadataService /// /// /// Force updating cover image even if underlying file has not been modified or chapter already has a cover image - private void UpdateSeriesCoverImage(Series series, bool forceUpdate) + private async Task UpdateSeriesCoverImage(Series series, bool forceUpdate) { if (series == null) return; @@ -133,6 +136,7 @@ public class MetadataService : IMetadataService } } series.CoverImage = firstCover?.CoverImage ?? coverImage; + await _messageHub.Clients.All.SendAsync(SignalREvents.CoverUpdate, MessageFactory.CoverUpdateEvent(series.Id, "series")); } @@ -141,7 +145,7 @@ public class MetadataService : IMetadataService /// /// /// - private void ProcessSeriesMetadataUpdate(Series series, bool forceUpdate) + private async Task ProcessSeriesMetadataUpdate(Series series, bool forceUpdate) { _logger.LogDebug("[MetadataService] Processing series {SeriesName}", series.OriginalName); try @@ -154,7 +158,7 @@ public class MetadataService : IMetadataService var index = 0; foreach (var chapter in volume.Chapters) { - var chapterUpdated = UpdateChapterCoverImage(chapter, forceUpdate); + var chapterUpdated = await UpdateChapterCoverImage(chapter, forceUpdate); // If cover was update, either the file has changed or first scan and we should force a metadata update UpdateChapterLastModified(chapter, forceUpdate || chapterUpdated); if (index == 0 && chapterUpdated) @@ -165,7 +169,7 @@ public class MetadataService : IMetadataService index++; } - var volumeUpdated = UpdateVolumeCoverImage(volume, firstChapterUpdated || forceUpdate); + var volumeUpdated = await UpdateVolumeCoverImage(volume, firstChapterUpdated || forceUpdate); if (volumeIndex == 0 && volumeUpdated) { firstVolumeUpdated = true; @@ -173,7 +177,7 @@ public class MetadataService : IMetadataService volumeIndex++; } - UpdateSeriesCoverImage(series, firstVolumeUpdated || forceUpdate); + await UpdateSeriesCoverImage(series, firstVolumeUpdated || forceUpdate); } catch (Exception ex) { @@ -222,7 +226,7 @@ public class MetadataService : IMetadataService { try { - ProcessSeriesMetadataUpdate(series, forceUpdate); + await ProcessSeriesMetadataUpdate(series, forceUpdate); } catch (Exception ex) { @@ -237,10 +241,11 @@ public class MetadataService : IMetadataService } await _unitOfWork.CommitAsync(); - foreach (var series in nonLibrarySeries) - { - await _messageHub.Clients.All.SendAsync(SignalREvents.RefreshMetadata, MessageFactory.RefreshMetadataEvent(library.Id, series.Id)); - } + // foreach (var series in nonLibrarySeries) + // { + // // TODO: This can be removed, we use CoverUpdate elsewhere + // await _messageHub.Clients.All.SendAsync(SignalREvents.RefreshMetadata, MessageFactory.RefreshMetadataEvent(library.Id, series.Id)); + // } _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); @@ -262,62 +267,6 @@ public class MetadataService : IMetadataService await _unitOfWork.GenreRepository.RemoveAllGenreNoLongerAssociated(); } - // TODO: Write out a single piece of code that can iterate over a collection/chunk and perform custom actions - private async Task PerformScan(Library library, bool forceUpdate, Action action) - { - var chunkInfo = await _unitOfWork.SeriesRepository.GetChunkInfo(library.Id); - var stopwatch = Stopwatch.StartNew(); - _logger.LogInformation("[MetadataService] Refreshing Library {LibraryName}. Total Items: {TotalSize}. Total Chunks: {TotalChunks} with {ChunkSize} size", library.Name, chunkInfo.TotalSize, chunkInfo.TotalChunks, chunkInfo.ChunkSize); - await _messageHub.Clients.All.SendAsync(SignalREvents.RefreshMetadataProgress, - MessageFactory.RefreshMetadataProgressEvent(library.Id, 0F)); - - for (var chunk = 1; chunk <= chunkInfo.TotalChunks; chunk++) - { - if (chunkInfo.TotalChunks == 0) continue; - stopwatch.Restart(); - - action(chunk, chunkInfo); - - // _logger.LogInformation("[MetadataService] Processing chunk {ChunkNumber} / {TotalChunks} with size {ChunkSize}. Series ({SeriesStart} - {SeriesEnd}", - // chunk, chunkInfo.TotalChunks, chunkInfo.ChunkSize, chunk * chunkInfo.ChunkSize, (chunk + 1) * chunkInfo.ChunkSize); - // var nonLibrarySeries = await _unitOfWork.SeriesRepository.GetFullSeriesForLibraryIdAsync(library.Id, - // new UserParams() - // { - // PageNumber = chunk, - // PageSize = chunkInfo.ChunkSize - // }); - // _logger.LogDebug("[MetadataService] Fetched {SeriesCount} series for refresh", nonLibrarySeries.Count); - // - // var chapterIds = await _unitOfWork.SeriesRepository.GetChapterIdWithSeriesIdForSeriesAsync(nonLibrarySeries.Select(s => s.Id).ToArray()); - // var allPeople = await _unitOfWork.PersonRepository.GetAllPeople(); - // var allGenres = await _unitOfWork.GenreRepository.GetAllGenres(); - // - // - // var seriesIndex = 0; - // foreach (var series in nonLibrarySeries) - // { - // try - // { - // ProcessSeriesMetadataUpdate(series, chapterIds, allPeople, allGenres, forceUpdate); - // } - // catch (Exception ex) - // { - // _logger.LogError(ex, "[MetadataService] There was an exception during metadata refresh for {SeriesName}", series.Name); - // } - // var index = chunk * seriesIndex; - // var progress = Math.Max(0F, Math.Min(1F, index * 1F / chunkInfo.TotalSize)); - // - // await _messageHub.Clients.All.SendAsync(SignalREvents.RefreshMetadataProgress, - // MessageFactory.RefreshMetadataProgressEvent(library.Id, progress)); - // seriesIndex++; - // } - - await _unitOfWork.CommitAsync(); - } - } - - - /// /// Refreshes Metadata for a Series. Will always force updates. /// @@ -336,17 +285,17 @@ public class MetadataService : IMetadataService await _messageHub.Clients.All.SendAsync(SignalREvents.RefreshMetadataProgress, MessageFactory.RefreshMetadataProgressEvent(libraryId, 0F)); - ProcessSeriesMetadataUpdate(series, forceUpdate); + await ProcessSeriesMetadataUpdate(series, forceUpdate); + + + if (_unitOfWork.HasChanges()) + { + await _unitOfWork.CommitAsync(); + } await _messageHub.Clients.All.SendAsync(SignalREvents.RefreshMetadataProgress, MessageFactory.RefreshMetadataProgressEvent(libraryId, 1F)); - - if (_unitOfWork.HasChanges() && await _unitOfWork.CommitAsync()) - { - await _messageHub.Clients.All.SendAsync(SignalREvents.RefreshMetadata, MessageFactory.RefreshMetadataEvent(series.LibraryId, series.Id)); - } - await RemoveAbandonedMetadataKeys(); _logger.LogInformation("[MetadataService] Updated metadata for {SeriesName} in {ElapsedMilliseconds} milliseconds", series.Name, sw.ElapsedMilliseconds); diff --git a/API/Services/Tasks/ScannerService.cs b/API/Services/Tasks/ScannerService.cs index 374bffb3d..2e9bc58ff 100644 --- a/API/Services/Tasks/ScannerService.cs +++ b/API/Services/Tasks/ScannerService.cs @@ -379,6 +379,11 @@ public class ScannerService : IScannerService await _messageHub.Clients.All.SendAsync(SignalREvents.SeriesRemoved, MessageFactory.SeriesRemovedEvent(missing.Id, missing.Name, library.Id)); } + foreach (var series in librarySeries) + { + await _messageHub.Clients.All.SendAsync(SignalREvents.ScanSeries, MessageFactory.ScanSeriesEvent(series.Id, series.Name)); + } + var progress = Math.Max(0, Math.Min(1, ((chunk + 1F) * chunkInfo.ChunkSize) / chunkInfo.TotalSize)); await _messageHub.Clients.All.SendAsync(SignalREvents.ScanLibraryProgress, MessageFactory.ScanLibraryProgressEvent(library.Id, progress)); diff --git a/API/SignalR/MessageFactory.cs b/API/SignalR/MessageFactory.cs index 25262430a..385f42437 100644 --- a/API/SignalR/MessageFactory.cs +++ b/API/SignalR/MessageFactory.cs @@ -1,4 +1,5 @@ using System; +using System.Threading; using API.DTOs.Update; namespace API.SignalR @@ -75,20 +76,6 @@ namespace API.SignalR } - - public static SignalRMessage RefreshMetadataEvent(int libraryId, int seriesId) - { - return new SignalRMessage() - { - Name = SignalREvents.RefreshMetadata, - Body = new - { - SeriesId = seriesId, - LibraryId = libraryId - } - }; - } - public static SignalRMessage BackupDatabaseProgressEvent(float progress) { return new SignalRMessage() @@ -161,5 +148,18 @@ namespace API.SignalR } }; } + + public static SignalRMessage CoverUpdateEvent(int id, string entityType) + { + return new SignalRMessage() + { + Name = SignalREvents.CoverUpdate, + Body = new + { + Id = id, + EntityType = entityType, + } + }; + } } } diff --git a/API/SignalR/SignalREvents.cs b/API/SignalR/SignalREvents.cs index 15590f426..1da613455 100644 --- a/API/SignalR/SignalREvents.cs +++ b/API/SignalR/SignalREvents.cs @@ -2,12 +2,14 @@ { public static class SignalREvents { - public const string UpdateAvailable = "UpdateAvailable"; - public const string ScanSeries = "ScanSeries"; /// - /// Event during Refresh Metadata for cover image change + /// An update is available for the Kavita instance /// - public const string RefreshMetadata = "RefreshMetadata"; + public const string UpdateAvailable = "UpdateAvailable"; + /// + /// Used to tell when a scan series completes + /// + public const string ScanSeries = "ScanSeries"; /// /// Event sent out during Refresh Metadata for progress tracking /// @@ -48,6 +50,9 @@ /// Event sent out during downloading of files /// public const string DownloadProgress = "DownloadProgress"; - + /// + /// A cover was updated + /// + public const string CoverUpdate = "CoverUpdate"; } } diff --git a/UI/Web/src/app/_models/events/cover-update-event.ts b/UI/Web/src/app/_models/events/cover-update-event.ts new file mode 100644 index 000000000..d15fab1a1 --- /dev/null +++ b/UI/Web/src/app/_models/events/cover-update-event.ts @@ -0,0 +1,7 @@ +/** + * Represents a generic cover update event. Id is used based on entityType + */ +export interface CoverUpdateEvent { + id: number; + entityType: 'series' | 'chapter' | 'volume' | 'collection'; +} \ No newline at end of file diff --git a/UI/Web/src/app/_services/image.service.ts b/UI/Web/src/app/_services/image.service.ts index b45896206..de1ea19e9 100644 --- a/UI/Web/src/app/_services/image.service.ts +++ b/UI/Web/src/app/_services/image.service.ts @@ -79,7 +79,7 @@ export class ImageService implements OnDestroy { * @returns Url with a random parameter attached */ randomize(url: string) { - const r = Math.random() * 100 + 1; + const r = Math.round(Math.random() * 100 + 1); if (url.indexOf('&random') >= 0) { return url + 1; } diff --git a/UI/Web/src/app/_services/message-hub.service.ts b/UI/Web/src/app/_services/message-hub.service.ts index bd0994f29..7547bf243 100644 --- a/UI/Web/src/app/_services/message-hub.service.ts +++ b/UI/Web/src/app/_services/message-hub.service.ts @@ -15,7 +15,6 @@ import { User } from '../_models/user'; export enum EVENTS { UpdateAvailable = 'UpdateAvailable', ScanSeries = 'ScanSeries', - RefreshMetadata = 'RefreshMetadata', RefreshMetadataProgress = 'RefreshMetadataProgress', SeriesAdded = 'SeriesAdded', SeriesRemoved = 'SeriesRemoved', @@ -25,7 +24,11 @@ export enum EVENTS { ScanLibraryError = 'ScanLibraryError', BackupDatabaseProgress = 'BackupDatabaseProgress', CleanupProgress = 'CleanupProgress', - DownloadProgress = 'DownloadProgress' + DownloadProgress = 'DownloadProgress', + /** + * A cover is updated + */ + CoverUpdate = 'CoverUpdate' } export interface Message { @@ -49,7 +52,6 @@ export class MessageHubService { public scanSeries: EventEmitter = new EventEmitter(); public scanLibrary: EventEmitter = new EventEmitter(); // TODO: Refactor this name to be generic public seriesAdded: EventEmitter = new EventEmitter(); - public refreshMetadata: EventEmitter = new EventEmitter(); isAdmin: boolean = false; @@ -152,12 +154,19 @@ export class MessageHubService { }); }); - this.hubConnection.on(EVENTS.RefreshMetadata, resp => { + // this.hubConnection.on(EVENTS.RefreshMetadata, resp => { + // this.messagesSource.next({ + // event: EVENTS.RefreshMetadata, + // payload: resp.body + // }); + // this.refreshMetadata.emit(resp.body); // TODO: Remove this + // }); + + this.hubConnection.on(EVENTS.CoverUpdate, resp => { this.messagesSource.next({ - event: EVENTS.RefreshMetadata, + event: EVENTS.CoverUpdate, payload: resp.body }); - this.refreshMetadata.emit(resp.body); }); this.hubConnection.on(EVENTS.UpdateAvailable, resp => { diff --git a/UI/Web/src/app/cards/series-card/series-card.component.ts b/UI/Web/src/app/cards/series-card/series-card.component.ts index 8966cc1be..c9acf96d3 100644 --- a/UI/Web/src/app/cards/series-card/series-card.component.ts +++ b/UI/Web/src/app/cards/series-card/series-card.component.ts @@ -61,13 +61,7 @@ export class SeriesCardComponent implements OnInit, OnChanges, OnDestroy { ngOnInit(): void { if (this.data) { - this.imageUrl = this.imageService.randomize(this.imageService.getSeriesCoverImage(this.data.id)); - - this.hubService.refreshMetadata.pipe(takeWhile(event => event.libraryId === this.libraryId), takeUntil(this.onDestroy)).subscribe((event: RefreshMetadataEvent) => { - if (this.data.id === event.seriesId) { - this.imageUrl = this.imageService.randomize(this.imageService.getSeriesCoverImage(this.data.id)); - } - }); + this.imageUrl = this.imageService.getSeriesCoverImage(this.data.id); } } diff --git a/UI/Web/src/app/library/library.component.ts b/UI/Web/src/app/library/library.component.ts index b93d27d8f..64c08d35b 100644 --- a/UI/Web/src/app/library/library.component.ts +++ b/UI/Web/src/app/library/library.component.ts @@ -3,6 +3,7 @@ import { Title } from '@angular/platform-browser'; import { Router } from '@angular/router'; import { Subject } from 'rxjs'; import { take, takeUntil } from 'rxjs/operators'; +import { RefreshMetadataEvent } from '../_models/events/refresh-metadata-event'; import { SeriesAddedEvent } from '../_models/events/series-added-event'; import { SeriesRemovedEvent } from '../_models/events/series-removed-event'; import { Library } from '../_models/library'; diff --git a/UI/Web/src/app/series-detail/series-detail.component.ts b/UI/Web/src/app/series-detail/series-detail.component.ts index 86da2b219..d2462ffc2 100644 --- a/UI/Web/src/app/series-detail/series-detail.component.ts +++ b/UI/Web/src/app/series-detail/series-detail.component.ts @@ -200,13 +200,14 @@ export class SeriesDetailComponent implements OnInit, OnDestroy { this.toastr.info('This series no longer exists'); this.router.navigateByUrl('/libraries'); } - } else if (event.event === EVENTS.RefreshMetadata) { - const seriesRemovedEvent = event.payload as RefreshMetadataEvent; - if (seriesRemovedEvent.seriesId === this.series.id) { + } else if (event.event === EVENTS.ScanSeries) { + const seriesCoverUpdatedEvent = event.payload as ScanSeriesEvent; + if (seriesCoverUpdatedEvent.seriesId === this.series.id) { + console.log('ScanSeries called') this.seriesService.getMetadata(this.series.id).pipe(take(1)).subscribe(metadata => { this.seriesMetadata = metadata; this.createHTML(); - }) + }); } } }); diff --git a/UI/Web/src/app/shared/image/image.component.ts b/UI/Web/src/app/shared/image/image.component.ts index 9cc8c8113..f2cc07347 100644 --- a/UI/Web/src/app/shared/image/image.component.ts +++ b/UI/Web/src/app/shared/image/image.component.ts @@ -1,5 +1,8 @@ -import { Component, ElementRef, Input, OnChanges, OnInit, Renderer2, SimpleChanges, ViewChild } from '@angular/core'; +import { Component, ElementRef, Input, OnChanges, OnDestroy, OnInit, Renderer2, SimpleChanges, ViewChild } from '@angular/core'; +import { Subject } from 'rxjs'; +import { takeUntil } from 'rxjs/operators'; import { ImageService } from 'src/app/_services/image.service'; +import { EVENTS, MessageHubService } from 'src/app/_services/message-hub.service'; /** * This is used for images with placeholder fallback. @@ -9,7 +12,7 @@ import { ImageService } from 'src/app/_services/image.service'; templateUrl: './image.component.html', styleUrls: ['./image.component.scss'] }) -export class ImageComponent implements OnChanges { +export class ImageComponent implements OnChanges, OnDestroy { /** * Source url to load image @@ -38,7 +41,15 @@ export class ImageComponent implements OnChanges { @ViewChild('img', {static: true}) imgElem!: ElementRef; - constructor(public imageService: ImageService, private renderer: Renderer2) { } + private readonly onDestroy = new Subject(); + + constructor(public imageService: ImageService, private renderer: Renderer2, private hubService: MessageHubService) { + this.hubService.messages$.pipe(takeUntil(this.onDestroy)).subscribe(res => { + if (res.event === EVENTS.CoverUpdate) { + this.imageUrl = this.imageService.randomize(this.imageUrl); + } + }); + } ngOnChanges(changes: SimpleChanges): void { if (this.width != '') { @@ -60,7 +71,11 @@ export class ImageComponent implements OnChanges { if (this.borderRadius != '') { this.renderer.setStyle(this.imgElem.nativeElement, 'border-radius', this.borderRadius); } - + } + + ngOnDestroy() { + this.onDestroy.next(); + this.onDestroy.complete(); } }