Handle ApiVersion in CachingOperationInvoker
Prior to this commit, ApiVersion was treated as a mandatory parameter in CachingOperationInvokerAdvisor and thus prevented the CachingOperationInvoker to kick in. By skipping ApiVersion in the same way we're skipping SecurityContext we can avoid this. In order to not return the same cached response, this commit also changes the cache handling in CachingOperationInvoker to account for different ApiVersions being passed. See gh-18961
This commit is contained in:
parent
81c0d6a5f7
commit
0bdcd2ee67
|
@ -19,11 +19,13 @@ package org.springframework.boot.actuate.endpoint.invoker.cache;
|
|||
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 class CachingOperationInvoker implements OperationInvoker {
|
|||
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();
|
||||
}
|
||||
|
|
|
@ -21,6 +21,7 @@ import java.util.function.Function;
|
|||
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 class CachingOperationInvokerAdvisor implements OperationInvokerAdvisor {
|
|||
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -27,6 +27,7 @@ import org.mockito.MockitoAnnotations;
|
|||
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 @@ class CachingOperationInvokerAdvisorTests {
|
|||
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 @@ class CachingOperationInvokerAdvisorTests {
|
|||
return "";
|
||||
}
|
||||
|
||||
String getWithApiVersion(ApiVersion apiVersion, @Nullable String bar) {
|
||||
return "";
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -28,6 +28,7 @@ import reactor.core.publisher.Mono;
|
|||
|
||||
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 @@ class CachingOperationInvokerTests {
|
|||
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;
|
||||
|
|
Loading…
Reference in New Issue