De-dupe Tags

Fixed: Remove duplicate tags and prevent new ones from being created

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
 {

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<TagResource>
+    public class TagModule : NzbDroneRestModuleWithSignalR<TagResource, Tag>, IHandle<TagsUpdatedEvent>
     {
         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);
+        }
     }
 }

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<Core.Datastore.Migration.dedupe_tags>
+    {
+        [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<TagRepository>().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<TagRepository>().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<TagRepository>().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<TagRepository>().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<SeriesRepository>().Get(1).Tags.First().Should().Be(1);
+        }
+    }
+}

src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj

@@ -121,6 +121,7 @@
     <Compile Include="Datastore\Migration\074_disable_eztv.cs" />
     <Compile Include="Datastore\Migration\072_history_grabIdFixture.cs" />
     <Compile Include="Datastore\Migration\070_delay_profileFixture.cs" />
+    <Compile Include="Datastore\Migration\079_dedupe_tagsFixture.cs" />
     <Compile Include="Datastore\Migration\075_force_lib_updateFixture.cs" />
     <Compile Include="Datastore\ObjectDatabaseFixture.cs" />
     <Compile Include="Datastore\PagingSpecExtensionsTests\PagingOffsetFixture.cs" />

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 = '[]'");
         }
     }
 }

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<TagReplacement079>();
+
+            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<Tag079> GetTags(IDbConnection conn, IDbTransaction tran)
+        {
+            var tags = new List<Tag079>();
+
+            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<TagReplacement079> replacements)
+        {
+            var tagged = new List<TaggedModel079>();
+
+            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<HashSet<int>>(tags)
+                                       });
+                        }
+                    }
+                }
+            }
+
+            var toUpdate = new List<TaggedModel079>();
+
+            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<TagReplacement079> 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<int> Tags { get; set; }
+        }
+    }
+}

src/NzbDrone.Core/NzbDrone.Core.csproj

@@ -246,6 +246,7 @@
     <Compile Include="Datastore\Migration\074_disable_eztv.cs" />
     <Compile Include="Datastore\Migration\073_clear_ratings.cs" />
     <Compile Include="Datastore\Migration\077_add_add_options_to_series.cs" />
+    <Compile Include="Datastore\Migration\079_dedupe_tags.cs" />
     <Compile Include="Datastore\Migration\070_delay_profile.cs" />
     <Compile Include="Datastore\Migration\Framework\MigrationContext.cs" />
     <Compile Include="Datastore\Migration\Framework\MigrationController.cs" />
@@ -809,6 +810,7 @@
     <Compile Include="Tags\Tag.cs" />
     <Compile Include="Tags\TagRepository.cs" />
     <Compile Include="Tags\TagService.cs" />
+    <Compile Include="Tags\TagsUpdatedEvent.cs" />
     <Compile Include="ThingiProvider\ConfigContractNotFoundException.cs" />
     <Compile Include="ThingiProvider\Events\ProviderUpdatedEvent.cs" />
     <Compile Include="ThingiProvider\IProvider.cs" />

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<Tag> 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());
         }
     }
 }

src/NzbDrone.Core/Tags/TagsUpdatedEvent.cs

@@ -0,0 +1,8 @@
+using NzbDrone.Common.Messaging;
+
+namespace NzbDrone.Core.Tags
+{
+    public class TagsUpdatedEvent : IEvent
+    {
+    }
+}

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);
+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();