search

reactor / reactor-rabbitmq

Reactor RabbitMQ
Apache License 2.0
157 stars 55 forks source link

[#109]Implement Correlation metadata on Sender OutboundMessages #110

Closed Sage-Pierce closed 5 years ago

Sage-Pierce commented 5 years ago

Still a work-in-progress, but looking for feedback. Trying to consider an approach that isn't technically backward incompatible due to the now-necessary generic type declaration on the Flux's OutboundMessage correlation metadata type.

Sage-Pierce commented 5 years ago

@acogoluegnes @simonbasle Thank you both for the callouts and suggestions 👍 I'll refactor to attempt addressing your comments

Sage-Pierce commented 5 years ago

@acogoluegnes @simonbasle Your recommendations ended up helping me clean these changes up a bit. The main thing that needed generification was the type of OutboundMessage. Once that was generified, nothing outside of type compilation issues needed to be addressed, and I was able to reuse the existing PublishConfirmOperator as-is. I still needed to add new methods for "typed" sends in order to avoid a backward incompatible change (returning Flux<OutboundMessageResult> vs. Flux<OutboundMessageResult<T>>). I believe that makes this changeset now fully backward compatible

Sage-Pierce commented 5 years ago

The API could be made a bit more strict and restrict users from being able to use their own extensions of OutboundMessage, and only allow OutboundMessage or CorrelableOutboundMessage, but once I realized this approach worked, figured it would be worthwhile to get your opinions on it

Sage-Pierce commented 5 years ago

Thanks for the quick feedback and merge turnaround, @acogoluegnes and @simonbasle !

Is there anywhere I can monitor the release timeline for a new version that will have this update in it? Looks like 1.4.0?

acogoluegnes commented 5 years ago

This feature will indeed be in 1.4, which should be released in a few weeks. It should contain #111 as well and anything related to feedback from 1.3.