From 2163b3e27543c1a1962dc7785c1a8c97db1ebbef Mon Sep 17 00:00:00 2001 From: hutiefang Date: Wed, 1 Jul 2026 23:35:08 +0800 Subject: [PATCH 1/3] Fix circuit breaker Feign builder direct usage Signed-off-by: hutiefang --- .../FeignCircuitBreakerInvocationHandler.java | 8 ++- .../openfeign/FeignClientsConfiguration.java | 11 ++- .../FeignAutoConfigurationTests.java | 71 +++++++++++++++++++ 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreakerInvocationHandler.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreakerInvocationHandler.java index 6acac2c0d..b58304efa 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreakerInvocationHandler.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreakerInvocationHandler.java @@ -95,9 +95,11 @@ else if ("toString".equals(method.getName())) { return toString(); } - String circuitName = circuitBreakerNameResolver.resolveCircuitBreakerName(feignClientName, target, method); - CircuitBreaker circuitBreaker = circuitBreakerGroupEnabled ? factory.create(circuitName, feignClientName) - : factory.create(circuitName); + String resolvedFeignClientName = feignClientName != null ? feignClientName : target.name(); + String circuitName = circuitBreakerNameResolver.resolveCircuitBreakerName(resolvedFeignClientName, target, + method); + CircuitBreaker circuitBreaker = circuitBreakerGroupEnabled + ? factory.create(circuitName, resolvedFeignClientName) : factory.create(circuitName); Supplier supplier = asSupplier(method, args); if (this.nullableFallbackFactory != null) { Function fallbackFunction = throwable -> { diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java index f85e59b30..10734ed26 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java @@ -36,6 +36,7 @@ import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -248,8 +249,14 @@ public Feign.Builder defaultFeignBuilder(Retryer retryer) { @Scope("prototype") @ConditionalOnMissingBean @ConditionalOnBean(CircuitBreakerFactory.class) - public Feign.Builder circuitBreakerFeignBuilder() { - return FeignCircuitBreaker.builder(); + public Feign.Builder circuitBreakerFeignBuilder(CircuitBreakerFactory circuitBreakerFactory, + @Value("${spring.cloud.openfeign.circuitbreaker.group.enabled:false}") boolean circuitBreakerGroupEnabled, + ObjectProvider circuitBreakerNameResolver) { + return FeignCircuitBreaker.builder() + .circuitBreakerFactory(circuitBreakerFactory) + .circuitBreakerGroupEnabled(circuitBreakerGroupEnabled) + .circuitBreakerNameResolver(circuitBreakerNameResolver + .getIfAvailable(() -> (feignClientName, target, method) -> Feign.configKey(target.type(), method))); } } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java index 705a4153b..0aae57b8c 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java @@ -17,7 +17,11 @@ package org.springframework.cloud.openfeign; import java.lang.reflect.Method; +import java.util.Map; +import java.util.function.Supplier; +import feign.Feign; +import feign.InvocationHandlerFactory; import feign.Target; import org.assertj.core.api.Condition; import org.junit.jupiter.api.Test; @@ -25,6 +29,7 @@ import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.assertj.AssertableApplicationContext; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.client.circuitbreaker.CircuitBreaker; import org.springframework.cloud.client.circuitbreaker.CircuitBreakerFactory; import org.springframework.cloud.openfeign.FeignAutoConfiguration.CircuitBreakerPresentFeignTargeterConfiguration.AlphanumericCircuitBreakerNameResolver; import org.springframework.cloud.openfeign.security.OAuth2AccessTokenInterceptor; @@ -33,7 +38,10 @@ import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** * @author Tim Peeters @@ -91,6 +99,63 @@ void shouldInstantiateFeignCircuitBreakerTargeterWhenEnabledWithCustomCircuitBre }); } + @Test + void shouldConfigureCircuitBreakerFeignBuilderWhenUsedDirectly() { + CircuitBreakerFactory circuitBreakerFactory = mock(CircuitBreakerFactory.class); + new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(FeignAutoConfiguration.class, FeignClientsConfiguration.class)) + .withPropertyValues("spring.cloud.openfeign.circuitbreaker.enabled=true", + "spring.cloud.openfeign.circuitbreaker.group.enabled=true", + "spring.cloud.openfeign.httpclient.hc5.enabled=false") + .withBean(CircuitBreakerFactory.class, () -> circuitBreakerFactory) + .run(ctx -> { + CircuitBreakerNameResolver circuitBreakerNameResolver = ctx.getBean(CircuitBreakerNameResolver.class); + Feign.Builder builder = ctx.getBean(Feign.Builder.class); + + assertThat(builder).isInstanceOf(FeignCircuitBreaker.Builder.class) + .hasFieldOrPropertyWithValue("circuitBreakerFactory", circuitBreakerFactory) + .hasFieldOrPropertyWithValue("circuitBreakerGroupEnabled", true) + .hasFieldOrPropertyWithValue("circuitBreakerNameResolver", circuitBreakerNameResolver); + }); + } + + @Test + void shouldConfigureCircuitBreakerFeignBuilderWithoutNameResolverBean() { + CircuitBreakerFactory circuitBreakerFactory = mock(CircuitBreakerFactory.class); + new ApplicationContextRunner().withConfiguration(AutoConfigurations.of(FeignClientsConfiguration.class)) + .withPropertyValues("spring.cloud.openfeign.circuitbreaker.enabled=true", + "spring.cloud.openfeign.httpclient.hc5.enabled=false") + .withBean(CircuitBreakerFactory.class, () -> circuitBreakerFactory) + .run(ctx -> { + Feign.Builder builder = ctx.getBean(Feign.Builder.class); + + assertThat(builder).isInstanceOf(FeignCircuitBreaker.Builder.class) + .hasFieldOrPropertyWithValue("circuitBreakerFactory", circuitBreakerFactory) + .hasFieldOrProperty("circuitBreakerNameResolver"); + }); + } + + @Test + void shouldUseTargetNameWhenFeignClientNameIsNotSet() throws Throwable { + CircuitBreakerFactory circuitBreakerFactory = mock(CircuitBreakerFactory.class); + CircuitBreaker circuitBreaker = mock(CircuitBreaker.class); + Method method = DirectCircuitBreakerClient.class.getMethod("get"); + InvocationHandlerFactory.MethodHandler methodHandler = mock(InvocationHandlerFactory.MethodHandler.class); + Target target = new Target.HardCodedTarget<>(DirectCircuitBreakerClient.class, + "directClient", "http://localhost"); + FeignCircuitBreakerInvocationHandler handler = new FeignCircuitBreakerInvocationHandler(circuitBreakerFactory, + null, target, Map.of(method, methodHandler), null, true, + (feignClientName, targetType, targetMethod) -> feignClientName + "#" + targetMethod.getName()); + + when(circuitBreakerFactory.create("directClient#get", "directClient")).thenReturn(circuitBreaker); + when(circuitBreaker.run(any(Supplier.class))) + .thenAnswer(invocation -> invocation.>getArgument(0).get()); + when(methodHandler.invoke(null)).thenReturn("ok"); + + assertThat(handler.invoke(null, method, null)).isEqualTo("ok"); + verify(circuitBreakerFactory).create("directClient#get", "directClient"); + } + @Test void shouldInstantiateFeignOAuth2FeignRequestInterceptorWithoutInterceptors() { runner @@ -147,6 +212,12 @@ private void assertThatFeignCircuitBreakerTargeterHasSameCircuitBreakerNameResol assertThat(bean).isExactlyInstanceOf(beanClass); } + interface DirectCircuitBreakerClient { + + String get(); + + } + static class CustomCircuitBreakerNameResolver implements CircuitBreakerNameResolver { @Override From 9a187265d98642fb0eebaf6a3de8f07a59201bb2 Mon Sep 17 00:00:00 2001 From: hutiefang Date: Sat, 4 Jul 2026 10:37:59 +0800 Subject: [PATCH 2/3] Align direct Feign builder circuit breaker resolver Signed-off-by: hutiefang --- .../openfeign/FeignClientsConfiguration.java | 12 +++++- .../FeignAutoConfigurationTests.java | 41 ++++++++++++++++++- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java index 10734ed26..dc54d4b5c 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java @@ -46,6 +46,8 @@ import org.springframework.boot.http.converter.autoconfigure.ClientHttpMessageConvertersCustomizer; import org.springframework.cloud.client.circuitbreaker.CircuitBreaker; import org.springframework.cloud.client.circuitbreaker.CircuitBreakerFactory; +import org.springframework.cloud.openfeign.FeignAutoConfiguration.CircuitBreakerPresentFeignTargeterConfiguration.AlphanumericCircuitBreakerNameResolver; +import org.springframework.cloud.openfeign.FeignAutoConfiguration.CircuitBreakerPresentFeignTargeterConfiguration.DefaultCircuitBreakerNameResolver; import org.springframework.cloud.openfeign.clientconfig.FeignClientConfigurer; import org.springframework.cloud.openfeign.support.AbstractFormWriter; import org.springframework.cloud.openfeign.support.FeignEncoderProperties; @@ -251,12 +253,20 @@ public Feign.Builder defaultFeignBuilder(Retryer retryer) { @ConditionalOnBean(CircuitBreakerFactory.class) public Feign.Builder circuitBreakerFeignBuilder(CircuitBreakerFactory circuitBreakerFactory, @Value("${spring.cloud.openfeign.circuitbreaker.group.enabled:false}") boolean circuitBreakerGroupEnabled, + @Value("${spring.cloud.openfeign.circuitbreaker.alphanumeric-ids.enabled:true}") boolean alphanumericIdsEnabled, ObjectProvider circuitBreakerNameResolver) { return FeignCircuitBreaker.builder() .circuitBreakerFactory(circuitBreakerFactory) .circuitBreakerGroupEnabled(circuitBreakerGroupEnabled) .circuitBreakerNameResolver(circuitBreakerNameResolver - .getIfAvailable(() -> (feignClientName, target, method) -> Feign.configKey(target.type(), method))); + .getIfAvailable(() -> defaultCircuitBreakerNameResolver(alphanumericIdsEnabled))); + } + + private CircuitBreakerNameResolver defaultCircuitBreakerNameResolver(boolean alphanumericIdsEnabled) { + if (alphanumericIdsEnabled) { + return new AlphanumericCircuitBreakerNameResolver(); + } + return new DefaultCircuitBreakerNameResolver(); } } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java index 0aae57b8c..9d5d607cb 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java @@ -32,10 +32,12 @@ import org.springframework.cloud.client.circuitbreaker.CircuitBreaker; import org.springframework.cloud.client.circuitbreaker.CircuitBreakerFactory; import org.springframework.cloud.openfeign.FeignAutoConfiguration.CircuitBreakerPresentFeignTargeterConfiguration.AlphanumericCircuitBreakerNameResolver; +import org.springframework.cloud.openfeign.FeignAutoConfiguration.CircuitBreakerPresentFeignTargeterConfiguration.DefaultCircuitBreakerNameResolver; import org.springframework.cloud.openfeign.security.OAuth2AccessTokenInterceptor; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.security.oauth2.client.OAuth2AuthorizedClientService; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -120,7 +122,7 @@ void shouldConfigureCircuitBreakerFeignBuilderWhenUsedDirectly() { } @Test - void shouldConfigureCircuitBreakerFeignBuilderWithoutNameResolverBean() { + void shouldConfigureCircuitBreakerFeignBuilderWithoutNameResolverBean() throws NoSuchMethodException { CircuitBreakerFactory circuitBreakerFactory = mock(CircuitBreakerFactory.class); new ApplicationContextRunner().withConfiguration(AutoConfigurations.of(FeignClientsConfiguration.class)) .withPropertyValues("spring.cloud.openfeign.circuitbreaker.enabled=true", @@ -128,10 +130,45 @@ void shouldConfigureCircuitBreakerFeignBuilderWithoutNameResolverBean() { .withBean(CircuitBreakerFactory.class, () -> circuitBreakerFactory) .run(ctx -> { Feign.Builder builder = ctx.getBean(Feign.Builder.class); + CircuitBreakerNameResolver circuitBreakerNameResolver = (CircuitBreakerNameResolver) ReflectionTestUtils + .getField(builder, "circuitBreakerNameResolver"); + Method method = DirectCircuitBreakerClient.class.getMethod("get"); + Target target = new Target.HardCodedTarget<>( + DirectCircuitBreakerClient.class, "directClient", "http://localhost"); assertThat(builder).isInstanceOf(FeignCircuitBreaker.Builder.class) .hasFieldOrPropertyWithValue("circuitBreakerFactory", circuitBreakerFactory) - .hasFieldOrProperty("circuitBreakerNameResolver"); + .hasFieldOrPropertyWithValue("circuitBreakerNameResolver", circuitBreakerNameResolver); + assertThat(circuitBreakerNameResolver) + .isExactlyInstanceOf(AlphanumericCircuitBreakerNameResolver.class); + assertThat(circuitBreakerNameResolver.resolveCircuitBreakerName("directClient", target, method)) + .isEqualTo(Feign.configKey(target.type(), method).replaceAll("[^a-zA-Z0-9]", "")); + }); + } + + @Test + void shouldConfigureDefaultCircuitBreakerFeignBuilderNameResolverWhenAlphanumericIdsDisabled() + throws NoSuchMethodException { + CircuitBreakerFactory circuitBreakerFactory = mock(CircuitBreakerFactory.class); + new ApplicationContextRunner().withConfiguration(AutoConfigurations.of(FeignClientsConfiguration.class)) + .withPropertyValues("spring.cloud.openfeign.circuitbreaker.enabled=true", + "spring.cloud.openfeign.circuitbreaker.alphanumeric-ids.enabled=false", + "spring.cloud.openfeign.httpclient.hc5.enabled=false") + .withBean(CircuitBreakerFactory.class, () -> circuitBreakerFactory) + .run(ctx -> { + Feign.Builder builder = ctx.getBean(Feign.Builder.class); + CircuitBreakerNameResolver circuitBreakerNameResolver = (CircuitBreakerNameResolver) ReflectionTestUtils + .getField(builder, "circuitBreakerNameResolver"); + Method method = DirectCircuitBreakerClient.class.getMethod("get"); + Target target = new Target.HardCodedTarget<>( + DirectCircuitBreakerClient.class, "directClient", "http://localhost"); + + assertThat(builder).isInstanceOf(FeignCircuitBreaker.Builder.class) + .hasFieldOrPropertyWithValue("circuitBreakerFactory", circuitBreakerFactory) + .hasFieldOrPropertyWithValue("circuitBreakerNameResolver", circuitBreakerNameResolver); + assertThat(circuitBreakerNameResolver).isExactlyInstanceOf(DefaultCircuitBreakerNameResolver.class); + assertThat(circuitBreakerNameResolver.resolveCircuitBreakerName("directClient", target, method)) + .isEqualTo(Feign.configKey(target.type(), method)); }); } From 34f202a4761f76405286f82e7af8e942ec620200 Mon Sep 17 00:00:00 2001 From: hutiefang Date: Tue, 7 Jul 2026 00:02:48 +0800 Subject: [PATCH 3/3] Preserve circuit breaker builder method signature Signed-off-by: hutiefang --- .../openfeign/FeignClientsConfiguration.java | 20 +++++++++++++++++-- .../FeignAutoConfigurationTests.java | 10 ++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java index dc54d4b5c..6f8e2557c 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java @@ -240,6 +240,18 @@ public Feign.Builder feignBuilder(Retryer retryer) { @ConditionalOnProperty("spring.cloud.openfeign.circuitbreaker.enabled") protected static class CircuitBreakerPresentFeignBuilderConfiguration { + @Autowired + private CircuitBreakerFactory circuitBreakerFactory; + + @Value("${spring.cloud.openfeign.circuitbreaker.group.enabled:false}") + private boolean circuitBreakerGroupEnabled; + + @Value("${spring.cloud.openfeign.circuitbreaker.alphanumeric-ids.enabled:true}") + private boolean alphanumericIdsEnabled; + + @Autowired + private ObjectProvider circuitBreakerNameResolver; + @Bean @Scope("prototype") @ConditionalOnMissingBean({ Feign.Builder.class, CircuitBreakerFactory.class }) @@ -251,9 +263,13 @@ public Feign.Builder defaultFeignBuilder(Retryer retryer) { @Scope("prototype") @ConditionalOnMissingBean @ConditionalOnBean(CircuitBreakerFactory.class) + public Feign.Builder circuitBreakerFeignBuilder() { + return circuitBreakerFeignBuilder(circuitBreakerFactory, circuitBreakerGroupEnabled, alphanumericIdsEnabled, + circuitBreakerNameResolver); + } + public Feign.Builder circuitBreakerFeignBuilder(CircuitBreakerFactory circuitBreakerFactory, - @Value("${spring.cloud.openfeign.circuitbreaker.group.enabled:false}") boolean circuitBreakerGroupEnabled, - @Value("${spring.cloud.openfeign.circuitbreaker.alphanumeric-ids.enabled:true}") boolean alphanumericIdsEnabled, + boolean circuitBreakerGroupEnabled, boolean alphanumericIdsEnabled, ObjectProvider circuitBreakerNameResolver) { return FeignCircuitBreaker.builder() .circuitBreakerFactory(circuitBreakerFactory) diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java index 9d5d607cb..63158692a 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java @@ -17,6 +17,7 @@ package org.springframework.cloud.openfeign; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Map; import java.util.function.Supplier; @@ -101,6 +102,15 @@ void shouldInstantiateFeignCircuitBreakerTargeterWhenEnabledWithCustomCircuitBre }); } + @Test + void shouldKeepNoArgCircuitBreakerFeignBuilderSignature() throws NoSuchMethodException { + Method method = FeignClientsConfiguration.CircuitBreakerPresentFeignBuilderConfiguration.class + .getDeclaredMethod("circuitBreakerFeignBuilder"); + + assertThat(Modifier.isPublic(method.getModifiers())).isTrue(); + assertThat(method.getReturnType()).isEqualTo(Feign.Builder.class); + } + @Test void shouldConfigureCircuitBreakerFeignBuilderWhenUsedDirectly() { CircuitBreakerFactory circuitBreakerFactory = mock(CircuitBreakerFactory.class);