Use reflection for Commons Compress codecs to avoid hard dep. #737
Conversation
|
@arturobernalg Why is |
|
@arturobernalg Please sort out the problem with multiple "brotli" dependencies. |
|
@arturobernalg OK. As a first step please do the following. Make |
@ok2c I just Replaced Commons Compress with brotli4j for both encode and decode in the async round-trip example. Please let me now if is what you was thinking |
|
@arturobernalg This is much, much better in my opinion .I just do not like that two distinct changes got mixed up in a single change-set. Would it be a big deal for you to spin Brotli related changes into a separate pull request? Some javadocs ( |
arturobernalg
force-pushed
the
cc-reflective-codecs
branch
2 times, most recently
from
a638555 to
15290d1
Compare
@fixed. I'm separating the PR in two. Another one is coming with the only reflection on Brotli |
ok2c
left a comment
ok2c
left a comment
There was a problem hiding this comment.
@arturobernalg Looks good otherwise.
| m.put(ContentCoding.GZIP, | ||
| new Codec( | ||
| // encoder | ||
| org.apache.hc.client5.http.entity.GzipCompressingEntity::new, |
There was a problem hiding this comment.
@arturobernalg There is no need to use a fully qualified class name. It can be imported
| ent -> new DecompressingEntity(ent, GZIPInputStream::new))); | ||
| m.put(ContentCoding.DEFLATE, | ||
| new Codec( | ||
| org.apache.hc.client5.http.entity.DeflateCompressingEntity::new, |
Prevent EIIE when commons-compress is absent; wrap failures as IOException. Fix runtimeAvailable() to probe CC classes; decouple from helper JARs.
arturobernalg
force-pushed
the
cc-reflective-codecs
branch
from
ab58854 to
8e05f72
Compare
| <dependency> | ||
| <groupId>org.brotli</groupId> | ||
| <artifactId>dec</artifactId> | ||
| <optional>true</optional> | ||
| </dependency> |
There was a problem hiding this comment.
I'm confused. Why has this dependency declaration been removed? There are still (reflective) usages of these classes in the library:
httpclient5/src/main/java/org/apache/hc/client5/http/entity/BrotliDecompressingEntity.java
52: Class.forName("org.brotli.dec.BrotliInputStream");
httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressCodecFactory.java
76: private static final String H_BROTLI = "org.brotli.dec.BrotliInputStream";
There was a problem hiding this comment.
@rschmitt I asked @arturobernalg to move brotli related changes to a separate change-set. This must have contributed to the confusion.
There was a problem hiding this comment.
@arturobernalg @rschmitt I would like to revert some of reflective calls in CommonsCompressCodecFactory. They are unnecessary if ContentCodecRegistry works correctly and does not call any of CommonsCompressCodecFactory methods at runtime if Commons Compress is not present.
Code optimization and javadoc update
Code optimization and javadoc update
3 participants
Refactor codec integration to load Commons Compress via reflection. This removes compile-time references, respects the optional dependency, and eliminates
ExceptionInInitializerErrorwhen the library is missing.runtimeAvailable()now probes Commons Compress algorithm classes