From fa8f67d7fe7ca20ace7c5529146fdbb74637c558 Mon Sep 17 00:00:00 2001 From: "kay.one" Date: Tue, 7 May 2013 22:47:15 -0700 Subject: [PATCH] fixed service registration for event handlers and executors. --- Gruntfile.js | 2 +- NzbDrone.Api/Commands/CommandModule.cs | 10 +-- NzbDrone.App.Test/ContainerFixture.cs | 24 ++++++- .../MessageAggregatorCommandTests.cs | 49 +++++++------- .../MessageAggregatorEventTests.cs | 64 ++++++++++--------- NzbDrone.Common/ContainerBuilderBase.cs | 12 +++- .../Messaging/MessageAggregator.cs | 38 +++++++---- .../Messaging/MessageExtensions.cs | 17 +++++ NzbDrone.Common/NzbDrone.Common.csproj | 5 +- NzbDrone.Common/ServiceFactory.cs | 39 +++++++++++ .../IndexerIntegrationFixture.cs | 6 +- .../NzbDrone.Integration.Test.csproj | 1 + 12 files changed, 181 insertions(+), 86 deletions(-) create mode 100644 NzbDrone.Common/Messaging/MessageExtensions.cs create mode 100644 NzbDrone.Common/ServiceFactory.cs diff --git a/Gruntfile.js b/Gruntfile.js index c781b8ee1..2b9bb3fa1 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -5,7 +5,7 @@ module.exports = function (grunt) { pkg: grunt.file.readJSON('package.json'), curl: { - 'UI/JsLibraries/backbone.js' : 'http://backbonejs.org/backbone.js', + 'UI/JsLibraries/backbone.js' : 'http://documentcloud.github.io/backbone/backbone.js', 'UI/JsLibraries/backbone.marionette.js' : 'http://marionettejs.com/downloads/backbone.marionette.js', 'UI/JsLibraries/backbone.modelbinder.js' : 'http://raw.github.com/theironcook/Backbone.ModelBinder/master/Backbone.ModelBinder.js', 'UI/JsLibraries/backbone.mutators.js' : 'http://raw.github.com/asciidisco/Backbone.Mutators/master/backbone.mutators.js', diff --git a/NzbDrone.Api/Commands/CommandModule.cs b/NzbDrone.Api/Commands/CommandModule.cs index e3d30bf20..ae99bb893 100644 --- a/NzbDrone.Api/Commands/CommandModule.cs +++ b/NzbDrone.Api/Commands/CommandModule.cs @@ -23,13 +23,13 @@ namespace NzbDrone.Api.Commands { var commandType = _commands.Single(c => c.GetType().Name.Replace("Command", "").Equals(resource.Command, StringComparison.InvariantCultureIgnoreCase)) .GetType(); - var command = (object)Request.Body.FromJson(commandType); - var method = typeof(IMessageAggregator).GetMethod("PublishCommand"); - var genericMethod = method.MakeGenericMethod(commandType); - genericMethod.Invoke(_messageAggregator, new[] { command }); - return resource; + var command = Request.Body.FromJson(commandType); + + _messageAggregator.PublishCommand(command); + + return resource; } } } \ No newline at end of file diff --git a/NzbDrone.App.Test/ContainerFixture.cs b/NzbDrone.App.Test/ContainerFixture.cs index 3893a898f..59098aa77 100644 --- a/NzbDrone.App.Test/ContainerFixture.cs +++ b/NzbDrone.App.Test/ContainerFixture.cs @@ -1,10 +1,13 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using NUnit.Framework; +using NzbDrone.Common; using NzbDrone.Common.Messaging; using NzbDrone.Core.Download; using NzbDrone.Core.Indexers; using NzbDrone.Test.Common; using FluentAssertions; +using TinyIoC; namespace NzbDrone.App.Test { @@ -28,5 +31,24 @@ namespace NzbDrone.App.Test { MainAppContainerBuilder.BuildContainer().Resolve>().Should().NotBeEmpty(); } + + [Test] + public void container_should_inject_itself() + { + var factory = MainAppContainerBuilder.BuildContainer().Resolve(); + + factory.Build().Should().NotBeNull(); + } + + [Test] + public void should_resolve_command_executor_by_name() + { + var genericExecutor = typeof(IExecute<>).MakeGenericType(typeof(RssSyncCommand)); + + var executor = MainAppContainerBuilder.BuildContainer().Resolve(genericExecutor); + + executor.Should().NotBeNull(); + executor.Should().BeAssignableTo>(); + } } } \ No newline at end of file diff --git a/NzbDrone.Common.Test/EventingTests/MessageAggregatorCommandTests.cs b/NzbDrone.Common.Test/EventingTests/MessageAggregatorCommandTests.cs index b82d79820..35f8b036c 100644 --- a/NzbDrone.Common.Test/EventingTests/MessageAggregatorCommandTests.cs +++ b/NzbDrone.Common.Test/EventingTests/MessageAggregatorCommandTests.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using Moq; using NUnit.Framework; using NzbDrone.Common.Messaging; @@ -8,31 +7,35 @@ using NzbDrone.Test.Common; namespace NzbDrone.Common.Test.EventingTests { [TestFixture] - public class MessageAggregatorCommandTests : TestBase + public class MessageAggregatorCommandTests : TestBase { - [Test] - public void should_publish_command_to_executor() + private Mock> _executorA; + private Mock> _executorB; + + [SetUp] + public void Setup() { - var commandA = new CommandA(); + _executorA = new Mock>(); + _executorB = new Mock>(); - var executor = new Mock>(); - var aggregator = new MessageAggregator(TestLogger, () => new List { executor.Object }); - aggregator.PublishCommand(commandA); + Mocker.GetMock() + .Setup(c => c.Build(typeof(IExecute))) + .Returns(_executorA.Object); - executor.Verify(c => c.Execute(commandA), Times.Once()); - } + Mocker.GetMock() + .Setup(c => c.Build(typeof(IExecute))) + .Returns(_executorB.Object); + } [Test] - public void should_throw_if_more_than_one_handler() + public void should_publish_command_to_executor() { var commandA = new CommandA(); - var executor1 = new Mock>(); - var executor2 = new Mock>(); - var aggregator = new MessageAggregator(TestLogger, () => new List { executor1.Object, executor2.Object }); + Subject.PublishCommand(commandA); - Assert.Throws(() => aggregator.PublishCommand(commandA)); + _executorA.Verify(c => c.Execute(commandA), Times.Once()); } [Test] @@ -40,14 +43,11 @@ namespace NzbDrone.Common.Test.EventingTests { var commandA = new CommandA(); - var executor1 = new Mock>(); - var executor2 = new Mock>(); - var aggregator = new MessageAggregator(TestLogger, () => new List { executor1.Object, executor2.Object }); - aggregator.PublishCommand(commandA); + Subject.PublishCommand(commandA); - executor1.Verify(c => c.Execute(commandA), Times.Once()); - executor2.Verify(c => c.Execute(It.IsAny()), Times.Never()); + _executorA.Verify(c => c.Execute(commandA), Times.Once()); + _executorB.Verify(c => c.Execute(It.IsAny()), Times.Never()); } [Test] @@ -55,13 +55,10 @@ namespace NzbDrone.Common.Test.EventingTests { var commandA = new CommandA(); - var executor = new Mock>(); - - executor.Setup(c => c.Execute(It.IsAny())) + _executorA.Setup(c => c.Execute(It.IsAny())) .Throws(new NotImplementedException()); - var aggregator = new MessageAggregator(TestLogger, () => new List { executor.Object }); - Assert.Throws(() => aggregator.PublishCommand(commandA)); + Assert.Throws(() => Subject.PublishCommand(commandA)); } } diff --git a/NzbDrone.Common.Test/EventingTests/MessageAggregatorEventTests.cs b/NzbDrone.Common.Test/EventingTests/MessageAggregatorEventTests.cs index e30d71339..52daf6148 100644 --- a/NzbDrone.Common.Test/EventingTests/MessageAggregatorEventTests.cs +++ b/NzbDrone.Common.Test/EventingTests/MessageAggregatorEventTests.cs @@ -8,33 +8,42 @@ using NzbDrone.Test.Common; namespace NzbDrone.Common.Test.EventingTests { [TestFixture] - public class MessageAggregatorEventTests : TestBase + public class MessageAggregatorEventTests : TestBase { - [Test] - public void should_publish_event_to_handlers() + private Mock> HandlerA1; + private Mock> HandlerA2; + + private Mock> HandlerB1; + private Mock> HandlerB2; + + + [SetUp] + public void Setup() { - var eventA = new EventA(); + HandlerA1 = new Mock>(); + HandlerA2 = new Mock>(); + HandlerB1 = new Mock>(); + HandlerB2 = new Mock>(); + Mocker.GetMock() + .Setup(c => c.BuildAll>()) + .Returns(new List> { HandlerA1.Object, HandlerA2.Object }); - var intHandler = new Mock>(); - var aggregator = new MessageAggregator(TestLogger, () => new List { intHandler.Object }); - aggregator.PublishEvent(eventA); + Mocker.GetMock() + .Setup(c => c.BuildAll>()) + .Returns(new List> { HandlerB1.Object, HandlerB2.Object }); - intHandler.Verify(c => c.Handle(eventA), Times.Once()); } [Test] - public void should_publish_to_more_than_one_handler() + public void should_publish_event_to_handlers() { var eventA = new EventA(); - var intHandler1 = new Mock>(); - var intHandler2 = new Mock>(); - var aggregator = new MessageAggregator(TestLogger, () => new List { intHandler1.Object, intHandler2.Object }); - aggregator.PublishEvent(eventA); + Subject.PublishEvent(eventA); - intHandler1.Verify(c => c.Handle(eventA), Times.Once()); - intHandler2.Verify(c => c.Handle(eventA), Times.Once()); + HandlerA1.Verify(c => c.Handle(eventA), Times.Once()); + HandlerA2.Verify(c => c.Handle(eventA), Times.Once()); } [Test] @@ -42,14 +51,14 @@ namespace NzbDrone.Common.Test.EventingTests { var eventA = new EventA(); - var aHandler = new Mock>(); - var bHandler = new Mock>(); - var aggregator = new MessageAggregator(TestLogger, () => new List { aHandler.Object, bHandler.Object }); - aggregator.PublishEvent(eventA); + Subject.PublishEvent(eventA); - aHandler.Verify(c => c.Handle(eventA), Times.Once()); - bHandler.Verify(c => c.Handle(It.IsAny()), Times.Never()); + HandlerA1.Verify(c => c.Handle(eventA), Times.Once()); + HandlerA2.Verify(c => c.Handle(eventA), Times.Once()); + + HandlerB1.Verify(c => c.Handle(It.IsAny()), Times.Never()); + HandlerB2.Verify(c => c.Handle(It.IsAny()), Times.Never()); } @@ -58,19 +67,14 @@ namespace NzbDrone.Common.Test.EventingTests { var eventA = new EventA(); - var intHandler1 = new Mock>(); - var intHandler2 = new Mock>(); - var intHandler3 = new Mock>(); - intHandler2.Setup(c => c.Handle(It.IsAny())) + HandlerA1.Setup(c => c.Handle(It.IsAny())) .Throws(new NotImplementedException()); - var aggregator = new MessageAggregator(TestLogger, () => new List { intHandler1.Object, intHandler2.Object , intHandler3.Object}); - aggregator.PublishEvent(eventA); - - intHandler1.Verify(c => c.Handle(eventA), Times.Once()); - intHandler3.Verify(c => c.Handle(eventA), Times.Once()); + Subject.PublishEvent(eventA); + HandlerA1.Verify(c => c.Handle(eventA), Times.Once()); + HandlerA2.Verify(c => c.Handle(eventA), Times.Once()); ExceptionVerification.ExpectedErrors(1); } diff --git a/NzbDrone.Common/ContainerBuilderBase.cs b/NzbDrone.Common/ContainerBuilderBase.cs index 735f33c6e..ccde67f0b 100644 --- a/NzbDrone.Common/ContainerBuilderBase.cs +++ b/NzbDrone.Common/ContainerBuilderBase.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; +using NzbDrone.Common.Messaging; using TinyIoC; namespace NzbDrone.Common @@ -27,9 +28,16 @@ namespace NzbDrone.Common private void AutoRegisterInterfaces() { - var interfaces = _loadedTypes.Where(t => t.IsInterface); + var simpleInterfaces = _loadedTypes.Where(t => t.IsInterface).ToList(); + var appliedInterfaces = _loadedTypes.SelectMany(t => t.GetInterfaces()).Where(i => i.Assembly.FullName.Contains("NzbDrone")).ToList(); - foreach (var contract in interfaces) + var contracts = simpleInterfaces.Union(appliedInterfaces) + .Except(new List { typeof(IMessage), typeof(ICommand), typeof(IEvent) }); + + + var count = contracts.Count(); + + foreach (var contract in simpleInterfaces.Union(contracts)) { AutoRegisterImplementations(contract); } diff --git a/NzbDrone.Common/Messaging/MessageAggregator.cs b/NzbDrone.Common/Messaging/MessageAggregator.cs index 6089dd4ec..041dc2cba 100644 --- a/NzbDrone.Common/Messaging/MessageAggregator.cs +++ b/NzbDrone.Common/Messaging/MessageAggregator.cs @@ -1,6 +1,5 @@ using System; -using System.Collections.Generic; -using System.Linq; +using System.Reflection; using System.Threading.Tasks; using NLog; @@ -9,24 +8,20 @@ namespace NzbDrone.Common.Messaging public class MessageAggregator : IMessageAggregator { private readonly Logger _logger; - private readonly Func> _handlerFactory; + private readonly IServiceFactory _serviceFactory; - public MessageAggregator(Logger logger, Func> handlers) + public MessageAggregator(Logger logger, IServiceFactory serviceFactory) { _logger = logger; - _handlerFactory = handlers; + _serviceFactory = serviceFactory; } public void PublishEvent(TEvent @event) where TEvent : IEvent { _logger.Trace("Publishing {0}", @event.GetType().Name); - - var handlers = _handlerFactory().ToList(); - - //call synchronous handlers first. - foreach (var handler in handlers.OfType>()) + foreach (var handler in _serviceFactory.BuildAll>()) { try { @@ -40,7 +35,7 @@ namespace NzbDrone.Common.Messaging } } - foreach (var handler in handlers.OfType>()) + foreach (var handler in _serviceFactory.BuildAll>()) { var handlerLocal = handler; Task.Factory.StartNew(() => @@ -55,10 +50,27 @@ namespace NzbDrone.Common.Messaging public void PublishCommand(TCommand command) where TCommand : ICommand { + var handlerContract = typeof(IExecute<>).MakeGenericType(command.GetType()); + _logger.Trace("Publishing {0}", command.GetType().Name); - var handler = _handlerFactory().OfType>().Single(); + + var handler = _serviceFactory.Build(handlerContract); + _logger.Debug("{0} -> {1}", command.GetType().Name, handler.GetType().Name); - handler.Execute(command); + + try + { + handlerContract.GetMethod("Execute").Invoke(handler, new object[] { command }); + } + catch (TargetInvocationException e) + { + if (e.InnerException != null) + { + throw e.InnerException; + } + throw; + } + _logger.Debug("{0} <- {1}", command.GetType().Name, handler.GetType().Name); } } diff --git a/NzbDrone.Common/Messaging/MessageExtensions.cs b/NzbDrone.Common/Messaging/MessageExtensions.cs new file mode 100644 index 000000000..e5f9ec2fe --- /dev/null +++ b/NzbDrone.Common/Messaging/MessageExtensions.cs @@ -0,0 +1,17 @@ +using System; + +namespace NzbDrone.Common.Messaging +{ + public static class MessageExtensions + { + public static string GetExecutorName(this Type commandType) + { + if (!typeof(ICommand).IsAssignableFrom(commandType)) + { + throw new ArgumentException("commandType must implement IExecute"); + } + + return string.Format("I{0}Executor", commandType.Name); + } + } +} \ No newline at end of file diff --git a/NzbDrone.Common/NzbDrone.Common.csproj b/NzbDrone.Common/NzbDrone.Common.csproj index 7269dd716..f31a6a3a7 100644 --- a/NzbDrone.Common/NzbDrone.Common.csproj +++ b/NzbDrone.Common/NzbDrone.Common.csproj @@ -116,7 +116,9 @@ + + @@ -161,9 +163,6 @@ - - -