Handle required parameters in endpoint infrastructure
Closes gh-10372
This commit is contained in:
parent
c903f87429
commit
f1cfad6755
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
@ -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))
|
||||
|
|
|
@ -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",
|
||||
|
|
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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());
|
||||
}
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
|
|
@ -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 {
|
||||
|
||||
|
|
|
@ -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();
|
||||
|
|
Loading…
Reference in New Issue