diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/solr/SolrHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/solr/SolrHealthIndicator.java index fe6a56b1cd5..f9edd53f461 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/solr/SolrHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/solr/SolrHealthIndicator.java @@ -16,20 +16,15 @@ package org.springframework.boot.actuate.solr; -import java.io.IOException; - import org.apache.solr.client.solrj.SolrClient; -import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException; import org.apache.solr.client.solrj.request.CoreAdminRequest; -import org.apache.solr.client.solrj.response.CoreAdminResponse; import org.apache.solr.common.params.CoreAdminParams; import org.springframework.boot.actuate.health.AbstractHealthIndicator; import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.boot.actuate.health.Status; -import org.springframework.http.HttpStatus; /** * {@link HealthIndicator} for Apache Solr. @@ -37,78 +32,104 @@ import org.springframework.http.HttpStatus; * @author Andy Wilkinson * @author Stephane Nicoll * @author Markus Schuch + * @author Phillip Webb * @since 2.0.0 */ public class SolrHealthIndicator extends AbstractHealthIndicator { + private static final int HTTP_NOT_FOUND_STATUS = 404; + private final SolrClient solrClient; - private PathType detectedPathType = PathType.UNKNOWN; + private volatile StatusCheck statusCheck; public SolrHealthIndicator(SolrClient solrClient) { super("Solr health check failed"); this.solrClient = solrClient; } + @Override protected void doHealthCheck(Health.Builder builder) throws Exception { - int statusCode; - - if (this.detectedPathType == PathType.ROOT) { - statusCode = doCoreAdminCheck(); - } - else if (this.detectedPathType == PathType.PARTICULAR_CORE) { - statusCode = doPingCheck(); - } - else { - // We do not know yet, which is the promising - // health check strategy, so we start trying with - // a CoreAdmin check, which is the common case - try { - statusCode = doCoreAdminCheck(); - // When the CoreAdmin request returns with a - // valid response, we can assume that this - // SolrClient is configured with a baseUrl - // pointing to the root path of the Solr instance - this.detectedPathType = PathType.ROOT; - } - catch (HttpSolrClient.RemoteSolrException ex) { - // CoreAdmin requests to not work with - // SolrClients configured with a baseUrl pointing to - // a particular core and a 404 response indicates - // that this might be the case. - if (ex.code() == HttpStatus.NOT_FOUND.value()) { - statusCode = doPingCheck(); - // When the SolrPing returns with a valid - // response, we can assume that the baseUrl - // of this SolrClient points to a particular core - this.detectedPathType = PathType.PARTICULAR_CORE; - } - else { - // Rethrow every other response code leaving us - // in the dark about the type of the baseUrl - throw ex; - } - } - } + int statusCode = initializeStatusCheck(); Status status = (statusCode != 0) ? Status.DOWN : Status.UP; builder.status(status).withDetail("status", statusCode).withDetail("detectedPathType", - this.detectedPathType.toString()); + this.statusCheck.getPathType()); } - private int doCoreAdminCheck() throws IOException, SolrServerException { - CoreAdminRequest request = new CoreAdminRequest(); - request.setAction(CoreAdminParams.CoreAdminAction.STATUS); - CoreAdminResponse response = request.process(this.solrClient); - return response.getStatus(); + private int initializeStatusCheck() throws Exception { + StatusCheck statusCheck = this.statusCheck; + if (statusCheck != null) { + // Already initilized + return statusCheck.getStatus(this.solrClient); + } + try { + return initializeStatusCheck(new RootStatusCheck()); + } + catch (RemoteSolrException ex) { + // 404 is thrown when SolrClient has a baseUrl pointing to a particular core. + if (ex.code() == HTTP_NOT_FOUND_STATUS) { + return initializeStatusCheck(new ParticularCoreStatusCheck()); + } + throw ex; + } } - private int doPingCheck() throws IOException, SolrServerException { - return this.solrClient.ping().getStatus(); + private int initializeStatusCheck(StatusCheck statusCheck) throws Exception { + int result = statusCheck.getStatus(this.solrClient); + this.statusCheck = statusCheck; + return result; } - enum PathType { + /** + * Strategy used to perform the status check. + */ + private abstract static class StatusCheck { - ROOT, PARTICULAR_CORE, UNKNOWN + private final String pathType; + + StatusCheck(String pathType) { + this.pathType = pathType; + } + + abstract int getStatus(SolrClient client) throws Exception; + + String getPathType() { + return this.pathType; + } + + } + + /** + * {@link StatusCheck} used when {@code baseUrl} points to the root context. + */ + private static class RootStatusCheck extends StatusCheck { + + RootStatusCheck() { + super("root"); + } + + @Override + public int getStatus(SolrClient client) throws Exception { + CoreAdminRequest request = new CoreAdminRequest(); + request.setAction(CoreAdminParams.CoreAdminAction.STATUS); + return request.process(client).getStatus(); + } + + } + + /** + * {@link StatusCheck} used when {@code baseUrl} points to the particular core. + */ + private static class ParticularCoreStatusCheck extends StatusCheck { + + ParticularCoreStatusCheck() { + super("particular core"); + } + + @Override + public int getStatus(SolrClient client) throws Exception { + return client.ping().getStatus(); + } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/solr/SolrHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/solr/SolrHealthIndicatorTests.java index 5a157880e91..89e87c8bc6f 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/solr/SolrHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/solr/SolrHealthIndicatorTests.java @@ -19,7 +19,7 @@ package org.springframework.boot.actuate.solr; import java.io.IOException; import org.apache.solr.client.solrj.SolrClient; -import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException; import org.apache.solr.client.solrj.request.CoreAdminRequest; import org.apache.solr.client.solrj.response.SolrPingResponse; import org.apache.solr.common.util.NamedList; @@ -43,6 +43,8 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; * Tests for {@link SolrHealthIndicator} * * @author Andy Wilkinson + * @author Markus Schuch + * @author Phillip Webb */ public class SolrHealthIndicatorTests { @@ -56,40 +58,69 @@ public class SolrHealthIndicatorTests { } @Test - public void solrIsUpWithBaseUrlPointsToRoot() throws Exception { + public void healthWhenSolrStatusUpAndBaseUrlPointsToRootReturnsUp() throws Exception { SolrClient solrClient = mock(SolrClient.class); given(solrClient.request(any(CoreAdminRequest.class), isNull())).willReturn(mockResponse(0)); SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); - Health health = healthIndicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.UP); - assertThat(health.getDetails().get("status")).isEqualTo(0); - assertThat(health.getDetails().get("detectedPathType")).isEqualTo(SolrHealthIndicator.PathType.ROOT.toString()); + assertHealth(healthIndicator, Status.UP, 0, "root"); verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull()); verifyNoMoreInteractions(solrClient); } @Test - public void solrIsUpWithBaseUrlPointsToParticularCore() throws Exception { + public void healthWhenSolrStatusDownAndBaseUrlPointsToRootReturnsDown() throws Exception { + SolrClient solrClient = mock(SolrClient.class); + given(solrClient.request(any(CoreAdminRequest.class), isNull())).willReturn(mockResponse(400)); + SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); + assertHealth(healthIndicator, Status.DOWN, 400, "root"); + verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull()); + verifyNoMoreInteractions(solrClient); + } + + @Test + public void healthWhenSolrStatusUpAndBaseUrlPointsToParticularCoreReturnsUp() throws Exception { SolrClient solrClient = mock(SolrClient.class); given(solrClient.request(any(CoreAdminRequest.class), isNull())) - .willThrow(new HttpSolrClient.RemoteSolrException("mock", 404, "", null)); + .willThrow(new RemoteSolrException("mock", 404, "", null)); given(solrClient.ping()).willReturn(mockPingResponse(0)); SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); - Health health = healthIndicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.UP); - assertThat(health.getDetails().get("status")).isEqualTo(0); - assertThat(health.getDetails().get("detectedPathType")) - .isEqualTo(SolrHealthIndicator.PathType.PARTICULAR_CORE.toString()); + assertHealth(healthIndicator, Status.UP, 0, "particular core"); verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull()); verify(solrClient, times(1)).ping(); verifyNoMoreInteractions(solrClient); } @Test - public void pathTypeIsRememberedForConsecutiveChecks() throws Exception { + public void healthWhenSolrStatusDownAndBaseUrlPointsToParticularCoreReturnsDown() throws Exception { SolrClient solrClient = mock(SolrClient.class); given(solrClient.request(any(CoreAdminRequest.class), isNull())) - .willThrow(new HttpSolrClient.RemoteSolrException("mock", 404, "", null)); + .willThrow(new RemoteSolrException("mock", 404, "", null)); + given(solrClient.ping()).willReturn(mockPingResponse(400)); + SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); + assertHealth(healthIndicator, Status.DOWN, 400, "particular core"); + verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull()); + verify(solrClient, times(1)).ping(); + verifyNoMoreInteractions(solrClient); + } + + @Test + public void healthWhenSolrConnectionFailsReturnsDown() throws Exception { + SolrClient solrClient = mock(SolrClient.class); + given(solrClient.request(any(CoreAdminRequest.class), isNull())) + .willThrow(new IOException("Connection failed")); + SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); + Health health = healthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat((String) health.getDetails().get("error")).contains("Connection failed"); + verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull()); + verifyNoMoreInteractions(solrClient); + } + + @Test + public void healthWhenMakingMultipleCallsRemembersStatusStrategy() throws Exception { + SolrClient solrClient = mock(SolrClient.class); + given(solrClient.request(any(CoreAdminRequest.class), isNull())) + .willThrow(new RemoteSolrException("mock", 404, "", null)); given(solrClient.ping()).willReturn(mockPingResponse(0)); SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); healthIndicator.health(); @@ -101,47 +132,12 @@ public class SolrHealthIndicatorTests { verifyNoMoreInteractions(solrClient); } - @Test - public void solrIsUpAndRequestFailedWithBaseUrlPointsToRoot() throws Exception { - SolrClient solrClient = mock(SolrClient.class); - given(solrClient.request(any(CoreAdminRequest.class), isNull())).willReturn(mockResponse(400)); - SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); + private void assertHealth(SolrHealthIndicator healthIndicator, Status expectedStatus, int expectedStatusCode, + String expectedPathType) { Health health = healthIndicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.DOWN); - assertThat(health.getDetails().get("status")).isEqualTo(400); - assertThat(health.getDetails().get("detectedPathType")).isEqualTo(SolrHealthIndicator.PathType.ROOT.toString()); - verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull()); - verifyNoMoreInteractions(solrClient); - } - - @Test - public void solrIsUpAndRequestFailedWithBaseUrlPointsToParticularCore() throws Exception { - SolrClient solrClient = mock(SolrClient.class); - given(solrClient.request(any(CoreAdminRequest.class), isNull())) - .willThrow(new HttpSolrClient.RemoteSolrException("mock", 404, "", null)); - given(solrClient.ping()).willReturn(mockPingResponse(400)); - SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); - Health health = healthIndicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.DOWN); - assertThat(health.getDetails().get("status")).isEqualTo(400); - assertThat(health.getDetails().get("detectedPathType")) - .isEqualTo(SolrHealthIndicator.PathType.PARTICULAR_CORE.toString()); - verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull()); - verify(solrClient, times(1)).ping(); - verifyNoMoreInteractions(solrClient); - } - - @Test - public void solrIsDown() throws Exception { - SolrClient solrClient = mock(SolrClient.class); - given(solrClient.request(any(CoreAdminRequest.class), isNull())) - .willThrow(new IOException("Connection failed")); - SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient); - Health health = healthIndicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.DOWN); - assertThat((String) health.getDetails().get("error")).contains("Connection failed"); - verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull()); - verifyNoMoreInteractions(solrClient); + assertThat(health.getStatus()).isEqualTo(expectedStatus); + assertThat(health.getDetails().get("status")).isEqualTo(expectedStatusCode); + assertThat(health.getDetails().get("detectedPathType")).isEqualTo(expectedPathType); } private NamedList mockResponse(int status) {