Support Duplicate Detection

[jcono]

Azure Service Bus supports a duplicate message detection mechanism (https://docs.microsoft.com/en-us/azure/service-bus-messaging/duplicate-detection).

To allow this the MessageId needs to be able to be set on the Microsoft.Azure.ServiceBus.Message to a unique value. This is the simplest way I could think to get this done.

Basically a message can optionally implement the IUniqueMessage interface which sets the MessageId appropriately. The service bus then can detect the duplicates.

[niemyjski]

Thanks for this pr. I think this makes more sense just storing the message id in the IMessage.Properties, but it sounds like from that article it needs to be stored on the message itself. Perhaps it would make sense to store a this as a first class property on IMessage (as well as correlation id?) @ejsmith? Also please feel free to join our discord to chat with us in real time.

[jcono]

Thanks @niemyjski .

From the article:

Application control of the identifier is essential, because only that allows the application to tie the MessageId to a business process context from which it can be predictably reconstructed when a failure occurs.

This is pretty crucial. It means it's up to the code sending the message (i.e external to this library) to determine what the identifier should be (and thus what is a duplicate). There are probably a few ways to do this but basically the identifier needs to be passed in the parameters to IMessageBus.PublishAsync. The message itself makes the most sense to me as then it makes it to the subscriber as well but perhaps you have other ways in mind?

It looks to me like the IMessage interface seems like it's basically an internal one so not sure how that would work?

Also I wasn't sure how specific to Azure Service Bus this was so didn't want it to creep into the base library.

Also please feel free to join our discord to chat with us in real time.

I think time difference might make that impractical but I'll take a look :)

[ejsmith]

Could this be part of the message options? I think I'm good with it being an interface we check for as well.

[jcono]

Apologies @ejsmith. I've been distracted for the last few days.

Could this be part of the message options?

What message options do you mean?

[jcono]

I have discovered another issue that's kind of related to duplicate detection in the queue implementation as well so I've included the fix for that. Basically when a message has a specified identifier then you can't use that as the lock token in the queue anymore. I wasn't sure of the consequences of altering the id on the QueueEntry object so I added the lock token value in as a properties on the entry to be recalled later when conducting lock related work.

Hope that's ok to include in this PR. It all seems related to duplicate detection to me.

[niemyjski]

What message options do you mean?

I'm not 100% sure on the context of this, he will be back shortly from vacation.

Nice find on the lock, is this property guaranteed to be in properties or is it a setting you must set when creating the queue. Any docs you could link to would go a long ways for documentation. I think this is good to merge, my only change would be to possibly add MessageId/CorrelationId to the IMessage interface. We can discus this in discord too.

[jcono]

Sorry guys. Have had zero time to put into this in recent weeks.

[jcono]

Regarding the lock token, I think since the receive mode is hard coded and therefore there is definitely a lock token on the message then this should all be ok.

If the library decides to support more receive modes in the future then you might have to confirm more about whether there is a lock token always available.

[ejsmith]

Apologies @ejsmith. I've been distracted for the last few days.

Could this be part of the message options?

What message options do you mean?

Sorry for the delayed response. I was referring to this:
https://github.com/FoundatioFx/Foundatio/blob/a54157463200a6433812c119143b7290f22af1b6/src/Foundatio/Queues/IQueue.cs#L67

That is allowing options to be set for a queue entry. I like the feature you are adding, my only concern is whether it makes sense to have the new IUniqueMessage interface that is going to be really hard to discover or if it should just be a property on the QueueEntryOptions class. Then the only concern with adding it to the QueueEntryOptions class is that it implies that it's supported on all queue implementations. I think generally speaking though that having an identifier on the queue entry is a general purpose thing. I am thinking that I would like to add it to the QueueEntryOptions class as a property called QueueEntryId and add some docs that say that the value is used to uniquely identify a queue entry.

[jcono]

Ok I see what you're referring to. How do you think the subscriber might get the id when it receives the message then? Would the options have to be passed to the subscribers handler as well?

[jcono]

@ejsmith & @niemyjski

I apologise as I've had many other things to occupy my time in the last month or two but is there anything I can do to get this merged?

[niemyjski]

@ejsmith I kinda feel we should just add this as a first class property to our messages

[jcono]

@niemyjski Do you mean force every object that is being sent as a message to have an Id?

My only thought is whether that has any relevance outside the Azure Service Bus implementation (and even then only one with duplicate detection turned on)? I guess you could potentially use a unique id on a message for other reasons?

[jcono]

@niemyjski and @ejsmith Does anything need to happen to merge this?

[niemyjski]

That is allowing options to be set for a queue entry. I like the feature you are adding, my only concern is whether it makes sense to have the new IUniqueMessage interface that is going to be really hard to discover or if it should just be a property on the QueueEntryOptions class. Then the only concern with adding it to the QueueEntryOptions class is that it implies that it's supported on all queue implementations. I think generally speaking though that having an identifier on the queue entry is a general purpose thing. I am thinking that I would like to add it to the QueueEntryOptions class as a property called QueueEntryId and add some docs that say that the value is used to uniquely identify a queue entry.

I think this was the only thing missing to being merged. Following up to your question, from reading this over breifly I think this would act just like the CorrelationId today (see usages).

[jcono]

@niemyjski Ok. My only thought is that if subscriber ever needs to know the unique id of a message (e.g. to republish) then this will have to change a little again as the information will be lost. What you're suggesting works fine for my scenario though so I guess you can deal with that when you need to.

Anyway, to get this to work with the IMessageBus there needed to be a breaking change in the dependent repository. I've created PR's for the other repositories that I think needed to change as well. I hope I haven't missed any.

• Support for UniqueId on QueueEntryOptions (with MessageBus support) Foundatio#268
• Update to support modified IMessagePublisher interface Foundatio.Redis#76
• Update to support modified IMessagePublisher interface Foundatio.RabbitMQ#30

[ejsmith]

@jcono Really sorry how long this has taken. All of the relevant PRs have been merged and the QueueEntryOptions has support for UniqueId now. If you update this PR, I will get it merged ASAP and a new version out.

[ejsmith]

@niemyjski Ok. My only thought is that if subscriber ever needs to know the unique id of a message (e.g. to republish) then this will have to change a little again as the information will be lost. What you're suggesting works fine for my scenario though so I guess you can deal with that when you need to.

If the UniqueId is being set on the BrokeredMessage as the MessageId, then that should be available later and we can still store that value in the IQueueEntryMetadata so that it can be accessed from there as well. I just wanted a way for the option to be more discoverable.

[jcono]

All done @ejsmith. Sorry for taking a few days to get to it.

If the UniqueId is being set on the BrokeredMessage as the MessageId, then that should be available later and we can still store that value in the IQueueEntryMetadata so that it can be accessed from there as well. I just wanted a way for the option to be more discoverable.

Yeah I think what I was thinking of wasn't on queues but message buses. With a queue you get the IQueueEntry<T> when you dequeue. The message bus subscriber interface only handles the type T and I couldn't see how it'd then have access to the BrokeredMessage or metadata (including the unique id).

I might have missed something thought and like I said not a deal breaker for me at the moment, but might possibly be at some point.

Thanks.

[ejsmith]

Thanks!