[#3728] Port the Axon Framework 4 saga repositories into axon-legacy - #5009
Merged
Conversation
MateuszNaKodach
requested review from
hatzlj,
jangalinski and
zambrovski
and removed request for
a team
September 3, 2026 11:10
MateuszNaKodach
marked this pull request as draft
September 3, 2026 11:13
MateuszNaKodach
added
Priority 1: Must
The saga store landed in axon-legacy without the layer that drives it. A SagaStore on its own cannot be used: something has to turn a stored saga into an AnnotatedSaga, write it back at the end of a unit of work, and keep two threads off the same saga. That layer is SagaRepository and its two implementations, and none of it is in a built artifact today. This commit moves the part of it that needs no decisions. SagaRepository, ResourceInjector, NoResourceInjector and SagaCreationException are the four classes in that closure which never touched the Axon Framework 4 unit of work, so they compile unchanged and can be carried across as a pure rename. Keeping them separate means the next two commits, which do have to replace CurrentUnitOfWork with a ProcessingContext, show only that rewiring rather than burying it in a large move. ResourceInjector and NoResourceInjector come along because AnnotatedSagaRepository.Builder takes an injector and defaults to the no-op one. AbstractResourceInjector and SimpleResourceInjector deliberately stay behind: nothing in the repository closure reaches them, and this module only takes what it needs. No content changes, so git log --follow still reaches the Axon Framework 4 history of all four files.
Axon Framework 4 found the ambient unit of work through the CurrentUnitOfWork thread local, which is why none of these three methods needed a parameter for it. Axon Framework 5 deliberately has no such global, and this layer genuinely needs the lifecycle: it registers the store write on prepare-commit and releases the saga lock on completion. So the context has to be passed in. This is the only place in the saga stack where that is true, and it is where the SagaStore pull request concluded the parameter belongs, having removed it from the storage SPI for exactly the reason that the storage layer does not need a lifecycle at all. The parameter is added to find as well, even though the implementation ignores it. A partially context-aware SPI is worse than either alternative: an implementor cannot tell from the signature whether the omission is a statement about find or an oversight, and a custom repository may well want the context there to consult uncommitted state. Uniformity costs one unused parameter and removes that question. load's return is now explicitly @nullable. The package is @NullMarked, and the javadoc has always promised null for a saga that no longer exists, so without the annotation the contract and the code would disagree. Doing this before the implementations move keeps the change reviewable: no call sites exist in the reactor yet, so this commit is the signature decision on its own.
This is the class that keeps two threads off the same saga, so it has to come across before anything can safely use the store. A pure rename was not possible: it is one of only two classes in the closure that read CurrentUnitOfWork, and that class is not being moved. The rewiring is as small as the mechanism allows. lockSagaAccess no longer looks up the ambient unit of work; it takes the ProcessingContext that is now on the SPI. unitOfWork.root().onCleanup(...) becomes context.doFinally(...), which is onError plus whenComplete, and the unit of work runs exactly one of the two, so the lock is still released precisely once on both commit and rollback. The abstract doLoad and doCreateInstance take the context too, because AnnotatedSagaRepository needs it to register its store write. Nothing else changes: the Builder, the PessimisticLockFactory default and the order of lock-then-load are untouched, so the guarantee that one processing context at a time operates on a saga is the Axon Framework 4 one. The ported test comes with it, since the class cannot be green without it. Its mocked LockFactory and Lock stay, and so do its assertions: the only edits are a real UnitOfWork from SimpleUnitOfWorkFactory in place of LegacyDefaultUnitOfWork, and the context argument on the verify calls. Mockito here is deliberate rather than inherited laziness: the assertion is that the repository does not touch the lock before the unit of work completes, which is about interactions and cannot be phrased as state. A third case is added for rollback, which Axon Framework 4 did not cover but doFinally now makes worth stating. Two behaviours are covered that Axon Framework 4 left to inspection. The first is serialisation itself, with two real units of work on two threads against a real PessimisticLockFactory: the second one is observed not to get in until the first completes. The Axon Framework 4 test only checked that a lock had been obtained and released, which says nothing about mutual exclusion. The second is a hazard that Axon Framework 4 could not have: its unit of work lived in a thread local, so the thread that released the lock was always the thread that took it. In Axon Framework 5 the release runs wherever the unit of work runs its completion handlers, and unless forceSyncProcessing is on, that is whichever thread completed the last phase. The tests pin both directions -- off-thread release with the default configuration, same-thread release under forcedSameThreadInvocation, which is what a TransactionManager answering true to requiresSameThreadInvocations arranges. Since PessimisticLockFactory hands out a thread-owned lock, the off-thread case means unlock throws IllegalMonitorStateException, the unit of work swallows it as a warning, and the saga stays locked for the life of the JVM. That is documented on the class as a configuration requirement rather than guarded against, because a guard would be new behaviour and the real answer is the transaction manager the module already requires for the stores. Writing that test first was worth it: the first attempt handed the work to another thread with supplyAsync and passed for the wrong reason, because the result completed before the unit of work chained onto it, leaving the release on the invoking thread. The test now completes the handler's result only after execute() has attached those handlers, and the comment records why -- it is also the reason this hazard is intermittent in production rather than reliable.
The last of the three classes, and the one that actually persists a saga. Like LockingSagaRepository it read CurrentUnitOfWork, so a pure rename was not possible; everything else about it is unchanged. Three substitutions, all forced. CurrentUnitOfWork.get() and its .root() become the ProcessingContext parameter: Axon Framework 4 needed the distinction because a nested unit of work had its own phase sequence while resources and cleanup belonged to the outermost one, and Axon Framework 5 collapses it, since a branched context forwards every lifecycle registration and every non-overridden resource to the root. onCleanup becomes doFinally, onPrepareCommit becomes runOnPrepareCommit, and the unsaved-sagas resource moves from a string key on the unit of work to a Context.ResourceKey built per repository instance, keeping the Axon Framework 4 label so it still identifies itself when debugging. The managedSagas map stays a repository instance field, JVM-wide as it was, because that is what makes find return sagas another thread has not committed yet -- a property the Axon Framework 4 suite pins and which moving the map into the context would have silently dropped. Everything else keeps its Axon Framework 4 shape and visibility: the Builder, the protected commit, deleteSaga, updateSaga, storeSaga, doLoadSaga and unsavedSagaResource hooks, and the TreeSet merge in find. Subclasses in user code may depend on all of it. The ported test comes with it. Its spy on InMemorySagaStore and its verifications stay, because they assert which store calls happen and in which order, which is not expressible as state. What changed is only the unit of work: work that Axon Framework 4 did against an ambient unit of work now runs inside an invocation action of a real UnitOfWork, and assertions that were made before committing are made inside that action. Two of the ported tests need explaining. loadedFromNestedUnitOfWorkAfterCreate nested a unit of work during normal invocation. Its Axon Framework 5 equivalent is a branched ProcessingContext, which is the same thing the nested unit of work was: a deeper scope within one processing session. Renamed to say so, assertions untouched, and it passes -- which is the useful part, since it shows a branch shares the managed saga instance and the unsaved-saga set with its parent. loadedFromNestedUnitOfWorkAfterCreateAndStore loads a saga from within prepare-commit, and is @disabled because it cannot pass yet. It fails with "Failed to register handler in phase PREPARE_COMMIT (20000). ProcessingContext is already in phase PREPARE_COMMIT (20000)." This is not a synthetic scenario: SimpleEventBus publishes events that were published with a context during PREPARE_COMMIT, and SubscribingEventProcessor handles them in that same context, so a saga on a subscribing processor meets this on its first event. Axon Framework 4 saved the saga through a nested unit of work whose prepare-commit ran immediately. The repository cannot fix that, and the alternatives are worth recording so they are not tried again. Saving inline at load time is wrong: Axon Framework 4 registered at load and saved after the handler had mutated the saga, so an inline save would persist the saga without the associations that handler adds, and would insert a saga the handler ended. Registering on COMMIT instead is wrong: the transaction manager registers its own commit on that phase back at PRE_INVOCATION, and phase actions run in registration order, so the write would land after the transaction closed. Relaxing the framework's phase guard would work, and the machinery already supports it, but that guard is a deliberate framework-wide invariant and this is not the change to relax it in. The fix belongs to the component that invokes the saga, because that is the only layer that knows when handling finished. The class javadoc says so, and the test stays in its Axon Framework 4 form rather than being inverted to assert the exception, so that it simply turns green when that component lands. Writing the test before the move earned its keep twice. It first passed for the wrong reason, because registering the prepare-commit action outside the invocation put it ahead of the repository's own action and inverted the Axon Framework 4 order. Fixing that produced the failure above.
The Axon Framework 4 saga repository has a handful of behaviours that look like oversights and are not. Nothing in the ported suite stated them, so the next person to read this code has no way to tell "deliberate" from "nobody noticed", and the obvious reaction to several of them is to tidy them up. Under this module's rule that a deviation ships only if Axon Framework 5 forces it, tidying them up is exactly what must not happen, so each one now has a test that says so. What is pinned, and why each is worth stating: A new saga that ends before its context commits is never written, because doCreateInstance guards the insert with saga.isActive(). A loaded saga that ends is deleted. Those two are asymmetric on purpose and it is easy to "fix" one into the other. An ended new saga keeps its identifier in the context's unsaved-saga set for the rest of that context, because doCreateInstance removes it inside the isActive branch while doLoad removes it unconditionally. Harmless, but the asymmetry is exactly the kind of thing a later reader straightens out without realising it is inherited. Loading an identifier that does not exist asks the store again on every call, since computeIfAbsent does not store a null. That is a real repeated query, not a caching bug to be fixed here. Deleting a saga passes the associations it still has together with the ones it lost during the context, so a removed association cannot outlive the saga it pointed at. find reports a saga that is both managed and stored once rather than twice, because the two sources are merged into one set. A failing saga factory surfaces as SagaCreationException with the original exception as its cause, since doCreateInstance wraps its whole body. The locking repository takes the lock for an identifier even when no saga exists, and holds it for the rest of the context. These are characterisation tests: they pass on the first run by design. Their value is the opposite of a red-then-green cycle -- they fail if the move to ProcessingContext quietly changed one of these behaviours, which is precisely the risk this port carries. Ending a saga goes through the AnnotatedSaga instance rather than the static SagaLifecycle the Axon Framework 4 tests used, since that static API is gone. The lifecycle-parameter route a user's saga takes is exercised where it belongs, in the tests that drive a saga through its event handlers.
The SagaStore pull request dropped two Axon Framework 4 tests because both drive AnnotatedSagaRepository rather than a store, and recorded them as deferred rather than lost, to return when the repository moved into axon-legacy. It has, so they return. JpaSagaStoreTest.addingAnInactiveSagaDoesntStoreIt asserts that a saga created and ended within one processing context leaves the database untouched. The decision is the repository's, not the store's -- the store writes what it is told -- which is why it could not live in the shared store suite. It is equivalent to the in-memory case pinned on the repository itself, but against a real database and asserting on rows, which is the form that catches a store that writes anyway. CachingSagaStoreTest.canHandleConcurrentReadsAndWritesThroughAnnotatedSagaRepository runs 32 concurrent create-find-load cycles through a repository over the caching store. Its value is that it exercises the caches under a real access pattern: the repository interleaves finds with inserts across threads, which the store-level concurrency test cannot produce because it never holds a saga across calls. The concurrency shape, the operation count and the timeout are the Axon Framework 4 ones; only the units of work are new, since the Axon Framework 4 version drove CurrentUnitOfWork by hand. Ending the saga goes through the AnnotatedSaga instance, because the static SagaLifecycle that the Axon Framework 4 test called from inside the saga's own method no longer exists.
Sending a command is the thing a saga is for, and the Axon Framework 4 property worth caring about is that a failure does not leave a half-finished saga behind. Axon Framework 4 delivered that by nesting the command's unit of work inside the saga's. That mechanism is gone, so the property had to be tested rather than assumed. The test drives the real path: a saga created through the repository over the JPA store, handling an event, sending a command through a CommandGateway it holds in a field -- which is how an Axon Framework 4 saga received one from a ResourceInjector -- and handing it the ProcessingContext its handler was invoked with. On success both the saga and the command's write are in the database; on failure the saga is not. Two things it deliberately does not claim. It does not treat "a command sent from a saga gets its own unit of work" as a regression. CommandDispatcher and CommandGateway do carry the context through to CommandBus#dispatch, and SimpleCommandBus then creates its own unit of work anyway, so the context reaches interceptors and resources but not the lifecycle. That is correct: it is the same shape a distributed command bus has, where the handler runs in another JVM and could not share the sender's unit of work at all. Axon Framework 4 gave the local SimpleCommandBus a guarantee that disappeared the moment the bus was distributed; Axon Framework 5 is consistent between the two, and a saga reacts to the command's result rather than relying on shared atomicity. It also does not assert that the command's write is rolled back by the saga's failure. In this configuration it is, because EntityManagerTransactionManager returns a no-op Transaction when one is already active and the whole thing runs on one EntityManager. But that is the transaction manager's propagation, not something the saga repository provides, and pinning it here would turn a property of the wiring into an apparent saga guarantee. Getting the failing case right needed care, and the first attempt was wrong in a way worth recording. Failing the unit of work from runOnAfterCommit left the saga in the database, because that phase runs after the transaction has already been committed. The failure has to be registered from within the invocation, so it lands behind the repository's own prepare-commit action: after the saga was written, while the transaction is still open. That the saga then disappears is the actual evidence that the repository writes inside the caller's transaction.
…uired This is not a new restriction, it is the Axon Framework 4 contract being kept. Axon Framework 4 invoked a saga's handler through EventMessageHandler#handleSync and ignored its return value, so a handler returning a CompletableFuture had it dropped on the floor: whatever it did later ran outside every unit of work and every transaction, unobserved. Saga now extends EventHandlingComponent, which can express an asynchronous result, and the unit of work awaits it. That is worse than what Axon Framework 4 did, because it makes asynchronous saga handling look supported while quietly voiding the guarantee the store half of this work relies on: JdbcSagaStore and JpaSagaStore join the surrounding transaction through a thread-bound provider, which only holds if the store call happens on the thread that opened the transaction. A handler completing elsewhere breaks that premise, and the JdbcSagaStore integration tests already show a write escaping the transaction and surviving a rollback with nothing signalling a problem. So AnnotatedSaga now checks whether the handler's result is done before handing it back, and fails handling with a SagaExecutionException naming the handler and the saga if it is not. An already completed future is accepted, since it is indistinguishable from a synchronous return. Two limitations are documented on the class rather than papered over. It detects rather than prevents: by the time the check runs, the handler has already started whatever it started, and only the transactional part can still be rolled back -- which is still strictly better than committing as if nothing happened. A handler that hands work to an executor and returns void is not detectable at all, in Axon Framework 5 as much as in Axon Framework 4. SagaExecutionException gains a message-only constructor. The alternative was passing a null cause, which reads as an oversight. Written test-first, and the first run was informative: the pending-result case failed on a timeout rather than on a rejection, which is precisely the "awaited, not rejected" behaviour this commit removes.
A migrating user meets this layer through its signatures, and two of the consequences of the port are things they can hit at runtime without any compiler error to warn them. The class and method tables carry the moves and the added ProcessingContext parameters; the processing-context page carries the two behavioural notes, next to the SagaStore section that explains why the storage SPI went the other way. The first note is that nested units of work are gone. That is mostly invisible, because a branched ProcessingContext produces the same outcome the nesting existed for, but it does mean loading a saga from within PREPARE_COMMIT now fails, and it says plainly which configuration reaches that -- a saga on a subscribing event processor fed by events published with a context -- rather than leaving someone to discover it. The second is that a saga event handler must complete on the invoking thread, framed as the Axon Framework 4 rule now being enforced rather than as a new restriction, since Axon Framework 4 dropped an asynchronous result unawaited and never ran it in a transaction either. The lock's matching thread requirement is recorded with it.
The unit-level disabled test says the repository cannot be called from inside PREPARE_COMMIT. It does not say which configuration puts it there, and that is the part someone wiring this up needs to know. This integration test states it in terms of the two event processors. Behind a PooledStreamingEventProcessor the saga is stored. The processor creates a unit of work per batch and invokes its components during INVOCATION, which leaves PREPARE_COMMIT free for the repository to write in. Behind a SubscribingEventProcessor it depends on how the event was published. Published without a context, the processor opens a unit of work of its own and everything works. Published with one, as a command handler would, SimpleEventBus queues the event and delivers it during PREPARE_COMMIT of that same context, and the repository is then asked to register for the phase that is already running. Handling fails and the publishing unit of work fails with it. The stand-in for the future saga event handling component does the one thing that component must do -- reach the repository with the context it was invoked in -- so the test is about the repository's phase requirement rather than about any particular component design. Worth noting from writing it: the phase rejection reaches the caller as a SagaCreationException, because doCreateInstance wraps anything its body throws. That is inherited Axon Framework 4 behaviour, pinned separately, and it means the underlying IllegalStateException is a cause rather than the thrown type.
A saga lock released on the wrong thread cannot be recovered: the default PessimisticLockFactory hands out a ReentrantLock owned by the thread that took it, so unlock elsewhere throws and the lock stays held for the life of the JVM. Documenting the configuration requirement, as the previous commit did, tells someone how to avoid it. It does not help the person who has already hit it, and that person gets the worst possible diagnostic: the processing lifecycle catches whatever a completion handler throws and logs "A Completion handler threw an exception", which names no saga, no identifier and no thread. The symptom they actually see is one saga that silently stops being loadable. So the release now goes through a method that catches the failure and logs an error naming the saga identifier, the thread that acquired the lock, the thread that tried to release it, and the configuration that fixes it. Nothing here can release the lock, and nothing pretends to; this turns a silent hang into something a log search can find. Deliberately not done two other ways. There is no fail-fast check, because a repository cannot ask a ProcessingContext whether its completion handlers will run on the invoking thread -- the context exposes isStarted, isError, isCommitted and isCompleted, and forceSyncProcessing is private to the unit of work. That query is the piece that would turn this documentation into a real guard, and it belongs in the framework, where a thread-bound transaction or JDBC connection has the same problem. And the reporting is driven by the release actually failing rather than by comparing threads, so a LockFactory whose locks are not thread-owned releases on any thread without being accused of anything. Two options were rejected outright. Swapping the default for a lock that is not thread-owned would deadlock: lockSagaAccess runs on every load, so loading one saga twice in a context acquires the same lock twice and relies on reentrancy, which loadedFromUnitOfWorkAfterPreviousLoad exercises. Releasing after invocation rather than at context completion guarantees the right thread and breaks what the lock is for, since the saga is written at prepare-commit and an early release would let another thread write the same saga concurrently. The test asserts on the log, which is unusual and is the point: the log is the entire behaviour being added. Its sibling asserts that an ordinary same-thread unit of work logs nothing, so the check cannot start crying wolf unnoticed.
The test showed a saga sending a command through a CommandGateway held in a field, which is what an Axon Framework 4 saga received from a ResourceInjector and therefore what a migrated saga looks like. But CommandDispatcher is the documented preferred way to send a command from inside a message handler, and a project migrating its sagas is likely to reach for it, so leaving it untested left the more interesting question unanswered. It works, and there is a condition worth knowing about. The resolver for a CommandDispatcher parameter is contributed by a ConfigurationEnhancer rather than registered through META-INF/services, so the ClasspathParameterResolverFactory that AnnotationSagaMetaModelFactory uses by default does not include it. A saga declaring that parameter therefore does not resolve unless the repository is built with a factory that has it, which an application configured through MessagingConfigurer has to hand over via AnnotatedSagaRepository.Builder#parameterResolverFactory. CommandDispatcher.forContext also resolves the gateway from the context's application context, so an empty one is not enough. Both routes are now covered, since a migrating project may be on either, and the condition is recorded in api-changes next to the rest of the repository's wiring notes rather than left for someone to rediscover.
The saga repository has to write the saga inside the caller's transaction, and the only lever it has for that is a lifecycle registration on the caller's ProcessingContext. Whether that lever exists depends on a framework rule that was, until now, asserted nowhere near the place it matters and documented nowhere at all: UnitOfWork#on rejects any phase whose order is at or below the order of the phase currently running. That rule collides with how events published with a context are delivered. SimpleEventBus queues them and drains the queue during PREPARE_COMMIT, and a SubscribingEventProcessor hands them to its components in that same context. A subscriber therefore runs inside PREPARE_COMMIT, which is exactly the phase the saga repository wants. Before changing anything in the repository, this needs to be established as framework behaviour rather than as a saga anecdote, because the repository's design follows from it and has to keep following from it. Two things are pinned here, and neither changes existing behaviour: Phase is an interface over an arbitrary int order, and DefaultPhases leaves 10000 between each of its entries. So a handler that cannot register for its own phase can still register for a custom phase in the gap above it, which runs once every action of the current phase is done and before the next default phase begins. ProcessingLifecycleTest covered custom phases only when registered before the lifecycle started, and covered mid-phase registration only into later default phases; the combination the repository is about to rely on was uncovered. It is asserted on order, not on mere execution, since the whole point is where the work lands relative to PREPARE_COMMIT and COMMIT. The rejection itself was covered for a strictly earlier phase (registeringHandlersInPastPhasesCausesHandlerToFail) but not for the equal case, which is the one the saga hits. Axon Framework 4 differed on precisely that case: DefaultUnitOfWork#addHandler asserted !phase.isBefore(phase()) with a strict comparison, so registering for the running phase was allowed and executed via a consuming drain over the live handler deque. Recording the equal case separately keeps that difference visible instead of leaving it implied by a message assertion about a different phase. The SimpleEventBus tests deliberately use a real UnitOfWork rather than StubProcessingContext. The stub assigns its currentPhase after running a phase's actions, so during a PREPARE_COMMIT action it still reports the earlier phase and accepts registrations production rejects; it also throws a different exception type. Any test written against the stub would have passed regardless of the rule it claims to exercise.
MateuszNaKodach
force-pushed
the
feat/saga-repository
branch
from
September 3, 2026 12:44
20dd233 to
411a58b
Compare
Solves: a saga behind a SubscribingEventProcessor could not be stored at all. Its first event failed the whole processing context with "Failed to register handler in phase PREPARE_COMMIT (20000). ProcessingContext is already in phase PREPARE_COMMIT (20000)." Since the same context carried the command handler that published the event, the command failed with it. That was pinned as a known gap by a disabled unit test and by an integration test asserting the failure, and called out as unresolved in api-changes. Why the subscribing processor specifically, and not the pooled one. The repository needs its store write to land in the caller's transaction, and its only lever for that is a lifecycle registration on the caller's ProcessingContext, which it made for PREPARE_COMMIT. A UnitOfWork rejects any registration for a phase at or below the phase it is currently running. A PooledStreamingEventProcessor creates a unit of work per batch and invokes its components during INVOCATION, so PREPARE_COMMIT was still ahead and the registration was accepted. A SubscribingEventProcessor does not create one: when handed a non-null context it calls processInGivenContext and handles the events in the publisher's context. SimpleEventBus, for events published with a context, queues them and drains that queue from a PREPARE_COMMIT action, passing the same context through to its subscribers. So the saga's handler ran inside PREPARE_COMMIT and asked for PREPARE_COMMIT. Nothing about the saga was wrong; the phase it was reached from was the phase it needed. Why Axon Framework 4 did not have this problem, which is not the reason it might appear. AF4 was not more permissive about the phase in general: DefaultUnitOfWork#addHandler asserted !phase.isBefore(phase()) and threw for a strictly earlier phase exactly as AF5 does. It differed only on the equal case, which its saga path did not use. AnnotatedSagaRepository opened a nested DefaultUnitOfWork: started while another unit of work sat on the CurrentUnitOfWork thread-local, it adopted that one as parent but kept its own handler map at phase NOT_STARTED and its own resources. Registration on the child was therefore legal whatever the parent was doing, and commitAsNested ran the child's PREPARE_COMMIT and COMMIT immediately, deferring only AFTER_COMMIT up the ancestor chain and wiring the child's rollback to the parent's. AF4 sidestepped the restriction by giving each nested scope a fresh phase timeline, not by registering into a running phase. AF5 has no nesting: a branched ProcessingContext forwards every lifecycle registration and every resource to the root. So the mechanism cannot be ported, and it is worth being precise that the mechanism is what is missing, not permission. What can be ported is the position that mechanism produced: after the handler mutated the saga, and before the caller's transaction commits. ProcessingLifecycle.Phase is an interface over an arbitrary order and DefaultPhases leaves 10000 between its entries, so that position can simply be named. SAGA_WRITE sits at PREPARE_COMMIT + 5000, and both doLoad and doCreateInstance register there. That also removes the dependency on where the repository was called from, which is what makes the two processors equivalent again: INVOCATION and PREPARE_COMMIT are both below SAGA_WRITE. The write stays transactional because TransactionManager#attachToProcessingLifecycle starts the transaction at PRE_INVOCATION, commits it with runOnCommit and rolls back onError; and it stays serialised because LockingSagaRepository releases its lock through doFinally, which runs after every phase. The trade-off: one write where Axon Framework 4 made two. AF4's nested unit of work had two properties that mattered here, and both are gone. It had its own resources, so the unsaved-saga bookkeeping the repository uses to write each saga once per session started empty in the child; looking at the same saga again therefore looked like a first sighting and scheduled a second write. And its prepare-commit ran immediately, so that second write happened there and then. AF4 consequently did insertSaga with whatever associations the saga had at that moment, then updateSaga with the rest. AF5 shares the bookkeeping with the root, so the second look schedules nothing; and the single write it did schedule is ordered after every PREPARE_COMMIT action, so by the time it runs the saga already carries everything. One insert instead of insert-then-update, and the same stored saga either way -- the difference is the number of round trips to the store, not the outcome. This is the only deviation from Axon Framework 4 in the port, and it is not one AF5 forces, so it needed a reason to be preferred rather than merely tolerated. The faithful alternative was implemented and measured: keep registering for PREPARE_COMMIT and fall back to SAGA_WRITE on the IllegalStateException. It works, and it turns the AF4 test green with its assertions untouched, because the second load then does come from inside PREPARE_COMMIT and does produce AF4's update. It was dropped for two reasons. It makes an exception part of normal control flow, with no way to avoid it -- a ProcessingContext exposes isStarted, isError, isCommitted and isCompleted, but not its phase, so the repository cannot ask before trying. More importantly it makes the write's phase, and therefore its order against the event store append and the token store update which both sit in PREPARE_COMMIT, depend on which processor happens to front the saga. One phase everywhere is worth more than fidelity in a scenario that stores the same result either way. The AF4 test is kept, renamed to what it now asserts, and its javadoc explains what a nested unit of work was, which of its two properties produced the second write, and why neither survives -- so the next reader meets the reasoning rather than a quietly adjusted assertion. Two tests were added for the position rather than for the outcome, since the outcome would still hold if the write drifted into the wrong phase: one at unit level asserting the write falls between the last PREPARE_COMMIT action and COMMIT, and one in the integration test asserting it falls between a recording transaction manager's start and its commit. A third asserts that a failure elsewhere in the context leaves the saga unwritten, which is the property the phase choice exists to preserve. The integration test's subscribing case now asserts the saga is stored instead of asserting the exception.
MateuszNaKodach
force-pushed
the
feat/saga-repository
branch
from
September 3, 2026 13:07
411a58b to
43afff7
Compare
MateuszNaKodach
commented
Sep 3, 2026
smcvb
approved these changes
Sep 4, 2026
smcvb
left a comment
smcvb
left a comment
Contributor
There was a problem hiding this comment.
Bunch of nits, nothing you can't resolve. Hence, preemptively approving this PR.
- Use correct awaitility import - Timeout thread.join - Rename constant for clarity #3728
smcvb
approved these changes
Sep 4, 2026
smcvb
left a comment
smcvb
left a comment
Contributor
There was a problem hiding this comment.
My concerns have been addressed, hence I'm approving this pull request.
smcvb
enabled auto-merge
September 4, 2026 10:56
MateuszNaKodach
added a commit
that referenced
this pull request
Sep 4, 2026
Axon Framework 4 asked every loaded Saga instance whether it took the event and counted it as invoked only when it said yes. That answer is what a SagaCreationPolicy.IF_NONE_FOUND policy consults: no Saga of this type took the event, so start one. The port asked a single type-level question instead and counted every loaded instance as invoked, so a Saga that declines still suppresses creation. The two answers diverge whenever the store index still lists an association value the live instance no longer holds. A handler that calls SagaLifecycle.removeAssociationWith earlier in the same unit of work produces exactly that, because the store write only lands at AnnotatedSagaRepository.SAGA_WRITE; so does AnnotatedSagaRepository.managedSagas handing back an instance another thread has mutated but not committed, which #5009 deliberately preserved. In both, Axon Framework 4 starts a fresh Saga and the port silently does not. Only IF_NONE_FOUND is affected, which is worth stating because it bounds the change: shouldCreateSaga never creates under NONE whatever the flag says, and short-circuits on ALWAYS before reading it. The rest of the creation-policy chain was already faithful. The filter itself never disappeared. AnnotatedSaga#handle still narrows its handlers to the ones whose association value the instance holds, so a declining Saga never runs its handler and nothing is written. What disappeared is the ability to observe that decision, because handle returns a stream with no entries by construction and a Saga that declined is indistinguishable from one that handled the event without producing anything. The manager therefore has to ask a second question, as Axon Framework 4 did, and the predicate is extracted rather than written: canHandle and handle now share one lookup instead of duplicating it, as Axon Framework 4 duplicated it. Axon Framework 4 reached that question through MessageHandler#canHandle, a public default method that Saga inherited by way of EventMessageHandler. Axon Framework 5 reduced MessageHandler to an empty marker, so the declaration moves onto Saga itself. This preserves the Axon Framework 4 surface rather than widening it: saga.canHandle(event) was callable on any Axon Framework 4 Saga. Narrowing it is not available either, since interface members are implicitly public and a package-private abstract method cannot be declared. Computing the predicate inside AnnotatedSagaManager was the alternative, and it was rejected because the manager's SagaModel and the repository's are separate instances, so a hand-wired setup can have them configured differently and the manager would consult the wrong one; it would also drop the branch under which a handler that resolves no association value at all counts as a match. The manager keeps its own check, which is the other, unrelated Axon Framework 4 canHandle: does this Saga type declare a handler for this event. It moves to an early return because that is where Axon Framework 4 asked it, from the event processor before handle was entered. It stays worth asking even though Axon Framework 5 gates on supports(QualifiedName) first, since that compares a name where this compares payload assignability and resolvable parameters, and is therefore the coarser of the two. Finally, startNewSaga now asks the new Saga before invoking it, as Axon Framework 4 did by routing creation through the same invocation path. For an AnnotatedSaga the answer is always yes, since the instance carries exactly the association value the starting handler resolved; it matters only for a custom Saga implementation, and it costs nothing now that the question exists. #3728
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves
SagaRepository,LockingSagaRepositoryandAnnotatedSagaRepositoryout ofstash/legacyand intoaxon-legacy. This is the layer between the two slices that already landed: #4992 brought theSagaStoreand its implementations, #4997 broughtSaga,AnnotatedSaga, theSagaLifecyclereplacement, the annotations and the metamodel. Neither is usable without this one: nothing turned a stored saga into anAnnotatedSaga, wrote it back at the end of a unit of work, or kept two threads off the same saga.The governing rule for this module is unchanged: a deviation from Axon Framework 4 ships only if Axon Framework 5 forces it. Not if it would be an improvement, not if it matches how AF5 does things elsewhere. Every edit here is traceable to a compile error or to a framework mechanism that no longer exists, and the odd inherited behaviours are pinned with tests rather than tidied up.
What moved
Only what the two repositories need:
SagaRepository,ResourceInjector,NoResourceInjector,SagaCreationException,LockingSagaRepository,AnnotatedSagaRepository, and the two Axon Framework 4 tests that cover them.AbstractResourceInjector,SimpleResourceInjector,AssociationValueMapand the saga managers stay instash, since nothing in this closure reaches them.The one API change:
ProcessingContextonSagaRepositoryAxon Framework 4 read the ambient unit of work from
CurrentUnitOfWork, a thread local that Axon Framework 5 deliberately does not have. UnlikeSagaStore, this layer genuinely needs the lifecycle: it registers the store write on a phase of the caller's context, which is what puts that write in the caller's transaction, and it releases the saga's lock when processing completes. This is where #4992 concluded the parameter belongs, having removed it from the storage SPI for the opposite reason.Substitutions in the two implementations
axon-legacyCurrentUnitOfWork.get()/.root()ProcessingContextparameterunitOfWork.root().onCleanup(...)context.doFinally(...)unitOfWork.onPrepareCommit(...)context.runOn(AnnotatedSagaRepository.SAGA_WRITE, ...), see belowgetOrComputeResource("Repository[X]/UnsavedSagas", ...)context.computeResourceIfAbsent(key, HashSet::new), key built per repository instancemanagedSagasinstance fieldThe
unitOfWork/processRootdistinction collapses: Axon Framework 4 needed it because a nested unit of work had its own phase sequence while resources and cleanup belonged to the outermost one, and in Axon Framework 5 a branched context forwards every lifecycle registration and every non-overridden resource to the root anyway.loadedFromBranchedContextAfterCreateproves that: a branch shares the managed saga instance and the unsaved-saga set, so the saga is still written exactly once.managedSagasstays a repository instance field, JVM-wide as it was. That is what makesfindreturn sagas another thread has not committed yet, which the Axon Framework 4 suite pins and which moving the map into the context would have silently dropped.Choices worth explaining
The saga managers are out of scope.
Saga event handlers must complete on the invoking thread.
AnnotatedSaga.handlenow fails with aSagaExecutionExceptionif a handler returns a result that is not done yet. This is not a new restriction -- it is the Axon Framework 4 contract being kept:Axon Framework 4 invoked the handler through
EventMessageHandler#handleSyncand ignored its return value, so an asynchronous result never ran inside a transaction.EventHandlingComponent#handlecan express one and the unit of work awaits it, which silently voids #4992's safety argument: the stores join the surrounding transaction through a thread-bound provider, which only holds if the store call happens on the thread that opened the transaction. An already completedCompletableFutureis accepted, being indistinguishable from a synchronous return. Two limitations are documented on the class rather than papered over -- it detects rather than prevents, and avoidhandler that hands work to an executor is undetectable, as it was in Axon Framework 4.A saga is written in a phase of its own, which is what makes it work behind a
SubscribingEventProcessor.What was broken. A saga behind a
SubscribingEventProcessorcould not be stored at all. Its first event failed the whole processing context withFailed to register handler in phase PREPARE_COMMIT (20000). ProcessingContext is already in phase PREPARE_COMMIT (20000).and, because that same context carried the command handler that published the event, the command failed with it.Why the subscribing processor was problematic The repository needs its store write to land in the caller's transaction, and its only lever is a lifecycle registration on the caller's
ProcessingContext. AUnitOfWorkrejects any registration for a phase at or below the phase it is currently running. APooledStreamingEventProcessorcreates a unit of work per batch and invokes its components duringINVOCATION, soPREPARE_COMMITwas still ahead and the registration was accepted. ASubscribingEventProcessorcreates none: handed a non-null context it callsprocessInGivenContextand handles the events in the publisher's context.SimpleEventBus, for events published with a context, queues them and drains that queue from aPREPARE_COMMITaction, passing the same context to its subscribers. So the saga's handler ran insidePREPARE_COMMITand asked forPREPARE_COMMIT. Nothing about the saga was wrong; the phase it was reached from was the phase it needed.Why Axon Framework 4 did not have this problem, which is not the reason it might appear. Axon Framework 4 was not more permissive about phases in general:
DefaultUnitOfWork#addHandlerasserted!phase.isBefore(phase())and threw for a strictly earlier phase exactly as Axon Framework 5 does. It differed only on the equal case, and its saga path did not use that either.AnnotatedSagaRepositoryopened a nestedDefaultUnitOfWork: started while another unit of work sat on theCurrentUnitOfWorkthread local, it adopted that one as parent but kept its own handler map at phaseNOT_STARTEDand its own resources. Registration on the child was therefore legal whatever the parent was doing, andcommitAsNestedran the child'sPREPARE_COMMITandCOMMITimmediately, deferring onlyAFTER_COMMITup the ancestor chain and wiring the child's rollback to the parent's. Axon Framework 4 sidestepped the restriction by giving each nested scope a fresh phase timeline, not by registering into a running phase. Axon Framework 5 has no nesting: a branchedProcessingContextforwards every registration and every resource to the root. The mechanism is what is missing, not the permission.What was ported instead. The position that mechanism produced: after the handler mutated the saga, before the caller's transaction commits.
ProcessingLifecycle.Phaseis an interface over an arbitrary order andDefaultPhasesleaves 10000 between its entries, so that position can simply be named:Both
doLoadanddoCreateInstanceregister there. That removes the dependency on where the repository was called from, which is what makes the two processors equivalent again:INVOCATIONandPREPARE_COMMITare both belowSAGA_WRITE. The write stays transactional becauseTransactionManager#attachToProcessingLifecyclestarts the transaction atPRE_INVOCATION, commits it withrunOnCommitand rolls backonError; and it stays serialised becauseLockingSagaRepositoryreleases its lock throughdoFinally, which runs after every phase. The constant is public so surrounding code can order its own actions against the write. Note that a phase declaring the same order does not run before or after the write but alongside it, since actions sharing an order may be invoked in parallel.SagaRepositoryEventProcessorITnow asserts the saga is stored behind both processors, that the write falls between a recordingTransactionManager's start and its commit, and that a failure elsewhere in the context leaves the saga unwritten.Out of scope
AbstractSagaManager/AnnotatedSagaManageras anEventHandlingComponent, and with themAnnotatedSaga's twoUnsupportedOperationException("TODO")methods.