Call get request on delegates

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

@@ -933,6 +933,11 @@ to `false`.
 
 WARNING: Although the basic, non-cached, implementation is useful for prototyping and testing, it's much less efficient than the cached versions, so we recommend always using the cached version in production. If the caching is already done by the `DiscoveryClient` implementation, for example `EurekaDiscoveryClient`, the load-balancer caching should be disabled to prevent double caching.
 
+====
+
+NOTE: When you create your own configuration, if you use `CachingServiceInstanceListSupplier` make sure to place it in the hierarchy directly after the supplier that retrieves the instances over the network, for example, `DiscoveryClientServiceInstanceListSupplier`, before any other filtering suppliers.
+
+====
 === Zone-Based Load-Balancing
 
 To enable zone-based load-balancing, we provide the `ZonePreferenceServiceInstanceListSupplier`.
@@ -950,7 +955,7 @@ If the zone is `null` or there are no instances within the same zone, it returns
 In order to use the zone-based load-balancing approach, you will have to instantiate a `ZonePreferenceServiceInstanceListSupplier` bean in a <<custom-loadbalancer-configuration,custom configuration>>.
 
 We use delegates to work with `ServiceInstanceListSupplier` beans.
-We suggest passing a `DiscoveryClientServiceInstanceListSupplier` delegate in the constructor of `ZonePreferenceServiceInstanceListSupplier` and, in turn, wrapping the latter with a `CachingServiceInstanceListSupplier` to leverage <<loadbalancer-caching, LoadBalancer caching mechanism>>.
+We suggest using a `DiscoveryClientServiceInstanceListSupplier` delegate, wrapping it with a `CachingServiceInstanceListSupplier` to leverage <<loadbalancer-caching, LoadBalancer caching mechanism>>, and then passing the resulting bean in the constructor of `ZonePreferenceServiceInstanceListSupplier`.
 
 You can use this sample configuration to set it up:
 
@@ -964,8 +969,8 @@ public class CustomLoadBalancerConfiguration {
 			ConfigurableApplicationContext context) {
 		return ServiceInstanceListSupplier.builder()
 					.withDiscoveryClient()
+                    .withCaching()
 					.withZonePreference()
-					.withCaching()
 					.build(context);
 	}
 }
@@ -1026,6 +1031,12 @@ You can also pass your own `WebClient` or `RestTemplate` instance to be used for
 
 WARNING: `HealthCheckServiceInstanceListSupplier` has its own caching mechanism based on Reactor Flux `replay()`. Therefore, if it's being used, you may want to skip wrapping that supplier with `CachingServiceInstanceListSupplier`.
 
+====
+
+NOTE: When you create your own configuration, `HealthCheckServiceInstanceListSupplier`, make sure to place it in the hierarchy directly after the supplier that retrieves the instances over the network, for example, `DiscoveryClientServiceInstanceListSupplier`, before any other filtering suppliers.
+
+====
+
 === Same instance preference for LoadBalancer
 
 You can set up the LoadBalancer in such a way that it prefers the instance that was previously selected, if that instance is available.
@@ -1110,8 +1121,8 @@ public class CustomLoadBalancerConfiguration {
 			ConfigurableApplicationContext context) {
 		return ServiceInstanceListSupplier.builder()
 					.withDiscoveryClient()
+                    .withCaching()
 					.withHints()
-					.withCaching()
 					.build(context);
 	}
 }
@@ -1221,11 +1232,18 @@ public class MyConfiguration {
 	}
 }
 ----
+====
 
 NOTE: The classes you pass as `@LoadBalancerClient` or `@LoadBalancerClients` configuration arguments should either not be annotated with `@Configuration` or be outside component scan scope.
 
 ====
 
+====
+
+NOTE: When you create your own configuration, if you use `CachingServiceInstanceListSupplier` or `HealthCheckServiceInstanceListSupplier`, makes sure to use one of them, not both, and make sure to place it in the hierarchy directly after the supplier that retrieves the instances over the network, for example, `DiscoveryClientServiceInstanceListSupplier`, before any other filtering suppliers.
+
+====
+
 [[loadbalancer-lifecycle]]
 === Spring Cloud LoadBalancer Lifecycle
 
@@ -1301,6 +1319,8 @@ The per-client configuration properties work for most of the properties, apart f
 
 NOTE: For the properties where maps where already used, where you can specify a different value per-client without using the `clients` keyword (for example, `hints`, `health-check.path`), we have kept that behaviour in order to keep the library backwards compatible. It will be modified in the next major release.
 
