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

Ability to not generate legacy NServiceBus.Transport.Encoding header #1072

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ramonsmits
Copy link
Member

Add operation extension to not generate legacy NServiceBus.Transport.Encoding header in native custom properties / ApplicationProperties

…rt.Encoding` header in native custom properties / ApplicationProperties
@ramonsmits ramonsmits self-assigned this Oct 10, 2024
var message = new ServiceBusMessage(outgoingMessage.Body)
{
// Cannot re-use MessageId to be compatible with ASB transport that could have native de-dup enabled
MessageId = Guid.NewGuid().ToString()
};

// The value needs to be "application/octect-stream" and not "application/octet-stream" for interop with ASB transport
message.ApplicationProperties[TransportMessageHeaders.TransportEncoding] = "application/octect-stream";
var disableLegacyHeaders = outgoingTransportOperation.Properties.ContainsKey(TransportOperationExt.DisableLegacyHeadersKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

If I recall correctly, those properties get serialized into the outbox too. It might be worthwhile adding a comment here to indicate why the property is not removed (because it needs to be preserved even with the outbox if I understand this correctly).

From a more general perspective I wonder if it would make sense to actually moved this up to the transport level configuration. For all users that do not need to interop the old transport this header is unnecessary. By moving it to the transport level we can make it opt-out, later opt-in and eventually deprecate it from the transport entirely.

This seems to be the more future-proof way of handling this change instead of introducing yet another configuration layer deep down on the transport operation level.

Copy link
Member Author

Choose a reason for hiding this comment

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

@danielmarbach I like your reasoning about uplifting this and not have it per operation but as an endpoint configuration. Here it would be AND also on the top level configuration as the ***** connector uses the transport seam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants