From 08afebca52c7a0d097dd42e1319de90ceaf82480 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Wed, 24 May 2023 13:23:11 +0200 Subject: [PATCH] Allow excluding certain violations from being reported (#4) --- README.md | 17 ++++ .../api/exclusions/NoViolationExclusions.java | 10 +++ .../api/exclusions/ViolationExclusions.java | 7 ++ .../api/model/OpenApiViolation.java | 2 + .../core/DefaultViolationLogger.java | 8 +- .../core/ValidationReportHandler.java | 38 +++++---- ...RequestBasedValidationReportThrottler.java | 36 ++++---- .../throttle/ValidationReportThrottler.java | 5 +- .../ValidationReportThrottlerNone.java | 5 +- ...estBasedValidationReportThrottlerTest.java | 83 ++++++++----------- .../LibraryAutoConfiguration.java | 13 ++- 11 files changed, 124 insertions(+), 100 deletions(-) create mode 100644 openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/exclusions/NoViolationExclusions.java create mode 100644 openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/exclusions/ViolationExclusions.java diff --git a/README.md b/README.md index 200d465..ebca60c 100644 --- 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 new file mode 100644 index 0000000..ddb7329 --- /dev/null +++ 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 new file mode 100644 index 0000000..30efecc --- /dev/null +++ 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 index dcfd8c3..37f2f45 100644 --- 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 operationId; private final Optional normalizedPath; private final Optional instance; + private final Optional schema; private final Optional 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 index 1220f38..8b9e1b2 100644 --- 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 index a672f36..51f6d18 100644 --- 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 getPointersInstance(ValidationReport.Message mes .map(ValidationReport.MessageContext.Pointers::getInstance); } + private static Optional getPointersSchema(ValidationReport.Message message) { + return message.getContext() + .flatMap(ValidationReport.MessageContext::getPointers) + .map(ValidationReport.MessageContext.Pointers::getSchema); + } + private static Optional 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 index 2a82eca..eab319d 100644 --- 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 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 index c8543f3..5368701 100644 --- 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 index 86ab645..67d4c52 100644 --- 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 index aeb386d..8708613 100644 --- 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 index dbe0e72..af264cd 100644 --- 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 @@ -1,6 +1,8 @@ package com.getyourguide.openapi.validation.autoconfigure; import com.getyourguide.openapi.validation.OpenApiValidationApplicationProperties; +import com.getyourguide.openapi.validation.api.exclusions.NoViolationExclusions; +import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions; import com.getyourguide.openapi.validation.api.log.LogLevel; import com.getyourguide.openapi.validation.api.log.LoggerExtension; import com.getyourguide.openapi.validation.api.log.NoOpLoggerExtension; @@ -51,15 +53,18 @@ public ViolationLogger violationLogger(Optional loggerExtension public ValidationReportHandler validationReportHandler( ValidationReportThrottler validationReportThrottler, ViolationLogger logger, - Optional metrics + Optional metrics, + Optional violationExclusions ) { - var metricName = - properties.getValidationReportMetricName() != null ? properties.getValidationReportMetricName() : - DEFAULT_METRIC_NAME; + var metricName = properties.getValidationReportMetricName() != null + ? properties.getValidationReportMetricName() + : DEFAULT_METRIC_NAME; + return new ValidationReportHandler( validationReportThrottler, logger, metrics.orElseGet(NoOpMetricsReporter::new), + violationExclusions.orElseGet(NoViolationExclusions::new), ValidationReportHandler.Configuration.builder() .metricName(metricName) .metricAdditionalTags(properties.getValidationReportMetricAdditionalTags())