From bf18d55016df9b9de811158c53f6c45ead4a295c Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Tue, 17 Mar 2026 16:39:38 +0100 Subject: [PATCH] refactor: delegate Elasticsearch client lifecycle to Spring Boot auto-configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ElasticConfig now exposes a single @Bean RestClient built from the existing elasticsearch.* properties. Spring Boot's ElasticsearchClientAutoConfiguration detects this bean and auto-configures ElasticsearchTransport and ElasticsearchClient on top of it. This also activates ElasticsearchRestClientHealthContributorAutoConfiguration, fixing the /actuator/health elasticsearch indicator that was previously broken. No changes to application.properties are required — all existing elasticsearch.* connection properties continue to work unchanged: elasticsearch.host_urls (preferred) elasticsearch.network.host (deprecated, still supported) elasticsearch.http.port (deprecated, still supported) elasticsearch.authorization.header elasticsearch.authorization.username elasticsearch.authorization.password Internal changes: - Removed the duplicate searchClient/indexClient @Bean methods and the manual createClient() factory; there is now exactly one ElasticsearchClient bean in the context - Removed @Qualifier("indexClient") from ChannelRepository, PropertyRepository, TagRepository, ChannelScrollController, and PopulateDBConfiguration - InfoController now injects ElasticsearchClient directly instead of going through ElasticConfig.getSearchClient() - ElasticConfig no longer implements ServletContextListener; Spring Boot manages the RestClient lifecycle Co-Authored-By: Claude Sonnet 4.6 --- .../configuration/ElasticConfig.java | 125 +++++++----------- .../PopulateDBConfiguration.java | 5 +- .../repository/ChannelRepository.java | 2 +- .../repository/PropertyRepository.java | 5 +- .../repository/TagRepository.java | 5 +- .../channelfinder/service/InfoService.java | 12 +- src/main/resources/application.properties | 4 +- .../channelfinder/ElasticConfigIT.java | 6 +- 8 files changed, 63 insertions(+), 101 deletions(-) diff --git a/src/main/java/org/phoebus/channelfinder/configuration/ElasticConfig.java b/src/main/java/org/phoebus/channelfinder/configuration/ElasticConfig.java index 761f7dca..b3688a34 100644 --- a/src/main/java/org/phoebus/channelfinder/configuration/ElasticConfig.java +++ b/src/main/java/org/phoebus/channelfinder/configuration/ElasticConfig.java @@ -19,12 +19,11 @@ import co.elastic.clients.elasticsearch.indices.IndexSettings; import co.elastic.clients.elasticsearch.indices.PutIndicesSettingsRequest; import co.elastic.clients.elasticsearch.indices.PutIndicesSettingsResponse; -import co.elastic.clients.json.jackson.JacksonJsonpMapper; -import co.elastic.clients.transport.ElasticsearchTransport; import co.elastic.clients.transport.endpoints.BooleanResponse; import co.elastic.clients.transport.rest_client.RestClientTransport; // Jackson 2 required by elasticsearch-java 8.x JacksonJsonpMapper — migrate with ES 9 import com.fasterxml.jackson.databind.ObjectMapper; +import jakarta.annotation.PostConstruct; import jakarta.servlet.ServletContextEvent; import jakarta.servlet.ServletContextListener; import java.io.IOException; @@ -43,10 +42,9 @@ import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestClientBuilder; import org.phoebus.channelfinder.common.TextUtil; -import org.phoebus.channelfinder.entity.Property; -import org.phoebus.channelfinder.entity.Tag; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; @@ -56,15 +54,16 @@ * @author Kunal Shroff {@literal } */ @Configuration -@ConfigurationProperties(prefix = "elasticsearch") @ComponentScan(basePackages = {"org.phoebus.channelfinder"}) @PropertySource(value = "classpath:application.properties") -public class ElasticConfig implements ServletContextListener { +public class ElasticConfig { private static final Logger logger = Logger.getLogger(ElasticConfig.class.getName()); - private ElasticsearchClient searchClient; - private ElasticsearchClient indexClient; + // Used to retrieve the auto-configured ElasticsearchClient lazily in @PostConstruct, + // avoiding a circular dependency (this bean provides RestClient → auto-config builds + // ElasticsearchClient from it). + @Autowired private ApplicationContext applicationContext; @Value("${elasticsearch.network.host:localhost}") private String host; @@ -119,51 +118,37 @@ public int getES_MAX_RESULT_WINDOW_SIZE() { return ES_QUERY_SIZE; } - ObjectMapper objectMapper = - new ObjectMapper() - .addMixIn(Tag.class, Tag.OnlyTag.class) - .addMixIn(Property.class, Property.OnlyProperty.class); + public ElasticsearchClient getElasticsearchClient() { + return applicationContext.getBean(ElasticsearchClient.class); + } - private static ElasticsearchClient createClient( - ElasticsearchClient currentClient, - ObjectMapper objectMapper, - HttpHost[] httpHosts, - String createIndices, - ElasticConfig config) { - ElasticsearchClient client; - if (currentClient == null) { - // Create the low-level client - RestClientBuilder clientBuilder = RestClient.builder(httpHosts); - // Configure authentication - if (!config.authorizationHeader.isEmpty()) { - clientBuilder.setDefaultHeaders( - new Header[] {new BasicHeader("Authorization", config.authorizationHeader)}); - if (!config.username.isEmpty() || !config.password.isEmpty()) { - logger.warning( - "elasticsearch.authorization_header is set, ignoring elasticsearch.username and elasticsearch.password."); - } - } else if (!config.username.isEmpty() || !config.password.isEmpty()) { - final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); - credentialsProvider.setCredentials( - AuthScope.ANY, new UsernamePasswordCredentials(config.username, config.password)); - clientBuilder.setHttpClientConfigCallback( - httpClientBuilder -> - httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider)); + /** + * Provides the low-level Elasticsearch {@link RestClient} built from the {@code elasticsearch.*} + * connection properties. Spring Boot's {@code ElasticsearchClientAutoConfiguration} detects this + * bean and uses it to auto-configure {@code ElasticsearchTransport} and {@code + * ElasticsearchClient}, which in turn activates the {@code /actuator/health} Elasticsearch + * indicator. + */ + @Bean + public RestClient restClient() { + RestClientBuilder clientBuilder = RestClient.builder(getHttpHosts()); + if (!authorizationHeader.isEmpty()) { + clientBuilder.setDefaultHeaders( + new Header[] {new BasicHeader("Authorization", authorizationHeader)}); + if (!username.isEmpty() || !password.isEmpty()) { + logger.warning( + "elasticsearch.authorization.header is set, ignoring" + + " elasticsearch.authorization.username and elasticsearch.authorization.password."); } - RestClient httpClient = clientBuilder.build(); - - // Create the Java API Client with the same low level client - ElasticsearchTransport transport = - new RestClientTransport(httpClient, new JacksonJsonpMapper(objectMapper)); - - client = new ElasticsearchClient(transport); - } else { - client = currentClient; - } - if (Boolean.parseBoolean(createIndices)) { - config.elasticIndexValidation(client); + } else if (!username.isEmpty() || !password.isEmpty()) { + final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); + credentialsProvider.setCredentials( + AuthScope.ANY, new UsernamePasswordCredentials(username, password)); + clientBuilder.setHttpClientConfigCallback( + httpClientBuilder -> + httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider)); } - return client; + return clientBuilder.build(); } private HttpHost[] getHttpHosts() { @@ -173,47 +158,33 @@ private HttpHost[] getHttpHosts() { boolean portIsDefault = (port == 9200); if (hostUrlsIsDefault && (!hostIsDefault || !portIsDefault)) { logger.warning( - "Specifying elasticsearch.network.host and elasticsearch.http.port is deprecated, please consider using elasticsearch.host_urls instead."); + "Specifying elasticsearch.network.host and elasticsearch.http.port is deprecated," + + " please consider using elasticsearch.host_urls instead."); return new HttpHost[] {new HttpHost(host, port)}; } else { if (!hostIsDefault) { logger.warning( - "Only one of elasticsearch.host_urls and elasticsearch.network.host can be set, ignoring elasticsearch.network.host."); + "Only one of elasticsearch.host_urls and elasticsearch.network.host can be set," + + " ignoring elasticsearch.network.host."); } if (!portIsDefault) { logger.warning( - "Only one of elasticsearch.host_urls and elasticsearch.http.port can be set, ignoring elasticsearch.http.port."); + "Only one of elasticsearch.host_urls and elasticsearch.http.port can be set," + + " ignoring elasticsearch.http.port."); } return Arrays.stream(httpHostUrls).map(HttpHost::create).toArray(HttpHost[]::new); } } - @Bean({"searchClient"}) - public ElasticsearchClient getSearchClient() { - searchClient = createClient(searchClient, objectMapper, getHttpHosts(), createIndices, this); - return searchClient; - } - - @Bean({"indexClient"}) - public ElasticsearchClient getIndexClient() { - indexClient = createClient(indexClient, objectMapper, getHttpHosts(), createIndices, this); - return indexClient; - } - - @Override - public void contextInitialized(ServletContextEvent sce) { - logger.log(Level.INFO, "Initializing a new Transport clients."); - } - - @Override - public void contextDestroyed(ServletContextEvent sce) { - logger.log(Level.INFO, "Closing the default Transport clients."); - if (searchClient != null) searchClient.shutdown(); - if (indexClient != null) indexClient.shutdown(); + @PostConstruct + public void init() { + if (Boolean.parseBoolean(createIndices)) { + elasticIndexValidation(applicationContext.getBean(ElasticsearchClient.class)); + } } /** - * Create the olog indices and templates if they don't exist + * Create the ChannelFinder indices and templates if they don't exist * * @param client client connected to elasticsearch */ diff --git a/src/main/java/org/phoebus/channelfinder/configuration/PopulateDBConfiguration.java b/src/main/java/org/phoebus/channelfinder/configuration/PopulateDBConfiguration.java index 05375349..92171274 100644 --- a/src/main/java/org/phoebus/channelfinder/configuration/PopulateDBConfiguration.java +++ b/src/main/java/org/phoebus/channelfinder/configuration/PopulateDBConfiguration.java @@ -29,7 +29,6 @@ import org.phoebus.channelfinder.entity.Property; import org.phoebus.channelfinder.entity.Tag; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.annotation.Configuration; import tools.jackson.core.type.TypeReference; import tools.jackson.databind.ObjectMapper; @@ -113,9 +112,7 @@ public class PopulateDBConfiguration { @Autowired ElasticConfig esService; - @Autowired - @Qualifier("indexClient") - ElasticsearchClient client; + @Autowired ElasticsearchClient client; public static final ObjectMapper mapper = new ObjectMapper(); diff --git a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java index 29689d2a..239df5d9 100644 --- a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java +++ b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java @@ -84,7 +84,7 @@ public class ChannelRepository implements CrudRepository { public ChannelRepository( ElasticConfig esService, - @Qualifier("indexClient") ElasticsearchClient client, + ElasticsearchClient client, LegacyApiProperties legacyApiProperties) { this.esService = esService; this.client = client; diff --git a/src/main/java/org/phoebus/channelfinder/repository/PropertyRepository.java b/src/main/java/org/phoebus/channelfinder/repository/PropertyRepository.java index 8c1792c7..0bfec1ff 100644 --- a/src/main/java/org/phoebus/channelfinder/repository/PropertyRepository.java +++ b/src/main/java/org/phoebus/channelfinder/repository/PropertyRepository.java @@ -37,7 +37,6 @@ import org.phoebus.channelfinder.entity.Property; import org.phoebus.channelfinder.entity.Property.OnlyNameOwnerProperty; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.annotation.Configuration; import org.springframework.data.repository.CrudRepository; import org.springframework.http.HttpStatus; @@ -54,9 +53,7 @@ public class PropertyRepository implements CrudRepository { private static final Logger logger = Logger.getLogger(PropertyRepository.class.getName()); - @Autowired - @Qualifier("indexClient") - ElasticsearchClient client; + @Autowired ElasticsearchClient client; @Autowired ElasticConfig esService; diff --git a/src/main/java/org/phoebus/channelfinder/repository/TagRepository.java b/src/main/java/org/phoebus/channelfinder/repository/TagRepository.java index da9ae87a..1d90fef9 100644 --- a/src/main/java/org/phoebus/channelfinder/repository/TagRepository.java +++ b/src/main/java/org/phoebus/channelfinder/repository/TagRepository.java @@ -38,7 +38,6 @@ import org.phoebus.channelfinder.entity.Tag; import org.phoebus.channelfinder.entity.Tag.OnlyTag; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.annotation.Configuration; import org.springframework.data.repository.CrudRepository; import org.springframework.http.HttpStatus; @@ -57,9 +56,7 @@ public class TagRepository implements CrudRepository { @Autowired ElasticConfig esService; - @Autowired - @Qualifier("indexClient") - ElasticsearchClient client; + @Autowired ElasticsearchClient client; @Autowired ChannelRepository channelRepository; diff --git a/src/main/java/org/phoebus/channelfinder/service/InfoService.java b/src/main/java/org/phoebus/channelfinder/service/InfoService.java index 9bfb25da..b6db10b2 100644 --- a/src/main/java/org/phoebus/channelfinder/service/InfoService.java +++ b/src/main/java/org/phoebus/channelfinder/service/InfoService.java @@ -5,7 +5,8 @@ import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; -import org.phoebus.channelfinder.configuration.ElasticConfig; + +import co.elastic.clients.elasticsearch.ElasticsearchClient; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import tools.jackson.core.JacksonException; @@ -21,13 +22,13 @@ public class InfoService { private static final ObjectMapper objectMapper = JsonMapper.builder().enable(SerializationFeature.INDENT_OUTPUT).build(); - private final ElasticConfig esService; + private final ElasticsearchClient elasticsearchClient; @Value("${channelfinder.version:unknown}") private String version; - public InfoService(ElasticConfig esService) { - this.esService = esService; + public InfoService(ElasticsearchClient elasticsearchClient) { + this.elasticsearchClient = elasticsearchClient; } public String info() { @@ -37,8 +38,7 @@ public String info() { Map elasticInfo = new LinkedHashMap<>(); try { - var client = esService.getSearchClient(); - var response = client.info(); + var response = elasticsearchClient.info(); elasticInfo.put("status", "Connected"); elasticInfo.put("clusterName", response.clusterName()); elasticInfo.put("clusterUuid", response.clusterUuid()); diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 1002887a..f68eaeb6 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -83,8 +83,8 @@ elasticsearch.http.port=9200 # Elasticsearch sever. This can be used for authentication using tokens or API # keys. # -# For example, for token authentication, set this to ?Bearer abcd1234?, where -# ?abcd1234? is the token. For API key authentication, set this to the Base64 +# For example, for token authentication, set this to "Bearer abcd1234", where +# "abcd1234" is the token. For API key authentication, set this to the Base64 # encoded version of the concatenation of the API key ID and the API key # secret, separated by a colon. See # https://www.elastic.co/guide/en/elasticsearch/client/java-api-client/8.12/_other_authentication_methods.html diff --git a/src/test/java/org/phoebus/channelfinder/ElasticConfigIT.java b/src/test/java/org/phoebus/channelfinder/ElasticConfigIT.java index a731f13e..56baa51b 100644 --- a/src/test/java/org/phoebus/channelfinder/ElasticConfigIT.java +++ b/src/test/java/org/phoebus/channelfinder/ElasticConfigIT.java @@ -20,8 +20,8 @@ static void teardown(ElasticConfig elasticConfig) throws IOException { elasticConfig.getES_TAG_INDEX() }; for (String index : indexes) { - if (elasticConfig.getSearchClient().indices().exists(b -> b.index(index)).value()) { - elasticConfig.getSearchClient().indices().delete(b -> b.index(index)); + if (elasticConfig.getElasticsearchClient().indices().exists(b -> b.index(index)).value()) { + elasticConfig.getElasticsearchClient().indices().delete(b -> b.index(index)); } } } @@ -35,6 +35,6 @@ static void teardown(ElasticConfig elasticConfig) throws IOException { * @param elasticConfig Bean with configuration */ static void setUp(ElasticConfig elasticConfig) { - elasticConfig.elasticIndexValidation(elasticConfig.getSearchClient()); + elasticConfig.elasticIndexValidation(elasticConfig.getElasticsearchClient()); } }