Skip to content

Commit

Permalink
Fix support of "qualifier" parameter as feign client bean name (#642)
Browse files Browse the repository at this point in the history
Fixes #611

Co-authored-by: skarpenko <[email protected]>
  • Loading branch information
kptfh and skarpenko authored Nov 14, 2023
1 parent 7651d2c commit d890bd6
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;


Expand All @@ -48,7 +49,7 @@ public ReactiveFeignBuilder configure(
ReactiveFeignBuilder builder,
ReactiveFeignNamedContext namedContext) {

if (namedContext.getProperties().isDefaultToProperties()) {
if (isDefaultToProperties(namedContext)) {
builder = configureUsingConfiguration(builder, namedContext);
for(ReactiveFeignClientsProperties.ReactiveFeignClientProperties<?> config : namedContext.getConfigsReverted()){
builder = configureUsingProperties(builder, namedContext, config);
Expand All @@ -62,6 +63,13 @@ public ReactiveFeignBuilder configure(
return builder;
}

private boolean isDefaultToProperties(ReactiveFeignNamedContext namedContext){
Map<String, ReactiveFeignClientsProperties.ReactiveFeignClientProperties<?>> config = namedContext.getProperties().getConfig();
return Optional.ofNullable(config.get(namedContext.getClientName()))
.map(ReactiveFeignClientsProperties.ReactiveFeignClientProperties::getDefaultToProperties)
.orElse(namedContext.getProperties().isDefaultToProperties());
}

private ReactiveFeignBuilder configureUsingConfiguration(ReactiveFeignBuilder builder, ReactiveFeignNamedContext namedContext) {
ReactiveFeignBuilder resultBuilder = builder;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import reactivefeign.client.statushandler.ReactiveStatusHandler;
import reactivefeign.retry.ReactiveRetryPolicy;

import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -65,6 +64,8 @@ public int hashCode() {

public static class ReactiveFeignClientProperties<O extends ReactiveOptions.Builder> {

private Boolean defaultToProperties;

private O options;

//used for no cloud configuration
Expand Down Expand Up @@ -92,6 +93,14 @@ public static class ReactiveFeignClientProperties<O extends ReactiveOptions.Buil

private Map<String, List<String>> defaultQueryParameters;

public Boolean getDefaultToProperties() {
return defaultToProperties;
}

public void setDefaultToProperties(Boolean defaultToProperties) {
this.defaultToProperties = defaultToProperties;
}

public O getOptions() {
return options;
}
Expand Down Expand Up @@ -193,7 +202,8 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ReactiveFeignClientProperties that = (ReactiveFeignClientProperties) o;
return Objects.equals(options, that.options) &&
return Objects.equals(defaultToProperties, that.defaultToProperties) &&
Objects.equals(options, that.options) &&
Objects.equals(retryOnSame, that.retryOnSame) &&
Objects.equals(retryOnNext, that.retryOnNext) &&
Objects.equals(statusHandler, that.statusHandler) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.springframework.core.type.filter.TypeFilter;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;

import java.io.IOException;
import java.net.MalformedURLException;
Expand All @@ -53,6 +52,7 @@
import java.util.Map;
import java.util.Set;

import static org.springframework.util.StringUtils.hasText;
import static reactivefeign.utils.StringUtils.cutTail;

/**
Expand Down Expand Up @@ -148,9 +148,11 @@ protected boolean match(ClassMetadata metadata) {
.getAnnotationAttributes(
ReactiveFeignClient.class.getCanonicalName());

String name = getClientName(attributes);
registerClientConfiguration(registry, name,
attributes.get("configuration"));
String name = getQualifier(attributes);
if(!hasText(name)){
name = getClientName(attributes);
}
registerClientConfiguration(registry, name, attributes.get("configuration"));

registerReactiveFeignClient(registry, annotationMetadata, attributes);
}
Expand Down Expand Up @@ -183,7 +185,7 @@ private void registerReactiveFeignClient(BeanDefinitionRegistry registry,
beanDefinition.setPrimary(primary);

String qualifier = getQualifier(attributes);
if (StringUtils.hasText(qualifier)) {
if (hasText(qualifier)) {
alias = qualifier;
}

Expand Down Expand Up @@ -215,18 +217,18 @@ static void validateFallbackFactory(final Class clazz) {

/* for testing */ String getName(Map<String, Object> attributes) {
String name = (String) attributes.get("serviceId");
if (!StringUtils.hasText(name)) {
if (!hasText(name)) {
name = (String) attributes.get("name");
}
if (!StringUtils.hasText(name)) {
if (!hasText(name)) {
name = (String) attributes.get("value");
}
name = resolve(name);
return getName(name);
}

static String getName(String name) {
if (!StringUtils.hasText(name)) {
if (!hasText(name)) {
return "";
}

Expand All @@ -248,7 +250,7 @@ static String getName(String name) {
}

private String resolve(String value) {
if (StringUtils.hasText(value)) {
if (hasText(value)) {
return this.environment.resolvePlaceholders(value);
}
return value;
Expand All @@ -260,7 +262,7 @@ private String getUrl(Map<String, Object> attributes) {
}

static String getUrl(String url) {
if (StringUtils.hasText(url) && !(url.startsWith("#{") && url.contains("}"))) {
if (hasText(url) && !(url.startsWith("#{") && url.contains("}"))) {
if (!url.contains("://")) {
url = "http://" + url;
}
Expand All @@ -280,7 +282,7 @@ private String getPath(Map<String, Object> attributes) {
}

static String getPath(String path) {
if (StringUtils.hasText(path)) {
if (hasText(path)) {
path = path.trim();
if (!path.startsWith("/")) {
path = "/" + path;
Expand Down Expand Up @@ -311,12 +313,12 @@ protected Set<String> getBasePackages(AnnotationMetadata importingClassMetadata)

Set<String> basePackages = new HashSet<>();
for (String pkg : (String[]) attributes.get("value")) {
if (StringUtils.hasText(pkg)) {
if (hasText(pkg)) {
basePackages.add(pkg);
}
}
for (String pkg : (String[]) attributes.get("basePackages")) {
if (StringUtils.hasText(pkg)) {
if (hasText(pkg)) {
basePackages.add(pkg);
}
}
Expand All @@ -336,7 +338,7 @@ private String getQualifier(Map<String, Object> client) {
return null;
}
String qualifier = (String) client.get("qualifier");
if (StringUtils.hasText(qualifier)) {
if (hasText(qualifier)) {
return qualifier;
}
return null;
Expand All @@ -347,13 +349,13 @@ private String getClientName(Map<String, Object> client) {
return null;
}
String value = (String) client.get("value");
if (!StringUtils.hasText(value)) {
if (!hasText(value)) {
value = (String) client.get("name");
}
if (!StringUtils.hasText(value)) {
if (!hasText(value)) {
value = (String) client.get("serviceId");
}
if (StringUtils.hasText(value)) {
if (hasText(value)) {
return value;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package reactivefeign.spring.config.cloud2;

import com.github.tomakehurst.wiremock.WireMockServer;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import reactivefeign.spring.config.EnableReactiveFeignClients;
import reactivefeign.spring.config.ReactiveFeignClient;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathMatching;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
import static java.util.Arrays.asList;
import static reactivefeign.spring.config.cloud2.BasicAutoconfigurationTest.MOCK_SERVER_PORT_PROPERTY;
import static reactivefeign.spring.config.cloud2.QualifierTest.RFGN_PROPS;

@RunWith(SpringJUnit4ClassRunner.class)
@SpringBootTest(classes = QualifierTest.TestConfiguration.class, webEnvironment = SpringBootTest.WebEnvironment.NONE,
properties = {
"spring.cloud.discovery.client.simple.instances."+RFGN_PROPS+"[0].uri=http://localhost:${"+ MOCK_SERVER_PORT_PROPERTY+"}",
"reactive.feign.client.config.default.options.readTimeoutMillis=100",
"reactive.feign.client.config.rfgn-proper.options.readTimeoutMillis=500"
})
@DirtiesContext
public class QualifierTest extends BasicAutoconfigurationTest{

public static final String RFGN_PROPS = "rfgn-proper";

private static final WireMockServer mockHttpServer = new WireMockServer(wireMockConfig().dynamicPort());

@Autowired
private PropsSampleClient propsSampleClient;

@Autowired
private PropsSampleClientWithOtherQualifier propsSampleClientWithOtherQualifier;


@Test
public void shouldRunClientsWithSameNameAndDifferentQualifiers() {
mockHttpServer.stubFor(get(urlPathMatching("/sampleUrl"))
.willReturn(aResponse()
.withFixedDelay(300)
.withBody("OK")));

asList(propsSampleClient, propsSampleClientWithOtherQualifier).forEach(feignClient -> {
StepVerifier.create(feignClient.sampleMethod())
.expectNext("OK")
.verifyComplete();
});
}

@ReactiveFeignClient(name = RFGN_PROPS, qualifier = "FeignClient1")
protected interface PropsSampleClient extends SampleClient {

@RequestMapping(method = RequestMethod.GET, value = "/sampleUrl")
Mono<String> sampleMethod();
}

@ReactiveFeignClient(name = RFGN_PROPS, qualifier = "FeignClient2")
protected interface PropsSampleClientWithOtherQualifier extends SampleClient {

@RequestMapping(method = RequestMethod.GET, value = "/sampleUrl")
Mono<String> sampleMethod();
}

protected interface SampleClient{
Mono<String> sampleMethod();
}

@Configuration
@EnableAutoConfiguration
@EnableReactiveFeignClients(
clients = {PropsSampleClient.class,
PropsSampleClientWithOtherQualifier.class})
protected static class TestConfiguration {
}

@BeforeClass
public static void setupStubs() {
mockHttpServer.start();

System.setProperty(MOCK_SERVER_PORT_PROPERTY, Integer.toString(mockHttpServer.port()));
}

@Before
public void reset() throws InterruptedException {
//to close circuit breaker
mockHttpServer.resetAll();
}

@AfterClass
public static void teardown() {
mockHttpServer.stop();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public class SampleConfigurationsTest extends BasicAutoconfigurationTest{
public static final int UPDATE_INTERVAL = 5;
public static final int SLEEP_WINDOW = 1000;

private static WireMockServer mockHttpServer = new WireMockServer(wireMockConfig().dynamicPort());
private static final WireMockServer mockHttpServer = new WireMockServer(wireMockConfig().dynamicPort());

//configured via properties file
@Autowired
Expand All @@ -107,7 +107,7 @@ public class SampleConfigurationsTest extends BasicAutoconfigurationTest{
@Autowired
private ErrorDecoderSampleClient errorDecoderSampleClient;

private static CircuitBreakerRegistry circuitBreakerRegistry = CircuitBreakerRegistry.ofDefaults();
private static final CircuitBreakerRegistry circuitBreakerRegistry = CircuitBreakerRegistry.ofDefaults();

//this test checks that default readTimeoutMillis is overridden for each client
// (one in via properties file and other via configuration class)
Expand All @@ -119,7 +119,7 @@ public void shouldRetryAndFailOnRibbon() {
.withBody("OK")));

asList(propertiesSampleClient, configsSampleClient).forEach(feignClient -> {
StepVerifier.create(propertiesSampleClient.sampleMethod())
StepVerifier.create(feignClient.sampleMethod())
.expectErrorMatches(throwable ->
throwable instanceof RuntimeException
&& throwable.getCause() instanceof OutOfRetriesException
Expand Down Expand Up @@ -204,34 +204,37 @@ public void shouldOpenCircuitBreakerButNotWrapException() throws InterruptedExce
}

@ReactiveFeignClient(name = RFGN_PROPER)
protected interface PropertiesSampleClient {
protected interface PropertiesSampleClient extends SampleClient{

@RequestMapping(method = RequestMethod.GET, value = "/sampleUrl")
Mono<String> sampleMethod();
}

@ReactiveFeignClient(name = RFGN_CONFIGS,
configuration = ReactiveFeignSampleConfiguration.class)
protected interface ConfigsSampleClient {
protected interface ConfigsSampleClient extends SampleClient{

@RequestMapping(method = RequestMethod.GET, value = "/sampleUrl")
Mono<String> sampleMethod();
}

@ReactiveFeignClient(name = RFGN_FALLBACK,
fallback = ReactiveFeignFallbackConfiguration.Fallback.class,
configuration = ReactiveFeignFallbackConfiguration.class)
protected interface FallbackSampleClient {
protected interface FallbackSampleClient extends SampleClient{
@RequestMapping(method = RequestMethod.GET, value = "/sampleUrl")
Mono<String> sampleMethod();
}

@ReactiveFeignClient(name = RFGN_ERRORDECODER, fallbackFactory = ErrorDecoderSkipFallbackFactory.class)
protected interface ErrorDecoderSampleClient {
protected interface ErrorDecoderSampleClient extends SampleClient{
@RequestMapping(method = RequestMethod.GET, value = "/sampleUrl")
Mono<String> sampleMethod();
}

protected interface SampleClient{
Mono<String> sampleMethod();
}

protected static class ErrorDecoderSkipFallbackFactory implements FallbackFactory<ErrorDecoderSampleClient> {

@Override
Expand Down Expand Up @@ -297,7 +300,7 @@ protected static class ReactiveFeignSampleConfiguration {

@Bean
public ReactiveOptions reactiveOptions(){
return new WebReactiveOptions.Builder().setReadTimeoutMillis(500).build();
return new WebReactiveOptions.Builder().setReadTimeoutMillis(300).build();
}

@Bean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ reactive.feign.client.config.rfgn-proper.retryOnSame.builder=reactivefeign.retry
reactive.feign.client.config.rfgn-proper.retryOnSame.args.maxRetries=1
reactive.feign.client.config.rfgn-proper.retryOnSame.args.backoffInMs=10

reactive.feign.client.config.rfgn-configs.defaultToProperties=false

reactive.feign.client.config.rfgn-errordecoder.errorDecoder=reactivefeign.spring.config.cloud2.ErrorDecoder

0 comments on commit d890bd6

Please sign in to comment.