Fixed: Smarter application update completed message

Closes #1864
2024-11-04 10:02:40 +01:00 · 2017-04-21 10:52:31 -07:00 · 2017-04-21 10:52:31 -07:00 · e97e13e897
commit e97e13e897
parent f8d5f1fc94
5 changed files with 218 additions and 127 deletions
--- a/src/NzbDrone.Core.Test/Messaging/Commands/CommandExecutorFixture.cs
+++ b/src/NzbDrone.Core.Test/Messaging/Commands/CommandExecutorFixture.cs
@ -1,121 +1,211 @@
-//using System;
-//using System.Collections.Generic;
-//using Moq;
-//using NUnit.Framework;
-//using NzbDrone.Common;
-//using NzbDrone.Core.Messaging.Commands;
-//using NzbDrone.Core.Messaging.Commands.Tracking;
-//using NzbDrone.Core.Messaging.Events;
-//using NzbDrone.Test.Common;
-//
-//namespace NzbDrone.Core.Test.Messaging.Commands
-//{
-//    [TestFixture]
-//    public class CommandExecutorFixture : TestBase<CommandExecutor>
-//    {
-//        private Mock<IExecute<CommandA>> _executorA;
-//        private Mock<IExecute<CommandB>> _executorB;
-//
-//        [SetUp]
-//        public void Setup()
-//        {
-//            _executorA = new Mock<IExecute<CommandA>>();
-//            _executorB = new Mock<IExecute<CommandB>>();
-//
-//            Mocker.GetMock<IServiceFactory>()
-//                  .Setup(c => c.Build(typeof(IExecute<CommandA>)))
-//                  .Returns(_executorA.Object);
-//
-//            Mocker.GetMock<IServiceFactory>()
-//                  .Setup(c => c.Build(typeof(IExecute<CommandB>)))
-//                  .Returns(_executorB.Object);
-//
-//
-//            Mocker.GetMock<ITrackCommands>()
-//                  .Setup(c => c.FindExisting(It.IsAny<Command>()))
-//                  .Returns<Command>(null);
-//        }
-//
-//        [Test]
-//        public void should_publish_command_to_executor()
-//        {
-//            var commandA = new CommandA();
-//
-//            Subject.Push(commandA);
-//
-//            _executorA.Verify(c => c.Execute(commandA), Times.Once());
-//        }
-//
-//        [Test]
-//        public void should_publish_command_by_with_optional_arg_using_name()
-//        {
-//            Mocker.GetMock<IServiceFactory>().Setup(c => c.GetImplementations(typeof(Command)))
-//                  .Returns(new List<Type> { typeof(CommandA), typeof(CommandB) });
-//
-//            Subject.Push(typeof(CommandA).FullName);
-//            _executorA.Verify(c => c.Execute(It.IsAny<CommandA>()), Times.Once());
-//        }
-//
-//
-//        [Test]
-//        public void should_not_publish_to_incompatible_executor()
-//        {
-//            var commandA = new CommandA();
-//
-//            Subject.Push(commandA);
-//
-//            _executorA.Verify(c => c.Execute(commandA), Times.Once());
-//            _executorB.Verify(c => c.Execute(It.IsAny<CommandB>()), Times.Never());
-//        }
-//
-//        [Test]
-//        public void broken_executor_should_throw_the_exception()
-//        {
-//            var commandA = new CommandA();
-//
-//            _executorA.Setup(c => c.Execute(It.IsAny<CommandA>()))
-//                       .Throws(new NotImplementedException());
-//
-//            Assert.Throws<NotImplementedException>(() => Subject.Push(commandA));
-//        }
-//
-//
-//        [Test]
-//        public void broken_executor_should_publish_executed_event()
-//        {
-//            var commandA = new CommandA();
-//
-//            _executorA.Setup(c => c.Execute(It.IsAny<CommandA>()))
-//                       .Throws(new NotImplementedException());
-//
-//            Assert.Throws<NotImplementedException>(() => Subject.Push(commandA));
-//
-//            VerifyEventPublished<CommandExecutedEvent>();
-//        }
-//
-//        [Test]
-//        public void should_publish_executed_event_on_success()
-//        {
-//            var commandA = new CommandA();
-//            Subject.Push(commandA);
-//
-//            VerifyEventPublished<CommandExecutedEvent>();
-//        }
-//    }
-//
-//    public class CommandA : Command
-//    {
-//        public CommandA(int id = 0)
-//        {
-//        }
-//    }
-//
-//    public class CommandB : Command
-//    {
-//
-//        public CommandB()
-//        {
-//        }
-//    }
-//
-//}
+using System;
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Threading;
+using Moq;
+using NUnit.Framework;
+using NzbDrone.Common;
+using NzbDrone.Core.Lifecycle;
+using NzbDrone.Core.Messaging.Commands;
+using NzbDrone.Core.Messaging.Events;
+using NzbDrone.Test.Common;
+
+namespace NzbDrone.Core.Test.Messaging.Commands
+{
+    [TestFixture]
+    public class CommandExecutorFixture : TestBase<CommandExecutor>
+    {
+        private BlockingCollection<CommandModel> _commandQueue;
+        private Mock<IExecute<CommandA>> _executorA;
+        private Mock<IExecute<CommandB>> _executorB;
+        private bool _commandExecuted = false;
+
+        [SetUp]
+        public void Setup()
+        {
+            _executorA = new Mock<IExecute<CommandA>>();
+            _executorB = new Mock<IExecute<CommandB>>();
+
+            Mocker.GetMock<IServiceFactory>()
+                  .Setup(c => c.Build(typeof(IExecute<CommandA>)))
+                  .Returns(_executorA.Object);
+
+            Mocker.GetMock<IServiceFactory>()
+                  .Setup(c => c.Build(typeof(IExecute<CommandB>)))
+                  .Returns(_executorB.Object);
+        }
+
+        private void GivenCommandQueue()
+        {
+            _commandQueue = new BlockingCollection<CommandModel>(new CommandQueue());
+
+            Mocker.GetMock<IManageCommandQueue>()
+                  .Setup(s => s.Queue(It.IsAny<CancellationToken>()))
+                  .Returns(_commandQueue.GetConsumingEnumerable);
+        }
+
+        private void WaitForExecution(CommandModel commandModel)
+        {
+            Mocker.GetMock<IManageCommandQueue>()
+                  .Setup(s => s.Complete(It.Is<CommandModel>(c => c == commandModel), It.IsAny<string>()))
+                  .Callback(() => _commandExecuted = true);
+
+            Mocker.GetMock<IManageCommandQueue>()
+                  .Setup(s => s.Fail(It.Is<CommandModel>(c => c == commandModel), It.IsAny<string>(), It.IsAny<Exception>()))
+                  .Callback(() => _commandExecuted = true);
+
+            while (!_commandExecuted)
+            {
+                Thread.Sleep(100);
+            }
+
+            var t1 = 1;
+        }
+
+        [Test]
+        public void should_start_executor_threads()
+        {
+            Subject.Handle(new ApplicationStartedEvent());
+
+            Mocker.GetMock<IManageCommandQueue>()
+                  .Verify(v => v.Queue(It.IsAny<CancellationToken>()), Times.AtLeastOnce());
+        }
+
+        [Test]
+        public void should_execute_on_executor()
+        {
+            GivenCommandQueue();
+            var commandA = new CommandA();
+            var commandModel = new CommandModel
+                               {
+                                   Body = commandA
+                               };
+
+            Subject.Handle(new ApplicationStartedEvent());
+            _commandQueue.Add(commandModel);
+
+            WaitForExecution(commandModel);
+
+            _executorA.Verify(c => c.Execute(commandA), Times.Once());
+        }
+
+        [Test]
+        public void should_not_execute_on_incompatible_executor()
+        {
+            GivenCommandQueue();
+            var commandA = new CommandA();
+            var commandModel = new CommandModel
+            {
+                Body = commandA
+            };
+
+            Subject.Handle(new ApplicationStartedEvent());
+            _commandQueue.Add(commandModel);
+
+            WaitForExecution(commandModel);
+
+            _executorA.Verify(c => c.Execute(commandA), Times.Once());
+            _executorB.Verify(c => c.Execute(It.IsAny<CommandB>()), Times.Never());
+        }
+
+        [Test]
+        public void broken_executor_should_publish_executed_event()
+        {
+            GivenCommandQueue();
+            var commandA = new CommandA();
+            var commandModel = new CommandModel
+            {
+                Body = commandA
+            };
+
+            _executorA.Setup(s => s.Execute(It.IsAny<CommandA>()))
+                      .Throws(new NotImplementedException());
+
+            Subject.Handle(new ApplicationStartedEvent());
+            _commandQueue.Add(commandModel);
+
+            WaitForExecution(commandModel);
+
+            VerifyEventPublished<CommandExecutedEvent>();
+            ExceptionVerification.ExpectedErrors(1);
+        }
+
+        [Test]
+        public void should_publish_executed_event_on_success()
+        {
+            GivenCommandQueue();
+            var commandA = new CommandA();
+            var commandModel = new CommandModel
+            {
+                Body = commandA
+            };
+
+            Subject.Handle(new ApplicationStartedEvent());
+            _commandQueue.Add(commandModel);
+
+            WaitForExecution(commandModel);
+
+            VerifyEventPublished<CommandExecutedEvent>();
+        }
+
+        [Test]
+        public void should_use_completion_message()
+        {
+            GivenCommandQueue();
+            var commandA = new CommandA();
+            var commandModel = new CommandModel
+            {
+                Body = commandA
+            };
+
+            Subject.Handle(new ApplicationStartedEvent());
+            _commandQueue.Add(commandModel);
+
+            WaitForExecution(commandModel);
+
+            Mocker.GetMock<IManageCommandQueue>()
+                  .Setup(s => s.Complete(It.Is<CommandModel>(c => c == commandModel), commandA.CompletionMessage))
+                  .Callback(() => _commandExecuted = true);
+        }
+
+        [Test]
+        public void should_use_last_progress_message_if_completion_message_is_null()
+        {
+            GivenCommandQueue();
+            var commandA = new CommandA();
+            var commandModel = new CommandModel
+            {
+                Body = commandA,
+                Message = "Do work"
+            };
+
+            Subject.Handle(new ApplicationStartedEvent());
+            _commandQueue.Add(commandModel);
+
+            WaitForExecution(commandModel);
+
+            Mocker.GetMock<IManageCommandQueue>()
+                  .Setup(s => s.Complete(It.Is<CommandModel>(c => c == commandModel), commandModel.Message))
+                  .Callback(() => _commandExecuted = true);
+        }
+    }
+
+    public class CommandA : Command
+    {
+        public CommandA(int id = 0)
+        {
+        }
+    }
+
+    public class CommandB : Command
+    {
+
+        public CommandB()
+        {
+            
+        }
+
+        public override string CompletionMessage => null;
+    }
+
+}
--- a/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs
+++ b/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs
@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Threading;
 using NLog;
 using NzbDrone.Common;
