bugfix: on exception log 500 status code instead of 200 status code (#65

)
getyourguide · Nov 22, 2023 · 0055dd4 · 0055dd4
1 parent 91b5852
commit 0055dd4
Show file tree

Hide file tree

Showing 38 changed files with 1,838 additions and 479 deletions.
diff --git a/build.gradle b/build.gradle
@@ -24,7 +24,7 @@ allprojects {
 }
 
 subprojects {
-    if(it.parent.name == 'examples' || it.parent.name == 'test') {
+    if(it.parent.name == 'examples') {
         apply plugin: 'java'
     } else {
         apply plugin: 'java-library'
@@ -70,6 +70,9 @@ subprojects {
         toolVersion = libs.versions.checkstyle.get()
         configDirectory.set(file("$rootProject.projectDir/config"))
         checkstyleMain.source = "src/main/java"
+        checkstyleMain.exclude('**/build/generated/**')
+        checkstyleTest.source = "src/main/java"
+        checkstyleTest.exclude('**/build/generated/**')
     }
 
     pmd {

diff --git a/...openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapperTest.java b/...openapi/validation/core/validator/MultipleSpecOpenApiInteractionValidatorWrapperTest.java
@@ -61,8 +61,7 @@ private static MockValidatorResult mockValidator() {
         var catchAllValidationReport = mock(ValidationReport.class);
         when(catchAllValidator.validateRequest(any())).thenReturn(catchAllValidationReport);
         when(catchAllValidator.validateResponse(any(), any(), any())).thenReturn(catchAllValidationReport);
-        MockValidatorResult result = new MockValidatorResult(catchAllValidator, catchAllValidationReport);
-        return result;
+        return new MockValidatorResult(catchAllValidator, catchAllValidationReport);
     }
 
     private record MockValidatorResult(

diff --git a/settings.gradle b/settings.gradle
@@ -13,3 +13,7 @@ include(':metrics-reporter:metrics-reporter-datadog-spring-boot')
 include(':examples:examples-common')
 include(':examples:example-spring-boot-starter-web')
 include(':examples:example-spring-boot-starter-webflux')
+
+include(':test:openapi-web')
+include(':test:openapi-webflux')
+include(':test:test-utils')
diff --git a/spring-boot-starter/spring-boot-starter-web/build.gradle b/spring-boot-starter/spring-boot-starter-web/build.gradle
@@ -2,10 +2,9 @@ import org.springframework.boot.gradle.plugin.SpringBootPlugin
 
 plugins {
     alias(libs.plugins.spring.boot) apply false
+    alias(libs.plugins.openapi.generator)
 }
 
-apply from: "${rootDir}/gradle/publish-module.gradle"
-
 dependencies {
     implementation platform(SpringBootPlugin.BOM_COORDINATES)
 
@@ -20,7 +19,8 @@ dependencies {
     // TODO use spotbugs instead and also apply to all modules?
     implementation(libs.find.bugs)
 
-    testImplementation 'org.springframework.boot:spring-boot-starter-test'
+    testImplementation project(':test:test-utils')
+    testImplementation project(':test:openapi-web')
     testImplementation 'org.springframework:spring-web'
     testImplementation 'org.springframework:spring-webmvc'
     testImplementation 'org.apache.tomcat.embed:tomcat-embed-core' // For jakarta.servlet.ServletContext

diff --git a/.../com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java b/.../com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java
@@ -6,11 +6,14 @@
 import com.getyourguide.openapi.validation.core.OpenApiRequestValidator;
 import com.getyourguide.openapi.validation.factory.ContentCachingWrapperFactory;
 import com.getyourguide.openapi.validation.factory.ServletMetaDataFactory;
-import com.getyourguide.openapi.validation.filter.OpenApiValidationHttpFilter;
+import com.getyourguide.openapi.validation.filter.OpenApiValidationFilter;
+import com.getyourguide.openapi.validation.filter.OpenApiValidationInterceptor;
 import lombok.AllArgsConstructor;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
+import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
 
 @Configuration
 @AllArgsConstructor
@@ -30,17 +33,39 @@ public ContentCachingWrapperFactory contentCachingWrapperFactory() {
 
     @Bean
     @ConditionalOnWebApplication(type = Type.SERVLET)
-    public OpenApiValidationHttpFilter openApiValidationHttpFilter(
+    public OpenApiValidationFilter openApiValidationFilter(
         OpenApiRequestValidator validator,
         TrafficSelector trafficSelector,
         ServletMetaDataFactory metaDataFactory,
         ContentCachingWrapperFactory contentCachingWrapperFactory
     ) {
-        return new OpenApiValidationHttpFilter(
+        return new OpenApiValidationFilter(
             validator,
             trafficSelector,
             metaDataFactory,
             contentCachingWrapperFactory
         );
     }
+
+    @Bean
+    @ConditionalOnWebApplication(type = Type.SERVLET)
+    public WebMvcConfigurer addOpenApiValidationInterceptor(
+        OpenApiRequestValidator validator,
+        TrafficSelector trafficSelector,
+        ServletMetaDataFactory metaDataFactory,
+        ContentCachingWrapperFactory contentCachingWrapperFactory
+    ) {
+        var interceptor = new OpenApiValidationInterceptor(
+            validator,
+            trafficSelector,
+            metaDataFactory,
+            contentCachingWrapperFactory
+        );
+        return new WebMvcConfigurer() {
+            @Override
+            public void addInterceptors(final InterceptorRegistry registry) {
+                registry.addInterceptor(interceptor);
+            }
+        };
+    }
 }
diff --git a/...c/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java b/...c/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java
@@ -2,15 +2,36 @@
 
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
+import javax.annotation.Nullable;
 import org.springframework.web.util.ContentCachingRequestWrapper;
 import org.springframework.web.util.ContentCachingResponseWrapper;
+import org.springframework.web.util.WebUtils;
 
 public class ContentCachingWrapperFactory {
     public ContentCachingRequestWrapper buildContentCachingRequestWrapper(HttpServletRequest request) {
+        if (request instanceof ContentCachingRequestWrapper) {
+            return (ContentCachingRequestWrapper) request;
+        }
+
         return new ContentCachingRequestWrapper(request);
     }
 
     public ContentCachingResponseWrapper buildContentCachingResponseWrapper(HttpServletResponse response) {
+        var cachingResponse = getCachingResponse(response);
+        if (cachingResponse != null) {
+            return cachingResponse;
+        }
+
         return new ContentCachingResponseWrapper(response);
     }
+
+    @Nullable
+    public ContentCachingResponseWrapper getCachingResponse(final HttpServletResponse response) {
+        return WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class);
+    }
+
+    @Nullable
+    public ContentCachingRequestWrapper getCachingRequest(HttpServletRequest request) {
+        return request instanceof ContentCachingRequestWrapper ? (ContentCachingRequestWrapper) request : null;
+    }
 }
diff --git a/...web/src/main/java/com/getyourguide/openapi/validation/factory/ServletMetaDataFactory.java b/...web/src/main/java/com/getyourguide/openapi/validation/factory/ServletMetaDataFactory.java
@@ -17,7 +17,12 @@ public RequestMetaData buildRequestMetaData(HttpServletRequest request) {
     }
 
     public ResponseMetaData buildResponseMetaData(HttpServletResponse response) {
-        return new ResponseMetaData(response.getStatus(), response.getContentType(), getHeaders(response));
+        return buildResponseMetaData(response, null);
+    }
+
+    public ResponseMetaData buildResponseMetaData(HttpServletResponse response, Exception exception) {
+        var status = response.getStatus() == 200 && exception != null ? 500 : response.getStatus();
+        return new ResponseMetaData(status, response.getContentType(), getHeaders(response));
     }
 
     private static TreeMap<String, String> getHeaders(HttpServletRequest request) {

diff --git a/...web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationFilter.java b/...web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationFilter.java
@@ -0,0 +1,56 @@
+package com.getyourguide.openapi.validation.filter;
+
+import com.getyourguide.openapi.validation.api.selector.TrafficSelector;
+import com.getyourguide.openapi.validation.core.OpenApiRequestValidator;
+import com.getyourguide.openapi.validation.factory.ContentCachingWrapperFactory;
+import com.getyourguide.openapi.validation.factory.ServletMetaDataFactory;
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import lombok.AllArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+@Slf4j
+@AllArgsConstructor
+public class OpenApiValidationFilter extends OncePerRequestFilter {
+    public static final String ATTRIBUTE_SKIP_VALIDATION = "gyg.openapi-validation.skipValidation";
+    public static final String ATTRIBUTE_REQUEST_META_DATA = "gyg.openapi-validation.requestMetaData";
+
+    private final OpenApiRequestValidator validator;
+    private final TrafficSelector trafficSelector;
+    private final ServletMetaDataFactory metaDataFactory;
+    private final ContentCachingWrapperFactory contentCachingWrapperFactory;
+
+    @Override
+    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
+        throws ServletException, IOException {
+        var requestMetaData = metaDataFactory.buildRequestMetaData(request);
+        request.setAttribute(ATTRIBUTE_REQUEST_META_DATA, requestMetaData);
+        if (!validator.isReady() || !trafficSelector.shouldRequestBeValidated(requestMetaData)) {
+            request.setAttribute(ATTRIBUTE_SKIP_VALIDATION, true);
+            request.setAttribute(ATTRIBUTE_SKIP_VALIDATION, true);
+            filterChain.doFilter(request, response);
+            return;
+        }
+
+        var requestToUse = contentCachingWrapperFactory.buildContentCachingRequestWrapper(request);
+        var responseToUse = contentCachingWrapperFactory.buildContentCachingResponseWrapper(response);
+        filterChain.doFilter(requestToUse, responseToUse);
+
+        // in case the response was cached it has to be written to the original response
+        if (!isAsyncStarted(requestToUse)) {
+            var cachingResponse = contentCachingWrapperFactory.getCachingResponse(responseToUse);
+            if (cachingResponse != null) {
+                cachingResponse.copyBodyToResponse();
+            }
+        }
+    }
+
+    @Override
+    protected boolean shouldNotFilterAsyncDispatch() {
+        return false;
+    }
+}
diff --git a/...src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java b/...src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java