Merge pull request #18961 from dreis2211

* pr/18961: Handle ApiVersion in CachingOperationInvoker Closes gh-18961
spring-projects · Nov 21, 2019 · 0b4dcf9 · 0b4dcf9
2 parents 81c0d6a + 0bdcd2e
commit 0b4dcf9
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 4 deletions.
diff --git a/...java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java b/...java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java
@@ -19,11 +19,13 @@
 import java.time.Duration;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
 import org.springframework.boot.actuate.endpoint.InvocationContext;
+import org.springframework.boot.actuate.endpoint.http.ApiVersion;
 import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
@@ -46,7 +48,7 @@ public class CachingOperationInvoker implements OperationInvoker {
 
 	private final long timeToLive;
 
-	private volatile CachedResponse cachedResponse;
+	private final Map<ApiVersion, CachedResponse> cachedResponses;
 
 	/**
 	 * Create a new instance with the target {@link OperationInvoker} to use to compute
@@ -58,6 +60,7 @@ public class CachingOperationInvoker implements OperationInvoker {
 		Assert.isTrue(timeToLive > 0, "TimeToLive must be strictly positive");
 		this.invoker = invoker;
 		this.timeToLive = timeToLive;
+		this.cachedResponses = new ConcurrentHashMap<>();
 	}
 
 	/**
@@ -74,11 +77,12 @@ public Object invoke(InvocationContext context) {
 			return this.invoker.invoke(context);
 		}
 		long accessTime = System.currentTimeMillis();
-		CachedResponse cached = this.cachedResponse;
+		ApiVersion contextApiVersion = context.getApiVersion();
+		CachedResponse cached = this.cachedResponses.get(contextApiVersion);
 		if (cached == null || cached.isStale(accessTime, this.timeToLive)) {
 			Object response = this.invoker.invoke(context);
 			cached = createCachedResponse(response, accessTime);
-			this.cachedResponse = cached;
+			this.cachedResponses.put(contextApiVersion, cached);
 		}
 		return cached.getResponse();
 	}

diff --git a/...g/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java b/...g/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java
@@ -21,6 +21,7 @@
 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.http.ApiVersion;
 import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
 import org.springframework.boot.actuate.endpoint.invoke.OperationInvokerAdvisor;
 import org.springframework.boot.actuate.endpoint.invoke.OperationParameter;
@@ -54,7 +55,8 @@ public OperationInvoker apply(EndpointId endpointId, OperationType operationType
 
 	private boolean hasMandatoryParameter(OperationParameters parameters) {
 		for (OperationParameter parameter : parameters) {
-			if (parameter.isMandatory() && !SecurityContext.class.isAssignableFrom(parameter.getType())) {
+			if (parameter.isMandatory() && !ApiVersion.class.isAssignableFrom(parameter.getType())
+					&& !SecurityContext.class.isAssignableFrom(parameter.getType())) {
 				return true;
 			}
 		}

diff --git a/...ingframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisorTests.java b/...ingframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisorTests.java
@@ -27,6 +27,7 @@
 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.http.ApiVersion;
 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;
@@ -117,6 +118,13 @@ void applyWithSecurityContextShouldAddAdvise() {
 		assertAdviseIsApplied(parameters);
 	}
 
+	@Test
+	void applyWithApiVersionShouldAddAdvise() {
+		OperationParameters parameters = getParameters("getWithApiVersion", ApiVersion.class, String.class);
+		given(this.timeToLive.apply(any())).willReturn(100L);
+		assertAdviseIsApplied(parameters);
+	}
+
 	private void assertAdviseIsApplied(OperationParameters parameters) {
 		OperationInvoker advised = this.advisor.apply(EndpointId.of("foo"), OperationType.READ, parameters,
 				this.invoker);
@@ -152,6 +160,10 @@ String getWithSecurityContext(SecurityContext securityContext, @Nullable String
 			return "";
 		}
 
+		String getWithApiVersion(ApiVersion apiVersion, @Nullable 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
@@ -28,6 +28,7 @@
 
 import org.springframework.boot.actuate.endpoint.InvocationContext;
 import org.springframework.boot.actuate.endpoint.SecurityContext;
+import org.springframework.boot.actuate.endpoint.http.ApiVersion;
 import org.springframework.boot.actuate.endpoint.invoke.MissingParametersException;
 import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
 
@@ -152,6 +153,26 @@ void targetInvokedWhenCacheExpires() throws InterruptedException {
 		verify(target, times(2)).invoke(context);
 	}
 
+	@Test
+	void targetInvokedWithDifferentApiVersion() {
+		OperationInvoker target = mock(OperationInvoker.class);
+		Object expectedV2 = new Object();
+		Object expectedV3 = new Object();
+		InvocationContext contextV2 = new InvocationContext(ApiVersion.V2, mock(SecurityContext.class),
+				Collections.emptyMap());
+		InvocationContext contextV3 = new InvocationContext(ApiVersion.V3, mock(SecurityContext.class),
+				Collections.emptyMap());
+		given(target.invoke(contextV2)).willReturn(expectedV2);
+		given(target.invoke(contextV3)).willReturn(expectedV3);
+		CachingOperationInvoker invoker = new CachingOperationInvoker(target, 500L);
+		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 int invocations;