Unzip/Deflate content on error status for Default Client

[gdufrene]

Summary by CodeRabbit

• Refactor: Simplified the handling of gzip and deflate encoded responses in the core application. This change enhances the software's reliability by ensuring consistent handling of encoded responses.
• Test: Added new tests to verify the correct handling of gzip and deflate encoded error responses. These tests improve the robustness of the software by ensuring that it behaves as expected even in error scenarios.
• Test: Updated various client tests to include checks for gzip and deflate encoding error handling. This ensures that the software's performance remains consistent across different client implementations.

[coderabbitai]

Walkthrough

The changes primarily focus on improving the handling of gzip and deflate encoded responses in the convertResponse function. The code now always creates an input stream and then checks for encoding. Additionally, new test methods have been added to verify the correct handling of zipped responses with error status codes.

Changes

File Path	Summary
core/src/main/java/feign/Client.java	Simplified the convertResponse function to always create an input stream and then check for gzip or deflate encoding.
core/src/test/java/feign/client/...	Added new test methods canSupportGzipOnError and canSupportDeflateOnError to verify correct handling of zipped error responses.
googlehttpclient/src/test/java/feign/googlehttpclient/... jaxrs2/src/test/java/feign/jaxrs2/... okhttp/src/test/java/feign/okhttp/...	Added override methods canSupportGzipOnError and canSupportDeflateOnError with similar logic to existing methods.

🐇💻

In the land of code, where the brackets lie,

Changes were made, under the GitHub sky.

Gzip and deflate, no longer a fuss,

With new tests in place, to catch any mistrust.

So here's to the coder, with their keyboard's click-clatter,

Making the digital world, just a little bit better. 🎉

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• Mention @coderabbitai in any review comment for bot assistance.
• Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

• Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
• Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
  resume reviews, enter @coderabbitai resume as any issue or review comment.

[coderabbitai]

Review Status

Actionable comments generated: 3

Commits

Files that changed from the base of the PR and between b5e6809 and 4eafa31.

Files selected for processing (5)

• core/src/main/java/feign/Client.java (1 hunks)
• core/src/test/java/feign/client/AbstractClientTest.java (4 hunks)
• googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java (1 hunks)
• jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java (1 hunks)
• okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java (1 hunks)

Files skipped from review due to trivial changes (2)

• core/src/main/java/feign/Client.java
• googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java

Additional comments (Suppressed): 5

core/src/test/java/feign/client/AbstractClientTest.java (4)

• 20-21: The import statement for org.junit.Assert.fail has been added. Ensure that this new import does not conflict with any existing methods in the class.

• 35-35: The import statement for java.nio.ByteBuffer has been added. Make sure this new import is used within the code and it doesn't conflict with any existing methods in the class.

• 415-439: A new test method canSupportGzipOnError() has been added. This test checks if the client can handle gzip compressed error responses correctly. The test enqueues a mock response with a 400 status code and gzip encoding, then asserts that the response body is correctly unzipped.

• 459-482: A new test method canSupportDeflateOnError() has been added. This test checks if the client can handle deflate compressed error responses correctly. The test enqueues a mock response with a 400 status code and deflate encoding, then asserts that the response body is correctly unzipped.

jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java (1)

• 167-168: The new test methods canSupportGzipOnError and canSupportDeflateOnError have been added to verify the handling of gzip and deflate encoded error responses. However, these tests are set to always fail with assumeFalse(), indicating that the JaxRS client does not support gzip or deflate compression. If this is the expected behavior, then the changes are fine. Otherwise, the logic in these tests may need to be revised.

[gdufrene]

This MR is about processing gzip or deflate content when error status is >= 400 on default http client.
This is related to issue #1386

Hope it would help solving this issue from 2021 :)

[velo]

Hope it would help solving this issue from 2021 :)

Plenty more from well dates waiting for community fixes.

[coderabbitai]

Review Status

Actionable comments generated: 0

Commits

Files that changed from the base of the PR and between 4eafa31 and 40773b8.

Files selected for processing (1)

• okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java (1 hunks)

Files skipped from review due to trivial changes (1)

• okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java

[gdufrene]

Plenty more from well dates waiting for community fixes.

And this is probably not the most awaited one.
But still, first contribution on that project, have to start small :)

Hope I will help for others in the future.

[coderabbitai]

Review Status

Actionable comments generated: 4

Commits

Files that changed from the base of the PR and between 25c419d and 8a8c490.

Files selected for processing (5)

• core/src/main/java/feign/Client.java (1 hunks)
• core/src/test/java/feign/client/AbstractClientTest.java (4 hunks)
• googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java (1 hunks)
• jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java (1 hunks)
• okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java (1 hunks)

Files skipped from review due to trivial changes (1)

• googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java

Additional comments (Suppressed): 7

core/src/test/java/feign/client/AbstractClientTest.java (4)

• 20-20: The import statement for fail from org.junit.Assert has been added. Ensure that this import is necessary and used in the updated code.

• 35-35: The import statement for ByteBuffer from java.nio has been added. Ensure that this import is necessary and used in the updated code.

• 415-439: The new test method canSupportGzipOnError has been added. This method tests the behavior when receiving a gzip-encoded error response. The test checks that the response is correctly unzipped and matches the expected data. The test setup and assertions look correct.

• 459-482: The new test method canSupportDeflateOnError has been added. This method tests the behavior when receiving a deflate-encoded error response. The test checks that the response is correctly unzipped and matches the expected data. The test setup and assertions look correct.

jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java (3)

• 152-158: The new methods canSupportGzipOnError and canSupportDeflateOnError have been added to test the behavior when receiving a zipped response with a 400 status code. However, they are currently set to always assume false, indicating that the JAX-RS client does not support gzip or deflate compression. If this is the intended behavior, then the changes are fine. If not, the logic within these methods should be updated to accurately reflect the capabilities of the JAX-RS client.

• 165-171: The method canSupportDeflateOnError has been added to test the behavior when receiving a deflated response with a 400 status code. However, it is currently set to always assume false, indicating that the JAX-RS client does not support deflate compression. If this is the intended behavior, then the changes are fine. If not, the logic within this method should be updated to accurately reflect the capabilities of the JAX-RS client.

• 172-172: The method canExceptCaseInsensitiveHeader is still checking for gzip compression support, which doesn't align with its name. This was previously flagged in the comment chain and the author suggested addressing it in a separate PR. However, it's important to ensure that our tests accurately reflect what they're supposed to be testing. In this case, the canExceptCaseInsensitiveHeader method is currently checking for gzip compression support, which doesn't align with its name or intended purpose. I recommend correcting it now to prevent potential confusion or errors in future testing.

   @Override
   public void canExceptCaseInsensitiveHeader() throws Exception {
-    assumeFalse("JaxRS client do not support gzip compression", false);
+    // Corrected test logic should go here
+    // For example:
+    assumeTrue("JaxRS client should support case-insensitive headers", true);
   }

[gdufrene]

rebased this last changes on master branch.
Let me know if anything else needs to be done to allow merging this fix.