Skip to content

Commit

Permalink
Allow excluding certain violations from being reported (#4)
Browse files Browse the repository at this point in the history
  • Loading branch information
pboos committed May 24, 2023
1 parent 2b0a876 commit 08afebc
Show file tree
Hide file tree
Showing 11 changed files with 124 additions and 100 deletions.
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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(
Expand All @@ -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));
}
Expand Down Expand Up @@ -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\\).*");
}

Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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();
}
}
Loading

0 comments on commit 08afebc

Please sign in to comment.