Fix condition. (#834)

Author: OlgaMaciaszek

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.

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 @@ private Supplier<Object> asSupplier(final Method method, final Object[] args) {
 	}
 
 	/**
-	 * 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<Method, Method> toFallbackMethod(Map<Method, InvocationHandlerFactory.MethodHandler> dispatch) {

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 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 @@
 @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 {
 

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 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 {

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 {
+
+	}
+
+}

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 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 @@
 /**
  * @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<String, RequestInterceptor> interceptors = feignContext.getInstances("foo",
 							RequestInterceptor.class);
@@ -62,6 +64,22 @@ void testInterceptors() {
 				});
 	}
 
+	@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<String, RequestInterceptor> interceptors = feignContext.getInstances("foo",
+							RequestInterceptor.class);
+					assertThat(interceptors).isEmpty();
+				});
+	}
+
 	@Configuration(proxyBeanMethods = false)
 	protected static class PlainConfig {
 
@@ -85,4 +103,14 @@ public ApacheHttpClient client() {
 
 	}
 
+	@Configuration(proxyBeanMethods = false)
+	static class OkHttpClientConfiguration {
+
+		@Bean
+		OkHttpClient okHttpClient() {
+			return new OkHttpClient();
+		}
+
+	}
+
 }