#17 Add ability to allow removal of deprecated APIs: added parameter …

…to switch deprecated API handling

README.md

@@ -54,6 +54,18 @@ Multiple reporters can be set up at the same time by separating the types with c
 --output-format=JSON,HTML
 ```
 
+## API deprecation handling
+The [OpenAPI specification](https://swagger.io/specification/#fixed-fields-8) 
+defines the `deprecated` attribute on operation level for marking an API deprecated. 
+Swagger Brake utilizes this property so that it's possible to delete a deprecated API 
+without marking the API broken.
+
+The default behavior is that deprecated APIs can be deleted without any issue but there is
+a possibility to override this behavior so that the new API will be considered as broken.
+
+The `--deprecated-api-deletion-allowed` parameter is responsible for setting this behavior
+in Swagger Brake. The default value is `true` but it can be overridden to `false` anytime. 
+
 ## Latest artifact resolution
 For easier CI integration, there is a possibility not to provide the old API path directly 
 but to resolve the latest artifact containing the Swagger definition file from any Maven2
@@ -80,7 +92,7 @@ It's also possible that the repository is secured with username and password. Th
 - `--maven-repo-password`
 
 #### Implementation details
-The mechanism under the hood is to resolve the latest artifact based on the maven-metadata.xml
+The mechanism under the hood is to resolve the latest artifact based on the `maven-metadata.xml`
 for a given groupId and artifactId. After the latest version has been determined, it will be 
 downloaded to the temp directory. The downloaded JAR will be scanned for any of the following 
 files which will be used for providing the old API:

swagger-brake-cli/src/main/java/io/redskap/swagger/brake/cli/options/CliHelpProvider.java

@@ -31,7 +31,7 @@ public String getHelp() {
     }
 
     private String formatPadCliOption(String option) {
-        return StringUtils.rightPad(formatCliOption(option), 20);
+        return StringUtils.rightPad(formatCliOption(option), 40);
     }
 
     private String formatCliOption(String option) {

swagger-brake-cli/src/main/java/io/redskap/swagger/brake/cli/options/CliOptions.java

@@ -13,6 +13,8 @@ public abstract class CliOptions {
     public static final String ARTIFACT_ID = "artifactId";
     public static final String GROUP_ID = "groupId";
 
+    public static final String DEPRECATED_API_DELETION_ALLOWED = "deprecated-api-deletion-allowed";
+
     public static String getAsCliOption(String option) {
         return "--" + option;
     }

swagger-brake-cli/src/main/java/io/redskap/swagger/brake/cli/options/handler/DeprecatedApiDeletionAllowedHandler.java

@@ -0,0 +1,27 @@
+package io.redskap.swagger.brake.cli.options.handler;
+
+import io.redskap.swagger.brake.cli.options.CliOptions;
+import io.redskap.swagger.brake.runner.Options;
+import org.apache.commons.lang3.BooleanUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.springframework.stereotype.Component;
+
+@Component
+public class DeprecatedApiDeletionAllowedHandler implements CliOptionHandler {
+    @Override
+    public void handle(String propertyValue, Options options) {
+        if (StringUtils.isNotBlank(propertyValue)) {
+            options.setDeprecatedApiDeletionAllowed(BooleanUtils.toBooleanObject(propertyValue));
+        }
+    }
+
+    @Override
+    public String getHandledPropertyName() {
+        return CliOptions.DEPRECATED_API_DELETION_ALLOWED;
+    }
+
+    @Override
+    public String getHelpMessage() {
+        return "Whether to allow the deletion of deprecated APIs. Defaults to true.";
+    }
+}

swagger-brake-cli/src/test/java/io/redskap/swagger/brake/cli/options/handler/DeprecatedApiDeletionAllowedHandlerTest.java

@@ -0,0 +1,65 @@
+package io.redskap.swagger.brake.cli.options.handler;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.redskap.swagger.brake.runner.Options;
+import org.junit.Test;
+
+public class DeprecatedApiDeletionAllowedHandlerTest {
+    private DeprecatedApiDeletionAllowedHandler underTest = new DeprecatedApiDeletionAllowedHandler();
+
+    @Test
+    public void testHandleShouldGiveTrueValueForTheOptionWhenNullValueGiven() {
+        // given
+        Options options = new Options();
+
+        // when
+        underTest.handle(null, options);
+        // then
+        assertThat(options.getDeprecatedApiDeletionAllowed()).isNull();
+    }
+
+    @Test
+    public void testHandleShouldGiveTrueValueForTheOptionWhenEmptyValueGiven() {
+        // given
+        Options options = new Options();
+
+        // when
+        underTest.handle("", options);
+        // then
+        assertThat(options.getDeprecatedApiDeletionAllowed()).isNull();
+    }
+
+    @Test
+    public void testHandleShouldGiveTrueValueForTheOptionWhenTrueValueGiven() {
+        // given
+        Options options = new Options();
+
+        // when
+        underTest.handle("true", options);
+        // then
+        assertThat(options.getDeprecatedApiDeletionAllowed()).isTrue();
+    }
+
+    @Test
+    public void testHandleShouldGiveTrueValueForTheOptionWhenRandomValueGiven() {
+        // given
+        Options options = new Options();
+
+        // when
+        underTest.handle("asd", options);
+        // then
+        assertThat(options.getDeprecatedApiDeletionAllowed()).isNull();
+    }
+
+    @Test
+    public void testHandleShouldGiveFalseValueForTheOptionWhenFalseValueGiven() {
+        // given
+        Options options = new Options();
+
+        // when
+        underTest.handle("false", options);
+        // then
+        assertThat(options.getDeprecatedApiDeletionAllowed()).isFalse();
+    }
+}

swagger-brake/src/main/java/io/redskap/swagger/brake/core/BreakChecker.java

@@ -5,5 +5,5 @@
 import io.redskap.swagger.brake.core.model.Specification;
 
 public interface BreakChecker {
-    Collection<BreakingChange> check(Specification oldApi, Specification newApi);
+    Collection<BreakingChange> check(Specification oldApi, Specification newApi, CheckerOptions checkerOptions);
 }

swagger-brake/src/main/java/io/redskap/swagger/brake/core/CheckerOptions.java

@@ -0,0 +1,8 @@
+package io.redskap.swagger.brake.core;
+
+import lombok.Data;
+
+@Data
+public class CheckerOptions {
+    private boolean deprecatedApiDeletionAllowed = true;
+}

swagger-brake/src/main/java/io/redskap/swagger/brake/core/CheckerOptionsProvider.java

@@ -0,0 +1,24 @@
+package io.redskap.swagger.brake.core;
+
+import org.springframework.stereotype.Component;
+
+@Component
+public class CheckerOptionsProvider {
+    private CheckerOptions checkerOptions;
+
+    void set(CheckerOptions options) {
+        if (options == null) {
+            throw new IllegalArgumentException("options cannot be null");
+        }
+        checkerOptions = options;
+    }
+
+
+    public CheckerOptions get() {
+        if (checkerOptions == null) {
+            throw new RuntimeException("CheckerOptions has not been set up yet.");
+        }
+        return checkerOptions;
+    }
+
+}

swagger-brake/src/main/java/io/redskap/swagger/brake/core/DefaultBreakChecker.java

@@ -16,9 +16,10 @@
 @Slf4j
 public class DefaultBreakChecker implements BreakChecker {
     private final Collection<BreakingChangeRule<? extends BreakingChange>> rules;
+    private final CheckerOptionsProvider checkerOptionsProvider;
 
     @Override
-    public Collection<BreakingChange> check(Specification oldApi, Specification newApi) {
+    public Collection<BreakingChange> check(Specification oldApi, Specification newApi, CheckerOptions checkerOptions) {
         if (log.isDebugEnabled()) {
             rules.stream().map(BreakingChangeRule::getClass).map(Class::getName).forEach(name -> log.debug("Rule configured: {}", name));
         }
@@ -28,6 +29,10 @@ public Collection<BreakingChange> check(Specification oldApi, Specification newA
         if (newApi == null) {
             throw new IllegalArgumentException("newApi must be provided");
         }
+        if (checkerOptions == null) {
+            throw new IllegalArgumentException("checkerOptions must be provided");
+        }
+        checkerOptionsProvider.set(checkerOptions);
         return rules.stream().map(rule -> rule.checkRule(oldApi, newApi)).flatMap(Collection::stream).collect(toList());
     }
 }

swagger-brake/src/main/java/io/redskap/swagger/brake/core/rule/path/PathDeletedRule.java

@@ -3,22 +3,29 @@
 import java.util.ArrayList;
 import java.util.Collection;
 
+import io.redskap.swagger.brake.core.CheckerOptions;
+import io.redskap.swagger.brake.core.CheckerOptionsProvider;
 import io.redskap.swagger.brake.core.model.Path;
 import io.redskap.swagger.brake.core.model.Specification;
 import io.redskap.swagger.brake.core.rule.BreakingChangeRule;
+import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.stereotype.Component;
 
 @Component
 @Slf4j
+@RequiredArgsConstructor
 public class PathDeletedRule implements BreakingChangeRule<PathDeletedBreakingChange> {
+    private final CheckerOptionsProvider checkerOptionsProvider;
+
     @Override
     public Collection<PathDeletedBreakingChange> checkRule(Specification oldApi, Specification newApi) {
         log.debug("Checking for path deletions..");
+        CheckerOptions checkerOptions = checkerOptionsProvider.get();
         Collection<PathDeletedBreakingChange> breakingChanges = new ArrayList<>();
         for (Path p : oldApi.getPaths()) {
             if (!newApi.getPath(p).isPresent()) {
-                if (!p.isDeprecated()) {
+                if (!p.isDeprecated() || !checkerOptions.isDeprecatedApiDeletionAllowed()) {
                     log.debug("Path {} is not included in the new API", p);
                     breakingChanges.add(new PathDeletedBreakingChange(p.getPath(), p.getMethod()));
                 } else {

swagger-brake/src/main/java/io/redskap/swagger/brake/runner/Checker.java

@@ -4,6 +4,7 @@
 
 import io.redskap.swagger.brake.core.BreakChecker;
 import io.redskap.swagger.brake.core.BreakingChange;
+import io.redskap.swagger.brake.core.CheckerOptions;
 import io.redskap.swagger.brake.core.model.Specification;
 import io.redskap.swagger.brake.core.model.transformer.Transformer;
 import io.swagger.v3.oas.models.OpenAPI;
@@ -18,9 +19,9 @@ public class Checker {
     private final Transformer<OpenAPI, Specification> transformer;
     private final BreakChecker breakChecker;
 
-    public Collection<BreakingChange> check(OpenAPI oldApi, OpenAPI newApi) {
+    public Collection<BreakingChange> check(OpenAPI oldApi, OpenAPI newApi, CheckerOptions checkerOptions) {
         log.info("Starting the check for breaking API changes");
-        Collection<BreakingChange> breakingChanges = breakChecker.check(transformer.transform(oldApi), transformer.transform(newApi));
+        Collection<BreakingChange> breakingChanges = breakChecker.check(transformer.transform(oldApi), transformer.transform(newApi), checkerOptions);
         log.info("Check is finished");
         return breakingChanges;
     }

swagger-brake/src/main/java/io/redskap/swagger/brake/runner/CheckerOptionsFactory.java

@@ -0,0 +1,18 @@
+package io.redskap.swagger.brake.runner;
+
+import io.redskap.swagger.brake.core.CheckerOptions;
+import org.apache.commons.lang3.BooleanUtils;
+import org.springframework.stereotype.Component;
+
+@Component
+public class CheckerOptionsFactory {
+    public CheckerOptions create(Options options) {
+        CheckerOptions checkerOptions = new CheckerOptions();
+        checkerOptions.setDeprecatedApiDeletionAllowed(isDeprecatedApiDeletionAllowed(options));
+        return checkerOptions;
+    }
+
+    private boolean isDeprecatedApiDeletionAllowed(Options options) {
+        return BooleanUtils.toBooleanDefaultIfNull(options.getDeprecatedApiDeletionAllowed(), true);
+    }
+}

swagger-brake/src/main/java/io/redskap/swagger/brake/runner/Options.java

@@ -18,4 +18,6 @@ public class Options {
     private String artifactId;
     private String mavenRepoUsername;
     private String mavenRepoPassword;
+
+    private Boolean deprecatedApiDeletionAllowed;
 }

swagger-brake/src/main/java/io/redskap/swagger/brake/runner/Runner.java

@@ -3,6 +3,7 @@
 import java.util.Collection;
 
 import io.redskap.swagger.brake.core.BreakingChange;
+import io.redskap.swagger.brake.core.CheckerOptions;
 import io.redskap.swagger.brake.report.ReporterFactory;
 import io.redskap.swagger.brake.runner.download.ArtifactDownloaderHandler;
 import io.redskap.swagger.brake.runner.openapi.OpenApiFactory;
@@ -20,6 +21,7 @@ public class Runner {
     private final ArtifactDownloaderHandler artifactDownloaderHandler;
     private final OpenApiFactory openApiFactory;
     private final Checker checker;
+    private final CheckerOptionsFactory checkerOptionsFactory;
 
     public Collection<BreakingChange> run(Options options) {
         artifactDownloaderHandler.handle(options);
@@ -36,7 +38,8 @@ public Collection<BreakingChange> run(Options options) {
         OpenAPI oldApi = openApiFactory.fromFile(oldApiPath);
         OpenAPI newApi = openApiFactory.fromFile(newApiPath);
         log.info("Successfully loaded APIs");
-        Collection<BreakingChange> breakingChanges = checker.check(oldApi, newApi);
+        CheckerOptions checkerOptions = checkerOptionsFactory.create(options);
+        Collection<BreakingChange> breakingChanges = checker.check(oldApi, newApi, checkerOptions);
         reporterFactory.create(options).report(breakingChanges, options);
         return breakingChanges;
     }

swagger-brake/src/main/java/io/redskap/swagger/brake/runner/Starter.java

@@ -3,6 +3,7 @@
 import java.util.Collection;
 
 import io.redskap.swagger.brake.core.BreakingChange;
+import io.redskap.swagger.brake.core.CheckerOptions;
 import io.redskap.swagger.brake.core.CoreConfiguration;
 import io.redskap.swagger.brake.maven.MavenConfiguration;
 import io.redskap.swagger.brake.report.ReporterConfiguration;
@@ -23,7 +24,21 @@ public static Collection<BreakingChange> start(Options options) {
      * @return a collection of breaking changes. The collection is never null.
      */
     public static Collection<BreakingChange> check(OpenAPI oldApi, OpenAPI newApi) {
-        return createApplicationContext().getBean(Checker.class).check(oldApi, newApi);
+        return check(oldApi, newApi, new CheckerOptions());
+    }
+
+    /**
+     * Checks breaking changes between two {@link OpenAPI} instances. The check configuration also can be provided
+     * using this method.
+     * <br>
+     * For library users who want to use the check functionality only.
+     * @param oldApi the old API
+     * @param newApi the new API
+     * @param checkerOptions the options for the check
+     * @return a collection of breaking changes. The collection is never null.
+     */
+    public static Collection<BreakingChange> check(OpenAPI oldApi, OpenAPI newApi, CheckerOptions checkerOptions) {
+        return createApplicationContext().getBean(Checker.class).check(oldApi, newApi, checkerOptions);
     }
 
     private static AnnotationConfigApplicationContext createApplicationContext() {

swagger-brake/src/test/java/io/redskap/swagger/brake/integration/AbstractSwaggerBrakeIntTest.java

@@ -23,6 +23,10 @@ protected Collection<BreakingChange> execute(String oldApiPath, String newApiPat
         Options options = new Options();
         options.setOldApiPath(oldApiPath);
         options.setNewApiPath(newApiPath);
+        return execute(options);
+    }
+
+    protected Collection<BreakingChange> execute(Options options) {
         options.setOutputFormats(ImmutableSet.of(OutputFormat.STDOUT));
         return underTest.run(options);
     }

swagger-brake/src/test/java/io/redskap/swagger/brake/integration/path/PathDeletionIntTest.java

@@ -9,6 +9,7 @@
 import io.redskap.swagger.brake.core.model.HttpMethod;
 import io.redskap.swagger.brake.core.rule.path.PathDeletedBreakingChange;
 import io.redskap.swagger.brake.integration.AbstractSwaggerBrakeIntTest;
+import io.redskap.swagger.brake.runner.Options;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.test.context.junit4.SpringRunner;
@@ -39,4 +40,22 @@ public void testPathDeletionDoesntTriggerWhenDeprecated() {
         // then
         assertThat(result).isEmpty();
     }
+
+    @Test
+    public void testPathDeletionTriggeredWhenDeprecatedAndDeprecationIsNotAllowed() {
+        // given
+        String oldApiPath = "path/deleted/deprecated/petstore.yaml";
+        String newApiPath = "path/deleted/deprecated/petstore_v2.yaml";
+        Options options = new Options();
+        options.setOldApiPath(oldApiPath);
+        options.setNewApiPath(newApiPath);
+        options.setDeprecatedApiDeletionAllowed(false);
+        PathDeletedBreakingChange bc = new PathDeletedBreakingChange("/pet/findByStatus", HttpMethod.GET);
+        Collection<BreakingChange> expected = Collections.singleton(bc);
+        // when
+        Collection<BreakingChange> result = execute(options);
+        // then
+        assertThat(result).hasSize(1);
+        assertThat(result).hasSameElementsAs(expected);
+    }
 }