Skip to content

Add option to allow response enum additions #532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ usage: openapi-diff <old> <new>
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 <property=value> use given header for authorisation
--html <file> export diff as html in given file
Expand Down Expand Up @@ -117,6 +118,7 @@ usage: openapi-diff <old> <new>
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
Expand Down
13 changes: 12 additions & 1 deletion cli/src/main/java/org/openapitools/openapidiff/cli/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -40,7 +41,25 @@ public static ChangedOpenApi fromContents(String oldContent, String newContent)
*/
public static ChangedOpenApi fromContents(
String oldContent, String newContent, List<AuthorizationValue> 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<AuthorizationValue> auths,
OpenApiDiffOptions options) {
return fromSpecifications(
readContent(oldContent, auths), readContent(newContent, auths), options);
}

/**
Expand All @@ -64,7 +83,21 @@ public static ChangedOpenApi fromFiles(File oldFile, File newFile) {
*/
public static ChangedOpenApi fromFiles(
File oldFile, File newFile, List<AuthorizationValue> 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<AuthorizationValue> auths, OpenApiDiffOptions options) {
return fromLocations(oldFile.getAbsolutePath(), newFile.getAbsolutePath(), auths, options);
}

/**
Expand All @@ -88,7 +121,25 @@ public static ChangedOpenApi fromLocations(String oldLocation, String newLocatio
*/
public static ChangedOpenApi fromLocations(
String oldLocation, String newLocation, List<AuthorizationValue> 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<AuthorizationValue> auths,
OpenApiDiffOptions options) {
return fromSpecifications(
readLocation(oldLocation, auths), readLocation(newLocation, auths), options);
}

/**
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class OpenApiDiff {
private MetadataDiff metadataDiff;
private final OpenAPI oldSpecOpenApi;
private final OpenAPI newSpecOpenApi;
private final OpenApiDiffOptions options;
private List<Endpoint> newEndpoints;
private List<Endpoint> missingEndpoints;
private List<ChangedOperation> changedOperations;
Expand All @@ -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() {
Expand All @@ -87,6 +94,10 @@ private void initializeFields() {
this.deferredSchemaCache = new DeferredSchemaCache(this);
}

public OpenApiDiffOptions getOptions() {
return options;
}

private ChangedOpenApi compare() {
preProcess(oldSpecOpenApi);
preProcess(newSpecOpenApi);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public DeferredChanged<ChangedPaths> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> parameters;
private PathItem.HttpMethod method;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ public ChangedEnum(List<T> oldValue, List<T> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@
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";
private final String OPENAPI_DOC2 = "backwardCompatibility/bc_2.yaml";
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() {
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();
}
Expand Down
Loading