From 61cf1ccb7cf7929ab5288aab07a4e6c613275542 Mon Sep 17 00:00:00 2001 From: Qstick Date: Wed, 20 Mar 2019 22:14:27 -0400 Subject: [PATCH] New: Already Imported Decision Specification (#661) --- src/Lidarr.Api.V1/History/HistoryModule.cs | 2 +- .../AlreadyImportedSpecificationFixture.cs | 172 ++++++++++++++++++ .../NzbDrone.Core.Test.csproj | 1 + .../AlreadyImportedSpecification.cs | 106 +++++++++++ .../History/HistoryRepository.cs | 7 +- src/NzbDrone.Core/History/HistoryService.cs | 6 +- src/NzbDrone.Core/NzbDrone.Core.csproj | 1 + 7 files changed, 287 insertions(+), 8 deletions(-) create mode 100644 src/NzbDrone.Core.Test/DecisionEngineTests/AlreadyImportedSpecificationFixture.cs create mode 100644 src/NzbDrone.Core/DecisionEngine/Specifications/AlreadyImportedSpecification.cs diff --git a/src/Lidarr.Api.V1/History/HistoryModule.cs b/src/Lidarr.Api.V1/History/HistoryModule.cs index 5acd53128..62d29a682 100644 --- a/src/Lidarr.Api.V1/History/HistoryModule.cs +++ b/src/Lidarr.Api.V1/History/HistoryModule.cs @@ -138,7 +138,7 @@ namespace Lidarr.Api.V1.History { int albumId = Convert.ToInt32(queryAlbumId.Value); - return _historyService.GetByAlbum(artistId, albumId, eventType).Select(h => MapToResource(h, includeArtist, includeAlbum, includeTrack)).ToList(); + return _historyService.GetByAlbum(albumId, eventType).Select(h => MapToResource(h, includeArtist, includeAlbum, includeTrack)).ToList(); } return _historyService.GetByArtist(artistId, eventType).Select(h => MapToResource(h, includeArtist, includeAlbum, includeTrack)).ToList(); diff --git a/src/NzbDrone.Core.Test/DecisionEngineTests/AlreadyImportedSpecificationFixture.cs b/src/NzbDrone.Core.Test/DecisionEngineTests/AlreadyImportedSpecificationFixture.cs new file mode 100644 index 000000000..26c429e2a --- /dev/null +++ b/src/NzbDrone.Core.Test/DecisionEngineTests/AlreadyImportedSpecificationFixture.cs @@ -0,0 +1,172 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using FizzWare.NBuilder; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.Configuration; +using NzbDrone.Core.History; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.Qualities; +using NzbDrone.Core.Music; +using NzbDrone.Core.DecisionEngine.Specifications; +using NzbDrone.Core.Indexers; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Core.MediaFiles; +using NzbDrone.Core.Languages; + +namespace NzbDrone.Core.Test.DecisionEngineTests +{ + [TestFixture] + public class AlreadyImportedSpecificationFixture : CoreTest + { + private const int FIRST_ALBUM_ID = 1; + private const string TITLE = "Some.Artist-Some.Album-2018-320kbps-CD-Lidarr"; + + private Artist _artist; + private QualityModel _mp3; + private QualityModel _flac; + private RemoteAlbum _remoteAlbum; + private List _history; + private TrackFile _firstFile; + + [SetUp] + public void Setup() + { + var singleAlbumList = new List + { + new Album + { + Id = FIRST_ALBUM_ID, + Title = "Some Album" + } + }; + + _artist = Builder.CreateNew() + .Build(); + + _firstFile = new TrackFile { Quality = new QualityModel(Quality.FLAC, new Revision(version: 2)), DateAdded = DateTime.Now, Language = Language.English }; + + _mp3 = new QualityModel(Quality.MP3_320, new Revision(version: 1)); + _flac = new QualityModel(Quality.FLAC, new Revision(version: 1)); + + _remoteAlbum = new RemoteAlbum + { + Artist = _artist, + ParsedAlbumInfo = new ParsedAlbumInfo { Quality = _mp3 }, + Albums = singleAlbumList, + Release = Builder.CreateNew() + .Build() + }; + + _history = new List(); + + Mocker.GetMock() + .SetupGet(s => s.EnableCompletedDownloadHandling) + .Returns(true); + + Mocker.GetMock() + .Setup(s => s.GetByAlbum(It.IsAny(), null)) + .Returns(_history); + + Mocker.GetMock() + .Setup(c => c.GetFilesByAlbum(It.IsAny())) + .Returns(new List { _firstFile }); + } + + private void GivenCdhDisabled() + { + Mocker.GetMock() + .SetupGet(s => s.EnableCompletedDownloadHandling) + .Returns(false); + } + + private void GivenHistoryItem(string downloadId, string sourceTitle, QualityModel quality, HistoryEventType eventType) + { + _history.Add(new History.History + { + DownloadId = downloadId, + SourceTitle = sourceTitle, + Quality = quality, + Date = DateTime.UtcNow, + EventType = eventType + }); + } + + [Test] + public void should_be_accepted_if_CDH_is_disabled() + { + GivenCdhDisabled(); + + Subject.IsSatisfiedBy(_remoteAlbum, null).Accepted.Should().BeTrue(); + } + + [Test] + public void should_be_accepted_if_album_does_not_have_a_file() + { + Mocker.GetMock() + .Setup(c => c.GetFilesByAlbum(It.IsAny())) + .Returns(new List { }); + + Subject.IsSatisfiedBy(_remoteAlbum, null).Accepted.Should().BeTrue(); + } + + [Test] + public void should_be_accepted_if_album_does_not_have_grabbed_event() + { + Subject.IsSatisfiedBy(_remoteAlbum, null).Accepted.Should().BeTrue(); + } + + [Test] + public void should_be_accepted_if_album_does_not_have_imported_event() + { + GivenHistoryItem(Guid.NewGuid().ToString().ToUpper(), TITLE, _mp3, HistoryEventType.Grabbed); + + Subject.IsSatisfiedBy(_remoteAlbum, null).Accepted.Should().BeTrue(); + } + + [Test] + public void should_be_accepted_if_grabbed_and_imported_quality_is_the_same() + { + var downloadId = Guid.NewGuid().ToString().ToUpper(); + + GivenHistoryItem(downloadId, TITLE, _mp3, HistoryEventType.Grabbed); + GivenHistoryItem(downloadId, TITLE, _mp3, HistoryEventType.DownloadImported); + + Subject.IsSatisfiedBy(_remoteAlbum, null).Accepted.Should().BeTrue(); + } + + [Test] + public void should_be_rejected_if_grabbed_download_id_matches_release_torrent_hash() + { + var downloadId = Guid.NewGuid().ToString().ToUpper(); + + GivenHistoryItem(downloadId, TITLE, _mp3, HistoryEventType.Grabbed); + GivenHistoryItem(downloadId, TITLE, _flac, HistoryEventType.DownloadImported); + + _remoteAlbum.Release = Builder.CreateNew() + .With(t => t.DownloadProtocol = DownloadProtocol.Torrent) + .With(t => t.InfoHash = downloadId) + .Build(); + + Subject.IsSatisfiedBy(_remoteAlbum, null).Accepted.Should().BeFalse(); + } + + [Test] + public void should_be_rejected_if_release_title_matches_grabbed_event_source_title() + { + var downloadId = Guid.NewGuid().ToString().ToUpper(); + + GivenHistoryItem(downloadId, TITLE, _mp3, HistoryEventType.Grabbed); + GivenHistoryItem(downloadId, TITLE, _flac, HistoryEventType.DownloadImported); + + _remoteAlbum.Release = Builder.CreateNew() + .With(t => t.DownloadProtocol = DownloadProtocol.Torrent) + .With(t => t.InfoHash = downloadId) + .Build(); + + Subject.IsSatisfiedBy(_remoteAlbum, null).Accepted.Should().BeFalse(); + } + } +} diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index d4306cc23..4231188f7 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -133,6 +133,7 @@ + diff --git a/src/NzbDrone.Core/DecisionEngine/Specifications/AlreadyImportedSpecification.cs b/src/NzbDrone.Core/DecisionEngine/Specifications/AlreadyImportedSpecification.cs new file mode 100644 index 000000000..e214c4ddc --- /dev/null +++ b/src/NzbDrone.Core/DecisionEngine/Specifications/AlreadyImportedSpecification.cs @@ -0,0 +1,106 @@ +using System; +using System.Linq; +using NLog; +using NzbDrone.Core.Configuration; +using NzbDrone.Core.History; +using NzbDrone.Core.Indexers; +using NzbDrone.Core.IndexerSearch.Definitions; +using NzbDrone.Core.MediaFiles; +using NzbDrone.Core.Parser.Model; + +namespace NzbDrone.Core.DecisionEngine.Specifications +{ + public class AlreadyImportedSpecification : IDecisionEngineSpecification + { + private readonly IHistoryService _historyService; + private readonly IConfigService _configService; + private readonly IMediaFileService _mediaFileService; + private readonly Logger _logger; + + public AlreadyImportedSpecification(IHistoryService historyService, + IConfigService configService, + IMediaFileService mediaFileService, + Logger logger) + { + _historyService = historyService; + _mediaFileService = mediaFileService; + _configService = configService; + _logger = logger; + } + + public SpecificationPriority Priority => SpecificationPriority.Database; + public RejectionType Type => RejectionType.Permanent; + + public Decision IsSatisfiedBy(RemoteAlbum subject, SearchCriteriaBase searchCriteria) + { + var cdhEnabled = _configService.EnableCompletedDownloadHandling; + + if (!cdhEnabled) + { + _logger.Debug("Skipping already imported check because CDH is disabled"); + return Decision.Accept(); + } + + _logger.Debug("Performing already imported check on report"); + foreach (var album in subject.Albums) + { + var trackFiles = _mediaFileService.GetFilesByAlbum(album.Id); + + if (trackFiles.Count() == 0) + { + _logger.Debug("Skipping already imported check for album without files"); + continue; + } + + var historyForAlbum = _historyService.GetByAlbum(album.Id, null); + var lastGrabbed = historyForAlbum.FirstOrDefault(h => h.EventType == HistoryEventType.Grabbed); + + if (lastGrabbed == null) + { + continue; + } + + var imported = historyForAlbum.FirstOrDefault(h => + h.EventType == HistoryEventType.DownloadImported && + h.DownloadId == lastGrabbed.DownloadId); + + if (imported == null) + { + continue; + } + + // This is really only a guard against redownloading the same release over + // and over when the grabbed and imported qualities do not match, if they do + // match skip this check. + if (lastGrabbed.Quality.Equals(imported.Quality)) + { + continue; + } + + var release = subject.Release; + + if (release.DownloadProtocol == DownloadProtocol.Torrent) + { + var torrentInfo = release as TorrentInfo; + + if (torrentInfo != null && torrentInfo.InfoHash.ToUpper() == lastGrabbed.DownloadId) + { + _logger.Debug("Has same torrent hash as a grabbed and imported release"); + return Decision.Reject("Has same torrent hash as a grabbed and imported release"); + } + } + + // Only based on title because a release with the same title on another indexer/released at + // a different time very likely has the exact same content and we don't need to also try it. + + if (release.Title.Equals(lastGrabbed.SourceTitle, StringComparison.InvariantCultureIgnoreCase)) + { + _logger.Debug("Has same release name as a grabbed and imported release"); + return Decision.Reject("Has same release name as a grabbed and imported release"); + } + } + + return Decision.Accept(); + } + } +} diff --git a/src/NzbDrone.Core/History/HistoryRepository.cs b/src/NzbDrone.Core/History/HistoryRepository.cs index 223266158..e21efec4f 100644 --- a/src/NzbDrone.Core/History/HistoryRepository.cs +++ b/src/NzbDrone.Core/History/HistoryRepository.cs @@ -15,7 +15,7 @@ namespace NzbDrone.Core.History History MostRecentForDownloadId(string downloadId); List FindByDownloadId(string downloadId); List GetByArtist(int artistId, HistoryEventType? eventType); - List GetByAlbum(int artistId, int albumId, HistoryEventType? eventType); + List GetByAlbum(int albumId, HistoryEventType? eventType); List FindDownloadHistory(int idArtistId, QualityModel quality); void DeleteForArtist(int artistId); List Since(DateTime date, HistoryEventType? eventType); @@ -66,11 +66,10 @@ namespace NzbDrone.Core.History return query; } - public List GetByAlbum(int artistId, int albumId, HistoryEventType? eventType) + public List GetByAlbum(int albumId, HistoryEventType? eventType) { var query = Query.Join(JoinType.Inner, h => h.Album, (h, e) => h.AlbumId == e.Id) - .Where(h => h.ArtistId == artistId) - .AndWhere(h => h.AlbumId == albumId); + .Where(h => h.AlbumId == albumId); if (eventType.HasValue) { diff --git a/src/NzbDrone.Core/History/HistoryService.cs b/src/NzbDrone.Core/History/HistoryService.cs index 9a82d3c5b..e34d3c7ee 100644 --- a/src/NzbDrone.Core/History/HistoryService.cs +++ b/src/NzbDrone.Core/History/HistoryService.cs @@ -24,7 +24,7 @@ namespace NzbDrone.Core.History History MostRecentForDownloadId(string downloadId); History Get(int historyId); List GetByArtist(int artistId, HistoryEventType? eventType); - List GetByAlbum(int artistId, int albumId, HistoryEventType? eventType); + List GetByAlbum(int albumId, HistoryEventType? eventType); List Find(string downloadId, HistoryEventType eventType); List FindByDownloadId(string downloadId); List Since(DateTime date, HistoryEventType? eventType); @@ -75,9 +75,9 @@ namespace NzbDrone.Core.History return _historyRepository.GetByArtist(artistId, eventType); } - public List GetByAlbum(int artistId, int albumId, HistoryEventType? eventType) + public List GetByAlbum(int albumId, HistoryEventType? eventType) { - return _historyRepository.GetByAlbum(artistId, albumId, eventType); + return _historyRepository.GetByAlbum(albumId, eventType); } public List Find(string downloadId, HistoryEventType eventType) diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index 06a1b6f28..3881bcb2d 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -225,6 +225,7 @@ +