Merge pull request #16477 from schuch

* pr/16477:
  Polish "Fallback to ping if Solr URL references core"
  Fallback to ping if Solr URL references core

Closes gh-16477

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

@@ -17,8 +17,8 @@
 package org.springframework.boot.actuate.solr;
 
 import org.apache.solr.client.solrj.SolrClient;
+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;
@@ -31,12 +31,18 @@ import org.springframework.boot.actuate.health.Status;
  *
  * @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 volatile StatusCheck statusCheck;
+
 	public SolrHealthIndicator(SolrClient solrClient) {
 		super("Solr health check failed");
 		this.solrClient = solrClient;
@@ -44,12 +50,87 @@ public class SolrHealthIndicator extends AbstractHealthIndicator {
 
 	@Override
 	protected void doHealthCheck(Health.Builder builder) throws Exception {
-		CoreAdminRequest request = new CoreAdminRequest();
-		request.setAction(CoreAdminParams.CoreAdminAction.STATUS);
-		CoreAdminResponse response = request.process(this.solrClient);
-		int statusCode = response.getStatus();
+		int statusCode = initializeStatusCheck();
 		Status status = (statusCode != 0) ? Status.DOWN : Status.UP;
-		builder.status(status).withDetail("status", statusCode);
+		builder.status(status).withDetail("status", statusCode).withDetail("detectedPathType",
+				this.statusCheck.getPathType());
+	}
+
+	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 initializeStatusCheck(StatusCheck statusCheck) throws Exception {
+		int result = statusCheck.getStatus(this.solrClient);
+		this.statusCheck = statusCheck;
+		return result;
+	}
+
+	/**
+	 * Strategy used to perform the status check.
+	 */
+	private abstract static class StatusCheck {
+
+		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();
+		}
+
 	}
 
 }

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.RemoteSolrException;
 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,11 +35,16 @@ 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}
  *
  * @author Andy Wilkinson
+ * @author Markus Schuch
+ * @author Phillip Webb
  */
 public class SolrHealthIndicatorTests {
 
@@ -51,27 +58,53 @@ public class SolrHealthIndicatorTests {
 	}
 
 	@Test
-	public void solrIsUp() 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);
+		assertHealth(healthIndicator, Status.UP, 0, "root");
+		verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
+		verifyNoMoreInteractions(solrClient);
 	}
 
 	@Test
-	public void solrIsUpAndRequestFailed() 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);
-		Health health = healthIndicator.health();
-		assertThat(health.getStatus()).isEqualTo(Status.DOWN);
-		assertThat(health.getDetails().get("status")).isEqualTo(400);
+		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 RemoteSolrException("mock", 404, "", null));
+		given(solrClient.ping()).willReturn(mockPingResponse(0));
+		SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient);
+		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 healthWhenSolrStatusDownAndBaseUrlPointsToParticularCoreReturnsDown() 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(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 solrIsDown() throws Exception {
+	public void healthWhenSolrConnectionFailsReturnsDown() throws Exception {
 		SolrClient solrClient = mock(SolrClient.class);
 		given(solrClient.request(any(CoreAdminRequest.class), isNull()))
 				.willThrow(new IOException("Connection failed"));
@@ -79,6 +112,32 @@ 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);
+	}
+
+	@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();
+		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);
+	}
+
+	private void assertHealth(SolrHealthIndicator healthIndicator, Status expectedStatus, int expectedStatusCode,
+			String expectedPathType) {
+		Health health = healthIndicator.health();
+		assertThat(health.getStatus()).isEqualTo(expectedStatus);
+		assertThat(health.getDetails().get("status")).isEqualTo(expectedStatusCode);
+		assertThat(health.getDetails().get("detectedPathType")).isEqualTo(expectedPathType);
 	}
 
 	private NamedList<Object> mockResponse(int status) {
@@ -89,4 +148,10 @@ public class SolrHealthIndicatorTests {
 		return response;
 	}
 
+	private SolrPingResponse mockPingResponse(int status) {
+		SolrPingResponse pingResponse = new SolrPingResponse();
+		pingResponse.setResponse(mockResponse(status));
+		return pingResponse;
+	}
+
 }