Add a way to signal that an endpoint request is invalid
This commit adds InvalidEndpointRequestException as a technology agnostic way to signal that an endpoint request is invalid. When such exception is thrown, the web layer translates that to a 400. Rather than overriding the reason, this commit makes sure to reuse the error infrastructure. Closes gh-10618
This commit is contained in:
parent
55c8ceb440
commit
1f4a32f0ad
|
@ -0,0 +1,48 @@
|
|||
/*
|
||||
* Copyright 2012-2018 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.boot.actuate.endpoint;
|
||||
|
||||
/**
|
||||
* Indicate that an endpoint request is invalid.
|
||||
*
|
||||
* @author Stephane Nicoll
|
||||
* @since 2.0.0
|
||||
*/
|
||||
public class InvalidEndpointRequestException extends RuntimeException {
|
||||
|
||||
private final String reason;
|
||||
|
||||
public InvalidEndpointRequestException(String message, String reason) {
|
||||
super(message);
|
||||
this.reason = reason;
|
||||
}
|
||||
|
||||
public InvalidEndpointRequestException(String message, String reason,
|
||||
Throwable cause) {
|
||||
super(message, cause);
|
||||
this.reason = reason;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the reason explaining why the request is invalid, potentially {@code null}.
|
||||
* @return the reason for the failure
|
||||
*/
|
||||
public String getReason() {
|
||||
return this.reason;
|
||||
}
|
||||
|
||||
}
|
|
@ -17,7 +17,9 @@
|
|||
package org.springframework.boot.actuate.endpoint.invoke;
|
||||
|
||||
import java.util.Set;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException;
|
||||
import org.springframework.util.StringUtils;
|
||||
|
||||
/**
|
||||
|
@ -28,14 +30,17 @@ import org.springframework.util.StringUtils;
|
|||
* @author Phillip Webb
|
||||
* @since 2.0.0
|
||||
*/
|
||||
public class MissingParametersException extends RuntimeException {
|
||||
public final class MissingParametersException extends InvalidEndpointRequestException {
|
||||
|
||||
private final Set<OperationParameter> missingParameters;
|
||||
|
||||
public MissingParametersException(Set<OperationParameter> missingParameters) {
|
||||
super("Failed to invoke operation because the following required "
|
||||
+ "parameters were missing: "
|
||||
+ StringUtils.collectionToCommaDelimitedString(missingParameters));
|
||||
+ StringUtils.collectionToCommaDelimitedString(missingParameters),
|
||||
"Missing parameters: " + missingParameters.stream()
|
||||
.map(OperationParameter::getName)
|
||||
.collect(Collectors.joining(",")));
|
||||
this.missingParameters = missingParameters;
|
||||
}
|
||||
|
||||
|
|
|
@ -16,6 +16,8 @@
|
|||
|
||||
package org.springframework.boot.actuate.endpoint.invoke;
|
||||
|
||||
import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException;
|
||||
|
||||
/**
|
||||
* A {@code ParameterMappingException} is thrown when a failure occurs during
|
||||
* {@link ParameterValueMapper#mapParameterValue operation parameter mapping}.
|
||||
|
@ -23,7 +25,7 @@ package org.springframework.boot.actuate.endpoint.invoke;
|
|||
* @author Andy Wilkinson
|
||||
* @since 2.0.0
|
||||
*/
|
||||
public class ParameterMappingException extends RuntimeException {
|
||||
public final class ParameterMappingException extends InvalidEndpointRequestException {
|
||||
|
||||
private final OperationParameter parameter;
|
||||
|
||||
|
@ -39,7 +41,7 @@ public class ParameterMappingException extends RuntimeException {
|
|||
public ParameterMappingException(OperationParameter parameter, Object value,
|
||||
Throwable cause) {
|
||||
super("Failed to map " + value + " of type " + value.getClass() + " to "
|
||||
+ parameter, cause);
|
||||
+ parameter, "Parameter mapping failure", cause);
|
||||
this.parameter = parameter;
|
||||
this.value = value;
|
||||
}
|
||||
|
|
|
@ -31,8 +31,7 @@ import javax.management.ReflectionException;
|
|||
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
import org.springframework.boot.actuate.endpoint.invoke.MissingParametersException;
|
||||
import org.springframework.boot.actuate.endpoint.invoke.ParameterMappingException;
|
||||
import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.ClassUtils;
|
||||
|
||||
|
@ -103,7 +102,7 @@ public class EndpointMBean implements DynamicMBean {
|
|||
}
|
||||
return this.responseMapper.mapResponse(result);
|
||||
}
|
||||
catch (MissingParametersException | ParameterMappingException ex) {
|
||||
catch (InvalidEndpointRequestException ex) {
|
||||
throw new IllegalArgumentException(ex.getMessage(), ex);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -38,8 +38,7 @@ import org.glassfish.jersey.server.model.Resource;
|
|||
import org.glassfish.jersey.server.model.Resource.Builder;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
import org.springframework.boot.actuate.endpoint.invoke.MissingParametersException;
|
||||
import org.springframework.boot.actuate.endpoint.invoke.ParameterMappingException;
|
||||
import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException;
|
||||
import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver;
|
||||
import org.springframework.boot.actuate.endpoint.web.EndpointMediaTypes;
|
||||
import org.springframework.boot.actuate.endpoint.web.ExposableWebEndpoint;
|
||||
|
@ -155,7 +154,7 @@ public class JerseyEndpointResourceFactory {
|
|||
Object response = this.operation.invoke(arguments);
|
||||
return convertToJaxRsResponse(response, data.getRequest().getMethod());
|
||||
}
|
||||
catch (MissingParametersException | ParameterMappingException ex) {
|
||||
catch (InvalidEndpointRequestException ex) {
|
||||
return Response.status(Status.BAD_REQUEST).build();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -26,10 +26,9 @@ import reactor.core.publisher.Mono;
|
|||
import reactor.core.publisher.MonoSink;
|
||||
import reactor.core.scheduler.Schedulers;
|
||||
|
||||
import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException;
|
||||
import org.springframework.boot.actuate.endpoint.OperationType;
|
||||
import org.springframework.boot.actuate.endpoint.invoke.MissingParametersException;
|
||||
import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
|
||||
import org.springframework.boot.actuate.endpoint.invoke.ParameterMappingException;
|
||||
import org.springframework.boot.actuate.endpoint.web.EndpointMediaTypes;
|
||||
import org.springframework.boot.actuate.endpoint.web.ExposableWebEndpoint;
|
||||
import org.springframework.boot.actuate.endpoint.web.WebEndpointResponse;
|
||||
|
@ -52,6 +51,7 @@ import org.springframework.web.reactive.result.condition.ProducesRequestConditio
|
|||
import org.springframework.web.reactive.result.condition.RequestMethodsRequestCondition;
|
||||
import org.springframework.web.reactive.result.method.RequestMappingInfo;
|
||||
import org.springframework.web.reactive.result.method.RequestMappingInfoHandlerMapping;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
import org.springframework.web.server.ServerWebExchange;
|
||||
import org.springframework.web.util.pattern.PathPatternParser;
|
||||
|
||||
|
@ -288,10 +288,9 @@ public abstract class AbstractWebFluxEndpointHandlerMapping
|
|||
private Mono<ResponseEntity<Object>> handleResult(Publisher<?> result,
|
||||
HttpMethod httpMethod) {
|
||||
return Mono.from(result).map(this::toResponseEntity)
|
||||
.onErrorReturn(MissingParametersException.class,
|
||||
new ResponseEntity<>(HttpStatus.BAD_REQUEST))
|
||||
.onErrorReturn(ParameterMappingException.class,
|
||||
new ResponseEntity<>(HttpStatus.BAD_REQUEST))
|
||||
.onErrorMap(InvalidEndpointRequestException.class, (ex) ->
|
||||
new ResponseStatusException(HttpStatus.BAD_REQUEST,
|
||||
ex.getReason()))
|
||||
.defaultIfEmpty(new ResponseEntity<>(httpMethod == HttpMethod.GET
|
||||
? HttpStatus.NOT_FOUND : HttpStatus.NO_CONTENT));
|
||||
}
|
||||
|
|
|
@ -27,9 +27,8 @@ import javax.servlet.http.HttpServletRequest;
|
|||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
import org.springframework.beans.factory.InitializingBean;
|
||||
import org.springframework.boot.actuate.endpoint.invoke.MissingParametersException;
|
||||
import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException;
|
||||
import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
|
||||
import org.springframework.boot.actuate.endpoint.invoke.ParameterMappingException;
|
||||
import org.springframework.boot.actuate.endpoint.web.EndpointMediaTypes;
|
||||
import org.springframework.boot.actuate.endpoint.web.ExposableWebEndpoint;
|
||||
import org.springframework.boot.actuate.endpoint.web.WebEndpointResponse;
|
||||
|
@ -44,6 +43,7 @@ import org.springframework.util.StringUtils;
|
|||
import org.springframework.web.bind.annotation.RequestBody;
|
||||
import org.springframework.web.bind.annotation.RequestMethod;
|
||||
import org.springframework.web.bind.annotation.ResponseBody;
|
||||
import org.springframework.web.bind.annotation.ResponseStatus;
|
||||
import org.springframework.web.cors.CorsConfiguration;
|
||||
import org.springframework.web.servlet.HandlerMapping;
|
||||
import org.springframework.web.servlet.mvc.condition.ConsumesRequestCondition;
|
||||
|
@ -243,8 +243,8 @@ public abstract class AbstractWebMvcEndpointHandlerMapping
|
|||
return handleResult(this.invoker.invoke(arguments),
|
||||
HttpMethod.valueOf(request.getMethod()));
|
||||
}
|
||||
catch (MissingParametersException | ParameterMappingException ex) {
|
||||
return new ResponseEntity<Void>(HttpStatus.BAD_REQUEST);
|
||||
catch (InvalidEndpointRequestException ex) {
|
||||
throw new BadOperationRequestException(ex.getReason());
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -300,4 +300,13 @@ public abstract class AbstractWebMvcEndpointHandlerMapping
|
|||
|
||||
}
|
||||
|
||||
@ResponseStatus(code = HttpStatus.BAD_REQUEST)
|
||||
private static class BadOperationRequestException extends RuntimeException {
|
||||
|
||||
BadOperationRequestException(String message) {
|
||||
super(message);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -42,6 +42,7 @@ import org.springframework.context.annotation.Import;
|
|||
import org.springframework.core.env.MapPropertySource;
|
||||
import org.springframework.core.io.ByteArrayResource;
|
||||
import org.springframework.core.io.Resource;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.lang.Nullable;
|
||||
import org.springframework.test.web.reactive.server.WebTestClient;
|
||||
|
@ -157,8 +158,13 @@ public abstract class AbstractWebEndpointIntegrationTests<T extends Configurable
|
|||
|
||||
@Test
|
||||
public void readOperationWithMappingFailureProducesBadRequestResponse() {
|
||||
load(QueryEndpointConfiguration.class, (client) -> client.get()
|
||||
.uri("/query?two=two").exchange().expectStatus().isBadRequest());
|
||||
load(QueryEndpointConfiguration.class, (client) -> {
|
||||
WebTestClient.BodyContentSpec body = client.get()
|
||||
.uri("/query?two=two").exchange().expectStatus().isBadRequest()
|
||||
.expectBody();
|
||||
validateErrorBody(body, HttpStatus.BAD_REQUEST, "/endpoints/query",
|
||||
"Missing parameters: one");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -275,8 +281,13 @@ public abstract class AbstractWebEndpointIntegrationTests<T extends Configurable
|
|||
|
||||
@Test
|
||||
public void readOperationWithMissingRequiredParametersReturnsBadRequestResponse() {
|
||||
load(RequiredParameterEndpointConfiguration.class, (client) -> client.get()
|
||||
.uri("/requiredparameters").exchange().expectStatus().isBadRequest());
|
||||
load(RequiredParameterEndpointConfiguration.class, (client) -> {
|
||||
WebTestClient.BodyContentSpec body = client.get()
|
||||
.uri("/requiredparameters").exchange().expectStatus().isBadRequest()
|
||||
.expectBody();
|
||||
validateErrorBody(body, HttpStatus.BAD_REQUEST,
|
||||
"/endpoints/requiredparameters", "Missing parameters: foo");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -321,6 +332,14 @@ public abstract class AbstractWebEndpointIntegrationTests<T extends Configurable
|
|||
|
||||
protected abstract int getPort(T context);
|
||||
|
||||
protected void validateErrorBody(WebTestClient.BodyContentSpec body,
|
||||
HttpStatus status, String path, String message) {
|
||||
body.jsonPath("status").isEqualTo(status.value())
|
||||
.jsonPath("error").isEqualTo(status.getReasonPhrase())
|
||||
.jsonPath("path").isEqualTo(path)
|
||||
.jsonPath("message").isEqualTo(message);
|
||||
}
|
||||
|
||||
private void load(Class<?> configuration,
|
||||
BiConsumer<ApplicationContext, WebTestClient> consumer) {
|
||||
load(configuration, "/endpoints", consumer);
|
||||
|
|
|
@ -37,6 +37,8 @@ import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebSe
|
|||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.core.env.Environment;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.test.web.reactive.server.WebTestClient;
|
||||
|
||||
/**
|
||||
* Integration tests for web endpoints exposed using Jersey.
|
||||
|
@ -64,6 +66,12 @@ public class JerseyWebEndpointIntegrationTests extends
|
|||
return context.getWebServer().getPort();
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void validateErrorBody(WebTestClient.BodyContentSpec body,
|
||||
HttpStatus status, String path, String message) {
|
||||
// Jersey doesn't support the general error page handling
|
||||
}
|
||||
|
||||
@Configuration
|
||||
static class JerseyConfiguration {
|
||||
|
||||
|
|
|
@ -23,6 +23,8 @@ import org.junit.Test;
|
|||
import org.springframework.boot.actuate.endpoint.web.EndpointMediaTypes;
|
||||
import org.springframework.boot.actuate.endpoint.web.annotation.AbstractWebEndpointIntegrationTests;
|
||||
import org.springframework.boot.actuate.endpoint.web.annotation.WebEndpointDiscoverer;
|
||||
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.web.reactive.error.ErrorWebFluxAutoConfiguration;
|
||||
import org.springframework.boot.endpoint.web.EndpointMapping;
|
||||
import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory;
|
||||
import org.springframework.boot.web.reactive.context.AnnotationConfigReactiveWebServerApplicationContext;
|
||||
|
@ -95,6 +97,7 @@ public class WebFluxEndpointIntegrationTests
|
|||
|
||||
@Configuration
|
||||
@EnableWebFlux
|
||||
@ImportAutoConfiguration(ErrorWebFluxAutoConfiguration.class)
|
||||
static class ReactiveConfiguration {
|
||||
|
||||
private int port;
|
||||
|
|
|
@ -23,6 +23,13 @@ import org.junit.Test;
|
|||
import org.springframework.boot.actuate.endpoint.web.EndpointMediaTypes;
|
||||
import org.springframework.boot.actuate.endpoint.web.annotation.AbstractWebEndpointIntegrationTests;
|
||||
import org.springframework.boot.actuate.endpoint.web.annotation.WebEndpointDiscoverer;
|
||||
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.web.servlet.ServletWebServerFactoryAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration;
|
||||
import org.springframework.boot.endpoint.web.EndpointMapping;
|
||||
import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory;
|
||||
import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext;
|
||||
|
@ -32,8 +39,6 @@ import org.springframework.core.env.Environment;
|
|||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.web.cors.CorsConfiguration;
|
||||
import org.springframework.web.servlet.DispatcherServlet;
|
||||
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
|
@ -89,7 +94,10 @@ public class MvcWebEndpointIntegrationTests extends
|
|||
}
|
||||
|
||||
@Configuration
|
||||
@EnableWebMvc
|
||||
@ImportAutoConfiguration({ JacksonAutoConfiguration.class,
|
||||
HttpMessageConvertersAutoConfiguration.class,
|
||||
ServletWebServerFactoryAutoConfiguration.class, WebMvcAutoConfiguration.class,
|
||||
DispatcherServletAutoConfiguration.class, ErrorMvcAutoConfiguration.class })
|
||||
static class WebMvcConfiguration {
|
||||
|
||||
@Bean
|
||||
|
@ -97,11 +105,6 @@ public class MvcWebEndpointIntegrationTests extends
|
|||
return new TomcatServletWebServerFactory(0);
|
||||
}
|
||||
|
||||
@Bean
|
||||
public DispatcherServlet dispatcherServlet() {
|
||||
return new DispatcherServlet();
|
||||
}
|
||||
|
||||
@Bean
|
||||
public WebMvcEndpointHandlerMapping webEndpointHandlerMapping(
|
||||
Environment environment, WebEndpointDiscoverer endpointDiscoverer,
|
||||
|
|
Loading…
Reference in New Issue