+NOTE: Starting with `3.1.7` in `2021.0.x` release train, `4.0.4` in `2022.0.x`  release train and `4.1.0` in the `2023.0.x` release train, we have introduced the `callGetWithRequestOnDelegates` flag in `LoadBalancerProperties`. If this flag is set to `true`, `ServiceInstanceListSupplier#get(Request request)` method will be implemented to call `delegate.get(request)` in classes assignable from `DelegatingServiceInstanceListSupplier` that don't already implement that method, with the exclusion of `CachingServiceInstanceListSupplier` and `HealthCheckServiceInstanceListSupplier`, which should be placed in the instance supplier hierarchy directly after the supplier performing instance retrieval over the network, before any request-based filtering is done. For `3.1.x` and `4.0.x` the flag is set to `false` by default, and since `4.1.0` it's going to be set to `true` by default.
+
 == Spring Cloud Circuit Breaker
 
 include::spring-cloud-circuitbreaker.adoc[leveloffset=+1]

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

@@ -73,6 +73,19 @@ public class LoadBalancerProperties {
 	 */
 	private boolean useRawStatusCodeInResponseData;
 
+	/**
+	 * If this flag is set to {@code true},
+	 * {@code ServiceInstanceListSupplier#get(Request request)} method will be implemented
+	 * to call {@code delegate.get(request)} in classes assignable from
+	 * {@code DelegatingServiceInstanceListSupplier} that don't already implement that
+	 * method, with the exclusion of {@code CachingServiceInstanceListSupplier} and
+	 * {@code HealthCheckServiceInstanceListSupplier}, which should be placed in the
+	 * instance supplier hierarchy directly after the supplier performing instance
+	 * retrieval over the network, before any request-based filtering is done. Note: in
+	 * 4.1, this behaviour will become the default
+	 */
+	private boolean callGetWithRequestOnDelegates;
+
 	public HealthCheck getHealthCheck() {
 		return healthCheck;
 	}
@@ -134,6 +147,36 @@ public void setUseRawStatusCodeInResponseData(boolean useRawStatusCodeInResponse
 		this.useRawStatusCodeInResponseData = useRawStatusCodeInResponseData;
 	}
 
+	/**
+	 * If this flag is set to {@code true},
+	 * {@code ServiceInstanceListSupplier#get(Request request)} method will be implemented
+	 * to call {@code delegate.get(request)} in classes assignable from
+	 * {@code DelegatingServiceInstanceListSupplier} that don't already implement that
+	 * method, with the exclusion of {@code CachingServiceInstanceListSupplier} and
+	 * {@code HealthCheckServiceInstanceListSupplier}, which should be placed in the
+	 * instance supplier hierarchy directly after the supplier performing instance
+	 * retrieval over the network, before any request-based filtering is done. Note: in
+	 * 4.1, this behaviour will become the default
+	 */
+	public boolean isCallGetWithRequestOnDelegates() {
+		return callGetWithRequestOnDelegates;
+	}
+
+	/**
+	 * If this flag is set to {@code true},
+	 * {@code ServiceInstanceListSupplier#get(Request request)} method will be implemented
+	 * to call {@code delegate.get(request)} in classes assignable from
+	 * {@code DelegatingServiceInstanceListSupplier} that don't already implement that
+	 * method, with the exclusion of {@code CachingServiceInstanceListSupplier} and
+	 * {@code HealthCheckServiceInstanceListSupplier}, which should be placed in the
+	 * instance supplier hierarchy directly after the supplier performing instance
+	 * retrieval over the network, before any request-based filtering is done. Note: in
+	 * 4.1, this behaviour will become the default
+	 */
+	public void setCallGetWithRequestOnDelegates(boolean callGetWithRequestOnDelegates) {
+		this.callGetWithRequestOnDelegates = callGetWithRequestOnDelegates;
+	}
+
 	public static class StickySession {
 
 		/**

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

@@ -93,7 +93,7 @@ public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier(
 		@Conditional(ZonePreferenceConfigurationCondition.class)
 		public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceListSupplier(
 				ConfigurableApplicationContext context) {
-			return ServiceInstanceListSupplier.builder().withDiscoveryClient().withZonePreference().withCaching()
+			return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching().withZonePreference()
 					.build(context);
 		}
 
@@ -119,8 +119,8 @@ public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceList
 		@Conditional(RequestBasedStickySessionConfigurationCondition.class)
 		public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServiceInstanceListSupplier(
 				ConfigurableApplicationContext context) {
-			return ServiceInstanceListSupplier.builder().withDiscoveryClient().withRequestBasedStickySession()
-					.withCaching().build(context);
+			return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching()
+					.withRequestBasedStickySession().build(context);
 		}
 
 		@Bean
@@ -129,8 +129,8 @@ public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServi
 		@Conditional(SameInstancePreferenceConfigurationCondition.class)
 		public ServiceInstanceListSupplier sameInstancePreferenceServiceInstanceListSupplier(
 				ConfigurableApplicationContext context) {
-			return ServiceInstanceListSupplier.builder().withDiscoveryClient().withSameInstancePreference()
-					.withCaching().build(context);
+			return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching()
+					.withSameInstancePreference().build(context);
 		}
 
 	}
@@ -155,8 +155,8 @@ public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier(
 		@Conditional(ZonePreferenceConfigurationCondition.class)
 		public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceListSupplier(
 				ConfigurableApplicationContext context) {
-			return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withZonePreference()
-					.withCaching().build(context);
+			return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
+					.withZonePreference().build(context);
 		}
 
 		@Bean
@@ -175,8 +175,8 @@ public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceList
 		@Conditional(RequestBasedStickySessionConfigurationCondition.class)
 		public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServiceInstanceListSupplier(
 				ConfigurableApplicationContext context) {
-			return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withRequestBasedStickySession()
-					.withCaching().build(context);
+			return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
+					.withRequestBasedStickySession().build(context);
 		}
 
 		@Bean
@@ -185,8 +185,8 @@ public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServi
 		@Conditional(SameInstancePreferenceConfigurationCondition.class)
 		public ServiceInstanceListSupplier sameInstancePreferenceServiceInstanceListSupplier(
 				ConfigurableApplicationContext context) {
-			return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withSameInstancePreference()
-					.withCaching().build(context);
+			return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
+					.withSameInstancePreference().build(context);
 		}
 
 	}

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

@@ -38,12 +38,12 @@ public DelegatingServiceInstanceListSupplier(ServiceInstanceListSupplier delegat
 	}
 
 	public ServiceInstanceListSupplier getDelegate() {
-		return this.delegate;
+		return delegate;
 	}
 
 	@Override
 	public String getServiceId() {
-		return this.delegate.getServiceId();
+		return delegate.getServiceId();
 	}
 
 	@Override

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

@@ -24,6 +24,8 @@
 import reactor.core.publisher.Flux;
 
 import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.client.loadbalancer.Request;
+import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer;
 
 /**
  * An implementation of {@link ServiceInstanceListSupplier} that selects the previously
@@ -39,10 +41,19 @@ public class SameInstancePreferenceServiceInstanceListSupplier extends Delegatin
 
 	private ServiceInstance previouslyReturnedInstance;
 
+	private boolean callGetWithRequestOnDelegates;
+
 	public SameInstancePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate) {
 		super(delegate);
 	}
 
+	public SameInstancePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
+			ReactiveLoadBalancer.Factory<ServiceInstance> loadBalancerClientFactory) {
+		super(delegate);
+		callGetWithRequestOnDelegates = loadBalancerClientFactory.getProperties(getServiceId())
+				.isCallGetWithRequestOnDelegates();
+	}
+
 	@Override
 	public String getServiceId() {
 		return delegate.getServiceId();
@@ -53,6 +64,14 @@ public Flux<List<ServiceInstance>> get() {
 		return delegate.get().map(this::filteredBySameInstancePreference);
 	}
 
+	@Override
+	public Flux<List<ServiceInstance>> get(Request request) {
+		if (callGetWithRequestOnDelegates) {
+			return delegate.get(request).map(this::filteredBySameInstancePreference);
+		}
+		return get();
+	}
+
 	private List<ServiceInstance> filteredBySameInstancePreference(List<ServiceInstance> serviceInstances) {
 		if (previouslyReturnedInstance != null && serviceInstances.contains(previouslyReturnedInstance)) {
 			if (LOG.isDebugEnabled()) {

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

@@ -57,8 +57,6 @@ public final class ServiceInstanceListSupplierBuilder {
 
 	private Creator baseCreator;
 
-	private DelegateCreator cachingCreator;
-
 	private final List<DelegateCreator> creators = new ArrayList<>();
 
 	ServiceInstanceListSupplierBuilder() {
@@ -148,8 +146,10 @@ public ServiceInstanceListSupplierBuilder withHealthChecks(WebClient webClient)
 	 * @return the {@link ServiceInstanceListSupplierBuilder} object
 	 */
 	public ServiceInstanceListSupplierBuilder withSameInstancePreference() {
-		DelegateCreator creator = (context,
-				delegate) -> new SameInstancePreferenceServiceInstanceListSupplier(delegate);
+		DelegateCreator creator = (context, delegate) -> {
+			LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
+			return new SameInstancePreferenceServiceInstanceListSupplier(delegate, loadBalancerClientFactory);
+		};
 		this.creators.add(creator);
 		return this;
 	}
@@ -191,8 +191,9 @@ public ServiceInstanceListSupplierBuilder withBlockingHealthChecks(RestTemplate
 	 */
 	public ServiceInstanceListSupplierBuilder withZonePreference() {
 		DelegateCreator creator = (context, delegate) -> {
+			LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
 			LoadBalancerZoneConfig zoneConfig = context.getBean(LoadBalancerZoneConfig.class);
-			return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig);
+			return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig, loadBalancerClientFactory);
 		};
 		this.creators.add(creator);
 		return this;
@@ -206,8 +207,9 @@ public ServiceInstanceListSupplierBuilder withZonePreference() {
 	 */
 	public ServiceInstanceListSupplierBuilder withZonePreference(String zoneName) {
 		DelegateCreator creator = (context, delegate) -> {
+			LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
 			LoadBalancerZoneConfig zoneConfig = new LoadBalancerZoneConfig(zoneName);
-			return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig);
+			return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig, loadBalancerClientFactory);
 		};
 		this.creators.add(creator);
 		return this;
@@ -228,19 +230,15 @@ public ServiceInstanceListSupplierBuilder withRequestBasedStickySession() {
 	}
 
 	/**
-	 * If {@link LoadBalancerCacheManager} is available in the context, wraps created
-	 * {@link ServiceInstanceListSupplier} hierarchy with a
-	 * {@link CachingServiceInstanceListSupplier} instance to provide a caching mechanism
-	 * for service instances. Uses {@link ObjectProvider} to lazily resolve
+	 * If {@link LoadBalancerCacheManager} is available in the context, adds a
+	 * {@link CachingServiceInstanceListSupplier} instance to the
+	 * {@link ServiceInstanceListSupplier} hierarchy to provide a caching mechanism for
+	 * service instances. Uses {@link ObjectProvider} to lazily resolve
 	 * {@link LoadBalancerCacheManager}.
 	 * @return the {@link ServiceInstanceListSupplierBuilder} object
 	 */
 	public ServiceInstanceListSupplierBuilder withCaching() {
-		if (cachingCreator != null && LOG.isWarnEnabled()) {
-			LOG.warn(
-					"Overriding a previously set cachingCreator with a CachingServiceInstanceListSupplier-based cachingCreator.");
-		}
-		this.cachingCreator = (context, delegate) -> {
+		DelegateCreator creator = (context, delegate) -> {
 			ObjectProvider<LoadBalancerCacheManager> cacheManagerProvider = context
 					.getBeanProvider(LoadBalancerCacheManager.class);
 			if (cacheManagerProvider.getIfAvailable() != null) {
@@ -251,6 +249,7 @@ public ServiceInstanceListSupplierBuilder withCaching() {
 			}
 			return delegate;
 		};
+		creators.add(creator);
 		return this;
 	}
 
@@ -297,9 +296,6 @@ public ServiceInstanceListSupplier build(ConfigurableApplicationContext context)
 			supplier = creator.apply(context, supplier);
 		}
 
-		if (this.cachingCreator != null) {
-			supplier = this.cachingCreator.apply(context, supplier);
-		}
 		return supplier;
 	}
 

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

@@ -23,6 +23,8 @@
 import reactor.core.publisher.Flux;
 
 import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.client.loadbalancer.Request;
+import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer;
 import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig;
 
 /**
@@ -43,17 +45,36 @@ public class ZonePreferenceServiceInstanceListSupplier extends DelegatingService
 
 	private String zone;
 
+	private boolean callGetWithRequestOnDelegates;
+
 	public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
 			LoadBalancerZoneConfig zoneConfig) {
 		super(delegate);
 		this.zoneConfig = zoneConfig;
 	}
 
+	public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
+			LoadBalancerZoneConfig zoneConfig,
+			ReactiveLoadBalancer.Factory<ServiceInstance> loadBalancerClientFactory) {
+		super(delegate);
+		this.zoneConfig = zoneConfig;
+		callGetWithRequestOnDelegates = loadBalancerClientFactory.getProperties(getServiceId())
+				.isCallGetWithRequestOnDelegates();
+	}
+
 	@Override
 	public Flux<List<ServiceInstance>> get() {
 		return getDelegate().get().map(this::filteredByZone);
 	}
 
+	@Override
+	public Flux<List<ServiceInstance>> get(Request request) {
+		if (callGetWithRequestOnDelegates) {
+			return getDelegate().get(request).map(this::filteredByZone);
+		}
+		return get();
+	}
+
 	private List<ServiceInstance> filteredByZone(List<ServiceInstance> serviceInstances) {
 		if (zone == null) {
 			zone = zoneConfig.getZone();