MQTTRetainedMessageManager improvement proposal

[logicaloud]

Currently the MQTTRetainedMessageManager invokes an event to let listeners (i.e. a backing store) know when retained messages have changed. However, events raised in the UpdateMessage method do not convey what type of update has been performed, and the backing store would need to implement the same logic as present in the UpdateMessage method to replicate the same update action (i.e., check if the application message's payload is empty).

What I'd like to propose is to change the RetainedMessageChangedEvent event signature slightly to indicate whether a message has been added, removed or replaced. The StoredRetainedMessages member can then be removed from event arguments (breaking change). Relying on the full list of stored retained messages in these events may prevent further improvements in this area.

The included Server_Invokes_Retained_Message_Events unit test demonstrates the use of the modified event.

In the future, the GetMessages method and associated logic would probably need another look as it demands that all retained messages are made available in memory. Once that is resolved then the need for a separate implementation (i.e. via IMqttRetainedMessagesManager as in #1663 or otherwise) may become less relevant.

Any thoughts?

[logicaloud]

I have looked a bit further into GetMessages / GetMessage and the steps required to avoid holding all retained messages in memory by using "Intercept" event handlers. Multiple event handlers would be required to make that efficient, i.e. for "Fetch" to intercept loading, or getting one or more messages; for "AddOrUpdate", for "Remove", for "Clear", and for "Filter". All of the event handlers should be present at the same time to make things work. Of course, there could be a "super" event that accumulates all event parameters into one, but that is ugly. This leads back to the IMqttRetainedMessageManager interface approach, which seems to be the cleaner option and is likely more efficient.

Here is my trial (I haven't tried with an actual database behind): https://github.com/logicaloud/MQTTnet/tree/improve-mqttretainedmessagemanager-with-intercept-events

It now becomes questionable whether it is worthwhile introducing a breaking change with this proposal, since it wouldn't prepare some path forward. So, I'd be happy to close. I'll leave this up for a few days in case there is some feedback, or feel free to close now.

[chkr1011]

Please see my comment here: #1764 (comment)

[logicaloud]

I close this one now; any improvements can flow into what comes out of discussion #1764.

[logicaloud]

Closing.