Handle WebServerNamespace in CachingOperationInvoker

Fixes gh-28882
spring-projects · Dec 18, 2021 · 4cc8012 · 4cc8012
1 parent d9d161c
commit 4cc8012
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 20 deletions.
diff --git a/...t-actuator/src/main/java/org/springframework/boot/actuate/endpoint/InvocationContext.java b/...t-actuator/src/main/java/org/springframework/boot/actuate/endpoint/InvocationContext.java
@@ -98,7 +98,6 @@ public InvocationContext(SecurityContext securityContext, Map<String, Object> ar
 	@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/...java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java b/...java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java
@@ -18,6 +18,7 @@
 
 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 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 Object invoke(InvocationContext context) {
 		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 Object invoke(InvocationContext context) {
 		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<Entry<CacheKey, CachedResponse>> iterator = this.cachedResponses.entrySet().iterator();
@@ -126,6 +136,15 @@ private CachedResponse createCachedResponse(Object response, long accessTime) {
 		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 @@ private static Object applyCaching(Object response, long timeToLive) {
 
 	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 boolean equals(Object obj) {
 			}
 			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 int hashCode() {
 			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/...g/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java b/...g/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java
@@ -18,13 +18,10 @@
 
 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 CachingOperationInvokerAdvisor(Function<EndpointId, Long> endpointIdTimeT
 	@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 OperationInvoker apply(EndpointId endpointId, OperationType operationType
 		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/...ingframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisorTests.java b/...ingframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisorTests.java
@@ -32,6 +32,7 @@
 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 @@ void applyWithApiVersionShouldAddAdvise() {
 		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 @@ String getWithApiVersion(ApiVersion apiVersion, @Nullable String bar) {
 			return "";
 		}
 
+		String getWithServerNamespace(WebServerNamespace serverNamespace, @Nullable String bar) {
+			return "";
+		}
+
+		String getWithServerNamespaceAndOtherMandatory(WebServerNamespace serverNamespace, String bar) {
+			return "";
+		}
+
 	}
 
 }
diff --git a/...org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerTests.java b/...org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerTests.java
@@ -33,6 +33,7 @@
 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.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -241,6 +242,26 @@ void targetInvokedWithDifferentApiVersion() {
 		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 @@ public boolean canResolve(Class<?> type) {
 
 	}
 
+	private static final class WebServerNamespaceArgumentResolver implements OperationArgumentResolver {
+
+		private final WebServerNamespace webServerNamespace;
+
+		private WebServerNamespaceArgumentResolver(WebServerNamespace webServerNamespace) {
+			this.webServerNamespace = webServerNamespace;
+		}
+
+		@SuppressWarnings("unchecked")
+		@Override
+		public <T> T resolve(Class<T> type) {
+			return (T) this.webServerNamespace;
+		}
+
+		@Override
+		public boolean canResolve(Class<?> type) {
+			return WebServerNamespace.class.equals(type);
+		}
+
+	}
+
 }