Option to allow response enum additions

- Addresses issue OpenAPITools#303 - Default behavior unchanged to not allow response enum additions. - Adds option to allow response enum additions, both in CLI and programmatically - Tests added for new behavior related to lenient vs strict response enum behavior - Tests also added for request enum behavior (which has been left unchanged).
westse · Jun 15, 2023 · 2ee5f18 · 2ee5f18
1 parent e1d436a
commit 2ee5f18
Show file tree

Hide file tree

Showing 12 changed files with 292 additions and 14 deletions.
diff --git 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
@@ -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<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);
   }
 
   /**
@@ -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);
   }
 
   /**
@@ -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);
   }
 
   /**
@@ -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
@@ -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;
@@ -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
@@ -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
@@ -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);

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
@@ -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;
@@ -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
@@ -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;

diff --git a/core/src/test/java/org/openapitools/openapidiff/core/BackwardCompatibilityTest.java b/core/src/test/java/org/openapitools/openapidiff/core/BackwardCompatibilityTest.java
@@ -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() {
@@ -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
@@ -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
@@ -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