Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.beans.factory.ObjectProvider;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.boot.autoconfigure.condition.AllNestedConditions;
import org.springframework.boot.autoconfigure.condition.AnyNestedCondition;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
Expand All @@ -48,8 +48,6 @@
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Conditional;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.client.RestClient;
import org.springframework.web.reactive.function.client.WebClient;

/**
* @author Daniel Lavoie
Expand Down Expand Up @@ -104,21 +102,21 @@ protected static class WebClientConfiguration {
value = { AbstractDiscoveryClientOptionalArgs.class, RestClientDiscoveryClientOptionalArgs.class },
search = SearchStrategy.CURRENT)
public WebClientDiscoveryClientOptionalArgs webClientDiscoveryClientOptionalArgs(TlsProperties tlsProperties,
ObjectProvider<WebClient.Builder> builder) throws GeneralSecurityException, IOException {
ConfigurableListableBeanFactory beanFactory) throws GeneralSecurityException, IOException {
if (LOG.isInfoEnabled()) {
LOG.info("Eureka HTTP Client uses WebClient.");
}
WebClientDiscoveryClientOptionalArgs result = new WebClientDiscoveryClientOptionalArgs(
builder::getIfAvailable);
EurekaClientBuilderSuppliers.webClientBuilder(beanFactory));
setupTLS(result, tlsProperties);
return result;
}

@Bean
@ConditionalOnMissingBean(value = TransportClientFactories.class, search = SearchStrategy.CURRENT)
public WebClientTransportClientFactories webClientTransportClientFactories(
ObjectProvider<WebClient.Builder> builder) {
return new WebClientTransportClientFactories(builder::getIfAvailable);
ConfigurableListableBeanFactory beanFactory) {
return new WebClientTransportClientFactories(EurekaClientBuilderSuppliers.webClientBuilder(beanFactory));
}

}
Expand Down Expand Up @@ -154,14 +152,13 @@ EurekaClientHttpRequestFactorySupplier defaultEurekaClientHttpRequestFactorySupp
search = SearchStrategy.CURRENT)
public RestClientDiscoveryClientOptionalArgs restClientDiscoveryClientOptionalArgs(TlsProperties tlsProperties,
EurekaClientHttpRequestFactorySupplier eurekaClientHttpRequestFactorySupplier,
ObjectProvider<RestClient.Builder> restClientBuilderProvider)
throws GeneralSecurityException, IOException {
ConfigurableListableBeanFactory beanFactory) throws GeneralSecurityException, IOException {
if (LOG.isInfoEnabled()) {
LOG.info("Eureka HTTP Client uses RestClient.");
}
RestClientDiscoveryClientOptionalArgs result = new RestClientDiscoveryClientOptionalArgs(
eurekaClientHttpRequestFactorySupplier,
() -> restClientBuilderProvider.getIfAvailable(RestClient::builder));
EurekaClientBuilderSuppliers.restClientBuilder(beanFactory));
setupTLS(result, tlsProperties);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright 2017-present 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.netflix.eureka.config;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.function.Supplier;

import jakarta.annotation.Priority;

import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.cloud.client.loadbalancer.LoadBalanced;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.web.client.RestClient;
import org.springframework.web.reactive.function.client.WebClient;

/**
* Supplies HTTP client builders for Eureka registry transport, excluding
* {@link LoadBalanced @LoadBalanced} beans so registry hosts are not resolved as service
* ids.
*
* @author arimu1
*/
final class EurekaClientBuilderSuppliers {

private EurekaClientBuilderSuppliers() {
}

static Supplier<RestClient.Builder> restClientBuilder(ConfigurableListableBeanFactory beanFactory) {
RestClient.Builder builder = resolveNonLoadBalancedBuilder(beanFactory, RestClient.Builder.class,
RestClient::builder);
return () -> builder;
}

static Supplier<WebClient.Builder> webClientBuilder(ConfigurableListableBeanFactory beanFactory) {
WebClient.Builder builder = resolveNonLoadBalancedBuilder(beanFactory, WebClient.Builder.class,
WebClient::builder);
return () -> builder;
}

private static <T> T resolveNonLoadBalancedBuilder(ConfigurableListableBeanFactory beanFactory, Class<T> type,
Comment thread
ryanjbaxter marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will end up picking one non-loadbalanced bean when there is more than one and there is no @Order/@Priority/@Primary. IMO in this case we should throw a NoUniqueBeanDefinitionException

Supplier<T> fallback) {
List<String> candidates = new ArrayList<>();
for (String name : beanFactory.getBeanNamesForType(type)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The selected bean ignores @Order/@Priority of beans

if (beanFactory.findAnnotationOnBean(name, LoadBalanced.class) == null) {
candidates.add(name);
}
}
if (candidates.isEmpty()) {
return fallback.get();
}
String beanName = selectHighestPriorityBeanName(beanFactory, candidates);
return beanFactory.getBean(beanName, type);
}

private static String selectHighestPriorityBeanName(ConfigurableListableBeanFactory beanFactory,
List<String> candidates) {
if (candidates.size() == 1) {
return candidates.get(0);
}
candidates.sort(
Comparator.comparingInt((String name) -> orderFor(beanFactory, name)).thenComparing(String::compareTo));
return candidates.get(0);
}

private static int orderFor(ConfigurableListableBeanFactory beanFactory, String beanName) {
if (beanFactory instanceof DefaultListableBeanFactory defaultListableBeanFactory) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause the bean to be instantiated. IMO we can just use beanFactory.findAnnotationOnBean here and if there isn't an annotation then treat it as Ordered.LOWEST_PRECEDENCE

return defaultListableBeanFactory.getOrder(beanName);
}
Order order = beanFactory.findAnnotationOnBean(beanName, Order.class);
if (order != null) {
return order.value();
}
Priority priority = beanFactory.findAnnotationOnBean(beanName, Priority.class);
if (priority != null) {
return priority.value();
}
Object bean = beanFactory.getBean(beanName);
if (bean instanceof Ordered ordered) {
return ordered.getOrder();
}
return Ordered.LOWEST_PRECEDENCE;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
/*
* Copyright 2017-present 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.netflix.eureka.config;

import java.util.function.Supplier;

import org.junit.jupiter.api.Test;

import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
import org.springframework.cloud.client.loadbalancer.LoadBalanced;
import org.springframework.cloud.netflix.eureka.http.RestClientDiscoveryClientOptionalArgs;
import org.springframework.cloud.netflix.eureka.http.WebClientTransportClientFactories;
import org.springframework.cloud.netflix.eureka.sample.EurekaSampleApplication;
import org.springframework.cloud.test.ClassPathExclusions;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.web.client.RestClient;
import org.springframework.web.reactive.function.client.WebClient;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author arimu1
*/
@ClassPathExclusions({ "jersey-client-*", "jersey-core-*", "jersey-apache-client4-*" })
class LoadBalancedEurekaHttpClientBuilderConfigurationTests {

@Test
void restClientTransportDoesNotUseLoadBalancedRestClientBuilderWhenItIsTheOnlyCandidate() {
new WebApplicationContextRunner()
.withUserConfiguration(EurekaSampleApplication.class, LoadBalancedRestClientConfiguration.class)
.withPropertyValues("eureka.client.webclient.enabled=false")
.run(context -> {
RestClientDiscoveryClientOptionalArgs args = context
.getBean(RestClientDiscoveryClientOptionalArgs.class);
RestClient.Builder loadBalancedBuilder = context.getBean("loadBalancedRestClientBuilder",
RestClient.Builder.class);
Supplier<RestClient.Builder> supplier = getRestClientBuilderSupplier(args);
assertThat(supplier.get()).isNotSameAs(loadBalancedBuilder);
});
}

@Test
void restClientTransportPrefersPlainRestClientBuilderOverLoadBalanced() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebClient should have a similar test

new WebApplicationContextRunner()
.withUserConfiguration(EurekaSampleApplication.class, PlainAndLoadBalancedRestClientConfiguration.class)
.withPropertyValues("eureka.client.webclient.enabled=false")
.run(context -> {
RestClientDiscoveryClientOptionalArgs args = context
.getBean(RestClientDiscoveryClientOptionalArgs.class);
RestClient.Builder plainBuilder = context.getBean("plainRestClientBuilder", RestClient.Builder.class);
Supplier<RestClient.Builder> supplier = getRestClientBuilderSupplier(args);
assertThat(supplier.get()).isSameAs(plainBuilder);
});
}

@Test
void webClientTransportDoesNotUseLoadBalancedWebClientBuilderWhenItIsTheOnlyCandidate() {
new WebApplicationContextRunner()
.withUserConfiguration(EurekaSampleApplication.class, LoadBalancedWebClientConfiguration.class)
.withPropertyValues("eureka.client.webclient.enabled=true")
.run(context -> {
WebClientTransportClientFactories factories = context.getBean(WebClientTransportClientFactories.class);
WebClient.Builder loadBalancedBuilder = context.getBean("loadBalancedWebClientBuilder",
WebClient.Builder.class);
Supplier<WebClient.Builder> supplier = getWebClientBuilderSupplier(factories);
assertThat(supplier.get()).isNotSameAs(loadBalancedBuilder);
});
}

@Test
void webClientTransportPrefersPlainWebClientBuilderOverLoadBalanced() {
new WebApplicationContextRunner()
.withUserConfiguration(EurekaSampleApplication.class, PlainAndLoadBalancedWebClientConfiguration.class)
.withPropertyValues("eureka.client.webclient.enabled=true")
.run(context -> {
WebClientTransportClientFactories factories = context.getBean(WebClientTransportClientFactories.class);
WebClient.Builder plainBuilder = context.getBean("plainWebClientBuilder", WebClient.Builder.class);
Supplier<WebClient.Builder> supplier = getWebClientBuilderSupplier(factories);
assertThat(supplier.get()).isSameAs(plainBuilder);
});
}

@SuppressWarnings("unchecked")
private static Supplier<RestClient.Builder> getRestClientBuilderSupplier(
RestClientDiscoveryClientOptionalArgs args) {
return (Supplier<RestClient.Builder>) ReflectionTestUtils.getField(args, "restClientBuilderSupplier");
}

@SuppressWarnings("unchecked")
private static Supplier<WebClient.Builder> getWebClientBuilderSupplier(
WebClientTransportClientFactories factories) {
return (Supplier<WebClient.Builder>) ReflectionTestUtils.getField(factories, "builder");
}

@Configuration(proxyBeanMethods = false)
static class LoadBalancedRestClientConfiguration {

@Bean
@LoadBalanced
RestClient.Builder loadBalancedRestClientBuilder() {
return RestClient.builder();
}

}

@Configuration(proxyBeanMethods = false)
static class PlainAndLoadBalancedRestClientConfiguration {

@Bean
RestClient.Builder plainRestClientBuilder() {
return RestClient.builder();
}

@Bean
@LoadBalanced
RestClient.Builder loadBalancedRestClientBuilder() {
return RestClient.builder();
}

}

@Configuration(proxyBeanMethods = false)
static class LoadBalancedWebClientConfiguration {

@Bean
@LoadBalanced
WebClient.Builder loadBalancedWebClientBuilder() {
return WebClient.builder();
}

}

@Configuration(proxyBeanMethods = false)
static class PlainAndLoadBalancedWebClientConfiguration {

@Bean
WebClient.Builder plainWebClientBuilder() {
return WebClient.builder();
}

@Bean
@LoadBalanced
WebClient.Builder loadBalancedWebClientBuilder() {
return WebClient.builder();
}

}

}