Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always process messages that have handlers regardless of conventions #5342

Merged
merged 5 commits into from Mar 19, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,62 @@
namespace NServiceBus.AcceptanceTests.Core.Conventions
{
using System.Threading.Tasks;
using AcceptanceTesting;
using AcceptanceTesting.Customization;
using EndpointTemplates;
using NUnit.Framework;

public class When_receiving_non_matching_message_with_handler : NServiceBusAcceptanceTest
{
[Test]
public async Task Should_process_message()
{
var context = await Scenario.Define<Context>()
.WithEndpoint<Sender>(c => c.When(s => s.Send(new NonMatchingMessageWithHandler())))
.WithEndpoint<Receiver>()
.Done(c => c.GotTheMessage)
.Run();

Assert.True(context.GotTheMessage);
}

public class Context : ScenarioContext
{
public bool GotTheMessage { get; set; }
}

public class Sender : EndpointConfigurationBuilder
{
public Sender()
{
EndpointSetup<DefaultServer>(c =>
{
c.ConfigureTransport().Routing().RouteToEndpoint(typeof(NonMatchingMessageWithHandler), typeof(Receiver));
});
}
}

public class Receiver : EndpointConfigurationBuilder
{
public Receiver()
{
EndpointSetup<DefaultServer>(c => c.Conventions().DefiningMessagesAs(t => false));
}

class MyHandler : IHandleMessages<NonMatchingMessageWithHandler>
{
public Context TestContext { get; set; }

public Task Handle(NonMatchingMessageWithHandler message, IMessageHandlerContext context)
{
TestContext.GotTheMessage = true;
return Task.FromResult(0);
}
}
}

public class NonMatchingMessageWithHandler : IMessage
{
}
}
}
Expand Up @@ -18,15 +18,15 @@ public class MessageHandlerRegistryTests
[TestCase(typeof(SagaWithIllegalDep))]
public void ShouldThrowIfUserTriesToBypassTheHandlerContext(Type handlerType)
{
var registry = new MessageHandlerRegistry(new Conventions());
var registry = new MessageHandlerRegistry();

Assert.Throws<Exception>(() => registry.RegisterHandler(handlerType));
}

