diff --git a/README.md b/README.md index c9cf9f78c..1c35d1002 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,7 @@ usage: openapi-diff compatible --fail-on-incompatible Fail only if API changes broke backward compatibility + --allow-response-enum-additions Do not fail backward compatibility check when enum values are added to responses -h,--help print this message --header use given header for authorisation --html export diff as html in given file @@ -117,6 +118,7 @@ usage: openapi-diff incompatible, compatible --fail-on-incompatible Fail only if API changes broke backward compatibility --fail-on-changed Fail if API changed but is backward compatible + --allow-response-enum-additions Do not fail backward compatibility check when enum values are added to responses --trace be extra verbose --version print the version information and exit --warn Print warning information diff --git a/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java b/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java index 21503e08a..a12f61819 100644 --- a/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java +++ b/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java @@ -17,6 +17,7 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.exception.ExceptionUtils; import org.openapitools.openapidiff.core.OpenApiCompare; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.openapitools.openapidiff.core.output.ConsoleRender; import org.openapitools.openapidiff.core.output.HtmlRender; @@ -49,6 +50,12 @@ public static void main(String... args) { .longOpt("fail-on-changed") .desc("Fail if API changed but is backward compatible") .build()); + options.addOption( + Option.builder() + .longOpt("allow-response-enum-additions") + .desc( + "Do not fail backward compatibility check when enum values are added to responses") + .build()); options.addOption(Option.builder().longOpt("trace").desc("be extra verbose").build()); options.addOption( Option.builder().longOpt("debug").desc("Print debugging information").build()); @@ -172,7 +179,11 @@ public static void main(String... args) { auths = Collections.singletonList(new AuthorizationValue(headers[0], headers[1], "header")); } - ChangedOpenApi result = OpenApiCompare.fromLocations(oldPath, newPath, auths); + OpenApiDiffOptions compareOpts = + OpenApiDiffOptions.builder() + .allowResponseEnumAdditions(line.hasOption("allow-response-enum-additions")) + .build(); + ChangedOpenApi result = OpenApiCompare.fromLocations(oldPath, newPath, auths, compareOpts); ConsoleRender consoleRender = new ConsoleRender(); if (!logLevel.equals("OFF")) { System.out.println(consoleRender.render(result)); diff --git a/core/src/main/java/org/openapitools/openapidiff/core/OpenApiCompare.java b/core/src/main/java/org/openapitools/openapidiff/core/OpenApiCompare.java index 4564733a5..daa3af9b1 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/OpenApiCompare.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/OpenApiCompare.java @@ -7,6 +7,7 @@ import java.io.File; import java.util.List; import org.openapitools.openapidiff.core.compare.OpenApiDiff; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; import org.openapitools.openapidiff.core.model.ChangedOpenApi; public class OpenApiCompare { @@ -40,7 +41,25 @@ public static ChangedOpenApi fromContents(String oldContent, String newContent) */ public static ChangedOpenApi fromContents( String oldContent, String newContent, List auths) { - return fromSpecifications(readContent(oldContent, auths), readContent(newContent, auths)); + return fromContents(oldContent, newContent, auths, OpenApiDiffOptions.builder().build()); + } + + /** + * compare two openapi doc + * + * @param oldContent old api-doc location:Json or Http + * @param newContent new api-doc location:Json or Http + * @param auths + * @param options + * @return Comparison result + */ + public static ChangedOpenApi fromContents( + String oldContent, + String newContent, + List auths, + OpenApiDiffOptions options) { + return fromSpecifications( + readContent(oldContent, auths), readContent(newContent, auths), options); } /** @@ -64,7 +83,21 @@ public static ChangedOpenApi fromFiles(File oldFile, File newFile) { */ public static ChangedOpenApi fromFiles( File oldFile, File newFile, List auths) { - return fromLocations(oldFile.getAbsolutePath(), newFile.getAbsolutePath(), auths); + return fromFiles(oldFile, newFile, auths, OpenApiDiffOptions.builder().build()); + } + + /** + * compare two openapi doc + * + * @param oldFile old api-doc file + * @param newFile new api-doc file + * @param auths + * @param options + * @return Comparison result + */ + public static ChangedOpenApi fromFiles( + File oldFile, File newFile, List auths, OpenApiDiffOptions options) { + return fromLocations(oldFile.getAbsolutePath(), newFile.getAbsolutePath(), auths, options); } /** @@ -88,7 +121,25 @@ public static ChangedOpenApi fromLocations(String oldLocation, String newLocatio */ public static ChangedOpenApi fromLocations( String oldLocation, String newLocation, List auths) { - return fromSpecifications(readLocation(oldLocation, auths), readLocation(newLocation, auths)); + return fromLocations(oldLocation, newLocation, auths, OpenApiDiffOptions.builder().build()); + } + + /** + * compare two openapi doc + * + * @param oldLocation old api-doc location (local or http) + * @param newLocation new api-doc location (local or http) + * @param auths + * @param options + * @return Comparison result + */ + public static ChangedOpenApi fromLocations( + String oldLocation, + String newLocation, + List auths, + OpenApiDiffOptions options) { + return fromSpecifications( + readLocation(oldLocation, auths), readLocation(newLocation, auths), options); } /** @@ -99,7 +150,20 @@ public static ChangedOpenApi fromLocations( * @return Comparison result */ public static ChangedOpenApi fromSpecifications(OpenAPI oldSpec, OpenAPI newSpec) { - return OpenApiDiff.compare(notNull(oldSpec, "old"), notNull(newSpec, "new")); + return fromSpecifications(oldSpec, newSpec, OpenApiDiffOptions.builder().build()); + } + + /** + * compare two openapi doc + * + * @param oldSpec old api-doc specification + * @param newSpec new api-doc specification + * @param options + * @return Comparison result + */ + public static ChangedOpenApi fromSpecifications( + OpenAPI oldSpec, OpenAPI newSpec, OpenApiDiffOptions options) { + return OpenApiDiff.compare(notNull(oldSpec, "old"), notNull(newSpec, "new"), options); } private static OpenAPI notNull(OpenAPI spec, String type) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiff.java index 1f0fe03bf..470b8c3d6 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiff.java @@ -41,6 +41,7 @@ public class OpenApiDiff { private MetadataDiff metadataDiff; private final OpenAPI oldSpecOpenApi; private final OpenAPI newSpecOpenApi; + private final OpenApiDiffOptions options; private List newEndpoints; private List missingEndpoints; private List changedOperations; @@ -50,18 +51,24 @@ public class OpenApiDiff { /* * @param oldSpecOpenApi * @param newSpecOpenApi + * @param diffOptions */ - private OpenApiDiff(OpenAPI oldSpecOpenApi, OpenAPI newSpecOpenApi) { + private OpenApiDiff(OpenAPI oldSpecOpenApi, OpenAPI newSpecOpenApi, OpenApiDiffOptions options) { this.oldSpecOpenApi = oldSpecOpenApi; this.newSpecOpenApi = newSpecOpenApi; + this.options = options; if (null == oldSpecOpenApi || null == newSpecOpenApi) { throw new RuntimeException("one of the old or new object is null"); } + if (null == options) { + throw new IllegalArgumentException("options parameter is null but is required"); + } initializeFields(); } - public static ChangedOpenApi compare(OpenAPI oldSpec, OpenAPI newSpec) { - return new OpenApiDiff(oldSpec, newSpec).compare(); + public static ChangedOpenApi compare( + OpenAPI oldSpec, OpenAPI newSpec, OpenApiDiffOptions diffOptions) { + return new OpenApiDiff(oldSpec, newSpec, diffOptions).compare(); } private void initializeFields() { @@ -87,6 +94,10 @@ private void initializeFields() { this.deferredSchemaCache = new DeferredSchemaCache(this); } + public OpenApiDiffOptions getOptions() { + return options; + } + private ChangedOpenApi compare() { preProcess(oldSpecOpenApi); preProcess(newSpecOpenApi); diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java new file mode 100644 index 000000000..7ca9db786 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java @@ -0,0 +1,31 @@ +package org.openapitools.openapidiff.core.compare; + +public class OpenApiDiffOptions { + // Whether to fail backward compatibility check when enum values are added to responses + private final boolean allowResponseEnumAdditions; + + private OpenApiDiffOptions(boolean allowResponseEnumAdditions) { + this.allowResponseEnumAdditions = allowResponseEnumAdditions; + } + + public boolean isAllowResponseEnumAdditions() { + return allowResponseEnumAdditions; + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private OpenApiDiffOptions built = new OpenApiDiffOptions(false); + + public Builder allowResponseEnumAdditions(boolean allowResponseEnumAdditions) { + built = new OpenApiDiffOptions(allowResponseEnumAdditions); + return this; + } + + public OpenApiDiffOptions build() { + return built; + } + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java index cb969f4fa..feb61df10 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java @@ -82,7 +82,7 @@ public DeferredChanged diff( params.put(oldParams.get(i), newParams.get(i)); } } - DiffContext context = new DiffContext(); + DiffContext context = new DiffContext(openApiDiff.getOptions()); context.setUrl(url); context.setParameters(params); context.setLeftAndRightUrls(url, rightUrl); diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/DiffContext.java b/core/src/main/java/org/openapitools/openapidiff/core/model/DiffContext.java index 485c3518b..a13d50b8e 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/DiffContext.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/DiffContext.java @@ -5,9 +5,11 @@ import java.util.Map; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; public class DiffContext { + private final OpenApiDiffOptions options; private String url; private Map parameters; private PathItem.HttpMethod method; @@ -17,7 +19,8 @@ public class DiffContext { private String leftUrl; private String rightUrl; - public DiffContext() { + public DiffContext(OpenApiDiffOptions options) { + this.options = options; parameters = new HashMap<>(); response = false; request = true; @@ -43,6 +46,10 @@ public DiffContext copyWithLeftRightUrls(String leftUrl, String rightUrl) { return copy().setLeftAndRightUrls(leftUrl, rightUrl); } + public OpenApiDiffOptions getOptions() { + return options; + } + private DiffContext setRequest() { this.request = true; this.response = false; @@ -82,7 +89,7 @@ private DiffContext setMethod(PathItem.HttpMethod method) { } private DiffContext copy() { - DiffContext context = new DiffContext(); + DiffContext context = new DiffContext(options); context.url = this.url; context.parameters = this.parameters; context.method = this.method; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedEnum.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedEnum.java index 0dcd5e6e9..0795ed518 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedEnum.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedEnum.java @@ -13,8 +13,11 @@ public ChangedEnum(List oldValue, List newValue, DiffContext context) { @Override public DiffResult isItemsChanged() { - if (context.isRequest() && getMissing().isEmpty() - || context.isResponse() && getIncreased().isEmpty()) { + if (context.isRequest() && getMissing().isEmpty()) { + return DiffResult.COMPATIBLE; + } + if (context.isResponse() + && (context.getOptions().isAllowResponseEnumAdditions() || getIncreased().isEmpty())) { return DiffResult.COMPATIBLE; } return DiffResult.INCOMPATIBLE; diff --git a/core/src/test/java/org/openapitools/openapidiff/core/BackwardCompatibilityTest.java b/core/src/test/java/org/openapitools/openapidiff/core/BackwardCompatibilityTest.java index 612700ca7..60d73498c 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/BackwardCompatibilityTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/BackwardCompatibilityTest.java @@ -4,6 +4,7 @@ import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; public class BackwardCompatibilityTest { private final String OPENAPI_DOC1 = "backwardCompatibility/bc_1.yaml"; @@ -11,6 +12,10 @@ public class BackwardCompatibilityTest { private final String OPENAPI_DOC3 = "backwardCompatibility/bc_3.yaml"; private final String OPENAPI_DOC4 = "backwardCompatibility/bc_4.yaml"; private final String OPENAPI_DOC5 = "backwardCompatibility/bc_5.yaml"; + private final String OPENAPI_DOC_ENUM_BASE = "backwardCompatibility/bc_enum_base.yaml"; + private final String OPENAPI_DOC_ENUM_REQ_ADDED = "backwardCompatibility/bc_enum_req_added.yaml"; + private final String OPENAPI_DOC_ENUM_RESP_ADDED = + "backwardCompatibility/bc_enum_resp_added.yaml"; @Test public void testNoChange() { @@ -46,4 +51,31 @@ public void testApiOperationChanged() { public void testApiReadWriteOnlyPropertiesChanged() { assertOpenApiBackwardCompatible(OPENAPI_DOC1, OPENAPI_DOC5, true); } + + @Test + public void testEnumRequestValuesAdded() { + assertOpenApiBackwardCompatible(OPENAPI_DOC_ENUM_BASE, OPENAPI_DOC_ENUM_REQ_ADDED, true); + } + + @Test + public void testEnumRequestValuesRemoved() { + assertOpenApiBackwardIncompatible(OPENAPI_DOC_ENUM_REQ_ADDED, OPENAPI_DOC_ENUM_BASE); + } + + @Test + public void testEnumResponseValuesAdded_lenient() { + OpenApiDiffOptions options = + OpenApiDiffOptions.builder().allowResponseEnumAdditions(true).build(); + assertOpenApiBackwardCompatible(OPENAPI_DOC_ENUM_BASE, OPENAPI_DOC_ENUM_RESP_ADDED, options); + } + + @Test + public void testEnumResponseValuesAdded_strict() { + assertOpenApiBackwardIncompatible(OPENAPI_DOC_ENUM_BASE, OPENAPI_DOC_ENUM_RESP_ADDED); + } + + @Test + public void testEnumResponseValuesRemoved() { + assertOpenApiBackwardCompatible(OPENAPI_DOC_ENUM_RESP_ADDED, OPENAPI_DOC_ENUM_BASE, true); + } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/TestUtils.java b/core/src/test/java/org/openapitools/openapidiff/core/TestUtils.java index 1180f2886..8145e1011 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/TestUtils.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/TestUtils.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.slf4j.LoggerFactory.getLogger; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.slf4j.Logger; @@ -27,7 +28,12 @@ public static void assertOpenApiChangedEndpoints(String oldSpec, String newSpec) public static void assertOpenApiBackwardCompatible( String oldSpec, String newSpec, boolean isDiff) { - ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(oldSpec, newSpec); + assertOpenApiBackwardCompatible(oldSpec, newSpec, OpenApiDiffOptions.builder().build()); + } + + public static void assertOpenApiBackwardCompatible( + String oldSpec, String newSpec, OpenApiDiffOptions options) { + ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(oldSpec, newSpec, null, options); LOG.info("Result: {}", changedOpenApi.isChanged().getValue()); assertThat(changedOpenApi.isCompatible()).isTrue(); } diff --git a/core/src/test/resources/backwardCompatibility/bc_enum_base.yaml b/core/src/test/resources/backwardCompatibility/bc_enum_base.yaml new file mode 100644 index 000000000..45cfa364a --- /dev/null +++ b/core/src/test/resources/backwardCompatibility/bc_enum_base.yaml @@ -0,0 +1,37 @@ +openapi: 3.0.0 +info: + description: test backward compatibility for enums + version: 1.0.0 + title: test backward compatibility for enums +paths: + /test-back-compat-enums: + get: + operationId: search + parameters: + - name: param-inline-enum + in: query + required: true + schema: + type: string + enum: + - param-inline-enum-val-1 + - param-inline-enum-val-2 + default: param-inline-enum-val-1 + responses: + '200': + description: successful operation + content: + application/json: + schema: + $ref: '#/components/schemas/SchemaWithEnum' +components: + schemas: + SchemaWithEnum: + type: object + properties: + enum-prop: + type: string + enum: + - enum-prop-val-1 + - enum-prop-val-2 + default: enum-prop-val-1 \ No newline at end of file diff --git a/core/src/test/resources/backwardCompatibility/bc_enum_req_added.yaml b/core/src/test/resources/backwardCompatibility/bc_enum_req_added.yaml new file mode 100644 index 000000000..e3b762e12 --- /dev/null +++ b/core/src/test/resources/backwardCompatibility/bc_enum_req_added.yaml @@ -0,0 +1,38 @@ +openapi: 3.0.0 +info: + description: test backward compatibility for enums + version: 1.0.0 + title: test backward compatibility for enums +paths: + /test-back-compat-enums: + get: + operationId: search + parameters: + - name: param-inline-enum + in: query + required: true + schema: + type: string + enum: + - param-inline-enum-val-1 + - param-inline-enum-val-2 + - param-inline-enum-val-3 + default: param-inline-enum-val-1 + responses: + '200': + description: successful operation + content: + application/json: + schema: + $ref: '#/components/schemas/SchemaWithEnum' +components: + schemas: + SchemaWithEnum: + type: object + properties: + enum-prop: + type: string + enum: + - enum-prop-val-1 + - enum-prop-val-2 + default: enum-prop-val-1 \ No newline at end of file diff --git a/core/src/test/resources/backwardCompatibility/bc_enum_resp_added.yaml b/core/src/test/resources/backwardCompatibility/bc_enum_resp_added.yaml new file mode 100644 index 000000000..e97e40be4 --- /dev/null +++ b/core/src/test/resources/backwardCompatibility/bc_enum_resp_added.yaml @@ -0,0 +1,38 @@ +openapi: 3.0.0 +info: + description: test backward compatibility for enums + version: 1.0.0 + title: test backward compatibility for enums +paths: + /test-back-compat-enums: + get: + operationId: search + parameters: + - name: param-inline-enum + in: query + required: true + schema: + type: string + enum: + - param-inline-enum-val-1 + - param-inline-enum-val-2 + default: param-inline-enum-val-1 + responses: + '200': + description: successful operation + content: + application/json: + schema: + $ref: '#/components/schemas/SchemaWithEnum' +components: + schemas: + SchemaWithEnum: + type: object + properties: + enum-prop: + type: string + enum: + - enum-prop-val-1 + - enum-prop-val-2 + - enum-prop-val-3 + default: enum-prop-val-1 \ No newline at end of file