From df57483682196bc5e123a2adf277bb8b5065f075 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Thu, 18 Jan 2024 13:29:48 +0100 Subject: [PATCH 1/4] [bugfix] Handle null validator when one of multiple specs is bad --- .../validation/core/OpenApiInteractionValidatorFactory.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java index 3cb05293..67e8317d 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java @@ -56,7 +56,9 @@ private MultipleSpecOpenApiInteractionValidatorWrapper buildMultipleSpecOpenApiI } var validator = buildSingleSpecOpenApiInteractionValidatorWrapper(specOptional.get(), configuration.getLevelResolverLevels(), configuration.getLevelResolverDefaultLevel()); - return Pair.of(entry.pathPattern(), (OpenApiInteractionValidatorWrapper) validator); + return validator != null + ? Pair.of(entry.pathPattern(), (OpenApiInteractionValidatorWrapper) validator) + : null; }) .filter(Objects::nonNull) .collect(Collectors.toList()); From b44757622c16a503c2f3c3217883f81e605c1265 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Thu, 18 Jan 2024 13:30:12 +0100 Subject: [PATCH 2/4] improve logging --- .../openapi/validation/core/DefaultViolationLogger.java | 2 +- .../validation/core/OpenApiInteractionValidatorFactory.java | 4 ++-- .../openapi/validation/core/OpenApiRequestValidator.java | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) 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 b7fb62a1..e1445c5f 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 @@ -25,7 +25,7 @@ public void log(OpenApiViolation violation) { default -> { /* do nothing */ } } } catch (IOException e) { - log.error("Could not add to LoggingContext", e); + log.error("[OpenAPI Validation] Could not add to LoggingContext", e); } } diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java index 67e8317d..6844c226 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java @@ -51,7 +51,7 @@ private MultipleSpecOpenApiInteractionValidatorWrapper buildMultipleSpecOpenApiI var path = entry.specificationFilePath(); var specOptional = loadSpecFromPath(path).or(() -> loadSpecFromResources(path)); if (specOptional.isEmpty()) { - log.error("OpenAPI spec file {} could not be found", path); + log.error("[OpenAPI Validation] Spec file {} could not be found", path); return null; } var validator = buildSingleSpecOpenApiInteractionValidatorWrapper(specOptional.get(), @@ -86,7 +86,7 @@ private SingleSpecOpenApiInteractionValidatorWrapper buildSingleSpecOpenApiInter .build(); return new SingleSpecOpenApiInteractionValidatorWrapper(validator); } catch (Throwable e) { - log.error("Could not initialize OpenApiInteractionValidator [validation disabled]", e); + log.error("[OpenAPI Validation] Could not initialize OpenApiInteractionValidator [validation disabled]", e); return null; } } diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java index 02e0aa08..1991fda4 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java @@ -94,7 +94,7 @@ public List validateRequestObject( var result = validator.validateRequest(simpleRequest); return mapper.map(result, request, response, Direction.REQUEST, requestBody); } catch (Exception e) { - log.error("Could not validate request", e); + log.error("[OpenAPI Validation] Could not validate request", e); return List.of(); } } @@ -138,7 +138,7 @@ public List validateResponseObject( ); return mapper.map(result, request, response, Direction.RESPONSE, responseBody); } catch (Exception e) { - log.error("Could not validate response", e); + log.error("[OpenAPI Validation] Could not validate response", e); return List.of(); } } From bae63339aaca5850d0b062110216ce890f0db363 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Thu, 18 Jan 2024 13:43:11 +0100 Subject: [PATCH 3/4] move handling to MultipleSpecOpenApiInteractionValidatorWrapper --- .../OpenApiInteractionValidatorFactory.java | 5 ++--- ...pecOpenApiInteractionValidatorWrapper.java | 2 +- ...penApiInteractionValidatorWrapperTest.java | 21 +++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java index 6844c226..5eb16ade 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiInteractionValidatorFactory.java @@ -56,9 +56,7 @@ private MultipleSpecOpenApiInteractionValidatorWrapper buildMultipleSpecOpenApiI } var validator = buildSingleSpecOpenApiInteractionValidatorWrapper(specOptional.get(), configuration.getLevelResolverLevels(), configuration.getLevelResolverDefaultLevel()); - return validator != null - ? Pair.of(entry.pathPattern(), (OpenApiInteractionValidatorWrapper) validator) - : null; + return Pair.of(entry.pathPattern(), (OpenApiInteractionValidatorWrapper) validator); }) .filter(Objects::nonNull) .collect(Collectors.toList()); @@ -71,6 +69,7 @@ private MultipleSpecOpenApiInteractionValidatorWrapper buildMultipleSpecOpenApiI return new MultipleSpecOpenApiInteractionValidatorWrapper(validators); } + @Nullable private SingleSpecOpenApiInteractionValidatorWrapper buildSingleSpecOpenApiInteractionValidatorWrapper( String spec, Map levelResolverLevels, diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapper.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapper.java index 8ba1a493..1b81ee18 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapper.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapper.java @@ -40,7 +40,7 @@ public ValidationReport validateResponse(String path, Request.Method method, Sim private Optional getValidatorForPath(String path) { for (var validator : validators) { if (validator.getLeft().matcher(path).matches()) { - return Optional.of(validator.getRight()); + return validator.getRight() == null ? Optional.of(validator.getRight()) : Optional.empty(); } } diff --git a/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapperTest.java b/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapperTest.java index df4de8bb..f54f4f02 100644 --- a/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapperTest.java +++ b/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapperTest.java @@ -56,6 +56,27 @@ public void testReturnsViolationWhenNoMatchingValidatorFound() { assertEquals("ValidatorConfiguration has no spec file matching path: /123", message.getMessage()); } + @Test + public void testReturnsViolationWhenMatchingValidatorIsNull() { + var catchAllValidator = mockValidator(); + + validator = new MultipleSpecOpenApiInteractionValidatorWrapper( + List.of( + Pair.of(Pattern.compile("/test/.*"), null), + Pair.of(Pattern.compile(".*"), catchAllValidator.validator()) + ) + ); + + var path = "/test/123"; + var report = validator.validateRequest(new SimpleRequest.Builder("GET", path).build()); + + var messages = report.getMessages(); + assertEquals(1, messages.size()); + var message = messages.get(0); + assertEquals(MESSAGE_KEY_NO_VALIDATOR_FOUND, message.getKey()); + assertEquals("ValidatorConfiguration has no spec file matching path: /test/123", message.getMessage()); + } + private static MockValidatorResult mockValidator() { var catchAllValidator = mock(OpenApiInteractionValidatorWrapper.class); var catchAllValidationReport = mock(ValidationReport.class); From 31c39ea168204b89f742bf244e4de5a117db7784 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Thu, 18 Jan 2024 13:46:25 +0100 Subject: [PATCH 4/4] small logic fix --- .../MultipleSpecOpenApiInteractionValidatorWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapper.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapper.java index 1b81ee18..ab6369ba 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapper.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapper.java @@ -40,7 +40,7 @@ public ValidationReport validateResponse(String path, Request.Method method, Sim private Optional getValidatorForPath(String path) { for (var validator : validators) { if (validator.getLeft().matcher(path).matches()) { - return validator.getRight() == null ? Optional.of(validator.getRight()) : Optional.empty(); + return validator.getRight() != null ? Optional.of(validator.getRight()) : Optional.empty(); } }