diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java index fadae7a1d..95e221318 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java @@ -83,25 +83,29 @@ public static SchemaDiffResult getSchemaDiffResult( protected static Schema resolveComposedSchema( Components components, Schema schema, Set visitedRefs) { - if (schema instanceof ComposedSchema) { - ComposedSchema composedSchema = (ComposedSchema) schema; - List composedSchemas = new ArrayList<>(); - Optional.ofNullable(composedSchema.getAllOf()).ifPresent(composedSchemas::addAll); - Optional.ofNullable(composedSchema.getAnyOf()).ifPresent(composedSchemas::addAll); + if (schema == null) { + return null; + } + // OpenAPI 3.0 compositions are deserialized into ComposedSchema instances, while OpenAPI 3.1 + // compositions become JsonSchema instances; since swagger-models 2.2.x both carry their + // allOf/anyOf members on the shared base Schema, so the members are merged into the parent + // the same way for both versions. + List composedSchemas = new ArrayList<>(); + Optional.ofNullable(schema.getAllOf()).ifPresent(composedSchemas::addAll); + Optional.ofNullable(schema.getAnyOf()).ifPresent(composedSchemas::addAll); - if (!composedSchemas.isEmpty()) { - for (Schema composed : composedSchemas) { - if (composed.get$ref() == null || !visitedRefs.contains(composed.get$ref())) { - Set updatedVisitedRefs = new HashSet<>(visitedRefs); - updatedVisitedRefs.add(composed.get$ref()); - composed = refPointer.resolveRef(components, composed, composed.get$ref()); - composed = resolveComposedSchema(components, composed, updatedVisitedRefs); - addSchema(schema, composed); - } + if (!composedSchemas.isEmpty()) { + for (Schema composed : composedSchemas) { + if (composed.get$ref() == null || !visitedRefs.contains(composed.get$ref())) { + Set updatedVisitedRefs = new HashSet<>(visitedRefs); + updatedVisitedRefs.add(composed.get$ref()); + composed = refPointer.resolveRef(components, composed, composed.get$ref()); + composed = resolveComposedSchema(components, composed, updatedVisitedRefs); + addSchema(schema, composed); } - composedSchema.setAllOf(null); - composedSchema.setAnyOf(null); } + schema.setAllOf(null); + schema.setAnyOf(null); } return schema; } @@ -152,6 +156,9 @@ protected static Schema addSchema(Schema schema, Schema fromSche if (fromSchema.getType() != null) { schema.setType(fromSchema.getType()); } + if (fromSchema.getTypes() != null) { + schema.setTypes(fromSchema.getTypes()); + } if (fromSchema.getEnum() != null) { if (schema.getEnum() == null) { schema.setEnum(new ArrayList<>()); @@ -303,6 +310,27 @@ private static String getSchemaRef(Schema schema) { return ofNullable(schema).map(Schema::get$ref).orElse(null); } + /** + * Returns the union of {@link Schema#getType()} and {@link Schema#getTypes()}. + * + *

The type of a schema is carried by different fields depending on the OpenAPI version and how + * the schema was deserialized: OpenAPI 3.1 schemas (JsonSchema) hold the `type` keyword in the + * `types` set with `getType()` returning null, while OpenAPI 3.0 schemas hold it in `type` (and + * typed concrete schemas mirror it in `types` too). Composed schemas merged from their + * allOf/anyOf children carry whichever field their members provided. Comparing the effective type + * set makes the type-change detection work across all of these representations. + */ + private static Set getEffectiveTypes(Schema schema) { + Set types = new HashSet<>(); + if (schema.getType() != null) { + types.add(schema.getType()); + } + if (schema.getTypes() != null) { + types.addAll(schema.getTypes()); + } + return types; + } + public DeferredChanged diff(Schema left, Schema right, DiffContext context) { return this.diff(new RecursiveSchemaSet(), left, right, context); } @@ -346,11 +374,15 @@ public DeferredChanged computeDiffForReal( left = resolveComposedSchema(leftComponents, left, refSet.getLeftKeys()); right = resolveComposedSchema(rightComponents, right, refSet.getRightKeys()); - // If type of schemas are different, just set old & new schema, set changedType to true in - // SchemaDiffResult and - // return the object + // If the effective type of the schemas are different, just set old & new schema, set + // changedType to true in SchemaDiffResult and return the object + // The comparison must be based on the effective type set (union of `Schema#getType()` and + // `Schema#getTypes()`) only: OpenAPI 3.0 schemas carry the `type` keyword in `type`, while + // OpenAPI 3.1 schemas (JsonSchema) hold it in the `types` set with `getType()` returning + // null, so a raw `getType()` comparison would report a false type change for equivalent + // schemas (see https://github.com/OpenAPITools/openapi-diff/issues/910) if ((left == null || right == null) - || !Objects.equals(left.getType(), right.getType()) + || !Objects.equals(getEffectiveTypes(left), getEffectiveTypes(right)) || !Objects.equals(left.getFormat(), right.getFormat())) { return getTypeChangedSchema(left, right, context); } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/SchemaDiffTest.java b/core/src/test/java/org/openapitools/openapidiff/core/SchemaDiffTest.java index b5cdae032..4039dff83 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/SchemaDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/SchemaDiffTest.java @@ -52,6 +52,22 @@ public void schemaPropertyTypeChanged() { assertTrue(changedSchema.getChangedProperties().get("field1").isChangedType()); } + @Test // issue #910 + public void schemaPropertyTypeChanged31() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "schemaDiff/schema-props-type-handling-3.1-1.yaml", + "schemaDiff/schema-props-type-handling-3.1-2.yaml"); + + assertEquals(1, changedOpenApi.getChangedOperations().size()); + assertEquals(1, changedOpenApi.getChangedSchemas().size()); + ChangedSchema changedSchema = changedOpenApi.getChangedSchemas().get(0); + assertEquals(1, changedSchema.getChangedProperties().size()); + assertTrue(changedSchema.getChangedProperties().containsKey("weight")); + assertTrue(changedSchema.getChangedProperties().get("weight").isChangedType()); + assertTrue(changedOpenApi.isIncompatible()); + } + @Test // issue #485 public void schemaBecomesDeprecatedTest() { ChangedOpenApi changedOpenApi = diff --git a/core/src/test/resources/schemaDiff/schema-props-type-handling-3.1-1.yaml b/core/src/test/resources/schemaDiff/schema-props-type-handling-3.1-1.yaml new file mode 100644 index 000000000..7e381be39 --- /dev/null +++ b/core/src/test/resources/schemaDiff/schema-props-type-handling-3.1-1.yaml @@ -0,0 +1,21 @@ +openapi: 3.1.0 +info: + title: Schema Type Handling Test - Before + version: '1' +paths: + /widgets: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/Widget' +components: + schemas: + Widget: + type: object + properties: + weight: + type: integer diff --git a/core/src/test/resources/schemaDiff/schema-props-type-handling-3.1-2.yaml b/core/src/test/resources/schemaDiff/schema-props-type-handling-3.1-2.yaml new file mode 100644 index 000000000..1cb874354 --- /dev/null +++ b/core/src/test/resources/schemaDiff/schema-props-type-handling-3.1-2.yaml @@ -0,0 +1,21 @@ +openapi: 3.1.0 +info: + title: Schema Type Handling Test - After + version: '1' +paths: + /widgets: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/Widget' +components: + schemas: + Widget: + type: object + properties: + weight: + type: string # Changed from integer to string