From 6efa85e2290fa3d8d274815de720b562da70fe77 Mon Sep 17 00:00:00 2001 From: Aliaksandr Pinchuk Date: Wed, 5 Aug 2026 11:35:59 +0200 Subject: [PATCH] fix #920: no type change when array is wrapped in allOf 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) --- .../ComposedSchemaDiffResult.java | 14 ++++++++ .../openapidiff/core/Issue887Test.java | 16 +++------- .../openapidiff/core/Issue920Test.java | 25 +++++++++++++++ core/src/test/resources/issue-920-1.yaml | 31 ++++++++++++++++++ core/src/test/resources/issue-920-2.yaml | 32 +++++++++++++++++++ 5 files changed, 106 insertions(+), 12 deletions(-) create mode 100644 core/src/test/java/org/openapitools/openapidiff/core/Issue920Test.java create mode 100644 core/src/test/resources/issue-920-1.yaml create mode 100644 core/src/test/resources/issue-920-2.yaml diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ComposedSchemaDiffResult.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ComposedSchemaDiffResult.java index 336c8b29..0c9ff762 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ComposedSchemaDiffResult.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ComposedSchemaDiffResult.java @@ -12,6 +12,7 @@ import org.apache.commons.collections4.CollectionUtils; import org.openapitools.openapidiff.core.compare.MapKeyDiff; import org.openapitools.openapidiff.core.compare.OpenApiDiff; +import org.openapitools.openapidiff.core.compare.SchemaDiff; import org.openapitools.openapidiff.core.model.ChangedSchema; import org.openapitools.openapidiff.core.model.DiffContext; import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; @@ -98,11 +99,24 @@ public , X> DeferredChanged diff( .build() .flatMap( values -> super.diff(refSet, leftComponents, rightComponents, left, right, context)); + } else if (isResolvedToPlainSchema(right)) { + // `right` is a composed schema whose allOf/anyOf members have already been merged into it + // by SchemaDiff#resolveComposedSchema, so structurally it is a plain schema. Types and + // formats were compared in SchemaDiff#computeDiffForReal before this result was selected, + // so diff both sides with the result matching the left-hand schema instead of reporting a + // type change. + return SchemaDiff.getSchemaDiffResult(left.getClass(), openApiDiff) + .diff(refSet, leftComponents, rightComponents, left, right, context); } else { return openApiDiff.getSchemaDiff().getTypeChangedSchema(left, right, context); } } + private static boolean isResolvedToPlainSchema(Schema schema) { + return schema instanceof ComposedSchema + && CollectionUtils.isEmpty(((ComposedSchema) schema).getOneOf()); + } + private Map getSchema( Components components, Map mapping, ComposedSchema composedSchema) { Map result = new LinkedHashMap<>(); diff --git a/core/src/test/java/org/openapitools/openapidiff/core/Issue887Test.java b/core/src/test/java/org/openapitools/openapidiff/core/Issue887Test.java index f1406b8b..899bc111 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/Issue887Test.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/Issue887Test.java @@ -1,6 +1,5 @@ package org.openapitools.openapidiff.core; -import static org.assertj.core.api.Assertions.assertThatCode; import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiAreEquals; import org.junit.jupiter.api.Test; @@ -11,19 +10,12 @@ public class Issue887Test { private final String DIRECT_ARRAY = "issue-887-2.yaml"; @Test - public void testAllOfArrayToDirectArrayDoesNotThrow() { - assertThatCode(() -> OpenApiCompare.fromLocations(ALLOF_ARRAY, DIRECT_ARRAY)) - .doesNotThrowAnyException(); - } - - @Test - public void testDirectArrayToAllOfArrayDoesNotThrow() { - assertThatCode(() -> OpenApiCompare.fromLocations(DIRECT_ARRAY, ALLOF_ARRAY)) - .doesNotThrowAnyException(); + public void testAllOfArrayToDirectArrayAreEquals() { + assertOpenApiAreEquals(ALLOF_ARRAY, DIRECT_ARRAY); } @Test - public void testAllOfArrayToDirectArrayAreEquals() { - assertOpenApiAreEquals(ALLOF_ARRAY, DIRECT_ARRAY); + public void testDirectArrayToAllOfArrayAreEquals() { + assertOpenApiAreEquals(DIRECT_ARRAY, ALLOF_ARRAY); } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/Issue920Test.java b/core/src/test/java/org/openapitools/openapidiff/core/Issue920Test.java new file mode 100644 index 00000000..b26561a8 --- /dev/null +++ b/core/src/test/java/org/openapitools/openapidiff/core/Issue920Test.java @@ -0,0 +1,25 @@ +package org.openapitools.openapidiff.core; + +import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; + +import org.junit.jupiter.api.Test; + +public class Issue920Test { + + private final String DIRECT_ARRAY_STRING_ITEM = "issue-920-1.yaml"; + private final String ALLOF_ARRAY_INTEGER_ITEM = "issue-920-2.yaml"; + + /** + * Items must still be compared when a direct array is replaced by an allOf-wrapped array, + * otherwise falling through to a plain schema diff would silently hide item changes. + */ + @Test + public void testDirectArrayToAllOfArrayDetectsChangedItemType() { + assertOpenApiBackwardIncompatible(DIRECT_ARRAY_STRING_ITEM, ALLOF_ARRAY_INTEGER_ITEM); + } + + @Test + public void testAllOfArrayToDirectArrayDetectsChangedItemType() { + assertOpenApiBackwardIncompatible(ALLOF_ARRAY_INTEGER_ITEM, DIRECT_ARRAY_STRING_ITEM); + } +} diff --git a/core/src/test/resources/issue-920-1.yaml b/core/src/test/resources/issue-920-1.yaml new file mode 100644 index 00000000..3237bda6 --- /dev/null +++ b/core/src/test/resources/issue-920-1.yaml @@ -0,0 +1,31 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 + +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestResponse' + +components: + schemas: + TestResponse: + type: object + properties: + valuations: + type: array + items: + $ref: '#/components/schemas/Valuation' + + Valuation: + type: object + properties: + value: + type: string diff --git a/core/src/test/resources/issue-920-2.yaml b/core/src/test/resources/issue-920-2.yaml new file mode 100644 index 00000000..756e1129 --- /dev/null +++ b/core/src/test/resources/issue-920-2.yaml @@ -0,0 +1,32 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 + +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestResponse' + +components: + schemas: + TestResponse: + type: object + properties: + valuations: + allOf: + - $ref: '#/components/schemas/Valuations' + + Valuations: + type: array + items: + type: object + properties: + value: + type: integer