Fallback to ping if Solr URL references core

Update `SolrHealthIndicator` to fallback to a basic ping operation if
the `baseUrl` references a particular core rather than the root context.

Prior to this commit, if the Solr `baseUrl` pointed to a particular
core then the health indicator would incorrectly report `DOWN`.

See gh-16477

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/solr/SolrHealthIndicator.java

@@ -16,7 +16,11 @@
 
 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.request.CoreAdminRequest;
 import org.apache.solr.client.solrj.response.CoreAdminResponse;
 import org.apache.solr.common.params.CoreAdminParams;
@@ -25,31 +29,87 @@ 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.
  *
  * @author Andy Wilkinson
  * @author Stephane Nicoll
+ * @author Markus Schuch
  * @since 2.0.0
  */
 public class SolrHealthIndicator extends AbstractHealthIndicator {
 
 	private final SolrClient solrClient;
 
+	private PathType detectedPathType = PathType.UNKNOWN;
+
 	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;
+				}
+			}
+		}
+		Status status = (statusCode != 0) ? Status.DOWN : Status.UP;
+		builder.status(status).withDetail("status", statusCode).withDetail("detectedPathType",
+				this.detectedPathType.toString());
+	}
+
+	private int doCoreAdminCheck() throws IOException, SolrServerException {
 		CoreAdminRequest request = new CoreAdminRequest();
 		request.setAction(CoreAdminParams.CoreAdminAction.STATUS);
 		CoreAdminResponse response = request.process(this.solrClient);
-		int statusCode = response.getStatus();
+		return response.getStatus();
-		Status status = (statusCode != 0) ? Status.DOWN : Status.UP;
+	}
-		builder.status(status).withDetail("status", statusCode);
+
+	private int doPingCheck() throws IOException, SolrServerException {
+		return this.solrClient.ping().getStatus();
+	}
+
+	enum PathType {
+
+		ROOT, PARTICULAR_CORE, UNKNOWN
+
 	}
 
 }

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/solr/SolrHealthIndicatorTests.java

@@ -19,7 +19,9 @@ 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.request.CoreAdminRequest;
+import org.apache.solr.client.solrj.response.SolrPingResponse;
 import org.apache.solr.common.util.NamedList;
 import org.junit.After;
 import org.junit.Test;
@@ -33,6 +35,9 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 /**
  * Tests for {@link SolrHealthIndicator}
@@ -51,23 +56,79 @@ public class SolrHealthIndicatorTests {
 	}
 
 	@Test
-	public void solrIsUp() throws Exception {
+	public void solrIsUpWithBaseUrlPointsToRoot() 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());
+		verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
+		verifyNoMoreInteractions(solrClient);
 	}
 
 	@Test
-	public void solrIsUpAndRequestFailed() throws Exception {
+	public void solrIsUpWithBaseUrlPointsToParticularCore() 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(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());
+		verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
+		verify(solrClient, times(1)).ping();
+		verifyNoMoreInteractions(solrClient);
+	}
+
+	@Test
+	public void pathTypeIsRememberedForConsecutiveChecks() 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(0));
+		SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient);
+		healthIndicator.health();
+		verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
+		verify(solrClient, times(1)).ping();
+		verifyNoMoreInteractions(solrClient);
+		healthIndicator.health();
+		verify(solrClient, times(2)).ping();
+		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);
 		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
@@ -79,6 +140,8 @@ public class SolrHealthIndicatorTests {
 		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);
 	}
 
 	private NamedList<Object> mockResponse(int status) {
@@ -89,4 +152,10 @@ public class SolrHealthIndicatorTests {
 		return response;
 	}
 
+	private SolrPingResponse mockPingResponse(int status) {
+		SolrPingResponse pingResponse = new SolrPingResponse();
+		pingResponse.setResponse(mockResponse(status));
+		return pingResponse;
+	}
+
 }