From c6aa21b915f946023a19e57d487f4dfa8a4d0cb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Markovi=C4=8D?= Date: Thu, 9 Sep 2021 12:55:04 +0200 Subject: [PATCH 1/4] Enable empty username and password in ElasticsearchHibernatePropertiesBuilder --- .../lastn/ElasticsearchRestClientFactory.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchRestClientFactory.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchRestClientFactory.java index a221a754b6f..766945bbc23 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchRestClientFactory.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchRestClientFactory.java @@ -20,6 +20,7 @@ package ca.uhn.fhir.jpa.search.lastn; * #L% */ +import org.apache.commons.lang3.StringUtils; import org.apache.http.Header; import org.apache.http.HttpHost; import org.apache.http.auth.AuthScope; @@ -31,6 +32,8 @@ import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestClientBuilder; import org.elasticsearch.client.RestHighLevelClient; +import javax.annotation.Nullable; + public class ElasticsearchRestClientFactory { @@ -52,20 +55,21 @@ public class ElasticsearchRestClientFactory { } } - static public RestHighLevelClient createElasticsearchHighLevelRestClient(String theHostname, int thePort, String theUsername, String thePassword) { - final CredentialsProvider credentialsProvider = - new BasicCredentialsProvider(); - credentialsProvider.setCredentials(AuthScope.ANY, - new UsernamePasswordCredentials(theUsername, thePassword)); - RestClientBuilder clientBuilder = RestClient.builder( - new HttpHost(stripHostOfScheme(theHostname), thePort, determineScheme(theHostname))) - .setHttpClientConfigCallback(httpClientBuilder -> httpClientBuilder - .setDefaultCredentialsProvider(credentialsProvider)); + static public RestHighLevelClient createElasticsearchHighLevelRestClient( + String theHostname, int thePort, @Nullable String theUsername, @Nullable String thePassword) { - Header[] defaultHeaders = new Header[]{new BasicHeader("Content-Type", "application/json")}; - clientBuilder.setDefaultHeaders(defaultHeaders); + RestClientBuilder clientBuilder = RestClient.builder( + new HttpHost(stripHostOfScheme(theHostname), thePort, determineScheme(theHostname))); + if (StringUtils.isNotBlank(theUsername) && StringUtils.isNotBlank(thePassword)) { + final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); + credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(theUsername, thePassword)); + clientBuilder.setHttpClientConfigCallback(httpClientBuilder -> httpClientBuilder + .setDefaultCredentialsProvider(credentialsProvider)); + } + Header[] defaultHeaders = new Header[]{new BasicHeader("Content-Type", "application/json")}; + clientBuilder.setDefaultHeaders(defaultHeaders); - return new RestHighLevelClient(clientBuilder); + return new RestHighLevelClient(clientBuilder); - } + } } From ed325e9e11951416e3b4d1fb815b952a3efd83f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Markovi=C4=8D?= Date: Fri, 10 Sep 2021 00:14:21 +0200 Subject: [PATCH 2/4] Enable multiple hosts in ElasticsearchHibernatePropertiesBuilder --- .../ca/uhn/hapi/fhir/docs/server_jpa/lastn.md | 7 ++- .../jpa/search/SearchCoordinatorSvcImpl.java | 2 +- ...asticsearchHibernatePropertiesBuilder.java | 31 ++++--------- .../lastn/ElasticsearchRestClientFactory.java | 46 ++++++++++--------- .../search/lastn/ElasticsearchSvcImpl.java | 10 ++-- .../config/ElasticsearchWithPrefixConfig.java | 2 +- .../config/TestR4ConfigWithElasticSearch.java | 2 +- .../TestR4ConfigWithElasticsearchClient.java | 2 +- .../jpa/dao/r4/ElasticsearchPrefixTest.java | 2 +- ...csearchHibernatePropertiesBuilderTest.java | 4 +- ...lasticsearchSvcMultipleObservationsIT.java | 2 +- ...tNElasticsearchSvcSingleObservationIT.java | 2 +- 12 files changed, 52 insertions(+), 60 deletions(-) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md index 4f70dac2307..f84b85ea65d 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md @@ -31,12 +31,11 @@ In addition, the Elasticsearch client service, `ElasticsearchSvcImpl` will need ```java @Bean() public ElasticsearchSvcImpl elasticsearchSvc() { - String elasticsearchHost = "localhost"; - String elasticsearchUserId = "elastic"; + String elasticsearchHost = "localhost:9301"; + String elasticsearchUsername = "elastic"; String elasticsearchPassword = "changeme"; - int elasticsearchPort = 9301; - return new ElasticsearchSvcImpl(elasticsearchHost, elasticsearchPort, elasticsearchUserId, elasticsearchPassword); + return new ElasticsearchSvcImpl(elasticsearchHost, elasticsearchUsername, elasticsearchPassword); } ``` diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index 4d266271bd9..aa67d53c597 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -1087,7 +1087,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { ourLog.trace("Performing count"); ISearchBuilder sb = newSearchBuilder(); Iterator countIterator = sb.createCountQuery(myParams, mySearch.getUuid(), myRequest, myRequestPartitionId); - Long count = countIterator.hasNext() ? countIterator.next() : 0; + Long count = countIterator.hasNext() ? countIterator.next() : 0L; ourLog.trace("Got count {}", count); TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java index 165b0478489..a39fb59c4a3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java @@ -37,6 +37,7 @@ import org.hibernate.search.backend.elasticsearch.cfg.ElasticsearchIndexSettings import org.hibernate.search.mapper.orm.schema.management.SchemaManagementStrategyName; import org.slf4j.Logger; +import javax.annotation.Nullable; import java.io.IOException; import java.util.Arrays; import java.util.Properties; @@ -49,13 +50,13 @@ import static org.slf4j.LoggerFactory.getLogger; * FHIR JPA server. This class also injects a starter template into the ES cluster. */ public class ElasticsearchHibernatePropertiesBuilder { - private static final Logger ourLog = getLogger(ElasticsearchHibernatePropertiesBuilder.class); + private static final Logger ourLog = getLogger(ElasticsearchHibernatePropertiesBuilder.class); - private IndexStatus myRequiredIndexStatus = IndexStatus.YELLOW.YELLOW; + private IndexStatus myRequiredIndexStatus = IndexStatus.YELLOW; private SchemaManagementStrategyName myIndexSchemaManagementStrategy = SchemaManagementStrategyName.CREATE; - private String myRestUrl; + private String myHosts; private String myUsername; private String myPassword; private long myIndexManagementWaitTimeoutMillis = 10000L; @@ -77,11 +78,8 @@ public class ElasticsearchHibernatePropertiesBuilder { // the below properties are used for ElasticSearch integration theProperties.put(BackendSettings.backendKey(BackendSettings.TYPE), "elasticsearch"); - - theProperties.put(BackendSettings.backendKey(ElasticsearchIndexSettings.ANALYSIS_CONFIGURER), HapiElasticsearchAnalysisConfigurer.class.getName()); - - theProperties.put(BackendSettings.backendKey(ElasticsearchBackendSettings.HOSTS), myRestUrl); + theProperties.put(BackendSettings.backendKey(ElasticsearchBackendSettings.HOSTS), myHosts); theProperties.put(BackendSettings.backendKey(ElasticsearchBackendSettings.PROTOCOL), myProtocol); if (StringUtils.isNotBlank(myUsername)) { @@ -102,8 +100,7 @@ public class ElasticsearchHibernatePropertiesBuilder { //This tells elasticsearch to use our custom index naming strategy. theProperties.put(BackendSettings.backendKey(ElasticsearchBackendSettings.LAYOUT_STRATEGY), IndexNamePrefixLayoutStrategy.class.getName()); - injectStartupTemplate(myProtocol, myRestUrl, myUsername, myPassword); - + injectStartupTemplate(myProtocol, myHosts, myUsername, myPassword); } public ElasticsearchHibernatePropertiesBuilder setRequiredIndexStatus(IndexStatus theRequiredIndexStatus) { @@ -111,11 +108,8 @@ public class ElasticsearchHibernatePropertiesBuilder { return this; } - 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; + public ElasticsearchHibernatePropertiesBuilder setHosts(String hosts) { + myHosts = hosts; return this; } @@ -150,18 +144,13 @@ 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. */ - void injectStartupTemplate(String theProtocol, String theHostAndPort, String theUsername, String thePassword) { + void injectStartupTemplate(String protocol, String hosts, @Nullable String username, @Nullable String password) { PutIndexTemplateRequest ngramTemplate = new PutIndexTemplateRequest("ngram-template") .patterns(Arrays.asList("*resourcetable-*", "*termconcept-*")) .settings(Settings.builder().put("index.max_ngram_diff", 50)); - int colonIndex = theHostAndPort.indexOf(":"); - String host = theHostAndPort.substring(0, colonIndex); - Integer port = Integer.valueOf(theHostAndPort.substring(colonIndex + 1)); - String qualifiedHost = theProtocol + "://" + host; - try { - RestHighLevelClient elasticsearchHighLevelRestClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient(qualifiedHost, port, theUsername, thePassword); + RestHighLevelClient elasticsearchHighLevelRestClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient(protocol, hosts, username, password); ourLog.info("Adding starter template for large ngram diffs"); AcknowledgedResponse acknowledgedResponse = elasticsearchHighLevelRestClient.indices().putTemplate(ngramTemplate, RequestOptions.DEFAULT); assert acknowledgedResponse.isAcknowledged(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchRestClientFactory.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchRestClientFactory.java index 766945bbc23..b4cd687e313 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchRestClientFactory.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchRestClientFactory.java @@ -20,6 +20,7 @@ package ca.uhn.fhir.jpa.search.lastn; * #L% */ +import ca.uhn.fhir.context.ConfigurationException; import org.apache.commons.lang3.StringUtils; import org.apache.http.Header; import org.apache.http.HttpHost; @@ -28,38 +29,41 @@ import org.apache.http.auth.UsernamePasswordCredentials; import org.apache.http.client.CredentialsProvider; import org.apache.http.impl.client.BasicCredentialsProvider; import org.apache.http.message.BasicHeader; +import org.elasticsearch.client.Node; import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestClientBuilder; import org.elasticsearch.client.RestHighLevelClient; import javax.annotation.Nullable; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; public class ElasticsearchRestClientFactory { - private static String determineScheme(String theHostname) { - int schemeIdx = theHostname.indexOf("://"); - if (schemeIdx > 0) { - return theHostname.substring(0, schemeIdx); - } else { - return "http"; - } - } - - private static String stripHostOfScheme(String theHostname) { - int schemeIdx = theHostname.indexOf("://"); - if (schemeIdx > 0) { - return theHostname.substring(schemeIdx + 3); - } else { - return theHostname; - } - } - static public RestHighLevelClient createElasticsearchHighLevelRestClient( - String theHostname, int thePort, @Nullable String theUsername, @Nullable String thePassword) { + String protocol, String hosts, @Nullable String theUsername, @Nullable String thePassword) { - RestClientBuilder clientBuilder = RestClient.builder( - new HttpHost(stripHostOfScheme(theHostname), thePort, determineScheme(theHostname))); + if (hosts.contains("://")) { + throw new ConfigurationException("Elasticsearch URLs cannot include a protocol, that is a separate property. Remove http:// or https:// from this URL."); + } + String[] hostArray = hosts.split(","); + List clientNodes = Arrays.stream(hostArray) + .map(String::trim) + .filter(s -> s.contains(":")) + .map(h -> { + int colonIndex = h.indexOf(":"); + String host = h.substring(0, colonIndex); + int port = Integer.parseInt(h.substring(colonIndex + 1)); + return new Node(new HttpHost(host, port, protocol)); + }) + .collect(Collectors.toList()); + if (hostArray.length != clientNodes.size()) { + throw new ConfigurationException("Elasticsearch URLs have to contain ':' as a host:port separator. Example: localhost:9200,localhost:9201,localhost:9202"); + } + + RestClientBuilder clientBuilder = RestClient.builder(clientNodes.toArray(new Node[0])); if (StringUtils.isNotBlank(theUsername) && StringUtils.isNotBlank(thePassword)) { final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(theUsername, thePassword)); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchSvcImpl.java index 54a28d6cf3a..57faac61bd6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/lastn/ElasticsearchSvcImpl.java @@ -68,11 +68,11 @@ import org.elasticsearch.search.aggregations.bucket.terms.ParsedTerms; import org.elasticsearch.search.aggregations.bucket.terms.Terms; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.ParsedTopHits; -import org.elasticsearch.search.aggregations.support.ValueType; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.sort.SortOrder; import org.springframework.beans.factory.annotation.Autowired; +import javax.annotation.Nullable; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; @@ -125,13 +125,13 @@ public class ElasticsearchSvcImpl implements IElasticsearchSvc { private PartitionSettings myPartitionSettings; //This constructor used to inject a dummy partitionsettings in test. - public ElasticsearchSvcImpl(PartitionSettings thePartitionSetings, String theHostname, int thePort, String theUsername, String thePassword) { - this(theHostname, thePort, theUsername, thePassword); + public ElasticsearchSvcImpl(PartitionSettings thePartitionSetings, String theHostname, @Nullable String theUsername, @Nullable String thePassword) { + this(theHostname, theUsername, thePassword); this.myPartitionSettings = thePartitionSetings; } - public ElasticsearchSvcImpl(String theHostname, int thePort, String theUsername, String thePassword) { - myRestHighLevelClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient(theHostname, thePort, theUsername, thePassword); + public ElasticsearchSvcImpl(String theHostname, @Nullable String theUsername, @Nullable String thePassword) { + myRestHighLevelClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient("http", theHostname, theUsername, thePassword); try { createObservationIndexIfMissing(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/ElasticsearchWithPrefixConfig.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/ElasticsearchWithPrefixConfig.java index 5789ee204dc..0014407fb09 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/ElasticsearchWithPrefixConfig.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/ElasticsearchWithPrefixConfig.java @@ -128,7 +128,7 @@ public class ElasticsearchWithPrefixConfig { .settings(Settings.builder().put("index.max_ngram_diff", 50)); try { - RestHighLevelClient elasticsearchHighLevelRestClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient("http://" + host, httpPort, "", ""); + RestHighLevelClient elasticsearchHighLevelRestClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient("http", host + ":" + httpPort, "", ""); AcknowledgedResponse acknowledgedResponse = elasticsearchHighLevelRestClient.indices().putTemplate(ngramTemplate, RequestOptions.DEFAULT); assert acknowledgedResponse.isAcknowledged(); } catch (IOException theE) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4ConfigWithElasticSearch.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4ConfigWithElasticSearch.java index 2e358abe4ec..5c41dc91965 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4ConfigWithElasticSearch.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4ConfigWithElasticSearch.java @@ -41,7 +41,7 @@ public class TestR4ConfigWithElasticSearch extends TestR4Config { .setIndexSchemaManagementStrategy(SchemaManagementStrategyName.CREATE) .setIndexManagementWaitTimeoutMillis(10000) .setRequiredIndexStatus(IndexStatus.YELLOW) - .setRestUrl(host+ ":" + httpPort) + .setHosts(host + ":" + httpPort) .setProtocol("http") .setUsername("") .setPassword("") diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4ConfigWithElasticsearchClient.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4ConfigWithElasticsearchClient.java index 7e52caaf37f..3b2b011e505 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4ConfigWithElasticsearchClient.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4ConfigWithElasticsearchClient.java @@ -21,7 +21,7 @@ public class TestR4ConfigWithElasticsearchClient extends TestR4ConfigWithElastic public ElasticsearchSvcImpl myElasticsearchSvc() { int elasticsearchPort = elasticContainer().getMappedPort(9200); String host = elasticContainer().getHost(); - return new ElasticsearchSvcImpl(host, elasticsearchPort, "", ""); + return new ElasticsearchSvcImpl(host + ":" + elasticsearchPort, null, null); } @PreDestroy diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ElasticsearchPrefixTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ElasticsearchPrefixTest.java index 239eadda9e0..ae869fe00a8 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ElasticsearchPrefixTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ElasticsearchPrefixTest.java @@ -33,7 +33,7 @@ public class ElasticsearchPrefixTest { public void test() throws IOException { //Given RestHighLevelClient elasticsearchHighLevelRestClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient( - "http://" + elasticsearchContainer.getHost(), elasticsearchContainer.getMappedPort(9200), "", ""); + "http", elasticsearchContainer.getHost() + ":" + elasticsearchContainer.getMappedPort(9200), "", ""); //When RestClient lowLevelClient = elasticsearchHighLevelRestClient.getLowLevelClient(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java index acca2706551..fce54f4b93c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java @@ -42,7 +42,7 @@ class ElasticsearchHibernatePropertiesBuilderTest { //SUT try { - myPropertiesBuilder.setRestUrl(protocolHost); + myPropertiesBuilder.setHosts(protocolHost); fail(); } catch (ConfigurationException e ) { assertThat(e.getMessage(), is(equalTo(failureMessage))); @@ -50,7 +50,7 @@ class ElasticsearchHibernatePropertiesBuilderTest { Properties properties = new Properties(); myPropertiesBuilder - .setRestUrl(host) + .setHosts(host) .apply(properties); assertThat(properties.getProperty(BackendSettings.backendKey(ElasticsearchBackendSettings.HOSTS)), is(equalTo(host))); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcMultipleObservationsIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcMultipleObservationsIT.java index ae0bec25252..653c4de56f5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcMultipleObservationsIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcMultipleObservationsIT.java @@ -68,7 +68,7 @@ public class LastNElasticsearchSvcMultipleObservationsIT { public void before() throws IOException { PartitionSettings partitionSettings = new PartitionSettings(); partitionSettings.setPartitioningEnabled(false); - elasticsearchSvc = new ElasticsearchSvcImpl(partitionSettings, elasticsearchContainer.getHost(), elasticsearchContainer.getMappedPort(9200), "", ""); + elasticsearchSvc = new ElasticsearchSvcImpl(partitionSettings, elasticsearchContainer.getHost() + ":" + elasticsearchContainer.getMappedPort(9200), null, null); if (!indexLoaded) { createMultiplePatientsAndObservations(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcSingleObservationIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcSingleObservationIT.java index a8447121486..55a5056f8d8 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcSingleObservationIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcSingleObservationIT.java @@ -97,7 +97,7 @@ public class LastNElasticsearchSvcSingleObservationIT { public void before() { PartitionSettings partitionSettings = new PartitionSettings(); partitionSettings.setPartitioningEnabled(false); - elasticsearchSvc = new ElasticsearchSvcImpl(partitionSettings, elasticsearchContainer.getHost(), elasticsearchContainer.getMappedPort(9200), "", ""); + elasticsearchSvc = new ElasticsearchSvcImpl(partitionSettings, elasticsearchContainer.getHost() + ":" + elasticsearchContainer.getMappedPort(9200), "", ""); } @AfterEach From cfc0c7e6b3f0e8984e5f6212bf0504989cb8e982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Markovi=C4=8D?= Date: Fri, 10 Sep 2021 11:44:15 +0200 Subject: [PATCH 3/4] Add test for validation of multiple hosts in ElasticsearchHibernatePropertiesBuilder --- ...csearchHibernatePropertiesBuilderTest.java | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java index fce54f4b93c..df4598e86c8 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilderTest.java @@ -23,17 +23,11 @@ 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() { + public void testHostsCannotContainProtocol() { 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."; + String failureMessage = "Elasticsearch URLs cannot include a protocol, that is a separate property. Remove http:// or https:// from this URL."; myPropertiesBuilder .setProtocol("https") @@ -42,12 +36,14 @@ class ElasticsearchHibernatePropertiesBuilderTest { //SUT try { - myPropertiesBuilder.setHosts(protocolHost); + myPropertiesBuilder.setHosts(protocolHost) + .apply(new Properties()); fail(); } catch (ConfigurationException e ) { assertThat(e.getMessage(), is(equalTo(failureMessage))); } + doNothing().when(myPropertiesBuilder).injectStartupTemplate(any(), any(), any(), any()); Properties properties = new Properties(); myPropertiesBuilder .setHosts(host) @@ -57,4 +53,25 @@ class ElasticsearchHibernatePropertiesBuilderTest { } + @Test + public void testHostsValueValidation() { + String host = "localhost_9200,localhost:9201,localhost:9202"; + String failureMessage = "Elasticsearch URLs have to contain ':' as a host:port separator. Example: localhost:9200,localhost:9201,localhost:9202"; + + myPropertiesBuilder + .setProtocol("https") + .setHosts(host) + .setUsername("whatever") + .setPassword("whatever"); + + //SUT + try { + myPropertiesBuilder + .apply(new Properties()); + fail(); + } catch (ConfigurationException e ) { + assertThat(e.getMessage(), is(equalTo(failureMessage))); + } + } + } From d6d862015b33a3b31ba9c842548afce6fed76e4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Markovi=C4=8D?= Date: Fri, 10 Sep 2021 15:46:27 +0200 Subject: [PATCH 4/4] Address concerns in review for issue 2975 --- .../main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md | 2 +- .../elastic/ElasticsearchHibernatePropertiesBuilder.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md index f84b85ea65d..0848298ee66 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/lastn.md @@ -31,7 +31,7 @@ In addition, the Elasticsearch client service, `ElasticsearchSvcImpl` will need ```java @Bean() public ElasticsearchSvcImpl elasticsearchSvc() { - String elasticsearchHost = "localhost:9301"; + String elasticsearchHost = "localhost:9200"; String elasticsearchUsername = "elastic"; String elasticsearchPassword = "changeme"; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java index a39fb59c4a3..f428181d3f1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/ElasticsearchHibernatePropertiesBuilder.java @@ -144,13 +144,13 @@ 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. */ - void injectStartupTemplate(String protocol, String hosts, @Nullable String username, @Nullable String password) { + void injectStartupTemplate(String theProtocol, String theHosts, @Nullable String theUsername, @Nullable String thePassword) { PutIndexTemplateRequest ngramTemplate = new PutIndexTemplateRequest("ngram-template") .patterns(Arrays.asList("*resourcetable-*", "*termconcept-*")) .settings(Settings.builder().put("index.max_ngram_diff", 50)); try { - RestHighLevelClient elasticsearchHighLevelRestClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient(protocol, hosts, username, password); + RestHighLevelClient elasticsearchHighLevelRestClient = ElasticsearchRestClientFactory.createElasticsearchHighLevelRestClient(theProtocol, theHosts, theUsername, thePassword); ourLog.info("Adding starter template for large ngram diffs"); AcknowledgedResponse acknowledgedResponse = elasticsearchHighLevelRestClient.indices().putTemplate(ngramTemplate, RequestOptions.DEFAULT); assert acknowledgedResponse.isAcknowledged();