When an event is derived from multiple interfaces that don't have a hierarchy, only handlers for the first interface are called

[particularbot]

Symptoms

When an event, which concrete type is not shared between publisher and subscriber, is composed of multiple interfaces where there is no hierarchy among them, only handlers for the first interface are called. Handlers for the other interfaces are not invoked.

Who's affected

Affected are users of all versions of NServiceBus that want to use client-driven contracts (interfaces defined by message consumers, message implementation private to the producer) and there is at least once consumer that wants to consume more than one event interface

Details

For example, a publisher publishes event of type MyEvent:

public class MyEvent :  ISomeInterface, ISomeOtherInterface
{
}

The interfaces are defined in a common library as follows:

public interface ISomeInterface
{
}

public interface ISomeOtherInterface
{
}

A subscriber that has the following message handler does not get invoked

public class MyHandler : IHandleMessages<ISomeOtherInterface>
{
    public void Handle(ISomeOtherInterface message)
    {
        Console.WriteLine("Message received");
    }
}

More details, please see:
https://groups.google.com/forum/#!msg/particularsoftware/9COmFPo3MIo/Ua7ZRcRvKAUJ

[indualagarsamy]

This looks incorrect:
The number of types for the logical message is 0
https://github.com/Particular/NServiceBus/blob/develop/src/NServiceBus.Core/Unicast/Behaviors/LoadHandlersBehavior.cs#L25
And the instance of the LogicalMessage is that of the first interface instead of the type MyEvent

[johnsimons]

The number of types for the logical message is 0

Yes that does not look good

And the instance of the LogicalMessage is that of the first interface instead of the type MyEvent

That looks right, because the subscriber does not know about MyEvent

[johnsimons]

This looks like a hotfix

[johnsimons]

Is it only 4.6 that is broken?

[andreasohlund]

Is this really a hotfix? It will show during development and there is a workaround?

[johnsimons]

@andreasohlund yes it shows during development but the devil is in the detail!
Lets say that this works fine in version < 4.6, and the user has inflight messages when they decide to upgrade to v4.6, what happens then?
They miss that message firing, and there is no way to make it work, the only way i see is for them to downgrade to the working version.

I think first we need to find out if this is only broken in 4.6 or all versions.

[andreasohlund]

I think first we need to find out if this is only broken in 4.6 or all versions.
Agreed but looking at the v3+v4 code I'm convinced that this has been broken since the dawn of time.

After talking to @udidahan I belive this is not a hotfix since this issue would show up in development and therefore doesn't effect production environments. That said it must be fixed for v5 and the only way I see us fixing this is adding better header info that tells us the inheritance structure of the message.

This means backporting this new header to older versions so this becomes a hotfix anyway since we need to get this done before releasing v5.

Thoughts?

[andreasohlund]

To solve this correctly we'll need a better header to convey the inheritance hierarchy, how about a new header with name NServiceBus.MessageHierarchy

Given: class MyEvent: IMyEvent, IMySecondEvent
Value: "MyEvent:IMyEvent,IMySecondEvent"

Given:

class MyEvent: IMyEvent
interface IMyEvent: IMySecondEvent 

Value: "MyEvent:IMyEvent;IMyEvent:IMySecondEvent"

So : means inherits and ; means defining a new "subtree"

This header will only be present for single messages (not for the multimessages in < v5)

Thoughts?

[kijanawoodard]

This is certainly not a hot fix. Everything works fine if the base message is in the messages assembly with the two interfaces.

I think it's a bit of an edge case to want an event class that defines the base message in a non shared assembly.

[andreasohlund]

This also challenges a core assumption that each logical message in the payload translates to a logical message in the code. But in this case we might see this as 2 different messages since we have no knowledge of their "root" type. Another options is to have our mapper create a _impl class that implements both interfaces but that also breaks down since out code base assume there to always be a known root message type:

https://github.com/Particular/NServiceBus/blob/develop/src/NServiceBus.Core/Unicast/Messages/MessageMetadata.cs#L28

Lets talk this through tomorrow!

cc @udidahan

[lukemcgregor]

Thanks for looking into this issue. @andreasohlund @johnsimons In terms of versions I think this has behaved like this in all prev versions. I actually came across this in a 3.3 build of NSB and then upgraded to 4.6 to see if it was fixed there. I think a fix in v5 rather than a hotfix sounds fine.

[kijanawoodard]

@lukemcgregor What's the use case for having the published base message in a non-shared assembly?

Despite how that sentence may read, I'm not trolling. I want to understand what your motivations are so that, if code changes are made, they meet your real objectives, not the ones we all may be inventing in our minds.

[johnsimons]

@kijanawoodard I think this is what @udidahan talks about when talking about message composition.
So it is a valid and useful scenario.

[kijanawoodard]

@johnsimons not sure I'm following. The message composition aspects work perfectly if the base message is in the messages assembly.

What is at question is whether you should be able to Publish a message that isn't in a shared assembly, but implements interfaces that are.

Two things:

1. I was surprised this doesn't work already.
2. In the longer term, Assembly Neutral Interfaces change this conversation.

That said, I was trying to find out what @lukemcgregor is trying to accomplish by having the base message in a non-shared assembly. That in itself might be interesting. It might also color any implementation./fix.

[johnsimons]

What is at question is whether you should be able to Publish a message that isn't in a shared assembly, but implements interfaces that are.

I guess that is the bug, it should be possible.

I was surprised this doesn't work already.

We were too

[lukemcgregor]

I thought it might be good to outline (as requested by @kijanawoodard) a little more of one of the cases where I would like to use this kind of multiple interface messaging.

Probably the clearest case where we want to do this in real code is when we want to do versioning on our messages. We version the events we publish with a namespace convention for breaking changes.

For example:

    namespace Events.v1
    {
        public interface ISomething
        {
            bool Status{get;set;}
        }
    }

    namespace Events.v2
    {
        public interface ISomething
        {
            int Status{get;set;}//changing types will be a breaking change
        }
    }

    namespace Service.Implementations.Events
    {
        //its conveying the same meaning => same message but two possible interpretations
        public class Something : v1.ISomething, v2.ISomething
        {
            bool v1.ISomething.Status{get;set;}
            int v2.ISomething.Status{get;set;}
        }
    }

This way we can publish a single event which is backward compatible to both versions of the published interfaces.

[johnsimons]

So after reading all the comments it seems this should have been fixed in v5 but was not.

So is this still a priority?

[SimonCropp]

@johnsimons i would say high since it is effectively partial message loss

[johnsimons]

@SimonCropp I have marked it for triage, is that correct?

[SimonCropp]

triage is only if we cant agree "if it should be fixed". i dont think there is any doubt that this one should be. unless you think it is not a high prio??

[johnsimons]

ok updated

[SzymonPobiega]

Don't understand the need for an extra header. Correct me if I am wrong:

• The enclosed types header assumes the types form a single tree, not a forest, and the root (most concrete type) is the first one
• The receiver filters our all the types he does not understand (can't find metadata for them)
• If we send a composite message that is not included in the shared assembly, it is correctly added as the first type in enclosed types header but is removed by the receiver. Now the list contains essentially two one-element type trees.
• Serializers (XML and JSON), unless serialized form explicitly contains more than one message, instantiate only the first type assuming that following types are less concrete.

I propose to fix this by adding logic to serializers (to be extracted somewhere else in V6) that analyzes the type list and breaks it down into trees. It should treat each tree separately as in multi-message but use the same body to instantiate different types.