HSEARCH-4683 Simplify ElasticsearchClientFactory creation

By moving ElasticsearchHttpClientConfigurer resolution to backend startup, in particular. This will allow us to skip that resolution when a RestClient instance is provided explicitly through settings.
hibernate · Aug 26, 2022 · 2a903c6 · 2a903c6
1 parent 960a3f0
commit 2a903c6
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 80 deletions.
diff --git a/.../org/hibernate/search/backend/elasticsearch/cfg/spi/ElasticsearchBackendImplSettings.java b/.../org/hibernate/search/backend/elasticsearch/cfg/spi/ElasticsearchBackendImplSettings.java
@@ -6,11 +6,6 @@
  */
 package org.hibernate.search.backend.elasticsearch.cfg.spi;
 
-import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClientFactory;
-import org.hibernate.search.backend.elasticsearch.client.impl.ElasticsearchClientFactoryImpl;
-import org.hibernate.search.engine.environment.bean.BeanReference;
-import org.hibernate.search.engine.environment.bean.BeanRetrieval;
-
 /**
  * Implementation-related settings, used for testing only.
  */
@@ -21,15 +16,4 @@ private ElasticsearchBackendImplSettings() {
 
 	public static final String CLIENT_FACTORY = "client_factory";
 
-	/**
-	 * Default values for the different settings if no values are given.
-	 */
-	public static final class Defaults {
-
-		private Defaults() {
-		}
-
-		public static final BeanReference<ElasticsearchClientFactory> CLIENT_FACTORY =
-				ElasticsearchClientFactoryImpl.REFERENCE;
-	}
 }
diff --git a/...rg/hibernate/search/backend/elasticsearch/client/impl/ElasticsearchClientFactoryImpl.java b/...rg/hibernate/search/backend/elasticsearch/client/impl/ElasticsearchClientFactoryImpl.java
@@ -24,7 +24,6 @@
 import org.hibernate.search.engine.environment.bean.BeanReference;
 import org.hibernate.search.engine.environment.bean.BeanResolver;
 import org.hibernate.search.engine.environment.thread.spi.ThreadProvider;
-import org.hibernate.search.util.common.impl.SuppressingCloser;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 
 import org.apache.http.auth.AuthScope;
@@ -47,23 +46,6 @@ public class ElasticsearchClientFactoryImpl implements ElasticsearchClientFactor
 
 	private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() );
 
-	public static final BeanReference<ElasticsearchClientFactory> REFERENCE = (BeanResolver beanResolver) -> {
-		List<BeanReference<ElasticsearchHttpClientConfigurer>> httpClientConfigurerReferences =
-				beanResolver.allConfiguredForRole( ElasticsearchHttpClientConfigurer.class );
-		BeanHolder<List<ElasticsearchHttpClientConfigurer>> httpClientConfigurerHolders =
-				beanResolver.resolve( httpClientConfigurerReferences );
-		try {
-			ElasticsearchClientFactoryImpl factory = new ElasticsearchClientFactoryImpl(
-					httpClientConfigurerHolders.get() );
-			return BeanHolder.<ElasticsearchClientFactory>of( factory )
-					.withDependencyAutoClosing( httpClientConfigurerHolders );
-		}
-		catch (RuntimeException e) {
-			new SuppressingCloser( e ).push( httpClientConfigurerHolders );
-			throw e;
-		}
-	};
-
 	private static final OptionalConfigurationProperty<List<String>> HOSTS =
 			ConfigurationProperty.forKey( ElasticsearchBackendSettings.HOSTS )
 					.asString().multivalued()
@@ -141,12 +123,6 @@ public class ElasticsearchClientFactoryImpl implements ElasticsearchClientFactor
 			.asBeanReference( ElasticsearchHttpClientConfigurer.class )
 			.build();
 
