diff --git a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/Rules.java b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/Rules.java new file mode 100644 index 00000000..b9aa85ee --- /dev/null +++ b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/Rules.java @@ -0,0 +1,13 @@ +package com.getyourguide.openapi.validation.api; + +public class Rules { + public static class Request { + public static final String PATH_MISSING = "validation.request.path.missing"; + public static final String OPERATION_NOT_ALLOWED = "validation.request.operation.notAllowed"; + public static final String BODY_SCHEMA_ONE_OF = "validation.request.body.schema.oneOf"; + } + + public static class Response { + public static final String BODY_SCHEMA_ONE_OF = "validation.response.body.schema.oneOf"; + } +} 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 f69c5b60..febd4f6a 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 @@ -13,12 +13,36 @@ public class OpenApiViolation { private final RequestMetaData requestMetaData; private final String body; private final String rule; - private final Optional operationId; - private final Optional normalizedPath; - private final Optional instance; - private final Optional parameter; - private final Optional schema; - private final Optional responseStatus; + private final String operationId; + private final String normalizedPath; + private final String instance; + private final String parameter; + private final String schema; + private final Integer responseStatus; private final String message; private final String logMessage; + + public Optional getOperationId() { + return Optional.ofNullable(operationId); + } + + public Optional getNormalizedPath() { + return Optional.ofNullable(normalizedPath); + } + + public Optional getInstance() { + return Optional.ofNullable(instance); + } + + public Optional getParameter() { + return Optional.ofNullable(parameter); + } + + public Optional getSchema() { + return Optional.ofNullable(schema); + } + + public Optional getResponseStatus() { + return Optional.ofNullable(responseStatus); + } } 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 defea077..7e514d94 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 @@ -78,12 +78,12 @@ private OpenApiViolation buildOpenApiViolation( .requestMetaData(request) .body(body) .rule(message.getKey()) - .operationId(getOperationId(message)) - .normalizedPath(getNormalizedPath(message)) - .instance(pointersInstance) - .parameter(parameterName) - .schema(getPointersSchema(message)) - .responseStatus(getResponseStatus(response, message)) + .operationId(getOperationId(message).orElse(null)) + .normalizedPath(getNormalizedPath(message).orElse(null)) + .instance(pointersInstance.orElse(null)) + .parameter(parameterName.orElse(null)) + .schema(getPointersSchema(message).orElse(null)) + .responseStatus(getResponseStatus(response, message).orElse(null)) .logMessage(logMessage) .message(message.getMessage()) .build(); diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusions.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusions.java index 5c5f9ae2..3bacb7cb 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusions.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusions.java @@ -1,5 +1,6 @@ package com.getyourguide.openapi.validation.core.exclusions; +import com.getyourguide.openapi.validation.api.Rules; import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions; import com.getyourguide.openapi.validation.api.model.Direction; import com.getyourguide.openapi.validation.api.model.OpenApiViolation; @@ -7,26 +8,28 @@ @AllArgsConstructor public class InternalViolationExclusions { + private final ViolationExclusions customViolationExclusions; public boolean isExcluded(OpenApiViolation violation) { return falsePositive404(violation) || falsePositive400(violation) + || falsePositive405(violation) || customViolationExclusions.isExcluded(violation) || oneOfMatchesMoreThanOneSchema(violation); } private static boolean oneOfMatchesMoreThanOneSchema(OpenApiViolation violation) { return ( - "validation.response.body.schema.oneOf".equals(violation.getRule()) - || "validation.request.body.schema.oneOf".equals(violation.getRule()) + Rules.Response.BODY_SCHEMA_ONE_OF.equals(violation.getRule()) + || Rules.Request.BODY_SCHEMA_ONE_OF.equals(violation.getRule()) ) && violation.getMessage() .matches(".*Instance failed to match exactly one schema \\(matched [1-9][0-9]* out of \\d+\\).*"); } private boolean falsePositive404(OpenApiViolation violation) { - return "validation.request.path.missing".equals(violation.getRule()) + return Rules.Request.PATH_MISSING.equals(violation.getRule()) && ( violation.getDirection() == Direction.REQUEST || (violation.getDirection() == Direction.RESPONSE && violation.getResponseStatus().orElse(0) == 404) @@ -36,4 +39,9 @@ private boolean falsePositive404(OpenApiViolation violation) { private boolean falsePositive400(OpenApiViolation violation) { return violation.getDirection() == Direction.REQUEST && violation.getResponseStatus().orElse(0) == 400; } + + private boolean falsePositive405(OpenApiViolation violation) { + return violation.getResponseStatus().orElse(0) == 405 + && Rules.Request.OPERATION_NOT_ALLOWED.equals(violation.getRule()); + } } diff --git a/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusionsTest.java b/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusionsTest.java index c18526a5..343a1b55 100644 --- a/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusionsTest.java +++ b/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusionsTest.java @@ -9,7 +9,6 @@ import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions; import com.getyourguide.openapi.validation.api.model.Direction; import com.getyourguide.openapi.validation.api.model.OpenApiViolation; -import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -24,7 +23,7 @@ public void setup() { } @Test - public void testWhenViolationThenViolationNotExcluded() { + public void whenViolationThenViolationNotExcluded() { when(customViolationExclusions.isExcluded(any())).thenReturn(false); checkViolationNotExcluded(buildSimpleViolation(Direction.RESPONSE, 404)); @@ -38,20 +37,20 @@ private static OpenApiViolation buildSimpleViolation(Direction direction, Intege return OpenApiViolation.builder() .direction(direction) .rule("validation." + (direction == Direction.REQUEST ? "request" : "response") + ".something") - .responseStatus(responseStatus != null ? Optional.of(responseStatus) : Optional.empty()) + .responseStatus(responseStatus) .message("Some violation message") .build(); } @Test - public void testWhenCustomViolationExclusionThenViolationExcluded() { + public void whenCustomViolationExclusionThenViolationExcluded() { when(customViolationExclusions.isExcluded(any())).thenReturn(true); checkViolationExcluded(OpenApiViolation.builder().build()); } @Test - public void testWhenInstanceFailedToMatchExactlyOneThenViolationExcluded() { + public void whenInstanceFailedToMatchExactlyOneThenViolationExcluded() { when(customViolationExclusions.isExcluded(any())).thenReturn(false); checkViolationExcluded(OpenApiViolation.builder() @@ -60,7 +59,7 @@ public void testWhenInstanceFailedToMatchExactlyOneThenViolationExcluded() { } @Test - public void testWhenInstanceFailedToMatchExactlyOneWithOneOf24ThenViolationExcluded() { + public void whenInstanceFailedToMatchExactlyOneWithOneOf24ThenViolationExcluded() { when(customViolationExclusions.isExcluded(any())).thenReturn(false); checkViolationExcluded(OpenApiViolation.builder() @@ -70,36 +69,55 @@ public void testWhenInstanceFailedToMatchExactlyOneWithOneOf24ThenViolationExclu } @Test - public void testWhen404ResponseWithApiPathNotSpecifiedThenViolationExcluded() { + public void when404ResponseWithApiPathNotSpecifiedThenViolationExcluded() { when(customViolationExclusions.isExcluded(any())).thenReturn(false); checkViolationExcluded(OpenApiViolation.builder() .direction(Direction.RESPONSE) .rule("validation.request.path.missing") - .responseStatus(Optional.of(404)) + .responseStatus(404) .message("No API path found that matches request '/nothing'") .build()); } @Test - public void testWhenRequestWithApiPathNotSpecifiedThenViolationExcluded() { + public void whenRequestWithApiPathNotSpecifiedThenViolationExcluded() { when(customViolationExclusions.isExcluded(any())).thenReturn(false); checkViolationExcluded(OpenApiViolation.builder() .direction(Direction.REQUEST) .rule("validation.request.path.missing") - .responseStatus(Optional.empty()) + .responseStatus(null) .message("No API path found that matches request '/nothing'") .build()); } @Test - public void testWhenRequestViolationsAnd400ThenViolationExcluded() { + public void whenRequestViolationsAnd400ThenViolationExcluded() { when(customViolationExclusions.isExcluded(any())).thenReturn(false); checkViolationExcluded(OpenApiViolation.builder() .direction(Direction.REQUEST) - .responseStatus(Optional.of(400)) + .responseStatus(400) + .message("") + .build()); + } + + @Test + public void when405ResponseCodeWithOperationNotAllowedViolationThenViolationExcluded() { + when(customViolationExclusions.isExcluded(any())).thenReturn(false); + + checkViolationExcluded(OpenApiViolation.builder() + .direction(Direction.REQUEST) + .rule("validation.request.operation.notAllowed") + .responseStatus(405) + .message("") + .build()); + + checkViolationExcluded(OpenApiViolation.builder() + .direction(Direction.RESPONSE) + .rule("validation.request.operation.notAllowed") + .responseStatus(405) .message("") .build()); } 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 87086134..651d7a3a 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 @@ -8,7 +8,6 @@ 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; @@ -89,10 +88,10 @@ private OpenApiViolation buildViolation(Direction direction, Request.Method meth .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)) + .responseStatus(status) + .normalizedPath(path) + .instance(instance) + .schema(schema) .build(); } }