fix: RevisionSnapshotFilter#type(Class) never matched stored snapshots, silently disabling revision filtering#4644
Open
MateuszNaKodach wants to merge 1 commit into
Conversation
…type(Class) A RevisionSnapshotFilter compares the revision only when its configured type matches the aggregate type stored on the snapshot. Snapshots are stored under the aggregate's declared type, which is @AggregateRoot#type when set and the simple class name otherwise. A fully-qualified class name never matches that declared type, so test() returns true on the type check and leaves the revision unverified - allowing outdated or corrupt snapshots through even with a revision filter in place. RevisionSnapshotFilter.builder().type(Class) now resolves the declared type the same way the framework stores it, rather than the fully-qualified name. The resolution lives in a reusable AnnotatedAggregateMetaModelFactory#declaredTypeOf(Class), to which the metamodel's findDeclaredType delegates, keeping a single source of truth for declared-type resolution. Tests store snapshots under the realistic declared (simple) type. Added coverage for type(Class) resolution (simple name and @AggregateRoot override), declaredTypeOf, a fully-qualified type misconfiguration, and serializer-independence of revision filtering (XStream and Jackson). BREAKING CHANGE: RevisionSnapshotFilter.builder().type(Class) resolves the aggregate's declared type (@AggregateRoot#type or the simple class name) instead of the fully-qualified class name. A type(Class) configuration that matched nothing and allowed all snapshots will now filter by revision.
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
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.
Background
This was reported by a customer: they configured the filter with their aggregate class via
type(Class), but the resolved fully-qualified class name never matched the type their snapshots were actually stored under — and there was no warning or any other signal of the mismatch. The filter appeared correctly configured while revision filtering was silently not happening.The bug
RevisionSnapshotFilter.Builder#type(Class)resolved the type viaClass#getName()— the fully-qualified class name. Snapshots, however, are stored under the aggregate's declared type:@AggregateRoot#type()when set, and the simple class name otherwise.Since a fully-qualified name never equals the declared type, the filter's type check never matched any stored snapshot. Per the
SnapshotFiltercontract, a type mismatch makes the filter abstain (returntrue) so that filters for different aggregates can be AND-combined. The result: everytype(Class)-configured filter was a silent no-op — it looked installed, but never evaluated the revision, allowing outdated or corrupt snapshots through.The fix
type(Class)now resolves the declared type exactly the way the framework stores it. The resolution lives in a new publicAnnotatedAggregateMetaModelFactory#declaredTypeOf(Class), to which the metamodel's internalfindDeclaredTypenow delegates — a single source of truth for declared-type resolution. This also makes the manual builder path consistent withAggregateConfigurer, which already resolves the declared type via the metamodel.Why this is safe for a patch release, despite changing behavior
The commit carries a
BREAKING CHANGEmarker for visibility, but the previous behavior could not work: there was no configuration in whichtype(Class)produced a filter that actually filtered. Anyone using it had a no-op filter without knowing it. After this fix:type(Class)get the revision filtering they always intended to have — outdated snapshots are now correctly rejected (and the aggregate is rebuilt from events, the standard recovery path).type(Class). Such a setup would have had a working filter by accident; it can keep the old behavior explicitly withtype(MyAggregate.class.getName())via the unchangedtype(String)overload.Tests
type(Class)resolution: simple name and@AggregateRoot(type = ...)overridedeclaredTypeOfdefaults and overrideRevisionResolver, not in the serialized bodyNotes for reviewers
type(ParentClass)only matches parent-typed snapshots — same as the existingAggregateConfigurerbehavior, not a regression.