Particular / docs.particular.net

All content for ParticularDocs
https://docs.particular.net
Other
103 stars 300 forks source link

Feedback: 'Native message access' #6225

Open SeanFeldman opened 11 months ago

SeanFeldman commented 11 months ago

Feedback for 'Native message access' https://docs.particular.net/transports/azure-service-bus/native-message-access

Location in GitHub: https://github.com/Particular/docs.particular.net/blob/master/transports/azure-service-bus/native-message-access.md

It would be helpful to show how to test the effect of customizations when testing handlers.

DavidBoike commented 11 months ago

I'm pretty sure that, at least right now, it's not going to be possible to test that, as the testing framework doesn't know about any transport-specific bits. The customize method is just dropping an Action<ServiceBusMessage> into the extensions. I'm not sure how you assert on that.

SeanFeldman commented 11 months ago

This is what I've done to verify the customization

    [Theory]
    [InlineData(MessageIntent.Publish)]
    [InlineData(MessageIntent.Send)]
    [InlineData(MessageIntent.Reply)]
    async Task Preserve_original_message_intent(MessageIntent messageIntent)
    {
        var context = new TestableMessageHandlerContext();
        context.MessageHeaders[Headers.MessageIntent] = messageIntent.ToString();

        var handler = new SomeHandler();
        await handler.Handle(new SomeEvent(), context);

        SentMessage<object> sentMessage = context.SentMessages.First();
        var customizationAction = sentMessage.Options.GetExtensions().Get<Action<ServiceBusMessage>>("$ASB.CustomizationId");
        var nativeMessage = new ServiceBusMessage();
        customizationAction(nativeMessage);

        nativeMessage.ApplicationProperties[Headers.MessageIntent].Should().Be(messageIntent.ToString());
    }
DavidBoike commented 11 months ago

I keep staring at that trying to figure out how to make that a better testing story but I have difficulty imaginging anything that does anything but shorten it by a line or two.

DavidBoike commented 11 months ago

More to the point, are you trying to test the effect of the Action or make sure the framework actually calls it?

SeanFeldman commented 11 months ago

More to the point, are you trying to test the effect of the Action or make sure the framework actually calls it?

I trust NSB will execute it. That's not the concern. The concern is whatever is the change made in the customization.

I keep staring at that trying to figure out how to make that a better testing story but I have difficulty imaginging anything that does anything but shorten it by a line or two.

The concern shouldn't be focused on how to shorten the code I've shared. The concern is with this line:

var customizationAction = sentMessage.Options.GetExtensions().Get<Action>("$ASB.CustomizationId");

I shouldn't be reaching into the extension to pull out an action by the key that's undocumented and could be changed at any time. Rather, I should use the testing API to access the outcomes of the action execution, similar to how outgoing messages are provided. That's what I'd focus on if I'd think about API to provide.

DavidBoike commented 11 months ago

So then the difficulty becomes that would belong more in a NServiceBus.Transport.AzureServiceBus.Testing, which is a package we don't currently ship, and it would have just that one extension method GetMessageCustomizer() (or whatever) in it, right? I don't see that being appropriate in the transport package proper.

SeanFeldman commented 11 months ago

The issue is getting access to that callback. Exposing something in the transport package is not such a bad thing. It's not uncommon to mark privates as internal to have access for testing purposes. So why can't a callback accessor be shipped with the transport and used for testing? And if the number of these "workarounds" is growing, then there's a justification to have a standalone NServiceBus.Transport.AzureServiceBus.Testing package.