From 42c17beb92d351a22178545644c2d2c0bd494247 Mon Sep 17 00:00:00 2001 From: Aliaksandr Pinchuk Date: Thu, 13 Feb 2025 11:41:55 +0100 Subject: [PATCH] Change of 'multipleof' is not detected (#746) --- .../schemadiffresult/SchemaDiffResult.java | 1 + .../openapidiff/core/model/ChangedSchema.java | 23 ++++++++ .../core/model/schema/ChangedMultipleOf.java | 57 +++++++++++++++++++ .../openapidiff/core/ChangesResolver.java | 43 +++++++++++++- .../openapidiff/core/SchemaDiffTest.java | 56 +++++++++++++----- .../schemaDiff/schema-multiple-of-diff-1.yaml | 29 ++++++++++ .../schemaDiff/schema-multiple-of-diff-2.yaml | 29 ++++++++++ 7 files changed, 224 insertions(+), 14 deletions(-) create mode 100644 core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedMultipleOf.java create mode 100644 core/src/test/resources/schemaDiff/schema-multiple-of-diff-1.yaml create mode 100644 core/src/test/resources/schemaDiff/schema-multiple-of-diff-2.yaml 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 1cd63ae8..f3b1d44c 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 @@ -71,6 +71,7 @@ public , X> DeferredChanged diff( left.getExclusiveMaximum(), right.getExclusiveMaximum(), context)) + .setMultipleOf(new ChangedMultipleOf(left.getMultipleOf(), right.getMultipleOf())) .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 aa3de8d1..d11575ad 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 @@ -10,6 +10,7 @@ import java.util.stream.Stream; 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.ChangedNumericRange; import org.openapitools.openapidiff.core.model.schema.ChangedReadOnly; import org.openapitools.openapidiff.core.model.schema.ChangedRequired; @@ -37,6 +38,7 @@ public class ChangedSchema implements ComposedChanged { protected boolean changedType; protected ChangedMaxLength maxLength; protected ChangedNumericRange numericRange; + protected ChangedMultipleOf multipleOf; protected boolean discriminatorPropertyChanged; protected ChangedSchema items; protected ChangedOneOfSchema oneOfSchema; @@ -119,6 +121,7 @@ public List getChangedElements() { required, maxLength, numericRange, + multipleOf, extensions)) .collect(Collectors.toList()); } @@ -274,6 +277,14 @@ public ChangedMaxLength getMaxLength() { return this.maxLength; } + public ChangedNumericRange getNumericRange() { + return this.numericRange; + } + + public ChangedMultipleOf getMultipleOf() { + return this.multipleOf; + } + public boolean isDiscriminatorPropertyChanged() { return this.discriminatorPropertyChanged; } @@ -416,6 +427,12 @@ public ChangedSchema setNumericRange(final ChangedNumericRange numericRange) { return this; } + public ChangedSchema setMultipleOf(final ChangedMultipleOf multipleOf) { + clearChangedCache(); + this.multipleOf = multipleOf; + return this; + } + public ChangedSchema setDiscriminatorPropertyChanged(final boolean discriminatorPropertyChanged) { clearChangedCache(); this.discriminatorPropertyChanged = discriminatorPropertyChanged; @@ -473,6 +490,7 @@ public boolean equals(Object o) { && Objects.equals(writeOnly, that.writeOnly) && Objects.equals(maxLength, that.maxLength) && Objects.equals(numericRange, that.numericRange) + && Objects.equals(multipleOf, that.multipleOf) && Objects.equals(items, that.items) && Objects.equals(oneOfSchema, that.oneOfSchema) && Objects.equals(addProp, that.addProp) @@ -503,6 +521,7 @@ public int hashCode() { changedType, maxLength, numericRange, + multipleOf, discriminatorPropertyChanged, items, oneOfSchema, @@ -552,6 +571,10 @@ public java.lang.String toString() { + this.isChangedType() + ", maxLength=" + this.getMaxLength() + + ", numericRange=" + + this.getNumericRange() + + ", multipleOf=" + + this.getMultipleOf() + ", discriminatorPropertyChanged=" + this.isDiscriminatorPropertyChanged() + ", items=" diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedMultipleOf.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedMultipleOf.java new file mode 100644 index 00000000..adaf1571 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedMultipleOf.java @@ -0,0 +1,57 @@ +package org.openapitools.openapidiff.core.model.schema; + +import java.math.BigDecimal; +import java.util.Objects; +import org.openapitools.openapidiff.core.model.Changed; +import org.openapitools.openapidiff.core.model.DiffResult; + +public class ChangedMultipleOf implements Changed { + + private final BigDecimal left; + private final BigDecimal right; + + public ChangedMultipleOf(BigDecimal leftMultipleOf, BigDecimal rightMultipleOf) { + this.left = leftMultipleOf; + this.right = rightMultipleOf; + } + + @Override + public DiffResult isChanged() { + if (Objects.equals(left, right)) { + return DiffResult.NO_CHANGES; + } + + // multipleof removed -> compatible + if (right == null) { + return DiffResult.COMPATIBLE; + } + + return DiffResult.INCOMPATIBLE; + } + + public BigDecimal getLeft() { + return left; + } + + public BigDecimal getRight() { + return right; + } + + @Override + public String toString() { + return "ChangedMultipleOf [left=" + left + ", right=" + right + "]"; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ChangedMultipleOf that = (ChangedMultipleOf) 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/test/java/org/openapitools/openapidiff/core/ChangesResolver.java b/core/src/test/java/org/openapitools/openapidiff/core/ChangesResolver.java index 0050f95a..cb0fe37b 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/ChangesResolver.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/ChangesResolver.java @@ -17,6 +17,14 @@ public class ChangesResolver { + /** + * Get the ChangedOperation for the given method and path. + * + * @param changedOpenApi the ChangedOpenApi object + * @param method the HTTP method + * @param path the path + * @return the ChangedOperation object + */ @Nullable public static ChangedOperation getChangedOperation( ChangedOpenApi changedOpenApi, HttpMethod method, String path) { @@ -28,6 +36,15 @@ public static ChangedOperation getChangedOperation( .orElse(null); } + /** + * Get the ChangedParameter for the given method, path, and parameter name. + * + * @param changedOpenApi the ChangedOpenApi object + * @param method the HTTP method + * @param path the path + * @param parameterName the parameter name + * @return the ChangedParameter object + */ @Nullable public static ChangedParameter getChangedParameter( ChangedOpenApi changedOpenApi, HttpMethod method, String path, String parameterName) { @@ -47,6 +64,15 @@ public static ChangedParameter getChangedParameter( .orElse(null); } + /** + * Get the ChangedHeaders for the given method, path, and response code. + * + * @param changedOpenApi the ChangedOpenApi object + * @param method the HTTP method + * @param path the path + * @param responseCode the response code + * @return the ChangedHeaders object + */ @Nullable public static ChangedHeaders getChangedResponseHeaders( ChangedOpenApi changedOpenApi, HttpMethod method, String path, String responseCode) { @@ -58,9 +84,24 @@ public static ChangedHeaders getChangedResponseHeaders( .orElse(null); } + /** + * Get the ChangedSchema for the given method, path, and media type. + * + * @param changedOpenApi the ChangedOpenApi object + * @param method the HTTP method + * @param path the path + * @param mediaType the media type + * @return the ChangedSchema object + */ @Nullable public static ChangedSchema getRequestBodyChangedSchema( - ChangedOperation changedOperation, String mediaType) { + ChangedOpenApi changedOpenApi, HttpMethod method, String path, String mediaType) { + ChangedOperation changedOperation = getChangedOperation(changedOpenApi, method, path); + + if (changedOperation == null) { + return null; + } + return Optional.ofNullable(changedOperation) .map(ChangedOperation::getRequestBody) .map(ChangedRequestBody::getContent) 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 9016f2b9..0b4eab6c 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/SchemaDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/SchemaDiffTest.java @@ -1,19 +1,19 @@ package org.openapitools.openapidiff.core; +import static io.swagger.v3.oas.models.PathItem.HttpMethod.POST; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.openapitools.openapidiff.core.ChangesResolver.getChangedOperation; import static org.openapitools.openapidiff.core.ChangesResolver.getRequestBodyChangedSchema; import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardCompatible; -import io.swagger.v3.oas.models.PathItem.HttpMethod; import java.io.ByteArrayOutputStream; import java.io.OutputStreamWriter; +import java.math.BigDecimal; +import java.util.Map; import org.junit.jupiter.api.Test; import org.openapitools.openapidiff.core.model.ChangedOpenApi; -import org.openapitools.openapidiff.core.model.ChangedOperation; import org.openapitools.openapidiff.core.model.ChangedSchema; import org.openapitools.openapidiff.core.output.ConsoleRender; @@ -49,11 +49,9 @@ public void schemaBecomesDeprecatedTest() { "schemaDiff/schema-deprecated-handling-1.yaml", "schemaDiff/schema-deprecated-handling-2.yaml"); - ChangedOperation operation = - getChangedOperation(changedOpenApi, HttpMethod.POST, "/schema-diff/deprecated/added"); - assertNotNull(operation); - - ChangedSchema requestBodySchema = getRequestBodyChangedSchema(operation, "application/json"); + ChangedSchema requestBodySchema = + getRequestBodyChangedSchema( + changedOpenApi, POST, "/schema-diff/deprecated/added", "application/json"); assertNotNull(requestBodySchema); assertTrue(requestBodySchema.isChangeDeprecated()); } @@ -65,11 +63,9 @@ public void schemaBecomesNotDeprecatedTest() { "schemaDiff/schema-deprecated-handling-1.yaml", "schemaDiff/schema-deprecated-handling-2.yaml"); - ChangedOperation operation = - getChangedOperation(changedOpenApi, HttpMethod.POST, "/schema-diff/deprecated/removed"); - assertNotNull(operation); - - ChangedSchema requestBodySchema = getRequestBodyChangedSchema(operation, "application/json"); + ChangedSchema requestBodySchema = + getRequestBodyChangedSchema( + changedOpenApi, POST, "/schema-diff/deprecated/removed", "application/json"); assertNotNull(requestBodySchema); assertTrue(requestBodySchema.isChangeDeprecated()); } @@ -104,4 +100,38 @@ public void addPropertyInPutApiIsCompatible() { assertThat(changedOpenApi.isDifferent()).isTrue(); assertThat(changedOpenApi.isCompatible()).isTrue(); } + + @Test // issues #483 #463 + public void changeMultipleOfHandling() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "schemaDiff/schema-multiple-of-diff-1.yaml", + "schemaDiff/schema-multiple-of-diff-2.yaml"); + ChangedSchema changedSchema = + getRequestBodyChangedSchema( + changedOpenApi, POST, "/schema/numeric/multiple-of", "application/json"); + + assertThat(changedSchema).isNotNull(); + Map props = changedSchema.getChangedProperties(); + assertThat(props).isNotEmpty(); + + // Check changes in multipleOf + assertThat(props.get("field1").getMultipleOf().isIncompatible()).isTrue(); + assertThat(props.get("field1").getMultipleOf().getLeft()).isEqualTo(BigDecimal.valueOf(10)); + assertThat(props.get("field1").getMultipleOf().getRight()).isEqualTo(BigDecimal.valueOf(20)); + + assertThat(props.get("field2").getMultipleOf().isIncompatible()).isTrue(); + assertThat(props.get("field2").getMultipleOf().getLeft()).isEqualTo(BigDecimal.valueOf(0.01)); + assertThat(props.get("field2").getMultipleOf().getRight()).isEqualTo(BigDecimal.valueOf(0.1)); + + // Check addition of multipleOf + assertThat(props.get("field3").getMultipleOf().isIncompatible()).isTrue(); + assertThat(props.get("field3").getMultipleOf().getLeft()).isNull(); + assertThat(props.get("field3").getMultipleOf().getRight()).isEqualTo(BigDecimal.valueOf(10)); + + // Check deletion of multipleOf + assertThat(props.get("field4").getMultipleOf().isCompatible()).isTrue(); + assertThat(props.get("field4").getMultipleOf().getLeft()).isEqualTo(BigDecimal.valueOf(10)); + assertThat(props.get("field4").getMultipleOf().getRight()).isNull(); + } } diff --git a/core/src/test/resources/schemaDiff/schema-multiple-of-diff-1.yaml b/core/src/test/resources/schemaDiff/schema-multiple-of-diff-1.yaml new file mode 100644 index 00000000..49d3f3b1 --- /dev/null +++ b/core/src/test/resources/schemaDiff/schema-multiple-of-diff-1.yaml @@ -0,0 +1,29 @@ +openapi: 3.1.0 +info: + description: Schema diff + title: schema diff + version: 1.0.0 +paths: + /schema/numeric/multiple-of: + post: + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/TestDTO' +components: + schemas: + TestDTO: + type: object + properties: + field1: + type: integer + multipleOf: 10 + field2: + type: integer + multipleOf: 0.01 + field3: + type: integer + field4: + type: integer + multipleOf: 10 \ No newline at end of file diff --git a/core/src/test/resources/schemaDiff/schema-multiple-of-diff-2.yaml b/core/src/test/resources/schemaDiff/schema-multiple-of-diff-2.yaml new file mode 100644 index 00000000..58311d08 --- /dev/null +++ b/core/src/test/resources/schemaDiff/schema-multiple-of-diff-2.yaml @@ -0,0 +1,29 @@ +openapi: 3.1.0 +info: + description: Schema diff + title: schema diff + version: 1.0.0 +paths: + /schema/numeric/multiple-of: + post: + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/TestDTO' +components: + schemas: + TestDTO: + type: object + properties: + field1: + type: integer + multipleOf: 20 + field2: + type: integer + multipleOf: 0.1 + field3: + type: integer + multipleOf: 10 + field4: + type: integer \ No newline at end of file