From c29d57e61f0fa6ad47a3e2d01cf0228341ea890c Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Mon, 20 Feb 2023 14:27:34 +0100 Subject: [PATCH] Fix condition. --- .../main/asciidoc/spring-cloud-openfeign.adoc | 2 + .../FeignCircuitBreakerInvocationHandler.java | 14 ++--- ...gnAcceptGzipEncodingAutoConfiguration.java | 9 ++-- ...nContentGzipEncodingAutoConfiguration.java | 9 ++-- ...OkHttpFeignClientBeanMissingCondition.java | 51 +++++++++++++++++++ .../openfeign/FeignCompressionTests.java | 34 +++++++++++-- 6 files changed, 101 insertions(+), 18 deletions(-) create mode 100644 spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/OkHttpFeignClientBeanMissingCondition.java diff --git a/docs/src/main/asciidoc/spring-cloud-openfeign.adoc b/docs/src/main/asciidoc/spring-cloud-openfeign.adoc index 49d1040d..1858b0e3 100644 --- a/docs/src/main/asciidoc/spring-cloud-openfeign.adoc +++ b/docs/src/main/asciidoc/spring-cloud-openfeign.adoc @@ -561,6 +561,8 @@ feign.compression.request.min-request-size=2048 These properties allow you to be selective about the compressed media types and minimum request threshold length. +TIP: Since the OkHttpClient uses "transparent" compression, that is disabled if the `content-encoding` or `accept-encoding` header is present, we do not enable compression when `feign.okhttp.OkHttpClient` is present on the classpath and `feign.okhttp.enabled` is set to `true`. + === Feign logging A logger is created for each Feign client created. By default the name of the logger is the full class name of the interface used to create the Feign client. Feign logging only responds to the `DEBUG` level. 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 5c27e6c1..51258b0e 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 @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2023 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. @@ -154,12 +154,12 @@ class FeignCircuitBreakerInvocationHandler implements InvocationHandler { } /** - * If the method param of InvocationHandler.invoke is not accessible, i.e in a - * package-private interface, the fallback call will cause of access restrictions. But - * methods in dispatch are copied methods. So setting access to dispatch method - * doesn't take effect to the method in InvocationHandler.invoke. Use map to store a - * copy of method to invoke the fallback to bypass this and reducing the count of - * reflection calls. + * If the method param of {@link InvocationHandler#invoke(Object, Method, Object[])} + * is not accessible, i.e in a package-private interface, the fallback call will cause + * of access restrictions. But methods in dispatch are copied methods. So setting + * access to dispatch method doesn't take effect to the method in + * InvocationHandler.invoke. Use map to store a copy of method to invoke the fallback + * to bypass this and reducing the count of reflection calls. * @return cached methods map for fallback invoking */ static Map toFallbackMethod(Map dispatch) { diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/FeignAcceptGzipEncodingAutoConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/FeignAcceptGzipEncodingAutoConfiguration.java index 665ed107..b5f1f3ae 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/FeignAcceptGzipEncodingAutoConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/FeignAcceptGzipEncodingAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2023 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. @@ -22,17 +22,18 @@ import feign.Feign; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; -import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.openfeign.FeignAutoConfiguration; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; /** * Configures the Feign response compression. * * @author Jakub Narloch + * @author Olga Maciaszek-Sharma * @see FeignAcceptGzipEncodingInterceptor */ @Configuration(proxyBeanMethods = false) @@ -41,8 +42,8 @@ import org.springframework.context.annotation.Configuration; @ConditionalOnBean(Client.class) @ConditionalOnProperty(value = "feign.compression.response.enabled", matchIfMissing = false) // The OK HTTP client uses "transparent" compression. -// If the accept-encoding header is present it disable transparent compression -@ConditionalOnMissingBean(type = "okhttp3.OkHttpClient") +// If the accept-encoding header is present, it disables transparent compression. +@Conditional(OkHttpFeignClientBeanMissingCondition.class) @AutoConfigureAfter(FeignAutoConfiguration.class) public class FeignAcceptGzipEncodingAutoConfiguration { diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/FeignContentGzipEncodingAutoConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/FeignContentGzipEncodingAutoConfiguration.java index 2c7ddf33..a802f65e 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/FeignContentGzipEncodingAutoConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/FeignContentGzipEncodingAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2023 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. @@ -20,25 +20,26 @@ import feign.Feign; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; -import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.openfeign.FeignAutoConfiguration; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; /** * Configures the Feign request compression. * * @author Jakub Narloch + * @author Olga Maciaszek-Sharma * @see FeignContentGzipEncodingInterceptor */ @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties(FeignClientEncodingProperties.class) @ConditionalOnClass(Feign.class) // The OK HTTP client uses "transparent" compression. -// If the content-encoding header is present it disable transparent compression -@ConditionalOnMissingBean(type = "okhttp3.OkHttpClient") +// If the content-encoding header is present, it disables transparent compression. +@Conditional(OkHttpFeignClientBeanMissingCondition.class) @ConditionalOnProperty("feign.compression.request.enabled") @AutoConfigureAfter(FeignAutoConfiguration.class) public class FeignContentGzipEncodingAutoConfiguration { diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/OkHttpFeignClientBeanMissingCondition.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/OkHttpFeignClientBeanMissingCondition.java new file mode 100644 index 00000000..cb891278 --- /dev/null +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/encoding/OkHttpFeignClientBeanMissingCondition.java @@ -0,0 +1,51 @@ +/* + * Copyright 2023-2023 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.openfeign.encoding; + +import feign.Client; +import feign.okhttp.OkHttpClient; + +import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Condition; + +/** + * A {@link Condition} that verifies whether the conditions for creating Feign + * {@link Client} beans that either are of type {@link OkHttpClient} or have a delegate of + * type {@link OkHttpClient} are not met. + * + * @author Olga Maciaszek-Sharma + * @since 4.0.2 + */ +public class OkHttpFeignClientBeanMissingCondition extends AnyNestedCondition { + + public OkHttpFeignClientBeanMissingCondition() { + super(ConfigurationPhase.REGISTER_BEAN); + } + + @ConditionalOnMissingClass("feign.okhttp.OkHttpClient") + static class FeignOkHttpClientPresent { + + } + + @ConditionalOnProperty(value = "feign.okhttp.enabled", havingValue = "false") + static class FeignOkHttpClientEnabled { + + } + +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignCompressionTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignCompressionTests.java index 6f862629..2e5d5aff 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignCompressionTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignCompressionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2023 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. @@ -21,6 +21,7 @@ import java.util.Map; import feign.Client; import feign.RequestInterceptor; import feign.httpclient.ApacheHttpClient; +import okhttp3.OkHttpClient; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -39,18 +40,19 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Ryan Baxter * @author Biju Kunjummen + * @author Olga Maciaszek-Sharma */ class FeignCompressionTests { @Test - void testInterceptors() { + void shouldAddCompressionInterceptors() { new ApplicationContextRunner() .withPropertyValues("feign.compression.response.enabled=true", "feign.compression.request.enabled=true", "feign.okhttp.enabled=false") .withConfiguration(AutoConfigurations.of(FeignAutoConfiguration.class, FeignContentGzipEncodingAutoConfiguration.class, FeignAcceptGzipEncodingAutoConfiguration.class, HttpClientConfiguration.class, PlainConfig.class)) - .run(context -> { + .withUserConfiguration(OkHttpClientConfiguration.class).run(context -> { FeignContext feignContext = context.getBean(FeignContext.class); Map interceptors = feignContext.getInstances("foo", RequestInterceptor.class); @@ -62,6 +64,22 @@ class FeignCompressionTests { }); } + @Test + void shouldNotAddInterceptorsIfFeignOkHttpClientPresent() { + new ApplicationContextRunner() + .withPropertyValues("feign.compression.response.enabled=true", "feign.compression.request.enabled=true", + "feign.okhttp.enabled=true") + .withConfiguration(AutoConfigurations.of(FeignAutoConfiguration.class, + FeignContentGzipEncodingAutoConfiguration.class, FeignAcceptGzipEncodingAutoConfiguration.class, + HttpClientConfiguration.class)) + .run(context -> { + FeignContext feignContext = context.getBean(FeignContext.class); + Map interceptors = feignContext.getInstances("foo", + RequestInterceptor.class); + assertThat(interceptors).isEmpty(); + }); + } + @Configuration(proxyBeanMethods = false) protected static class PlainConfig { @@ -85,4 +103,14 @@ class FeignCompressionTests { } + @Configuration(proxyBeanMethods = false) + static class OkHttpClientConfiguration { + + @Bean + OkHttpClient okHttpClient() { + return new OkHttpClient(); + } + + } + }