Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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')
6 changes: 3 additions & 3 deletions spring-boot-starter/spring-boot-starter-web/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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)

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

This file was deleted.

Loading