Merge pull request #2371 from hapifhir/config-error-on-url-protocol

Throw ConfigurationError if you attempt to provide a Rest URL that contains a protocol.

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2372-throw-error-on-protocol-aware-url.yaml

@@ -0,0 +1,5 @@
+---
+type: change
+issue: 2372
+title: "ElasticsearchHibernatePropertiesBuilder will now reject any REST url which is protocol-aware. Protocol information should be set in the protocol field of the builder."
+

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java

@@ -109,6 +109,9 @@ public class ElasticsearchHibernatePropertiesBuilder {
 	}
 
 	public ElasticsearchHibernatePropertiesBuilder setRestUrl(String theRestUrl) {
+		if (theRestUrl.contains("://")) {
+			throw new ConfigurationException("Elasticsearch URL cannot include a protocol, that is a separate property. Remove http:// or https:// from this URL.");
+		}
 		myRestUrl = theRestUrl;
 		return this;
 	}
@@ -144,7 +147,7 @@ public class ElasticsearchHibernatePropertiesBuilder {
 	 * TODO GGG HS: In HS6.1, we should have a native way of performing index settings manipulation at bootstrap time, so this should
 	 * eventually be removed in favour of whatever solution they come up with.
 	 */
-	private void injectStartupTemplate(String theProtocol, String theHostAndPort, String theUsername, String thePassword) {
+	void injectStartupTemplate(String theProtocol, String theHostAndPort, String theUsername, String thePassword) {
 		PutIndexTemplateRequest ngramTemplate = new PutIndexTemplateRequest("ngram-template")
 			.patterns(Arrays.asList("resourcetable-*", "termconcept-*"))
 			.settings(Settings.builder().put("index.max_ngram_diff", 50));

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java

@@ -0,0 +1,60 @@
+package ca.uhn.fhir.jpa.search.elastic;
+
+import ca.uhn.fhir.context.ConfigurationException;
+import org.hibernate.search.backend.elasticsearch.cfg.ElasticsearchBackendSettings;
+import org.hibernate.search.engine.cfg.BackendSettings;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.util.Properties;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.spy;
+
+@ExtendWith(MockitoExtension.class)
+class ElasticsearchHibernatePropertiesBuilderTest {
+
+	ElasticsearchHibernatePropertiesBuilder myPropertiesBuilder = spy(ElasticsearchHibernatePropertiesBuilder.class);
+
+	@BeforeEach
+	public void prepMocks() {
+		//ensures we don't try to reach out to a real ES server on apply.
+		doNothing().when(myPropertiesBuilder).injectStartupTemplate(any(), any(), any(), any());
+	}
+
+	@Test
+	public void testRestUrlCannotContainProtocol() {
+		String host = "localhost:9200";
+		String protocolHost = "https://" + host;
+		String failureMessage = "Elasticsearch URL cannot include a protocol, that is a separate property. Remove http:// or https:// from this URL.";
+
+		myPropertiesBuilder
+			.setProtocol("https")
+			.setUsername("whatever")
+			.setPassword("whatever");
+
+		//SUT
+		try {
+			myPropertiesBuilder.setRestUrl(protocolHost);
+			fail();
+		} catch (ConfigurationException e ) {
+			assertThat(e.getMessage(), is(equalTo(failureMessage)));
+		}
+
+		Properties properties = new Properties();
+		myPropertiesBuilder
+			.setRestUrl(host)
+			.apply(properties);
+
+		assertThat(properties.getProperty(BackendSettings.backendKey(ElasticsearchBackendSettings.HOSTS)), is(equalTo(host)));
+
+	}
+
+}