allthingssecurity opened a new pull request, #29436: URL: https://github.com/apache/camel/pull/29436
# Description [CAMEL-26742](https://issues.apache.org/jira/browse/CAMEL-26742), part of [CAMEL-26630](https://issues.apache.org/jira/browse/CAMEL-26630) Step 3 of the order of work in `design/exchange-sanitizer.adoc`, first part: the on completion listener SPI in camel-api, the new `UnitOfWork` method and the calls, as in "The listener SPIs" and "Where they are called". Rule R2 itself (each consumer commit runs exactly once, for the unit of work that owns it) uses them in the next PR. It follows the step listener SPI of step 1 (CAMEL-26688, #29374). - **`OnCompletionListener`** (camel-api, `@since 4.24`), with the five methods of the design and empty defaults: `onUnitOfWork`, `onCompletionAdded`, `onCompletionHandover`, `onCompletionTaken`, `onCompletionRun`, and one more, `onCompletionRemoved`: the `Synchronization` contract of step 1 says a commit removed with `UnitOfWork.removeSynchronization` does not run, so R2 must see the removal, or it would report the commit as never run. A listener only observes: each call is in its own `try` with a `catch (Throwable)`, logged at WARN once per listener (then at DEBUG), and the lists it gets are unmodifiable and valid during the call. - **`ExtendedCamelContext`**: `addOnCompletionListener`, `removeOnCompletionListener`, `getOnCompletionListeners`, default methods as for the step listeners; `DefaultCamelContextExtension` keeps them in a copy-on-write list with a cached unmodifiable view, and ignores an equal listener. - **`UnitOfWork.handoverSynchronization(Exchange from, Exchange target, Predicate<Synchronization> filter)`**, a new default method that hands over and returns what moved. `DefaultUnitOfWork` implements it with the same loop as today (a `SynchronizationVetoable` that refuses is not in the list). The default hands over with the existing methods, as Camel does without a listener (the one-argument method when there is no filter), and returns `null`, so a custom `UnitOfWork` hands over as before, and the listener gets `moved == null` ("not known"), as the design's Compatibility section says ("reports no moved list"). For that the `moved` parameter of `onCompletionHandover` is `@Nullable`. - **The calls**, where Camel changes or runs on completions today: - `onUnitOfWork` in `AbstractExchange.setUnitOfWork` (also through `setSharedUnitOfWork`), before the on completions move; - `onCompletionAdded` in `DefaultUnitOfWork.addSynchronization`, after the add and outside its lock, and `onCompletionRemoved` in `removeSynchronization` when it removed one; - `onCompletionHandover` in `AbstractExchange.handoverCompletions(target)`, after the handover, when something moved (or the unit of work does not tell); - `onCompletionTaken` in `AbstractExchange.handoverCompletions()`; - `onCompletionRun` in `UnitOfWorkHelper.doneSynchronizations`, before each on completion runs. The design also names `ExtendedExchangeExtension`; since CAMEL-23686 it is no longer created (its code is inlined in `AbstractExchange`), so it is not changed. - A small internal helper, `OnCompletionListenerHelper` (camel-support), calls the listeners, as `EventHelper` does for the event notifiers. - `design/exchange-sanitizer.adoc`: the interface sketch and "Where they are called" now show `onCompletionRemoved`, the `@Nullable moved`, and that `ExtendedExchangeExtension` is no longer used. The order of the events is the order on one thread (the adds of a move come after `onUnitOfWork`, the adds of a handover before `onCompletionHandover`); events of different threads, such as parallel parts sharing a unit of work, are not ordered with each other. Not seen, as documented: an exchange created as a copy of another (`Exchange.copy()`) starts with that exchange's unit of work without `onUnitOfWork`; and with pooled exchanges (deprecated), whose unit of work exists before the consumer adds its commit, the commit shows as an add, not as part of a move. R2 deals with these in its own PR. With no listener installed, each place reads the cached view and checks `isEmpty()`, and does exactly what it did before (`handoverCompletions` calls the existing `UnitOfWork` methods, no list is copied). These calls are per exchange, not per step. Tests (`OnCompletionListenerTest`, camel-core), each checking the events of one on completion in order, with the exchanges and units of work named as they appear: - a commit added before routing, handed over to the copy of an `InOnly` send to seda: move, add, handover, move into the copy's unit of work, add, run once; - the seda hand-back of CAMEL-25123 (`discardWhenFull`): the copy that is not queued gives the commit back from its own list, and the exchange runs it once; the queued copy still holds its commit; - `shareUnitOfWork` (CAMEL-26576): the parent's commit stays in the shared unit of work and runs with the parent; a part hands over only the on completion it added, and the part is given the parent's unit of work; - `handoverCompletions()` then `UnitOfWorkHelper.doneSynchronizations` (aggregator, enrichers, `onCompletion`): taken, run with the failure; the listener cannot change the list; - no listener installed: the commit runs once; a listener added twice is installed once, and once removed it gets no events; - a `SynchronizationVetoable` commit that refuses the handover to the seda copy: no handover event, it runs once with the exchange; no handover event ever has an empty list; - a commit added to the unit of work and removed (and removed again, which is not reported): added, removed, never run; - a listener that throws in every method: the exchange is not failed, the commit runs once, the next listener still gets every event (also of a removal and of a take); - `OnCompletionListenerCustomUnitOfWorkTest`: a custom `UnitOfWork` (from a `UnitOfWorkFactory`, delegating to the default one but using the interface's default handover) on the seda route: the commit is handed over as before and runs once with the copy, and the listener gets `moved == null`; and (in `OnCompletionListenerTest`) the interface default itself calls the one- or two-argument method. Negative controls (each one call removed, 11 tests): without the `onCompletionHandover` call of the unit-of-work path 5 fail; without the `onCompletionRun` call 7 fail; without the `onCompletionRemoved` call 2 fail; without the "something moved" check of the handover event the vetoable test fails. 220/220 over 20 runs. camel-core and the other core modules and base components: 10426 tests, 0 failures, 61 skipped (offline, on current main). No upgrade guide entry: nothing changes for users without a listener, and the API additions are default methods. JMH: A/B without a listener, on the step-1 benchmark (`StepListenerTest.send` in camel-performance-tests, `listeners=none`): its route has five steps, a consumer commit added before routing and an `InOnly` send to seda with handover, so each message goes through `setUnitOfWork`, `addSynchronization`, `handoverCompletions(target)` and `doneSynchronizations`. Same settings as step 1 (3 forks, 5 warmup and 10 measurement iterations of 1 s, average time), 5 trials each, interleaved; A has the changed classes as on main, B as on this branch, everything else the same build. Mean per message: | Build | Mean (ns/op) | 99.9% CI | Trial scores | |---|---|---|---| | A: main (7dad7adffd26) | 1512 | ± 34 | 1482..1545 | | B: this PR, no listener | 1523 | ± 51 | 1504..1544 | Within noise: the intervals overlap, and a first series on the code before the review fixes (which only touched the listener paths) had B faster than A (1522 ± 44 vs 1538 ± 54). # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it): [CAMEL-26742](https://issues.apache.org/jira/browse/CAMEL-26742). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built the core modules and the base components with the formatter and import-sort plugins and ran their tests; no generated file changes. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
