diff --git a/docs/src/main/asciidoc/_configprops.adoc b/docs/src/main/asciidoc/_configprops.adoc index c8156a69..2da56589 100644 --- a/docs/src/main/asciidoc/_configprops.adoc +++ b/docs/src/main/asciidoc/_configprops.adoc @@ -34,6 +34,7 @@ |spring.cloud.loadbalancer.health-check.initial-delay | | Initial delay value for the HealthCheck scheduler. |spring.cloud.loadbalancer.health-check.interval | 25s | Interval for rerunning the HealthCheck scheduler. |spring.cloud.loadbalancer.health-check.path | | +|spring.cloud.loadbalancer.hint | | Allows setting the value of hint that is passed on to the LoadBalancer request and can subsequently be used in {@link ReactiveLoadBalancer} implementations. |spring.cloud.loadbalancer.retry.enabled | true | |spring.cloud.loadbalancer.ribbon.enabled | true | Causes `RibbonLoadBalancerClient` to be used by default. |spring.cloud.loadbalancer.service-discovery.timeout | | String representation of Duration of the timeout for calls to service discovery. diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index 962d7fb6..df2bddf1 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -917,8 +917,7 @@ in order to avoid retrying calls on a failing instance. `HealthCheckServiceInstanceListSupplier` uses properties prefixed with `spring.cloud.loadbalancer.health-check`. You can set the `initialDelay` and `interval` for the scheduler. You can set the default path for the healthcheck URL by setting -the value of the `spring.cloud.loadbalancer.health-check.path.default`. You can also set a specific value -for any given service by setting the value of the `spring.cloud.loadbalancer.health-check.path.[SERVICE_ID]`, substituting the `[SERVICE_ID]` with the correct ID of your service. If the path is not set, `/actuator/health` is used by default. +the value of the `spring.cloud.loadbalancer.health-check.path.default` property. You can also set a specific value for any given service by setting the value of the `spring.cloud.loadbalancer.health-check.path.[SERVICE_ID]` property, substituting `[SERVICE_ID]` with the correct ID of your service. If the path is not set, `/actuator/health` is used by default. TIP:: If you rely on the default path (`/actuator/health`), make sure you add `spring-boot-starter-actuator` to your collaborator's dependencies, unless you are planning to add such an endpoint on your own. @@ -947,6 +946,13 @@ public class CustomLoadBalancerConfiguration { NOTE:: `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`. +[[spring-cloud-loadbalancer-hints]] +=== Spring Cloud LoadBalancer Hints + +Spring Cloud LoadBalancer lets you set `String` hints that are passed to the LoadBalancer within the `Request` object and that can later be used in `ReactiveLoadBalancer` implementations that can handle them. + +You can set a default hint for all services by setting the value of the `spring.cloud.loadbalancer.hint.default` property. You can also set a specific value +for any given service by setting the value of the `spring.cloud.loadbalancer.hint.[SERVICE_ID]` property, substituting `[SERVICE_ID]` with the correct ID of your service. If the hint is not set by the user, `default` is used. [[spring-cloud-loadbalancer-starter]] === Spring Cloud LoadBalancer Starter @@ -1007,6 +1013,20 @@ public class MyConfiguration { ---- ==== +[[loadbalancer-lifecycle]] +=== Spring Cloud LoadBalancer Lifecycle + +One type of bean that it may be useful to register using <> is `LoadBalancerLifecycle`. + +The LoadBalancerLifecycle beans provide callback methods, named `onStart(Request request)` and `onComplete(CompletionContext completionContext)`, that you should implement to specify what actions should take place before and after load-balancing. + +`onStart(Request request)` takes a `Request` object as a parameter. It contains data that is used to select an appropriate instance, including the downstream client request and <>. On the other hand, a `CompletionContext` object is provided to the `onComplete(CompletionContext completionContext)` method. It contains the LoadBalancer `Response`, including the selected service instance, the `Status` of the request executed against that service instance and (if available) the response returned to the downstream client, and (if an exception has occurred) the corresponding `Throwable`. + +The `supports(Class requestContextClass, Class responseClass, +Class serverTypeClass)` method can be used to determine whether the processor in question handles objects of provided types. If not overridden by the user, it returns `true`. + +NOTE: In the preceding method calls, `RC` means `RequestContext` type, `RES` means client response type, and `T` means returned server type. + == Spring Cloud Circuit Breaker include::spring-cloud-circuitbreaker.adoc[leveloffset=+1] diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ClientRequestContext.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ClientRequestContext.java new file mode 100644 index 00000000..4772f351 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ClientRequestContext.java @@ -0,0 +1,39 @@ +/* + * 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.client.loadbalancer; + +import org.springframework.web.reactive.function.client.ClientRequest; + +/** + * @author Olga Maciaszek-Sharma + * @since 3.0.0 + */ +public class ClientRequestContext extends DefaultRequestContext { + + public ClientRequestContext(ClientRequest clientRequest) { + this(clientRequest, "default"); + } + + public ClientRequestContext(ClientRequest clientRequest, String hint) { + super(clientRequest, hint); + } + + public ClientRequest getClientRequest() { + return (ClientRequest) super.getClientRequest(); + } + +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/CompletionContext.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/CompletionContext.java index 78495d78..5571b3f0 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/CompletionContext.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/CompletionContext.java @@ -19,22 +19,46 @@ package org.springframework.cloud.client.loadbalancer; import org.springframework.core.style.ToStringCreator; /** + * Allows propagation of data related to load-balanced call completion status. + * * @author Spencer Gibb + * @author Olga Maciaszek-Sharma + * @since 3.0.0 */ -// TODO: add metrics -public class CompletionContext { +public class CompletionContext { private final Status status; private final Throwable throwable; + private final Response loadBalancerResponse; + + private final RES clientResponse; + public CompletionContext(Status status) { - this(status, null); + this(status, null, null, null); } - public CompletionContext(Status status, Throwable throwable) { + public CompletionContext(Status status, Response response) { + this(status, null, response, null); + } + + public CompletionContext(Status status, Throwable throwable, + Response loadBalancerResponse) { + this(status, throwable, loadBalancerResponse, null); + } + + public CompletionContext(Status status, Response loadBalancerResponse, + RES clientResponse) { + this(status, null, loadBalancerResponse, clientResponse); + } + + public CompletionContext(Status status, Throwable throwable, + Response loadBalancerResponse, RES clientResponse) { this.status = status; this.throwable = throwable; + this.loadBalancerResponse = loadBalancerResponse; + this.clientResponse = clientResponse; } public Status status() { @@ -45,11 +69,21 @@ public class CompletionContext { return this.throwable; } + public Response getLoadBalancerResponse() { + return loadBalancerResponse; + } + + public RES getClientResponse() { + return clientResponse; + } + @Override public String toString() { ToStringCreator to = new ToStringCreator(this); to.append("status", this.status); to.append("throwable", this.throwable); + to.append("loadBalancerResponse", loadBalancerResponse); + to.append("clientResponse", clientResponse); return to.toString(); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultRequest.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultRequest.java index 1d4ae931..9a911732 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultRequest.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultRequest.java @@ -16,6 +16,8 @@ package org.springframework.cloud.client.loadbalancer; +import org.springframework.core.style.ToStringCreator; + /** * A default implementation of {@link Request}. * @@ -43,4 +45,11 @@ public class DefaultRequest implements Request { this.context = context; } + @Override + public String toString() { + ToStringCreator to = new ToStringCreator(this); + to.append("context", context); + return to.toString(); + } + } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultRequestContext.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultRequestContext.java index 4e6bc5e0..53a212d6 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultRequestContext.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultRequestContext.java @@ -16,32 +16,43 @@ package org.springframework.cloud.client.loadbalancer; +import org.springframework.core.style.ToStringCreator; + /** * Contains information relevant to the request. * * @author Olga Maciaszek-Sharma */ -public class DefaultRequestContext { +public class DefaultRequestContext extends HintRequestContext { /** - * A {@link String} value of hint that can be used to choose the correct service - * instance. + * The request to be executed against the service instance selected by the + * LoadBalancer. */ - private String hint = "default"; + private final Object clientRequest; public DefaultRequestContext() { + clientRequest = null; } - public DefaultRequestContext(String hint) { - this.hint = hint; + public DefaultRequestContext(Object clientRequest) { + this.clientRequest = clientRequest; } - public String getHint() { - return hint; + public DefaultRequestContext(Object clientRequest, String hint) { + super(hint); + this.clientRequest = clientRequest; } - public void setHint(String hint) { - this.hint = hint; + public Object getClientRequest() { + return clientRequest; + } + + @Override + public String toString() { + ToStringCreator to = new ToStringCreator(this); + to.append("clientRequest", clientRequest); + return to.toString(); } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultResponse.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultResponse.java index 387bc332..5b1e6ea8 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultResponse.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/DefaultResponse.java @@ -17,9 +17,11 @@ package org.springframework.cloud.client.loadbalancer; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.core.style.ToStringCreator; /** * @author Spencer Gibb + * @author Olga Maciaszek-Sharma */ public class DefaultResponse implements Response { @@ -41,7 +43,14 @@ public class DefaultResponse implements Response { @Override public void onComplete(CompletionContext completionContext) { - // TODO: implement + // do nothing: deprecated interface method + } + + @Override + public String toString() { + ToStringCreator to = new ToStringCreator(this); + to.append("serviceInstance", serviceInstance); + return to.toString(); } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/EmptyResponse.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/EmptyResponse.java index f0c3ed4c..36ab015e 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/EmptyResponse.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/EmptyResponse.java @@ -35,7 +35,7 @@ public class EmptyResponse implements Response { @Override public void onComplete(CompletionContext completionContext) { - // TODO: implement + // do nothing: deprecated interface method } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/HintRequestContext.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/HintRequestContext.java new file mode 100644 index 00000000..ac3bec5e --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/HintRequestContext.java @@ -0,0 +1,56 @@ +/* + * 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.client.loadbalancer; + +import org.springframework.core.style.ToStringCreator; + +/** + * Allows propagating hints to the LoadBalancer. + * + * @author Olga Maciaszek-Sharma + */ +public class HintRequestContext { + + /** + * A {@link String} value of hint that can be used to choose the correct service + * instance. + */ + private String hint = "default"; + + public HintRequestContext() { + } + + public HintRequestContext(String hint) { + this.hint = hint; + } + + public String getHint() { + return hint; + } + + public void setHint(String hint) { + this.hint = hint; + } + + @Override + public String toString() { + ToStringCreator to = new ToStringCreator(this); + to.append("hint", hint); + return to.toString(); + } + +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/HttpRequestContext.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/HttpRequestContext.java new file mode 100644 index 00000000..7f9c1d12 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/HttpRequestContext.java @@ -0,0 +1,38 @@ +/* + * 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.client.loadbalancer; + +import org.springframework.http.HttpRequest; + +/** + * @author Olga Maciaszek-Sharma + */ +public class HttpRequestContext extends DefaultRequestContext { + + public HttpRequestContext(HttpRequest httpRequest) { + this(httpRequest, "default"); + } + + public HttpRequestContext(HttpRequest httpRequest, String hint) { + super(httpRequest, hint); + } + + public HttpRequest getClientRequest() { + return (HttpRequest) super.getClientRequest(); + } + +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java index 715234e8..1079c2ac 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java @@ -28,6 +28,9 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.client.ClientHttpRequestInterceptor; @@ -120,17 +123,21 @@ public class LoadBalancerAutoConfiguration { */ @Configuration(proxyBeanMethods = false) @ConditionalOnClass(RetryTemplate.class) + @ConditionalOnBean(ReactiveLoadBalancer.Factory.class) public static class RetryInterceptorAutoConfiguration { @Bean @ConditionalOnMissingBean public RetryLoadBalancerInterceptor loadBalancerInterceptor( LoadBalancerClient loadBalancerClient, - LoadBalancerRetryProperties properties, + LoadBalancerRetryProperties retryProperties, LoadBalancerRequestFactory requestFactory, - LoadBalancedRetryFactory loadBalancedRetryFactory) { - return new RetryLoadBalancerInterceptor(loadBalancerClient, properties, - requestFactory, loadBalancedRetryFactory); + LoadBalancedRetryFactory loadBalancedRetryFactory, + LoadBalancerProperties properties, + ReactiveLoadBalancer.Factory loadBalancerFactory) { + return new RetryLoadBalancerInterceptor(loadBalancerClient, retryProperties, + requestFactory, loadBalancedRetryFactory, properties, + loadBalancerFactory); } @Bean diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerLifecycle.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerLifecycle.java new file mode 100644 index 00000000..12346554 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerLifecycle.java @@ -0,0 +1,56 @@ +/* + * 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.client.loadbalancer; + +/** + * Allows to define actions that should be carried out before and after load-balancing. + * + * @author Olga Maciaszek-Sharma + */ +public interface LoadBalancerLifecycle { + + /** + * Allows to assess whether the lifecycle bean's callbacks should be executed. Some + * examples of possible implementations could comprise of verifying whether the + * classes passed in parameters are exactly or extend the classes that this lifecycle + * bean should process. + * @param requestContextClass The class of the {@link Request} context + * @param responseClass The class of the {@link CompletionContext} + * clientResponse + * @param serverTypeClass The type of Server that the LoadBalancer retrieves + * @return true if the lifecycle should be used to process given classes + */ + default boolean supports(Class requestContextClass, Class responseClass, + Class serverTypeClass) { + return true; + } + + /** + * A callback method executed before load-balancing. + * @param request the {@link Request} that will be used by the LoadBalancer to select + * a service instance + */ + void onStart(Request request); + + /** + * A callback method executed after load-balancing. + * @param completionContext the {@link CompletionContext} containing data relevant to + * the load-balancing and the response returned from the selected service instance + */ + void onComplete(CompletionContext completionContext); + +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerLifecycleValidator.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerLifecycleValidator.java new file mode 100644 index 00000000..c61b258b --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerLifecycleValidator.java @@ -0,0 +1,49 @@ +/* + * 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.client.loadbalancer; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Utility class containing methods that allow to filter out supported {@link LoadBalancerLifecycle} beans. + * + * @author Olga Maciaszek-Sharma + * @since 3.0 + */ +public final class LoadBalancerLifecycleValidator { + + private LoadBalancerLifecycleValidator() { + throw new IllegalStateException("Can't instantiate a utility class"); + } + + @SuppressWarnings("rawtypes") + public static Set getSupportedLifecycleProcessors( + Map lifecycleProcessors, + Class requestContextClass, Class clientResponseClass, Class serverTypeClass) { + if (lifecycleProcessors == null) { + return new HashSet<>(); + } + return lifecycleProcessors.values().stream() + .filter(lifecycle -> lifecycle.supports(requestContextClass, + clientResponseClass, serverTypeClass)) + .collect(Collectors.toSet()); + } + +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/Response.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/Response.java index 19c71662..e3d75806 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/Response.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/Response.java @@ -30,8 +30,8 @@ public interface Response { /** * Notification that the request completed. - * @deprecated onComplete callbacks for load-balanced calls are being - * moved to a separate interface + * @deprecated in favour of + * {@link LoadBalancerLifecycle#onComplete(CompletionContext)} * @param completionContext - completion context */ @Deprecated diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index 252caad5..034af9d7 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -18,8 +18,11 @@ package org.springframework.cloud.client.loadbalancer; import java.io.IOException; import java.net.URI; +import java.util.Set; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.http.HttpRequest; import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpRequestInterceptor; @@ -36,26 +39,34 @@ import org.springframework.util.StreamUtils; * @author Ryan Baxter * @author Will Tran * @author Gang Li + * @author Olga Maciaszek-Sharma */ +@SuppressWarnings({ "unchecked", "rawtypes" }) public class RetryLoadBalancerInterceptor implements ClientHttpRequestInterceptor { - private LoadBalancerClient loadBalancer; + private final LoadBalancerClient loadBalancer; - private LoadBalancerRetryProperties lbProperties; + private final LoadBalancerProperties properties; - private LoadBalancerRequestFactory requestFactory; + private final LoadBalancerRequestFactory requestFactory; - private LoadBalancedRetryFactory lbRetryFactory; + private final LoadBalancedRetryFactory lbRetryFactory; + + private final LoadBalancerRetryProperties retryProperties; + + private final ReactiveLoadBalancer.Factory loadBalancerFactory; public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, - LoadBalancerRetryProperties lbProperties, + LoadBalancerRetryProperties retryProperties, LoadBalancerRequestFactory requestFactory, - LoadBalancedRetryFactory lbRetryFactory) { + LoadBalancedRetryFactory lbRetryFactory, LoadBalancerProperties properties, + ReactiveLoadBalancer.Factory loadBalancerFactory) { this.loadBalancer = loadBalancer; - this.lbProperties = lbProperties; + this.retryProperties = retryProperties; this.requestFactory = requestFactory; this.lbRetryFactory = lbRetryFactory; - + this.properties = properties; + this.loadBalancerFactory = loadBalancerFactory; } @Override @@ -65,8 +76,8 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto final String serviceName = originalUri.getHost(); Assert.state(serviceName != null, "Request URI does not contain a valid hostname: " + originalUri); - final LoadBalancedRetryPolicy retryPolicy = this.lbRetryFactory - .createRetryPolicy(serviceName, this.loadBalancer); + final LoadBalancedRetryPolicy retryPolicy = lbRetryFactory + .createRetryPolicy(serviceName, loadBalancer); RetryTemplate template = createRetryTemplate(serviceName, request, retryPolicy); return template.execute(context -> { ServiceInstance serviceInstance = null; @@ -74,12 +85,29 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto LoadBalancedRetryContext lbContext = (LoadBalancedRetryContext) context; serviceInstance = lbContext.getServiceInstance(); } + Set supportedLifecycleProcessors = LoadBalancerLifecycleValidator + .getSupportedLifecycleProcessors( + loadBalancerFactory.getInstances(serviceName, + LoadBalancerLifecycle.class), + HttpRequestContext.class, ClientHttpResponse.class, + ServiceInstance.class); + String hint = getHint(serviceName); + DefaultRequest lbRequest = new DefaultRequest<>( + new HttpRequestContext(request, hint)); + supportedLifecycleProcessors + .forEach(lifecycle -> lifecycle.onStart(lbRequest)); if (serviceInstance == null) { - serviceInstance = this.loadBalancer.choose(serviceName); + serviceInstance = loadBalancer.choose(serviceName, lbRequest); + } + Response lbResponse = new DefaultResponse(serviceInstance); + if (serviceInstance == null) { + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle.onComplete( + new CompletionContext( + CompletionContext.Status.DISCARD, lbResponse))); } ClientHttpResponse response = RetryLoadBalancerInterceptor.this.loadBalancer .execute(serviceName, serviceInstance, - this.requestFactory.createRequest(request, body, execution)); + requestFactory.createRequest(request, body, execution)); int statusCode = response.getRawStatusCode(); if (retryPolicy != null && retryPolicy.retryableStatusCode(statusCode)) { byte[] bodyCopy = StreamUtils.copyToByteArray(response.getBody()); @@ -103,20 +131,24 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto private RetryTemplate createRetryTemplate(String serviceName, HttpRequest request, LoadBalancedRetryPolicy retryPolicy) { RetryTemplate template = new RetryTemplate(); - BackOffPolicy backOffPolicy = this.lbRetryFactory - .createBackOffPolicy(serviceName); + BackOffPolicy backOffPolicy = lbRetryFactory.createBackOffPolicy(serviceName); template.setBackOffPolicy( backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); template.setThrowLastExceptionOnExhausted(true); - RetryListener[] retryListeners = this.lbRetryFactory - .createRetryListeners(serviceName); + RetryListener[] retryListeners = lbRetryFactory.createRetryListeners(serviceName); if (retryListeners != null && retryListeners.length != 0) { template.setListeners(retryListeners); } - template.setRetryPolicy(!this.lbProperties.isEnabled() || retryPolicy == null + template.setRetryPolicy(!retryProperties.isEnabled() || retryPolicy == null ? new NeverRetryPolicy() : new InterceptorRetryPolicy(request, - retryPolicy, this.loadBalancer, serviceName)); + retryPolicy, loadBalancer, serviceName)); return template; } + private String getHint(String serviceId) { + String defaultHint = properties.getHint().getOrDefault("default", "default"); + String hintPropertyValue = properties.getHint().get(serviceId); + return hintPropertyValue != null ? hintPropertyValue : defaultHint; + } + } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ServiceInstanceChooser.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ServiceInstanceChooser.java index 37fcf07f..a3ca9a9f 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ServiceInstanceChooser.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ServiceInstanceChooser.java @@ -23,6 +23,7 @@ import org.springframework.cloud.client.ServiceInstance; * to. * * @author Ryan Baxter + * @author Olga Maciaszek-Sharma */ public interface ServiceInstanceChooser { @@ -33,4 +34,13 @@ public interface ServiceInstanceChooser { */ ServiceInstance choose(String serviceId); + /** + * Chooses a ServiceInstance from the LoadBalancer for the specified service and + * LoadBalancer request. + * @param serviceId The service ID to look up the LoadBalancer. + * @param request The request to pass on to the LoadBalancer + * @return A ServiceInstance that matches the serviceId. + */ + ServiceInstance choose(String serviceId, Request request); + } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java index d3f830d8..92a46fbf 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java @@ -36,6 +36,13 @@ public class LoadBalancerProperties { */ private HealthCheck healthCheck = new HealthCheck(); + /** + * Allows setting the value of hint that is passed on to the LoadBalancer + * request and can subsequently be used in {@link ReactiveLoadBalancer} + * implementations. + */ + private Map hint = new LinkedCaseInsensitiveMap<>(); + public HealthCheck getHealthCheck() { return healthCheck; } @@ -44,6 +51,14 @@ public class LoadBalancerProperties { this.healthCheck = healthCheck; } + public Map getHint() { + return hint; + } + + public void setHint(Map hint) { + this.hint = hint; + } + public static class HealthCheck { /** diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactiveLoadBalancer.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactiveLoadBalancer.java index effd4a5e..9f762ca2 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactiveLoadBalancer.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactiveLoadBalancer.java @@ -16,6 +16,8 @@ package org.springframework.cloud.client.loadbalancer.reactive; +import java.util.Map; + import org.reactivestreams.Publisher; import org.springframework.cloud.client.loadbalancer.DefaultRequest; @@ -49,11 +51,32 @@ public interface ReactiveLoadBalancer { return choose(REQUEST); } - @FunctionalInterface interface Factory { ReactiveLoadBalancer getInstance(String serviceId); + /** + * Allows accessing beans registered within client-specific LoadBalancer contexts. + * @param name Name of the beans to be returned + * @param type The class of the beans to be returned + * @param The type of the beans to be returned + * @return a {@link Map} of beans + * @see @LoadBalancerClient + */ + Map getInstances(String name, Class type); + + /** + * Allows accessing a bean registered within client-specific LoadBalancer + * contexts. + * @param name Name of the bean to be returned + * @param clazz The class of the bean to be returned + * @param generics The classes of generic types of the bean to be returned + * @param The type of the bean to be returned + * @return a {@link Map} of beans + * @see @LoadBalancerClient + */ + X getInstance(String name, Class clazz, Class... generics); + } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerClientAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerClientAutoConfiguration.java index ca7a16be..9a3f987d 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerClientAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerClientAutoConfiguration.java @@ -40,8 +40,10 @@ public class ReactorLoadBalancerClientAutoConfiguration { @ConditionalOnMissingBean @Bean public ReactorLoadBalancerExchangeFilterFunction loadBalancerExchangeFilterFunction( - ReactiveLoadBalancer.Factory loadBalancerFactory) { - return new ReactorLoadBalancerExchangeFilterFunction(loadBalancerFactory); + ReactiveLoadBalancer.Factory loadBalancerFactory, + LoadBalancerProperties properties) { + return new ReactorLoadBalancerExchangeFilterFunction(loadBalancerFactory, + properties); } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunction.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunction.java index f7eb35c0..da75399b 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunction.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunction.java @@ -17,14 +17,21 @@ package org.springframework.cloud.client.loadbalancer.reactive; import java.net.URI; +import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.ClientRequestContext; +import org.springframework.cloud.client.loadbalancer.CompletionContext; +import org.springframework.cloud.client.loadbalancer.DefaultRequest; import org.springframework.cloud.client.loadbalancer.EmptyResponse; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycleValidator; import org.springframework.cloud.client.loadbalancer.LoadBalancerUriTools; +import org.springframework.cloud.client.loadbalancer.Request; import org.springframework.cloud.client.loadbalancer.Response; import org.springframework.http.HttpStatus; import org.springframework.web.reactive.function.client.ClientRequest; @@ -39,6 +46,7 @@ import org.springframework.web.reactive.function.client.ExchangeFunction; * @author Olga Maciaszek-Sharma * @since 2.2.0 */ +@SuppressWarnings({ "rawtypes", "unchecked" }) public class ReactorLoadBalancerExchangeFilterFunction implements ExchangeFilterFunction { private static final Log LOG = LogFactory @@ -46,14 +54,19 @@ public class ReactorLoadBalancerExchangeFilterFunction implements ExchangeFilter private final ReactiveLoadBalancer.Factory loadBalancerFactory; + private final LoadBalancerProperties properties; + public ReactorLoadBalancerExchangeFilterFunction( - ReactiveLoadBalancer.Factory loadBalancerFactory) { + ReactiveLoadBalancer.Factory loadBalancerFactory, + LoadBalancerProperties properties) { this.loadBalancerFactory = loadBalancerFactory; + this.properties = properties; } @Override - public Mono filter(ClientRequest request, ExchangeFunction next) { - URI originalUrl = request.url(); + public Mono filter(ClientRequest clientRequest, + ExchangeFunction next) { + URI originalUrl = clientRequest.url(); String serviceId = originalUrl.getHost(); if (serviceId == null) { String message = String.format( @@ -65,13 +78,26 @@ public class ReactorLoadBalancerExchangeFilterFunction implements ExchangeFilter return Mono.just( ClientResponse.create(HttpStatus.BAD_REQUEST).body(message).build()); } - return choose(serviceId).flatMap(response -> { - ServiceInstance instance = response.getServer(); + Set supportedLifecycleProcessors = LoadBalancerLifecycleValidator + .getSupportedLifecycleProcessors( + loadBalancerFactory.getInstances(serviceId, + LoadBalancerLifecycle.class), + ClientRequestContext.class, ClientResponse.class, + ServiceInstance.class); + String hint = getHint(serviceId); + DefaultRequest lbRequest = new DefaultRequest<>( + new ClientRequestContext(clientRequest, hint)); + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle.onStart(lbRequest)); + return choose(serviceId, lbRequest).flatMap(lbResponse -> { + ServiceInstance instance = lbResponse.getServer(); if (instance == null) { String message = serviceInstanceUnavailableMessage(serviceId); if (LOG.isWarnEnabled()) { LOG.warn(message); } + supportedLifecycleProcessors.forEach( + lifecycle -> lifecycle.onComplete(new CompletionContext<>( + CompletionContext.Status.DISCARD, lbResponse))); return Mono.just(ClientResponse.create(HttpStatus.SERVICE_UNAVAILABLE) .body(serviceInstanceUnavailableMessage(serviceId)).build()); } @@ -81,9 +107,19 @@ public class ReactorLoadBalancerExchangeFilterFunction implements ExchangeFilter "Load balancer has retrieved the instance for service %s: %s", serviceId, instance.getUri())); } - ClientRequest newRequest = buildClientRequest(request, + ClientRequest newRequest = buildClientRequest(clientRequest, reconstructURI(instance, originalUrl)); - return next.exchange(newRequest); + return next.exchange(newRequest) + .doOnError(throwable -> supportedLifecycleProcessors + .forEach(lifecycle -> lifecycle.onComplete( + new CompletionContext( + CompletionContext.Status.FAILED, throwable, + lbResponse)))) + .doOnSuccess(clientResponse -> supportedLifecycleProcessors + .forEach(lifecycle -> lifecycle.onComplete( + new CompletionContext( + CompletionContext.Status.SUCCESS, lbResponse, + clientResponse)))); }); } @@ -91,13 +127,14 @@ public class ReactorLoadBalancerExchangeFilterFunction implements ExchangeFilter return LoadBalancerUriTools.reconstructURI(instance, original); } - protected Mono> choose(String serviceId) { + protected Mono> choose(String serviceId, + Request request) { ReactiveLoadBalancer loadBalancer = loadBalancerFactory .getInstance(serviceId); if (loadBalancer == null) { return Mono.just(new EmptyResponse()); } - return Mono.from(loadBalancer.choose()); + return Mono.from(loadBalancer.choose(request)); } private String serviceInstanceUnavailableMessage(String serviceId) { @@ -112,4 +149,10 @@ public class ReactorLoadBalancerExchangeFilterFunction implements ExchangeFilter .body(request.body()).build(); } + private String getHint(String serviceId) { + String defaultHint = properties.getHint().getOrDefault("default", "default"); + String hintPropertyValue = properties.getHint().get(serviceId); + return hintPropertyValue != null ? hintPropertyValue : defaultHint; + } + } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java index c1b7e330..da55a31d 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java @@ -16,7 +16,6 @@ package org.springframework.cloud.client.loadbalancer; -import java.io.IOException; import java.net.URI; import java.util.Collection; import java.util.Map; @@ -29,17 +28,22 @@ import org.springframework.boot.WebApplicationType; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Primary; import org.springframework.web.client.RestTemplate; import static org.assertj.core.api.BDDAssertions.then; +import static org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer.REQUEST; /** * @author Ryan Baxter * @author Tim Ysewyn + * @author Olga Maciaszek-Sharma */ public abstract class AbstractLoadBalancerAutoConfigurationTests { @@ -98,9 +102,20 @@ public abstract class AbstractLoadBalancerAutoConfigurationTests { return new NoopLoadBalancerClient(); } + @Bean + LoadBalancerProperties loadBalancerProperties() { + return new LoadBalancerProperties(); + } + + @Bean + ReactiveLoadBalancer.Factory loadBalancerFactory() { + return new TestLoadBalancerFactory(); + } + } @Configuration(proxyBeanMethods = false) + @Import(OneRestTemplate.class) protected static class TwoRestTemplates { @Primary @@ -109,17 +124,6 @@ public abstract class AbstractLoadBalancerAutoConfigurationTests { return new RestTemplate(); } - @LoadBalanced - @Bean - RestTemplate loadBalancedRestTemplate() { - return new RestTemplate(); - } - - @Bean - LoadBalancerClient loadBalancerClient() { - return new NoopLoadBalancerClient(); - } - @Configuration(proxyBeanMethods = false) protected static class Two { @@ -140,6 +144,11 @@ public abstract class AbstractLoadBalancerAutoConfigurationTests { @Override public ServiceInstance choose(String serviceId) { + return choose(serviceId, REQUEST); + } + + @Override + public ServiceInstance choose(String serviceId, Request request) { return new DefaultServiceInstance(serviceId, serviceId, serviceId, this.random.nextInt(40000), false); } @@ -147,7 +156,7 @@ public abstract class AbstractLoadBalancerAutoConfigurationTests { @Override public T execute(String serviceId, LoadBalancerRequest request) { try { - return request.apply(choose(serviceId)); + return request.apply(choose(serviceId, REQUEST)); } catch (Exception e) { throw new RuntimeException(e); @@ -156,9 +165,9 @@ public abstract class AbstractLoadBalancerAutoConfigurationTests { @Override public T execute(String serviceId, ServiceInstance serviceInstance, - LoadBalancerRequest request) throws IOException { + LoadBalancerRequest request) { try { - return request.apply(choose(serviceId)); + return request.apply(choose(serviceId, REQUEST)); } catch (Exception e) { throw new RuntimeException(e); @@ -172,4 +181,24 @@ public abstract class AbstractLoadBalancerAutoConfigurationTests { } + private static class TestLoadBalancerFactory + implements ReactiveLoadBalancer.Factory { + + @Override + public ReactiveLoadBalancer getInstance(String serviceId) { + throw new UnsupportedOperationException("Not implemented."); + } + + @Override + public Object getInstance(String name, Class clazz, Class[] generics) { + throw new UnsupportedOperationException("Not implemented."); + } + + @Override + public Map getInstances(String name, Class type) { + throw new UnsupportedOperationException("Not implemented."); + } + + } + } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AsyncLoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AsyncLoadBalancerAutoConfigurationTests.java index bfe1361b..8f36ff54 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AsyncLoadBalancerAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AsyncLoadBalancerAutoConfigurationTests.java @@ -16,7 +16,6 @@ package org.springframework.cloud.client.loadbalancer; -import java.io.IOException; import java.net.URI; import java.util.Collection; import java.util.List; @@ -38,10 +37,12 @@ import org.springframework.http.client.AsyncClientHttpRequestInterceptor; import org.springframework.web.client.AsyncRestTemplate; import static org.assertj.core.api.BDDAssertions.then; +import static org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer.REQUEST; /** * @author Rob Worsnop * @author Tim Ysewyn + * @author Olga Maciaszek-Sharma */ public class AsyncLoadBalancerAutoConfigurationTests { @@ -154,6 +155,11 @@ public class AsyncLoadBalancerAutoConfigurationTests { @Override public ServiceInstance choose(String serviceId) { + return choose(serviceId, REQUEST); + } + + @Override + public ServiceInstance choose(String serviceId, Request request) { return new DefaultServiceInstance(serviceId, serviceId, serviceId, this.random.nextInt(40000), false); } @@ -161,7 +167,7 @@ public class AsyncLoadBalancerAutoConfigurationTests { @Override public T execute(String serviceId, LoadBalancerRequest request) { try { - return request.apply(choose(serviceId)); + return request.apply(choose(serviceId, REQUEST)); } catch (Exception e) { throw new RuntimeException(e); @@ -170,9 +176,9 @@ public class AsyncLoadBalancerAutoConfigurationTests { @Override public T execute(String serviceId, ServiceInstance serviceInstance, - LoadBalancerRequest request) throws IOException { + LoadBalancerRequest request) { try { - return request.apply(choose(serviceId)); + return request.apply(choose(serviceId, REQUEST)); } catch (Exception e) { throw new RuntimeException(e); diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryConfigurationTests.java index 5e2aa0da..582b055f 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryConfigurationTests.java @@ -21,21 +21,24 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import org.springframework.boot.WebApplicationType; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.core.annotation.Order; import org.springframework.http.HttpRequest; import org.springframework.http.client.ClientHttpRequestExecution; import static org.assertj.core.api.BDDAssertions.then; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -134,13 +137,9 @@ public class LoadBalancerRequestFactoryConfigurationTests { } @Configuration(proxyBeanMethods = false) + @Import(NoTransformer.class) static class Transformer { - @Bean - public LoadBalancerClient loadBalancerClient() { - return mock(LoadBalancerClient.class); - } - @Bean public LoadBalancerRequestTransformer transformer() { return mock(LoadBalancerRequestTransformer.class); @@ -149,18 +148,9 @@ public class LoadBalancerRequestFactoryConfigurationTests { } @Configuration(proxyBeanMethods = false) + @Import(Transformer.class) static class TransformersAreOrdered { - @Bean - public LoadBalancerClient loadBalancerClient() { - return mock(LoadBalancerClient.class); - } - - @Bean - public LoadBalancerRequestTransformer transformer() { - return mock(LoadBalancerRequestTransformer.class); - } - @Bean @Order(LoadBalancerRequestTransformer.DEFAULT_ORDER + 1) public LoadBalancerRequestTransformer transformer2() { @@ -177,6 +167,17 @@ public class LoadBalancerRequestFactoryConfigurationTests { return mock(LoadBalancerClient.class); } + @Bean + LoadBalancerProperties loadBalancerProperties() { + return new LoadBalancerProperties(); + } + + @SuppressWarnings("unchecked") + @Bean + ReactiveLoadBalancer.Factory factory() { + return mock(ReactiveLoadBalancer.Factory.class); + } + } } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerAutoConfigurationTests.java index 2dc53c48..3799ce96 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerAutoConfigurationTests.java @@ -42,7 +42,7 @@ public class RetryLoadBalancerAutoConfigurationTests } @Test - public void testDefaultBackOffPolicy() throws Exception { + public void testDefaultBackOffPolicy() { ConfigurableApplicationContext context = init(OneRestTemplate.class); LoadBalancedRetryFactory loadBalancedRetryFactory = context .getBean(LoadBalancedRetryFactory.class); diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTests.java similarity index 57% rename from spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java rename to spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTests.java index 88e25187..ff3e08ea 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTests.java @@ -20,6 +20,12 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.net.URI; +import java.net.URISyntaxException; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import org.junit.After; import org.junit.Before; @@ -28,7 +34,10 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentMatchers; import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.http.HttpRequest; import org.springframework.http.HttpStatus; import org.springframework.http.client.ClientHttpRequestExecution; @@ -44,10 +53,11 @@ import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.listener.RetryListenerSupport; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.BDDAssertions.then; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -56,31 +66,38 @@ import static org.mockito.Mockito.when; /** * @author Ryan Baxter * @author Gang Li + * @author Olga Maciaszek-Sharma */ +@SuppressWarnings({ "unchecked", "rawtypes" }) @RunWith(MockitoJUnitRunner.class) -public class RetryLoadBalancerInterceptorTest { +public class RetryLoadBalancerInterceptorTests { private LoadBalancerClient client; - private LoadBalancerRetryProperties lbProperties; + private LoadBalancerRetryProperties retryProperties; private LoadBalancerRequestFactory lbRequestFactory; - private LoadBalancedRetryFactory loadBalancedRetryFactory = new LoadBalancedRetryFactory() { + private final LoadBalancedRetryFactory loadBalancedRetryFactory = new LoadBalancedRetryFactory() { }; + private LoadBalancerProperties properties; + + private ReactiveLoadBalancer.Factory lbFactory; + @Before public void setUp() { - this.client = mock(LoadBalancerClient.class); - this.lbProperties = new LoadBalancerRetryProperties(); - this.lbRequestFactory = mock(LoadBalancerRequestFactory.class); - + client = mock(LoadBalancerClient.class); + retryProperties = new LoadBalancerRetryProperties(); + lbRequestFactory = mock(LoadBalancerRequestFactory.class); + properties = new LoadBalancerProperties(); + lbFactory = mock(ReactiveLoadBalancer.Factory.class); } @After - public void tearDown() throws Exception { - this.client = null; - this.lbProperties = null; + public void tearDown() { + client = null; + retryProperties = null; } @Test(expected = IOException.class) @@ -88,31 +105,31 @@ public class RetryLoadBalancerInterceptorTest { HttpRequest request = mock(HttpRequest.class); when(request.getURI()).thenReturn(new URI("http://foo")); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("foo"))).thenReturn(serviceInstance); - when(this.client.execute(eq("foo"), eq(serviceInstance), + when(client.choose(eq("foo"), any())).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()); - this.lbProperties.setEnabled(false); + retryProperties.setEnabled(false); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - this.loadBalancedRetryFactory); + client, retryProperties, lbRequestFactory, loadBalancedRetryFactory, + properties, lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); - when(this.lbRequestFactory.createRequest(any(), any(), any())) + when(lbRequestFactory.createRequest(any(), any(), any())) .thenReturn(mock(LoadBalancerRequest.class)); interceptor.intercept(request, body, execution); - verify(this.lbRequestFactory).createRequest(request, body, execution); + verify(lbRequestFactory).createRequest(request, body, execution); } @Test(expected = IllegalStateException.class) public void interceptInvalidHost() throws Throwable { HttpRequest request = mock(HttpRequest.class); when(request.getURI()).thenReturn(new URI("http://foo_underscore")); - this.lbProperties.setEnabled(true); + retryProperties.setEnabled(true); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - this.loadBalancedRetryFactory); + client, retryProperties, lbRequestFactory, loadBalancedRetryFactory, + properties, lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); @@ -125,19 +142,19 @@ public class RetryLoadBalancerInterceptorTest { ClientHttpResponse clientHttpResponse = new MockClientHttpResponse(new byte[] {}, HttpStatus.OK); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("foo"))).thenReturn(serviceInstance); - when(this.client.execute(eq("foo"), eq(serviceInstance), + when(client.choose(eq("foo"), any())).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); - when(this.lbRequestFactory.createRequest(any(), any(), any())) + when(lbRequestFactory.createRequest(any(), any(), any())) .thenReturn(mock(LoadBalancerRequest.class)); - this.lbProperties.setEnabled(true); + retryProperties.setEnabled(true); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - this.loadBalancedRetryFactory); + client, retryProperties, lbRequestFactory, loadBalancedRetryFactory, + properties, lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); - verify(this.lbRequestFactory).createRequest(request, body, execution); + verify(lbRequestFactory).createRequest(request, body, execution); } @Test @@ -148,20 +165,20 @@ public class RetryLoadBalancerInterceptorTest { HttpStatus.OK); LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("foo"))).thenReturn(serviceInstance); - when(this.client.execute(eq("foo"), eq(serviceInstance), + when(client.choose(eq("foo"), any())).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); - when(this.lbRequestFactory.createRequest(any(), any(), any())) + when(lbRequestFactory.createRequest(any(), any(), any())) .thenReturn(mock(LoadBalancerRequest.class)); - this.lbProperties.setEnabled(true); + retryProperties.setEnabled(true); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - new MyLoadBalancedRetryFactory(policy)); + client, retryProperties, lbRequestFactory, + new MyLoadBalancedRetryFactory(policy), properties, lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); then(rsp).isEqualTo(clientHttpResponse); - verify(this.lbRequestFactory).createRequest(request, body, execution); + verify(lbRequestFactory).createRequest(request, body, execution); } @Test @@ -180,23 +197,23 @@ public class RetryLoadBalancerInterceptorTest { when(policy.canRetryNextServer(any(LoadBalancedRetryContext.class))) .thenReturn(true); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("foo"))).thenReturn(serviceInstance); - when(this.client.execute(eq("foo"), eq(serviceInstance), + when(client.choose(eq("foo"), any())).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), nullable(LoadBalancerRequest.class))) .thenReturn(clientHttpResponseNotFound) .thenReturn(clientHttpResponseOk); - this.lbProperties.setEnabled(true); + retryProperties.setEnabled(true); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - new MyLoadBalancedRetryFactory(policy)); + client, retryProperties, lbRequestFactory, + new MyLoadBalancedRetryFactory(policy), properties, lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); - verify(this.client, times(2)).execute(eq("foo"), eq(serviceInstance), + verify(client, times(2)).execute(eq("foo"), eq(serviceInstance), nullable(LoadBalancerRequest.class)); verify(notFoundStream, times(1)).close(); then(rsp).isEqualTo(clientHttpResponseOk); - verify(this.lbRequestFactory, times(2)).createRequest(request, body, execution); + verify(lbRequestFactory, times(2)).createRequest(request, body, execution); } @Test @@ -215,22 +232,22 @@ public class RetryLoadBalancerInterceptorTest { .thenReturn(false); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("foo"))).thenReturn(serviceInstance); - when(this.client.execute(eq("foo"), eq(serviceInstance), + when(client.choose(eq("foo"), any())).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), ArgumentMatchers.>any())) .thenReturn(clientHttpResponseNotFound); - this.lbProperties.setEnabled(true); + retryProperties.setEnabled(true); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - new MyLoadBalancedRetryFactory(policy)); + client, retryProperties, lbRequestFactory, + new MyLoadBalancedRetryFactory(policy), properties, lbFactory); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); - verify(this.client, times(1)).execute(eq("foo"), eq(serviceInstance), + verify(client, times(1)).execute(eq("foo"), eq(serviceInstance), ArgumentMatchers.>any()); - verify(this.lbRequestFactory, times(1)).createRequest(request, body, execution); + verify(lbRequestFactory, times(1)).createRequest(request, body, execution); verify(policy, times(2)).canRetryNextServer(any(LoadBalancedRetryContext.class)); // call twice in a retry attempt @@ -251,23 +268,24 @@ public class RetryLoadBalancerInterceptorTest { .thenReturn(true); MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("foo"))).thenReturn(serviceInstance); - when(this.client.execute(eq("foo"), eq(serviceInstance), + when(client.choose(eq("foo"), any())).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()) .thenReturn(clientHttpResponse); - when(this.lbRequestFactory.createRequest(any(), any(), any())) + when(lbRequestFactory.createRequest(any(), any(), any())) .thenReturn(mock(LoadBalancerRequest.class)); - this.lbProperties.setEnabled(true); + retryProperties.setEnabled(true); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - new MyLoadBalancedRetryFactory(policy, backOffPolicy)); + client, retryProperties, lbRequestFactory, + new MyLoadBalancedRetryFactory(policy, backOffPolicy), properties, + lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); - verify(this.client, times(2)).execute(eq("foo"), eq(serviceInstance), + verify(client, times(2)).execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class)); then(rsp).isEqualTo(clientHttpResponse); - verify(this.lbRequestFactory, times(2)).createRequest(request, body, execution); + verify(lbRequestFactory, times(2)).createRequest(request, body, execution); then(backOffPolicy.getBackoffAttempts()).isEqualTo(1); } @@ -281,20 +299,24 @@ public class RetryLoadBalancerInterceptorTest { when(policy.canRetryNextServer(any(LoadBalancedRetryContext.class))) .thenReturn(false); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("foo"))).thenReturn(serviceInstance); - when(this.client.execute(eq("foo"), eq(serviceInstance), + when(client.choose(eq("foo"), any())).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()) .thenReturn(clientHttpResponse); - when(this.lbRequestFactory.createRequest(any(), any(), any())) + when(lbRequestFactory.createRequest(any(), any(), any())) .thenReturn(mock(LoadBalancerRequest.class)); - this.lbProperties.setEnabled(true); + retryProperties.setEnabled(true); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - new MyLoadBalancedRetryFactory(policy)); + client, retryProperties, lbRequestFactory, + new MyLoadBalancedRetryFactory(policy), properties, lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); - verify(this.lbRequestFactory).createRequest(request, body, execution); + verify(lbRequestFactory).createRequest(request, body, execution); + } + + private static ServiceInstance defaultServiceInstance() { + return new DefaultServiceInstance("testInstance", "test", "testHost", 80, false); } @Test @@ -308,51 +330,30 @@ public class RetryLoadBalancerInterceptorTest { .thenReturn(true); MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("listener"))).thenReturn(serviceInstance); - when(this.client.execute(eq("listener"), eq(serviceInstance), + when(client.choose(eq("listener"), any())).thenReturn(serviceInstance); + when(client.execute(eq("listener"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()) .thenReturn(clientHttpResponse); - this.lbProperties.setEnabled(true); + retryProperties.setEnabled(true); MyRetryListener retryListener = new MyRetryListener(); - when(this.lbRequestFactory.createRequest(any(), any(), any())) + when(lbRequestFactory.createRequest(any(), any(), any())) .thenReturn(mock(LoadBalancerRequest.class)); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, + client, retryProperties, lbRequestFactory, new MyLoadBalancedRetryFactory(policy, backOffPolicy, - new RetryListener[] { retryListener })); + new RetryListener[] { retryListener }), + properties, lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); - verify(this.client, times(2)).execute(eq("listener"), eq(serviceInstance), + verify(client, times(2)).execute(eq("listener"), eq(serviceInstance), any(LoadBalancerRequest.class)); then(rsp).isEqualTo(clientHttpResponse); - verify(this.lbRequestFactory, times(2)).createRequest(request, body, execution); + verify(lbRequestFactory, times(2)).createRequest(request, body, execution); then(backOffPolicy.getBackoffAttempts()).isEqualTo(1); then(retryListener.getOnError()).isEqualTo(1); } - @Test(expected = TerminatedRetryException.class) - public void retryListenerTestNoRetry() throws Throwable { - HttpRequest request = mock(HttpRequest.class); - when(request.getURI()).thenReturn(new URI("http://noRetry")); - LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); - MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); - this.lbProperties.setEnabled(true); - RetryListener myRetryListener = new RetryListenerSupport() { - @Override - public boolean open(RetryContext context, - RetryCallback callback) { - return false; - } - }; - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - new MyLoadBalancedRetryFactory(policy, backOffPolicy, - new RetryListener[] { myRetryListener })); - ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); - interceptor.intercept(request, new byte[] {}, execution); - } - @Test public void retryWithDefaultConstructorTest() throws Throwable { HttpRequest request = mock(HttpRequest.class); @@ -364,29 +365,91 @@ public class RetryLoadBalancerInterceptorTest { .thenReturn(true); MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.client.choose(eq("default"))).thenReturn(serviceInstance); - when(this.client.execute(eq("default"), eq(serviceInstance), + when(client.choose(eq("default"), any())).thenReturn(serviceInstance); + when(client.execute(eq("default"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()) .thenReturn(clientHttpResponse); - this.lbProperties.setEnabled(true); - when(this.lbRequestFactory.createRequest(any(), any(), any())) + retryProperties.setEnabled(true); + when(lbRequestFactory.createRequest(any(), any(), any())) .thenReturn(mock(LoadBalancerRequest.class)); RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( - this.client, this.lbProperties, this.lbRequestFactory, - new MyLoadBalancedRetryFactory(policy, backOffPolicy)); + client, retryProperties, lbRequestFactory, + new MyLoadBalancedRetryFactory(policy, backOffPolicy), properties, + lbFactory); byte[] body = new byte[] {}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); - verify(this.client, times(2)).execute(eq("default"), eq(serviceInstance), + verify(client, times(2)).execute(eq("default"), eq(serviceInstance), any(LoadBalancerRequest.class)); then(rsp).isEqualTo(clientHttpResponse); - verify(this.lbRequestFactory, times(2)).createRequest(request, body, execution); + verify(lbRequestFactory, times(2)).createRequest(request, body, execution); then(backOffPolicy.getBackoffAttempts()).isEqualTo(1); } - class MyLoadBalancedRetryFactory implements LoadBalancedRetryFactory { + @Test(expected = TerminatedRetryException.class) + public void retryListenerTestNoRetry() throws Throwable { + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://noRetry")); + LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + retryProperties.setEnabled(true); + RetryListener myRetryListener = new RetryListenerSupport() { + @Override + public boolean open(RetryContext context, + RetryCallback callback) { + return false; + } + }; + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( + client, retryProperties, lbRequestFactory, + new MyLoadBalancedRetryFactory(policy, backOffPolicy, + new RetryListener[] { myRetryListener }), + properties, lbFactory); + ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); + interceptor.intercept(request, new byte[] {}, execution); + } - private LoadBalancedRetryPolicy loadBalancedRetryPolicy; + @Test + public void shouldNotDuplicateLifecycleCalls() + throws IOException, URISyntaxException { + Map lifecycleProcessors = new HashMap<>(); + lifecycleProcessors.put("testLifecycle", new TestLoadBalancerLifecycle()); + lifecycleProcessors.put("anotherLifecycle", new AnotherLoadBalancerLifecycle()); + when(lbFactory.getInstances("test", LoadBalancerLifecycle.class)) + .thenReturn(lifecycleProcessors); + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://test")); + TestLoadBalancerClient client = new TestLoadBalancerClient(); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor( + client, retryProperties, lbRequestFactory, loadBalancedRetryFactory, + properties, lbFactory); + + interceptor.intercept(request, new byte[] {}, + mock(ClientHttpRequestExecution.class)); + + assertThat(((TestLoadBalancerLifecycle) lifecycleProcessors.get("testLifecycle")) + .getStartLog()).hasSize(1); + assertThat(((TestLoadBalancerLifecycle) lifecycleProcessors.get("testLifecycle")) + .getCompleteLog()).hasSize(0); + assertThat( + ((TestLoadBalancerLifecycle) lifecycleProcessors.get("anotherLifecycle")) + .getStartLog()).hasSize(1); + assertThat( + ((TestLoadBalancerLifecycle) lifecycleProcessors.get("anotherLifecycle")) + .getCompleteLog()).hasSize(0); + assertThat(((TestLoadBalancerLifecycle) client.getLifecycleProcessors() + .get("testLifecycle")).getStartLog()).hasSize(0); + assertThat(((TestLoadBalancerLifecycle) client.getLifecycleProcessors() + .get("testLifecycle")).getCompleteLog()).hasSize(1); + assertThat(((TestLoadBalancerLifecycle) client.getLifecycleProcessors() + .get("anotherLifecycle")).getStartLog()).hasSize(0); + assertThat(((TestLoadBalancerLifecycle) client.getLifecycleProcessors() + .get("anotherLifecycle")).getCompleteLog()).hasSize(1); + } + + static class MyLoadBalancedRetryFactory implements LoadBalancedRetryFactory { + + private final LoadBalancedRetryPolicy loadBalancedRetryPolicy; private BackOffPolicy backOffPolicy; @@ -411,32 +474,32 @@ public class RetryLoadBalancerInterceptorTest { @Override public LoadBalancedRetryPolicy createRetryPolicy(String service, ServiceInstanceChooser serviceInstanceChooser) { - return this.loadBalancedRetryPolicy; + return loadBalancedRetryPolicy; } @Override public BackOffPolicy createBackOffPolicy(String service) { - if (this.backOffPolicy == null) { + if (backOffPolicy == null) { return new NoBackOffPolicy(); } else { - return this.backOffPolicy; + return backOffPolicy; } } @Override public RetryListener[] createRetryListeners(String service) { - if (this.retryListeners == null) { + if (retryListeners == null) { return new RetryListener[0]; } else { - return this.retryListeners; + return retryListeners; } } } - class MyBackOffPolicy implements BackOffPolicy { + static class MyBackOffPolicy implements BackOffPolicy { private int backoffAttempts = 0; @@ -453,27 +516,128 @@ public class RetryLoadBalancerInterceptorTest { @Override public void backOff(BackOffContext backOffContext) throws BackOffInterruptedException { - this.backoffAttempts++; + backoffAttempts++; } int getBackoffAttempts() { - return this.backoffAttempts; + return backoffAttempts; } } - class MyRetryListener extends RetryListenerSupport { + static class MyRetryListener extends RetryListenerSupport { private int onError = 0; @Override public void onError(RetryContext retryContext, RetryCallback retryCallback, Throwable throwable) { - this.onError++; + onError++; } int getOnError() { - return this.onError; + return onError; + } + + } + + protected static class TestLoadBalancerClient implements LoadBalancerClient { + + private final Map lifecycleProcessors = new HashMap<>(); + + TestLoadBalancerClient() { + lifecycleProcessors.put("testLifecycle", new TestLoadBalancerLifecycle()); + lifecycleProcessors.put("anotherLifecycle", + new AnotherLoadBalancerLifecycle()); + } + + @Override + public T execute(String serviceId, LoadBalancerRequest request) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public T execute(String serviceId, ServiceInstance serviceInstance, + LoadBalancerRequest request) { + Set supportedLoadBalancerProcessors = LoadBalancerLifecycleValidator + .getSupportedLifecycleProcessors(lifecycleProcessors, + DefaultRequestContext.class, Object.class, + ServiceInstance.class); + + T response = (T) new MockClientHttpResponse(new byte[] {}, HttpStatus.OK); + supportedLoadBalancerProcessors.forEach(lifecycle -> lifecycle + .onComplete(new CompletionContext(CompletionContext.Status.SUCCESS, + new DefaultResponse(defaultServiceInstance())))); + return response; + } + + @Override + public URI reconstructURI(ServiceInstance instance, URI original) { + throw new UnsupportedOperationException("Please, implement me."); + } + + @Override + public ServiceInstance choose(String serviceId) { + return defaultServiceInstance(); + } + + @Override + public ServiceInstance choose(String serviceId, Request request) { + return defaultServiceInstance(); + } + + Map getLifecycleProcessors() { + return lifecycleProcessors; + } + + } + + protected static class TestLoadBalancerLifecycle + implements LoadBalancerLifecycle { + + final ConcurrentHashMap> startLog = new ConcurrentHashMap<>(); + + final ConcurrentHashMap> completeLog = new ConcurrentHashMap<>(); + + @Override + public boolean supports(Class requestContextClass, Class responseClass, + Class serverTypeClass) { + return DefaultRequestContext.class.isAssignableFrom(requestContextClass) + && Object.class.isAssignableFrom(responseClass) + && ServiceInstance.class.isAssignableFrom(serverTypeClass); + } + + @Override + public void onStart(Request request) { + startLog.put(getName() + UUID.randomUUID(), request); + } + + @Override + public void onComplete( + CompletionContext completionContext) { + completeLog.put(getName() + UUID.randomUUID(), completionContext); + } + + ConcurrentHashMap> getStartLog() { + return startLog; + } + + ConcurrentHashMap> getCompleteLog() { + return completeLog; + } + + protected String getName() { + return getClass().getSimpleName(); + } + + } + + protected static class AnotherLoadBalancerLifecycle + extends TestLoadBalancerLifecycle { + + @Override + protected String getName() { + return getClass().getSimpleName(); } } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerClientAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerClientAutoConfigurationTests.java index 9f0320fc..091e587b 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerClientAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerClientAutoConfigurationTests.java @@ -106,7 +106,24 @@ public class ReactorLoadBalancerClientAutoConfigurationTests { @Bean ReactiveLoadBalancer.Factory reactiveLoadBalancerFactory() { - return serviceId -> new TestReactiveLoadBalancer(); + return new ReactiveLoadBalancer.Factory() { + @Override + public ReactiveLoadBalancer getInstance( + String serviceId) { + return new TestReactiveLoadBalancer(); + } + + @Override + public Map getInstances(String name, Class type) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public X getInstance(String name, Class clazz, + Class... generics) { + throw new UnsupportedOperationException("Not implemented."); + } + }; } @Bean @@ -115,6 +132,11 @@ public class ReactorLoadBalancerClientAutoConfigurationTests { }; } + @Bean + LoadBalancerProperties loadBalancerProperties() { + return new LoadBalancerProperties(); + } + } @Configuration diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunctionTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunctionTests.java index 18285118..357d4204 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunctionTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunctionTests.java @@ -17,13 +17,17 @@ package org.springframework.cloud.client.loadbalancer.reactive; import java.net.URI; +import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Random; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; @@ -36,18 +40,22 @@ import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.EnableDiscoveryClient; import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties; +import org.springframework.cloud.client.loadbalancer.CompletionContext; +import org.springframework.cloud.client.loadbalancer.DefaultRequestContext; import org.springframework.cloud.client.loadbalancer.DefaultResponse; import org.springframework.cloud.client.loadbalancer.EmptyResponse; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; import org.springframework.cloud.client.loadbalancer.Request; import org.springframework.cloud.client.loadbalancer.Response; import org.springframework.context.annotation.Bean; import org.springframework.http.HttpStatus; -import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.reactive.function.client.ClientResponse; import org.springframework.web.reactive.function.client.WebClient; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.BDDAssertions.then; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; @@ -56,7 +64,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen * * @author Olga Maciaszek-Sharma */ -@ExtendWith(SpringExtension.class) +@SuppressWarnings("ConstantConditions") @SpringBootTest(webEnvironment = RANDOM_PORT) class ReactorLoadBalancerExchangeFilterFunctionTests { @@ -66,6 +74,12 @@ class ReactorLoadBalancerExchangeFilterFunctionTests { @Autowired private SimpleDiscoveryProperties properties; + @Autowired + private LoadBalancerProperties loadBalancerProperties; + + @Autowired + private ReactiveLoadBalancer.Factory factory; + @LocalServerPort private int port; @@ -74,8 +88,15 @@ class ReactorLoadBalancerExchangeFilterFunctionTests { SimpleDiscoveryProperties.SimpleServiceInstance instance = new SimpleDiscoveryProperties.SimpleServiceInstance(); instance.setServiceId("testservice"); instance.setUri(URI.create("http://localhost:" + this.port)); + SimpleDiscoveryProperties.SimpleServiceInstance instanceWithNoLifecycleProcessors = new SimpleDiscoveryProperties.SimpleServiceInstance(); + instanceWithNoLifecycleProcessors + .setServiceId("serviceWithNoLifecycleProcessors"); + instanceWithNoLifecycleProcessors + .setUri(URI.create("http://localhost:" + this.port)); this.properties.getInstances().put("testservice", Collections.singletonList(instance)); + properties.getInstances().put("serviceWithNoLifecycleProcessors", + Collections.singletonList(instanceWithNoLifecycleProcessors)); } @Test @@ -101,9 +122,43 @@ class ReactorLoadBalancerExchangeFilterFunctionTests { then(clientResponse.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST); } + @Test + void exceptionNotThrownWhenFactoryReturnsNullLifecycleProcessorsMap() { + assertThatCode(() -> WebClient.builder() + .baseUrl("http://serviceWithNoLifecycleProcessors") + .filter(this.loadBalancerFunction).build().get().uri("/hello").exchange() + .block()).doesNotThrowAnyException(); + } + + @Test + void loadBalancerLifecycleCallbacksExecuted() { + final String callbackTestHint = "callbackTestHint"; + loadBalancerProperties.getHint().put("testservice", "callbackTestHint"); + final String result = "callbackTestResult"; + ClientResponse clientResponse = WebClient.builder().baseUrl("http://testservice") + .filter(this.loadBalancerFunction).build().get().uri("/callback") + .exchange().block(); + + Collection> lifecycleLogRequests = ((TestLoadBalancerLifecycle) factory + .getInstances("testservice", LoadBalancerLifecycle.class) + .get("loadBalancerLifecycle")).getStartLog().values(); + Collection> anotherLifecycleLogRequests = ((AnotherLoadBalancerLifecycle) factory + .getInstances("testservice", LoadBalancerLifecycle.class) + .get("anotherLoadBalancerLifecycle")).getCompleteLog().values(); + then(clientResponse.statusCode()).isEqualTo(HttpStatus.OK); + assertThat(lifecycleLogRequests).extracting( + request -> ((DefaultRequestContext) request.getContext()).getHint()) + .contains(callbackTestHint); + assertThat(anotherLifecycleLogRequests) + .extracting(completionContext -> ((ClientResponse) completionContext + .getClientResponse()).bodyToMono(String.class).block()) + .contains(result); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) @EnableDiscoveryClient @EnableAutoConfiguration - @SpringBootConfiguration + @SpringBootConfiguration(proxyBeanMethods = false) @RestController static class Config { @@ -112,11 +167,93 @@ class ReactorLoadBalancerExchangeFilterFunctionTests { return "Hello World"; } + @RequestMapping("/callback") + String callbackTestResult() { + return "callbackTestResult"; + } + @Bean ReactiveLoadBalancer.Factory reactiveLoadBalancerFactory( DiscoveryClient discoveryClient) { - return serviceId -> new DiscoveryClientBasedReactiveLoadBalancer(serviceId, - discoveryClient); + return new ReactiveLoadBalancer.Factory() { + + private final TestLoadBalancerLifecycle testLoadBalancerLifecycle = new TestLoadBalancerLifecycle(); + + private final TestLoadBalancerLifecycle anotherLoadBalancerLifecycle = new AnotherLoadBalancerLifecycle(); + + @Override + public ReactiveLoadBalancer getInstance( + String serviceId) { + return new DiscoveryClientBasedReactiveLoadBalancer(serviceId, + discoveryClient); + } + + @Override + public Map getInstances(String name, Class type) { + if (name.equals("serviceWithNoLifecycleProcessors")) { + return null; + } + Map lifecycleProcessors = new HashMap<>(); + lifecycleProcessors.put("loadBalancerLifecycle", + testLoadBalancerLifecycle); + lifecycleProcessors.put("anotherLoadBalancerLifecycle", + anotherLoadBalancerLifecycle); + return lifecycleProcessors; + } + + @Override + public X getInstance(String name, Class clazz, + Class... generics) { + return null; + } + }; + } + + @Bean + LoadBalancerProperties loadBalancerProperties() { + return new LoadBalancerProperties(); + } + + } + + protected static class TestLoadBalancerLifecycle + implements LoadBalancerLifecycle { + + ConcurrentHashMap> startLog = new ConcurrentHashMap<>(); + + ConcurrentHashMap> completeLog = new ConcurrentHashMap<>(); + + @Override + public void onStart(Request request) { + startLog.put(getName() + UUID.randomUUID(), request); + } + + @Override + public void onComplete( + CompletionContext completionContext) { + completeLog.put(getName() + UUID.randomUUID(), completionContext); + } + + ConcurrentHashMap> getStartLog() { + return startLog; + } + + ConcurrentHashMap> getCompleteLog() { + return completeLog; + } + + protected String getName() { + return this.getClass().getSimpleName(); + } + + } + + protected static class AnotherLoadBalancerLifecycle + extends TestLoadBalancerLifecycle { + + @Override + protected String getName() { + return this.getClass().getSimpleName(); } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClient.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClient.java index c71d6ab8..3dbdba77 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClient.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClient.java @@ -18,38 +18,67 @@ package org.springframework.cloud.loadbalancer.blocking.client; import java.io.IOException; import java.net.URI; +import java.util.Set; import reactor.core.publisher.Mono; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.CompletionContext; +import org.springframework.cloud.client.loadbalancer.DefaultRequest; +import org.springframework.cloud.client.loadbalancer.DefaultRequestContext; +import org.springframework.cloud.client.loadbalancer.DefaultResponse; +import org.springframework.cloud.client.loadbalancer.EmptyResponse; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycleValidator; import org.springframework.cloud.client.loadbalancer.LoadBalancerRequest; import org.springframework.cloud.client.loadbalancer.LoadBalancerUriTools; +import org.springframework.cloud.client.loadbalancer.Request; import org.springframework.cloud.client.loadbalancer.Response; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.util.ReflectionUtils; +import static org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer.REQUEST; + /** * The default {@link LoadBalancerClient} implementation. * * @author Olga Maciaszek-Sharma * @since 2.2.0 */ +@SuppressWarnings({ "unchecked", "rawtypes" }) public class BlockingLoadBalancerClient implements LoadBalancerClient { private final LoadBalancerClientFactory loadBalancerClientFactory; - public BlockingLoadBalancerClient( - LoadBalancerClientFactory loadBalancerClientFactory) { + private final LoadBalancerProperties properties; + + public BlockingLoadBalancerClient(LoadBalancerClientFactory loadBalancerClientFactory, + LoadBalancerProperties properties) { this.loadBalancerClientFactory = loadBalancerClientFactory; + this.properties = properties; + } @Override public T execute(String serviceId, LoadBalancerRequest request) throws IOException { - ServiceInstance serviceInstance = choose(serviceId); + String hint = getHint(serviceId); + DefaultRequest lbRequest = new DefaultRequest<>( + new DefaultRequestContext(request, hint)); + Set supportedLifecycleProcessors = LoadBalancerLifecycleValidator + .getSupportedLifecycleProcessors( + loadBalancerClientFactory.getInstances(serviceId, + LoadBalancerLifecycle.class), + DefaultRequestContext.class, Object.class, ServiceInstance.class); + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle.onStart(lbRequest)); + ServiceInstance serviceInstance = choose(serviceId, lbRequest); if (serviceInstance == null) { + supportedLifecycleProcessors + .forEach(lifecycle -> lifecycle.onComplete(new CompletionContext<>( + CompletionContext.Status.DISCARD, new EmptyResponse()))); throw new IllegalStateException("No instances available for " + serviceId); } return execute(serviceId, serviceInstance, request); @@ -58,13 +87,29 @@ public class BlockingLoadBalancerClient implements LoadBalancerClient { @Override public T execute(String serviceId, ServiceInstance serviceInstance, LoadBalancerRequest request) throws IOException { + DefaultResponse defaultResponse = new DefaultResponse(serviceInstance); + Set supportedLifecycleProcessors = LoadBalancerLifecycleValidator + .getSupportedLifecycleProcessors( + loadBalancerClientFactory.getInstances(serviceId, + LoadBalancerLifecycle.class), + DefaultRequestContext.class, Object.class, ServiceInstance.class); try { - return request.apply(serviceInstance); + T response = request.apply(serviceInstance); + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle + .onComplete(new CompletionContext<>(CompletionContext.Status.SUCCESS, + defaultResponse, response))); + return response; } catch (IOException iOException) { + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle + .onComplete(new CompletionContext<>(CompletionContext.Status.FAILED, + iOException, defaultResponse))); throw iOException; } catch (Exception exception) { + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle + .onComplete(new CompletionContext<>(CompletionContext.Status.FAILED, + exception, defaultResponse))); ReflectionUtils.rethrowRuntimeException(exception); } return null; @@ -77,17 +122,28 @@ public class BlockingLoadBalancerClient implements LoadBalancerClient { @Override public ServiceInstance choose(String serviceId) { + return choose(serviceId, REQUEST); + } + + @Override + public ServiceInstance choose(String serviceId, Request request) { ReactiveLoadBalancer loadBalancer = loadBalancerClientFactory .getInstance(serviceId); if (loadBalancer == null) { return null; } - Response loadBalancerResponse = Mono.from(loadBalancer.choose()) - .block(); + Response loadBalancerResponse = Mono + .from(loadBalancer.choose(request)).block(); if (loadBalancerResponse == null) { return null; } return loadBalancerResponse.getServer(); } + private String getHint(String serviceId) { + String defaultHint = properties.getHint().getOrDefault("default", "default"); + String hintPropertyValue = properties.getHint().get(serviceId); + return hintPropertyValue != null ? hintPropertyValue : defaultHint; + } + } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java index 96845e33..8200e16e 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java @@ -23,6 +23,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.cloud.client.loadbalancer.AsyncLoadBalancerAutoConfiguration; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; @@ -49,8 +50,9 @@ public class BlockingLoadBalancerClientAutoConfiguration { @ConditionalOnBean(LoadBalancerClientFactory.class) @ConditionalOnMissingBean public LoadBalancerClient blockingLoadBalancerClient( - LoadBalancerClientFactory loadBalancerClientFactory) { - return new BlockingLoadBalancerClient(loadBalancerClientFactory); + LoadBalancerClientFactory loadBalancerClientFactory, + LoadBalancerProperties properties) { + return new BlockingLoadBalancerClient(loadBalancerClientFactory, properties); } } diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java index bfbf0af3..8834ebc8 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java @@ -18,35 +18,42 @@ package org.springframework.cloud.loadbalancer.blocking.client; import java.io.IOException; import java.net.URI; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Random; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; import reactor.core.publisher.Mono; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.SpringBootConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties; +import org.springframework.cloud.client.loadbalancer.CompletionContext; +import org.springframework.cloud.client.loadbalancer.DefaultRequestContext; import org.springframework.cloud.client.loadbalancer.DefaultResponse; import org.springframework.cloud.client.loadbalancer.EmptyResponse; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; import org.springframework.cloud.client.loadbalancer.LoadBalancerRequest; import org.springframework.cloud.client.loadbalancer.Request; import org.springframework.cloud.client.loadbalancer.Response; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; +import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient; import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients; import org.springframework.cloud.loadbalancer.core.ReactorLoadBalancer; import org.springframework.cloud.loadbalancer.core.ReactorServiceInstanceLoadBalancer; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.test.context.junit.jupiter.SpringExtension; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.fail; /** @@ -55,7 +62,6 @@ import static org.assertj.core.api.Assertions.fail; * @author Olga Maciaszek-Sharma */ @SpringBootTest -@ExtendWith(SpringExtension.class) class BlockingLoadBalancerClientTests { @Autowired @@ -64,6 +70,12 @@ class BlockingLoadBalancerClientTests { @Autowired private SimpleDiscoveryProperties properties; + @Autowired + ReactiveLoadBalancer.Factory factory; + + @Autowired + LoadBalancerProperties loadBalancerProperties; + @BeforeEach void setUp() { properties.getInstances().put("myservice", @@ -141,19 +153,65 @@ class BlockingLoadBalancerClientTests { } } + @Test + void exceptionNotThrownWhenFactoryReturnsNullLifecycleProcessorsMap() { + assertThatCode( + () -> loadBalancerClient.execute("serviceWithNoLifecycleProcessors", + (LoadBalancerRequest) instance -> { + assertThat(instance.getHost()).isEqualTo("test.example"); + return "result"; + })).doesNotThrowAnyException(); + } + + @Test + void loadBalancerLifecycleCallbacksExecuted() throws IOException { + String callbackTestHint = "callbackTestHint"; + loadBalancerProperties.getHint().put("myservice", "callbackTestHint"); + final String result = "callbackTestResult"; + Object actualResult = loadBalancerClient.execute("myservice", + (LoadBalancerRequest) instance -> { + assertThat(instance.getHost()).isEqualTo("test.example"); + return result; + }); + + Collection> lifecycleLogRequests = ((TestLoadBalancerLifecycle) factory + .getInstances("myservice", LoadBalancerLifecycle.class) + .get("loadBalancerLifecycle")).getStartLog().values(); + Collection> anotherLifecycleLogRequests = ((AnotherLoadBalancerLifecycle) factory + .getInstances("myservice", LoadBalancerLifecycle.class) + .get("anotherLoadBalancerLifecycle")).getCompleteLog().values(); + assertThat(actualResult).isEqualTo(result); + assertThat(lifecycleLogRequests).extracting( + request -> ((DefaultRequestContext) request.getContext()).getHint()) + .contains(callbackTestHint); + assertThat(anotherLifecycleLogRequests) + .extracting(CompletionContext::getClientResponse).contains(result); + } + @Configuration(proxyBeanMethods = false) @EnableAutoConfiguration - @SpringBootConfiguration @LoadBalancerClients({ - @org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient( - name = "myservice", configuration = MyServiceConfig.class), - @org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient( - name = "unknownservice", - configuration = UnknownServiceConfig.class) }) + @LoadBalancerClient(name = "myservice", + configuration = MyServiceConfig.class), + @LoadBalancerClient(name = "unknownservice", + configuration = UnknownServiceConfig.class), + @LoadBalancerClient(name = "serviceWithNoLifecycleProcessors", + configuration = NoLifecycleProcessorsConfig.class) }) protected static class Config { } + protected static class NoLifecycleProcessorsConfig { + + @Bean + ReactorLoadBalancer reactiveLoadBalancer( + DiscoveryClient discoveryClient) { + return new DiscoveryClientBasedReactiveLoadBalancer("myservice", + discoveryClient); + } + + } + protected static class MyServiceConfig { @Bean @@ -163,6 +221,16 @@ class BlockingLoadBalancerClientTests { discoveryClient); } + @Bean + LoadBalancerLifecycle loadBalancerLifecycle() { + return new TestLoadBalancerLifecycle(); + } + + @Bean + LoadBalancerLifecycle anotherLoadBalancerLifecycle() { + return new AnotherLoadBalancerLifecycle(); + } + } protected static class UnknownServiceConfig { @@ -176,8 +244,51 @@ class BlockingLoadBalancerClientTests { } + protected static class TestLoadBalancerLifecycle + implements LoadBalancerLifecycle { + + final ConcurrentHashMap> startLog = new ConcurrentHashMap<>(); + + final ConcurrentHashMap> completeLog = new ConcurrentHashMap<>(); + + @Override + public void onStart(Request request) { + startLog.put(getName() + UUID.randomUUID(), request); + } + + @Override + public void onComplete( + CompletionContext completionContext) { + completeLog.put(getName() + UUID.randomUUID(), completionContext); + } + + ConcurrentHashMap> getStartLog() { + return startLog; + } + + ConcurrentHashMap> getCompleteLog() { + return completeLog; + } + + protected String getName() { + return this.getClass().getSimpleName(); + } + + } + + protected static class AnotherLoadBalancerLifecycle + extends TestLoadBalancerLifecycle { + + @Override + protected String getName() { + return this.getClass().getSimpleName(); + } + + } + } +@SuppressWarnings("rawtypes") class DiscoveryClientBasedReactiveLoadBalancer implements ReactorServiceInstanceLoadBalancer { diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/CachingServiceInstanceListSupplierTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/CachingServiceInstanceListSupplierTests.java index 6b3e51a3..605b1059 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/CachingServiceInstanceListSupplierTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/CachingServiceInstanceListSupplierTests.java @@ -110,8 +110,9 @@ class CachingServiceInstanceListSupplierTests { @Bean BlockingLoadBalancerClient blockingLoadBalancerClient( - LoadBalancerClientFactory loadBalancerClientFactory) { - return new BlockingLoadBalancerClient(loadBalancerClientFactory); + LoadBalancerClientFactory loadBalancerClientFactory, + LoadBalancerProperties properties) { + return new BlockingLoadBalancerClient(loadBalancerClientFactory, properties); } @Bean 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 9d219de3..b0c2658e 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 @@ -176,7 +176,7 @@ public class LoadBalancerTests { } @EnableAutoConfiguration - @SpringBootConfiguration + @SpringBootConfiguration(proxyBeanMethods = false) @LoadBalancerClients({ @LoadBalancerClient(name = "myservice", configuration = MyServiceConfig.class),