rebus-org / Rebus.AzureServiceBus

:bus: Azure Service Bus transport for Rebus
https://mookid.dk/category/rebus
Other
33 stars 20 forks source link

feat(message): use ServiceBusMessage properties #88

Closed riezebosch closed 1 year ago

riezebosch commented 1 year ago

Remove rbs2- headers that for data that is already available on the outgoing ServiceBusMessage Use SericeBusMessage properties from incoming message to recreate custom rbs2- headers


Rebus is MIT-licensed. The code submitted in this pull request needs to carry the MIT license too. By leaving this text in, I hereby acknowledge that the code submitted in the pull request has the MIT license and can be merged with the Rebus codebase.

riezebosch commented 1 year ago

I understand this change implies that all consumers connected to the bus needed to upgrade. If you're open to this change, I might also introduce an opt-out flag to still use the new behavior on incoming messages but still publish the rbs2-* custom properties.

riezebosch commented 1 year ago

This improves interoperability with other producers by using the provided Id and CorrelationId on the incoming messages. I can split this PR if you like?

mookid8000 commented 1 year ago

Hi @riezebosch , I actually quite like this change, BUT I think it should be opt-in. 🙂

Ideally, the transport initially worked like this, but it didn't, and that's why it does what it does today: Transfers all Rebus headers as custom properties, exposing some of them to tools (like Service Bus Explorer) for specific headers that have also make sense to Azure Service Bus.

Could you make it possible to e.g.

services.AddRebus(
    configure => configure
        .Transport(t => t.UseAzureServiceBus(...)
            .UseOnlyNativeHeadersWhenPossible())
);

or something like that?

riezebosch commented 1 year ago

Is it enough to only opt-out from the removal of rbs2-* headers? The copy of message-id and correlation-id have been there for quite some time already. So you can safely assume on incoming messages that the message-id and correlation-id of the service bus message equals the value of the rbs2-* headers.

mookid8000 commented 1 year ago

Is it enough to only opt-out from the removal of rbs2- headers? The copy of message-id and correlation-id have been there for quite some time already. So you can safely assume on incoming messages that the message-id and correlation-id of the service bus message equals the value of the rbs2- headers.

I guess you are right about the headers already present in the message's native fields. So yes I think so (but we'll need to test this with a couple of betaversions before committing to it)

riezebosch commented 1 year ago

I've updated it to provide a boolean flag to opt-in for the default header removal. But then refactored it into the concepts I saw in the main Rebus package. Message conversion is decoupled from transport and injected, and the headers are removed via decoration (using a configuration extension).

This also creates an extension point for others to do different message conversions.

I'd love to do the same for separating the transport from the topics and queues management.

mookid8000 commented 1 year ago

Hi @riezebosch , it looks really good 🙂

I know this is a small thing, but could you maybe rename IMessageConvert to IMessageConverter? (along with its implementation)

mookid8000 commented 1 year ago

btw there's a test failing, but I can see it's a test that I made 😅 so I'll merge your PR now!

mookid8000 commented 1 year ago

Thanks for contributing 👍