Handle required parameters in endpoint infrastructure

Closes gh-10372
This commit is contained in:
Madhura Bhave 2017-09-28 14:20:24 -07:00
parent c903f87429
commit f1cfad6755
14 changed files with 192 additions and 18 deletions

View File

@ -34,6 +34,7 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.boot.actuate.endpoint.EndpointInfo;
import org.springframework.boot.actuate.endpoint.OperationInvoker;
import org.springframework.boot.actuate.endpoint.ParameterMappingException;
import org.springframework.boot.actuate.endpoint.ParametersMissingException;
import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver;
import org.springframework.boot.actuate.endpoint.web.Link;
import org.springframework.boot.actuate.endpoint.web.WebEndpointOperation;
@ -165,7 +166,7 @@ class CloudFoundryWebEndpointServletHandlerMapping
try {
return handleResult(this.operationInvoker.invoke(arguments), httpMethod);
}
catch (ParameterMappingException ex) {
catch (ParametersMissingException | ParameterMappingException ex) {
return new ResponseEntity<Void>(HttpStatus.BAD_REQUEST);
}
}

View File

@ -22,6 +22,7 @@ import org.springframework.boot.actuate.audit.AuditEventsEndpoint.AuditEventsDes
import org.springframework.boot.actuate.endpoint.annotation.ReadOperation;
import org.springframework.boot.actuate.endpoint.web.WebEndpointResponse;
import org.springframework.boot.actuate.endpoint.web.annotation.WebEndpointExtension;
import org.springframework.lang.Nullable;
/**
* {@link WebEndpointExtension} for the {@link AuditEventsEndpoint}.
@ -40,10 +41,7 @@ public class AuditEventsWebEndpointExtension {
@ReadOperation
public WebEndpointResponse<AuditEventsDescriptor> eventsWithPrincipalDateAfterAndType(
String principal, Date after, String type) {
if (after == null) {
return new WebEndpointResponse<>(WebEndpointResponse.STATUS_BAD_REQUEST);
}
@Nullable String principal, Date after, @Nullable String type) {
AuditEventsDescriptor auditEvents = this.delegate
.eventsWithPrincipalDateAfterAndType(principal, after, type);
return new WebEndpointResponse<>(auditEvents);

View File

@ -39,7 +39,7 @@ public class ParameterMappingException extends RuntimeException {
* @param cause the cause of the mapping failure
*/
public ParameterMappingException(Object input, Class<?> type, Throwable cause) {
super("Failed to map " + input + " of type " + input.getClass() + " to type "
super("Failed to map " + input + " of type " + input.getClass() + " to type "
+ type, cause);
this.input = input;
this.type = type;

View File

@ -0,0 +1,48 @@
/*
* Copyright 2012-2017 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;
import java.util.Set;
import org.springframework.util.StringUtils;
/**
* {@link RuntimeException} thrown when an endpoint invocation does
* not contain required parameters.
*
* @author Madhura Bhave
* @since 2.0.0
*/
public class ParametersMissingException extends RuntimeException {
private final Set<String> parameters;
public ParametersMissingException(Set<String> parameters) {
super("Failed to invoke operation because the following required " +
"parameters were missing: " + StringUtils.collectionToCommaDelimitedString(parameters));
this.parameters = parameters;
}
/**
* Returns the parameters that were missing.
* @return the parameters
*/
public Set<String> getParameters() {
return this.parameters;
}
}

View File

@ -19,9 +19,11 @@ package org.springframework.boot.actuate.endpoint;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.springframework.lang.Nullable;
import org.springframework.util.ReflectionUtils;
/**
@ -56,10 +58,30 @@ public class ReflectiveOperationInvoker implements OperationInvoker {
@Override
public Object invoke(Map<String, Object> arguments) {
validateRequiredParameters(arguments);
return ReflectionUtils.invokeMethod(this.method, this.target,
resolveArguments(arguments));
}
private void validateRequiredParameters(Map<String, Object> arguments) {
Set<String> missingParameters = Stream.of(this.method.getParameters())
.filter(p -> isMissing(p, arguments))
.map(Parameter::getName)
.collect(Collectors.toSet());
if (!missingParameters.isEmpty()) {
throw new ParametersMissingException(missingParameters);
}
}
private boolean isMissing(Parameter parameter, Map<String, Object> arguments) {
Object resolved = arguments.get(parameter.getName());
return (resolved == null && !isExplicitNullable(parameter));
}
private boolean isExplicitNullable(Parameter parameter) {
return (parameter.getAnnotationsByType(Nullable.class).length != 0);
}
private Object[] resolveArguments(Map<String, Object> arguments) {
return Stream.of(this.method.getParameters())
.map((parameter) -> resolveArgument(parameter, arguments))

View File

@ -33,6 +33,8 @@ import javax.management.ReflectionException;
import reactor.core.publisher.Mono;
import org.springframework.boot.actuate.endpoint.EndpointInfo;
import org.springframework.boot.actuate.endpoint.ParameterMappingException;
import org.springframework.boot.actuate.endpoint.ParametersMissingException;
import org.springframework.util.ClassUtils;
/**
@ -79,11 +81,17 @@ public class EndpointMBean implements DynamicMBean {
if (operation != null) {
Map<String, Object> arguments = getArguments(params,
operation.getParameters());
Object result = operation.getInvoker().invoke(arguments);
if (REACTOR_PRESENT) {
result = ReactiveHandler.handle(result);
try {
Object result = operation.getInvoker().invoke(arguments);
if (REACTOR_PRESENT) {
result = ReactiveHandler.handle(result);
}
return this.operationResponseConverter.apply(result);
}
return this.operationResponseConverter.apply(result);
catch (ParametersMissingException | ParameterMappingException ex) {
throw new IllegalArgumentException(ex.getMessage());
}
}
throw new ReflectionException(new IllegalArgumentException(
String.format("Endpoint with id '%s' has no operation named %s",

View File

@ -41,6 +41,7 @@ import reactor.core.publisher.Mono;
import org.springframework.boot.actuate.endpoint.EndpointInfo;
import org.springframework.boot.actuate.endpoint.OperationInvoker;
import org.springframework.boot.actuate.endpoint.ParameterMappingException;
import org.springframework.boot.actuate.endpoint.ParametersMissingException;
import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver;
import org.springframework.boot.actuate.endpoint.web.Link;
import org.springframework.boot.actuate.endpoint.web.OperationRequestPredicate;
@ -145,7 +146,7 @@ public class JerseyEndpointResourceFactory {
Object response = this.operationInvoker.invoke(arguments);
return convertToJaxRsResponse(response, data.getRequest().getMethod());
}
catch (ParameterMappingException ex) {
catch (ParametersMissingException | ParameterMappingException ex) {
return Response.status(Status.BAD_REQUEST).build();
}
}

View File

@ -32,6 +32,7 @@ import org.springframework.boot.actuate.endpoint.EndpointInfo;
import org.springframework.boot.actuate.endpoint.OperationInvoker;
import org.springframework.boot.actuate.endpoint.OperationType;
import org.springframework.boot.actuate.endpoint.ParameterMappingException;
import org.springframework.boot.actuate.endpoint.ParametersMissingException;
import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver;
import org.springframework.boot.actuate.endpoint.web.Link;
import org.springframework.boot.actuate.endpoint.web.OperationRequestPredicate;
@ -219,6 +220,8 @@ public class WebFluxEndpointHandlerMapping extends RequestMappingInfoHandlerMapp
private Publisher<ResponseEntity<Object>> handleResult(Publisher<?> result,
HttpMethod httpMethod) {
return Mono.from(result).map(this::toResponseEntity)
.onErrorReturn(ParametersMissingException.class,
new ResponseEntity<>(HttpStatus.BAD_REQUEST))
.onErrorReturn(ParameterMappingException.class,
new ResponseEntity<>(HttpStatus.BAD_REQUEST))
.defaultIfEmpty(new ResponseEntity<>(httpMethod == HttpMethod.GET

View File

@ -28,6 +28,7 @@ import javax.servlet.http.HttpServletRequest;
import org.springframework.boot.actuate.endpoint.EndpointInfo;
import org.springframework.boot.actuate.endpoint.OperationInvoker;
import org.springframework.boot.actuate.endpoint.ParameterMappingException;
import org.springframework.boot.actuate.endpoint.ParametersMissingException;
import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver;
import org.springframework.boot.actuate.endpoint.web.Link;
import org.springframework.boot.actuate.endpoint.web.WebEndpointOperation;
@ -127,7 +128,7 @@ public class WebMvcEndpointHandlerMapping extends AbstractWebMvcEndpointHandlerM
try {
return handleResult(this.operationInvoker.invoke(arguments), httpMethod);
}
catch (ParameterMappingException ex) {
catch (ParametersMissingException | ParameterMappingException ex) {
return new ResponseEntity<Void>(HttpStatus.BAD_REQUEST);
}
}

View File

@ -42,6 +42,7 @@ import org.springframework.core.env.Environment;
import org.springframework.core.env.MutablePropertySources;
import org.springframework.core.env.PropertySource;
import org.springframework.core.env.StandardEnvironment;
import org.springframework.lang.Nullable;
import org.springframework.util.PropertyPlaceholderHelper;
import org.springframework.util.StringUtils;
import org.springframework.util.SystemPropertyUtils;
@ -72,7 +73,7 @@ public class EnvironmentEndpoint {
}
@ReadOperation
public EnvironmentDescriptor environment(String pattern) {
public EnvironmentDescriptor environment(@Nullable String pattern) {
if (StringUtils.hasText(pattern)) {
return getEnvironmentDescriptor(Pattern.compile(pattern).asPredicate());
}

View File

@ -42,6 +42,7 @@ import org.springframework.boot.actuate.endpoint.annotation.ReadOperation;
import org.springframework.boot.actuate.endpoint.web.WebEndpointResponse;
import org.springframework.core.io.FileSystemResource;
import org.springframework.core.io.Resource;
import org.springframework.lang.Nullable;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;
@ -72,7 +73,7 @@ public class HeapDumpWebEndpoint {
}
@ReadOperation
public WebEndpointResponse<Resource> heapDump(Boolean live) {
public WebEndpointResponse<Resource> heapDump(@Nullable Boolean live) {
try {
if (this.lock.tryLock(this.timeout, TimeUnit.MILLISECONDS)) {
try {

View File

@ -38,9 +38,6 @@ public class SessionsWebEndpointExtension {
@ReadOperation
public WebEndpointResponse<SessionsReport> sessionsForUsername(String username) {
if (username == null) {
return new WebEndpointResponse<>(WebEndpointResponse.STATUS_BAD_REQUEST);
}
SessionsReport sessions = this.delegate.sessionsForUsername(username);
return new WebEndpointResponse<>(sessions);
}

View File

@ -50,6 +50,7 @@ import org.springframework.boot.actuate.endpoint.convert.ConversionServiceOperat
import org.springframework.boot.actuate.endpoint.jmx.annotation.JmxAnnotationEndpointDiscoverer;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.lang.Nullable;
import static org.assertj.core.api.Assertions.assertThat;
@ -265,6 +266,52 @@ public class EndpointMBeanTests {
});
}
@Test
public void invokeWithParameterMappingExceptionMapsToIllegalArgumentException() throws Exception {
load(FooEndpoint.class, (discoverer) -> {
ObjectName objectName = registerEndpoint(discoverer, "foo");
try {
this.server.invoke(objectName, "getOne",
new Object[] { "wrong" }, new String[] { String.class.getName() });
}
catch (Exception ex) {
assertThat(ex.getCause()).isExactlyInstanceOf(IllegalArgumentException.class);
assertThat(ex.getCause().getMessage()).isEqualTo(String.format("Failed to map wrong of type " +
"%s to type %s", String.class, FooName.class));
}
});
}
@Test
public void invokeWithMissingRequiredParameterExceptionMapsToIllegalArgumentException() throws Exception {
load(RequiredParametersEndpoint.class, (discoverer) -> {
ObjectName objectName = registerEndpoint(discoverer, "requiredparameters");
try {
this.server.invoke(objectName, "read",
new Object[] {}, new String[] { String.class.getName() });
}
catch (Exception ex) {
assertThat(ex.getCause()).isExactlyInstanceOf(IllegalArgumentException.class);
assertThat(ex.getCause().getMessage()).isEqualTo("Failed to invoke operation because the following " +
"required parameters were missing: foo,baz");
}
});
}
@Test
public void invokeWithMissingNullableParameter() throws Exception {
load(RequiredParametersEndpoint.class, (discoverer) -> {
ObjectName objectName = registerEndpoint(discoverer, "requiredparameters");
try {
this.server.invoke(objectName, "read",
new Object[] {null, "hello", "world"}, new String[] { String.class.getName() });
}
catch (Exception ex) {
throw new AssertionError("Nullable parameter should not be required.");
}
});
}
private ObjectName registerEndpoint(JmxAnnotationEndpointDiscoverer discoverer,
String endpointId) {
Collection<EndpointMBean> mBeans = this.jmxEndpointMBeanFactory
@ -326,6 +373,16 @@ public class EndpointMBeanTests {
}
@Endpoint(id = "requiredparameters")
static class RequiredParametersEndpoint {
@ReadOperation
public String read(@Nullable String bar, String foo, String baz) {
return foo;
}
}
@Endpoint(id = "reactive")
static class ReactiveEndpoint {

View File

@ -47,6 +47,7 @@ import org.springframework.core.env.MapPropertySource;
import org.springframework.core.io.ByteArrayResource;
import org.springframework.core.io.Resource;
import org.springframework.http.MediaType;
import org.springframework.lang.Nullable;
import org.springframework.test.web.reactive.server.WebTestClient;
import static org.assertj.core.api.Assertions.assertThat;
@ -281,6 +282,20 @@ public abstract class AbstractWebEndpointIntegrationTests<T extends Configurable
.valueMatches("Content-Type", "text/plain(;charset=.*)?"));
}
@Test
public void readOperationWithMissingRequiredParametersReturnsBadRequestResponse() throws Exception {
load(RequiredParameterEndpointConfiguration.class,
(client) -> client.get().uri("/requiredparameters").exchange()
.expectStatus().isBadRequest());
}
@Test
public void readOperationWithMissingNullableParametersIsOk() throws Exception {
load(RequiredParameterEndpointConfiguration.class,
(client) -> client.get().uri("/requiredparameters?foo=hello").exchange()
.expectStatus().isOk());
}
protected abstract T createApplicationContext(Class<?>... config);
protected abstract int getPort(T context);
@ -489,6 +504,17 @@ public abstract class AbstractWebEndpointIntegrationTests<T extends Configurable
}
@Configuration
@Import(BaseConfiguration.class)
static class RequiredParameterEndpointConfiguration {
@Bean
public RequiredParametersEndpoint requiredParametersEndpoint() {
return new RequiredParametersEndpoint();
}
}
@Endpoint(id = "test")
static class TestEndpoint {
@ -509,7 +535,7 @@ public abstract class AbstractWebEndpointIntegrationTests<T extends Configurable
}
@WriteOperation
public void write(String foo, String bar) {
public void write(@Nullable String foo, @Nullable String bar) {
this.endpointDelegate.write(foo, bar);
}
@ -657,6 +683,16 @@ public abstract class AbstractWebEndpointIntegrationTests<T extends Configurable
}
@Endpoint(id = "requiredparameters")
static class RequiredParametersEndpoint {
@ReadOperation
public String read(String foo, @Nullable String bar) {
return foo;
}
}
public interface EndpointDelegate {
void write();