From 856e4520e566be5e27957a6be8182ed5511068d6 Mon Sep 17 00:00:00 2001 From: Aliaksandr Pinchuk Date: Fri, 14 Feb 2025 00:41:20 +0100 Subject: [PATCH] Change of 'nullable' is not detected Fixes #482 --- .../ComposedSchemaDiffResult.java | 2 +- .../schemadiffresult/SchemaDiffResult.java | 1 + .../openapidiff/core/model/ChangedSchema.java | 18 +++++++ .../core/model/schema/ChangedNullable.java | 54 +++++++++++++++++++ .../{ => schema}/ChangedOneOfSchema.java | 7 ++- .../core/output/MarkdownRender.java | 1 + .../openapidiff/core/SchemaDiffTest.java | 31 +++++++++++ .../schemaDiff/schema-nullable-diff-1.yaml | 28 ++++++++++ .../schemaDiff/schema-nullable-diff-2.yaml | 28 ++++++++++ 9 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNullable.java rename core/src/main/java/org/openapitools/openapidiff/core/model/{ => schema}/ChangedOneOfSchema.java (90%) create mode 100644 core/src/test/resources/schemaDiff/schema-nullable-diff-1.yaml create mode 100644 core/src/test/resources/schemaDiff/schema-nullable-diff-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 e76aa0155..2f86318eb 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,13 +12,13 @@ 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.model.ChangedOneOfSchema; import org.openapitools.openapidiff.core.model.ChangedSchema; import org.openapitools.openapidiff.core.model.DiffContext; import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; import org.openapitools.openapidiff.core.model.deferred.RealizedChanged; import org.openapitools.openapidiff.core.model.deferred.RecursiveSchemaSet; +import org.openapitools.openapidiff.core.model.schema.ChangedOneOfSchema; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/SchemaDiffResult.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/SchemaDiffResult.java index f3b1d44c9..b8a0f1410 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/SchemaDiffResult.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/SchemaDiffResult.java @@ -72,6 +72,7 @@ public , X> DeferredChanged diff( right.getExclusiveMaximum(), context)) .setMultipleOf(new ChangedMultipleOf(left.getMultipleOf(), right.getMultipleOf())) + .setNullable(new ChangedNullable(left.getNullable(), right.getNullable())) .setExamples(new ChangedExamples(left.getExamples(), right.getExamples())) .setExample(new ChangedExample(left.getExample(), right.getExample())); builder diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java index d11575ade..2344a3341 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java @@ -11,7 +11,9 @@ import org.openapitools.openapidiff.core.model.schema.ChangedEnum; import org.openapitools.openapidiff.core.model.schema.ChangedMaxLength; import org.openapitools.openapidiff.core.model.schema.ChangedMultipleOf; +import org.openapitools.openapidiff.core.model.schema.ChangedNullable; import org.openapitools.openapidiff.core.model.schema.ChangedNumericRange; +import org.openapitools.openapidiff.core.model.schema.ChangedOneOfSchema; import org.openapitools.openapidiff.core.model.schema.ChangedReadOnly; import org.openapitools.openapidiff.core.model.schema.ChangedRequired; import org.openapitools.openapidiff.core.model.schema.ChangedWriteOnly; @@ -39,6 +41,7 @@ public class ChangedSchema implements ComposedChanged { protected ChangedMaxLength maxLength; protected ChangedNumericRange numericRange; protected ChangedMultipleOf multipleOf; + protected ChangedNullable nullable; protected boolean discriminatorPropertyChanged; protected ChangedSchema items; protected ChangedOneOfSchema oneOfSchema; @@ -122,6 +125,7 @@ public List getChangedElements() { maxLength, numericRange, multipleOf, + nullable, extensions)) .collect(Collectors.toList()); } @@ -285,6 +289,10 @@ public ChangedMultipleOf getMultipleOf() { return this.multipleOf; } + public ChangedNullable getNullable() { + return this.nullable; + } + public boolean isDiscriminatorPropertyChanged() { return this.discriminatorPropertyChanged; } @@ -433,6 +441,12 @@ public ChangedSchema setMultipleOf(final ChangedMultipleOf multipleOf) { return this; } + public ChangedSchema setNullable(final ChangedNullable nullable) { + clearChangedCache(); + this.nullable = nullable; + return this; + } + public ChangedSchema setDiscriminatorPropertyChanged(final boolean discriminatorPropertyChanged) { clearChangedCache(); this.discriminatorPropertyChanged = discriminatorPropertyChanged; @@ -491,6 +505,7 @@ public boolean equals(Object o) { && Objects.equals(maxLength, that.maxLength) && Objects.equals(numericRange, that.numericRange) && Objects.equals(multipleOf, that.multipleOf) + && Objects.equals(nullable, that.nullable) && Objects.equals(items, that.items) && Objects.equals(oneOfSchema, that.oneOfSchema) && Objects.equals(addProp, that.addProp) @@ -522,6 +537,7 @@ public int hashCode() { maxLength, numericRange, multipleOf, + nullable, discriminatorPropertyChanged, items, oneOfSchema, @@ -575,6 +591,8 @@ public java.lang.String toString() { + this.getNumericRange() + ", multipleOf=" + this.getMultipleOf() + + ", nullable=" + + this.getNullable() + ", discriminatorPropertyChanged=" + this.isDiscriminatorPropertyChanged() + ", items=" diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNullable.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNullable.java new file mode 100644 index 000000000..befc1d16d --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNullable.java @@ -0,0 +1,54 @@ +package org.openapitools.openapidiff.core.model.schema; + +import java.util.Objects; +import org.openapitools.openapidiff.core.model.Changed; +import org.openapitools.openapidiff.core.model.DiffResult; + +public class ChangedNullable implements Changed { + + private final Boolean left; + private final Boolean right; + + public ChangedNullable(Boolean leftNullable, Boolean rightNullable) { + this.left = leftNullable; + this.right = rightNullable; + } + + @Override + public DiffResult isChanged() { + boolean leftValue = left != null && left; + boolean rightValue = right != null && right; + + if (leftValue != rightValue) { + return DiffResult.COMPATIBLE; + } + + return DiffResult.NO_CHANGES; + } + + public Boolean getLeft() { + return left; + } + + public Boolean getRight() { + return right; + } + + @Override + public String toString() { + return "ChangedNullable [left=" + left + ", right=" + right + "]"; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ChangedNullable that = (ChangedNullable) o; + return Objects.equals(left, that.getLeft()) && Objects.equals(right, that.getRight()); + } + + @Override + public int hashCode() { + return Objects.hash(left, right); + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOneOfSchema.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedOneOfSchema.java similarity index 90% rename from core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOneOfSchema.java rename to core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedOneOfSchema.java index d1ecfdbde..60159f569 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOneOfSchema.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedOneOfSchema.java @@ -1,4 +1,4 @@ -package org.openapitools.openapidiff.core.model; +package org.openapitools.openapidiff.core.model.schema; import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_ONEOF_DECREASED; import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_ONEOF_INCREASED; @@ -8,6 +8,11 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import org.openapitools.openapidiff.core.model.Changed; +import org.openapitools.openapidiff.core.model.ChangedSchema; +import org.openapitools.openapidiff.core.model.ComposedChanged; +import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.DiffResult; public class ChangedOneOfSchema implements ComposedChanged { private final Map oldMapping; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/output/MarkdownRender.java b/core/src/main/java/org/openapitools/openapidiff/core/output/MarkdownRender.java index 5e0d0dd76..e55179b15 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/output/MarkdownRender.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/output/MarkdownRender.java @@ -20,6 +20,7 @@ import org.apache.commons.lang3.StringUtils; import org.openapitools.openapidiff.core.exception.RendererException; import org.openapitools.openapidiff.core.model.*; +import org.openapitools.openapidiff.core.model.schema.ChangedOneOfSchema; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; import org.slf4j.Logger; 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 0b4eab6cb..318cae335 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/SchemaDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/SchemaDiffTest.java @@ -134,4 +134,35 @@ public void changeMultipleOfHandling() { assertThat(props.get("field4").getMultipleOf().getLeft()).isEqualTo(BigDecimal.valueOf(10)); assertThat(props.get("field4").getMultipleOf().getRight()).isNull(); } + + @Test // issue #482 + public void changeNullabeHandling() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "schemaDiff/schema-nullable-diff-1.yaml", "schemaDiff/schema-nullable-diff-2.yaml"); + ChangedSchema changedSchema = + getRequestBodyChangedSchema(changedOpenApi, POST, "/schema/nullable", "application/json"); + + assertThat(changedSchema).isNotNull(); + Map props = changedSchema.getChangedProperties(); + assertThat(props).isNotEmpty(); + + // Check no changes in nullable + assertThat(props.get("field0")).isNull(); + + // Check changes in nullable + assertThat(props.get("field1").getNullable().isCompatible()).isTrue(); + assertThat(props.get("field1").getNullable().getLeft()).isTrue(); + assertThat(props.get("field1").getNullable().getRight()).isFalse(); + + // Check deletion of nullable + assertThat(props.get("field2").getNullable().isCompatible()).isTrue(); + assertThat(props.get("field2").getNullable().getLeft()).isTrue(); + assertThat(props.get("field2").getNullable().getRight()).isNull(); + + // Check addition of nullable + assertThat(props.get("field3").getNullable().isCompatible()).isTrue(); + assertThat(props.get("field3").getNullable().getLeft()).isNull(); + assertThat(props.get("field3").getNullable().getRight()).isTrue(); + } } diff --git a/core/src/test/resources/schemaDiff/schema-nullable-diff-1.yaml b/core/src/test/resources/schemaDiff/schema-nullable-diff-1.yaml new file mode 100644 index 000000000..b467c3746 --- /dev/null +++ b/core/src/test/resources/schemaDiff/schema-nullable-diff-1.yaml @@ -0,0 +1,28 @@ +openapi: 3.0.1 +info: + description: Schema diff nullable + title: schema diff nullable + version: 1.0.0 +paths: + /schema/nullable: + post: + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/TestDTO' +components: + schemas: + TestDTO: + type: object + properties: + field0: + type: integer + field1: + type: integer + nullable: true + field2: + type: integer + nullable: true + field3: + type: integer \ No newline at end of file diff --git a/core/src/test/resources/schemaDiff/schema-nullable-diff-2.yaml b/core/src/test/resources/schemaDiff/schema-nullable-diff-2.yaml new file mode 100644 index 000000000..e186f76e6 --- /dev/null +++ b/core/src/test/resources/schemaDiff/schema-nullable-diff-2.yaml @@ -0,0 +1,28 @@ +openapi: 3.0.1 +info: + description: Schema diff nullable + title: schema diff nullable + version: 1.0.0 +paths: + /schema/nullable: + post: + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/TestDTO' +components: + schemas: + TestDTO: + type: object + properties: + field0: + type: integer + field1: + type: integer + nullable: false + field2: + type: integer + field3: + type: integer + nullable: true \ No newline at end of file