Handle required parameters in endpoint infrastructure

Closes gh-10372

spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointServletHandlerMapping.java

@@ -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);
 			}
 		}

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/audit/AuditEventsWebEndpointExtension.java

@@ -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);

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParameterMappingException.java

@@ -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;

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParametersMissingException.java

+/*
+ * 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;
+	}
+}
+

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ReflectiveOperationInvoker.java

@@ -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))

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java

@@ -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",

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/jersey/JerseyEndpointResourceFactory.java

@@ -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();
 			}
 		}

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/reactive/WebFluxEndpointHandlerMapping.java

@@ -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

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java

@@ -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);
 			}
 		}

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java

@@ -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());
 		}

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java

@@ -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 {

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/session/SessionsWebEndpointExtension.java

@@ -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);
 	}

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java

@@ -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 {
 

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/AbstractWebEndpointIntegrationTests.java

@@ -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();