Add ProtonDeliveryListener into spring-amqp-client#3287
Add ProtonDeliveryListener into spring-amqp-client#3287cppwfs merged 2 commits intospring-projects:mainfrom
ProtonDeliveryListener into spring-amqp-client#3287Conversation
Sometimes target projects would like to deal with native ProtonJ API but still rely on the Spring flexibility with listener container infrastructure. * Add `ProtonDeliveryListener` to handle `Delivery` directly * Test and document the feature * Fix copy/paste artifact in the `whats-new.adoc`
cppwfs
left a comment
cppwfs
left a comment
There was a problem hiding this comment.
Nice add on feature!
Just my usual annoying comments.
|
|
||
| === ProtonJ `Delivery` Consumption | ||
|
|
||
| For convenience, a `ProtonDeliveryListener` contract is provided to handle a native ProtonJ `Delivery` objects instead of Spring AMQP messages. |
There was a problem hiding this comment.
"handle a native ProtonJ Deliver objects " Remove the a since the syntax of the sentence deals with objects (plural)
|
|
||
| For convenience, a `ProtonDeliveryListener` contract is provided to handle a native ProtonJ `Delivery` objects instead of Spring AMQP messages. | ||
| It could be useful in scenarios where full control over the `Delivery` instance is required. | ||
| For example, adding some reject conditions, handling received data as an `InputStream`, replenished link credits dynamically, according to some target application logic. |
There was a problem hiding this comment.
replenished should be replenish
| NOTE: The manual delivery settlement still can be skipped, if the `AmqpMessageListenerContainer` is configured for `autoAccept = true` (default). | ||
| And the `AmqpMessageListenerContainer` performs credits replenishment automatically. | ||
|
|
||
| The `ProtonDeliveryListener` implementation should be injected into the `AmqpMessageListenerContainer` as regular a `MessageListener`: |
There was a problem hiding this comment.
Replace as regular a with as a regular.
| * Therefore, the listener must manually acknowledge the message or reject/requeue, according to its logic. | ||
| * <p> | ||
| * If a {@link ProtonDeliveryListener} is provided and {@code autoAccept == false}, | ||
| * it is this listener's responsibility to acknowledge the message manually and replenish the link credits. |
There was a problem hiding this comment.
Maybe the the listener's instead this listener's. WDYT
| } | ||
|
|
||
| if (this.autoAccept) { | ||
| replenishCreditOperation.run(); |
There was a problem hiding this comment.
Is it ok to replenish the credits after the onMessage for the non ProtonDeliveryListener path. Before we replenished the credits before the onMessage.
Just curious.
There was a problem hiding this comment.
Oh! I see your point.
Yes, I really meant it originally to replenish after the listener call.
So, let's treat this as a bug fix! 😄
Thank you!
cppwfs
left a comment
cppwfs
left a comment
There was a problem hiding this comment.
LGTM!
Thanks for the updates!
2 participants
Sometimes target projects would like to deal with native ProtonJ API but still rely on the Spring flexibility with listener container infrastructure.
ProtonDeliveryListenerto handleDeliverydirectlywhats-new.adoc