diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/InvocationContext.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/InvocationContext.java index eefb3e3fdb..dc90f50110 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/InvocationContext.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/InvocationContext.java @@ -98,7 +98,6 @@ public class InvocationContext { @Deprecated public org.springframework.boot.actuate.endpoint.http.ApiVersion getApiVersion() { ApiVersion version = resolveArgument(ApiVersion.class); - System.out.println(version); return (version != null) ? org.springframework.boot.actuate.endpoint.http.ApiVersion.valueOf(version.name()) : org.springframework.boot.actuate.endpoint.http.ApiVersion.LATEST; } 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 00f8ca1605..cde7eb46a8 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 @@ -18,6 +18,7 @@ package org.springframework.boot.actuate.endpoint.invoker.cache; import java.security.Principal; import java.time.Duration; +import java.util.Arrays; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; @@ -28,7 +29,11 @@ import reactor.core.publisher.Mono; import org.springframework.boot.actuate.endpoint.ApiVersion; import org.springframework.boot.actuate.endpoint.InvocationContext; +import org.springframework.boot.actuate.endpoint.SecurityContext; import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker; +import org.springframework.boot.actuate.endpoint.invoke.OperationParameter; +import org.springframework.boot.actuate.endpoint.invoke.OperationParameters; +import org.springframework.boot.actuate.endpoint.web.WebServerNamespace; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ConcurrentReferenceHashMap; @@ -85,9 +90,7 @@ public class CachingOperationInvoker implements OperationInvoker { if (this.cachedResponses.size() > CACHE_CLEANUP_THRESHOLD) { cleanExpiredCachedResponses(accessTime); } - ApiVersion contextApiVersion = context.resolveArgument(ApiVersion.class); - Principal principal = context.resolveArgument(Principal.class); - CacheKey cacheKey = new CacheKey(contextApiVersion, principal); + CacheKey cacheKey = getCacheKey(context); CachedResponse cached = this.cachedResponses.get(cacheKey); if (cached == null || cached.isStale(accessTime, this.timeToLive)) { Object response = this.invoker.invoke(context); @@ -97,6 +100,13 @@ public class CachingOperationInvoker implements OperationInvoker { return cached.getResponse(); } + private CacheKey getCacheKey(InvocationContext context) { + ApiVersion contextApiVersion = context.resolveArgument(ApiVersion.class); + Principal principal = context.resolveArgument(Principal.class); + WebServerNamespace serverNamespace = context.resolveArgument(WebServerNamespace.class); + return new CacheKey(contextApiVersion, principal, serverNamespace); + } + private void cleanExpiredCachedResponses(long accessTime) { try { Iterator> iterator = this.cachedResponses.entrySet().iterator(); @@ -126,6 +136,15 @@ public class CachingOperationInvoker implements OperationInvoker { return new CachedResponse(response, accessTime); } + static boolean isApplicable(OperationParameters parameters) { + for (OperationParameter parameter : parameters) { + if (parameter.isMandatory() && !CacheKey.containsType(parameter.getType())) { + return false; + } + } + return true; + } + /** * A cached response that encapsulates the response itself and the time at which it * was created. @@ -174,13 +193,23 @@ public class CachingOperationInvoker implements OperationInvoker { private static final class CacheKey { + public static final Class[] CACHEABLE_TYPES = new Class[] { ApiVersion.class, SecurityContext.class, + WebServerNamespace.class }; + private final ApiVersion apiVersion; private final Principal principal; - private CacheKey(ApiVersion apiVersion, Principal principal) { + private final WebServerNamespace serverNamespace; + + private CacheKey(ApiVersion apiVersion, Principal principal, WebServerNamespace serverNamespace) { this.principal = principal; this.apiVersion = apiVersion; + this.serverNamespace = serverNamespace; + } + + public static boolean containsType(Class type) { + return Arrays.stream(CacheKey.CACHEABLE_TYPES).anyMatch((c) -> c.isAssignableFrom(type)); } @Override @@ -193,7 +222,8 @@ public class CachingOperationInvoker implements OperationInvoker { } CacheKey other = (CacheKey) obj; return this.apiVersion.equals(other.apiVersion) - && ObjectUtils.nullSafeEquals(this.principal, other.principal); + && ObjectUtils.nullSafeEquals(this.principal, other.principal) + && ObjectUtils.nullSafeEquals(this.serverNamespace, other.serverNamespace); } @Override @@ -202,6 +232,7 @@ public class CachingOperationInvoker implements OperationInvoker { int result = 1; result = prime * result + this.apiVersion.hashCode(); result = prime * result + ObjectUtils.nullSafeHashCode(this.principal); + result = prime * result + ObjectUtils.nullSafeHashCode(this.serverNamespace); return result; } 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 51cab56b76..47c801609f 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 @@ -18,13 +18,10 @@ package org.springframework.boot.actuate.endpoint.invoker.cache; import java.util.function.Function; -import org.springframework.boot.actuate.endpoint.ApiVersion; import org.springframework.boot.actuate.endpoint.EndpointId; import org.springframework.boot.actuate.endpoint.OperationType; -import org.springframework.boot.actuate.endpoint.SecurityContext; 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; /** @@ -44,7 +41,7 @@ public class CachingOperationInvokerAdvisor implements OperationInvokerAdvisor { @Override public OperationInvoker apply(EndpointId endpointId, OperationType operationType, OperationParameters parameters, OperationInvoker invoker) { - if (operationType == OperationType.READ && !hasMandatoryParameter(parameters)) { + if (operationType == OperationType.READ && CachingOperationInvoker.isApplicable(parameters)) { Long timeToLive = this.endpointIdTimeToLive.apply(endpointId); if (timeToLive != null && timeToLive > 0) { return new CachingOperationInvoker(invoker, timeToLive); @@ -53,14 +50,4 @@ public class CachingOperationInvokerAdvisor implements OperationInvokerAdvisor { return invoker; } - private boolean hasMandatoryParameter(OperationParameters parameters) { - for (OperationParameter parameter : parameters) { - if (parameter.isMandatory() && !ApiVersion.class.isAssignableFrom(parameter.getType()) - && !SecurityContext.class.isAssignableFrom(parameter.getType())) { - 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 cc1ba56efa..e25d26f089 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 @@ -32,6 +32,7 @@ import org.springframework.boot.actuate.endpoint.SecurityContext; 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.boot.actuate.endpoint.web.WebServerNamespace; import org.springframework.lang.Nullable; import org.springframework.util.ReflectionUtils; @@ -126,6 +127,23 @@ class CachingOperationInvokerAdvisorTests { assertAdviseIsApplied(parameters); } + @Test + void applyWithWebServerNamespaceShouldAddAdvise() { + OperationParameters parameters = getParameters("getWithServerNamespace", WebServerNamespace.class, + String.class); + given(this.timeToLive.apply(any())).willReturn(100L); + assertAdviseIsApplied(parameters); + } + + @Test + void applyWithMandatoryCachedAndNonCachedShouldAddAdvise() { + OperationParameters parameters = getParameters("getWithServerNamespaceAndOtherMandatory", + WebServerNamespace.class, String.class); + OperationInvoker advised = this.advisor.apply(EndpointId.of("foo"), OperationType.READ, parameters, + this.invoker); + assertThat(advised).isSameAs(this.invoker); + } + private void assertAdviseIsApplied(OperationParameters parameters) { OperationInvoker advised = this.advisor.apply(EndpointId.of("foo"), OperationType.READ, parameters, this.invoker); @@ -165,6 +183,14 @@ class CachingOperationInvokerAdvisorTests { return ""; } + String getWithServerNamespace(WebServerNamespace serverNamespace, @Nullable String bar) { + return ""; + } + + String getWithServerNamespaceAndOtherMandatory(WebServerNamespace serverNamespace, 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 9141d123a1..d6cddec645 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 @@ -33,6 +33,7 @@ import org.springframework.boot.actuate.endpoint.InvocationContext; import org.springframework.boot.actuate.endpoint.OperationArgumentResolver; import org.springframework.boot.actuate.endpoint.SecurityContext; import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker; +import org.springframework.boot.actuate.endpoint.web.WebServerNamespace; import static org.assertj.core.api.Assertions.as; import static org.assertj.core.api.Assertions.assertThat; @@ -241,6 +242,26 @@ class CachingOperationInvokerTests { verify(target, times(1)).invoke(contextV3); } + @Test + public void targetInvokedWithDifferentWebServerNamespace() { + OperationInvoker target = mock(OperationInvoker.class); + Object expectedV2 = new Object(); + Object expectedV3 = new Object(); + InvocationContext contextV2 = new InvocationContext(mock(SecurityContext.class), Collections.emptyMap(), + new WebServerNamespaceArgumentResolver(WebServerNamespace.SERVER)); + InvocationContext contextV3 = new InvocationContext(mock(SecurityContext.class), Collections.emptyMap(), + new WebServerNamespaceArgumentResolver(WebServerNamespace.MANAGEMENT)); + given(target.invoke(contextV2)).willReturn(expectedV2); + given(target.invoke(contextV3)).willReturn(expectedV3); + CachingOperationInvoker invoker = new CachingOperationInvoker(target, CACHE_TTL); + Object response = invoker.invoke(contextV2); + assertThat(response).isSameAs(expectedV2); + verify(target, times(1)).invoke(contextV2); + Object cachedResponse = invoker.invoke(contextV3); + assertThat(cachedResponse).isNotSameAs(response); + verify(target, times(1)).invoke(contextV3); + } + private static class MonoOperationInvoker implements OperationInvoker { static AtomicInteger invocations = new AtomicInteger(); @@ -287,4 +308,25 @@ class CachingOperationInvokerTests { } + private static final class WebServerNamespaceArgumentResolver implements OperationArgumentResolver { + + private final WebServerNamespace webServerNamespace; + + private WebServerNamespaceArgumentResolver(WebServerNamespace webServerNamespace) { + this.webServerNamespace = webServerNamespace; + } + + @SuppressWarnings("unchecked") + @Override + public T resolve(Class type) { + return (T) this.webServerNamespace; + } + + @Override + public boolean canResolve(Class type) { + return WebServerNamespace.class.equals(type); + } + + } + }