Skip to content
This repository has been archived by the owner on Aug 3, 2024. It is now read-only.
/ ServerCommon Public archive

GetRawBody method #423

Merged
merged 1 commit into from
Feb 24, 2024
Merged

GetRawBody method #423

merged 1 commit into from
Feb 24, 2024

Conversation

agr
Copy link
Contributor

@agr agr commented Feb 24, 2024

One of the monitoring jobs requires access to raw message body data to properly deserialize the message. Calling BrokeredMessage.GetBody<Stream> worked with the previous version of the Service Bus library. Migrating to new version broke that (as there is no direct equivalent and our reimplementation disregarded that case). Adding explicit method to support that scenario.

Another potential solution here is to update GetBody<TStream> implementation to check if TStream is System.IO.Stream and do something else, but I'm not sure what else it might break. Creating new method is safer.

@agr agr requested a review from a team as a code owner February 24, 2024 00:52
@@ -20,5 +21,6 @@ public interface IReceivedBrokeredMessage
Task AbandonAsync();
string GetBody();
TStream GetBody<TStream>();
Stream GetRawBody();
Copy link

Choose a reason for hiding this comment

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

This is a breaking change on a public interface, do we need to be concerned about that here?

Copy link
Contributor Author

@agr agr Feb 24, 2024

Choose a reason for hiding this comment

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

We're the only consumer and the only meaningful implementation is shipped with the interface. It might break tests (in case we have test implementation of that interface), but I'll fix those as I update the package in consuming projects. So, I don't think there is a concern.

@agr agr merged commit f220838 into main Feb 24, 2024
2 checks passed
@agr agr deleted the agr-sb-rawdata branch March 28, 2024 18:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants