search

reactor / reactor-rabbitmq

Reactor RabbitMQ
Apache License 2.0
157 stars 55 forks source link

Make Sender#sendWithPublishConfirms use generics #112

Open acogoluegnes opened 5 years ago

acogoluegnes commented 5 years ago

Flux<OutboundMessageResult> sendWithPublishConfirms(Publisher<OutboundMessage>) => <OMSG extends OutboundMessage> Flux<OutboundMessageResult<OMSG>> sendWithPublishConfirms(Publisher<OMSG>).

This allows to use custom subclasses of OutboundMessage and get the instances back in the Flux<OutboundMessageResult<OMSG>>.

This is actually making Sender#sendWithTypedPublishConfirms the default.

Sender#sendWithTypedPublishConfirms should be deprecated and scheduled for removal in 3.0.

This is a breaking change as code like the following will not compile anymore:

Flux<OutboundMessageResult> confirmations = sender.sendWithPublishConfirms(...);

Code using directly the flux of confirmation is not affected.

Related to #109, #110.

mxandeco commented 4 years ago

Would make sense to extract OutboundMessage to an interface, and give the possibility of not having to extend a class to make use of the Typed publish?

acogoluegnes commented 4 years ago

This could be done in 2.0, but it's a major breaking change. Having a class in this case is an acceptable compromise between simplicity and flexibility I'd say. Could you elaborate on the limitations or use cases you're thinking about?

mxandeco commented 4 years ago

I don't think it change much for applications using libraries directly tbh, our case we are rewriting a message client/abstraction to use the reactive client as we are staring to expose some features with webflux, and the reactive just make things easier there.

Our client is not restricted to rabbit so we expose our own model (interfaces) to api consumers/clients, we abstract a lot of messaging publishing/consumption for very specific needs, and sometimes we just need to build the outbound message based on internal state, it's not really a big deal to build the object, but the the Sender was expecting an interface, would be much clean for us to give our own object that had the logic to provide the body/properties and so on, without having to have to build a Outbound message, maybe that can also be solved having methods on sender that accept Publishers.

Maybe the breaking changing could be solved changing the sender to accept the interface and make OutboundMessage to implement that, that should not break any current client, not sure its easy to find good naming for that approach though.

Yes so, I think that seems like a good things for us, because we are more like writing libraries on top of the client rather than using it in application directly.

Thanks for the work. =)