@ -76,7 +76,7 @@ private void ExecuteCommand<TCommand>(TCommand command, CommandModel commandMode

                handler.Execute(command);

-                _commandQueueManager.Complete(commandModel, command.CompletionMessage);
+                _commandQueueManager.Complete(commandModel, command.CompletionMessage ?? commandModel.Message);
            }
            catch (CommandFailedException ex)
            {
--- a/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs
+++ b/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs
@ -35,7 +35,7 @@ public class CommandQueueManager : IManageCommandQueue, IHandle<ApplicationStart
        private readonly Logger _logger;

        private readonly ICached<CommandModel> _commandCache;
-        private readonly BlockingCollection<CommandModel> _commandQueue; 
+        private readonly BlockingCollection<CommandModel> _commandQueue;

        public CommandQueueManager(ICommandRepository repo, 
                                   IServiceFactory serviceFactory,
--- a/src/NzbDrone.Core/Update/Commands/ApplicationUpdateCommand.cs
+++ b/src/NzbDrone.Core/Update/Commands/ApplicationUpdateCommand.cs
@ -6,6 +6,6 @@ public class ApplicationUpdateCommand : Command
    {
        public override bool SendUpdatesToClient => true;

-        public override string CompletionMessage => "Restarting Sonarr to apply updates";
+        public override string CompletionMessage => null;
    }
 }
--- a/src/NzbDrone.Core/Update/InstallUpdateService.cs
+++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs
@ -199,19 +199,20 @@ public void Execute(ApplicationUpdateCommand message)

            if (latestAvailable == null)
            {
-                _logger.ProgressDebug("No update available.");
+                _logger.ProgressDebug("No update available");
                return;
            }

            if (OsInfo.IsNotWindows && !_configFileProvider.UpdateAutomatically && message.Trigger != CommandTrigger.Manual)
            {
-                _logger.ProgressDebug("Auto-update not enabled, not installing available update.");
+                _logger.ProgressDebug("Auto-update not enabled, not installing available update");
                return;
            }

            try
            {
                InstallUpdate(latestAvailable);
+                _logger.ProgressDebug("Restarting Sonarr to apply updates");
            }
            catch (UpdateFolderNotWritableException ex)
            {