Allow caching for an Endpoint operation with optional arguments

This commit makes sure that caching is enabled if an operation has
nullable parameters and the actual invocation provides null values.

Closes gh-11795

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java

@@ -20,6 +20,7 @@ import java.util.Map;
 
 import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
 import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
 
 /**
  * An {@link OperationInvoker} that caches the response of an operation with a
@@ -58,6 +59,9 @@ public class CachingOperationInvoker implements OperationInvoker {
 
 	@Override
 	public Object invoke(Map<String, Object> arguments) {
+		if (hasArgument(arguments)) {
+			return this.invoker.invoke(arguments);
+		}
 		long accessTime = System.currentTimeMillis();
 		CachedResponse cached = this.cachedResponse;
 		if (cached == null || cached.isStale(accessTime, this.timeToLive)) {
@@ -68,6 +72,17 @@ public class CachingOperationInvoker implements OperationInvoker {
 		return cached.getResponse();
 	}
 
+	private boolean hasArgument(Map<String, Object> arguments) {
+		if (!ObjectUtils.isEmpty(arguments)) {
+			for (Object value : arguments.values()) {
+				if (value != null) {
+					return true;
+				}
+			}
+		}
+		return false;
+	}
+
 	/**
 	 * Apply caching configuration when appropriate to the given invoker.
 	 * @param invoker the invoker to wrap

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java

@@ -21,6 +21,7 @@ import java.util.function.Function;
 import org.springframework.boot.actuate.endpoint.OperationType;
 import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
 import org.springframework.boot.actuate.endpoint.invoke.OperationInvokerAdvisor;
+import org.springframework.boot.actuate.endpoint.invoke.OperationParameter;
 import org.springframework.boot.actuate.endpoint.invoke.OperationParameters;
 
 /**
@@ -40,7 +41,7 @@ public class CachingOperationInvokerAdvisor implements OperationInvokerAdvisor {
 	@Override
 	public OperationInvoker apply(String endpointId, OperationType operationType,
 			OperationParameters parameters, OperationInvoker invoker) {
-		if (operationType == OperationType.READ && !parameters.hasParameters()) {
+		if (operationType == OperationType.READ && !hasMandatoryParameter(parameters)) {
 			Long timeToLive = this.endpointIdTimeToLive.apply(endpointId);
 			if (timeToLive != null && timeToLive > 0) {
 				return new CachingOperationInvoker(invoker, timeToLive);
@@ -49,4 +50,13 @@ public class CachingOperationInvokerAdvisor implements OperationInvokerAdvisor {
 		return invoker;
 	}
 
+	private boolean hasMandatoryParameter(OperationParameters parameters) {
+		for (OperationParameter parameter : parameters) {
+			if (!parameter.isNullable()) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 }

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisorTests.java

@@ -28,6 +28,7 @@ import org.springframework.boot.actuate.endpoint.OperationType;
 import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
 import org.springframework.boot.actuate.endpoint.invoke.OperationParameters;
 import org.springframework.boot.actuate.endpoint.invoke.reflect.OperationMethod;
+import org.springframework.lang.Nullable;
 import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.util.ReflectionUtils;
 
@@ -40,6 +41,7 @@ import static org.mockito.Mockito.verify;
  * Tests for {@link CachingOperationInvokerAdvisor}.
  *
  * @author Phillip Webb
+ * @author Stephane Nicoll
  */
 public class CachingOperationInvokerAdvisorTests {
 
@@ -66,8 +68,9 @@ public class CachingOperationInvokerAdvisorTests {
 	}
 
 	@Test
-	public void applyWhenHasParametersShouldNotAddAdvise() {
-		OperationParameters parameters = getParameters("getWithParameter", String.class);
+	public void applyWhenHasAtLeaseOneMandatoryParameterShouldNotAddAdvise() {
+		OperationParameters parameters = getParameters("getWithParameter", String.class,
+				String.class);
 		OperationInvoker advised = this.advisor.apply("foo", OperationType.READ,
 				parameters, this.invoker);
 		assertThat(advised).isSameAs(this.invoker);
@@ -97,6 +100,18 @@ public class CachingOperationInvokerAdvisorTests {
 	public void applyShouldAddCacheAdvise() {
 		OperationParameters parameters = getParameters("get");
 		given(this.timeToLive.apply(any())).willReturn(100L);
+		assertAdviseIsApplied(parameters);
+	}
+
+	@Test
+	public void applyWithAllOptionalParameterShouldAddAdvise() {
+		OperationParameters parameters = getParameters("getWithAllOptionalParameters",
+				String.class, String.class);
+		given(this.timeToLive.apply(any())).willReturn(100L);
+		assertAdviseIsApplied(parameters);
+	}
+
+	private void assertAdviseIsApplied(OperationParameters parameters) {
 		OperationInvoker advised = this.advisor.apply("foo", OperationType.READ,
 				parameters, this.invoker);
 		assertThat(advised).isInstanceOf(CachingOperationInvoker.class);
@@ -123,7 +138,12 @@ public class CachingOperationInvokerAdvisorTests {
 			return "";
 		}
 
-		public String getWithParameter(String foo) {
+		public String getWithParameter(@Nullable String foo, String bar) {
+			return "";
+		}
+
+		public String getWithAllOptionalParameters(@Nullable String foo,
+				@Nullable String bar) {
 			return "";
 		}
 

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.boot.actuate.endpoint.invoker.cache;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -50,10 +51,21 @@ public class CachingOperationInvokerTests {
 	}
 
 	@Test
-	public void cacheInTtlRange() {
-		Object expected = new Object();
-		OperationInvoker target = mock(OperationInvoker.class);
+	public void cacheInTtlRangeWithNoParameter() {
+		assertCacheIsUsed(Collections.emptyMap());
+	}
+
+	@Test
+	public void cacheInTtlWithNullParameters() {
 		Map<String, Object> parameters = new HashMap<>();
+		parameters.put("first", null);
+		parameters.put("second", null);
+		assertCacheIsUsed(parameters);
+	}
+
+	private void assertCacheIsUsed(Map<String, Object> parameters) {
+		OperationInvoker target = mock(OperationInvoker.class);
+		Object expected = new Object();
 		given(target.invoke(parameters)).willReturn(expected);
 		CachingOperationInvoker invoker = new CachingOperationInvoker(target, 500L);
 		Object response = invoker.invoke(parameters);
@@ -64,6 +76,20 @@ public class CachingOperationInvokerTests {
 		verifyNoMoreInteractions(target);
 	}
 
+	@Test
+	public void targetAlwaysInvokedWithArguments() {
+		OperationInvoker target = mock(OperationInvoker.class);
+		Map<String, Object> parameters = new HashMap<>();
+		parameters.put("test", "value");
+		parameters.put("something", null);
+		given(target.invoke(parameters)).willReturn(new Object());
+		CachingOperationInvoker invoker = new CachingOperationInvoker(target, 500L);
+		invoker.invoke(parameters);
+		invoker.invoke(parameters);
+		invoker.invoke(parameters);
+		verify(target, times(3)).invoke(parameters);
+	}
+
 	@Test
 	public void targetInvokedWhenCacheExpires() throws InterruptedException {
 		OperationInvoker target = mock(OperationInvoker.class);