[Port] Align QualifiedName#(Class) outcome with annotated approach for nested classes#4685
Merged
Merged
Conversation
…aults for nested classes The framework derived two different qualified names for the same nested class, depending on the path: - QualifiedName(Class) - and thus MessageType(Class) - used Class.getName(), producing "pkg.Outer$Inner" for nested classes. - Annotation-based resolution (@Command/@event defaults via AnnotationMessageTypeResolver) uses getPackageName() + getSimpleName(), producing "pkg.Inner". - ClassBasedMessageTypeResolver documented getPackageName() + getSimpleName(), but implemented Class.getName(), contradicting its own javadoc for nested classes. As a result, a MessageType built from a nested payload class could never match messages dispatched through annotation-based naming - a silent mismatch, e.g. a QualifiedName-keyed interceptor or handler lookup that simply never matches. Nested payload classes are idiomatic in Kotlin (sealed command/event hierarchies), making this especially easy to hit there. Changes: - QualifiedName(Class) now combines getPackageName() and getSimpleName(), aligning with the annotation defaults. Primitive wrapper resolution is preserved. Classes without a simple name (e.g. anonymous classes) fall back to Class.getName(). Javadoc updated accordingly. - ClassBasedMessageTypeResolver now routes through the Class-based MessageType constructor, making the implementation match its documented behavior. - MessageTypeTest gains nested-class tests (written first, TDD) pinning that MessageType(Class) equals the package + simple name spelling for nested and top-level classes. - AsyncMessageHandlerTest's declarative registrations used the raw Class.getName() String and are updated to the Class-based QualifiedName constructor. Verified: messaging (3530 tests), eventsourcing (489), modelling (370), and conversion (174) suites pass. Notes for review: - Behavioral change: nested, un-annotated payload classes resolved through ClassBasedMessageTypeResolver change their name from "pkg.Outer$Inner" to "pkg.Inner". Events already stored under the old spelling will no longer match handlers/criteria without a mapping - needs a release note or migration guidance. - Nested classes with the same simple name in the same package now collide, mirroring the trade-off the annotation defaults already made; explicit @Command/@event name/namespace attributes remain the escape hatch. - String-based constructors (QualifiedName(String), MessageType(String, ...)) remain verbatim by design. Kotlin users should pass ::class.java rather than KClass.qualifiedName, which renders nested classes with a dotted spelling that does not match either.
…nnotationMessageTypeResolver The annotation-based resolver spelled its class-derived defaults (getPackageName() / getSimpleName()) itself, duplicating the naming policy that QualifiedName(Class) owns. Derive the defaults from QualifiedName(Class) instead, so the policy lives in exactly one place and partial annotation overrides (namespace without name, and vice versa) overlay the same class-derived name as every other path. Behavior is unchanged for regular classes, pinned by the new default-name tests in AnnotationMessageTypeResolverTest. For the exotic edge of an annotated type without a simple name, the local name now falls back to the binary-name-derived local name instead of failing on an empty local name.
smcvb
requested review from
jangalinski and
zambrovski
and removed request for
a team
smcvb
added
the
Type: Bug
smcvb
added
the
Priority 2: Should
smcvb
added
the
Type: Port
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
3 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.
This pull request is a port of #4630.