From 2982478dba42d9e38fd9b63ddeaa5101426c613b Mon Sep 17 00:00:00 2001 From: ta264 Date: Wed, 30 Sep 2020 22:37:45 +0100 Subject: [PATCH] Trigger fewer signalr broadcasts (cherry picked from commit b05b7ec4ad9368c8c3ae5ff5316caf5d23e24191) --- .../AlbumMonitoredServiceFixture.cs | 4 +-- .../MusicTests/MoveArtistServiceFixture.cs | 2 +- .../MusicTests/RefreshArtistServiceFixture.cs | 28 ++++++++-------- .../MediaCover/MediaCoverService.cs | 32 ++++++++++++++----- .../Music/Services/ArtistService.cs | 10 ++++-- .../Music/Services/RefreshAlbumService.cs | 6 ++-- .../Music/Services/RefreshArtistService.cs | 2 +- 7 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/NzbDrone.Core.Test/MusicTests/AlbumMonitoredServiceTests/AlbumMonitoredServiceFixture.cs b/src/NzbDrone.Core.Test/MusicTests/AlbumMonitoredServiceTests/AlbumMonitoredServiceFixture.cs index 641b87f7b..2dc67fff2 100644 --- a/src/NzbDrone.Core.Test/MusicTests/AlbumMonitoredServiceTests/AlbumMonitoredServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/AlbumMonitoredServiceTests/AlbumMonitoredServiceFixture.cs @@ -58,7 +58,7 @@ namespace NzbDrone.Core.Test.MusicTests.AlbumMonitoredServiceTests Subject.SetAlbumMonitoredStatus(_artist, null); Mocker.GetMock() - .Verify(v => v.UpdateArtist(It.IsAny()), Times.Once()); + .Verify(v => v.UpdateArtist(It.IsAny(), It.IsAny()), Times.Once()); Mocker.GetMock() .Verify(v => v.UpdateMany(It.IsAny>()), Times.Never()); @@ -72,7 +72,7 @@ namespace NzbDrone.Core.Test.MusicTests.AlbumMonitoredServiceTests Subject.SetAlbumMonitoredStatus(_artist, new MonitoringOptions { Monitored = true, AlbumsToMonitor = albumsToMonitor }); Mocker.GetMock() - .Verify(v => v.UpdateArtist(It.IsAny()), Times.Once()); + .Verify(v => v.UpdateArtist(It.IsAny(), It.IsAny()), Times.Once()); VerifyMonitored(e => e.ForeignAlbumId == _albums.First().ForeignAlbumId); VerifyNotMonitored(e => e.ForeignAlbumId != _albums.First().ForeignAlbumId); diff --git a/src/NzbDrone.Core.Test/MusicTests/MoveArtistServiceFixture.cs b/src/NzbDrone.Core.Test/MusicTests/MoveArtistServiceFixture.cs index 0b58817cd..58a3e7df0 100644 --- a/src/NzbDrone.Core.Test/MusicTests/MoveArtistServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/MoveArtistServiceFixture.cs @@ -87,7 +87,7 @@ namespace NzbDrone.Core.Test.MusicTests ExceptionVerification.ExpectedErrors(1); Mocker.GetMock() - .Verify(v => v.UpdateArtist(It.IsAny()), Times.Once()); + .Verify(v => v.UpdateArtist(It.IsAny(), It.IsAny()), Times.Once()); } [Test] diff --git a/src/NzbDrone.Core.Test/MusicTests/RefreshArtistServiceFixture.cs b/src/NzbDrone.Core.Test/MusicTests/RefreshArtistServiceFixture.cs index c3f384ae6..59a2fb129 100644 --- a/src/NzbDrone.Core.Test/MusicTests/RefreshArtistServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/RefreshArtistServiceFixture.cs @@ -100,8 +100,8 @@ namespace NzbDrone.Core.Test.MusicTests private void AllowArtistUpdate() { Mocker.GetMock(MockBehavior.Strict) - .Setup(x => x.UpdateArtist(It.IsAny())) - .Returns((Artist a) => a); + .Setup(x => x.UpdateArtist(It.IsAny(), It.IsAny())) + .Returns((Artist a, bool updated) => a); } [Test] @@ -151,7 +151,7 @@ namespace NzbDrone.Core.Test.MusicTests Subject.Execute(new RefreshArtistCommand(_artist.Id)); Mocker.GetMock() - .Verify(v => v.UpdateArtist(It.IsAny()), Times.Never()); + .Verify(v => v.UpdateArtist(It.IsAny(), It.IsAny()), Times.Never()); Mocker.GetMock() .Verify(v => v.DeleteArtist(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -169,7 +169,7 @@ namespace NzbDrone.Core.Test.MusicTests Subject.Execute(new RefreshArtistCommand(_artist.Id)); Mocker.GetMock() - .Verify(v => v.UpdateArtist(It.IsAny()), Times.Never()); + .Verify(v => v.UpdateArtist(It.IsAny(), It.IsAny()), Times.Never()); Mocker.GetMock() .Verify(v => v.DeleteArtist(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); @@ -197,8 +197,8 @@ namespace NzbDrone.Core.Test.MusicTests // Make sure that the artist is updated before we refresh the albums Mocker.GetMock(MockBehavior.Strict) .InSequence(seq) - .Setup(x => x.UpdateArtist(It.IsAny())) - .Returns((Artist a) => a); + .Setup(x => x.UpdateArtist(It.IsAny(), It.IsAny())) + .Returns((Artist a, bool updated) => a); Mocker.GetMock(MockBehavior.Strict) .InSequence(seq) @@ -208,13 +208,13 @@ namespace NzbDrone.Core.Test.MusicTests // Update called twice for a move/merge Mocker.GetMock(MockBehavior.Strict) .InSequence(seq) - .Setup(x => x.UpdateArtist(It.IsAny())) - .Returns((Artist a) => a); + .Setup(x => x.UpdateArtist(It.IsAny(), It.IsAny())) + .Returns((Artist a, bool updated) => a); Subject.Execute(new RefreshArtistCommand(_artist.Id)); Mocker.GetMock() - .Verify(v => v.UpdateArtist(It.Is(s => s.ArtistMetadataId == 100 && s.ForeignArtistId == newArtistInfo.ForeignArtistId)), + .Verify(v => v.UpdateArtist(It.Is(s => s.ArtistMetadataId == 100 && s.ForeignArtistId == newArtistInfo.ForeignArtistId), It.IsAny()), Times.Exactly(2)); } @@ -257,8 +257,8 @@ namespace NzbDrone.Core.Test.MusicTests Mocker.GetMock(MockBehavior.Strict) .InSequence(seq) - .Setup(x => x.UpdateArtist(It.Is(a => a.Id == clash.Id))) - .Returns((Artist a) => a); + .Setup(x => x.UpdateArtist(It.Is(a => a.Id == clash.Id), It.IsAny())) + .Returns((Artist a, bool updated) => a); Mocker.GetMock(MockBehavior.Strict) .InSequence(seq) @@ -268,14 +268,14 @@ namespace NzbDrone.Core.Test.MusicTests // Update called twice for a move/merge Mocker.GetMock(MockBehavior.Strict) .InSequence(seq) - .Setup(x => x.UpdateArtist(It.IsAny())) - .Returns((Artist a) => a); + .Setup(x => x.UpdateArtist(It.IsAny(), It.IsAny())) + .Returns((Artist a, bool updated) => a); Subject.Execute(new RefreshArtistCommand(_artist.Id)); // the retained artist gets updated Mocker.GetMock() - .Verify(v => v.UpdateArtist(It.Is(s => s.Id == clash.Id)), Times.Exactly(2)); + .Verify(v => v.UpdateArtist(It.Is(s => s.Id == clash.Id), It.IsAny()), Times.Exactly(2)); // the old one gets removed Mocker.GetMock() diff --git a/src/NzbDrone.Core/MediaCover/MediaCoverService.cs b/src/NzbDrone.Core/MediaCover/MediaCoverService.cs index 9c61b4368..ed3a107bb 100644 --- a/src/NzbDrone.Core/MediaCover/MediaCoverService.cs +++ b/src/NzbDrone.Core/MediaCover/MediaCoverService.cs @@ -20,7 +20,7 @@ namespace NzbDrone.Core.MediaCover { void ConvertToLocalUrls(int entityId, MediaCoverEntity coverEntity, IEnumerable covers); string GetCoverPath(int entityId, MediaCoverEntity coverEntity, MediaCoverTypes mediaCoverTypes, string extension, int? height = null); - void EnsureAlbumCovers(Album album); + bool EnsureAlbumCovers(Album album); } public class MediaCoverService : @@ -114,8 +114,9 @@ namespace NzbDrone.Core.MediaCover return Path.Combine(_coverRootFolder, "Albums", albumId.ToString()); } - private void EnsureArtistCovers(Artist artist) + private bool EnsureArtistCovers(Artist artist) { + bool updated = false; var toResize = new List>(); foreach (var cover in artist.Metadata.Value.Images) @@ -132,6 +133,7 @@ namespace NzbDrone.Core.MediaCover if (!alreadyExists) { DownloadCover(artist, cover, serverFileHeaders.LastModified ?? DateTime.Now); + updated = true; } } catch (WebException e) @@ -159,10 +161,14 @@ namespace NzbDrone.Core.MediaCover { _semaphore.Release(); } + + return updated; } - public void EnsureAlbumCovers(Album album) + public bool EnsureAlbumCovers(Album album) { + bool updated = false; + foreach (var cover in album.Images.Where(e => e.CoverType == MediaCoverTypes.Cover)) { var fileName = GetCoverPath(album.Id, MediaCoverEntity.Album, cover.CoverType, cover.Extension, null); @@ -176,6 +182,7 @@ namespace NzbDrone.Core.MediaCover if (!alreadyExists) { DownloadAlbumCover(album, cover, serverFileHeaders.LastModified ?? DateTime.Now); + updated = true; } } catch (WebException e) @@ -187,6 +194,8 @@ namespace NzbDrone.Core.MediaCover _logger.Error(e, "Couldn't download media cover for {0}", album); } } + + return updated; } private void DownloadCover(Artist artist, MediaCover cover, DateTime lastModified) @@ -273,15 +282,18 @@ namespace NzbDrone.Core.MediaCover public void HandleAsync(ArtistRefreshCompleteEvent message) { - EnsureArtistCovers(message.Artist); + var updated = EnsureArtistCovers(message.Artist); var albums = _albumService.GetAlbumsByArtist(message.Artist.Id); - foreach (Album album in albums) + foreach (var album in albums) { - EnsureAlbumCovers(album); + updated |= EnsureAlbumCovers(album); } - _eventAggregator.PublishEvent(new MediaCoversUpdatedEvent(message.Artist)); + if (updated) + { + _eventAggregator.PublishEvent(new MediaCoversUpdatedEvent(message.Artist)); + } } public void HandleAsync(ArtistDeletedEvent message) @@ -297,7 +309,11 @@ namespace NzbDrone.Core.MediaCover { if (message.DoRefresh) { - EnsureAlbumCovers(message.Album); + var updated = EnsureAlbumCovers(message.Album); + if (updated) + { + _eventAggregator.PublishEvent(new MediaCoversUpdatedEvent(message.Album)); + } } } diff --git a/src/NzbDrone.Core/Music/Services/ArtistService.cs b/src/NzbDrone.Core/Music/Services/ArtistService.cs index 131920196..68b78a457 100644 --- a/src/NzbDrone.Core/Music/Services/ArtistService.cs +++ b/src/NzbDrone.Core/Music/Services/ArtistService.cs @@ -24,7 +24,7 @@ namespace NzbDrone.Core.Music void DeleteArtist(int artistId, bool deleteFiles, bool addImportListExclusion = false); List GetAllArtists(); List AllForTag(int tagId); - Artist UpdateArtist(Artist artist); + Artist UpdateArtist(Artist artist, bool publishUpdatedEvent = true); List UpdateArtists(List artist, bool useExistingRelativeFolder); bool ArtistPathExists(string folder); void RemoveAddOptions(Artist artist); @@ -194,12 +194,16 @@ namespace NzbDrone.Core.Music _artistRepository.SetFields(artist, s => s.AddOptions); } - public Artist UpdateArtist(Artist artist) + public Artist UpdateArtist(Artist artist, bool publishUpdatedEvent = true) { _cache.Clear(); var storedArtist = GetArtist(artist.Id); var updatedArtist = _artistRepository.Update(artist); - _eventAggregator.PublishEvent(new ArtistEditedEvent(updatedArtist, storedArtist)); + + if (publishUpdatedEvent) + { + _eventAggregator.PublishEvent(new ArtistEditedEvent(updatedArtist, storedArtist)); + } return updatedArtist; } diff --git a/src/NzbDrone.Core/Music/Services/RefreshAlbumService.cs b/src/NzbDrone.Core/Music/Services/RefreshAlbumService.cs index 816c593f6..542d28559 100644 --- a/src/NzbDrone.Core/Music/Services/RefreshAlbumService.cs +++ b/src/NzbDrone.Core/Music/Services/RefreshAlbumService.cs @@ -174,8 +174,10 @@ namespace NzbDrone.Core.Music // Force update and fetch covers if images have changed so that we can write them into tags if (remote.Images.Any() && !local.Images.SequenceEqual(remote.Images)) { - _mediaCoverService.EnsureAlbumCovers(remote); - result = UpdateResult.UpdateTags; + if (_mediaCoverService.EnsureAlbumCovers(remote)) + { + result = UpdateResult.UpdateTags; + } } local.UseMetadataFrom(remote); diff --git a/src/NzbDrone.Core/Music/Services/RefreshArtistService.cs b/src/NzbDrone.Core/Music/Services/RefreshArtistService.cs index 7e39dd4a5..2ccfafef1 100644 --- a/src/NzbDrone.Core/Music/Services/RefreshArtistService.cs +++ b/src/NzbDrone.Core/Music/Services/RefreshArtistService.cs @@ -190,7 +190,7 @@ namespace NzbDrone.Core.Music protected override void SaveEntity(Artist local) { - _artistService.UpdateArtist(local); + _artistService.UpdateArtist(local, publishUpdatedEvent: false); } protected override void DeleteEntity(Artist local, bool deleteFiles)