Replace the old InputStreamFactory registries with the new Decoder / Encoder pipeline #660

arturobernalg · 2025-06-25T13:11:28Z

This change swaps out the legacy compression infrastructure for the new decoder / encoder model introduced in 5.6. The codebase now routes all (de)compression through ContentCodecRegistry, uses functional interfaces instead of InputStreamFactory, and updates tests and docs accordingly while preserving existing public classes as deprecated shims. Everything builds and the full test suite passes.

garydgregory · 2025-06-25T17:45:20Z

httpclient5/src/main/java/org/apache/hc/client5/http/entity/BrotliDecompressingEntity.java

@@ -32,9 +32,12 @@
 * {@link org.apache.hc.core5.http.io.entity.HttpEntityWrapper} responsible for
 * handling br Content Coded responses.
 *
+ * @deprecated Use Decoder API


I think it best to use '@link' in '@deprecated' comments. It makes it much easier to follow the bouncing ball 😉

garydgregory · 2025-06-25T17:46:31Z

httpclient5/src/main/java/org/apache/hc/client5/http/entity/InputStreamFactory.java

@@ -31,9 +31,10 @@

 /**
 * Factory for decorated {@link InputStream}s.
- *
+ * @deprecated


The comment with an '@link' is missing, which happens in many places in the PR.

garydgregory · 2025-06-25T17:53:03Z

httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/Encoder.java

+ * @since 5.6
+ */
+@FunctionalInterface
+public interface Encoder {


We could reuse or extend UnaryOperator instead of inventing our own.

Prefer a dedicated Encoder over UnaryOperator – the name conveys intent and leaves room for HTTP-specific defaults later. Keeps symmetry with Decoder and avoids leaking java.util.function into the public API.

garydgregory · 2025-06-25T17:58:45Z

httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java

+    /**
+     * Custom decoders keyed by {@link ContentCoding}.
+     *
+     * @since 5.6


We don't need '@SInCE' tags for private code.

arturobernalg · 2025-06-26T09:10:59Z

Added a terse Javadoc note that points users to the new Decoder/Encoder API.

ok2c · 2025-06-28T10:09:51Z

httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/Decoder.java

+ * @since 5.6
+ */
+@FunctionalInterface
+public interface Decoder {


@arturobernalg Should not Decoder be working the same was as Encoder and unwrap HttpEntity?

@ok2c Decoder only swaps the input stream—DecompressingEntity already wraps the HttpEntity—so returning an InputStream keeps the API minimal and avoids duplicating header-handling logic.

@arturobernalg to me it is more important for the API to be logically consistent than saving a few lines of header handling code that will likely get optimized out by the compiler. In my opinion The Encoder / Decoder interfaces should work either with InputStream / OutputStream or with HttpEntity, but not with a mix. Feel free to leave it as is and merge the change-set but I will likely try to normalize the APIs later.

@ok2c please do another pass.

ok2c · 2025-06-29T09:12:49Z

httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/ContentCodecRegistry.java

+    /**
+     * Returns the {@link Codec} (or {@code null}).
+     */
+    public static Codec codec(final ContentCoding coding) {


@arturobernalg Here we have a scope violation: a public method exposing a package private class. Either make the method package private or make the class public. It is internal anyway.

ok2c · 2025-06-29T09:18:19Z

httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/Decoder.java

+ * @since 5.6
+ */
+@FunctionalInterface
+public interface Decoder {


@arturobernalg to me it is more important for the API to be logically consistent than saving a few lines of header handling code that will likely get optimized out by the compiler. In my opinion The Encoder / Decoder interfaces should work either with InputStream / OutputStream or with HttpEntity, but not with a mix. Feel free to leave it as is and merge the change-set but I will likely try to normalize the APIs later.

ok2c · 2025-07-04T15:47:56Z

httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/Decoder.java

+     * @throws IOException if the decoding entity cannot be created or an
+     *                     underlying I/O error occurs
+     */
+    HttpEntity wrap(HttpEntity src) throws IOException;


@arturobernalg I think this method does not need to throw IOException. At least it does not appear to be thrown anywhere in the existing code.

I also agree with @garydgregory here. I think we could just use standard Function here, but this is a matter of taste so I will not object.

Looks good otherwise.

HI @ok2c please do another pass. Now I'm using java functions instead.

HI @ok2c please do another pass. Now I'm using java functions instead.

@arturobernalg Oh, man. I meant something different. I think IOFunction was fine. It makes sense to have a function that throws IOException. What I meant the Decoder and Encoder could be just UnaryOperator or Function as suggested by @garydgregory

ok2c

@arturobernalg I would prefer to have IOFunction restored and Encoder and Decoder replaced but this is more of a matter of taste. You can keep them if you strongly prefer it

…Encoder pipeline. Adds ContentCodecRegistry, rewrites DecompressingEntity, updates HttpClient internals and tests, deprecates legacy helpers, and keeps backward compatibility.

…ic Encoder/Decoder API, introduce IOFunction and ContentCodecRegistry, update DecompressingEntity and all tests accordingly.

arturobernalg · 2025-07-07T11:37:08Z

@arturobernalg I would prefer to have IOFunction restored and Encoder and Decoder replaced but this is more of a matter of taste. You can keep them if you strongly prefer it

@ok2c i just

@arturobernalg I would prefer to have IOFunction restored and Encoder and Decoder replaced but this is more of a matter of taste. You can keep them if you strongly prefer it

HI @ok2c i just implement the UnaryOperator instead. please do another pass.

arturobernalg requested a review from ok2c June 25, 2025 13:11

garydgregory reviewed Jun 25, 2025

View reviewed changes

arturobernalg force-pushed the content-codec-registry branch 2 times, most recently from 3915567 to d0ed410 Compare June 26, 2025 09:10

arturobernalg requested a review from garydgregory June 27, 2025 12:09

ok2c reviewed Jun 28, 2025

View reviewed changes

arturobernalg requested a review from ok2c June 28, 2025 16:50

ok2c reviewed Jun 29, 2025

View reviewed changes

ok2c reviewed Jul 4, 2025

View reviewed changes

arturobernalg requested a review from ok2c July 5, 2025 12:24

arturobernalg force-pushed the content-codec-registry branch from c591d1e to 24ac6ca Compare July 5, 2025 12:59

ok2c approved these changes Jul 5, 2025

View reviewed changes

arturobernalg force-pushed the content-codec-registry branch from d3b855c to 8c31bf1 Compare July 7, 2025 11:30

arturobernalg added 3 commits July 7, 2025 13:32

Replace the old InputStreamFactory registries with the new Decoder / …

8e7aed4

…Encoder pipeline. Adds ContentCodecRegistry, rewrites DecompressingEntity, updates HttpClient internals and tests, deprecates legacy helpers, and keeps backward compatibility.

Unify content-coding support: replace InputStreamFactory with symmetr…

69875b6

…ic Encoder/Decoder API, introduce IOFunction and ContentCodecRegistry, update DecompressingEntity and all tests accordingly.

Use UnaryOperator instead

474c669

arturobernalg force-pushed the content-codec-registry branch from 8c31bf1 to 474c669 Compare July 7, 2025 11:32

arturobernalg requested a review from ok2c July 7, 2025 11:37

ok2c approved these changes Jul 8, 2025

View reviewed changes

Replace the old InputStreamFactory registries with the new Decoder / Encoder pipeline #660

Are you sure you want to change the base?

Replace the old InputStreamFactory registries with the new Decoder / Encoder pipeline #660

Uh oh!

Conversation

arturobernalg commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

garydgregory Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arturobernalg commented Jun 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ok2c left a comment

Choose a reason for hiding this comment

Uh oh!

arturobernalg commented Jul 7, 2025

Uh oh!

Uh oh!

garydgregory Jun 25, 2025 •

edited

Loading