Allow excluding certain violations from being reported (#4)

pboos · web-flow · commit 08afebca52c7 · 2023-05-24T13:23:11.000+02:00
diff --git a/README.md b/README.md
@@ -201,6 +201,23 @@ public class ValidatorConfiguration {
 }
 ```
 
+### Exclude certain violations
+Certain violations can get excluded from reporting. This can be achieved as demonstrated in the following snippet.
+
+**Note:** Only use this where it is really needed. It is best practice to fix the actual violations by either adjusting
+the specification, the server implementation, or the client sending wrong requests.
+
+```java
+@Component
+public class ViolationExclusionsExample implements ViolationExclusions {
+  @Override
+  public boolean isExcluded(OpenApiViolation violation) {
+    return violation.getDirection().equals(Direction.REQUEST)
+            && violation.getMessage().equals("[Path '/name'] Instance type (integer) does not match any allowed primitive type (allowed: [\"string\"])");
+  }
+}
+```
+
 ## Examples
 Run examples with `./gradlew :examples:example-spring-boot-starter-web:bootRun` or `./gradlew :examples:example-spring-boot-starter-webflux:bootRun`.
 
diff --git a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/exclusions/NoViolationExclusions.java b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/exclusions/NoViolationExclusions.java
@@ -0,0 +1,10 @@
+package com.getyourguide.openapi.validation.api.exclusions;
+
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
+
+public class NoViolationExclusions implements ViolationExclusions {
+    @Override
+    public boolean isExcluded(OpenApiViolation violation) {
+        return false;
+    }
+}
diff --git a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/exclusions/ViolationExclusions.java b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/exclusions/ViolationExclusions.java
@@ -0,0 +1,7 @@
+package com.getyourguide.openapi.validation.api.exclusions;
+
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
+
+public interface ViolationExclusions {
+    boolean isExcluded(OpenApiViolation violation);
+}
diff --git a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/model/OpenApiViolation.java b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/model/OpenApiViolation.java
@@ -16,6 +16,8 @@ public class OpenApiViolation {
     private final Optional<String> operationId;
     private final Optional<String> normalizedPath;
     private final Optional<String> instance;
+    private final Optional<String> schema;
     private final Optional<Integer> responseStatus;
     private final String message;
+    private final String logMessage;
 }
diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/DefaultViolationLogger.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/DefaultViolationLogger.java
@@ -19,11 +19,11 @@ public class DefaultViolationLogger implements ViolationLogger {
     public void log(OpenApiViolation violation) {
         try (var ignored = loggerExtension.addToLoggingContext(buildLoggingContext(violation))) {
             switch (violation.getLevel()) {
-                case INFO -> log.info(violation.getMessage());
-                case WARN -> log.warn(violation.getMessage());
-                case ERROR -> log.error(violation.getMessage());
+                case INFO -> log.info(violation.getLogMessage());
+                case WARN -> log.warn(violation.getLogMessage());
+                case ERROR -> log.error(violation.getLogMessage());
                 // keeping ignored as debug for now
-                case IGNORE -> log.debug(violation.getMessage());
+                case IGNORE -> log.debug(violation.getLogMessage());
                 default -> { /* do nothing */ }
             }
         } catch (IOException e) {
diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/ValidationReportHandler.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/ValidationReportHandler.java
@@ -1,6 +1,7 @@
 package com.getyourguide.openapi.validation.core;
 
 import com.atlassian.oai.validator.report.ValidationReport;
+import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions;
 import com.getyourguide.openapi.validation.api.log.LogLevel;
 import com.getyourguide.openapi.validation.api.log.ViolationLogger;
 import com.getyourguide.openapi.validation.api.metrics.MetricTag;
@@ -15,14 +16,13 @@
 import lombok.AllArgsConstructor;
 import lombok.Builder;
 import lombok.Getter;
-import lombok.extern.slf4j.Slf4j;
 
-@Slf4j
 @AllArgsConstructor
 public class ValidationReportHandler {
     private final ValidationReportThrottler throttleHelper;
     private final ViolationLogger logger;
     private final MetricsReporter metrics;
+    private final ViolationExclusions violationExclusions;
     private final Configuration configuration;
 
     public void handleValidationReport(
@@ -34,19 +34,14 @@ public void handleValidationReport(
         if (!result.getMessages().isEmpty()) {
             result
                 .getMessages()
-                .stream().filter(message -> !isExcludedMessage(message))
-                .forEach(message -> throttleHelper.throttle(message, direction,
-                    () -> logValidationError(message, request, body, direction)));
+                .stream()
+                .map(message -> buildOpenApiViolation(message, request, body, direction))
+                .filter(violation -> !isViolationExcluded(violation))
+                .forEach(violation -> throttleHelper.throttle(violation, () -> logValidationError(violation)));
         }
     }
 
-    private void logValidationError(
-        ValidationReport.Message message,
-        RequestMetaData request,
-        String body,
-        Direction direction
-    ) {
-        var openApiViolation = buildOpenApiViolation(message, request, body, direction);
+    private void logValidationError(OpenApiViolation openApiViolation) {
         logger.log(openApiViolation);
         metrics.increment(configuration.getMetricName(), createTags(openApiViolation));
     }
@@ -78,17 +73,20 @@ private OpenApiViolation buildOpenApiViolation(
             .operationId(getOperationId(message))
             .normalizedPath(getNormalizedPath(message))
             .instance(getPointersInstance(message))
+            .schema(getPointersSchema(message))
             .responseStatus(getResponseStatus(message))
-            .message(logMessage)
+            .logMessage(logMessage)
+            .message(message.getMessage())
             .build();
     }
 
-    private boolean isExcludedMessage(ValidationReport.Message message) {
+    private boolean isViolationExcluded(OpenApiViolation openApiViolation) {
         return
-            // JSON parse errors should be ignored as it can't be compared to the schema then (this also prevents logging personal data!)
-            message.getMessage().startsWith("Unable to parse JSON")
+            violationExclusions.isExcluded(openApiViolation)
+                // JSON parse errors should be ignored as it can't be compared to the schema then (this also prevents logging personal data!)
+                || openApiViolation.getMessage().startsWith("Unable to parse JSON")
                 // If it matches more than 1, then we don't want to log a validation error
-                || message.getMessage().matches(
+                || openApiViolation.getMessage().matches(
                 ".*\\[Path '[^']+'] Instance failed to match exactly one schema \\(matched [1-9][0-9]* out of \\d\\).*");
     }
 
@@ -114,6 +112,12 @@ private static Optional<String> getPointersInstance(ValidationReport.Message mes
             .map(ValidationReport.MessageContext.Pointers::getInstance);
     }
 
+    private static Optional<String> getPointersSchema(ValidationReport.Message message) {
+        return message.getContext()
+            .flatMap(ValidationReport.MessageContext::getPointers)
+            .map(ValidationReport.MessageContext.Pointers::getSchema);
+    }
+
     private static Optional<String> getOperationId(ValidationReport.Message message) {
         return message.getContext()
             .flatMap(ValidationReport.MessageContext::getApiOperation)
diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/throttle/RequestBasedValidationReportThrottler.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/throttle/RequestBasedValidationReportThrottler.java
@@ -1,7 +1,6 @@
 package com.getyourguide.openapi.validation.core.throttle;
 
-import com.atlassian.oai.validator.report.ValidationReport;
-import com.getyourguide.openapi.validation.api.model.Direction;
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import lombok.AllArgsConstructor;
@@ -16,21 +15,21 @@ public class RequestBasedValidationReportThrottler implements ValidationReportTh
     private final Map<String, DateTime> loggedMessages = new ConcurrentHashMap<>();
 
     @Override
-    public void throttle(ValidationReport.Message message, Direction direction, Runnable runnable) {
-        if (isThrottled(message, direction)) {
+    public void throttle(OpenApiViolation openApiViolation, Runnable runnable) {
+        if (isThrottled(openApiViolation)) {
             return;
         }
 
         runnable.run();
-        registerLoggedMessage(message, direction);
+        registerLoggedMessage(openApiViolation);
     }
 
-    private void registerLoggedMessage(ValidationReport.Message message, Direction direction) {
-        loggedMessages.put(buildKey(message, direction), DateTime.now());
+    private void registerLoggedMessage(OpenApiViolation openApiViolation) {
+        loggedMessages.put(buildKey(openApiViolation), DateTime.now());
     }
 
-    private boolean isThrottled(ValidationReport.Message message, Direction direction) {
-        var key = buildKey(message, direction);
+    private boolean isThrottled(OpenApiViolation openApiViolation) {
+        var key = buildKey(openApiViolation);
         var lastLoggedTime = loggedMessages.get(key);
         if (lastLoggedTime == null) {
             return false;
@@ -39,18 +38,13 @@ private boolean isThrottled(ValidationReport.Message message, Direction directio
     }
 
     @NonNull
-    private String buildKey(ValidationReport.Message message, Direction direction) {
-        var keyBuilder = new StringBuilder(direction.toString() + ":");
-        message.getContext().ifPresentOrElse(
-            messageContext -> {
-                var method = messageContext.getRequestMethod().map(Enum::name).orElse("N/A");
-                var apiOperationPathNormalized = messageContext.getApiOperation().map(apiOperation -> apiOperation.getApiPath().normalised()).orElse("N/A");
-                var responseStatus = messageContext.getResponseStatus().map(Object::toString).orElse("N/A");
-                var schema = messageContext.getPointers().map(ValidationReport.MessageContext.Pointers::getSchema).orElse("N/A");
-                keyBuilder.append(String.format("%s:%s:%s:%s", method, apiOperationPathNormalized, responseStatus, schema));
-            },
-            () -> keyBuilder.append("N/A")
-        );
+    private String buildKey(OpenApiViolation openApiViolation) {
+        var keyBuilder = new StringBuilder(openApiViolation.getDirection().toString() + ":");
+        var method = openApiViolation.getRequestMetaData().getMethod();
+        var apiOperationPathNormalized = openApiViolation.getNormalizedPath().orElse("N/A");
+        var responseStatus = openApiViolation.getResponseStatus().map(Object::toString).orElse("N/A");
+        var schema = openApiViolation.getSchema().orElse("N/A");
+        keyBuilder.append(String.format("%s:%s:%s:%s", method, apiOperationPathNormalized, responseStatus, schema));
         return keyBuilder.toString();
     }
 }
diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/throttle/ValidationReportThrottler.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/throttle/ValidationReportThrottler.java
@@ -1,9 +1,8 @@
 package com.getyourguide.openapi.validation.core.throttle;
 
-import com.atlassian.oai.validator.report.ValidationReport;
-import com.getyourguide.openapi.validation.api.model.Direction;
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
 
 public interface ValidationReportThrottler {
 
-    void throttle(ValidationReport.Message message, Direction direction, Runnable runnable);
+    void throttle(OpenApiViolation openApiViolation, Runnable runnable);
 }
diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/throttle/ValidationReportThrottlerNone.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/throttle/ValidationReportThrottlerNone.java
@@ -1,11 +1,10 @@
 package com.getyourguide.openapi.validation.core.throttle;
 
-import com.atlassian.oai.validator.report.ValidationReport;
-import com.getyourguide.openapi.validation.api.model.Direction;
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
 
 public class ValidationReportThrottlerNone implements ValidationReportThrottler {
     @Override
-    public void throttle(ValidationReport.Message message, Direction direction, Runnable runnable) {
+    public void throttle(OpenApiViolation openApiViolation, Runnable runnable) {
         runnable.run();
     }
 }
diff --git a/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/throttle/RequestBasedValidationReportThrottlerTest.java b/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/throttle/RequestBasedValidationReportThrottlerTest.java
@@ -1,14 +1,13 @@
 package com.getyourguide.openapi.validation.core.throttle;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
-import com.atlassian.oai.validator.model.ApiOperation;
-import com.atlassian.oai.validator.model.ApiPath;
 import com.atlassian.oai.validator.model.Request;
-import com.atlassian.oai.validator.report.ValidationReport;
 import com.getyourguide.openapi.validation.api.model.Direction;
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
+import com.getyourguide.openapi.validation.api.model.RequestMetaData;
+import java.net.URI;
+import java.util.Collections;
 import java.util.Optional;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -27,85 +26,73 @@ public void beforeEach() {
 
     @Test
     public void testNotThrottledIfNoEntry() {
-        var message = mockMessage(Request.Method.GET, "/path", 200);
+        var violation = buildViolation(DIRECTION, Request.Method.GET, "/path", 200);
 
-        assertThrottled(false, message, DIRECTION);
+        assertThrottled(false, violation);
     }
 
     @Test
     public void testThrottledIfEntryExists() {
-        var message = mockMessage(Request.Method.GET, "/path", 200);
-        throttler.throttle(message, DIRECTION, NO_OP_RUNNABLE);
+        var violation = buildViolation(DIRECTION, Request.Method.GET, "/path", 200);
+        throttler.throttle(violation, NO_OP_RUNNABLE);
 
-        assertThrottled(true, message, DIRECTION);
+        assertThrottled(true, violation);
     }
 
     @Test
     public void testNotThrottledIfSmallDifference() {
-        var message = mockMessage(Request.Method.GET, "/path", 200);
-        throttler.throttle(message, DIRECTION, NO_OP_RUNNABLE);
+        var violation = buildViolation(DIRECTION, Request.Method.GET, "/path", 200);
+        throttler.throttle(violation, NO_OP_RUNNABLE);
 
-        assertThrottled(false, mockMessage(Request.Method.GET, "/path", 200), Direction.RESPONSE);
-        assertThrottled(false, mockMessage(Request.Method.POST, "/path", 200), DIRECTION);
-        assertThrottled(false, mockMessage(Request.Method.GET, "/other-path", 200), DIRECTION);
-        assertThrottled(false, mockMessage(Request.Method.GET, "/path", 402), DIRECTION);
+        assertThrottled(false, buildViolation(Direction.RESPONSE, Request.Method.GET, "/path", 200));
+        assertThrottled(false, buildViolation(DIRECTION, Request.Method.POST, "/path", 200));
+        assertThrottled(false, buildViolation(DIRECTION, Request.Method.GET, "/other-path", 200));
+        assertThrottled(false, buildViolation(DIRECTION, Request.Method.GET, "/path", 402));
     }
 
     @Test
     public void testThrottledIfInstanceContainsArrayIndex() {
-        var message = mockMessage(Request.Method.GET, "/path", 200, "/items/1/name", "/properties/items/items/properties/name");
-        throttler.throttle(message, DIRECTION, NO_OP_RUNNABLE);
+        var violation = buildViolation(DIRECTION, Request.Method.GET, "/path", 200, "/items/1/name", "/properties/items/items/properties/name");
+        throttler.throttle(violation, NO_OP_RUNNABLE);
 
         assertThrottled(
             true,
-            mockMessage(Request.Method.GET, "/path", 200, "/items/2/name", "/properties/items/items/properties/name"),
-            DIRECTION
+            buildViolation(DIRECTION, Request.Method.GET, "/path", 200, "/items/2/name", "/properties/items/items/properties/name")
         );
         assertThrottled(
             true,
-            mockMessage(Request.Method.GET, "/path", 200, "/items/3/name", "/properties/items/items/properties/name"),
-            DIRECTION
+            buildViolation(DIRECTION, Request.Method.GET, "/path", 200, "/items/3/name", "/properties/items/items/properties/name")
         );
         assertThrottled(
             false,
-            mockMessage(Request.Method.GET, "/path", 200, "/items/4/description", "/properties/items/items/properties/description"),
-            DIRECTION
+            buildViolation(DIRECTION, Request.Method.GET, "/path", 200, "/items/4/description", "/properties/items/items/properties/description")
         );
     }
 
-    private void assertThrottled(boolean expectThrottled, ValidationReport.Message message, Direction direction) {
+    private void assertThrottled(boolean expectThrottled, OpenApiViolation openApiViolation) {
         var ref = new Object() {
             boolean wasThrottled = true;
         };
 
-        throttler.throttle(message, direction, () -> ref.wasThrottled = false);
+        throttler.throttle(openApiViolation, () -> ref.wasThrottled = false);
 
         assertEquals(expectThrottled, ref.wasThrottled);
     }
 
-    private ValidationReport.Message mockMessage(Request.Method method, String path, int status) {
-        return mockMessage(method, path, status, "/items/1/name", "/properties/items/items/properties/name");
+    private OpenApiViolation buildViolation(Direction direction, Request.Method method, String path, int status) {
+        return buildViolation(direction, method, path, status, "/items/1/name", "/properties/items/items/properties/name");
     }
 
-    private ValidationReport.Message mockMessage(Request.Method method, String path, int status, String instance, String schema) {
-        var message = mock(ValidationReport.Message.class);
-        var context = mock(ValidationReport.MessageContext.class);
-
-        when(context.getRequestMethod()).thenReturn(Optional.of(method));
-
-        var apiOperation = mock(ApiOperation.class);
-        var apiPath = mock(ApiPath.class);
-        when(apiPath.normalised()).thenReturn(path);
-        when(apiOperation.getApiPath()).thenReturn(apiPath);
-        when(context.getApiOperation()).thenReturn(Optional.of(apiOperation));
-        when(context.getResponseStatus()).thenReturn(Optional.of(status));
-
-        var pointers = mock(ValidationReport.MessageContext.Pointers.class);
-        when(pointers.getInstance()).thenReturn(instance);
-        when(pointers.getSchema()).thenReturn(schema);
-        when(context.getPointers()).thenReturn(Optional.of(pointers));
-
-        when(message.getContext()).thenReturn(Optional.of(context));
-        return message;
+    private OpenApiViolation buildViolation(Direction direction, Request.Method method, String path, int status, String instance, String schema) {
+        return OpenApiViolation.builder()
+            .direction(direction)
+            .requestMetaData(
+                new RequestMetaData(method.toString(), URI.create("https://example.com" + path), Collections.emptyMap())
+            )
+            .responseStatus(Optional.of(status))
+            .normalizedPath(Optional.of(path))
+            .instance(Optional.of(instance))
+            .schema(Optional.of(schema))
+            .build();
     }
 }
diff --git a/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/LibraryAutoConfiguration.java b/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/LibraryAutoConfiguration.java

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,8 @@ public class OpenApiViolation {`
`16`	`16`	`private final Optional<String> operationId;`
`17`	`17`	`private final Optional<String> normalizedPath;`
`18`	`18`	`private final Optional<String> instance;`
	`19`	`+ private final Optional<String> schema;`
`19`	`20`	`private final Optional<Integer> responseStatus;`
`20`	`21`	`private final String message;`
	`22`	`+ private final String logMessage;`
`21`	`23`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,8 @@`
`1`	`1`	`package com.getyourguide.openapi.validation.core.throttle;`
`2`	`2`
`3`		`-import com.atlassian.oai.validator.report.ValidationReport;`
`4`		`-import com.getyourguide.openapi.validation.api.model.Direction;`
	`3`	`+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;`
`5`	`4`
`6`	`5`	`public interface ValidationReportThrottler {`
`7`	`6`
`8`		`- void throttle(ValidationReport.Message message, Direction direction, Runnable runnable);`
	`7`	`+ void throttle(OpenApiViolation openApiViolation, Runnable runnable);`
`9`	`8`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,10 @@`
`1`	`1`	`package com.getyourguide.openapi.validation.core.throttle;`
`2`	`2`
`3`		`-import com.atlassian.oai.validator.report.ValidationReport;`
`4`		`-import com.getyourguide.openapi.validation.api.model.Direction;`
	`3`	`+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;`
`5`	`4`
`6`	`5`	`public class ValidationReportThrottlerNone implements ValidationReportThrottler {`
`7`	`6`	`@Override`
`8`		`- public void throttle(ValidationReport.Message message, Direction direction, Runnable runnable) {`
	`7`	`+ public void throttle(OpenApiViolation openApiViolation, Runnable runnable) {`
`9`	`8`	`runnable.run();`
`10`	`9`	`}`
`11`	`10`	`}`