Skip to content

Commit

Permalink
refactor: pass in OpenApiViolationHandler to interceptor/webfilter
Browse files Browse the repository at this point in the history
  • Loading branch information
pboos committed Nov 27, 2023
1 parent 8289cd9 commit b21da21
Show file tree
Hide file tree
Showing 16 changed files with 185 additions and 152 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.getyourguide.openapi.validation.api.log;

import com.getyourguide.openapi.validation.api.model.OpenApiViolation;

public interface OpenApiViolationHandler {
void onOpenApiViolation(OpenApiViolation violation);
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@
public class OpenApiRequestValidationConfiguration {
private double sampleRate;
private int validationReportThrottleWaitSeconds;
private boolean shouldFailOnRequestViolation;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
import com.atlassian.oai.validator.model.Request;
import com.atlassian.oai.validator.model.SimpleRequest;
import com.atlassian.oai.validator.model.SimpleResponse;
import com.atlassian.oai.validator.report.ValidationReport;
import com.getyourguide.openapi.validation.api.log.OpenApiViolationHandler;
import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
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 com.getyourguide.openapi.validation.api.model.ResponseMetaData;
import com.getyourguide.openapi.validation.api.model.ValidationResult;
import com.getyourguide.openapi.validation.core.mapper.ValidationReportToOpenApiViolationsMapper;
import com.getyourguide.openapi.validation.core.validator.OpenApiInteractionValidatorWrapper;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ThreadPoolExecutor;
import javax.annotation.Nullable;
Expand All @@ -22,20 +24,18 @@
public class OpenApiRequestValidator {
private final ThreadPoolExecutor threadPoolExecutor;
private final OpenApiInteractionValidatorWrapper validator;
private final ValidationReportHandler validationReportHandler;
private final OpenApiRequestValidationConfiguration configuration;
private final ValidationReportToOpenApiViolationsMapper mapper;

public OpenApiRequestValidator(
ThreadPoolExecutor threadPoolExecutor,
ValidationReportHandler validationReportHandler,
MetricsReporter metricsReporter,
OpenApiInteractionValidatorWrapper validator,
ValidationReportToOpenApiViolationsMapper mapper,
OpenApiRequestValidationConfiguration configuration
) {
this.threadPoolExecutor = threadPoolExecutor;
this.validator = validator;
this.validationReportHandler = validationReportHandler;
this.configuration = configuration;
this.mapper = mapper;

metricsReporter.reportStartup(
validator != null,
Expand All @@ -48,12 +48,28 @@ public boolean isReady() {
return validator != null;
}

public void validateRequestObjectAsync(final RequestMetaData request, @Nullable ResponseMetaData response, String requestBody) {
executeAsync(() -> validateRequestObject(request, response, requestBody));
public void validateRequestObjectAsync(
final RequestMetaData request,
@Nullable ResponseMetaData response,
String requestBody,
OpenApiViolationHandler listener
) {
executeAsync(() -> {
var violations = validateRequestObject(request, response, requestBody);
violations.forEach(listener::onOpenApiViolation);
});
}

public void validateResponseObjectAsync(final RequestMetaData request, ResponseMetaData response, final String responseBody) {
executeAsync(() -> validateResponseObject(request, response, responseBody));
public void validateResponseObjectAsync(
final RequestMetaData request,
ResponseMetaData response,
final String responseBody,
OpenApiViolationHandler listener
) {
executeAsync(() -> {
var violations = validateResponseObject(request, response, responseBody);
violations.forEach(listener::onOpenApiViolation);
});
}

private void executeAsync(Runnable command) {
Expand All @@ -64,28 +80,22 @@ private void executeAsync(Runnable command) {
}
}

public ValidationResult validateRequestObject(final RequestMetaData request, String requestBody) {
public List<OpenApiViolation> validateRequestObject(final RequestMetaData request, String requestBody) {
return validateRequestObject(request, null, requestBody);
}

public ValidationResult validateRequestObject(
public List<OpenApiViolation> validateRequestObject(
final RequestMetaData request,
@Nullable final ResponseMetaData response,
String requestBody
) {
try {
var simpleRequest = buildSimpleRequest(request, requestBody);
var result = validator.validateRequest(simpleRequest);
// TODO this should not be done here, but currently the only way to do it -> Refactor this so that logging
// is actually done in the interceptor/filter where logging can easily be skipped then.
if (!configuration.isShouldFailOnRequestViolation()) {
validationReportHandler
.handleValidationReport(request, response, Direction.REQUEST, requestBody, result);
}
return buildValidationResult(result);
return mapper.map(result, request, response, Direction.REQUEST, requestBody);
} catch (Exception e) {
log.error("Could not validate request", e);
return ValidationResult.NOT_APPLICABLE;
return List.of();
}
}

Expand All @@ -108,7 +118,7 @@ private static String nullSafeUrlDecode(String value) {
return URLDecoder.decode(value, StandardCharsets.UTF_8);
}

public ValidationResult validateResponseObject(
public List<OpenApiViolation> validateResponseObject(
final RequestMetaData request,
final ResponseMetaData response,
final String responseBody
Expand All @@ -126,23 +136,10 @@ public ValidationResult validateResponseObject(
Request.Method.valueOf(request.getMethod().toUpperCase()),
responseBuilder.build()
);
validationReportHandler.handleValidationReport(request, response, Direction.RESPONSE, responseBody, result);
return buildValidationResult(result);
return mapper.map(result, request, response, Direction.RESPONSE, responseBody);
} catch (Exception e) {
log.error("Could not validate response", e);
return ValidationResult.NOT_APPLICABLE;
return List.of();
}
}

private ValidationResult buildValidationResult(ValidationReport validationReport) {
if (validationReport == null) {
return ValidationResult.NOT_APPLICABLE;
}

if (validationReport.getMessages().isEmpty()) {
return ValidationResult.VALID;
}

return ValidationResult.INVALID;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.getyourguide.openapi.validation.core.log;

import com.getyourguide.openapi.validation.api.log.OpenApiViolationHandler;
import com.getyourguide.openapi.validation.api.log.ViolationLogger;
import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
import com.getyourguide.openapi.validation.core.exclusions.InternalViolationExclusions;
import com.getyourguide.openapi.validation.core.throttle.ValidationReportThrottler;
import lombok.AllArgsConstructor;

@AllArgsConstructor
public class DefaultOpenApiViolationHandler implements OpenApiViolationHandler {
private final ValidationReportThrottler throttleHelper;
private final ViolationLogger logger;
private final MetricsReporter metrics;
private final InternalViolationExclusions violationExclusions;

@Override
public void onOpenApiViolation(OpenApiViolation violation) {
if (violationExclusions.isExcluded(violation)) {
return;
}

throttleHelper.throttle(violation, () -> logValidationError(violation));
}

private void logValidationError(OpenApiViolation openApiViolation) {
logger.log(openApiViolation);
metrics.reportViolation(openApiViolation);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.atlassian.oai.validator.model.SimpleRequest;
import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
import com.getyourguide.openapi.validation.api.model.RequestMetaData;
import com.getyourguide.openapi.validation.core.mapper.ValidationReportToOpenApiViolationsMapper;
import com.getyourguide.openapi.validation.core.validator.OpenApiInteractionValidatorWrapper;
import java.net.URI;
import java.util.HashMap;
import java.util.List;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ThreadPoolExecutor;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -29,14 +32,15 @@ public class OpenApiRequestValidatorTest {
public void setup() {
threadPoolExecutor = mock();
validator = mock();
ValidationReportHandler validationReportHandler = mock();
MetricsReporter metricsReporter = mock();
var mapper = mock(ValidationReportToOpenApiViolationsMapper.class);
when(mapper.map(any(), any(), any(), any(), any())).thenReturn(List.of());

openApiRequestValidator = new OpenApiRequestValidator(
threadPoolExecutor,
validationReportHandler,
metricsReporter,
validator,
mapper,
mock()
);
}
Expand All @@ -45,7 +49,7 @@ public void setup() {
public void testWhenThreadPoolExecutorRejectsExecutionThenItShouldNotThrow() {
Mockito.doThrow(new RejectedExecutionException()).when(threadPoolExecutor).execute(any());

openApiRequestValidator.validateRequestObjectAsync(mock(), null, null);
openApiRequestValidator.validateRequestObjectAsync(mock(), null, null, mock());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ public OpenApiRequestValidationConfiguration toOpenApiRequestValidationConfigura
return OpenApiRequestValidationConfiguration.builder()
.sampleRate(getSampleRate())
.validationReportThrottleWaitSeconds(getValidationReportThrottleWaitSeconds())
.shouldFailOnRequestViolation(getShouldFailOnRequestViolation() != null && getShouldFailOnRequestViolation())
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.getyourguide.openapi.validation.api.log.LogLevel;
import com.getyourguide.openapi.validation.api.log.LoggerExtension;
import com.getyourguide.openapi.validation.api.log.NoOpLoggerExtension;
import com.getyourguide.openapi.validation.api.log.OpenApiViolationHandler;
import com.getyourguide.openapi.validation.api.log.ViolationLogger;
import com.getyourguide.openapi.validation.api.metrics.DefaultMetricsReporter;
import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
Expand All @@ -16,8 +17,8 @@
import com.getyourguide.openapi.validation.core.DefaultViolationLogger;
import com.getyourguide.openapi.validation.core.OpenApiInteractionValidatorFactory;
import com.getyourguide.openapi.validation.core.OpenApiRequestValidator;
import com.getyourguide.openapi.validation.core.ValidationReportHandler;
import com.getyourguide.openapi.validation.core.exclusions.InternalViolationExclusions;
import com.getyourguide.openapi.validation.core.log.DefaultOpenApiViolationHandler;
import com.getyourguide.openapi.validation.core.mapper.ValidationReportToOpenApiViolationsMapper;
import com.getyourguide.openapi.validation.core.throttle.RequestBasedValidationReportThrottler;
import com.getyourguide.openapi.validation.core.throttle.ValidationReportThrottler;
Expand Down Expand Up @@ -72,18 +73,17 @@ public MetricsReporter metricsReporter(Optional<MetricsClient> metricsClient) {
}

@Bean
public ValidationReportHandler validationReportHandler(
public OpenApiViolationHandler openApiViolationHandler(
ValidationReportThrottler validationReportThrottler,
ViolationLogger logger,
MetricsReporter metricsReporter,
Optional<ViolationExclusions> violationExclusions
) {
return new ValidationReportHandler(
return new DefaultOpenApiViolationHandler(
validationReportThrottler,
logger,
metricsReporter,
new InternalViolationExclusions(violationExclusions.orElseGet(NoViolationExclusions::new)),
new ValidationReportToOpenApiViolationsMapper()
new InternalViolationExclusions(violationExclusions.orElseGet(NoViolationExclusions::new))
);
}

Expand All @@ -101,7 +101,7 @@ public ValidatorConfiguration validatorConfiguration() {

@Bean
public OpenApiRequestValidator openApiRequestValidator(
ValidationReportHandler validationReportHandler,
OpenApiViolationHandler openApiViolationHandler,
MetricsReporter metricsReporter,
ValidatorConfiguration validatorConfiguration
) {
Expand All @@ -116,10 +116,10 @@ public OpenApiRequestValidator openApiRequestValidator(

return new OpenApiRequestValidator(
threadPoolExecutor,
validationReportHandler,
metricsReporter,
new OpenApiInteractionValidatorFactory()
.build(properties.getSpecificationFilePath(), validatorConfiguration),
new ValidationReportToOpenApiViolationsMapper(),
properties.toOpenApiRequestValidationConfiguration()
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type;

import com.getyourguide.openapi.validation.api.log.OpenApiViolationHandler;
import com.getyourguide.openapi.validation.api.selector.TrafficSelector;
import com.getyourguide.openapi.validation.core.OpenApiRequestValidator;
import com.getyourguide.openapi.validation.factory.ContentCachingWrapperFactory;
Expand Down Expand Up @@ -53,13 +54,15 @@ public WebMvcConfigurer addOpenApiValidationInterceptor(
OpenApiRequestValidator validator,
TrafficSelector trafficSelector,
ServletMetaDataFactory metaDataFactory,
OpenApiViolationHandler openApiViolationHandler,
ContentCachingWrapperFactory contentCachingWrapperFactory
) {
var interceptor = new OpenApiValidationInterceptor(
validator,
trafficSelector,
metaDataFactory,
contentCachingWrapperFactory
contentCachingWrapperFactory,
openApiViolationHandler
);
return new WebMvcConfigurer() {
@Override
Expand Down
Loading

0 comments on commit b21da21

Please sign in to comment.