[Test]
public async Task ShouldIndicateWhetherAHandlerIsATimeoutHandler()
{
var registry = new MessageHandlerRegistry(new Conventions());
var registry = new MessageHandlerRegistry();

registry.RegisterHandler(typeof(SagaWithTimeoutOfMessage));

Expand Down
14 changes: 7 additions & 7 deletions src/NServiceBus.Core.Tests/Unicast/HandlerInvocationCache.cs
Expand Up @@ -15,7 +15,7 @@ public class HandlerInvocationCachePerformanceTests
[Test]
public async Task RunNew()
{
var cache = new MessageHandlerRegistry(new Conventions());
var cache = new MessageHandlerRegistry();
cache.RegisterHandler(typeof(StubMessageHandler));
cache.RegisterHandler(typeof(StubTimeoutHandler));
var stubMessage = new StubMessage();
Expand Down Expand Up @@ -67,7 +67,7 @@ public class When_invoking_a_cached_message_handler
[Test]
public async Task Should_invoke_handle_method()
{
var cache = new MessageHandlerRegistry(new Conventions());
var cache = new MessageHandlerRegistry();
cache.RegisterHandler(typeof(StubHandler));

var handler = cache.GetCachedHandlerForMessage<StubMessage>();
Expand All @@ -80,7 +80,7 @@ public async Task Should_invoke_handle_method()
[Test]
public async Task Should_have_passed_through_correct_message()
{
var cache = new MessageHandlerRegistry(new Conventions());
var cache = new MessageHandlerRegistry();
cache.RegisterHandler(typeof(StubHandler));

var handler = cache.GetCachedHandlerForMessage<StubMessage>();
Expand All @@ -94,7 +94,7 @@ public async Task Should_have_passed_through_correct_message()
[Test]
public async Task Should_have_passed_through_correct_context()
{
var cache = new MessageHandlerRegistry(new Conventions());
var cache = new MessageHandlerRegistry();
cache.RegisterHandler(typeof(StubHandler));

var handler = cache.GetCachedHandlerForMessage<StubMessage>();
Expand Down Expand Up @@ -130,7 +130,7 @@ public class When_invoking_a_cached_timeout_handler
[Test]
public async Task Should_invoke_timeout_method()
{
var cache = new MessageHandlerRegistry(new Conventions());
var cache = new MessageHandlerRegistry();
cache.RegisterHandler(typeof(StubHandler));

var handler = cache.GetCachedHandlerForMessage<StubTimeoutState>();
Expand All @@ -143,7 +143,7 @@ public async Task Should_invoke_timeout_method()
[Test]
public async Task Should_have_passed_through_correct_state()
{
var cache = new MessageHandlerRegistry(new Conventions());
var cache = new MessageHandlerRegistry();
cache.RegisterHandler(typeof(StubHandler));

var stubState = new StubTimeoutState();
Expand All @@ -157,7 +157,7 @@ public async Task Should_have_passed_through_correct_state()
[Test]
public async Task Should_have_passed_through_correct_context()
{
var cache = new MessageHandlerRegistry(new Conventions());
var cache = new MessageHandlerRegistry();
cache.RegisterHandler(typeof(StubHandler));

var handler = cache.GetCachedHandlerForMessage<StubTimeoutState>();
Expand Down
Expand Up @@ -11,7 +11,7 @@ public class LoadHandlersBehaviorTests
[Test]
public void Should_throw_when_there_are_no_registered_message_handlers()
{
var behavior = new LoadHandlersConnector(new MessageHandlerRegistry(new Conventions()), new InMemorySynchronizedStorage(), new InMemoryTransactionalSynchronizedStorageAdapter());
var behavior = new LoadHandlersConnector(new MessageHandlerRegistry(), new InMemorySynchronizedStorage(), new InMemoryTransactionalSynchronizedStorageAdapter());

var context = new TestableIncomingLogicalMessageContext();

Expand Down
Expand Up @@ -43,7 +43,7 @@ static void LoadMessageHandlers(FeatureConfigurationContext context, List<Type>

static void ConfigureMessageHandlersIn(FeatureConfigurationContext context, IEnumerable<Type> types)
{
var handlerRegistry = new MessageHandlerRegistry(context.Settings.Get<Conventions>());
var handlerRegistry = new MessageHandlerRegistry();

foreach (var t in types.Where(IsMessageHandler))
{
Expand Down
12 changes: 0 additions & 12 deletions src/NServiceBus.Core/Unicast/MessageHandlerRegistry.cs
Expand Up @@ -13,11 +13,6 @@
/// </summary>
public class MessageHandlerRegistry
{
internal MessageHandlerRegistry(Conventions conventions)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to keep an internal constructor around to prevent the default public constructor from being added.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we made the ctor internal back in the day. Do we see any drawback in exposing the public ctor?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't suppose there is any problem with allowing this. You'll need to update the API approval test, then.

{
this.conventions = conventions;
}

/// <summary>
/// Gets the list of handlers <see cref="Type" />s for the given
/// <paramref name="messageType" />.
Expand All @@ -26,10 +21,6 @@ internal MessageHandlerRegistry(Conventions conventions)
public List<MessageHandler> GetHandlersFor(Type messageType)
{
Guard.AgainstNull(nameof(messageType), messageType);
if (!conventions.IsMessageType(messageType))
{
return noMessageHandlers;
andreasohlund marked this conversation as resolved.
Show resolved Hide resolved
}

var messageHandlers = new List<MessageHandler>();
// ReSharper disable once LoopCanBeConvertedToQuery
Expand Down Expand Up @@ -60,7 +51,6 @@ public IEnumerable<Type> GetMessageTypes()
return (from messagesBeingHandled in handlerAndMessagesHandledByHandlerCache.Values
from typeHandled in messagesBeingHandled
let messageType = typeHandled.MessageType
where conventions.IsMessageType(messageType)
select messageType).Distinct();
}

Expand Down Expand Up @@ -181,9 +171,7 @@ void ValidateHandlerType(Type handlerType)
}
}

readonly Conventions conventions;
readonly Dictionary<Type, List<DelegateHolder>> handlerAndMessagesHandledByHandlerCache = new Dictionary<Type, List<DelegateHolder>>();
static List<MessageHandler> noMessageHandlers = new List<MessageHandler>(0);
static ILog Log = LogManager.GetLogger<MessageHandlerRegistry>();

class DelegateHolder
Expand Down