From 25bd1544c70832a68a3f80da28b92c7ad285d886 Mon Sep 17 00:00:00 2001 From: Timo Rohrberg Date: Tue, 24 Mar 2020 12:39:25 +0100 Subject: [PATCH] Fix Issue #136 new read-only property breaks PUT Adding a new read-only property into an API model should be considered a breaking change for PUT operations. A client not knowing of the new read-only property would get a resource from the API which includes the new read-only property and would send a full update back via PUT omitting that new property. The result is that the property would be cleared on the server side as the client did not sent the retrieved value back. --- .../openapi/diff/model/ChangedSchema.java | 12 ++- .../openapi/test/AddPropPutDiffTest.java | 22 ++++++ src/test/resources/add-prop-put-1.yaml | 71 ++++++++++++++++++ src/test/resources/add-prop-put-2.yaml | 73 +++++++++++++++++++ 4 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 src/test/java/com/qdesrame/openapi/test/AddPropPutDiffTest.java create mode 100644 src/test/resources/add-prop-put-1.yaml create mode 100644 src/test/resources/add-prop-put-2.yaml diff --git a/src/main/java/com/qdesrame/openapi/diff/model/ChangedSchema.java b/src/main/java/com/qdesrame/openapi/diff/model/ChangedSchema.java index ea3d1b2e2..16d1621b7 100644 --- a/src/main/java/com/qdesrame/openapi/diff/model/ChangedSchema.java +++ b/src/main/java/com/qdesrame/openapi/diff/model/ChangedSchema.java @@ -1,6 +1,7 @@ package com.qdesrame.openapi.diff.model; import com.qdesrame.openapi.diff.model.schema.*; +import io.swagger.v3.oas.models.PathItem; import io.swagger.v3.oas.models.media.Schema; import java.util.LinkedHashMap; import java.util.List; @@ -76,10 +77,9 @@ public DiffResult isCoreChanged() { && !discriminatorPropertyChanged) { return DiffResult.NO_CHANGES; } - boolean compatibleForRequest = (oldSchema != null || newSchema == null); boolean compatibleForResponse = missingProperties.isEmpty() && (oldSchema == null || newSchema != null); - if ((context.isRequest() && compatibleForRequest + if ((context.isRequest() && compatibleForRequest() || context.isResponse() && compatibleForResponse) && !changedType && !discriminatorPropertyChanged) { @@ -87,4 +87,12 @@ public DiffResult isCoreChanged() { } return DiffResult.INCOMPATIBLE; } + + private boolean compatibleForRequest() { + if (PathItem.HttpMethod.PUT.equals(context.getMethod())) { + if (increasedProperties.size() > 0) return false; + } + + return (oldSchema != null || newSchema == null); + } } diff --git a/src/test/java/com/qdesrame/openapi/test/AddPropPutDiffTest.java b/src/test/java/com/qdesrame/openapi/test/AddPropPutDiffTest.java new file mode 100644 index 000000000..70852e888 --- /dev/null +++ b/src/test/java/com/qdesrame/openapi/test/AddPropPutDiffTest.java @@ -0,0 +1,22 @@ +package com.qdesrame.openapi.test; + +import static com.qdesrame.openapi.test.TestUtils.assertOpenApiAreEquals; +import static com.qdesrame.openapi.test.TestUtils.assertOpenApiBackwardIncompatible; + +import org.junit.jupiter.api.Test; + +/** Created by trohrberg on 23/03/19. */ +public class AddPropPutDiffTest { + private final String OPENAPI_DOC1 = "add-prop-put-1.yaml"; + private final String OPENAPI_DOC2 = "add-prop-put-2.yaml"; + + @Test + public void testDiffSame() { + assertOpenApiAreEquals(OPENAPI_DOC1, OPENAPI_DOC1); + } + + @Test + public void testDiffDifferent() { + assertOpenApiBackwardIncompatible(OPENAPI_DOC1, OPENAPI_DOC2); + } +} diff --git a/src/test/resources/add-prop-put-1.yaml b/src/test/resources/add-prop-put-1.yaml new file mode 100644 index 000000000..e764df63f --- /dev/null +++ b/src/test/resources/add-prop-put-1.yaml @@ -0,0 +1,71 @@ +openapi: 3.0.0 +servers: + - description: SwaggerHub API Auto Mocking + url: https://virtserver.swaggerhub.com/anshul10s/pet-store/1.0.0 +info: + description: | + This is a sample Petstore server. You can find + out more about Swagger at + [http://swagger.io](http://swagger.io) or on + [irc.freenode.net, #swagger](http://swagger.io/irc/). + version: "1.0.0" + title: Swagger Petstore + termsOfService: 'http://swagger.io/terms/' + contact: + email: apiteam@swagger.io + license: + name: Apache 2.0 + url: 'http://www.apache.org/licenses/LICENSE-2.0.html' +tags: + - name: pet + description: Everything about your Pets + externalDocs: + description: Find out more + url: 'http://swagger.io' + - name: store + description: Access to Petstore orders + - name: user + description: Operations about user + externalDocs: + description: Find out more about our store + url: 'http://swagger.io' +paths: + /store/inventory/{id}: + put: + tags: + - store + summary: Updates the inventory with the given id + description: Updates the inventory with the given id and returns it + operationId: putInventory + parameters: + - name: id + in: path + description: Unique Id of the inventory + required: true + example: a-b-c-d + schema: + type: string + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/Inventory' + responses: + '200': + description: successful operation + content: + application/json: + schema: + $ref: '#/components/schemas/Inventory' +externalDocs: + description: Find out more about Swagger + url: 'http://swagger.io' +components: + schemas: + Inventory: + type: object + properties: + id: + type: string + details: + type: count \ No newline at end of file diff --git a/src/test/resources/add-prop-put-2.yaml b/src/test/resources/add-prop-put-2.yaml new file mode 100644 index 000000000..392301c8f --- /dev/null +++ b/src/test/resources/add-prop-put-2.yaml @@ -0,0 +1,73 @@ +openapi: 3.0.0 +servers: + - description: SwaggerHub API Auto Mocking + url: https://virtserver.swaggerhub.com/anshul10s/pet-store/1.0.0 +info: + description: | + This is a sample Petstore server. You can find + out more about Swagger at + [http://swagger.io](http://swagger.io) or on + [irc.freenode.net, #swagger](http://swagger.io/irc/). + version: "1.0.0" + title: Swagger Petstore + termsOfService: 'http://swagger.io/terms/' + contact: + email: apiteam@swagger.io + license: + name: Apache 2.0 + url: 'http://www.apache.org/licenses/LICENSE-2.0.html' +tags: + - name: pet + description: Everything about your Pets + externalDocs: + description: Find out more + url: 'http://swagger.io' + - name: store + description: Access to Petstore orders + - name: user + description: Operations about user + externalDocs: + description: Find out more about our store + url: 'http://swagger.io' +paths: + /store/inventory/{id}: + put: + tags: + - store + summary: Updates the inventory with the given id + description: Updates the inventory with the given id and returns it + operationId: putInventory + parameters: + - name: id + in: path + description: Unique Id of the inventory + required: true + example: a-b-c-d + schema: + type: string + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/Inventory' + responses: + '200': + description: successful operation + content: + application/json: + schema: + $ref: '#/components/schemas/Inventory' +externalDocs: + description: Find out more about Swagger + url: 'http://swagger.io' +components: + schemas: + Inventory: + type: object + properties: + id: + type: string + details: + type: count + extra_info: + type: string \ No newline at end of file