fix #920: no type change when array is wrapped in allOf - #921
Merged
Conversation
SchemaDiff#computeDiffForReal selects the diff result from right.getClass(), so comparing a direct `type: array` against an allOf-wrapped array landed in ComposedSchemaDiffResult with a non-composed left schema and reported a spurious `array -> array` type change. When the right-hand composed schema has already been flattened by resolveComposedSchema (no oneOf left), delegate to the diff result matching the left-hand schema instead. Array items keep being compared because ArraySchemaDiffResult reads them through Schema#getItems(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Fixes #920. Follow-up to #916.
Comparing a direct
type: arrayagainst anallOf-wrapped array reported a spurious breaking change between two equivalent specs:SchemaDiff#computeDiffForRealpicks the diff result fromright.getClass().resolveComposedSchemamergesallOfmembers into the parent but leaves the instance aComposedSchema, so with theallOfspec on the right,ComposedSchemaDiffResultwas selected and itselsebranch reported a type change becauseleftwas anArraySchemarather than aComposedSchema— even though the type/format equality check upstream had already passed.Change
When
leftis not composed andrightis a composed schema thatresolveComposedSchemahas already flattened (nooneOfremaining), delegate to the diff result matchingleft.getClass()instead of declaring a type change.Delegating rather than calling
super.diff(...)matters:SchemaDiffResultdoes not compare arrayitems, so a plain fall-through would trade a false positive for a false negative. Routing throughleft.getClass()sends the array case toArraySchemaDiffResult, which reads both sides viaSchema#getItems()after #916.A composed schema that still carries
oneOfkeeps reporting a type change — that is a genuine structural difference, not anallOfwrapper.Tests
Issue887Test— both directions now assertassertOpenApiAreEqualsinstead of only "does not throw".testDirectArrayToAllOfArrayAreEqualsfails without the main-code change; theClassCastExceptionregression from ClassCastException when comparing schema changed from allOf to direct array type #887 stays covered, since a throw fails the equality assertion too.Issue920Test(new, with fixtures) — a changed item type across the sameallOf/direct boundary must still be reported as incompatible. This guards the new delegation path against silently skippingitems; note it does not reproduce the original bug, because the old spurious type change also rendered as incompatible../mvnw verifypasses on all modules (core 270 tests, maven 13 tests, 0 failures).🤖 Generated with Claude Code
Summary by cubic
Fixes a false breaking change when comparing a direct array schema to an allOf-wrapped array. Flattened composed schemas are now treated as plain, and the diff routes via the left-hand schema to keep item comparisons intact.
Issue887now asserts equality both ways; newIssue920ensures item-type changes are flagged.Written for commit 6efa85e. Summary will update on new commits.