From c849a0aba1a1f784a8a890244e06e40b2ff16328 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 19 Mar 2015 16:23:04 +0000 Subject: [PATCH] Cache /health response irrespective of sensitivity and security Previously, the response from /health was not cached if the request was secure, i.e. the user has authenticated, or the endpoint was configured as not being sensitive. The commit updates HealthMvcEndpoint to apply the caching logic all the time. Users that do not want caching can disable it by configuring the TTL with a value of zero. Closes gh-2630 --- .../endpoint/mvc/HealthMvcEndpoint.java | 16 +++++++----- .../endpoint/mvc/HealthMvcEndpointTests.java | 26 ++++--------------- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java index 4ce1e184305..86bcbe64dfc 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -137,11 +137,11 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { private Health getHealth(Principal principal) { long accessTime = System.currentTimeMillis(); - if (isCacheStale(accessTime) || isSecure(principal) || isUnrestricted()) { + if (isCacheStale(accessTime)) { this.lastAccess = accessTime; this.cached = this.delegate.invoke(); } - if (isSecure(principal) || isUnrestricted()) { + if (exposeHealthDetails(principal)) { return this.cached; } return Health.status(this.cached.getStatus()).build(); @@ -154,9 +154,8 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { return (accessTime - this.lastAccess) > this.delegate.getTimeToLive(); } - private boolean isUnrestricted() { - Boolean sensitive = this.propertyResolver.getProperty("sensitive", Boolean.class); - return !this.secure || Boolean.FALSE.equals(sensitive); + private boolean exposeHealthDetails(Principal principal) { + return isSecure(principal) || isUnrestricted(); } private boolean isSecure(Principal principal) { @@ -164,6 +163,11 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { .contains("Anonymous")); } + private boolean isUnrestricted() { + Boolean sensitive = this.propertyResolver.getProperty("sensitive", Boolean.class); + return !this.secure || Boolean.FALSE.equals(sensitive); + } + @Override public String getPath() { return "/" + this.delegate.getId(); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java index 8bc937ffb62..05a2061b2df 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -44,6 +44,7 @@ import static org.mockito.Mockito.mock; * * @author Christian Dupuis * @author Dave Syer + * @author Andy Wilkinson */ public class HealthMvcEndpointTests { @@ -115,23 +116,7 @@ public class HealthMvcEndpointTests { } @Test - public void secureNotCached() { - given(this.endpoint.getTimeToLive()).willReturn(10000L); - given(this.endpoint.isSensitive()).willReturn(false); - given(this.endpoint.invoke()).willReturn( - new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.user); - assertTrue(result instanceof Health); - assertTrue(((Health) result).getStatus() == Status.UP); - given(this.endpoint.invoke()).willReturn(new Health.Builder().down().build()); - result = this.mvc.invoke(this.user); - @SuppressWarnings("unchecked") - Health health = ((ResponseEntity) result).getBody(); - assertTrue(health.getStatus() == Status.DOWN); - } - - @Test - public void unsecureCached() { + public void healthIsCached() { given(this.endpoint.getTimeToLive()).willReturn(10000L); given(this.endpoint.isSensitive()).willReturn(true); given(this.endpoint.invoke()).willReturn( @@ -164,9 +149,8 @@ public class HealthMvcEndpointTests { } @Test - public void unsecureIsNotCachedWhenAnonymousAccessIsUnrestricted() { - this.environment.getPropertySources().addLast(NON_SENSITIVE); - given(this.endpoint.getTimeToLive()).willReturn(10000L); + public void noCachingWhenTimeToLiveIsZero() { + given(this.endpoint.getTimeToLive()).willReturn(0L); given(this.endpoint.invoke()).willReturn( new Health.Builder().up().withDetail("foo", "bar").build()); Object result = this.mvc.invoke(null);