From b7e609a7d57a5b0d5e68656313568a54fc5ec930 Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Thu, 12 Feb 2015 08:07:28 -0800 Subject: [PATCH] De-dupe Tags Fixed: Remove duplicate tags and prevent new ones from being created --- src/NzbDrone.Api/Series/SeriesModule.cs | 1 - src/NzbDrone.Api/Tags/TagModule.cs | 14 +- .../Migration/079_dedupe_tagsFixture.cs | 148 +++++++++++++++++ .../NzbDrone.Core.Test.csproj | 1 + .../Datastore/Migration/066_add_tags.cs | 3 + .../Datastore/Migration/079_dedupe_tags.cs | 157 ++++++++++++++++++ src/NzbDrone.Core/NzbDrone.Core.csproj | 2 + src/NzbDrone.Core/Tags/TagService.cs | 32 +++- src/NzbDrone.Core/Tags/TagsUpdatedEvent.cs | 8 + src/UI/Tags/TagCollection.js | 21 ++- 10 files changed, 368 insertions(+), 19 deletions(-) create mode 100644 src/NzbDrone.Core.Test/Datastore/Migration/079_dedupe_tagsFixture.cs create mode 100644 src/NzbDrone.Core/Datastore/Migration/079_dedupe_tags.cs create mode 100644 src/NzbDrone.Core/Tags/TagsUpdatedEvent.cs diff --git a/src/NzbDrone.Api/Series/SeriesModule.cs b/src/NzbDrone.Api/Series/SeriesModule.cs index 00b9707df..c5b503717 100644 --- a/src/NzbDrone.Api/Series/SeriesModule.cs +++ b/src/NzbDrone.Api/Series/SeriesModule.cs @@ -16,7 +16,6 @@ using NzbDrone.Core.Tv.Events; using NzbDrone.Core.Validation.Paths; using NzbDrone.Core.DataAugmentation.Scene; using NzbDrone.SignalR; -using Omu.ValueInjecter; namespace NzbDrone.Api.Series { diff --git a/src/NzbDrone.Api/Tags/TagModule.cs b/src/NzbDrone.Api/Tags/TagModule.cs index b6cdb7431..82b4c8d4b 100644 --- a/src/NzbDrone.Api/Tags/TagModule.cs +++ b/src/NzbDrone.Api/Tags/TagModule.cs @@ -1,15 +1,20 @@ using System; using System.Collections.Generic; using NzbDrone.Api.Mapping; +using NzbDrone.Core.Datastore.Events; +using NzbDrone.Core.Messaging.Events; using NzbDrone.Core.Tags; +using NzbDrone.SignalR; namespace NzbDrone.Api.Tags { - public class TagModule : NzbDroneRestModule + public class TagModule : NzbDroneRestModuleWithSignalR, IHandle { private readonly ITagService _tagService; - public TagModule(ITagService tagService) + public TagModule(IBroadcastSignalRMessage signalRBroadcaster, + ITagService tagService) + : base(signalRBroadcaster) { _tagService = tagService; @@ -44,5 +49,10 @@ namespace NzbDrone.Api.Tags { _tagService.Delete(id); } + + public void Handle(TagsUpdatedEvent message) + { + BroadcastResourceChange(ModelAction.Sync); + } } } diff --git a/src/NzbDrone.Core.Test/Datastore/Migration/079_dedupe_tagsFixture.cs b/src/NzbDrone.Core.Test/Datastore/Migration/079_dedupe_tagsFixture.cs new file mode 100644 index 000000000..47ac33b8a --- /dev/null +++ b/src/NzbDrone.Core.Test/Datastore/Migration/079_dedupe_tagsFixture.cs @@ -0,0 +1,148 @@ +using System; +using System.Linq; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.Jobs; +using NzbDrone.Core.Tags; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Core.Tv; + +namespace NzbDrone.Core.Test.Datastore.Migration +{ + [TestFixture] + public class dedupe_tags : MigrationTest + { + [Test] + public void should_not_fail_if_series_tags_are_null() + { + WithTestDb(c => + { + c.Insert.IntoTable("Series").Row(new + { + Tvdbid = 1, + TvRageId = 1, + Title = "Title1", + CleanTitle = "CleanTitle1", + Status = 1, + Images = "", + Path = "c:\\test", + Monitored = 1, + SeasonFolder = 1, + Runtime = 0, + SeriesType = 0, + UseSceneNumbering = 0, + LastInfoSync = "2000-01-01 00:00:00" + }); + + c.Insert.IntoTable("Tags").Row(new + { + Label = "test" + }); + }); + + Mocker.Resolve().All().Should().HaveCount(1); + } + + [Test] + public void should_not_fail_if_series_tags_are_empty() + { + WithTestDb(c => + { + c.Insert.IntoTable("Series").Row(new + { + Tvdbid = 1, + TvRageId = 1, + Title = "Title1", + CleanTitle = "CleanTitle1", + Status = 1, + Images = "", + Path = "c:\\test", + Monitored = 1, + SeasonFolder = 1, + Runtime = 0, + SeriesType = 0, + UseSceneNumbering = 0, + LastInfoSync = "2000-01-01 00:00:00", + Tags = "[]" + }); + + c.Insert.IntoTable("Tags").Row(new + { + Label = "test" + }); + }); + + Mocker.Resolve().All().Should().HaveCount(1); + } + + [Test] + public void should_remove_duplicate_labels_from_tags() + { + WithTestDb(c => + { + c.Insert.IntoTable("Tags").Row(new + { + Label = "test" + }); + + c.Insert.IntoTable("Tags").Row(new + { + Label = "test" + }); + }); + + Mocker.Resolve().All().Should().HaveCount(1); + } + + [Test] + public void should_not_allow_duplicate_tag_to_be_inserted() + { + WithTestDb(c => + { + c.Insert.IntoTable("Tags").Row(new + { + Label = "test" + }); + }); + + Assert.That(() => Mocker.Resolve().Insert(new Tag { Label = "test" }), Throws.Exception); + } + + [Test] + public void should_replace_duplicated_tag_with_proper_tag() + { + WithTestDb(c => + { + c.Insert.IntoTable("Series").Row(new + { + Tvdbid = 1, + TvRageId = 1, + Title = "Title1", + CleanTitle = "CleanTitle1", + Status = 1, + Images = "", + Path = "c:\\test", + Monitored = 1, + SeasonFolder = 1, + Runtime = 0, + SeriesType = 0, + UseSceneNumbering = 0, + LastInfoSync = "2000-01-01 00:00:00", + Tags = "[2]" + }); + + c.Insert.IntoTable("Tags").Row(new + { + Label = "test" + }); + + c.Insert.IntoTable("Tags").Row(new + { + Label = "test" + }); + }); + + Mocker.Resolve().Get(1).Tags.First().Should().Be(1); + } + } +} diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index a2df9a896..7bfc92ab3 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -121,6 +121,7 @@ + diff --git a/src/NzbDrone.Core/Datastore/Migration/066_add_tags.cs b/src/NzbDrone.Core/Datastore/Migration/066_add_tags.cs index dc8e94892..7a0c09838 100644 --- a/src/NzbDrone.Core/Datastore/Migration/066_add_tags.cs +++ b/src/NzbDrone.Core/Datastore/Migration/066_add_tags.cs @@ -16,6 +16,9 @@ namespace NzbDrone.Core.Datastore.Migration Alter.Table("Notifications") .AddColumn("Tags").AsString().Nullable(); + + Execute.Sql("UPDATE Series SET Tags = '[]'"); + Execute.Sql("UPDATE Notifications SET Tags = '[]'"); } } } diff --git a/src/NzbDrone.Core/Datastore/Migration/079_dedupe_tags.cs b/src/NzbDrone.Core/Datastore/Migration/079_dedupe_tags.cs new file mode 100644 index 000000000..e990a0bc7 --- /dev/null +++ b/src/NzbDrone.Core/Datastore/Migration/079_dedupe_tags.cs @@ -0,0 +1,157 @@ +using System; +using System.Collections.Generic; +using System.Data; +using System.Linq; +using FluentMigrator; +using NzbDrone.Common.Extensions; +using NzbDrone.Common.Serializer; +using NzbDrone.Core.Datastore.Migration.Framework; + +namespace NzbDrone.Core.Datastore.Migration +{ + [Migration(79)] + public class dedupe_tags : NzbDroneMigrationBase + { + protected override void MainDbUpgrade() + { + Execute.WithConnection(CleanupTags); + + Alter.Table("Tags").AlterColumn("Label").AsString().Unique(); + } + + private void CleanupTags(IDbConnection conn, IDbTransaction tran) + { + var tags = GetTags(conn, tran); + var grouped = tags.GroupBy(t => t.Label.ToLowerInvariant()); + var replacements = new List(); + + foreach (var group in grouped.Where(g => g.Count() > 1)) + { + var first = group.First().Id; + + foreach (var other in group.Skip(1).Select(t => t.Id)) + { + replacements.Add(new TagReplacement079 { OldId = other, NewId = first }); + } + } + + UpdateTaggedModel(conn, tran, "Series", replacements); + UpdateTaggedModel(conn, tran, "Notifications", replacements); + UpdateTaggedModel(conn, tran, "DelayProfiles", replacements); + UpdateTaggedModel(conn, tran, "Restrictions", replacements); + + DeleteTags(conn, tran, replacements); + } + + private List GetTags(IDbConnection conn, IDbTransaction tran) + { + var tags = new List(); + + using (IDbCommand tagCmd = conn.CreateCommand()) + { + tagCmd.Transaction = tran; + tagCmd.CommandText = @"SELECT Id, Label FROM Tags"; + + using (IDataReader tagReader = tagCmd.ExecuteReader()) + { + while (tagReader.Read()) + { + var id = tagReader.GetInt32(0); + var label = tagReader.GetString(1); + + tags.Add(new Tag079 { Id = id, Label = label }); + } + } + } + + return tags; + } + + private void UpdateTaggedModel(IDbConnection conn, IDbTransaction tran, string table, List replacements) + { + var tagged = new List(); + + using (IDbCommand tagCmd = conn.CreateCommand()) + { + tagCmd.Transaction = tran; + tagCmd.CommandText = String.Format("SELECT Id, Tags FROM {0}", table); + + using (IDataReader tagReader = tagCmd.ExecuteReader()) + { + while (tagReader.Read()) + { + if (!tagReader.IsDBNull(1)) + { + var id = tagReader.GetInt32(0); + var tags = tagReader.GetString(1); + + tagged.Add(new TaggedModel079 + { + Id = id, + Tags = Json.Deserialize>(tags) + }); + } + } + } + } + + var toUpdate = new List(); + + foreach (var model in tagged) + { + foreach (var replacement in replacements) + { + if (model.Tags.Contains(replacement.OldId)) + { + model.Tags.Remove(replacement.OldId); + model.Tags.Add(replacement.NewId); + + toUpdate.Add(model); + } + } + } + + foreach (var model in toUpdate.DistinctBy(m => m.Id)) + { + using (IDbCommand updateCmd = conn.CreateCommand()) + { + updateCmd.Transaction = tran; + updateCmd.CommandText = String.Format(@"UPDATE {0} SET Tags = ?", table); + updateCmd.AddParameter(model.Tags.ToJson()); + + updateCmd.ExecuteNonQuery(); + } + } + } + + private void DeleteTags(IDbConnection conn, IDbTransaction tran, List replacements) + { + var idsToRemove = replacements.Select(r => r.OldId).Distinct(); + + using (IDbCommand removeCmd = conn.CreateCommand()) + { + removeCmd.Transaction = tran; + removeCmd.CommandText = String.Format("DELETE FROM Tags WHERE Id IN ({0})", String.Join(",", idsToRemove)); + removeCmd.ExecuteNonQuery(); + } + } + + private class Tag079 + { + public int Id { get; set; } + public string Label { get; set; } + } + + private class TagReplacement079 + { + public int OldId { get; set; } + public int NewId { get; set; } + } + + private class TaggedModel079 + { + public int Id { get; set; } + public HashSet Tags { get; set; } + } + } +} diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index 7af72a5cb..b9a7686f5 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -246,6 +246,7 @@ + @@ -809,6 +810,7 @@ + diff --git a/src/NzbDrone.Core/Tags/TagService.cs b/src/NzbDrone.Core/Tags/TagService.cs index 2cfe66281..599aa51d4 100644 --- a/src/NzbDrone.Core/Tags/TagService.cs +++ b/src/NzbDrone.Core/Tags/TagService.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using NzbDrone.Core.Messaging.Events; namespace NzbDrone.Core.Tags { @@ -15,36 +16,51 @@ namespace NzbDrone.Core.Tags public class TagService : ITagService { - private readonly ITagRepository _tagRepository; + private readonly ITagRepository _repo; + private readonly IEventAggregator _eventAggregator; - public TagService(ITagRepository tagRepository) + public TagService(ITagRepository repo, IEventAggregator eventAggregator) { - _tagRepository = tagRepository; + _repo = repo; + _eventAggregator = eventAggregator; } public Tag GetTag(Int32 tagId) { - return _tagRepository.Get(tagId); + return _repo.Get(tagId); } public List All() { - return _tagRepository.All().ToList(); + return _repo.All().OrderBy(t => t.Label).ToList(); } public Tag Add(Tag tag) { - return _tagRepository.Insert(tag); + //TODO: check for duplicate tag by label and return that tag instead? + + tag.Label = tag.Label.ToLowerInvariant(); + + _repo.Insert(tag); + _eventAggregator.PublishEvent(new TagsUpdatedEvent()); + + return tag; } public Tag Update(Tag tag) { - return _tagRepository.Update(tag); + tag.Label = tag.Label.ToLowerInvariant(); + + _repo.Update(tag); + _eventAggregator.PublishEvent(new TagsUpdatedEvent()); + + return tag; } public void Delete(Int32 tagId) { - _tagRepository.Delete(tagId); + _repo.Delete(tagId); + _eventAggregator.PublishEvent(new TagsUpdatedEvent()); } } } diff --git a/src/NzbDrone.Core/Tags/TagsUpdatedEvent.cs b/src/NzbDrone.Core/Tags/TagsUpdatedEvent.cs new file mode 100644 index 000000000..27712c215 --- /dev/null +++ b/src/NzbDrone.Core/Tags/TagsUpdatedEvent.cs @@ -0,0 +1,8 @@ +using NzbDrone.Common.Messaging; + +namespace NzbDrone.Core.Tags +{ + public class TagsUpdatedEvent : IEvent + { + } +} diff --git a/src/UI/Tags/TagCollection.js b/src/UI/Tags/TagCollection.js index b9a860864..069b8f315 100644 --- a/src/UI/Tags/TagCollection.js +++ b/src/UI/Tags/TagCollection.js @@ -1,11 +1,16 @@ -var Backbone = require('backbone'); +var Backbone = require('backbone'); var TagModel = require('./TagModel'); var ApiData = require('../Shared/ApiData'); -module.exports = (function(){ - var Collection = Backbone.Collection.extend({ - url : window.NzbDrone.ApiRoot + '/tag', - model : TagModel - }); - return new Collection(ApiData.get('tag')); -}).call(this); \ No newline at end of file +require('../Mixins/backbone.signalr.mixin'); + +var collection = Backbone.Collection.extend({ + url : window.NzbDrone.ApiRoot + '/tag', + model : TagModel, + + comparator : function(model){ + return model.get('label'); + } +}); + +module.exports = new collection(ApiData.get('tag')).bindSignalR();