From 235c50b4dc5da4c2f3838eb090d31e97196d4964 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Wed, 13 May 2020 23:52:13 -0400 Subject: [PATCH] Adds ConfigServerInstanceProvider.Function interface. This allows for another implementation besides DiscoveryClient. Moves SmartApplicationListener from bootstrap configuration to HeartbeatListener class. --- .../client/ConfigServerInstanceProvider.java | 17 +- ...ntConfigServiceBootstrapConfiguration.java | 154 ++++++++++-------- ...figServiceBootstrapConfigurationTests.java | 18 +- ...figServiceBootstrapConfigurationTests.java | 23 +++ 4 files changed, 137 insertions(+), 75 deletions(-) diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java index 3478eb31..8a53bf01 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java @@ -34,16 +34,20 @@ public class ConfigServerInstanceProvider { private static Log logger = LogFactory.getLog(ConfigServerInstanceProvider.class); - private final DiscoveryClient client; + private final Function function; public ConfigServerInstanceProvider(DiscoveryClient client) { - this.client = client; + this.function = client::getInstances; + } + + public ConfigServerInstanceProvider(Function function) { + this.function = function; } @Retryable(interceptor = "configServerRetryInterceptor") public List getConfigServerInstances(String serviceId) { logger.debug("Locating configserver (" + serviceId + ") via discovery"); - List instances = this.client.getInstances(serviceId); + List instances = this.function.apply(serviceId); if (instances.isEmpty()) { throw new IllegalStateException( "No instances found of configserver (" + serviceId + ")"); @@ -53,4 +57,11 @@ public class ConfigServerInstanceProvider { return instances; } + @FunctionalInterface + public interface Function { + + List apply(String serviceId); + + } + } diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfiguration.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfiguration.java index e9c423df..4d3815ed 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfiguration.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfiguration.java @@ -22,6 +22,7 @@ import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.ServiceInstance; @@ -48,100 +49,119 @@ import org.springframework.context.event.SmartApplicationListener; @Configuration(proxyBeanMethods = false) @Import({ UtilAutoConfiguration.class }) @EnableDiscoveryClient -public class DiscoveryClientConfigServiceBootstrapConfiguration - implements SmartApplicationListener { +public class DiscoveryClientConfigServiceBootstrapConfiguration { private static Log logger = LogFactory .getLog(DiscoveryClientConfigServiceBootstrapConfiguration.class); - @Autowired - private ConfigClientProperties config; - - @Autowired - private ConfigServerInstanceProvider instanceProvider; - - private HeartbeatMonitor monitor = new HeartbeatMonitor(); - @Bean public ConfigServerInstanceProvider configServerInstanceProvider( - DiscoveryClient discoveryClient) { - return new ConfigServerInstanceProvider(discoveryClient); - } - - @Override - public boolean supportsEventType(Class eventType) { - return ContextRefreshedEvent.class.isAssignableFrom(eventType) - || HeartbeatEvent.class.isAssignableFrom(eventType); - } - - @Override - public void onApplicationEvent(ApplicationEvent event) { - if (event instanceof ContextRefreshedEvent) { - startup((ContextRefreshedEvent) event); + ObjectProvider function, + ObjectProvider discoveryClient) { + ConfigServerInstanceProvider.Function fn = function.getIfAvailable(); + if (fn != null) { + return new ConfigServerInstanceProvider(fn); } - else if (event instanceof HeartbeatEvent) { - heartbeat((HeartbeatEvent) event); + DiscoveryClient client = discoveryClient.getIfAvailable(); + if (client == null) { + throw new IllegalStateException( + "ConfigServerInstanceProvider reqiures a DiscoveryClient or Function"); } + return new ConfigServerInstanceProvider(client); } - public void startup(ContextRefreshedEvent event) { - refresh(); + @Bean + public SmartApplicationListener heartbeatListener( + ConfigServerInstanceProvider provider) { + return new HeartbeatListener(); } - public void heartbeat(HeartbeatEvent event) { - if (this.monitor.update(event.getValue())) { + private static class HeartbeatListener implements SmartApplicationListener { + + @Autowired + private ConfigClientProperties config; + + @Autowired + private ConfigServerInstanceProvider instanceProvider; + + private HeartbeatMonitor monitor = new HeartbeatMonitor(); + + @Override + public boolean supportsEventType(Class eventType) { + return ContextRefreshedEvent.class.isAssignableFrom(eventType) + || HeartbeatEvent.class.isAssignableFrom(eventType); + } + + @Override + public void onApplicationEvent(ApplicationEvent event) { + if (event instanceof ContextRefreshedEvent) { + startup((ContextRefreshedEvent) event); + } + else if (event instanceof HeartbeatEvent) { + heartbeat((HeartbeatEvent) event); + } + } + + public void startup(ContextRefreshedEvent event) { refresh(); } - } - private void refresh() { - try { - String serviceId = this.config.getDiscovery().getServiceId(); - List listOfUrls = new ArrayList<>(); - List serviceInstances = this.instanceProvider - .getConfigServerInstances(serviceId); + public void heartbeat(HeartbeatEvent event) { + if (this.monitor.update(event.getValue())) { + refresh(); + } + } - for (int i = 0; i < serviceInstances.size(); i++) { + private void refresh() { + try { + String serviceId = this.config.getDiscovery().getServiceId(); + List listOfUrls = new ArrayList<>(); + List serviceInstances = this.instanceProvider + .getConfigServerInstances(serviceId); - ServiceInstance server = serviceInstances.get(i); - String url = getHomePage(server); + for (int i = 0; i < serviceInstances.size(); i++) { - if (server.getMetadata().containsKey("password")) { - String user = server.getMetadata().get("user"); - user = user == null ? "user" : user; - this.config.setUsername(user); - String password = server.getMetadata().get("password"); - this.config.setPassword(password); - } + ServiceInstance server = serviceInstances.get(i); + String url = getHomePage(server); - if (server.getMetadata().containsKey("configPath")) { - String path = server.getMetadata().get("configPath"); - if (url.endsWith("/") && path.startsWith("/")) { - url = url.substring(0, url.length() - 1); + if (server.getMetadata().containsKey("password")) { + String user = server.getMetadata().get("user"); + user = user == null ? "user" : user; + this.config.setUsername(user); + String password = server.getMetadata().get("password"); + this.config.setPassword(password); } - url = url + path; + + if (server.getMetadata().containsKey("configPath")) { + String path = server.getMetadata().get("configPath"); + if (url.endsWith("/") && path.startsWith("/")) { + url = url.substring(0, url.length() - 1); + } + url = url + path; + } + + listOfUrls.add(url); } - listOfUrls.add(url); - } + String[] uri = new String[listOfUrls.size()]; + uri = listOfUrls.toArray(uri); + this.config.setUri(uri); - String[] uri = new String[listOfUrls.size()]; - uri = listOfUrls.toArray(uri); - this.config.setUri(uri); - - } - catch (Exception ex) { - if (this.config.isFailFast()) { - throw ex; } - else { - logger.warn("Could not locate configserver via discovery", ex); + catch (Exception ex) { + if (this.config.isFailFast()) { + throw ex; + } + else { + logger.warn("Could not locate configserver via discovery", ex); + } } } - } - private String getHomePage(ServiceInstance server) { - return server.getUri().toString() + "/"; + private String getHomePage(ServiceInstance server) { + return server.getUri().toString() + "/"; + } + } } diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java index d7086af9..8a26a68c 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java @@ -65,7 +65,7 @@ public abstract class BaseDiscoveryClientConfigServiceBootstrapConfigurationTest void givenDiscoveryClientReturnsInfo() { given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) - .willReturn(Arrays.asList(this.info)); + .willReturn(Collections.singletonList(this.info)); } void givenDiscoveryClientReturnsInfoForMultipleInstances(ServiceInstance info1, @@ -78,7 +78,7 @@ public abstract class BaseDiscoveryClientConfigServiceBootstrapConfigurationTest given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) .willReturn(Collections.emptyList()) .willReturn(Collections.emptyList()) - .willReturn(Arrays.asList(this.info)); + .willReturn(Collections.singletonList(this.info)); } void expectNoInstancesOfConfigServerException() { @@ -128,16 +128,24 @@ public abstract class BaseDiscoveryClientConfigServiceBootstrapConfigurationTest } void setup(String... env) { + setup(true, true, env); + } + + void setup(boolean refresh, boolean registerDiscoveryClient, String... env) { this.context = new AnnotationConfigApplicationContext(); TestPropertyValues.of(env).applyTo(this.context); TestPropertyValues.of("eureka.client.enabled=false").applyTo(this.context); - this.context.getDefaultListableBeanFactory().registerSingleton("discoveryClient", - this.client); + if (registerDiscoveryClient) { + this.context.getDefaultListableBeanFactory() + .registerSingleton("discoveryClient", this.client); + } this.context.register(UtilAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, DiscoveryClientConfigServiceBootstrapConfiguration.class, ConfigServiceBootstrapConfiguration.class, ConfigClientProperties.class); - this.context.refresh(); + if (refresh) { + this.context.refresh(); + } } } diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationTests.java index 4a8ed67f..b788dbff 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationTests.java @@ -16,6 +16,8 @@ package org.springframework.cloud.config.client; +import java.util.Collections; + import org.junit.Test; import org.springframework.cloud.client.DefaultServiceInstance; @@ -26,6 +28,10 @@ import org.springframework.cloud.config.client.ConfigClientProperties.Credential import org.springframework.context.annotation.AnnotationConfigApplicationContext; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.springframework.cloud.config.client.ConfigClientProperties.Discovery.DEFAULT_CONFIG_SERVER; /** * @author Dave Syer @@ -56,6 +62,23 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests expectConfigClientPropertiesHasConfigurationFromEureka(); } + @Test + public void configServerInstanceProviderFunction() { + ConfigServerInstanceProvider.Function function = mock( + ConfigServerInstanceProvider.Function.class); + given(function.apply(DEFAULT_CONFIG_SERVER)) + .willReturn(Collections.singletonList(this.info)); + + setup(false, false, "spring.cloud.config.discovery.enabled=true"); + this.context.getDefaultListableBeanFactory().registerSingleton("myFunction", + function); + this.context.refresh(); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + verify(function).apply(DEFAULT_CONFIG_SERVER); + expectConfigClientPropertiesHasConfigurationFromEureka(); + } + @Test public void onWhenHeartbeat() throws Exception { setup("spring.cloud.config.discovery.enabled=true");