Configurable incompatibility checks (#552)

config-file and config-prop CLI options

- Gives user fine-grained control of what constitutes an incompatibile change.

Maven plugin config parameters

- configFiles param for a separate yaml config file Corresponds to CLI --config-file option.
- configProps param for specifying config props in pom.xml. Corresponds to CLI --config-prop option.
- OpenApiDiffMojoTest updated to verify parameters work.

Relies on recent exhaustive testing of incompatible checks to ensure no regressions (see #545).

Fix #551
Fix #550
Fix #303

Author: westse

README.md

@@ -51,6 +51,8 @@ usage: openapi-diff <old> <new>
                                 compatible
     --fail-on-incompatible      Fail only if API changes broke backward
                                 compatibility
+    --config-file               Config file to override default behavior. Supported file formats: .yaml
+    --config-prop               Config property to override default behavior with key:value format (e.g. my.prop:true)
  -h,--help                      print this message
     --header <property=value>   use given header for authorisation
     --html <file>               export diff as html in given file
@@ -119,6 +121,8 @@ 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
+    --config-file               Config file to override default behavior. Supported file formats: .yaml
+    --config-prop               Config property to override default behavior with key:value format (e.g. my.prop:true)
     --trace                     be extra verbose
     --version                   print the version information and exit
     --warn                      Print warning information
@@ -153,6 +157,14 @@ Add openapi-diff to your POM to show diffs when you test your Maven project. You
         <jsonOutputFileName>${project.basedir}/../maven/target/diff.json</jsonOutputFileName>
         <!-- Supply file path for markdown output to file if desired. -->
         <markdownOutputFileName>${project.basedir}/../maven/target/diff.md</markdownOutputFileName>
+        <!-- Supply config file(s), e.g. to disable incompatibility checks. Later files override earlier files -->
+        <configFiles>
+          <configFile>my/config-file.yaml</configFile>
+        </configFiles>
+        <!-- Supply config properties, e.g. to disable incompatibility checks. Overrides configFiles. -->
+        <configProps>
+          <incompatible.response.enum.increased>false</incompatible.response.enum.increased>
+        </configProps>
       </configuration>
     </execution>
   </executions>

cli/src/main/java/org/openapitools/openapidiff/cli/Main.java

@@ -3,6 +3,7 @@
 import ch.qos.logback.classic.Level;
 import io.swagger.v3.parser.core.models.AuthorizationValue;
 import java.io.ByteArrayOutputStream;
+import java.io.File;
 import java.io.FileOutputStream;
 import java.io.OutputStreamWriter;
 import java.util.Collections;
@@ -16,6 +17,7 @@
 import org.apache.commons.cli.ParseException;
 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.AsciidocRender;
 import org.openapitools.openapidiff.core.output.ConsoleRender;
@@ -49,6 +51,19 @@ 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("config-file")
+            .hasArg()
+            .desc("Config file to override default behavior. Supported file formats: .yaml")
+            .build());
+    options.addOption(
+        Option.builder()
+            .longOpt("config-prop")
+            .hasArg()
+            .desc(
+                "Config property to override default behavior. Arg in format of [propKey]:[propVal]")
+            .build());
     options.addOption(Option.builder().longOpt("trace").desc("be extra verbose").build());
     options.addOption(
         Option.builder().longOpt("debug").desc("Print debugging information").build());
@@ -179,7 +194,27 @@ public static void main(String... args) {
         auths = Collections.singletonList(new AuthorizationValue(headers[0], headers[1], "header"));
       }
 
-      ChangedOpenApi result = OpenApiCompare.fromLocations(oldPath, newPath, auths);
+      OpenApiDiffOptions.Builder optionBuilder = OpenApiDiffOptions.builder();
+      String[] configFilePaths = line.getOptionValues("config-file");
+      if (configFilePaths != null) {
+        for (String configFilePath : configFilePaths) {
+          optionBuilder.configYaml(new File(configFilePath));
+        }
+      }
+
+      String[] configProps = line.getOptionValues("config-prop");
+      if (configProps != null) {
+        for (String propKeyAndVal : configProps) {
+          String[] split = propKeyAndVal.split(":");
+          if (split.length != 2 || split[0].isEmpty() || split[1].isEmpty()) {
+            throw new IllegalArgumentException("--config-prop unexpected format: " + propKeyAndVal);
+          }
+          optionBuilder.configProperty(split[0], split[1]);
+        }
+      }
+      OpenApiDiffOptions compareOpts = optionBuilder.build();
+
+      ChangedOpenApi result = OpenApiCompare.fromLocations(oldPath, newPath, auths, compareOpts);
       ConsoleRender consoleRender = new ConsoleRender();
       if (!logLevel.equals("OFF")) {
         ByteArrayOutputStream outputStream = new ByteArrayOutputStream();

core/pom.xml

@@ -40,6 +40,10 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-collections4</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-configuration2</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-api</artifactId>

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) {

core/src/main/java/org/openapitools/openapidiff/core/compare/OAuthFlowDiff.java

@@ -8,6 +8,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import org.openapitools.openapidiff.core.model.ChangedOAuthFlow;
+import org.openapitools.openapidiff.core.model.DiffContext;
 
 public class OAuthFlowDiff {
   private final OpenApiDiff openApiDiff;
@@ -20,8 +21,8 @@ private static Map<String, Object> getExtensions(OAuthFlow oAuthFlow) {
     return ofNullable(oAuthFlow).map(OAuthFlow::getExtensions).orElse(null);
   }
 
-  public Optional<ChangedOAuthFlow> diff(OAuthFlow left, OAuthFlow right) {
-    ChangedOAuthFlow changedOAuthFlow = new ChangedOAuthFlow(left, right);
+  public Optional<ChangedOAuthFlow> diff(OAuthFlow left, OAuthFlow right, DiffContext context) {
+    ChangedOAuthFlow changedOAuthFlow = new ChangedOAuthFlow(left, right, context);
     if (left != null && right != null) {
       changedOAuthFlow
           .setAuthorizationUrl(

core/src/main/java/org/openapitools/openapidiff/core/compare/OAuthFlowsDiff.java

@@ -7,6 +7,7 @@
 import java.util.Map;
 import java.util.Optional;
 import org.openapitools.openapidiff.core.model.ChangedOAuthFlows;
+import org.openapitools.openapidiff.core.model.DiffContext;
 
 public class OAuthFlowsDiff {
   private final OpenApiDiff openApiDiff;
@@ -19,24 +20,24 @@ private static Map<String, Object> getExtensions(OAuthFlows oAuthFlow) {
     return ofNullable(oAuthFlow).map(OAuthFlows::getExtensions).orElse(null);
   }
 
-  public Optional<ChangedOAuthFlows> diff(OAuthFlows left, OAuthFlows right) {
+  public Optional<ChangedOAuthFlows> diff(OAuthFlows left, OAuthFlows right, DiffContext context) {
     ChangedOAuthFlows changedOAuthFlows = new ChangedOAuthFlows(left, right);
     if (left != null && right != null) {
       openApiDiff
           .getOAuthFlowDiff()
-          .diff(left.getImplicit(), right.getImplicit())
+          .diff(left.getImplicit(), right.getImplicit(), context)
           .ifPresent(changedOAuthFlows::setImplicitOAuthFlow);
       openApiDiff
           .getOAuthFlowDiff()
-          .diff(left.getPassword(), right.getPassword())
+          .diff(left.getPassword(), right.getPassword(), context)
           .ifPresent(changedOAuthFlows::setPasswordOAuthFlow);
       openApiDiff
           .getOAuthFlowDiff()
-          .diff(left.getClientCredentials(), right.getClientCredentials())
+          .diff(left.getClientCredentials(), right.getClientCredentials(), context)
           .ifPresent(changedOAuthFlows::setClientCredentialOAuthFlow);
       openApiDiff
           .getOAuthFlowDiff()
-          .diff(left.getAuthorizationCode(), right.getAuthorizationCode())
+          .diff(left.getAuthorizationCode(), right.getAuthorizationCode(), context)
           .ifPresent(changedOAuthFlows::setAuthorizationCodeOAuthFlow);
     }
     openApiDiff

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);
@@ -163,7 +174,7 @@ private void preProcess(OpenAPI openApi) {
   }
 
   private ChangedOpenApi getChangedOpenApi() {
-    return new ChangedOpenApi()
+    return new ChangedOpenApi(options)
         .setMissingEndpoints(missingEndpoints)
         .setNewEndpoints(newEndpoints)
         .setNewSpecOpenApi(newSpecOpenApi)

core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java

@@ -0,0 +1,49 @@
+package org.openapitools.openapidiff.core.compare;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import org.apache.commons.configuration2.CompositeConfiguration;
+import org.apache.commons.configuration2.YAMLConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+
+public class OpenApiDiffOptions {
+  private final CompositeConfiguration config;
+
+  private OpenApiDiffOptions(CompositeConfiguration config) {
+    this.config = config;
+  }
+
+  public CompositeConfiguration getConfig() {
+    return config;
+  }
+
+  public static Builder builder() {
+    return new Builder();
+  }
+
+  public static class Builder {
+    private OpenApiDiffOptions built = new OpenApiDiffOptions(new CompositeConfiguration());
+
+    public Builder configYaml(File file) {
+      YAMLConfiguration yamlConfig = new YAMLConfiguration();
+      try {
+        yamlConfig.read(new FileReader(file));
+      } catch (ConfigurationException | FileNotFoundException e) {
+        throw new IllegalArgumentException("Problem loading config. file=" + file, e);
+      }
+      // Ideally immutable, but since it isn't, we just modify the config directly
+      built.getConfig().addConfigurationFirst(yamlConfig);
+      return this;
+    }
+
+    public Builder configProperty(String propKey, String propVal) {
+      built.getConfig().setProperty(propKey, propVal);
+      return this;
+    }
+
+    public OpenApiDiffOptions build() {
+      return built;
+    }
+  }
+}

core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java

@@ -40,7 +40,7 @@ public DeferredChanged<ChangedPaths> diff(
       final Map<String, PathItem> left, final Map<String, PathItem> right) {
     DeferredBuilder<Changed> builder = new DeferredBuilder<>();
 
-    ChangedPaths changedPaths = new ChangedPaths(left, right);
+    ChangedPaths changedPaths = new ChangedPaths(left, right, openApiDiff.getOptions());
     changedPaths.getIncreased().putAll(right);
 
     left.keySet()
@@ -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);