allthingssecurity opened a new pull request, #26866:
URL: https://github.com/apache/camel/pull/26866

   # Description
   
   [CAMEL-25006](https://issues.apache.org/jira/browse/CAMEL-25006)
   
   Since CAMEL-24144 (4.22.0), `InMemorySagaCoordinator` removes itself from 
`InMemorySagaService` once it is completed or compensated. `getSaga(id)` then 
returns `null`, and the saga processors treat `null` as "this exchange is not 
in a saga". As a result, after a saga timed out and was compensated:
   - a `REQUIRED` step on an exchange of that saga started a new saga of its 
own and completed it;
   - a `SUPPORTS` step ran outside of any saga.
   
   For example:
   ```java
   from("direct:order").saga().timeout(200, 
MILLISECONDS).compensation("direct:cancelOrder")
       .to("direct:slowCall")            // outlasts the saga timeout
       .to("direct:payment");
   from("direct:payment").saga()        // REQUIRED
       .compensation("direct:refund").completion("direct:confirmPayment")
       .to("direct:takePayment");
   ```
   The order was compensated, but the payment was taken and confirmed, and 
nothing will ever refund it. Before CAMEL-24144, the payment step failed with 
`Cannot begin: status is COMPENSATED`. It still fails that way when it arrives 
while the saga is still being compensated, so the outcome depended on timing.
   
   This change: `RequiredSagaProcessor` and `SupportsSagaProcessor` fail with 
`IllegalStateException("Cannot begin: saga <id> is not active or not known")` 
when the exchange carries a saga id that the saga service no longer knows. The 
id is the one from the exchange's internal state, or from the header for 
services that support it, as today.
   - The saga is still removed from the service, so the memory fix of 
CAMEL-24144 is kept (`SagaCoordinatorCleanupTest` is unchanged).
   - `MANDATORY` already fails.
   - `REQUIRES_NEW`, `NOT_SUPPORTED` and `NEVER` are unchanged, since they do 
not need the outer saga to be active.
   - `LRASagaService.getSaga` never returns `null`, so LRA is unaffected.
   - The 4.23 upgrade guide has a short entry next to the other saga entry.
   
   Tests: two new tests in `SagaTimeoutTest`. The owner route waits 
(Awaitility, on the saga service) until its saga has timed out, been 
compensated and been removed, and then calls a `REQUIRED` or `SUPPORTS` step. 
Without the fix:
   ```
   testRequiredStepAfterTimeoutDoesNotStartNewSaga     AssertionError: 
mock://complete Received message count. Expected: <0> but was: <1>
   testSupportsStepAfterTimeoutDoesNotRunOutsideSaga   AssertionError: 
mock://payment Received message count. Expected: <0> but was: <1>
   ```
   With the fix both pass. All `*Saga*` tests in camel-core pass (37 tests). 
The camel-lra tests could not be run offline here.
   
   Found with a TLA+ model of the in-memory saga (coordinator, timeout, 
participants with each propagation), then reproduced against the real classes. 
With this change, the property "a participant carrying the saga's id never runs 
in another saga, or outside a saga, while that saga compensates" holds, with 
synchronous and asynchronous participants and timeouts. The reproduction now 
fails the payment step: `compOwner=1 paymentAction=0 complPayment=0`.
   
   This is separate from the check-then-enlist race in 
`InMemorySagaCoordinator.beginStep` (a step that joins while the saga is 
finalized is never compensated), which I am sending as its own fix.
   
   The check is in `SagaProcessor`, so it applies to any `CamelSagaService` 
whose `getSaga` returns no coordinator for the exchange's saga id. 
`LRASagaService.getSaga` never returns null, so camel-lra is not affected. The 
upgrade-guide entry also mentions custom services that read the 
`Long-Running-Action` header.
   
   Related, not changed here: when `compensate()`/`complete()` of a new saga 
fails, `handleSagaCompletion` does not restore the previous saga of the 
exchange, so the exchange keeps the id of the ended saga. A later 
REQUIRED/SUPPORTS step on that exchange (for example after a `doCatch`) now 
fails, as before 4.22. I can send a small follow-up that restores the previous 
saga on that path too.
   
   As the regression is in 4.22, this may be worth a backport to `camel-4.22.x`.
   
   # 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).
   
   # 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 and tested the affected modules, including the formatter and 
import-sort plugins. 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]

Reply via email to