[#3195] feat(migration): fix Kotlin OpenRewrite migration gaps#4605
Merged
Conversation
MateuszNaKodach
requested review from
hatzlj,
jangalinski,
laura-devriendt-lemon and
smcvb
and removed request for
a team
MateuszNaKodach
added
the
Priority 3: Could
MateuszNaKodach
added
Type: Enhancement
hatzlj
approved these changes
May 29, 2026
Contributor
hatzlj
left a comment
hatzlj
left a comment
There was a problem hiding this comment.
I gave the migration another run on my mixed Kotlin/Java AF4 project with the following outcome
- Aggregate id type was migrated correctly
- AggregateLifecycle.apply(..).andThen(...) was flattened correctly to subsequent EventAppender.append(..) calls
- Command routing keys were inferred correctly from TargetAggregateIdentifier
- the MessageInterceptor lambda was correctly migrated (including a helpful TODO)
- all imports (Timestamp, TrackingToken, EventProcessor, ...) worked
What didnt work (but it's nothing blocking because i think it's edge cases)
- in the one case where i had the AggregateLifecycle.apply(...) defined with an alias, it still skipped just the first occurence when migrating to EventAppender
- it didnt put the TODO above
registerSequencingPolicyin the configuration as you suggested in the PR description
Overall nice additions to the recipe
Contributor
one further thing I recognized @MateuszNaKodach: |
Address nine issues discovered while running the AF4 → AF5 OpenRewrite
recipes against a real Kotlin codebase.
Entity / lifecycle:
- ConfigureEventSourcedAnnotation now emits `UUID::class` (not
`UUID::class.java`) for Kotlin annotation arguments, matching Kotlin's
KClass-to-Class conversion rule for annotation parameters.
- ReplaceAggregateLifecycleApply scans the compilation unit for aliased
Kotlin static imports (`import …apply as foo`) and treats matching
local identifiers as `apply` calls, fixing call sites the
MethodMatcher could not resolve through the alias.
- ReplaceAggregateLifecycleApply flattens `apply(X).andThen { body }`
chains into sequential `eventAppender.append(X)` + body statements,
since `EventAppender#append` returns `void` in AF5.
Commands:
- AddCommandAnnotation now lifts `@TargetAggregateIdentifier` /
`@TargetEntityId` field names onto `@Command(routingKey = "…")` in
addition to `@RoutingKey`, preserving the field annotation.
Sequencing / interceptors:
- New AnnotateProgrammaticSequencingPolicyRegistration inserts a
TODO comment above `EventProcessingConfigurer#registerSequencingPolicy`
call sites, pointing to the AF5 `@SequencingPolicy` annotation.
- New MigrateMessageInterceptorLambda rewrites
`MessageHandlerInterceptor<…> = (uow, chain) -> …` lambdas to the
three-parameter AF5 shape and updates `chain.proceed()` to
`chain.proceed(message, processingContext)`.
Import targets:
- EventProcessor / StreamingEventProcessor / SubscribingEventProcessor
redirected to `processing.*` / `processing.streaming.*` /
`processing.subscribing.*` subpackages.
- \`@Timestamp\` redirected to \`messaging.eventhandling.annotation\`.
- \`TrackingToken\` redirected to
\`messaging.eventhandling.processing.streaming.token\`.
Add focused tests for the recipes touched in the previous commit so each new behavior has explicit coverage and a regression boundary. - AddCommandAnnotationKotlinTest / AddCommandAnnotationJavaRecordTest: routing-key lift from @TargetAggregateIdentifier (pre-rename) and @TargetEntityId (post-modelling-rename), plus the "no routing key marker" baseline. - AnnotateProgrammaticSequencingPolicyRegistrationTest: TODO comment insertion above EventProcessingConfigurer#registerSequencingPolicy calls, idempotency on re-run, and unrelated-call passthrough. - MigrateMessageInterceptorLambdaTest: two-parameter → three-parameter lambda rewrite, chain.proceed() argument injection, preservation of user-chosen chain parameter names, AF5-shape idempotency, and passthrough for unrelated functional interfaces. - Axon4ToAxon5MessagingTest: StreamingEventProcessor, SubscribingEventProcessor, @timestamp, and TrackingToken relocations into their AF5 subpackages. The TODO comment in MigrateMessageInterceptorLambda is now emitted as a block comment (/* ... */) instead of a line comment so it does not swallow the lambda when the initializer sits on the same line as the variable declaration.
The TODO above migrated `EventProcessingConfigurer#registerSequencingPolicy` calls implied only `@SequencingPolicy` on the handler class was available. AF5 also keeps a declarative path through `EventProcessorDefinition` (`org.axonframework.extension.spring.config`), customised with `.customized(c -> c.sequencingPolicy(...))`. Picking between annotation and declarative configuration needs human judgement (which handler class vs. which processor name), so the recipe still emits a TODO rather than a mechanical rewrite — it now names both options.
ReplaceAggregateLifecycleApply: the Kotlin import-alias scan is now rerun for every compilation unit. The earlier `aliasesScanned` guard fired once for the whole recipe run because OpenRewrite reuses visitor instances across files, so alias names from the first file would leak into every subsequent file and an unrelated function happening to share the alias name would be misidentified as `apply`. Added a regression test that pairs an aliased CU with a second CU that defines an unrelated function of the same name. AnnotateProgrammaticSequencingPolicyRegistration: dropped the dead fall-through branch and the misleading comment referring to a non-existent `visitStatement`. `J.MethodInvocation` implements `Statement`, so `firstEnclosing(Statement.class)` from inside `visitMethodInvocation` always returns the invocation itself — the branch the comment promised never fired. Collapsed to the simple single-branch form.
Three issues surfaced when the umbrella was run on a real mixed Kotlin/Java AF4 codebase: ReplaceAggregateLifecycleApply: the aliased-`apply` rewrite never fired for Kotlin sources because `JavaIsoVisitor#visitCompilationUnit` only triggers for `J.CompilationUnit`, not the `K.CompilationUnit` the Kotlin parser produces. The import-alias scan is now keyed off the enclosing `SourceFile` (looked up via the cursor) and supports both `J.` and `K.` compilation-unit imports, with the qualid matched by walking the field-access chain so it tolerates the Kotlin parser's `J.Empty`-targeted heads. Added an end-to-end test that drives the umbrella recipe and pins the aliased single-call rewrite. AnnotateProgrammaticSequencingPolicyRegistration: the recipe matched the AF4 `EventProcessingConfigurer` FQN only, but `Axon4ToAxon5Common` renames the type into the AF5 configuration namespace before this recipe runs. The MethodMatcher binds to the LST's resolved type at the call site, so the AF5 FQN must also be matched — otherwise the TODO is silently skipped on every codebase that runs the umbrella in order. Added a test that pins the post-rename FQN case. Tests: also added a `MetaData → core.Metadata` pair (Java + Kotlin) to lock in the rename of `MetaData.emptyInstance()` call sites that a review run flagged as uncovered — the existing `ChangeType` rule does handle it, the new tests make that explicit.
MateuszNaKodach
force-pushed
the
openrewrite-kotlin-migration-fixes
branch
from
03f530e to
2e13563
Compare
|
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.
Validation
Check one more time on some Kotlin project before merging.
Summary
Address issues discovered while running the AF4 → AF5 OpenRewrite
recipes against a real mixed Kotlin/Java codebase.
Entity / lifecycle:
UUID::class(notUUID::class.java) for Kotlin annotation arguments, matching Kotlin'sKClass-to-Class conversion rule for annotation parameters.
(
import …apply as foo) and treats matching local identifiers asapplycalls, fixing call sites the MethodMatcher could not resolvethrough the alias. The alias scan is keyed off the enclosing
SourceFileso it works for bothJ.CompilationUnitandK.CompilationUnit, and does not leak state across files.apply(X).andThen { body }chains into sequential
eventAppender.append(X)+ body statements,since
EventAppender#appendreturnsvoidin AF5.Commands:
@TargetAggregateIdentifier/@TargetEntityIdfield names onto@Command(routingKey = "…")inaddition to
@RoutingKey, preserving the field annotation.Sequencing / interceptors:
comment above
EventProcessingConfigurer#registerSequencingPolicycall sites. The TODO points at both AF5 replacements:
@SequencingPolicyon the handler class, or declarative configuration via
EventProcessorDefinition.customized(c -> c.sequencingPolicy(...)).Matches both the AF4 FQN and the post-
Axon4ToAxon5Common-rename AF5FQN so the recipe still fires when run through the umbrella.
MessageHandlerInterceptor<…> = (uow, chain) -> …lambdas to thethree-parameter AF5 shape and updates
chain.proceed()tochain.proceed(message, processingContext).Import targets:
redirected to
processing.*/processing.streaming.*/processing.subscribing.*subpackages.@Timestampredirected tomessaging.eventhandling.annotation.TrackingTokenredirected tomessaging.eventhandling.processing.streaming.token.MetaData → core.Metadatarename acrossJava and Kotlin sources (the existing
ChangeTyperule handles it;the tests lock the behaviour in).
Test plan
./mvnw -pl migration test).AggregateLifecycle.applyrewrite.