From be3152e630e71e6bd432aae8fd92391dedd60727 Mon Sep 17 00:00:00 2001 From: Leonardo Galli Date: Mon, 8 Jul 2019 00:15:35 +0200 Subject: [PATCH] Fixed: When refreshing info about a movie, the alt titles should now correctly be deleted / updated, even from TMDB. (#3603) * Fixed: When refreshing info about a movie, the alt titles should now correctly be deleted / updated, even from TMDB. Fixes #3542 * Fixed: Small things fixup. --- .../AlternativeTitleFixture.cs | 32 ++++++ .../AlternativeTitleServiceFixture.cs | 104 ++++++++++++++++++ .../AlternativeTitles/AlternativeTitle.cs | 6 +- .../AlternativeTitleRepository.cs | 2 + .../AlternativeTitleService.cs | 28 ++++- .../Movies/RefreshMovieService.cs | 18 +-- 6 files changed, 168 insertions(+), 22 deletions(-) create mode 100644 src/NzbDrone.Core.Test/MovieTests/AlternativeTitleServiceTests/AlternativeTitleFixture.cs create mode 100644 src/NzbDrone.Core.Test/MovieTests/AlternativeTitleServiceTests/AlternativeTitleServiceFixture.cs diff --git a/src/NzbDrone.Core.Test/MovieTests/AlternativeTitleServiceTests/AlternativeTitleFixture.cs b/src/NzbDrone.Core.Test/MovieTests/AlternativeTitleServiceTests/AlternativeTitleFixture.cs new file mode 100644 index 000000000..31b9edbf0 --- /dev/null +++ b/src/NzbDrone.Core.Test/MovieTests/AlternativeTitleServiceTests/AlternativeTitleFixture.cs @@ -0,0 +1,32 @@ +using FizzWare.NBuilder; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.Movies.AlternativeTitles; +using NzbDrone.Core.Test.Framework; + +namespace NzbDrone.Core.Test.MovieTests.AlternativeTitleServiceTests +{ + [TestFixture] + public class AlternativeTitleFixture : CoreTest + { + private AlternativeTitle CreateFakeTitle(SourceType source, int votes) + { + return Builder.CreateNew().With(t => t.SourceType = source).With(t => t.Votes = votes) + .Build(); + } + + [TestCase(SourceType.TMDB, -1, true)] + [TestCase(SourceType.TMDB, 1000, true)] + [TestCase(SourceType.Mappings, 0, false)] + [TestCase(SourceType.Mappings, 4, true)] + [TestCase(SourceType.Mappings, -1, false)] + [TestCase(SourceType.Indexer, 0, true)] + [TestCase(SourceType.User, 0, true)] + public void should_be_trusted(SourceType source, int votes, bool trusted) + { + var fakeTitle = CreateFakeTitle(source, votes); + + fakeTitle.IsTrusted().Should().Be(trusted); + } + } +} diff --git a/src/NzbDrone.Core.Test/MovieTests/AlternativeTitleServiceTests/AlternativeTitleServiceFixture.cs b/src/NzbDrone.Core.Test/MovieTests/AlternativeTitleServiceTests/AlternativeTitleServiceFixture.cs new file mode 100644 index 000000000..bd2b49fcb --- /dev/null +++ b/src/NzbDrone.Core.Test/MovieTests/AlternativeTitleServiceTests/AlternativeTitleServiceFixture.cs @@ -0,0 +1,104 @@ +using System.Collections.Generic; +using System.Linq; +using FizzWare.NBuilder; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using NzbDrone.Common.Extensions; +using NzbDrone.Core.Movies; +using NzbDrone.Core.Movies.AlternativeTitles; +using NzbDrone.Core.Test.Framework; + +namespace NzbDrone.Core.Test.MovieTests.AlternativeTitleServiceTests +{ + [TestFixture] + public class AlternativeTitleServiceFixture : CoreTest + { + private AlternativeTitle _title1; + private AlternativeTitle _title2; + private AlternativeTitle _title3; + + private Movie _movie; + + [SetUp] + public void Setup() + { + var titles = Builder.CreateListOfSize(3).All().With(t => t.MovieId = 0).Build(); + _title1 = titles[0]; + _title2 = titles[1]; + _title3 = titles[2]; + _movie = Builder.CreateNew().With(m => m.CleanTitle = "myothertitle").With(m => m.Id = 1).Build(); + } + + private void GivenExistingTitles(params AlternativeTitle[] titles) + { + Mocker.GetMock().Setup(r => r.FindByMovieId(_movie.Id)) + .Returns(titles.ToList()); + } + + [Test] + public void should_update_insert_remove_titles() + { + var titles = new List {_title2, _title3}; + var updates = new List {_title2}; + var deletes = new List {_title1}; + var inserts = new List {_title3}; + GivenExistingTitles(_title1, _title2); + + Subject.UpdateTitles(titles, _movie); + + Mocker.GetMock().Verify(r => r.InsertMany(inserts), Times.Once()); + Mocker.GetMock().Verify(r => r.UpdateMany(updates), Times.Once()); + Mocker.GetMock().Verify(r => r.DeleteMany(deletes), Times.Once()); + } + + [Test] + public void should_not_insert_duplicates() + { + GivenExistingTitles(); + var titles = new List {_title1, _title1}; + var inserts = new List{ _title1 }; + + Subject.UpdateTitles(titles, _movie); + + Mocker.GetMock().Verify(r => r.InsertMany(inserts), Times.Once()); + } + + [Test] + public void should_not_insert_main_title() + { + GivenExistingTitles(); + var titles = new List{_title1}; + var movie = Builder.CreateNew().With(m => m.CleanTitle = _title1.CleanTitle).Build(); + + Subject.UpdateTitles(titles, movie); + + Mocker.GetMock().Verify(r => r.InsertMany(new List()), Times.Once()); + } + + [Test] + public void should_update_movie_id() + { + GivenExistingTitles(); + var titles = new List {_title1, _title2}; + + Subject.UpdateTitles(titles, _movie); + + _title1.MovieId.Should().Be(_movie.Id); + _title2.MovieId.Should().Be(_movie.Id); + } + + [Test] + public void should_update_with_correct_id() + { + var existingTitle = Builder.CreateNew().With(t => t.Id = 2).Build(); + GivenExistingTitles(existingTitle); + var updateTitle = existingTitle.JsonClone(); + updateTitle.Id = 0; + + Subject.UpdateTitles(new List {updateTitle}, _movie); + + Mocker.GetMock().Verify(r => r.UpdateMany(It.Is>(list => list.First().Id == existingTitle.Id)), Times.Once()); + } + } +} diff --git a/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitle.cs b/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitle.cs index 720bdf35a..2f277e646 100644 --- a/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitle.cs +++ b/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitle.cs @@ -33,15 +33,15 @@ public AlternativeTitle(string title, SourceType sourceType = SourceType.TMDB, i Language = language ?? Language.English; } - public bool IsTrusted(int minVotes = 3) + public bool IsTrusted(int minVotes = 4) { switch (SourceType) { - case SourceType.TMDB: + case SourceType.Mappings: return Votes >= minVotes; default: return true; - } + } } public override bool Equals(object obj) diff --git a/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitleRepository.cs b/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitleRepository.cs index 1759adced..f40eb4776 100644 --- a/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitleRepository.cs +++ b/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitleRepository.cs @@ -1,5 +1,7 @@ using System.Collections.Generic; +using System.Data; using System.Linq; +using Marr.Data; using NzbDrone.Common.Extensions; using NzbDrone.Core.Datastore; using NzbDrone.Core.Messaging.Events; diff --git a/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitleService.cs b/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitleService.cs index 8567b6b9a..1b4fc197f 100644 --- a/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitleService.cs +++ b/src/NzbDrone.Core/Movies/AlternativeTitles/AlternativeTitleService.cs @@ -3,6 +3,7 @@ using NzbDrone.Core.Messaging.Events; using System.Collections.Generic; using System.Linq; +using NzbDrone.Common.Extensions; using NzbDrone.Core.Movies.Events; namespace NzbDrone.Core.Movies.AlternativeTitles @@ -14,7 +15,7 @@ public interface IAlternativeTitleService List AddAltTitles(List titles, Movie movie); AlternativeTitle GetById(int id); List GetAllTitles(); - void DeleteNotEnoughVotes(List mappingsTitles); + List UpdateTitles(List titles, Movie movie); } public class AlternativeTitleService : IAlternativeTitleService, IHandleAsync @@ -69,11 +70,28 @@ public void RemoveTitle(AlternativeTitle title) _titleRepo.Delete(title); } - public void DeleteNotEnoughVotes(List mappingsTitles) + public List UpdateTitles(List titles, Movie movie) { - var toRemove = mappingsTitles.Where(t => t.SourceType == SourceType.Mappings && t.Votes < 4); - var realT = _titleRepo.FindBySourceIds(toRemove.Select(t => t.SourceId).ToList()); - _titleRepo.DeleteMany(realT); + int movieId = movie.Id; + // First update the movie ids so we can correlate them later. + titles.ForEach(t => t.MovieId = movieId); + // Then make sure none of them are the same as the main title. + titles = titles.Where(t => t.CleanTitle != movie.CleanTitle).ToList(); + // Then make sure they are all distinct titles + titles = titles.DistinctBy(t => t.CleanTitle).ToList(); + + // Now find titles to delete, update and insert. + var existingTitles = _titleRepo.FindByMovieId(movieId); + + var insert = titles.Where(t => !existingTitles.Contains(t)); + var update = existingTitles.Where(t => titles.Contains(t)); + var delete = existingTitles.Where(t => !titles.Contains(t)); + + _titleRepo.DeleteMany(delete.ToList()); + _titleRepo.UpdateMany(update.ToList()); + _titleRepo.InsertMany(insert.ToList()); + + return titles; } public void HandleAsync(MovieDeletedEvent message) diff --git a/src/NzbDrone.Core/Movies/RefreshMovieService.cs b/src/NzbDrone.Core/Movies/RefreshMovieService.cs index 45a389372..083a3cb5d 100644 --- a/src/NzbDrone.Core/Movies/RefreshMovieService.cs +++ b/src/NzbDrone.Core/Movies/RefreshMovieService.cs @@ -104,26 +104,16 @@ private void RefreshMovieInfo(Movie movie) _logger.Warn(e, "Couldn't update movie path for " + movie.Path); } - movieInfo.AlternativeTitles = movieInfo.AlternativeTitles.Where(t => t.CleanTitle != movie.CleanTitle) - .DistinctBy(t => t.CleanTitle) - .ExceptBy(t => t.CleanTitle, movie.AlternativeTitles, t => t.CleanTitle, EqualityComparer.Default).ToList(); - try { - movie.AlternativeTitles.AddRange(_titleService.AddAltTitles(movieInfo.AlternativeTitles, movie)); - var mappings = _apiClient.AlternativeTitlesAndYearForMovie(movieInfo.TmdbId); var mappingsTitles = mappings.Item1; - _titleService.DeleteNotEnoughVotes(mappingsTitles); + mappingsTitles = mappingsTitles.Where(t => t.IsTrusted()).ToList(); - mappingsTitles = mappingsTitles.ExceptBy(t => t.CleanTitle, movie.AlternativeTitles, - t => t.CleanTitle, EqualityComparer.Default).ToList(); - - - mappingsTitles = mappingsTitles.Where(t => t.Votes > 3).ToList(); - - movie.AlternativeTitles.AddRange(_titleService.AddAltTitles(mappingsTitles, movie)); + movieInfo.AlternativeTitles.AddRange(mappingsTitles); + + movie.AlternativeTitles = _titleService.UpdateTitles(movieInfo.AlternativeTitles, movie); if (mappings.Item2 != null) {