From de0a14b7a5a3064c967ce90d012fcffc68436805 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Sat, 9 May 2020 09:16:41 -0400 Subject: [PATCH] Removes deprecated ServiceInstanceSupplier Fixes gh-753 --- .../LoadBalancerClientConfiguration.java | 40 ---------- .../core/CachingServiceInstanceSupplier.java | 77 ------------------- ...iscoveryClientServiceInstanceSupplier.java | 64 --------------- .../core/NoopServiceInstanceSupplier.java | 42 ---------- .../core/RoundRobinLoadBalancer.java | 45 +---------- .../core/ServiceInstanceSupplier.java | 34 -------- .../support/ServiceInstanceSuppliers.java | 58 -------------- .../LoadBalancerClientConfigurationTests.java | 14 ---- .../loadbalancer/core/LoadBalancerTests.java | 22 ++---- 9 files changed, 8 insertions(+), 388 deletions(-) delete mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/CachingServiceInstanceSupplier.java delete mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/DiscoveryClientServiceInstanceSupplier.java delete mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/NoopServiceInstanceSupplier.java delete mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceSupplier.java delete mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/ServiceInstanceSuppliers.java diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java index ed3b1d5d..d056e318 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java @@ -16,7 +16,6 @@ package org.springframework.cloud.loadbalancer.annotation; -import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -26,16 +25,11 @@ 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.loadbalancer.cache.LoadBalancerCacheManager; -import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceSupplier; -import org.springframework.cloud.loadbalancer.core.DiscoveryClientServiceInstanceSupplier; import org.springframework.cloud.loadbalancer.core.ReactorLoadBalancer; import org.springframework.cloud.loadbalancer.core.RoundRobinLoadBalancer; import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSuppliers; -import org.springframework.cloud.loadbalancer.core.ServiceInstanceSupplier; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; -import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -101,23 +95,6 @@ public class LoadBalancerClientConfiguration { .withHealthChecks().withCaching().build(context); } - @Bean - @ConditionalOnBean(ReactiveDiscoveryClient.class) - @ConditionalOnMissingBean - public ServiceInstanceSupplier discoveryClientServiceInstanceSupplier( - ReactiveDiscoveryClient discoveryClient, Environment env, - ApplicationContext context) { - DiscoveryClientServiceInstanceSupplier delegate = new DiscoveryClientServiceInstanceSupplier( - discoveryClient, env); - ObjectProvider cacheManagerProvider = context - .getBeanProvider(LoadBalancerCacheManager.class); - if (cacheManagerProvider.getIfAvailable() != null) { - return new CachingServiceInstanceSupplier(delegate, - cacheManagerProvider.getIfAvailable()); - } - return delegate; - } - } @Configuration(proxyBeanMethods = false) @@ -158,23 +135,6 @@ public class LoadBalancerClientConfiguration { .withHealthChecks().withCaching().build(context); } - @Bean - @ConditionalOnBean(DiscoveryClient.class) - @ConditionalOnMissingBean - public ServiceInstanceSupplier discoveryClientServiceInstanceSupplier( - DiscoveryClient discoveryClient, Environment env, - ApplicationContext context) { - DiscoveryClientServiceInstanceSupplier delegate = new DiscoveryClientServiceInstanceSupplier( - discoveryClient, env); - ObjectProvider cacheManagerProvider = context - .getBeanProvider(LoadBalancerCacheManager.class); - if (cacheManagerProvider.getIfAvailable() != null) { - return new CachingServiceInstanceSupplier(delegate, - cacheManagerProvider.getIfAvailable()); - } - return delegate; - } - } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/CachingServiceInstanceSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/CachingServiceInstanceSupplier.java deleted file mode 100644 index df1a8fdb..00000000 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/CachingServiceInstanceSupplier.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright 2012-2020 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.loadbalancer.core; - -import java.util.List; - -import reactor.cache.CacheFlux; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; - -import org.springframework.cache.Cache; -import org.springframework.cache.CacheManager; -import org.springframework.cloud.client.ServiceInstance; - -/** - * @deprecated Use {@link CachingServiceInstanceListSupplier} instead. - * @author Spencer Gibb - */ -@Deprecated -public class CachingServiceInstanceSupplier implements ServiceInstanceSupplier { - - /** - * Name of the service cache instance. - */ - public static final String SERVICE_INSTANCE_CACHE_NAME = CachingServiceInstanceSupplier.class - .getSimpleName() + "Cache"; - - private final ServiceInstanceSupplier delegate; - - private final Flux serviceInstances; - - @SuppressWarnings("unchecked") - public CachingServiceInstanceSupplier(ServiceInstanceSupplier delegate, - CacheManager cacheManager) { - this.delegate = delegate; - this.serviceInstances = CacheFlux.lookup(key -> { - // TODO: configurable cache name - Cache cache = cacheManager.getCache(SERVICE_INSTANCE_CACHE_NAME); - List list = cache.get(key, List.class); - if (list == null || list.isEmpty()) { - return Mono.empty(); - } - return Flux.fromIterable(list).materialize().collectList(); - }, delegate.getServiceId()).onCacheMissResume(this.delegate::get) - .andWriteWith((key, signals) -> Flux.fromIterable(signals).dematerialize() - .cast(ServiceInstance.class).collectList().doOnNext(instances -> { - Cache cache = cacheManager - .getCache(SERVICE_INSTANCE_CACHE_NAME); - cache.put(key, instances); - }).then()); - } - - @Override - public Flux get() { - return this.serviceInstances; - } - - @Override - public String getServiceId() { - return this.delegate.getServiceId(); - } - -} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/DiscoveryClientServiceInstanceSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/DiscoveryClientServiceInstanceSupplier.java deleted file mode 100644 index b57bb003..00000000 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/DiscoveryClientServiceInstanceSupplier.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2012-2020 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.loadbalancer.core; - -import reactor.core.publisher.Flux; -import reactor.core.scheduler.Schedulers; - -import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.client.discovery.DiscoveryClient; -import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient; -import org.springframework.core.env.Environment; - -import static org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory.PROPERTY_NAME; - -/** - * @deprecated Use {@link DiscoveryClientServiceInstanceListSupplier} instead. - * @author Spencer Gibb - * @author Tim Ysewyn - */ -@Deprecated -public class DiscoveryClientServiceInstanceSupplier implements ServiceInstanceSupplier { - - private final String serviceId; - - private final Flux serviceInstances; - - public DiscoveryClientServiceInstanceSupplier(DiscoveryClient delegate, - Environment environment) { - this.serviceId = environment.getProperty(PROPERTY_NAME); - this.serviceInstances = Flux - .defer(() -> Flux.fromIterable(delegate.getInstances(serviceId))) - .subscribeOn(Schedulers.boundedElastic()); - } - - public DiscoveryClientServiceInstanceSupplier(ReactiveDiscoveryClient delegate, - Environment environment) { - this.serviceId = environment.getProperty(PROPERTY_NAME); - this.serviceInstances = delegate.getInstances(serviceId); - } - - @Override - public Flux get() { - return this.serviceInstances; - } - - public String getServiceId() { - return this.serviceId; - } - -} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/NoopServiceInstanceSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/NoopServiceInstanceSupplier.java deleted file mode 100644 index 4cb891b1..00000000 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/NoopServiceInstanceSupplier.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2012-2020 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.loadbalancer.core; - -import reactor.core.publisher.Flux; - -import org.springframework.cloud.client.ServiceInstance; - -/** - * A no-op implementation of {@link ServiceInstanceSupplier}. - * - * @author Olga Maciaszek-Sharma - * @deprecated Use {@link NoopServiceInstanceListSupplier} instead. - */ -@Deprecated -public class NoopServiceInstanceSupplier implements ServiceInstanceSupplier { - - @Override - public String getServiceId() { - return ""; - } - - @Override - public Flux get() { - return Flux.empty(); - } - -} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java index be7a59df..5a641079 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java @@ -43,25 +43,10 @@ public class RoundRobinLoadBalancer implements ReactorServiceInstanceLoadBalance private final AtomicInteger position; - @Deprecated - private ObjectProvider serviceInstanceSupplier; - private ObjectProvider serviceInstanceListSupplierProvider; private final String serviceId; - /** - * @param serviceId id of the service for which to choose an instance - * @param serviceInstanceSupplier a provider of {@link ServiceInstanceSupplier} that - * will be used to get available instances - * @deprecated Use {@link #RoundRobinLoadBalancer(ObjectProvider, String)}} instead. - */ - @Deprecated - public RoundRobinLoadBalancer(String serviceId, - ObjectProvider serviceInstanceSupplier) { - this(serviceId, serviceInstanceSupplier, new Random().nextInt(1000)); - } - /** * @param serviceInstanceListSupplierProvider a provider of * {@link ServiceInstanceListSupplier} that will be used to get available instances @@ -87,39 +72,15 @@ public class RoundRobinLoadBalancer implements ReactorServiceInstanceLoadBalance this.position = new AtomicInteger(seedPosition); } - /** - * @param serviceId id of the service for which to choose an instance - * @param serviceInstanceSupplier a provider of {@link ServiceInstanceSupplier} that - * will be used to get available instances - * @param seedPosition Round Robin element position marker - * @deprecated Use {@link #RoundRobinLoadBalancer(ObjectProvider, String, int)}} - * instead. - */ - @Deprecated - public RoundRobinLoadBalancer(String serviceId, - ObjectProvider serviceInstanceSupplier, - int seedPosition) { - this.serviceId = serviceId; - this.serviceInstanceSupplier = serviceInstanceSupplier; - this.position = new AtomicInteger(seedPosition); - } - @SuppressWarnings("rawtypes") @Override // see original // https://github.com/Netflix/ocelli/blob/master/ocelli-core/ // src/main/java/netflix/ocelli/loadbalancer/RoundRobinLoadBalancer.java public Mono> choose(Request request) { - // TODO: move supplier to Request? - // Temporary conditional logic till deprecated members are removed. - if (serviceInstanceListSupplierProvider != null) { - ServiceInstanceListSupplier supplier = serviceInstanceListSupplierProvider - .getIfAvailable(NoopServiceInstanceListSupplier::new); - return supplier.get().next().map(this::getInstanceResponse); - } - ServiceInstanceSupplier supplier = this.serviceInstanceSupplier - .getIfAvailable(NoopServiceInstanceSupplier::new); - return supplier.get().collectList().map(this::getInstanceResponse); + ServiceInstanceListSupplier supplier = serviceInstanceListSupplierProvider + .getIfAvailable(NoopServiceInstanceListSupplier::new); + return supplier.get().next().map(this::getInstanceResponse); } private Response getInstanceResponse( diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceSupplier.java deleted file mode 100644 index a432e762..00000000 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceSupplier.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2012-2020 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.loadbalancer.core; - -import java.util.function.Supplier; - -import reactor.core.publisher.Flux; - -import org.springframework.cloud.client.ServiceInstance; - -/** - * @author Spencer Gibb - * @deprecated Use {@link ServiceInstanceListSupplier} instead. - */ -@Deprecated -public interface ServiceInstanceSupplier extends Supplier> { - - String getServiceId(); - -} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/ServiceInstanceSuppliers.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/ServiceInstanceSuppliers.java deleted file mode 100644 index 9d45339b..00000000 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/ServiceInstanceSuppliers.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2012-2020 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.loadbalancer.support; - -import reactor.core.publisher.Flux; - -import org.springframework.beans.factory.ObjectProvider; -import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.loadbalancer.core.ServiceInstanceSupplier; - -/** - * Utility class for service instances. - * - * @author Spencer Gibb - * @deprecated Use {@link ServiceInstanceListSuppliers} instead. - */ -@Deprecated -public final class ServiceInstanceSuppliers { - - private ServiceInstanceSuppliers() { - throw new IllegalStateException("Can't instantiate a utility class"); - } - - public static ServiceInstanceSupplier from(String serviceId, - ServiceInstance... instances) { - return new ServiceInstanceSupplier() { - @Override - public Flux get() { - return Flux.just(instances); - } - - @Override - public String getServiceId() { - return serviceId; - } - }; - } - - public static ObjectProvider toProvider(String serviceId, - ServiceInstance... instances) { - return new SimpleObjectProvider<>(from(serviceId, instances)); - } - -} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java index 5c4ed56b..6f0d62c4 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java @@ -26,12 +26,10 @@ import org.springframework.cloud.client.loadbalancer.LoadBalanced; import org.springframework.cloud.loadbalancer.config.LoadBalancerAutoConfiguration; import org.springframework.cloud.loadbalancer.config.LoadBalancerCacheAutoConfiguration; import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceListSupplier; -import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceSupplier; import org.springframework.cloud.loadbalancer.core.DelegatingServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.DiscoveryClientServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.HealthCheckServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier; -import org.springframework.cloud.loadbalancer.core.ServiceInstanceSupplier; import org.springframework.cloud.loadbalancer.core.ZonePreferenceServiceInstanceListSupplier; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -150,18 +148,6 @@ class LoadBalancerClientConfigurationTests { }); } - @Test - void shouldInstantiateServiceInstanceSupplierRegardlessOfConfigurationProperty() { - reactiveDiscoveryClientRunner - .withPropertyValues( - "spring.cloud.loadbalancer.configurations=zone-preference") - .run(context -> { - ServiceInstanceSupplier supplier = context - .getBean(ServiceInstanceSupplier.class); - then(supplier).isInstanceOf(CachingServiceInstanceSupplier.class); - }); - } - @Configuration protected static class TestConfig { diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/LoadBalancerTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/LoadBalancerTests.java index 5fc6b3df..24488ffd 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/LoadBalancerTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/LoadBalancerTests.java @@ -44,7 +44,6 @@ import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient; import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.cloud.loadbalancer.support.ServiceInstanceListSuppliers; -import org.springframework.cloud.loadbalancer.support.ServiceInstanceSuppliers; import org.springframework.context.annotation.Bean; import org.springframework.core.ResolvableType; import org.springframework.core.env.Environment; @@ -122,22 +121,6 @@ public class LoadBalancerTests { @Test public void staticConfigurationWorks() { - String serviceId = "test1"; - RoundRobinLoadBalancer loadBalancer = new RoundRobinLoadBalancer(serviceId, - ServiceInstanceSuppliers.toProvider(serviceId, - instance(serviceId, "1host", false), - instance(serviceId, "2host-secure", true)), - -1); - assertLoadBalancer(loadBalancer, Arrays.asList("1host", "2host-secure")); - } - - private static DefaultServiceInstance instance(String serviceId, String host, - boolean secure) { - return new DefaultServiceInstance(serviceId, serviceId, host, 80, secure); - } - - @Test - public void staticConfigurationWorksWithServiceInstanceListSupplier() { String serviceId = "test1"; RoundRobinLoadBalancer loadBalancer = new RoundRobinLoadBalancer( ServiceInstanceListSuppliers.toProvider(serviceId, @@ -147,6 +130,11 @@ public class LoadBalancerTests { assertLoadBalancer(loadBalancer, Arrays.asList("1host", "2host-secure")); } + private static DefaultServiceInstance instance(String serviceId, String host, + boolean secure) { + return new DefaultServiceInstance(serviceId, serviceId, host, 80, secure); + } + @SuppressWarnings("ConstantConditions") @Test public void canPassHintViaRequest() {