diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java index 96960b75a48..c284f940e85 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java +++ b/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 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 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 diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java index b0f02f6095c..639930472dd 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java +++ b/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; + } + } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisorTests.java index cbedccee0af..6811ecf3eae 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisorTests.java +++ b/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 ""; } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerTests.java index dbfe0eb052d..d8a37c228c5 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerTests.java +++ b/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 parameters = new HashMap<>(); + parameters.put("first", null); + parameters.put("second", null); + assertCacheIsUsed(parameters); + } + + private void assertCacheIsUsed(Map 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 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);