Use configuration properties instead of environment. (#662)

* Use configuration properties instead of environment.

* Fix after review.

docs/src/main/asciidoc/_configprops.adoc

@@ -31,6 +31,7 @@
 |spring.cloud.loadbalancer.cache.ttl | 30s | Time To Live - time counted from writing of the record, after which cache entries are expired, expressed as a {@link Duration}. The property {@link String} has to be in keeping with the appropriate syntax as specified in Spring Boot <code>StringToDurationConverter</code>. @see <a href= "https://github.com/spring-projects/spring-boot/blob/master/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/convert/StringToDurationConverter.java">StringToDurationConverter.java</a>
 |spring.cloud.loadbalancer.retry.enabled | true | 
 |spring.cloud.loadbalancer.ribbon.enabled | true | Causes `RibbonLoadBalancerClient` to be used by default.
+|spring.cloud.loadbalancer.zone |  | 
 |spring.cloud.refresh.enabled | true | Enables autoconfiguration for the refresh scope and associated features.
 |spring.cloud.refresh.extra-refreshable | true | Additional class names for beans to post process into refresh scope.
 |spring.cloud.service-registry.auto-registration.enabled | true | Whether service auto-registration is enabled. Defaults to true.

docs/src/main/asciidoc/spring-cloud-commons.adoc

@@ -881,6 +881,8 @@ We use `DiscoveryClient`-specific `zone` configuration (for example, `eureka.ins
 
 NOTE: You can also override `DiscoveryClient`-specific zone setup by setting the value of `spring.cloud.loadbalancer.zone` property.
 
+WARNING: For the time being, only Eureka Discovery Client is instrumented to set the LoadBalancer zone. For other discovery client, set the `spring.cloud.loadbalancer.zone` property. More instrumentations coming shortly.
+
 NOTE: To determine the zone of a retrieved `ServiceInstance`, we check the value under the `"zone"` key in its metadata map.
 
 The `ZonePreferenceServiceInstanceListSupplier` filters retrieved instances and only returns the ones within the same zone.
@@ -901,11 +903,12 @@ public class CustomLoadBalancerConfiguration {
 	@Bean
 	public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier(
 			ReactiveDiscoveryClient discoveryClient, Environment environment,
+			LoadBalancerProperties loadBalancerProperties,
 			ApplicationContext context) {
 		DiscoveryClientServiceInstanceListSupplier firstDelegate = new DiscoveryClientServiceInstanceListSupplier(
 				discoveryClient, environment);
 		ZonePreferenceServiceInstanceListSupplier delegate = new ZonePreferenceServiceInstanceListSupplier(firstDelegate,
-				environment);
+				loadBalancerProperties);
 		ObjectProvider<LoadBalancerCacheManager> cacheManagerProvider = context
 				.getBeanProvider(LoadBalancerCacheManager.class);
 		if (cacheManagerProvider.getIfAvailable() != null) {
@@ -914,8 +917,7 @@ public class CustomLoadBalancerConfiguration {
 		}
 		return delegate;
 	}
-
-}
+	}
 ----
 
 [[spring-cloud-loadbalancer-starter]]
@@ -952,10 +954,9 @@ public class MyConfiguration {
 ----
 ====
 
-You can use this feature to instantiate different implementations of `ServiceInstanceListSupplier` or `ReactorLoadBalancer`,
-either written by you, or provided by us as alternatives (for example `ZonePreferenceServiceInstanceListSupplier`) to override the default setup.
+You can use this feature to instantiate different implementations of `ServiceInstanceListSupplier` or `ReactorLoadBalancer`, either written by you, or provided by us as alternatives (for example `ZonePreferenceServiceInstanceListSupplier`) to override the default setup.
 
-You can see an example of a custom cofiguration <<zoned-based-custom-loadbalancer-configuration,here>>.
+You can see an example of a custom configuration <<zoned-based-custom-loadbalancer-configuration,here>>.
 
 NOTE: The annotation `value` arguments (`stores` in the example above) specifies the service id of the service that we should send the requests to with the given custom configuration.
 

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2012-2019 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.client.loadbalancer.reactive;
+
+import org.springframework.boot.context.properties.ConfigurationProperties;
+
+/**
+ * A {@link ConfigurationProperties} bean for Spring Cloud LoadBalancer.
+ *
+ * @author Olga Maciaszek-Sharma
+ * @since 2.2.1
+ */
+@ConfigurationProperties("spring.cloud.loadbalancer")
+public class LoadBalancerProperties {
+
+	/**
+	 * A {@link String} representation of the <code>zone</code> used for filtering
+	 * instances by zoned load-balancing implementations.
+	 */
+	private String zone;
+
+	public String getZone() {
+		return zone;
+	}
+
+	public void setZone(String zone) {
+		this.zone = zone;
+	}
+
+}

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java

@@ -26,6 +26,7 @@ import org.springframework.cloud.client.ConditionalOnReactiveDiscoveryEnabled;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient;
+import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties;
 import org.springframework.cloud.loadbalancer.cache.LoadBalancerCacheManager;
 import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceListSupplier;
 import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceSupplier;
@@ -48,12 +49,18 @@ import org.springframework.core.env.Environment;
  * @author Tim Ysewyn
  */
 @Configuration(proxyBeanMethods = false)
-@EnableConfigurationProperties
+@EnableConfigurationProperties(LoadBalancerProperties.class)
 @ConditionalOnDiscoveryEnabled
 public class LoadBalancerClientConfiguration {
 
 	private static final int REACTIVE_SERVICE_INSTANCE_SUPPLIER_ORDER = 193827465;
 
+	@Bean
+	@ConditionalOnMissingBean
+	LoadBalancerProperties loadBalancerProperties() {
+		return new LoadBalancerProperties();
+	}
+
 	@Bean
 	@ConditionalOnMissingBean
 	public ReactorLoadBalancer<ServiceInstance> reactorServiceInstanceLoadBalancer(

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplier.java

@@ -23,7 +23,7 @@ import java.util.Map;
 import reactor.core.publisher.Flux;
 
 import org.springframework.cloud.client.ServiceInstance;
-import org.springframework.core.env.Environment;
+import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties;
 
 /**
  * An implementation of {@link ServiceInstanceListSupplier} that filters instances
@@ -42,14 +42,14 @@ public class ZonePreferenceServiceInstanceListSupplier
 
 	private final ServiceInstanceListSupplier delegate;
 
-	private final Environment environment;
+	private final LoadBalancerProperties loadBalancerProperties;
 
 	private String zone;
 
 	public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
-			Environment environment) {
+			LoadBalancerProperties loadBalancerProperties) {
 		this.delegate = delegate;
-		this.environment = environment;
+		this.loadBalancerProperties = loadBalancerProperties;
 	}
 
 	@Override
@@ -64,7 +64,7 @@ public class ZonePreferenceServiceInstanceListSupplier
 
 	private List<ServiceInstance> filteredByZone(List<ServiceInstance> serviceInstances) {
 		if (zone == null) {
-			zone = environment.getProperty("spring.cloud.loadbalancer.zone");
+			zone = loadBalancerProperties.getZone();
 		}
 		if (zone != null) {
 			List<ServiceInstance> filteredInstances = new ArrayList<>();

spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplierTests.java

@@ -27,7 +27,7 @@ import reactor.core.publisher.Flux;
 
 import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
-import org.springframework.core.env.Environment;
+import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
@@ -44,10 +44,10 @@ class ZonePreferenceServiceInstanceListSupplierTests {
 	private DiscoveryClientServiceInstanceListSupplier delegate = mock(
 			DiscoveryClientServiceInstanceListSupplier.class);
 
-	private Environment environment = mock(Environment.class);
+	private LoadBalancerProperties loadBalancerProperties = new LoadBalancerProperties();
 
 	private ZonePreferenceServiceInstanceListSupplier supplier = new ZonePreferenceServiceInstanceListSupplier(
-			delegate, environment);
+			delegate, loadBalancerProperties);
 
 	private ServiceInstance first = serviceInstance("test-1", buildZoneMetadata("zone1"));
 
@@ -63,8 +63,7 @@ class ZonePreferenceServiceInstanceListSupplierTests {
 
 	@Test
 	void shouldFilterInstancesByZone() {
-		when(environment.getProperty("spring.cloud.loadbalancer.zone"))
-				.thenReturn("zone1");
+		loadBalancerProperties.setZone("zone1");
 		when(delegate.get()).thenReturn(
 				Flux.just(Arrays.asList(first, second, third, fourth, fifth)));
 
@@ -79,8 +78,7 @@ class ZonePreferenceServiceInstanceListSupplierTests {
 
 	@Test
 	void shouldReturnAllInstancesIfNoZoneInstances() {
-		when(environment.getProperty("spring.cloud.loadbalancer.zone"))
-				.thenReturn("zone1");
+		loadBalancerProperties.setZone("zone1");
 		when(delegate.get()).thenReturn(Flux.just(Arrays.asList(third, fourth)));
 
 		List<ServiceInstance> filtered = supplier.get().blockFirst();
@@ -91,8 +89,7 @@ class ZonePreferenceServiceInstanceListSupplierTests {
 
 	@Test
 	void shouldNotThrowNPEIfNullInstanceMetadata() {
-		when(environment.getProperty("spring.cloud.loadbalancer.zone"))
-				.thenReturn("zone1");
+		loadBalancerProperties.setZone("zone1");
 		when(delegate.get()).thenReturn(
 				Flux.just(Collections.singletonList(serviceInstance("test-6", null))));
 		assertThatCode(() -> supplier.get().blockFirst()).doesNotThrowAnyException();
@@ -100,7 +97,7 @@ class ZonePreferenceServiceInstanceListSupplierTests {
 
 	@Test
 	void shouldReturnAllInstancesIfNoZone() {
-		when(environment.getProperty("spring.cloud.loadbalancer.zone")).thenReturn(null);
+		loadBalancerProperties.setZone(null);
 		when(delegate.get())
 				.thenReturn(Flux.just(Arrays.asList(first, second, third, fourth)));