Croway opened a new pull request, #29332: URL: https://github.com/apache/camel/pull/29332
JIRA: [CAMEL-26647](https://issues.apache.org/jira/browse/CAMEL-26647) ### What was wrong With `synchronousPull=true` and `ackMode=NONE`, the route acks or nacks through the `AcknowledgeSync` it finds in the `CamelGooglePubsubAcknowledge` header. When the exchange given to `ack`/`nack` is an aggregated one, whose body is a `List<Exchange>` (for example the result of `GroupedExchangeAggregationStrategy`), `AcknowledgeSync.getAckIdList` is meant to collect the ack id of every grouped exchange. Inside the loop it read the `CamelGooglePubsubMsgAckId` header of the outer exchange instead of the element's. That bug came in with CAMEL-14514; the CAMEL-10178 code read each grouped exchange's id. - `GroupedExchangeAggregationStrategy` builds a new outer exchange with no ack id header. So the request had no ids at all, none of the grouped messages was acked or nacked, and Pub/Sub delivered them again once their ack deadline expired. The emulator accepts the empty request, although the Pub/Sub API documents `ack_ids` as required and "must not be empty". - If a strategy keeps one of the grouped exchanges as the aggregated one, its id was sent N+1 times and the other ids never. ### What changed - `AcknowledgeSync.getAckIdList` reads the ack id of each grouped exchange, adds the outer exchange's own id when it has one, and sends each id once, in order. `ack` and `nack` both use this method, so the nack is fixed too. - An exchange that is not aggregated still sends its own ack id, as before. With the default `ackMode=AUTO`, each pulled exchange is still acked by its own completion. The asynchronous consumer (`AcknowledgeAsync`) is unchanged. ### Tests - `GooglePubsubConsumerAggregatedAckTest` (mocked `SubscriberStub`, no container): one pull of 3 messages, aggregated by `GroupedExchangeAggregationStrategy`, then ack or nack of the aggregated exchange. Also a direct `AcknowledgeSync` test for an aggregated exchange that is itself one of the grouped ones, and one for a plain exchange. On `main`, 3 of the 4 fail: `expected: <[ack-1, ack-2, ack-3]> but was: <[]>` (ack and nack) and `but was: <[ack-1, ack-1, ack-1, ack-1]>`. With the fix, all 4 pass. - `AggregatedManualAcknowledgementIT` (Pub/Sub emulator): an aggregated ack is not redelivered past the 10 s ack deadline, and an aggregated nack redelivers the 3 messages right away. On `main`, both fail (`Expected: <3> but was: <6>` and `Received message count 3, expected at least 6`). With the fix, both pass. - Module: `mvn verify` in `camel-google-pubsub` passes (44 unit tests, 18 emulator ITs). ### Docs and upgrade guide - The "Manual Acknowledgement" section of `google-pubsub-component.adoc` (and its catalog copy) now explains: - which ack id each consumer uses; - that with the synchronous consumer, an ack of an aggregated exchange acks every grouped message; - that the synchronous consumer does not extend ack deadlines; - that with the asynchronous consumer, each grouped exchange must be acked with its own header. - No upgrade-guide entry: no option, default or header changes, and the behaviour for exchanges that are not aggregated is the same. ### Backport This is a candidate for `camel-4.22.x` and `camel-4.18.x`. The same code is on both branches. The change only affects acks and nacks of an exchange with a `List<Exchange>` body, through the synchronous consumer's `AcknowledgeSync`, and that case never worked before. _Claude Code on behalf of Federico Mariani_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
