search

paolosalvatori / ServiceBusExplorer

The Service Bus Explorer allows users to connect to a Service Bus namespace and administer messaging entities in an easy manner. The tool provides advanced features like import/export functionality or the ability to test topic, queues, subscriptions, relay services, notification hubs and events hubs.
MIT License
2.01k stars 584 forks source link

Resubmitting message with changed message body #492

Closed djstanic1 closed 3 years ago

djstanic1 commented 3 years ago

Switched to 5.0.1.

When updating the message body (change anything in the body) and resubmit the message, the message gets resubmitted, but nothing is changed inside the body, so the message stays unchanged.

paolosalvatori commented 3 years ago

@SeanFeldman @ErikMogensen do you happen to know the root cause of this regression bug?

ErikMogensen commented 3 years ago

It is the fix in #247. It uses the content of the existing message and ignores the content in the GUI.

This is in a complicated part of the code and difficult to get right with all the different message formats.

paolosalvatori commented 3 years ago

@ErikMogensen got it, how do we want to handle it? Is the fix done by @SeanFeldman in src/ServiceBusExplorer/Forms/MessageForm.cs file that causes the issue? I mean, we can change the code to solve the issue. The goal is to let users change the content of the payload and resubmit the message. The difficult part was the WCF format. Do we still support this format after migrating from Microsoft.Azure.ServiceBus to Azure.Messaging.ServiceBus?

ErikMogensen commented 3 years ago

It wasn't Sean who implemented it. I suggest we revert the change as a short term solution.

This part of the code is still using the old Service Bus SDK. If we switch to the latest SDK, Azure.Messaging.ServiceBus I am pretty sure we cannot use WCF. I would like to see that our aim was to replace all of the old SDK with the new SDK but then SBE would probably not be able to handle some old messages and send the messages in the same format as before.

There is more about this at https://github.com/Azure/azure-service-bus-dotnet/issues/239#issuecomment-320820562

SeanFeldman commented 3 years ago

Thank you @ErikMogensen. The last time I've touched that form was 4 years ago and AFAIK it wasn't broken 😉 Git Blame is a powerful tool @paolosalvatori.

If we switch to the latest SDK, Azure.Messaging.ServiceBus I am pretty sure we cannot use WCF.

Correct. Though I'm not entirely sure what do you mean by "we cannot use WCF". We can't use SBMP but that's not something that would stop us. If you're referring to BodyType.Wcf, that's not WCF APIs but the payload, which can be achieved. That'd be for really old systems though.

but then SBE would probably not be able to handle some old messages and send the messages in the same format as before.

Incorrect.

Message interoperability is not a function of the SDK but how the payload is sent and received.

paolosalvatori commented 3 years ago

Message interoperability has never been a function of the SDK @SeanFeldman. This is why I implemented WCF support in the tool years ago. At that time I spent a great amount of time to make it work and guarantee that Service Bus messages could be sent or received to guarantee interoperability with BizTalk Server WCF adapters. Now, the context has changed, I don't think there is anybody out there that use WCF anymore, so we can remove the support for this format. I don't get what you intended to demonstrate with Git Blame @SeanFeldman as I'm perfectly aware of what I did, including the code for cloning messages when using WCF payloads :) The point is different: if we think that WCF support is obsolete or it's a problem to maintain (e.g. message cloning), we can just remove it from SBE. This is why I was asking.

SeanFeldman commented 3 years ago

Git Blame was to demonstrate how to quickly identify who was involved in changes and PRs affecting the files under review. Eliminates guessing.

Dropping WCF body body format is fine. It was mostly coming from track 0 SDK which is quite old and still can be found in use here and there.

paolosalvatori commented 3 years ago

Sounds good, but you didn't have to demonstrate to me who wrote what :) I quickly asked just because I'm still on holiday and I wrote the previous comment via my mobile, not from my laptop :) We'll eventually remove WCF support.

SeanFeldman commented 3 years ago

Is the fix done by @SeanFeldman in src/ServiceBusExplorer/Forms/MessageForm.cs file that causes the issue?

The link I've provided would answer your question. I'm a bit sensitive when my name is associated with something I didn't do. Given that the repo has multiple contributors and we don't have a solid way to identify who causes a regression, I'd rather use PR/commit links and not names. But that's a personal preference, right? 🙂

Responding to comments from a phone is always a challenge if it's not a trivial "yes" or "no". That's why I'm training myself to resists answering from my phone if it requires more than one-liner. Am I successful at my own preaching? Still a way to go 😀

paolosalvatori commented 3 years ago

I used the wrong words, I wanted to say "who did review / approve the PR in addition to me?" It's clear that #247 was submitted by somebody else. :)