From f093580a5b4d0fbec1d59b1fab68b89904c7bbfa Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 27 Jun 2023 16:19:47 -0600 Subject: [PATCH 01/26] config-file and config-prop CLI options - Gives user fine-grained control of what constitutes an incompatibile change. --- README.md | 2 + .../openapitools/openapidiff/cli/Main.java | 37 +++++++++- core/pom.xml | 4 ++ .../openapidiff/core/OpenApiCompare.java | 72 +++++++++++++++++-- .../openapidiff/core/compare/OpenApiDiff.java | 17 ++++- .../core/compare/OpenApiDiffOptions.java | 49 +++++++++++++ .../openapidiff/core/compare/PathsDiff.java | 2 +- .../openapidiff/core/model/DiffContext.java | 19 ++++- pom.xml | 5 ++ 9 files changed, 196 insertions(+), 11 deletions(-) create mode 100644 core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java diff --git a/README.md b/README.md index 60d722e7f..4bdd643da 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 + --config-file Config file to override default behaviro. Supported file formats: .yaml -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 + --config-file Config file to override default behaviro. Supported file formats: .yaml --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 f19826a93..330657666 100644 --- a/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java +++ b/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.ConsoleRender; import org.openapitools.openapidiff.core.output.HtmlRender; @@ -48,6 +50,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()); @@ -171,7 +186,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(); diff --git a/core/pom.xml b/core/pom.xml index 50b8ce87d..0f056b319 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -40,6 +40,10 @@ org.apache.commons commons-collections4 + + org.apache.commons + commons-configuration2 + org.slf4j slf4j-api 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..23f4e1c37 --- /dev/null +++ b/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; + } + } +} 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..f2003d375 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 @@ -1,13 +1,17 @@ package org.openapitools.openapidiff.core.model; +import com.fasterxml.jackson.annotation.JsonIgnore; import io.swagger.v3.oas.models.PathItem; import java.util.HashMap; import java.util.Map; +import org.apache.commons.configuration2.Configuration; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; public class DiffContext { + @JsonIgnore private final OpenApiDiffOptions options; private String url; private Map parameters; private PathItem.HttpMethod method; @@ -17,7 +21,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 +48,16 @@ public DiffContext copyWithLeftRightUrls(String leftUrl, String rightUrl) { return copy().setLeftAndRightUrls(leftUrl, rightUrl); } + @JsonIgnore + public OpenApiDiffOptions getOptions() { + return options; + } + + @JsonIgnore + public Configuration getConfig() { + return options.getConfig(); + } + private DiffContext setRequest() { this.request = true; this.response = false; @@ -82,7 +97,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/pom.xml b/pom.xml index 67adc694d..16d0e27e4 100644 --- a/pom.xml +++ b/pom.xml @@ -126,6 +126,11 @@ commons-collections4 4.4 + + org.apache.commons + commons-configuration2 + 2.9.0 + commons-cli commons-cli From 19d79016de0dc83e27a1e32c342e8ab370c4caf4 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Wed, 26 Jul 2023 10:28:22 -0600 Subject: [PATCH 02/26] 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. --- README.md | 14 ++++++- maven-example/pom.xml | 6 +++ .../openapidiff/maven/OpenApiDiffMojo.java | 21 ++++++++++- .../maven/OpenApiDiffMojoTest.java | 15 ++++++++ maven/src/test/resources/newspec.yaml | 36 +++++++++++++----- maven/src/test/resources/oldspec.yaml | 37 ++++++++++++++++--- .../src/test/resources/test-config-file.yaml | 8 ++++ 7 files changed, 120 insertions(+), 17 deletions(-) create mode 100644 maven/src/test/resources/test-config-file.yaml diff --git a/README.md b/README.md index 4bdd643da..f4d7b75f0 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,8 @@ usage: openapi-diff compatible --fail-on-incompatible Fail only if API changes broke backward compatibility - --config-file Config file to override default behaviro. Supported file formats: .yaml + --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 use given header for authorisation --html export diff as html in given file @@ -118,7 +119,8 @@ 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 - --config-file Config file to override default behaviro. Supported file formats: .yaml + --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 +155,14 @@ Add openapi-diff to your POM to show diffs when you test your Maven project. You ${project.basedir}/../maven/target/diff.json ${project.basedir}/../maven/target/diff.md + + + my/config-file.yaml + + + + false + diff --git a/maven-example/pom.xml b/maven-example/pom.xml index bb3d9c7c5..356a10e0e 100644 --- a/maven-example/pom.xml +++ b/maven-example/pom.xml @@ -36,6 +36,12 @@ ${project.basedir}/../maven/target/diff.txt ${project.basedir}/../maven/target/diff.json ${project.basedir}/../maven/target/diff.md + + ${project.basedir}/../maven/src/test/resources/test-config-file.yaml + + + false + diff --git a/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java b/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java index 7e0e82fb7..cb9e58dfd 100644 --- a/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java +++ b/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java @@ -3,9 +3,12 @@ import static org.openapitools.openapidiff.core.utils.FileUtils.writeToFile; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.OutputStreamWriter; import java.io.UncheckedIOException; +import java.util.List; +import java.util.Map; import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; @@ -13,6 +16,7 @@ import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; 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.JsonRender; @@ -46,6 +50,12 @@ public class OpenApiDiffMojo extends AbstractMojo { @Parameter(property = "markdownOutputFileName") String markdownOutputFileName; + @Parameter(property = "configFiles") + List configFiles; + + @Parameter(property = "configProps") + Map configProps; + @Override public void execute() throws MojoExecutionException, MojoFailureException { if (Boolean.TRUE.equals(skip)) { @@ -54,7 +64,16 @@ public void execute() throws MojoExecutionException, MojoFailureException { } try { - final ChangedOpenApi diff = OpenApiCompare.fromLocations(oldSpec, newSpec); + OpenApiDiffOptions.Builder optionBuilder = OpenApiDiffOptions.builder(); + if (configFiles != null) { + configFiles.forEach(optionBuilder::configYaml); + } + if (configProps != null) { + configProps.forEach(optionBuilder::configProperty); + } + OpenApiDiffOptions options = optionBuilder.build(); + + final ChangedOpenApi diff = OpenApiCompare.fromLocations(oldSpec, newSpec, null, options); try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream)) { diff --git a/maven/src/test/java/org/openapitools/openapidiff/maven/OpenApiDiffMojoTest.java b/maven/src/test/java/org/openapitools/openapidiff/maven/OpenApiDiffMojoTest.java index 10bd186e5..3e77eeaa6 100644 --- a/maven/src/test/java/org/openapitools/openapidiff/maven/OpenApiDiffMojoTest.java +++ b/maven/src/test/java/org/openapitools/openapidiff/maven/OpenApiDiffMojoTest.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.Collections; import org.apache.maven.plugin.MojoExecutionException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -21,6 +22,7 @@ class OpenApiDiffMojoTest { private final File oldSpecFile = new File("src/test/resources/oldspec.yaml"); private final File newSpecFile = new File("src/test/resources/newspec.yaml"); + private final File configFile = new File("src/test/resources/test-config-file.yaml"); private final File consoleOutputfile = new File("target/diff.txt"); private final File markdownOutputfile = new File("target/diff.md"); @@ -105,6 +107,19 @@ void Should_BackwardIncompatibilityException_When_WantsExceptionAndSpecIsIncompa assertThrows(BackwardIncompatibilityException.class, mojo::execute); } + @Test + void Should_NotThrow_When_ConfigWithIncompatibleChecksDisabled() { + final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); + mojo.oldSpec = newSpecFile.getAbsolutePath(); + mojo.newSpec = oldSpecFile.getAbsolutePath(); + mojo.failOnIncompatible = true; + mojo.configFiles = Collections.singletonList(configFile); + mojo.configProps = + Collections.singletonMap("incompatible.request.params.required.increased", "false"); + + assertDoesNotThrow(mojo::execute); + } + @Test void Should_Skip_Mojo_WhenSkipIsTrue() { final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); diff --git a/maven/src/test/resources/newspec.yaml b/maven/src/test/resources/newspec.yaml index e95978f04..7d326b882 100644 --- a/maven/src/test/resources/newspec.yaml +++ b/maven/src/test/resources/newspec.yaml @@ -1,15 +1,33 @@ ---- -openapi: 3.0.3 +openapi: 3.0.0 info: - title: Generated API - version: "1.0" + description: myDesc + title: myTitle + version: 1.0.0 paths: - /hello: + /widgets: get: + operationId: listWidgets + parameters: + - name: query-param-1 + in: query + schema: + type: string + - name: query-param-2 + in: query + schema: + type: string responses: - "200": - description: OK + '200': + description: successful operation content: - text/plain: + application/json: schema: - type: string + type: object + properties: + prop1: + type: string + prop2: + type: string + required: + - prop1 + - prop2 diff --git a/maven/src/test/resources/oldspec.yaml b/maven/src/test/resources/oldspec.yaml index 2f2735763..40cd4175e 100644 --- a/maven/src/test/resources/oldspec.yaml +++ b/maven/src/test/resources/oldspec.yaml @@ -1,6 +1,33 @@ ---- -openapi: 3.0.3 +openapi: 3.0.0 info: - title: Generated API - version: "1.0" -paths: {} + description: myDesc + title: myTitle + version: 1.0.0 +paths: + /widgets: + get: + operationId: listWidgets + parameters: + - name: query-param-1 + in: query + required: true + schema: + type: string + - name: query-param-2 + in: query + schema: + type: string + responses: + '200': + description: successful operation + content: + application/json: + schema: + type: object + properties: + prop1: + type: string + prop2: + type: string + required: + - prop1 diff --git a/maven/src/test/resources/test-config-file.yaml b/maven/src/test/resources/test-config-file.yaml new file mode 100644 index 000000000..633ad0be3 --- /dev/null +++ b/maven/src/test/resources/test-config-file.yaml @@ -0,0 +1,8 @@ +#### +## Config file for openapi-diff +#### + +incompatible: + response: + required: + decreased: false From d1b0245d013ace8a396c925d6a554d055a3a50c9 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 18 Jul 2023 16:08:09 -0600 Subject: [PATCH 03/26] Configurable incompatible checks - Enum - Adds BackwardIncompatibleProp enum with first values - TestUtils.assertSpecIncompatible() utility to assert incompatible when the supplied BackwardIncompatibleProp is true, and to assert changed-but-compatibile when false. - EnumBCTest updated to test with BackwardIncompatibleProp enabled and disabled. --- .../core/model/BackwardIncompatibleProp.java | 38 +++++++++++++++++++ .../core/model/schema/ChangedEnum.java | 17 +++++++-- .../openapidiff/core/TestUtils.java | 20 ++++++++++ .../core/backcompat/EnumBCTest.java | 11 ++++-- 4 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java new file mode 100644 index 000000000..9237981c0 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -0,0 +1,38 @@ +package org.openapitools.openapidiff.core.model; + +import static org.apache.commons.lang3.ArrayUtils.isEmpty; + +import org.apache.commons.configuration2.Configuration; + +// Properties to enable/disable backward incompatibility checks +// Note: order is not programmatically significant but keep alphabetical for maintainability +public enum BackwardIncompatibleProp { + REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), + RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), + ; + + private final String propertyName; + private final boolean enabledByDefault; + + BackwardIncompatibleProp(String propertyName, boolean enabledByDefault) { + this.propertyName = propertyName; + this.enabledByDefault = enabledByDefault; + } + + public String getPropertyName() { + return propertyName; + } + + public boolean isEnabledByDefault() { + return enabledByDefault; + } + + public boolean enabled(DiffContext context, Object... formatArgs) { + return enabled(context.getConfig(), formatArgs); + } + + public boolean enabled(Configuration cfg, Object... formatArgs) { + String propName = isEmpty(formatArgs) ? propertyName : String.format(propertyName, formatArgs); + return cfg.getBoolean(propName, enabledByDefault); + } +} 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..ef4486628 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 @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model.schema; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_ENUM_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_ENUM_INCREASED; + import java.util.List; import org.openapitools.openapidiff.core.model.ChangedList; import org.openapitools.openapidiff.core.model.DiffContext; @@ -13,10 +16,16 @@ public ChangedEnum(List oldValue, List newValue, DiffContext context) { @Override public DiffResult isItemsChanged() { - if (context.isRequest() && getMissing().isEmpty() - || context.isResponse() && getIncreased().isEmpty()) { - return DiffResult.COMPATIBLE; + if (context.isRequest() && !getMissing().isEmpty()) { + if (REQUEST_ENUM_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (context.isResponse() && !getIncreased().isEmpty()) { + if (RESPONSE_ENUM_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } } 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 5e07d0bcd..64dac2767 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,8 @@ 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.BackwardIncompatibleProp; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.openapitools.openapidiff.core.model.DiffResult; import org.slf4j.Logger; @@ -40,6 +42,24 @@ public static void assertSpecChangedButCompatible(String oldSpec, String newSpec assertThat(diffResult.isCompatible()).isTrue(); } + public static void assertSpecIncompatible( + String oldSpec, String newSpec, BackwardIncompatibleProp prop) { + OpenApiDiffOptions.Builder builder = OpenApiDiffOptions.builder(); + // Expect incompatible when BackwardIncompatibleProp enabled + builder.configProperty(prop.getPropertyName(), "true"); + OpenApiDiffOptions optsIncompat = builder.build(); + ChangedOpenApi apiIncompat = OpenApiCompare.fromLocations(oldSpec, newSpec, null, optsIncompat); + LOG.info("Result: {}", apiIncompat.isChanged().getValue()); + assertThat(apiIncompat.isIncompatible()).isTrue(); + // Expect changed but compatible when BackwardIncompatibleProp disabled + builder.configProperty(prop.getPropertyName(), "false"); + OpenApiDiffOptions optsCompat = builder.build(); + ChangedOpenApi apiCompat = OpenApiCompare.fromLocations(oldSpec, newSpec, null, optsCompat); + LOG.info("Result: {}", apiCompat.isChanged().getValue()); + assertThat(apiCompat.isDifferent()).isTrue(); + assertThat(apiCompat.isCompatible()).isTrue(); + } + public static void assertOpenApiBackwardCompatible( String oldSpec, String newSpec, boolean isDiff) { ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(oldSpec, newSpec); diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/EnumBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/EnumBCTest.java index 3eecfda39..d5a7921a4 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/EnumBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/EnumBCTest.java @@ -1,10 +1,13 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_ENUM_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_ENUM_INCREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class EnumBCTest { private final String BASE = "bc_enum_base.yaml"; @@ -21,11 +24,13 @@ public void changedButCompatible() { @Test public void requestDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_enum_decreased.yaml"); + BackwardIncompatibleProp prop = REQUEST_ENUM_DECREASED; + assertSpecIncompatible(BASE, "bc_request_enum_decreased.yaml", prop); } @Test public void responseIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_enum_increased.yaml"); + BackwardIncompatibleProp prop = RESPONSE_ENUM_INCREASED; + assertSpecIncompatible(BASE, "bc_response_enum_increased.yaml", prop); } } From a19683a711e163a34e6b1779e00a348fa01773cb Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 18 Jul 2023 16:15:53 -0600 Subject: [PATCH 04/26] Configurable incompatible checks - MaxLength --- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../core/model/schema/ChangedMaxLength.java | 17 +++++++++++++---- .../core/backcompat/MaxLengthBCTest.java | 11 ++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 9237981c0..880f879a8 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -8,7 +8,9 @@ // Note: order is not programmatically significant but keep alphabetical for maintainability public enum BackwardIncompatibleProp { REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), + REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), + RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), ; private final String propertyName; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedMaxLength.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedMaxLength.java index 8bb5933da..efb068bcf 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedMaxLength.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedMaxLength.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model.schema; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_MAX_LENGTH_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_MAX_LENGTH_INCREASED; + import java.util.Objects; import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.DiffContext; @@ -15,11 +18,17 @@ public DiffResult isChanged() { if (Objects.equals(oldValue, newValue)) { return DiffResult.NO_CHANGES; } - if (context.isRequest() && (newValue == null || oldValue != null && oldValue <= newValue) - || context.isResponse() && (oldValue == null || newValue != null && newValue <= oldValue)) { - return DiffResult.COMPATIBLE; + if (context.isRequest() && (oldValue == null || newValue != null && newValue < oldValue)) { + if (REQUEST_MAX_LENGTH_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (context.isResponse() && (newValue == null || oldValue != null && newValue > oldValue)) { + if (RESPONSE_MAX_LENGTH_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public ChangedMaxLength( diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/MaxLengthBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/MaxLengthBCTest.java index 96c019b40..243ff892a 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/MaxLengthBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/MaxLengthBCTest.java @@ -1,9 +1,12 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_MAX_LENGTH_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_MAX_LENGTH_INCREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class MaxLengthBCTest { private final String BASE = "bc_maxlength_base.yaml"; @@ -15,11 +18,13 @@ public void maxLengthUnchanged() { @Test public void requestMaxLengthDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_maxlength_decreased.yaml"); + BackwardIncompatibleProp prop = REQUEST_MAX_LENGTH_DECREASED; + assertSpecIncompatible(BASE, "bc_request_maxlength_decreased.yaml", prop); } @Test public void responseMaxLengthIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_maxlength_increased.yaml"); + BackwardIncompatibleProp prop = RESPONSE_MAX_LENGTH_INCREASED; + assertSpecIncompatible(BASE, "bc_response_maxlength_increased.yaml", prop); } } From 3f43dc55b97fdf0910c84cb8ba61ecc08fdfdbfb Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 18 Jul 2023 16:26:46 -0600 Subject: [PATCH 05/26] Configurable incompatible checks - Required --- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../core/model/schema/ChangedRequired.java | 17 +++++++++++++---- .../core/backcompat/RequiredBCTest.java | 11 ++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 880f879a8..19170880f 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -9,8 +9,10 @@ public enum BackwardIncompatibleProp { REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), + REQUEST_REQUIRED_INCREASED("incompatible.request.required.increased", true), RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), + RESPONSE_REQUIRED_DECREASED("incompatible.response.required.decreased", true), ; private final String propertyName; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedRequired.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedRequired.java index d6b41f3cd..58a9ae49a 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedRequired.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedRequired.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model.schema; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_REQUIRED_INCREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_REQUIRED_DECREASED; + import java.util.List; import org.openapitools.openapidiff.core.model.ChangedList; import org.openapitools.openapidiff.core.model.DiffContext; @@ -13,10 +16,16 @@ public ChangedRequired(List oldValue, List newValue, DiffContext @Override public DiffResult isItemsChanged() { - if (context.isRequest() && getIncreased().isEmpty() - || context.isResponse() && getMissing().isEmpty()) { - return DiffResult.COMPATIBLE; + if (context.isRequest() && !getIncreased().isEmpty()) { + if (REQUEST_REQUIRED_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (context.isResponse() && !getMissing().isEmpty()) { + if (RESPONSE_REQUIRED_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/RequiredBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/RequiredBCTest.java index bda8b2d8e..c7cfadb0a 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/RequiredBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/RequiredBCTest.java @@ -1,10 +1,13 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_REQUIRED_INCREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_REQUIRED_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class RequiredBCTest { @@ -22,11 +25,13 @@ public void changedButCompatible() { @Test public void requestRequiredIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_required_increased.yaml"); + BackwardIncompatibleProp prop = REQUEST_REQUIRED_INCREASED; + assertSpecIncompatible(BASE, "bc_request_required_increased.yaml", prop); } @Test public void responseRequiredDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_required_decreased.yaml"); + BackwardIncompatibleProp prop = RESPONSE_REQUIRED_DECREASED; + assertSpecIncompatible(BASE, "bc_response_required_decreased.yaml", prop); } } From ec14f5844b0c5fa49f1bf722005b8fd0b51c9237 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 18 Jul 2023 16:33:12 -0600 Subject: [PATCH 06/26] Configurable incompatible checks - ApiResponse --- .../core/model/BackwardIncompatibleProp.java | 1 + .../openapidiff/core/model/ChangedApiResponse.java | 10 +++++++--- .../openapidiff/core/backcompat/ApiResponseBCTest.java | 7 +++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 19170880f..fc66fcc4d 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -13,6 +13,7 @@ public enum BackwardIncompatibleProp { RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), RESPONSE_REQUIRED_DECREASED("incompatible.response.required.decreased", true), + RESPONSE_RESPONSES_DECREASED("incompatible.response.responses.decreased", true), ; private final String propertyName; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedApiResponse.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedApiResponse.java index e496f7a71..0b12a0ead 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedApiResponse.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedApiResponse.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_RESPONSES_DECREASED; + import io.swagger.v3.oas.models.responses.ApiResponse; import io.swagger.v3.oas.models.responses.ApiResponses; import java.util.LinkedHashMap; @@ -39,10 +41,12 @@ public DiffResult isCoreChanged() { if (increased.isEmpty() && missing.isEmpty()) { return DiffResult.NO_CHANGES; } - if (!increased.isEmpty() && missing.isEmpty()) { - return DiffResult.COMPATIBLE; + if (!missing.isEmpty()) { + if (RESPONSE_RESPONSES_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public ApiResponses getOldApiResponses() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ApiResponseBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ApiResponseBCTest.java index c93bc5d4a..045a16e48 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ApiResponseBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ApiResponseBCTest.java @@ -1,10 +1,12 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_RESPONSES_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class ApiResponseBCTest { private final String BASE = "bc_response_apiresponse_base.yaml"; @@ -21,6 +23,7 @@ public void changedButCompatible() { @Test public void decreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_apiresponse_decreased.yaml"); + BackwardIncompatibleProp prop = RESPONSE_RESPONSES_DECREASED; + assertSpecIncompatible(BASE, "bc_response_apiresponse_decreased.yaml", prop); } } From d3702b88ef6bb1b3a38ae718d5c0ce3f08b2bf6e Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 18 Jul 2023 16:39:34 -0600 Subject: [PATCH 07/26] Configurable incompatible checks - Content --- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../openapidiff/core/model/ChangedContent.java | 14 +++++++++++--- .../openapidiff/core/backcompat/ContentBCTest.java | 11 ++++++++--- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index fc66fcc4d..ac9c78b15 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -7,9 +7,11 @@ // Properties to enable/disable backward incompatibility checks // Note: order is not programmatically significant but keep alphabetical for maintainability public enum BackwardIncompatibleProp { + REQUEST_CONTENT_DECREASED("incompatible.request.content.decreased", true), REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), REQUEST_REQUIRED_INCREASED("incompatible.request.required.increased", true), + RESPONSE_CONTENT_DECREASED("incompatible.response.content.decreased", true), RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), RESPONSE_REQUIRED_DECREASED("incompatible.response.required.decreased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedContent.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedContent.java index 192b49e66..472803ffd 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedContent.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedContent.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_CONTENT_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_CONTENT_DECREASED; + import io.swagger.v3.oas.models.media.Content; import io.swagger.v3.oas.models.media.MediaType; import java.util.*; @@ -31,10 +34,15 @@ public DiffResult isCoreChanged() { if (increased.isEmpty() && missing.isEmpty()) { return DiffResult.NO_CHANGES; } - if (context.isRequest() && missing.isEmpty() || context.isResponse() && missing.isEmpty()) { - return DiffResult.COMPATIBLE; + if (!missing.isEmpty()) { + if (context.isRequest() && REQUEST_CONTENT_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + if (context.isResponse() && RESPONSE_CONTENT_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public Content getOldContent() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ContentBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ContentBCTest.java index 130255b80..8bd95764e 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ContentBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ContentBCTest.java @@ -1,10 +1,13 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_CONTENT_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_CONTENT_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class ContentBCTest { private final String BASE = "bc_content_base.yaml"; @@ -21,11 +24,13 @@ public void changedButCompatible() { @Test public void requestDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_content_decreased.yaml"); + BackwardIncompatibleProp prop = REQUEST_CONTENT_DECREASED; + assertSpecIncompatible(BASE, "bc_request_content_decreased.yaml", prop); } @Test public void responseDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_content_decreased.yaml"); + BackwardIncompatibleProp prop = RESPONSE_CONTENT_DECREASED; + assertSpecIncompatible(BASE, "bc_response_content_decreased.yaml", prop); } } From e53eb32883327deac118c3bdbaa1d1b49421f7f1 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Wed, 19 Jul 2023 10:44:02 -0600 Subject: [PATCH 08/26] Configurable incompatible checks - Schema - Some incompatible conditions checked for by ChangedSchema did not seem to be reachable when creating unit tests for them. Leaving logic in place to avoid unintentional behavior change, but left in comments. --- .../core/model/BackwardIncompatibleProp.java | 2 + .../openapidiff/core/model/ChangedSchema.java | 58 ++++++++++++++----- .../core/backcompat/SchemaBCTest.java | 15 ++++- 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index ac9c78b15..af80fd900 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -16,6 +16,8 @@ public enum BackwardIncompatibleProp { RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), RESPONSE_REQUIRED_DECREASED("incompatible.response.required.decreased", true), RESPONSE_RESPONSES_DECREASED("incompatible.response.responses.decreased", true), + SCHEMA_DISCRIMINATOR_CHANGED("incompatible.schema.discriminator.changed", true), + SCHEMA_TYPE_CHANGED("incompatible.schema.type.changed", true), ; private final String propertyName; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java index d62cd7b69..a8200ba40 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java @@ -1,5 +1,9 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_REQUIRED_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SCHEMA_DISCRIMINATOR_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SCHEMA_TYPE_CHANGED; + import io.swagger.v3.oas.models.media.Schema; import java.util.*; import java.util.stream.Collectors; @@ -139,24 +143,50 @@ private DiffResult calculateCoreChanged() { && !discriminatorPropertyChanged) { return DiffResult.NO_CHANGES; } - boolean missingRequiredProperties = - oldSchema != null - && oldSchema.getRequired() != null - && missingProperties.keySet().stream() - .anyMatch(missingProperty -> oldSchema.getRequired().contains(missingProperty)); - boolean compatibleForResponse = - !missingRequiredProperties && (oldSchema == null || newSchema != null); - if ((context.isRequest() && compatibleForRequest() - || context.isResponse() && compatibleForResponse) - && !changedType - && !discriminatorPropertyChanged) { - return DiffResult.COMPATIBLE; + if (changedType) { + if (SCHEMA_TYPE_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (discriminatorPropertyChanged) { + if (SCHEMA_DISCRIMINATOR_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + + if (!compatibleForRequest() || !compatibleForResponse()) { + return DiffResult.INCOMPATIBLE; } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } private boolean compatibleForRequest() { - return (oldSchema != null || newSchema == null); + if (context.isRequest()) { + if (oldSchema == null && newSchema != null) { + // TODO: dead code? If not, create test. + return false; + } + } + return true; + } + + private boolean compatibleForResponse() { + if (context.isResponse()) { + if (oldSchema != null) { + if (newSchema == null) { + // TODO: dead code? If not, create test. + return false; + } + if (oldSchema.getRequired() != null + && missingProperties.keySet().stream() + .anyMatch(prop -> oldSchema.getRequired().contains(prop))) { + if (RESPONSE_REQUIRED_DECREASED.enabled(context)) { + return false; + } + } + } + } + return true; } public DiffContext getContext() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SchemaBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SchemaBCTest.java index f0e683b79..781b48f2a 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SchemaBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SchemaBCTest.java @@ -2,9 +2,14 @@ import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_REQUIRED_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SCHEMA_DISCRIMINATOR_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SCHEMA_TYPE_CHANGED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class SchemaBCTest { private final String BASE = "bc_schema_base.yaml"; @@ -21,7 +26,8 @@ public void changedButCompatible() { @Test public void discriminatorChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_schema_discriminator_changed.yaml"); + BackwardIncompatibleProp prop = SCHEMA_DISCRIMINATOR_CHANGED; + assertSpecIncompatible(BASE, "bc_schema_discriminator_changed.yaml", prop); } @Test @@ -37,6 +43,7 @@ public void requestFormatIncreased() { @Test public void requestPropsPutIncreased() { + // See https://github.com/OpenAPITools/openapi-diff/issues/537 assertSpecChangedButCompatible(BASE, "bc_request_schema_props_put_increased.yaml"); } @@ -53,11 +60,13 @@ public void responseFormatIncreased() { @Test public void responsePropsRequiredDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_schema_props_required_decreased.yaml"); + BackwardIncompatibleProp prop = RESPONSE_REQUIRED_DECREASED; + assertSpecIncompatible(BASE, "bc_response_schema_props_required_decreased.yaml", prop); } @Test public void typeChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_schema_type_changed.yaml"); + BackwardIncompatibleProp prop = SCHEMA_TYPE_CHANGED; + assertSpecIncompatible(BASE, "bc_schema_type_changed.yaml", prop); } } From acac9d2e0e98bd38c28138471ad5f931ca277193 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Wed, 19 Jul 2023 11:03:18 -0600 Subject: [PATCH 09/26] Configurable incompatible checks - Extensions --- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../core/model/ChangedExtensions.java | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index af80fd900..406c25def 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -7,6 +7,8 @@ // Properties to enable/disable backward incompatibility checks // Note: order is not programmatically significant but keep alphabetical for maintainability public enum BackwardIncompatibleProp { + EXTENSION_CONTENT_TYPES_DECREASED("incompatible.extension.content.types.decreased", false), + EXTENSION_CONTENT_TYPE_DELETED("incompatible.extension.content.type.%s.deleted", false), REQUEST_CONTENT_DECREASED("incompatible.request.content.decreased", true), REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedExtensions.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedExtensions.java index cde3b1511..f189b3e8c 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedExtensions.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedExtensions.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.EXTENSION_CONTENT_TYPES_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.EXTENSION_CONTENT_TYPE_DELETED; + import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -32,7 +35,20 @@ public List getChangedElements() { @Override public DiffResult isCoreChanged() { - return DiffResult.NO_CHANGES; + if (increased.isEmpty() && missing.isEmpty()) { + return DiffResult.NO_CHANGES; + } + if (!missing.isEmpty()) { + if (EXTENSION_CONTENT_TYPES_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + for (String key : missing.keySet()) { + if (EXTENSION_CONTENT_TYPE_DELETED.enabled(context, key)) { + return DiffResult.INCOMPATIBLE; + } + } + } + return DiffResult.COMPATIBLE; } public Map getOldExtensions() { From 290a4891fa763a25699425abdc1ceeb5335f9fec Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Wed, 19 Jul 2023 13:31:19 -0600 Subject: [PATCH 10/26] Fix NumericRange compatibility checks - Responses should be incompatible if a numeric range widens. - See https://github.com/OpenAPITools/openapi-diff/issues/550 - See later commit which makes configurable the enabling/disabling of this incompatibility check (disabled by default to avoid a breaking change) --- .../model/schema/ChangedNumericRange.java | 64 +++++++++++-------- .../core/backcompat/NumericRangeBCTest.java | 25 ++------ ...c_numericrange_changed_but_compatible.yaml | 19 +++--- 3 files changed, 50 insertions(+), 58 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNumericRange.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNumericRange.java index 537496cd9..79e605600 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNumericRange.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNumericRange.java @@ -25,35 +25,43 @@ public DiffResult isChanged() { && Objects.equals(oldMaximumExclusiveValue, newMaximumExclusiveValue)) { return DiffResult.NO_CHANGES; } - if (context.isRequest() - && (newMinimumValue == null - || oldMinimumValue != null - && oldMinimumValue.unscaledValue().compareTo(newMinimumValue.unscaledValue()) - >= 0) - && (newMaximumValue == null - || oldMaximumValue != null - && oldMaximumValue.unscaledValue().compareTo(newMaximumValue.unscaledValue()) - <= 0) - && (newMinimumExclusiveValue == null - || oldMinimumExclusiveValue != null && newMinimumExclusiveValue == true) - && (newMaximumExclusiveValue == null - || oldMaximumExclusiveValue != null && newMaximumExclusiveValue == true) - || context.isResponse() - && (newMinimumValue == null - || oldMinimumValue != null - && oldMinimumValue.unscaledValue().compareTo(newMinimumValue.unscaledValue()) - >= 0) - && (newMaximumValue == null - || oldMaximumValue != null - && oldMaximumValue.unscaledValue().compareTo(newMaximumValue.unscaledValue()) - <= 0) - && (newMinimumExclusiveValue == null - || oldMinimumExclusiveValue != null && newMinimumExclusiveValue == true) - && (newMaximumExclusiveValue == null - || oldMaximumExclusiveValue != null && newMaximumExclusiveValue == true)) { - return DiffResult.COMPATIBLE; + + boolean exclusiveMaxOld = oldMaximumExclusiveValue != null && oldMaximumExclusiveValue; + boolean exclusiveMinOld = oldMinimumExclusiveValue != null && oldMinimumExclusiveValue; + boolean exclusiveMaxNew = newMaximumExclusiveValue != null && newMaximumExclusiveValue; + boolean exclusiveMinNew = newMinimumExclusiveValue != null && newMinimumExclusiveValue; + int diffMax = compare(oldMaximumValue, newMaximumValue, false); + int diffMin = compare(oldMinimumValue, newMinimumValue, true); + + if (context.isRequest()) { + if (diffMax > 0 || (diffMax == 0 && !exclusiveMaxOld && exclusiveMaxNew)) { + return DiffResult.INCOMPATIBLE; + } + if (diffMin < 0 || (diffMin == 0 && !exclusiveMinOld && exclusiveMinNew)) { + return DiffResult.INCOMPATIBLE; + } + } else if (context.isResponse()) { + if (diffMax < 0 || (diffMax == 0 && exclusiveMaxOld && !exclusiveMaxNew)) { + return DiffResult.INCOMPATIBLE; + } + if (diffMin > 0 || (diffMin == 0 && exclusiveMinOld && !exclusiveMinNew)) { + return DiffResult.INCOMPATIBLE; + } + } + return DiffResult.COMPATIBLE; + } + + private int compare(BigDecimal left, BigDecimal right, boolean nullMeansLessThan) { + if (left == null && right == null) { + return 0; + } + if (left == null) { + return nullMeansLessThan ? -1 : 1; + } + if (right == null) { + return nullMeansLessThan ? 1 : -1; } - return DiffResult.INCOMPATIBLE; + return left.unscaledValue().compareTo(right.unscaledValue()); } public ChangedNumericRange( diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java index 1f713f98a..2f750d616 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java @@ -16,7 +16,6 @@ public void unchanged() { @Test public void changedButCompatible() { - // TODO: Fix bug where response range-narrowing is deemed incompatible, then test here assertSpecChangedButCompatible(BASE, "bc_numericrange_changed_but_compatible.yaml"); } @@ -62,9 +61,7 @@ public void requestMinIncreased() { @Test public void responseExclusiveMaxDeleted() { - // TODO: Should be incompatible because clients may be unprepared for wider range - // (test added to avoid unintentional regression) - assertSpecChangedButCompatible(BASE, "bc_response_numericrange_exclusive_max_deleted.yaml"); + assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_exclusive_max_deleted.yaml"); } @Test @@ -74,9 +71,7 @@ public void responseExclusiveMaxUnset() { @Test public void responseExclusiveMinDeleted() { - // TODO: Should be incompatible because clients may be unprepared for wider range - // (test added to avoid unintentional regression) - assertSpecChangedButCompatible(BASE, "bc_response_numericrange_exclusive_min_deleted.yaml"); + assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_exclusive_min_deleted.yaml"); } @Test @@ -86,29 +81,21 @@ public void responseExclusiveMinUnset() { @Test public void responseMaxDeleted() { - // TODO: Should be incompatible because clients may be unprepared for wider range - // (test added to avoid unintentional regression) - assertSpecChangedButCompatible(BASE, "bc_response_numericrange_max_deleted.yaml"); + assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_max_deleted.yaml"); } @Test public void responseMaxIncreased() { - // TODO: Should be incompatible because clients may be unprepared - // (test added to avoid unintentional regression) - assertSpecChangedButCompatible(BASE, "bc_response_numericrange_max_increased.yaml"); + assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_max_increased.yaml"); } @Test public void responseMinDecreased() { - // TODO: Should be incompatible because clients may be unprepared for wider range - // (test added to avoid unintentional regression) - assertSpecChangedButCompatible(BASE, "bc_response_numericrange_min_decreased.yaml"); + assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_min_decreased.yaml"); } @Test public void responseMinDeleted() { - // TODO: Should be incompatible because clients may be unprepared for wider range - // (test added to avoid unintentional regression) - assertSpecChangedButCompatible(BASE, "bc_response_numericrange_min_deleted.yaml"); + assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_min_deleted.yaml"); } } diff --git a/core/src/test/resources/bc_numericrange_changed_but_compatible.yaml b/core/src/test/resources/bc_numericrange_changed_but_compatible.yaml index b38f2fe9e..f7e5057f4 100644 --- a/core/src/test/resources/bc_numericrange_changed_but_compatible.yaml +++ b/core/src/test/resources/bc_numericrange_changed_but_compatible.yaml @@ -14,7 +14,6 @@ paths: type: integer format: int32 exclusiveMinimum: false - exclusiveMaximum: false application/text: schema: type: integer @@ -25,10 +24,9 @@ paths: schema: type: integer format: int32 - minimum: 9 - maximum: 21 - exclusiveMinimum: true - exclusiveMaximum: true + minimum: 10 + maximum: 20 + exclusiveMinimum: false responses: '200': description: successful operation @@ -37,21 +35,20 @@ paths: schema: type: integer format: int32 - exclusiveMinimum: false - exclusiveMaximum: false + exclusiveMinimum: true application/text: schema: type: integer format: int32 - # TODO: Narrowing response range fails as incompatible, but shouldn't it be ok? minimum: 10 maximum: 20 + exclusiveMinimum: true + exclusiveMaximum: true application/xml: schema: type: integer format: int32 - # TODO: Narrowing response range fails as incompatible, but shouldn't it be ok? - minimum: 10 - maximum: 20 + minimum: 11 + maximum: 19 exclusiveMinimum: true exclusiveMaximum: true From 2111467e116ffd5eb53a3191b91899f2af3f926f Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Thu, 20 Jul 2023 12:27:14 -0600 Subject: [PATCH 11/26] Configurable incompatible checks - NumericRange - See https://github.com/OpenAPITools/openapi-diff/issues/550 - Makes configurable the enabling/disabling of this incompatibility check (disabled by default to avoid a breaking change) --- .../core/model/BackwardIncompatibleProp.java | 2 + .../model/schema/ChangedNumericRange.java | 8 ++++ .../core/backcompat/NumericRangeBCTest.java | 44 ++++++++++++------- 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 406c25def..c3eee533d 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -12,10 +12,12 @@ public enum BackwardIncompatibleProp { REQUEST_CONTENT_DECREASED("incompatible.request.content.decreased", true), REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), + REQUEST_NUMERIC_RANGE_DECREASED("incompatible.request.numeric.range.decreased", true), REQUEST_REQUIRED_INCREASED("incompatible.request.required.increased", true), RESPONSE_CONTENT_DECREASED("incompatible.response.content.decreased", true), RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), + RESPONSE_NUMERIC_RANGE_INCREASED("incompatible.response.numeric.range.increased", false), RESPONSE_REQUIRED_DECREASED("incompatible.response.required.decreased", true), RESPONSE_RESPONSES_DECREASED("incompatible.response.responses.decreased", true), SCHEMA_DISCRIMINATOR_CHANGED("incompatible.schema.discriminator.changed", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNumericRange.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNumericRange.java index 79e605600..cbb7b713d 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNumericRange.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedNumericRange.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model.schema; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_NUMERIC_RANGE_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_NUMERIC_RANGE_INCREASED; + import java.math.BigDecimal; import java.util.Objects; import org.openapitools.openapidiff.core.model.Changed; @@ -26,6 +29,11 @@ public DiffResult isChanged() { return DiffResult.NO_CHANGES; } + if ((context.isRequest() && !REQUEST_NUMERIC_RANGE_DECREASED.enabled(context)) + || (context.isResponse() && !RESPONSE_NUMERIC_RANGE_INCREASED.enabled(context))) { + return DiffResult.COMPATIBLE; + } + boolean exclusiveMaxOld = oldMaximumExclusiveValue != null && oldMaximumExclusiveValue; boolean exclusiveMinOld = oldMinimumExclusiveValue != null && oldMinimumExclusiveValue; boolean exclusiveMaxNew = newMaximumExclusiveValue != null && newMaximumExclusiveValue; diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java index 2f750d616..a589c7ea0 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java @@ -1,8 +1,10 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_NUMERIC_RANGE_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_NUMERIC_RANGE_INCREASED; import org.junit.jupiter.api.Test; @@ -21,81 +23,89 @@ public void changedButCompatible() { @Test public void requestExclusiveMaxCreated() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_exclusive_max_created.yaml"); + assertIncompatibleRequest("bc_request_numericrange_exclusive_max_created.yaml"); } @Test public void requestExclusiveMaxSet() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_exclusive_max_set.yaml"); + assertIncompatibleRequest("bc_request_numericrange_exclusive_max_set.yaml"); } @Test public void requestExclusiveMinCreated() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_exclusive_min_created.yaml"); + assertIncompatibleRequest("bc_request_numericrange_exclusive_min_created.yaml"); } @Test public void requestExclusiveMinSet() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_exclusive_min_set.yaml"); + assertIncompatibleRequest("bc_request_numericrange_exclusive_min_set.yaml"); } @Test public void requestMaxAdded() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_max_added.yaml"); + assertIncompatibleRequest("bc_request_numericrange_max_added.yaml"); } @Test public void requestMaxDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_max_decreased.yaml"); + assertIncompatibleRequest("bc_request_numericrange_max_decreased.yaml"); } @Test public void requestMinAdded() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_min_added.yaml"); + assertIncompatibleRequest("bc_request_numericrange_min_added.yaml"); } @Test public void requestMinIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_min_increased.yaml"); + assertIncompatibleRequest("bc_request_numericrange_min_increased.yaml"); } @Test public void responseExclusiveMaxDeleted() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_exclusive_max_deleted.yaml"); + assertIncompatibleResponse("bc_response_numericrange_exclusive_max_deleted.yaml"); } @Test public void responseExclusiveMaxUnset() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_exclusive_max_unset.yaml"); + assertIncompatibleResponse("bc_response_numericrange_exclusive_max_unset.yaml"); } @Test public void responseExclusiveMinDeleted() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_exclusive_min_deleted.yaml"); + assertIncompatibleResponse("bc_response_numericrange_exclusive_min_deleted.yaml"); } @Test public void responseExclusiveMinUnset() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_exclusive_min_unset.yaml"); + assertIncompatibleResponse("bc_response_numericrange_exclusive_min_unset.yaml"); } @Test public void responseMaxDeleted() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_max_deleted.yaml"); + assertIncompatibleResponse("bc_response_numericrange_max_deleted.yaml"); } @Test public void responseMaxIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_max_increased.yaml"); + assertIncompatibleResponse("bc_response_numericrange_max_increased.yaml"); } @Test public void responseMinDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_min_decreased.yaml"); + assertIncompatibleResponse("bc_response_numericrange_min_decreased.yaml"); } @Test public void responseMinDeleted() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_min_deleted.yaml"); + assertIncompatibleResponse("bc_response_numericrange_min_deleted.yaml"); + } + + private void assertIncompatibleRequest(String newSpec) { + assertSpecIncompatible(BASE, newSpec, REQUEST_NUMERIC_RANGE_DECREASED); + } + + private void assertIncompatibleResponse(String newSpec) { + assertSpecIncompatible(BASE, newSpec, RESPONSE_NUMERIC_RANGE_INCREASED); } } From fac75776db24b4d12f539000dd024e31c0377acd Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Thu, 20 Jul 2023 12:56:16 -0600 Subject: [PATCH 12/26] Configurable incompatible checks - ReadOnly --- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../core/model/schema/ChangedReadOnly.java | 15 ++++++++++++--- .../core/backcompat/ReadOnlyBCTest.java | 13 +++++++++---- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index c3eee533d..04ac9c0a9 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -13,6 +13,8 @@ public enum BackwardIncompatibleProp { REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), REQUEST_NUMERIC_RANGE_DECREASED("incompatible.request.numeric.range.decreased", true), + REQUEST_READONLY_INCREASED("incompatible.request.readonly.increased", true), + REQUEST_READONLY_REQUIRED_DECREASED("incompatible.request.readonly.required.decreased", true), REQUEST_REQUIRED_INCREASED("incompatible.request.required.increased", true), RESPONSE_CONTENT_DECREASED("incompatible.response.content.decreased", true), RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedReadOnly.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedReadOnly.java index 25d86af8f..9bde49acd 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedReadOnly.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedReadOnly.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model.schema; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_READONLY_INCREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_READONLY_REQUIRED_DECREASED; + import java.util.Objects; import java.util.Optional; import org.openapitools.openapidiff.core.model.Changed; @@ -29,10 +32,16 @@ public DiffResult isChanged() { } if (context.isRequest()) { if (Boolean.TRUE.equals(newValue)) { - return DiffResult.INCOMPATIBLE; - } else { - return context.isRequired() ? DiffResult.INCOMPATIBLE : DiffResult.COMPATIBLE; + if (REQUEST_READONLY_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } else if (context.isRequired()) { + // Incompatible because a prev RO prop (invalid) is now not RO and required + if (REQUEST_READONLY_REQUIRED_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } + return DiffResult.COMPATIBLE; } return DiffResult.UNKNOWN; } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ReadOnlyBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ReadOnlyBCTest.java index 579a7e178..02d2fa9a0 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ReadOnlyBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ReadOnlyBCTest.java @@ -1,10 +1,13 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_READONLY_INCREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_READONLY_REQUIRED_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class ReadOnlyBCTest { private final String BASE = "bc_readonly_base.yaml"; @@ -21,12 +24,14 @@ public void changedButCompatible() { @Test public void requestReadOnlyIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_readonly_increased.yaml"); + BackwardIncompatibleProp prop = REQUEST_READONLY_INCREASED; + assertSpecIncompatible(BASE, "bc_request_readonly_increased.yaml", prop); } @Test public void requestReadOnlyRequiredDecreased() { - // TODO: Document why desired or remove support (test added to avoid unintentional regression) - assertOpenApiBackwardIncompatible(BASE, "bc_request_readonly_required_decreased.yaml"); + // Incompatible because a prev RO prop (invalid) is now not RO and required + BackwardIncompatibleProp prop = REQUEST_READONLY_REQUIRED_DECREASED; + assertSpecIncompatible(BASE, "bc_request_readonly_required_decreased.yaml", prop); } } From 38340b6b6d1ed4b25e63255d091a79d4f6917617 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Thu, 20 Jul 2023 14:36:20 -0600 Subject: [PATCH 13/26] Configurable incompatible checks - WriteOnly --- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../core/model/schema/ChangedWriteOnly.java | 14 +++++++++++--- .../core/backcompat/WriteOnlyBCTest.java | 11 ++++++++--- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 04ac9c0a9..0732ef484 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -22,6 +22,8 @@ public enum BackwardIncompatibleProp { RESPONSE_NUMERIC_RANGE_INCREASED("incompatible.response.numeric.range.increased", false), RESPONSE_REQUIRED_DECREASED("incompatible.response.required.decreased", true), RESPONSE_RESPONSES_DECREASED("incompatible.response.responses.decreased", true), + RESPONSE_WRITEONLY_INCREASED("incompatible.response.writeonly.increased", true), + RESPONSE_WRITEONLY_REQUIRED_DECREASED("incompatible.response.writeonly.required.decreased", true), SCHEMA_DISCRIMINATOR_CHANGED("incompatible.schema.discriminator.changed", true), SCHEMA_TYPE_CHANGED("incompatible.schema.type.changed", true), ; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedWriteOnly.java b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedWriteOnly.java index 91d19a644..110b6ede2 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedWriteOnly.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/schema/ChangedWriteOnly.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model.schema; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_WRITEONLY_INCREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_WRITEONLY_REQUIRED_DECREASED; + import java.util.Objects; import java.util.Optional; import org.openapitools.openapidiff.core.model.Changed; @@ -29,10 +32,15 @@ public DiffResult isChanged() { } if (context.isResponse()) { if (Boolean.TRUE.equals(newValue)) { - return DiffResult.INCOMPATIBLE; - } else { - return context.isRequired() ? DiffResult.INCOMPATIBLE : DiffResult.COMPATIBLE; + if (RESPONSE_WRITEONLY_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } else if (context.isRequired()) { + if (RESPONSE_WRITEONLY_REQUIRED_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } + return DiffResult.COMPATIBLE; } return DiffResult.UNKNOWN; } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/WriteOnlyBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/WriteOnlyBCTest.java index f3ea427f0..a12853c82 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/WriteOnlyBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/WriteOnlyBCTest.java @@ -1,10 +1,13 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_WRITEONLY_INCREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_WRITEONLY_REQUIRED_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class WriteOnlyBCTest { private final String BASE = "bc_writeonly_base.yaml"; @@ -21,12 +24,14 @@ public void changedButCompatible() { @Test public void responseWriteOnlyIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_writeonly_increased.yaml"); + BackwardIncompatibleProp prop = RESPONSE_WRITEONLY_INCREASED; + assertSpecIncompatible(BASE, "bc_response_writeonly_increased.yaml", prop); } @Test public void responseWriteOnlyRequiredDecreased() { + BackwardIncompatibleProp prop = RESPONSE_WRITEONLY_REQUIRED_DECREASED; // TODO: Document why desired or remove support (test added to avoid unintentional regression) - assertOpenApiBackwardIncompatible(BASE, "bc_response_writeonly_required_decreased.yaml"); + assertSpecIncompatible(BASE, "bc_response_writeonly_required_decreased.yaml", prop); } } From 8ed4e54497341fe9fe3914822fca4044b7267c9f Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Thu, 20 Jul 2023 15:11:45 -0600 Subject: [PATCH 14/26] Configurable incompatible checks - Headers --- .../core/model/BackwardIncompatibleProp.java | 1 + .../openapidiff/core/model/ChangedHeaders.java | 10 +++++++--- .../core/backcompat/HeadersBCTest.java | 15 +++++++++------ ..._response_headers_changed_but_compatible.yaml} | 0 ...ng.yaml => bc_response_headers_decreased.yaml} | 0 5 files changed, 17 insertions(+), 9 deletions(-) rename core/src/test/resources/{bc_response_headers_increased.yaml => bc_response_headers_changed_but_compatible.yaml} (100%) rename core/src/test/resources/{bc_response_headers_missing.yaml => bc_response_headers_decreased.yaml} (100%) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 0732ef484..07bffbcce 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -18,6 +18,7 @@ public enum BackwardIncompatibleProp { REQUEST_REQUIRED_INCREASED("incompatible.request.required.increased", true), RESPONSE_CONTENT_DECREASED("incompatible.response.content.decreased", true), RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), + RESPONSE_HEADERS_DECREASED("incompatible.response.headers.decreased", true), RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), RESPONSE_NUMERIC_RANGE_INCREASED("incompatible.response.numeric.range.increased", false), RESPONSE_REQUIRED_DECREASED("incompatible.response.required.decreased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedHeaders.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedHeaders.java index 451e94f1e..dd228da0f 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedHeaders.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedHeaders.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_HEADERS_DECREASED; + import io.swagger.v3.oas.models.headers.Header; import java.util.ArrayList; import java.util.List; @@ -31,10 +33,12 @@ public DiffResult isCoreChanged() { if (increased.isEmpty() && missing.isEmpty()) { return DiffResult.NO_CHANGES; } - if (missing.isEmpty()) { - return DiffResult.COMPATIBLE; + if (!missing.isEmpty()) { + if (RESPONSE_HEADERS_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public Map getOldHeaders() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeadersBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeadersBCTest.java index b796a35e6..e08b4b8c8 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeadersBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeadersBCTest.java @@ -1,26 +1,29 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_HEADERS_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class HeadersBCTest { private final String BASE = "bc_response_headers_base.yaml"; @Test - public void responseHeadersUnchanged() { + public void unchanged() { assertSpecUnchanged(BASE, BASE); } @Test - public void responseHeadersIncreased() { - assertSpecChangedButCompatible(BASE, "bc_response_headers_increased.yaml"); + public void changedButCompatible() { + assertSpecChangedButCompatible(BASE, "bc_response_headers_changed_but_compatible.yaml"); } @Test - public void responseHeadersMissing() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_headers_missing.yaml"); + public void responseDecreased() { + BackwardIncompatibleProp prop = RESPONSE_HEADERS_DECREASED; + assertSpecIncompatible(BASE, "bc_response_headers_decreased.yaml", prop); } } diff --git a/core/src/test/resources/bc_response_headers_increased.yaml b/core/src/test/resources/bc_response_headers_changed_but_compatible.yaml similarity index 100% rename from core/src/test/resources/bc_response_headers_increased.yaml rename to core/src/test/resources/bc_response_headers_changed_but_compatible.yaml diff --git a/core/src/test/resources/bc_response_headers_missing.yaml b/core/src/test/resources/bc_response_headers_decreased.yaml similarity index 100% rename from core/src/test/resources/bc_response_headers_missing.yaml rename to core/src/test/resources/bc_response_headers_decreased.yaml From e0609bbb69231d8bb8c5f94383703f0e8eb56f8a Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Thu, 20 Jul 2023 16:22:20 -0600 Subject: [PATCH 15/26] Configurable incompatible checks - Header --- .../core/model/BackwardIncompatibleProp.java | 3 ++ .../openapidiff/core/model/ChangedHeader.java | 30 +++++++++++++++++-- .../core/backcompat/HeaderBCTest.java | 29 +++++++++++------- .../resources/bc_response_header_base.yaml | 8 +++++ ...sponse_header_changed_but_compatible.yaml} | 11 +++++++ ...> bc_response_header_explode_changed.yaml} | 7 +++++ ...c_response_header_required_decreased.yaml} | 10 +++++-- ...c_response_header_required_increased.yaml} | 8 +++++ 8 files changed, 90 insertions(+), 16 deletions(-) rename core/src/test/resources/{bc_response_header_required_deleted.yaml => bc_response_header_changed_but_compatible.yaml} (57%) rename core/src/test/resources/{bc_response_header_explode.yaml => bc_response_header_explode_changed.yaml} (72%) rename core/src/test/resources/{bc_response_header_deprecated.yaml => bc_response_header_required_decreased.yaml} (66%) rename core/src/test/resources/{bc_response_header_required_added.yaml => bc_response_header_required_increased.yaml} (69%) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 07bffbcce..8d14ec22b 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -18,6 +18,9 @@ public enum BackwardIncompatibleProp { REQUEST_REQUIRED_INCREASED("incompatible.request.required.increased", true), RESPONSE_CONTENT_DECREASED("incompatible.response.content.decreased", true), RESPONSE_ENUM_INCREASED("incompatible.response.enum.increased", true), + RESPONSE_HEADER_EXPLODE_CHANGED("incompatible.response.header.explode.changed", true), + RESPONSE_HEADER_REQUIRED_DECREASED("incompatible.response.header.required.decreased", true), + RESPONSE_HEADER_REQUIRED_INCREASED("incompatible.response.header.required.increased", true), RESPONSE_HEADERS_DECREASED("incompatible.response.headers.decreased", true), RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), RESPONSE_NUMERIC_RANGE_INCREASED("incompatible.response.numeric.range.increased", false), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedHeader.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedHeader.java index 8ec56568f..b7f505a9b 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedHeader.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedHeader.java @@ -1,5 +1,9 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_HEADER_EXPLODE_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_HEADER_REQUIRED_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_HEADER_REQUIRED_INCREASED; + import io.swagger.v3.oas.models.headers.Header; import java.util.Arrays; import java.util.List; @@ -34,10 +38,30 @@ public DiffResult isCoreChanged() { if (!required && !deprecated && !style && !explode) { return DiffResult.NO_CHANGES; } - if (!required && !style && !explode) { - return DiffResult.COMPATIBLE; + if (explode) { + if (RESPONSE_HEADER_EXPLODE_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (required) { + boolean requiredOld = oldHeader.getRequired() != null ? oldHeader.getRequired() : false; + boolean requiredNew = newHeader.getRequired() != null ? newHeader.getRequired() : false; + if (requiredOld && !requiredNew) { + if (RESPONSE_HEADER_REQUIRED_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (!requiredOld && requiredNew) { + // TODO: Document why desired or remove support. Client will just ignore new header? + if (RESPONSE_HEADER_REQUIRED_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + } + if (style) { + return DiffResult.INCOMPATIBLE; } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public Header getOldHeader() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeaderBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeaderBCTest.java index d97209d19..bf36d7772 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeaderBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeaderBCTest.java @@ -1,37 +1,44 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_HEADER_EXPLODE_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_HEADER_REQUIRED_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_HEADER_REQUIRED_INCREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class HeaderBCTest { private final String BASE = "bc_response_header_base.yaml"; @Test - public void responseHeaderUnchanged() { + public void unchanged() { assertSpecUnchanged(BASE, BASE); } @Test - public void responseHeaderDeprecated() { - assertSpecChangedButCompatible(BASE, "bc_response_header_deprecated.yaml"); + public void changedButCompatible() { + assertSpecChangedButCompatible(BASE, "bc_response_header_changed_but_compatible.yaml"); } @Test - public void responseHeaderRequiredAdded() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_header_required_added.yaml"); + public void responseExplodeChanged() { + BackwardIncompatibleProp prop = RESPONSE_HEADER_EXPLODE_CHANGED; + assertSpecIncompatible(BASE, "bc_response_header_explode_changed.yaml", prop); } @Test - public void responseHeaderRequiredDeleted() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_header_required_deleted.yaml"); + public void responseRequiredDecreased() { + BackwardIncompatibleProp prop = RESPONSE_HEADER_REQUIRED_DECREASED; + assertSpecIncompatible(BASE, "bc_response_header_required_decreased.yaml", prop); } @Test - public void responseHeaderExplode() { - String RESPONSE_HEADER_EXPLODE = "bc_response_header_explode.yaml"; - assertOpenApiBackwardIncompatible(BASE, RESPONSE_HEADER_EXPLODE); + public void responseRequiredIncreased() { + // TODO: Document why desired or remove support (test added to avoid unintentional regression) + BackwardIncompatibleProp prop = RESPONSE_HEADER_REQUIRED_INCREASED; + assertSpecIncompatible(BASE, "bc_response_header_required_increased.yaml", prop); } } diff --git a/core/src/test/resources/bc_response_header_base.yaml b/core/src/test/resources/bc_response_header_base.yaml index 17716ba06..1680fee21 100644 --- a/core/src/test/resources/bc_response_header_base.yaml +++ b/core/src/test/resources/bc_response_header_base.yaml @@ -19,3 +19,11 @@ paths: X-Header-2: schema: type: integer + X-Header-3: + schema: + type: integer + explode: false + X-Header-4: + schema: + type: integer + explode: true diff --git a/core/src/test/resources/bc_response_header_required_deleted.yaml b/core/src/test/resources/bc_response_header_changed_but_compatible.yaml similarity index 57% rename from core/src/test/resources/bc_response_header_required_deleted.yaml rename to core/src/test/resources/bc_response_header_changed_but_compatible.yaml index c227a3921..b660903db 100644 --- a/core/src/test/resources/bc_response_header_required_deleted.yaml +++ b/core/src/test/resources/bc_response_header_changed_but_compatible.yaml @@ -12,9 +12,20 @@ paths: description: successful operation headers: X-Header-1: + required: true schema: type: integer format: int32 + deprecated: false X-Header-2: schema: type: integer + deprecated: true + X-Header-3: + schema: + type: integer + explode: false + X-Header-4: + schema: + type: integer + explode: true diff --git a/core/src/test/resources/bc_response_header_explode.yaml b/core/src/test/resources/bc_response_header_explode_changed.yaml similarity index 72% rename from core/src/test/resources/bc_response_header_explode.yaml rename to core/src/test/resources/bc_response_header_explode_changed.yaml index d48a726a7..3403c28eb 100644 --- a/core/src/test/resources/bc_response_header_explode.yaml +++ b/core/src/test/resources/bc_response_header_explode_changed.yaml @@ -20,3 +20,10 @@ paths: schema: type: integer explode: true + X-Header-3: + schema: + type: integer + X-Header-4: + schema: + type: integer + explode: false diff --git a/core/src/test/resources/bc_response_header_deprecated.yaml b/core/src/test/resources/bc_response_header_required_decreased.yaml similarity index 66% rename from core/src/test/resources/bc_response_header_deprecated.yaml rename to core/src/test/resources/bc_response_header_required_decreased.yaml index 0c60132bd..9b457cdc7 100644 --- a/core/src/test/resources/bc_response_header_deprecated.yaml +++ b/core/src/test/resources/bc_response_header_required_decreased.yaml @@ -12,11 +12,17 @@ paths: description: successful operation headers: X-Header-1: - required: true schema: type: integer format: int32 X-Header-2: - deprecated: true schema: type: integer + X-Header-3: + schema: + type: integer + explode: false + X-Header-4: + schema: + type: integer + explode: true diff --git a/core/src/test/resources/bc_response_header_required_added.yaml b/core/src/test/resources/bc_response_header_required_increased.yaml similarity index 69% rename from core/src/test/resources/bc_response_header_required_added.yaml rename to core/src/test/resources/bc_response_header_required_increased.yaml index 83bceafe8..7d76aec7e 100644 --- a/core/src/test/resources/bc_response_header_required_added.yaml +++ b/core/src/test/resources/bc_response_header_required_increased.yaml @@ -17,6 +17,14 @@ paths: type: integer format: int32 X-Header-2: + schema: + type: integer required: true + X-Header-3: + schema: + type: integer + explode: false + X-Header-4: schema: type: integer + explode: true From ac3981c5ea4ac4580c815bfdfb5dc14f33801fce Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Thu, 20 Jul 2023 16:37:37 -0600 Subject: [PATCH 16/26] Configurable incompatible checks - OneOf --- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../core/model/ChangedOneOfSchema.java | 16 +++++++++++++--- .../openapidiff/core/backcompat/OneOfBCTest.java | 15 ++++++++++----- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 8d14ec22b..a5abf1971 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -13,6 +13,7 @@ public enum BackwardIncompatibleProp { REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), REQUEST_NUMERIC_RANGE_DECREASED("incompatible.request.numeric.range.decreased", true), + REQUEST_ONEOF_DECREASED("incompatible.request.oneof.decreased", true), REQUEST_READONLY_INCREASED("incompatible.request.readonly.increased", true), REQUEST_READONLY_REQUIRED_DECREASED("incompatible.request.readonly.required.decreased", true), REQUEST_REQUIRED_INCREASED("incompatible.request.required.increased", true), @@ -24,6 +25,7 @@ public enum BackwardIncompatibleProp { RESPONSE_HEADERS_DECREASED("incompatible.response.headers.decreased", true), RESPONSE_MAX_LENGTH_INCREASED("incompatible.response.max.length.increased", true), RESPONSE_NUMERIC_RANGE_INCREASED("incompatible.response.numeric.range.increased", false), + RESPONSE_ONEOF_INCREASED("incompatible.response.oneof.increased", true), RESPONSE_REQUIRED_DECREASED("incompatible.response.required.decreased", true), RESPONSE_RESPONSES_DECREASED("incompatible.response.responses.decreased", true), RESPONSE_WRITEONLY_INCREASED("incompatible.response.writeonly.increased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOneOfSchema.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOneOfSchema.java index ebf07d2b7..d1ecfdbde 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOneOfSchema.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOneOfSchema.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_ONEOF_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_ONEOF_INCREASED; + import io.swagger.v3.oas.models.media.Schema; import java.util.ArrayList; import java.util.List; @@ -31,10 +34,17 @@ public DiffResult isCoreChanged() { if (increased.isEmpty() && missing.isEmpty()) { return DiffResult.NO_CHANGES; } - if (context.isRequest() && missing.isEmpty() || context.isResponse() && increased.isEmpty()) { - return DiffResult.COMPATIBLE; + if (context.isRequest() && !missing.isEmpty()) { + if (REQUEST_ONEOF_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (context.isResponse() && !increased.isEmpty()) { + if (RESPONSE_ONEOF_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public Map getOldMapping() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OneOfBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OneOfBCTest.java index e42829cca..fb2c06291 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OneOfBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OneOfBCTest.java @@ -1,10 +1,13 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_ONEOF_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_ONEOF_INCREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class OneOfBCTest { private final String BASE = "bc_oneof_base.yaml"; @@ -20,12 +23,14 @@ public void changedButCompatible() { } @Test - public void requestOneOfDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_oneof_decreased.yaml"); + public void requestDecreased() { + BackwardIncompatibleProp prop = REQUEST_ONEOF_DECREASED; + assertSpecIncompatible(BASE, "bc_request_oneof_decreased.yaml", prop); } @Test - public void responseOneOfIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_response_oneof_increased.yaml"); + public void responseIncreased() { + BackwardIncompatibleProp prop = RESPONSE_ONEOF_INCREASED; + assertSpecIncompatible(BASE, "bc_response_oneof_increased.yaml", prop); } } From 839cb0490c6f11ddb41df1a79c2bb256a76e0dfb Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Fri, 21 Jul 2023 09:25:21 -0600 Subject: [PATCH 17/26] Configurable incompatible checks - OpenApi --- .../openapidiff/core/compare/OpenApiDiff.java | 2 +- .../core/model/BackwardIncompatibleProp.java | 1 + .../openapidiff/core/model/ChangedOpenApi.java | 16 +++++++++++++--- .../core/backcompat/OpenApiBCTest.java | 7 +++++-- .../openapidiff/core/utils/FileUtilsTest.java | 3 ++- 5 files changed, 22 insertions(+), 7 deletions(-) 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 470b8c3d6..f56ed04da 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 @@ -174,7 +174,7 @@ private void preProcess(OpenAPI openApi) { } private ChangedOpenApi getChangedOpenApi() { - return new ChangedOpenApi() + return new ChangedOpenApi(options) .setMissingEndpoints(missingEndpoints) .setNewEndpoints(newEndpoints) .setNewSpecOpenApi(newSpecOpenApi) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index a5abf1971..dc12836b0 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -9,6 +9,7 @@ public enum BackwardIncompatibleProp { EXTENSION_CONTENT_TYPES_DECREASED("incompatible.extension.content.types.decreased", false), EXTENSION_CONTENT_TYPE_DELETED("incompatible.extension.content.type.%s.deleted", false), + OPENAPI_ENDPOINTS_DECREASED("incompatible.openapi.endpoints.decreased", true), REQUEST_CONTENT_DECREASED("incompatible.request.content.decreased", true), REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOpenApi.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOpenApi.java index e1bdd20bd..d0f23ca16 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOpenApi.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOpenApi.java @@ -1,14 +1,18 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.OPENAPI_ENDPOINTS_DECREASED; + import com.fasterxml.jackson.annotation.JsonIgnore; import io.swagger.v3.oas.models.OpenAPI; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; import org.openapitools.openapidiff.core.utils.EndpointUtils; public class ChangedOpenApi implements ComposedChanged { + private final OpenApiDiffOptions options; @JsonIgnore private OpenAPI oldSpecOpenApi; @JsonIgnore private OpenAPI newSpecOpenApi; private List newEndpoints; @@ -17,6 +21,10 @@ public class ChangedOpenApi implements ComposedChanged { private List changedSchemas; private ChangedExtensions changedExtensions; + public ChangedOpenApi(OpenApiDiffOptions options) { + this.options = options; + } + public List getDeprecatedEndpoints() { return changedOperations.stream() .filter(ChangedOperation::isDeprecated) @@ -40,10 +48,12 @@ public DiffResult isCoreChanged() { if (newEndpoints.isEmpty() && missingEndpoints.isEmpty()) { return DiffResult.NO_CHANGES; } - if (missingEndpoints.isEmpty()) { - return DiffResult.COMPATIBLE; + if (!missingEndpoints.isEmpty()) { + if (OPENAPI_ENDPOINTS_DECREASED.enabled(options.getConfig())) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public OpenAPI getOldSpecOpenApi() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OpenApiBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OpenApiBCTest.java index 0cb493aa2..2d3e53065 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OpenApiBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OpenApiBCTest.java @@ -1,10 +1,12 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.OPENAPI_ENDPOINTS_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class OpenApiBCTest { private final String BASE = "bc_openapi_base.yaml"; @@ -21,6 +23,7 @@ public void changedButCompatible() { @Test public void endpointsDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_openapi_endpoints_decreased.yaml"); + BackwardIncompatibleProp prop = OPENAPI_ENDPOINTS_DECREASED; + assertSpecIncompatible(BASE, "bc_openapi_endpoints_decreased.yaml", prop); } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java index 5d9b4d79b..73c1e53b0 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java @@ -12,6 +12,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.openapitools.openapidiff.core.output.ConsoleRender; @@ -20,7 +21,7 @@ class FileUtilsTest { @BeforeEach void setup() { - changedOpenApi = new ChangedOpenApi(); + changedOpenApi = new ChangedOpenApi(OpenApiDiffOptions.builder().build()); changedOpenApi.setChangedSchemas(Collections.emptyList()); changedOpenApi.setChangedOperations(Collections.emptyList()); changedOpenApi.setNewEndpoints(Collections.emptyList()); From 2af9762b52a5c7b195f266ef3b981f718de465d0 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Fri, 21 Jul 2023 13:46:49 -0600 Subject: [PATCH 18/26] Configurable incompatible checks - Parameters --- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../core/model/ChangedParameters.java | 17 +++++++++++++---- .../core/backcompat/ParametersBCTest.java | 11 ++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index dc12836b0..c247c40c2 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -15,6 +15,8 @@ public enum BackwardIncompatibleProp { REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), REQUEST_NUMERIC_RANGE_DECREASED("incompatible.request.numeric.range.decreased", true), REQUEST_ONEOF_DECREASED("incompatible.request.oneof.decreased", true), + REQUEST_PARAMS_DECREASED("incompatible.request.params.decreased", true), + REQUEST_PARAMS_REQUIRED_INCREASED("incompatible.request.params.required.increased", true), REQUEST_READONLY_INCREASED("incompatible.request.readonly.increased", true), REQUEST_READONLY_REQUIRED_DECREASED("incompatible.request.readonly.required.decreased", true), REQUEST_REQUIRED_INCREASED("incompatible.request.required.increased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedParameters.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedParameters.java index 2b119bf80..86dc145fd 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedParameters.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedParameters.java @@ -1,5 +1,8 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_PARAMS_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_PARAMS_REQUIRED_INCREASED; + import io.swagger.v3.oas.models.parameters.Parameter; import java.util.ArrayList; import java.util.List; @@ -33,11 +36,17 @@ public DiffResult isCoreChanged() { if (increased.isEmpty() && missing.isEmpty()) { return DiffResult.NO_CHANGES; } - if (increased.stream().noneMatch(p -> p.getRequired() != null && p.getRequired()) - && missing.isEmpty()) { - return DiffResult.COMPATIBLE; + if (!missing.isEmpty()) { + if (REQUEST_PARAMS_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (increased.stream().anyMatch(p -> p.getRequired() != null && p.getRequired())) { + if (REQUEST_PARAMS_REQUIRED_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public List getOldParameterList() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ParametersBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ParametersBCTest.java index 1b913115d..5f2755bbc 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ParametersBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ParametersBCTest.java @@ -1,10 +1,13 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_PARAMS_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_PARAMS_REQUIRED_INCREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class ParametersBCTest { private final String BASE = "bc_request_params_base.yaml"; @@ -21,11 +24,13 @@ public void changedButCompatible() { @Test public void decreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_params_decreased.yaml"); + BackwardIncompatibleProp prop = REQUEST_PARAMS_DECREASED; + assertSpecIncompatible(BASE, "bc_request_params_decreased.yaml", prop); } @Test public void requiredIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_params_required_increased.yaml"); + BackwardIncompatibleProp prop = REQUEST_PARAMS_REQUIRED_INCREASED; + assertSpecIncompatible(BASE, "bc_request_params_required_increased.yaml", prop); } } From a38b6d45ba727786c568779a82d38feeabb088bb Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Fri, 21 Jul 2023 13:57:13 -0600 Subject: [PATCH 19/26] Configurable incompatible checks - Parameter --- .../core/model/BackwardIncompatibleProp.java | 3 ++ .../core/model/ChangedParameter.java | 28 +++++++++++++++---- .../core/backcompat/ParameterBCTest.java | 19 +++++++++---- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index c247c40c2..0c8aa9d3b 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -15,6 +15,9 @@ public enum BackwardIncompatibleProp { REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), REQUEST_NUMERIC_RANGE_DECREASED("incompatible.request.numeric.range.decreased", true), REQUEST_ONEOF_DECREASED("incompatible.request.oneof.decreased", true), + REQUEST_PARAM_ALLOWEMPTY_DECREASED("incompatible.request.param.allowempty.decreased", true), + REQUEST_PARAM_EXPLODE_CHANGED("incompatible.request.param.explode.changed", true), + REQUEST_PARAM_STYLE_CHANGED("incompatible.request.param.style.changed", true), REQUEST_PARAMS_DECREASED("incompatible.request.params.decreased", true), REQUEST_PARAMS_REQUIRED_INCREASED("incompatible.request.params.required.increased", true), REQUEST_READONLY_INCREASED("incompatible.request.readonly.increased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedParameter.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedParameter.java index 998570186..ff1ccfb48 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedParameter.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedParameter.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.*; + import io.swagger.v3.oas.models.parameters.Parameter; import java.util.Arrays; import java.util.List; @@ -41,13 +43,27 @@ public DiffResult isCoreChanged() { && !changeExplode) { return DiffResult.NO_CHANGES; } - if ((!changeRequired || Boolean.TRUE.equals(oldParameter.getRequired())) - && (!changeAllowEmptyValue || Boolean.TRUE.equals(newParameter.getAllowEmptyValue())) - && !changeStyle - && !changeExplode) { - return DiffResult.COMPATIBLE; + if (changeAllowEmptyValue && !Boolean.TRUE.equals(newParameter.getAllowEmptyValue())) { + if (REQUEST_PARAM_ALLOWEMPTY_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (changeExplode) { + if (REQUEST_PARAM_EXPLODE_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (changeRequired && !Boolean.TRUE.equals(oldParameter.getRequired())) { + if (REQUEST_PARAMS_REQUIRED_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (changeStyle) { + if (REQUEST_PARAM_STYLE_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public DiffContext getContext() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ParameterBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ParameterBCTest.java index 4f1d30852..edcd40073 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ParameterBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/ParameterBCTest.java @@ -1,10 +1,15 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_PARAMS_REQUIRED_INCREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_PARAM_ALLOWEMPTY_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_PARAM_EXPLODE_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_PARAM_STYLE_CHANGED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class ParameterBCTest { private final String BASE = "bc_request_param_base.yaml"; @@ -21,21 +26,25 @@ public void changedButCompatible() { @Test public void allowEmptyValueDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_param_allowemptyvalue_decreased.yaml"); + BackwardIncompatibleProp prop = REQUEST_PARAM_ALLOWEMPTY_DECREASED; + assertSpecIncompatible(BASE, "bc_request_param_allowemptyvalue_decreased.yaml", prop); } @Test public void explodeChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_param_explode_changed.yaml"); + BackwardIncompatibleProp prop = REQUEST_PARAM_EXPLODE_CHANGED; + assertSpecIncompatible(BASE, "bc_request_param_explode_changed.yaml", prop); } @Test public void requiredIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_param_required_increased.yaml"); + BackwardIncompatibleProp prop = REQUEST_PARAMS_REQUIRED_INCREASED; + assertSpecIncompatible(BASE, "bc_request_param_required_increased.yaml", prop); } @Test public void styleChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_param_style_changed.yaml"); + BackwardIncompatibleProp prop = REQUEST_PARAM_STYLE_CHANGED; + assertSpecIncompatible(BASE, "bc_request_param_style_changed.yaml", prop); } } From d5b61047ce727738c4deb2c7a08d37bdce9b5c75 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Fri, 21 Jul 2023 14:25:27 -0600 Subject: [PATCH 20/26] Configurable incompatible checks - Paths - Note: Using existing OPENAPI_ENDPOINTS_DECREASED property instead of creating a new one since decreasing paths is detected as a decrease in endpoints anyway. --- .../openapidiff/core/compare/PathsDiff.java | 2 +- .../openapidiff/core/model/ChangedPaths.java | 18 ++++++++++++++---- .../core/backcompat/PathsBCTest.java | 7 +++++-- 3 files changed, 20 insertions(+), 7 deletions(-) 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 feb61df10..49cbff9c4 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 @@ -40,7 +40,7 @@ public DeferredChanged diff( final Map left, final Map right) { DeferredBuilder builder = new DeferredBuilder<>(); - ChangedPaths changedPaths = new ChangedPaths(left, right); + ChangedPaths changedPaths = new ChangedPaths(left, right, openApiDiff.getOptions()); changedPaths.getIncreased().putAll(right); left.keySet() diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedPaths.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedPaths.java index 96af52785..5e032b498 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedPaths.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedPaths.java @@ -1,16 +1,24 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.OPENAPI_ENDPOINTS_DECREASED; + import io.swagger.v3.oas.models.PathItem; import java.util.*; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; public class ChangedPaths implements ComposedChanged { + private final OpenApiDiffOptions options; private final Map oldPathMap; private final Map newPathMap; private Map increased; private Map missing; private Map changed; - public ChangedPaths(Map oldPathMap, Map newPathMap) { + public ChangedPaths( + Map oldPathMap, + Map newPathMap, + OpenApiDiffOptions options) { + this.options = options; this.oldPathMap = oldPathMap; this.newPathMap = newPathMap; this.increased = new LinkedHashMap<>(); @@ -28,10 +36,12 @@ public DiffResult isCoreChanged() { if (increased.isEmpty() && missing.isEmpty()) { return DiffResult.NO_CHANGES; } - if (missing.isEmpty()) { - return DiffResult.COMPATIBLE; + if (!missing.isEmpty()) { + if (OPENAPI_ENDPOINTS_DECREASED.enabled(options.getConfig())) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public Map getOldPathMap() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/PathsBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/PathsBCTest.java index 495739d18..d865ee44a 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/PathsBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/PathsBCTest.java @@ -1,10 +1,12 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.OPENAPI_ENDPOINTS_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class PathsBCTest { private final String BASE = "bc_paths_base.yaml"; @@ -21,6 +23,7 @@ public void changedButCompatible() { @Test public void decreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_paths_decreased.yaml"); + BackwardIncompatibleProp prop = OPENAPI_ENDPOINTS_DECREASED; + assertSpecIncompatible(BASE, "bc_paths_decreased.yaml", prop); } } From 8c191950f48a46023153f194b33e1391a2ed8b99 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 25 Jul 2023 10:59:06 -0600 Subject: [PATCH 21/26] Configurable incompatible checks - Path - Note: Using existing OPENAPI_ENDPOINTS_DECREASED property instead of creating a new one since decreasing path operations is detected as a decrease in endpoints anyway. --- .../openapidiff/core/model/ChangedPath.java | 10 +++++++--- .../openapidiff/core/backcompat/PathBCTest.java | 7 +++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedPath.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedPath.java index e9fa4093a..2ba9830e7 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedPath.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedPath.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.OPENAPI_ENDPOINTS_DECREASED; + import io.swagger.v3.oas.models.Operation; import io.swagger.v3.oas.models.PathItem; import java.util.*; @@ -36,10 +38,12 @@ public DiffResult isCoreChanged() { if (increased.isEmpty() && missing.isEmpty()) { return DiffResult.NO_CHANGES; } - if (missing.isEmpty()) { - return DiffResult.COMPATIBLE; + if (!missing.isEmpty()) { + if (OPENAPI_ENDPOINTS_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public String getPathUrl() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/PathBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/PathBCTest.java index b0b9ab39c..c1b2a60f1 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/PathBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/PathBCTest.java @@ -1,10 +1,12 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.OPENAPI_ENDPOINTS_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class PathBCTest { private final String BASE = "bc_path_base.yaml"; @@ -21,6 +23,7 @@ public void changedButCompatible() { @Test public void opsDecreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_path_ops_decreased.yaml"); + BackwardIncompatibleProp prop = OPENAPI_ENDPOINTS_DECREASED; + assertSpecIncompatible(BASE, "bc_path_ops_decreased.yaml", prop); } } From 9b28c665bf3355663b9d04903907a502a8c5aca7 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 25 Jul 2023 11:12:14 -0600 Subject: [PATCH 22/26] Configurable incompatible checks - RequestBody --- .../openapidiff/core/model/BackwardIncompatibleProp.java | 1 + .../openapidiff/core/model/ChangedRequestBody.java | 7 ++++++- .../openapidiff/core/backcompat/RequestBodyBCTest.java | 7 +++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 0c8aa9d3b..27a221c0a 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -10,6 +10,7 @@ public enum BackwardIncompatibleProp { EXTENSION_CONTENT_TYPES_DECREASED("incompatible.extension.content.types.decreased", false), EXTENSION_CONTENT_TYPE_DELETED("incompatible.extension.content.type.%s.deleted", false), OPENAPI_ENDPOINTS_DECREASED("incompatible.openapi.endpoints.decreased", true), + REQUEST_BODY_REQUIRED_INCREASED("incompatible.request.body.increased", true), REQUEST_CONTENT_DECREASED("incompatible.request.content.decreased", true), REQUEST_ENUM_DECREASED("incompatible.request.enum.decreased", true), REQUEST_MAX_LENGTH_DECREASED("incompatible.request.max.length.decreased", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedRequestBody.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedRequestBody.java index 83fadd407..48cc4cf0e 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedRequestBody.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedRequestBody.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.OPENAPI_ENDPOINTS_DECREASED; + import io.swagger.v3.oas.models.parameters.RequestBody; import java.util.Arrays; import java.util.List; @@ -31,7 +33,10 @@ public DiffResult isCoreChanged() { if (!changeRequired) { return DiffResult.NO_CHANGES; } - return DiffResult.INCOMPATIBLE; + if (OPENAPI_ENDPOINTS_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + return DiffResult.COMPATIBLE; } public RequestBody getOldRequestBody() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/RequestBodyBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/RequestBodyBCTest.java index b02c0a3e0..5b1000702 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/RequestBodyBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/RequestBodyBCTest.java @@ -1,9 +1,11 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.OPENAPI_ENDPOINTS_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class RequestBodyBCTest { private final String BASE = "bc_request_body_base.yaml"; @@ -15,6 +17,7 @@ public void unchanged() { @Test public void requiredChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_request_body_required_changed.yaml"); + BackwardIncompatibleProp prop = OPENAPI_ENDPOINTS_DECREASED; + assertSpecIncompatible(BASE, "bc_request_body_required_changed.yaml", prop); } } From 4ff68fa07aa02c28f8f65199a9fa209ee6e89994 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 25 Jul 2023 13:17:55 -0600 Subject: [PATCH 23/26] Configurable incompatible checks - SecurityScheme --- .../core/compare/SecuritySchemeDiff.java | 11 +++--- .../core/model/BackwardIncompatibleProp.java | 5 +++ .../core/model/ChangedSecurityScheme.java | 39 ++++++++++++++----- .../core/backcompat/SecuritySchemeBCTest.java | 18 +++++---- 4 files changed, 51 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java index a27edb374..c08055a31 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java @@ -51,8 +51,8 @@ public DeferredChanged diff( changedSecuritySchemeOptional -> { ChangedSecurityScheme changedSecurityScheme = changedSecuritySchemeOptional.orElse( - new ChangedSecurityScheme(leftSecurityScheme, rightSecurityScheme)); - changedSecurityScheme = getCopyWithoutScopes(changedSecurityScheme); + new ChangedSecurityScheme(leftSecurityScheme, rightSecurityScheme, context)); + changedSecurityScheme = getCopyWithoutScopes(changedSecurityScheme, context); if (changedSecurityScheme != null && leftSecurityScheme.getType() == SecurityScheme.Type.OAUTH2) { @@ -71,7 +71,7 @@ protected DeferredChanged computeDiff( SecurityScheme rightSecurityScheme, DiffContext context) { ChangedSecurityScheme changedSecurityScheme = - new ChangedSecurityScheme(leftSecurityScheme, rightSecurityScheme); + new ChangedSecurityScheme(leftSecurityScheme, rightSecurityScheme, context); openApiDiff .getMetadataDiff() @@ -114,9 +114,10 @@ protected DeferredChanged computeDiff( return new RealizedChanged<>(changedSecurityScheme); } - private ChangedSecurityScheme getCopyWithoutScopes(ChangedSecurityScheme original) { + private ChangedSecurityScheme getCopyWithoutScopes( + ChangedSecurityScheme original, DiffContext context) { return new ChangedSecurityScheme( - original.getOldSecurityScheme(), original.getNewSecurityScheme()) + original.getOldSecurityScheme(), original.getNewSecurityScheme(), context) .setChangedType(original.isChangedType()) .setChangedIn(original.isChangedIn()) .setChangedScheme(original.isChangedScheme()) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 27a221c0a..dab795f39 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -37,6 +37,11 @@ public enum BackwardIncompatibleProp { RESPONSE_RESPONSES_DECREASED("incompatible.response.responses.decreased", true), RESPONSE_WRITEONLY_INCREASED("incompatible.response.writeonly.increased", true), RESPONSE_WRITEONLY_REQUIRED_DECREASED("incompatible.response.writeonly.required.decreased", true), + SECURITY_SCHEME_BEARER_FORMAT_CHANGED("incompatible.security.scheme.bearer.format.changed", true), + SECURITY_SCHEME_OPENIDCONNECT_URL_CHANGED( + "incompatible.security.scheme.openidconnect.url.changed", true), + SECURITY_SCHEME_SCHEME_CHANGED("incompatible.security.scheme.scheme.changed", true), + SECURITY_SCHEME_SCOPES_INCREASED("incompatible.security.scheme.scopes.increased", true), SCHEMA_DISCRIMINATOR_CHANGED("incompatible.schema.discriminator.changed", true), SCHEMA_TYPE_CHANGED("incompatible.schema.type.changed", true), ; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java index 2850e0f17..1adb56b6f 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.*; + import io.swagger.v3.oas.models.security.SecurityScheme; import java.util.Arrays; import java.util.List; @@ -8,6 +10,7 @@ public class ChangedSecurityScheme implements ComposedChanged { private SecurityScheme oldSecurityScheme; private SecurityScheme newSecurityScheme; + private final DiffContext context; private boolean changedType; private boolean changedIn; private boolean changedScheme; @@ -18,9 +21,11 @@ public class ChangedSecurityScheme implements ComposedChanged { private ChangedOAuthFlows oAuthFlows; private ChangedExtensions extensions; - public ChangedSecurityScheme(SecurityScheme oldSecurityScheme, SecurityScheme newSecurityScheme) { + public ChangedSecurityScheme( + SecurityScheme oldSecurityScheme, SecurityScheme newSecurityScheme, DiffContext context) { this.oldSecurityScheme = oldSecurityScheme; this.newSecurityScheme = newSecurityScheme; + this.context = context; } @Override @@ -38,22 +43,36 @@ public DiffResult isCoreChanged() { && (changedScopes == null || changedScopes.isUnchanged())) { return DiffResult.NO_CHANGES; } - if (!changedType - && !changedIn - && !changedScheme - && !changedBearerFormat - && !changedOpenIdConnectUrl - && (changedScopes == null || changedScopes.getIncreased().isEmpty())) { + if (changedBearerFormat) { + if (SECURITY_SCHEME_BEARER_FORMAT_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (changedOpenIdConnectUrl) { + if (SECURITY_SCHEME_OPENIDCONNECT_URL_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (changedScheme) { + if (SECURITY_SCHEME_SCHEME_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (changedScopes != null && !changedScopes.getIncreased().isEmpty()) { + if (SECURITY_SCHEME_SCOPES_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (changedIn || changedType) { // TODO: Dead code removal opportunity for changedType and changedIn. It appears that // SecuritySchemaDiff will never be given the chance to detect differences TYPE and // IN differences because that case has already been detected and filtered out by // SecurityRequirementsDiff and recorded as a dropped requirement in // ChangedSecurityRequirements. - - return DiffResult.COMPATIBLE; + return DiffResult.INCOMPATIBLE; } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public SecurityScheme getOldSecurityScheme() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecuritySchemeBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecuritySchemeBCTest.java index b275523dd..d89d824d5 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecuritySchemeBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecuritySchemeBCTest.java @@ -1,10 +1,10 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; -import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; -import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.TestUtils.*; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.*; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class SecuritySchemeBCTest { private final String BASE = "bc_security_scheme_base.yaml"; @@ -21,7 +21,8 @@ public void changedButCompatible() { @Test public void bearerFormatChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_security_scheme_bearer_format_changed.yaml"); + BackwardIncompatibleProp prop = SECURITY_SCHEME_BEARER_FORMAT_CHANGED; + assertSpecIncompatible(BASE, "bc_security_scheme_bearer_format_changed.yaml", prop); } @Test @@ -31,12 +32,14 @@ public void inChanged() { @Test public void openIdConnectUrlChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_security_scheme_open_id_connect_url_changed.yaml"); + BackwardIncompatibleProp prop = SECURITY_SCHEME_OPENIDCONNECT_URL_CHANGED; + assertSpecIncompatible(BASE, "bc_security_scheme_open_id_connect_url_changed.yaml", prop); } @Test public void schemeChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_security_scheme_scheme_changed.yaml"); + BackwardIncompatibleProp prop = SECURITY_SCHEME_SCHEME_CHANGED; + assertSpecIncompatible(BASE, "bc_security_scheme_scheme_changed.yaml", prop); } @Test @@ -46,6 +49,7 @@ public void typeChanged() { @Test public void scopesIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_security_scheme_scopes_increased.yaml"); + BackwardIncompatibleProp prop = SECURITY_SCHEME_SCOPES_INCREASED; + assertSpecIncompatible(BASE, "bc_security_scheme_scopes_increased.yaml", prop); } } From c51495f8e380ee84ab43d65a15b3dd08a54cbba6 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 25 Jul 2023 13:52:18 -0600 Subject: [PATCH 24/26] Configurable incompatible checks - SecurityRequirement --- .../core/compare/SecurityRequirementDiff.java | 2 +- .../core/model/BackwardIncompatibleProp.java | 2 ++ .../core/model/ChangedSecurityRequirement.java | 16 ++++++++++++---- .../backcompat/SecurityRequirementBCTest.java | 5 ++++- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java index 237e7815f..76b51a5d0 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java @@ -66,7 +66,7 @@ public DeferredChanged diff( DeferredBuilder builder = new DeferredBuilder<>(); ChangedSecurityRequirement changedSecurityRequirement = - new ChangedSecurityRequirement(left, right != null ? getCopy(right) : null); + new ChangedSecurityRequirement(left, right != null ? getCopy(right) : null, context); SecurityRequirement leftRequirement = left == null ? new SecurityRequirement() : left; SecurityRequirement rightRequirement = right == null ? new SecurityRequirement() : right; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index dab795f39..8441a3069 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -37,6 +37,8 @@ public enum BackwardIncompatibleProp { RESPONSE_RESPONSES_DECREASED("incompatible.response.responses.decreased", true), RESPONSE_WRITEONLY_INCREASED("incompatible.response.writeonly.increased", true), RESPONSE_WRITEONLY_REQUIRED_DECREASED("incompatible.response.writeonly.required.decreased", true), + SECURITY_REQUIREMENT_SCHEMES_INCREASED( + "incompatible.security.requirement.schemes.increased", true), SECURITY_SCHEME_BEARER_FORMAT_CHANGED("incompatible.security.scheme.bearer.format.changed", true), SECURITY_SCHEME_OPENIDCONNECT_URL_CHANGED( "incompatible.security.scheme.openidconnect.url.changed", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityRequirement.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityRequirement.java index f8e61f7dc..0c5ed1535 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityRequirement.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityRequirement.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_REQUIREMENT_SCHEMES_INCREASED; + import io.swagger.v3.oas.models.security.SecurityRequirement; import java.util.ArrayList; import java.util.List; @@ -8,14 +10,18 @@ public class ChangedSecurityRequirement implements ComposedChanged { private SecurityRequirement oldSecurityRequirement; private SecurityRequirement newSecurityRequirement; + private final DiffContext context; private SecurityRequirement missing; private SecurityRequirement increased; private List changed; public ChangedSecurityRequirement( - SecurityRequirement oldSecurityRequirement, SecurityRequirement newSecurityRequirement) { + SecurityRequirement oldSecurityRequirement, + SecurityRequirement newSecurityRequirement, + DiffContext context) { this.oldSecurityRequirement = oldSecurityRequirement; this.newSecurityRequirement = newSecurityRequirement; + this.context = context; this.changed = new ArrayList<>(); } @@ -29,10 +35,12 @@ public DiffResult isCoreChanged() { if (increased == null && missing == null) { return DiffResult.NO_CHANGES; } - if (increased == null) { - return DiffResult.COMPATIBLE; + if (increased != null) { + if (SECURITY_REQUIREMENT_SCHEMES_INCREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public void addMissing(String key, List scopes) { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecurityRequirementBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecurityRequirementBCTest.java index 80c79cec9..73a3ca227 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecurityRequirementBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecurityRequirementBCTest.java @@ -1,8 +1,10 @@ package org.openapitools.openapidiff.core.backcompat; import static org.openapitools.openapidiff.core.TestUtils.*; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_REQUIREMENT_SCHEMES_INCREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class SecurityRequirementBCTest { private final String BASE = "bc_security_requirement_base.yaml"; @@ -19,6 +21,7 @@ public void changedButCompatible() { @Test public void schemesIncreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_security_requirement_schemes_increased.yaml"); + BackwardIncompatibleProp prop = SECURITY_REQUIREMENT_SCHEMES_INCREASED; + assertSpecIncompatible(BASE, "bc_security_requirement_schemes_increased.yaml", prop); } } From 60c3f1ac4c466fda00d096dfa3128951731403a4 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Tue, 25 Jul 2023 13:56:32 -0600 Subject: [PATCH 25/26] Configurable incompatible checks - SecurityRequirements --- .../core/compare/SecurityRequirementsDiff.java | 2 +- .../core/model/BackwardIncompatibleProp.java | 1 + .../core/model/ChangedSecurityRequirements.java | 15 +++++++++++---- .../backcompat/SecurityRequirementsBCTest.java | 9 +++++---- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementsDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementsDiff.java index 291dd5c65..324bf0017 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementsDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementsDiff.java @@ -88,7 +88,7 @@ protected DeferredChanged diff( right = right == null ? new ArrayList<>() : getCopy(right); ChangedSecurityRequirements changedSecurityRequirements = - new ChangedSecurityRequirements(left, right); + new ChangedSecurityRequirements(left, right, context); for (SecurityRequirement leftSecurity : left) { Optional rightSecOpt = contains(right, leftSecurity); diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 8441a3069..731f3629a 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -39,6 +39,7 @@ public enum BackwardIncompatibleProp { RESPONSE_WRITEONLY_REQUIRED_DECREASED("incompatible.response.writeonly.required.decreased", true), SECURITY_REQUIREMENT_SCHEMES_INCREASED( "incompatible.security.requirement.schemes.increased", true), + SECURITY_REQUIREMENTS_DECREASED("incompatible.security.requirements.decreased", true), SECURITY_SCHEME_BEARER_FORMAT_CHANGED("incompatible.security.scheme.bearer.format.changed", true), SECURITY_SCHEME_OPENIDCONNECT_URL_CHANGED( "incompatible.security.scheme.openidconnect.url.changed", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityRequirements.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityRequirements.java index d8f813801..3a8f90a34 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityRequirements.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityRequirements.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_REQUIREMENTS_DECREASED; + import io.swagger.v3.oas.models.security.SecurityRequirement; import java.util.ArrayList; import java.util.List; @@ -9,15 +11,18 @@ public class ChangedSecurityRequirements implements ComposedChanged { private List oldSecurityRequirements; private List newSecurityRequirements; + private final DiffContext context; private List missing; private List increased; private List changed; public ChangedSecurityRequirements( List oldSecurityRequirements, - List newSecurityRequirements) { + List newSecurityRequirements, + DiffContext context) { this.oldSecurityRequirements = oldSecurityRequirements; this.newSecurityRequirements = newSecurityRequirements; + this.context = context; this.changed = new ArrayList<>(); } @@ -31,10 +36,12 @@ public DiffResult isCoreChanged() { if (CollectionUtils.isEmpty(missing) && CollectionUtils.isEmpty(increased)) { return DiffResult.NO_CHANGES; } - if (CollectionUtils.isEmpty(missing)) { - return DiffResult.COMPATIBLE; + if (CollectionUtils.isNotEmpty(missing)) { + if (SECURITY_REQUIREMENTS_DECREASED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.INCOMPATIBLE; + return DiffResult.COMPATIBLE; } public void addMissing(SecurityRequirement securityRequirement) { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecurityRequirementsBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecurityRequirementsBCTest.java index a9df11df0..931b35bce 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecurityRequirementsBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecurityRequirementsBCTest.java @@ -1,10 +1,10 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; -import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible; -import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.TestUtils.*; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_REQUIREMENTS_DECREASED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class SecurityRequirementsBCTest { private final String BASE = "bc_security_requirements_base.yaml"; @@ -26,7 +26,8 @@ public void changedButCompatible() { // requirements should be compatible. @Test public void decreased() { - assertOpenApiBackwardIncompatible(BASE, "bc_security_requirements_decreased.yaml"); + BackwardIncompatibleProp prop = SECURITY_REQUIREMENTS_DECREASED; + assertSpecIncompatible(BASE, "bc_security_requirements_decreased.yaml", prop); } // TODO: A missing incompatible check seems to be if requirements increase from zero to 1 or more From bec808b9147aede16a2552867d4f8b02f8ce4779 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Wed, 26 Jul 2023 16:36:19 -0600 Subject: [PATCH 26/26] Configurable incompatible checks - OauthFlow --- .../core/compare/OAuthFlowDiff.java | 5 ++-- .../core/compare/OAuthFlowsDiff.java | 11 +++---- .../core/compare/SecuritySchemeDiff.java | 2 +- .../core/model/BackwardIncompatibleProp.java | 6 ++++ .../core/model/ChangedOAuthFlow.java | 29 ++++++++++++++++--- .../core/backcompat/OAuthFlowBCTest.java | 15 +++++++--- 6 files changed, 52 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/OAuthFlowDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/OAuthFlowDiff.java index 48e5e0447..8dd404f5b 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/OAuthFlowDiff.java +++ b/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 getExtensions(OAuthFlow oAuthFlow) { return ofNullable(oAuthFlow).map(OAuthFlow::getExtensions).orElse(null); } - public Optional diff(OAuthFlow left, OAuthFlow right) { - ChangedOAuthFlow changedOAuthFlow = new ChangedOAuthFlow(left, right); + public Optional diff(OAuthFlow left, OAuthFlow right, DiffContext context) { + ChangedOAuthFlow changedOAuthFlow = new ChangedOAuthFlow(left, right, context); if (left != null && right != null) { changedOAuthFlow .setAuthorizationUrl( diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/OAuthFlowsDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/OAuthFlowsDiff.java index 98ceb509f..2b1136618 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/OAuthFlowsDiff.java +++ b/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 getExtensions(OAuthFlows oAuthFlow) { return ofNullable(oAuthFlow).map(OAuthFlows::getExtensions).orElse(null); } - public Optional diff(OAuthFlows left, OAuthFlows right) { + public Optional 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 diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java index c08055a31..f68d638d3 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java @@ -87,7 +87,7 @@ protected DeferredChanged computeDiff( case OAUTH2: openApiDiff .getOAuthFlowsDiff() - .diff(leftSecurityScheme.getFlows(), rightSecurityScheme.getFlows()) + .diff(leftSecurityScheme.getFlows(), rightSecurityScheme.getFlows(), context) .ifPresent(changedSecurityScheme::setOAuthFlows); break; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 731f3629a..f05a72f75 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -41,6 +41,12 @@ public enum BackwardIncompatibleProp { "incompatible.security.requirement.schemes.increased", true), SECURITY_REQUIREMENTS_DECREASED("incompatible.security.requirements.decreased", true), SECURITY_SCHEME_BEARER_FORMAT_CHANGED("incompatible.security.scheme.bearer.format.changed", true), + SECURITY_SCHEME_OAUTH2_AUTH_URL_CHANGED( + "incompatible.security.scheme.oauth2.auth.url.changed", true), + SECURITY_SCHEME_OAUTH2_REFRESH_URL_CHANGED( + "incompatible.security.scheme.oauth2.refresh.url.changed", true), + SECURITY_SCHEME_OAUTH2_TOKEN_URL_CHANGED( + "incompatible.security.scheme.oauth2.token.url.changed", true), SECURITY_SCHEME_OPENIDCONNECT_URL_CHANGED( "incompatible.security.scheme.openidconnect.url.changed", true), SECURITY_SCHEME_SCHEME_CHANGED("incompatible.security.scheme.scheme.changed", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOAuthFlow.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOAuthFlow.java index 5409d1d99..b6179e9be 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOAuthFlow.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOAuthFlow.java @@ -1,5 +1,9 @@ package org.openapitools.openapidiff.core.model; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_OAUTH2_AUTH_URL_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_OAUTH2_REFRESH_URL_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_OAUTH2_TOKEN_URL_CHANGED; + import io.swagger.v3.oas.models.security.OAuthFlow; import java.util.Collections; import java.util.List; @@ -8,14 +12,16 @@ public class ChangedOAuthFlow implements ComposedChanged { private OAuthFlow oldOAuthFlow; private OAuthFlow newOAuthFlow; + private final DiffContext context; private boolean authorizationUrl; private boolean tokenUrl; private boolean refreshUrl; private ChangedExtensions extensions; - public ChangedOAuthFlow(OAuthFlow oldOAuthFlow, OAuthFlow newOAuthFlow) { + public ChangedOAuthFlow(OAuthFlow oldOAuthFlow, OAuthFlow newOAuthFlow, DiffContext context) { this.oldOAuthFlow = oldOAuthFlow; this.newOAuthFlow = newOAuthFlow; + this.context = context; } @Override @@ -25,10 +31,25 @@ public List getChangedElements() { @Override public DiffResult isCoreChanged() { - if (authorizationUrl || tokenUrl || refreshUrl) { - return DiffResult.INCOMPATIBLE; + if (!authorizationUrl && !tokenUrl && !refreshUrl) { + return DiffResult.NO_CHANGES; + } + if (authorizationUrl) { + if (SECURITY_SCHEME_OAUTH2_AUTH_URL_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (refreshUrl) { + if (SECURITY_SCHEME_OAUTH2_REFRESH_URL_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } + if (tokenUrl) { + if (SECURITY_SCHEME_OAUTH2_TOKEN_URL_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } } - return DiffResult.NO_CHANGES; + return DiffResult.COMPATIBLE; } public OAuthFlow getOldOAuthFlow() { diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OAuthFlowBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OAuthFlowBCTest.java index 62cf3b9a0..476546504 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OAuthFlowBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/OAuthFlowBCTest.java @@ -1,9 +1,13 @@ package org.openapitools.openapidiff.core.backcompat; -import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; +import static org.openapitools.openapidiff.core.TestUtils.assertSpecIncompatible; import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_OAUTH2_AUTH_URL_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_OAUTH2_REFRESH_URL_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_OAUTH2_TOKEN_URL_CHANGED; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.BackwardIncompatibleProp; public class OAuthFlowBCTest { private final String BASE = "bc_oauthflow_base.yaml"; @@ -15,16 +19,19 @@ public void unchanged() { @Test public void authorizationUrlChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_oauthflow_authorization_url_changed.yaml"); + BackwardIncompatibleProp prop = SECURITY_SCHEME_OAUTH2_AUTH_URL_CHANGED; + assertSpecIncompatible(BASE, "bc_oauthflow_authorization_url_changed.yaml", prop); } @Test public void refreshUrlChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_oauthflow_refresh_url_changed.yaml"); + BackwardIncompatibleProp prop = SECURITY_SCHEME_OAUTH2_REFRESH_URL_CHANGED; + assertSpecIncompatible(BASE, "bc_oauthflow_refresh_url_changed.yaml", prop); } @Test public void tokenUrlChanged() { - assertOpenApiBackwardIncompatible(BASE, "bc_oauthflow_token_url_changed.yaml"); + BackwardIncompatibleProp prop = SECURITY_SCHEME_OAUTH2_TOKEN_URL_CHANGED; + assertSpecIncompatible(BASE, "bc_oauthflow_token_url_changed.yaml", prop); } }