-	private final List<ElasticsearchHttpClientConfigurer> httpClientConfigurers;
-
-	ElasticsearchClientFactoryImpl(List<ElasticsearchHttpClientConfigurer> httpClientConfigurers) {
-		this.httpClientConfigurers = httpClientConfigurers;
-	}
-
 	@Override
 	public ElasticsearchClientImplementor create(BeanResolver beanResolver, ConfigurationPropertySource propertySource,
 			ThreadProvider threadProvider, String threadNamePrefix,
@@ -179,7 +155,10 @@ private RestClient createClient(BeanResolver beanResolver, ConfigurationProperty
 		Optional<? extends BeanHolder<? extends ElasticsearchHttpClientConfigurer>> customConfig = CLIENT_CONFIGURER
 				.getAndMap( propertySource, beanResolver::resolve );
 
-		try {
+		List<BeanReference<ElasticsearchHttpClientConfigurer>> httpClientConfigurerReferences =
+				beanResolver.allConfiguredForRole( ElasticsearchHttpClientConfigurer.class );
+		try ( BeanHolder<List<ElasticsearchHttpClientConfigurer>> httpClientConfigurersHolder =
+				beanResolver.resolve( httpClientConfigurerReferences ) ) {
 			return builder
 					.setRequestConfigCallback( b -> customizeRequestConfig( b, propertySource ) )
 					.setHttpClientConfigCallback(
@@ -188,7 +167,7 @@ private RestClient createClient(BeanResolver beanResolver, ConfigurationProperty
 									beanResolver, propertySource,
 									threadProvider, threadNamePrefix,
 									hosts,
-									httpClientConfigurers, customConfig
+									httpClientConfigurersHolder.get(), customConfig
 							)
 					)
 					.build();

diff --git a/...ain/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java b/...ain/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java
@@ -13,6 +13,7 @@
 import org.hibernate.search.backend.elasticsearch.ElasticsearchVersion;
 import org.hibernate.search.backend.elasticsearch.cfg.ElasticsearchBackendSettings;
 import org.hibernate.search.backend.elasticsearch.cfg.spi.ElasticsearchBackendImplSettings;
+import org.hibernate.search.backend.elasticsearch.client.impl.ElasticsearchClientFactoryImpl;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClientFactory;
 import org.hibernate.search.backend.elasticsearch.dialect.impl.ElasticsearchDialectFactory;
 import org.hibernate.search.backend.elasticsearch.dialect.model.impl.ElasticsearchModelDialect;
@@ -62,10 +63,9 @@ public class ElasticsearchBackendFactory implements BackendFactory {
 					.withDefault( ElasticsearchBackendSettings.Defaults.LOG_JSON_PRETTY_PRINTING )
 					.build();
 
-	private static final ConfigurationProperty<BeanReference<? extends ElasticsearchClientFactory>> CLIENT_FACTORY =
+	private static final OptionalConfigurationProperty<BeanReference<? extends ElasticsearchClientFactory>> CLIENT_FACTORY =
 			ConfigurationProperty.forKey( ElasticsearchBackendImplSettings.CLIENT_FACTORY )
 					.asBeanReference( ElasticsearchClientFactory.class )
-					.withDefault( ElasticsearchBackendImplSettings.Defaults.CLIENT_FACTORY )
 					.build();
 
 	private static final ConfigurationProperty<TypeNameMappingStrategyName> MAPPING_TYPE_STRATEGY =
@@ -101,7 +101,18 @@ public BackendImplementor create(EventContext eventContext, BackendBuildContext
 		try {
 			threads = new BackendThreads( eventContext.render() );
 
-			clientFactoryHolder = CLIENT_FACTORY.getAndTransform( propertySource, beanResolver::resolve );
+			Optional<BeanHolder<? extends ElasticsearchClientFactory>> customClientFactoryHolderOptional =
+					CLIENT_FACTORY.getAndMap( propertySource, beanResolver::resolve );
+			if ( customClientFactoryHolderOptional.isPresent() ) {
+				clientFactoryHolder = customClientFactoryHolderOptional.get();
+				log.debugf(
+						"Elasticsearch backend will use client factory '%s'. Context: %s",
+						clientFactoryHolder, eventContext.render()
+				);
+			}
+			else {
+				clientFactoryHolder = BeanHolder.of( new ElasticsearchClientFactoryImpl() );
+			}
 
 			ElasticsearchDialectFactory dialectFactory = new ElasticsearchDialectFactory();
 			link = new ElasticsearchLinkImpl(

diff --git a/...search/integrationtest/backend/elasticsearch/client/ElasticsearchClientFactoryImplIT.java b/...search/integrationtest/backend/elasticsearch/client/ElasticsearchClientFactoryImplIT.java
@@ -34,12 +34,11 @@
 import javax.net.ssl.SSLContext;
 
 import org.hibernate.search.backend.elasticsearch.cfg.ElasticsearchBackendSettings;
+import org.hibernate.search.backend.elasticsearch.client.ElasticsearchHttpClientConfigurationContext;
+import org.hibernate.search.backend.elasticsearch.client.ElasticsearchHttpClientConfigurer;
 import org.hibernate.search.backend.elasticsearch.client.impl.ElasticsearchClientFactoryImpl;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClient;
-import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClientFactory;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClientImplementor;
-import org.hibernate.search.backend.elasticsearch.client.ElasticsearchHttpClientConfigurationContext;
-import org.hibernate.search.backend.elasticsearch.client.ElasticsearchHttpClientConfigurer;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchRequest;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchResponse;
 import org.hibernate.search.backend.elasticsearch.gson.spi.GsonProvider;
@@ -1008,13 +1007,10 @@ private ElasticsearchClientImplementor createClient(Consumer<BiConsumer<String,
 				AllAwareConfigurationPropertySource.fromMap( beanResolverConfiguration )
 
 		);
-		try ( BeanHolder<ElasticsearchClientFactory> factoryHolder =
-				beanResolver.resolve( ElasticsearchClientFactoryImpl.REFERENCE ) ) {
-			return factoryHolder.get().create( beanResolver, clientPropertySource,
-					threadPoolProvider.threadProvider(), "Client",
-					timeoutExecutorService,
-					GsonProvider.create( GsonBuilder::new, true ) );
-		}
+		return new ElasticsearchClientFactoryImpl().create( beanResolver, clientPropertySource,
+				threadPoolProvider.threadProvider(), "Client",
+				timeoutExecutorService,
+				GsonProvider.create( GsonBuilder::new, true ) );
 	}
 
 	private ElasticsearchResponse doPost(ElasticsearchClient client, String path, String payload) {

diff --git a/...ate/search/integrationtest/backend/elasticsearch/client/ElasticsearchContentLengthIT.java b/...ate/search/integrationtest/backend/elasticsearch/client/ElasticsearchContentLengthIT.java
@@ -26,7 +26,6 @@
 import org.hibernate.search.backend.elasticsearch.cfg.ElasticsearchBackendSettings;
 import org.hibernate.search.backend.elasticsearch.client.impl.ElasticsearchClientFactoryImpl;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClient;
-import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClientFactory;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClientImplementor;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchRequest;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchResponse;
@@ -208,13 +207,10 @@ private ElasticsearchClientImplementor createClient() {
 		ConfigurationPropertySource clientPropertySource = AllAwareConfigurationPropertySource.fromMap( clientProperties );
 
 		BeanResolver beanResolver = testConfigurationProvider.createBeanResolverForTest();
-		try ( BeanHolder<ElasticsearchClientFactory> factoryHolder =
-				beanResolver.resolve( ElasticsearchClientFactoryImpl.REFERENCE ) ) {
-			return factoryHolder.get().create( beanResolver, clientPropertySource,
-					threadPoolProvider.threadProvider(), "Client",
-					timeoutExecutorService,
-					GsonProvider.create( GsonBuilder::new, true ) );
-		}
+		return new ElasticsearchClientFactoryImpl().create( beanResolver, clientPropertySource,
+				threadPoolProvider.threadProvider(), "Client",
+				timeoutExecutorService,
+				GsonProvider.create( GsonBuilder::new, true ) );
 	}
 
 	private ElasticsearchResponse doPost(ElasticsearchClient client, String path, Collection<JsonObject> bodyParts) {

diff --git a/...search/integrationtest/backend/elasticsearch/testsupport/util/ElasticsearchClientSpy.java b/...search/integrationtest/backend/elasticsearch/testsupport/util/ElasticsearchClientSpy.java
@@ -78,14 +78,7 @@ public int getRequestCount() {
 	}
 
 	public BeanReference<ElasticsearchClientFactory> factoryReference() {
-		return beanResolver -> {
-			BeanHolder<ElasticsearchClientFactory> delegateHolder =
-					beanResolver.resolve( ElasticsearchClientFactoryImpl.REFERENCE );
-			SpyingElasticsearchClientFactory spyingFactory =
-					new SpyingElasticsearchClientFactory( delegateHolder.get() );
-			return BeanHolder.<ElasticsearchClientFactory>of( spyingFactory )
-					.withDependencyAutoClosing( delegateHolder );
-		};
+		return beanResolver -> BeanHolder.of( new SpyingElasticsearchClientFactory( new ElasticsearchClientFactoryImpl() ) );
 	}
 
 	public void expectNext(ElasticsearchRequest request, ElasticsearchRequestAssertionMode assertionMode) {

diff --git a/.../search/util/impl/integrationtest/backend/elasticsearch/rule/TestElasticsearchClient.java b/.../search/util/impl/integrationtest/backend/elasticsearch/rule/TestElasticsearchClient.java
@@ -27,7 +27,6 @@
 import org.hibernate.search.backend.elasticsearch.client.impl.ElasticsearchClientFactoryImpl;
 import org.hibernate.search.backend.elasticsearch.client.impl.ElasticsearchClientUtils;
 import org.hibernate.search.backend.elasticsearch.client.impl.Paths;
-import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClientFactory;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchClientImplementor;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchRequest;
 import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchResponse;
@@ -515,16 +514,13 @@ public void open(TestConfigurationProvider configurationProvider) {
 		 * We use a {@link ElasticsearchClientFactoryImpl} to create our low-level client.
 		 *
 		 * The main advantage is that we ensure we connect to Elasticsearch exactly the same way
-		 * as any test-created SearchFactory, allowing to support things like testing on AWS
+		 * as any test-created SearchFactory, enabling support for things like testing on AWS
 		 * (using the hibernate-search-elasticsearch-aws module).
 		 */
-		try ( BeanHolder<ElasticsearchClientFactory> factoryHolder =
-				beanResolver.resolve( ElasticsearchClientFactoryImpl.REFERENCE ) ) {
-			client = factoryHolder.get().create( beanResolver, backendProperties,
-					threadPoolProvider.threadProvider(), "Client",
-					timeoutExecutorService,
-					GsonProvider.create( GsonBuilder::new, true ) );
-		}
+		client = new ElasticsearchClientFactoryImpl().create( beanResolver, backendProperties,
+				threadPoolProvider.threadProvider(), "Client",
+				timeoutExecutorService,
+				GsonProvider.create( GsonBuilder::new, true ) );
 	}
 
 	@Override