garydgregory commented on code in PR #651: URL: https://github.com/apache/httpcomponents-client/pull/651#discussion_r2155229212
########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressDecoderFactory.java: ########## @@ -0,0 +1,122 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Locale; + +import org.apache.commons.compress.compressors.CompressorException; +import org.apache.commons.compress.compressors.CompressorStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * {@link InputStreamFactory} backed by + * <a href="https://commons.apache.org/proper/commons-compress/";>Apache Commons Compress</a>. + * <p> + * The class is compiled against Commons Compress but lives behind an <i>optional</i> + * dependency. At run-time it will only be loaded when the library is present, + * therefore callers may rely on it without pulling Commons Compress into every + * downstream build. + * </p> + * + * <h4>Run-time guards</h4> + * Some encodings (e.g. {@code br}, {@code zstd}, {@code xz/lzma}) + * depend on native helper JARs. {@link #runtimeAvailable(String)} performs a + * lightweight {@code Class.forName} probe so we register such codecs only when + * the helper is on the class-path. + * + * @since 5.6 + */ +@Internal +@Contract(threading = ThreadingBehavior.STATELESS) +final class CommonsCompressDecoderFactory implements InputStreamFactory { + + private final String encoding; + + CommonsCompressDecoderFactory(final String encoding) { + this.encoding = encoding.toLowerCase(Locale.ROOT); + } + + @Override + public String getContentEncoding() { + return encoding; + } + + @Override + public InputStream create(final InputStream source) throws IOException { + try { + return new CompressorStreamFactory() + .createCompressorInputStream(encoding, source); + } catch (final CompressorException | LinkageError ex) { + throw new IOException( + "Unable to decode Content-Encoding '" + encoding + '\'', ex); + } + } + + private enum Probe { + BR(ContentCoding.BROTLI.token(), "org.brotli.dec.BrotliInputStream"), + ZSTD(ContentCoding.ZSTD.token(), "com.github.luben.zstd.ZstdInputStream"), + XZ(ContentCoding.XZ.token(), "org.tukaani.xz.XZInputStream"), + LZMA(ContentCoding.LZMA.token(), "org.tukaani.xz.XZInputStream"); + + final String enc, probeClass; + + Probe(final String enc, final String probeClass) { Review Comment: `probeClass` is misleading to me because it's a _name_, not an actual class. I think "requiredTypeName" or "requiredClassName" would be a clearer name because "probe" doesn't really tell you anything (to me). ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressDecoderFactory.java: ########## @@ -0,0 +1,122 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Locale; + +import org.apache.commons.compress.compressors.CompressorException; +import org.apache.commons.compress.compressors.CompressorStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * {@link InputStreamFactory} backed by + * <a href="https://commons.apache.org/proper/commons-compress/";>Apache Commons Compress</a>. + * <p> + * The class is compiled against Commons Compress but lives behind an <i>optional</i> + * dependency. At run-time it will only be loaded when the library is present, + * therefore callers may rely on it without pulling Commons Compress into every + * downstream build. + * </p> + * + * <h4>Run-time guards</h4> + * Some encodings (e.g. {@code br}, {@code zstd}, {@code xz/lzma}) + * depend on native helper JARs. {@link #runtimeAvailable(String)} performs a + * lightweight {@code Class.forName} probe so we register such codecs only when + * the helper is on the class-path. + * + * @since 5.6 + */ +@Internal +@Contract(threading = ThreadingBehavior.STATELESS) +final class CommonsCompressDecoderFactory implements InputStreamFactory { + + private final String encoding; + + CommonsCompressDecoderFactory(final String encoding) { + this.encoding = encoding.toLowerCase(Locale.ROOT); + } + + @Override + public String getContentEncoding() { + return encoding; + } + + @Override + public InputStream create(final InputStream source) throws IOException { + try { + return new CompressorStreamFactory() + .createCompressorInputStream(encoding, source); + } catch (final CompressorException | LinkageError ex) { + throw new IOException( + "Unable to decode Content-Encoding '" + encoding + '\'', ex); + } + } + + private enum Probe { Review Comment: This feels a bit redundant with the new `ContentCoding` enum. What do you think about moving the "requires this type name" aspect (a.k.a. "probe class") of `Probe` to `ContentCoding` and turning this "Probe" into a `Set<ContentCoding>`? ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressDecoderFactory.java: ########## @@ -0,0 +1,122 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Locale; + +import org.apache.commons.compress.compressors.CompressorException; +import org.apache.commons.compress.compressors.CompressorStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * {@link InputStreamFactory} backed by + * <a href="https://commons.apache.org/proper/commons-compress/";>Apache Commons Compress</a>. Review Comment: If there's a non-breaking space between words 1 and 2, why not between words 2 and 3? I would just not bother since it might cause odd rendering on small screens like phones and tablets. ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressDecoderFactory.java: ########## @@ -0,0 +1,122 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Locale; + +import org.apache.commons.compress.compressors.CompressorException; +import org.apache.commons.compress.compressors.CompressorStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * {@link InputStreamFactory} backed by + * <a href="https://commons.apache.org/proper/commons-compress/";>Apache Commons Compress</a>. + * <p> + * The class is compiled against Commons Compress but lives behind an <i>optional</i> + * dependency. At run-time it will only be loaded when the library is present, + * therefore callers may rely on it without pulling Commons Compress into every + * downstream build. + * </p> + * + * <h4>Run-time guards</h4> + * Some encodings (e.g. {@code br}, {@code zstd}, {@code xz/lzma}) Review Comment: In general, I stay away from Latin abbreviations, I find it doesn't make the text clearer to read. No need for non-breaking space IMO. ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/ContentDecoderRegistry.java: ########## @@ -0,0 +1,147 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; + +import org.apache.hc.client5.http.entity.BrotliDecompressingEntity; +import org.apache.hc.client5.http.entity.BrotliInputStreamFactory; +import org.apache.hc.client5.http.entity.DeflateInputStreamFactory; +import org.apache.hc.client5.http.entity.GZIPInputStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * Run-time catalogue of all {@link InputStreamFactory} instances that + * implement HTTP <em>content-decoding</em>. + * + * <p>The catalogue is populated once at class-initialisation: + * <ul> + * <li><b>Built-ins</b> – {@code gzip} and {@code deflate} – are registered + * unconditionally via the long-standing singletons + * {@link GZIPInputStreamFactory#getInstance() GZIPInputStreamFactory} + * and {@link DeflateInputStreamFactory#getInstance() + * DeflateInputStreamFactory}.</li> + * <li>When <em>commons-compress</em> is present, the registry attempts to + * add additional codecs (br, zstd, xz, …) using + * {@link CommonsCompressDecoderFactory}. Each codec is included only if + * its own helper JAR (e.g. google-brotli, zstd-jni) is available.</li> + * <li>If Commons did <em>not</em> contribute a <code>br</code> decoder the + * fallback singleton {@link BrotliInputStreamFactory#getInstance()} + * is used when the native Brotli library is on the class-path.</li> + * </ul> + * + * <p>The resulting map is immutable and exposed via + * {@link #snapshot()} for safe concurrent use.</p> + * + * @since 5.6 + */ +@Internal +@Contract(threading = ThreadingBehavior.STATELESS) +public final class ContentDecoderRegistry { + + /** + * Canonical, immutable map built at class-initialisation. + */ + private static final Map<String, InputStreamFactory> REGISTRY = build(); + + + private static Map<String, InputStreamFactory> build() { + final LinkedHashMap<String, InputStreamFactory> m = new LinkedHashMap<>(); + + /* 1. Built-ins (always succeed) */ + register(m, new GZIPInputStreamFactory()); + register(m, new DeflateInputStreamFactory()); + + /* 2. Optional Commons-Compress decoders (reflection, safe) */ + if (commonsCompressPresent()) { + addCommons(m, "br"); + addCommons(m, "zstd"); + addCommons(m, "xz"); + addCommons(m, "lzma"); + addCommons(m, "lz4-framed"); + addCommons(m, "lz4-block"); + addCommons(m, "bzip2"); + addCommons(m, "pack200"); + addCommons(m, "deflate64"); + } + + /* 3. Native Brotli fallback when Commons is not present */ + if (!m.containsKey("br")) { + if (BrotliDecompressingEntity.isAvailable()) { + register(m, new BrotliInputStreamFactory()); + } + } + return m; + } + + + private static void register(final Map<String, InputStreamFactory> map, + final InputStreamFactory f) { + map.put(f.getContentEncoding().toLowerCase(Locale.ROOT), f); + } + + private static void addCommons(final Map<String, InputStreamFactory> map, + final String encoding) { + final InputStreamFactory f = CommonsCompressDecoderFactory.tryCreate(encoding); + if (f != null) { + map.put(encoding.toLowerCase(Locale.ROOT), f); + } + } + + /** + * Quick presence check: is the Commons factory class on the class-path? + */ + private static boolean commonsCompressPresent() { + try { + Class.forName( + "org.apache.commons.compress.compressors.CompressorStreamFactory", + false, + ContentDecoderRegistry.class.getClassLoader()); + return true; + } catch (final ClassNotFoundException e) { Review Comment: Ping? ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressDecoderFactory.java: ########## @@ -0,0 +1,122 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Locale; + +import org.apache.commons.compress.compressors.CompressorException; +import org.apache.commons.compress.compressors.CompressorStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * {@link InputStreamFactory} backed by + * <a href="https://commons.apache.org/proper/commons-compress/";>Apache Commons Compress</a>. + * <p> + * The class is compiled against Commons Compress but lives behind an <i>optional</i> Review Comment: I would not use `<i>` in Javadocs, that's up to the rederer to decide what style to apply, which is why Javadoc supports `<em>` for ephasis which is usually translated to italics and `<strong>` which is usually translated to bold. ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressDecoderFactory.java: ########## @@ -0,0 +1,122 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Locale; + +import org.apache.commons.compress.compressors.CompressorException; +import org.apache.commons.compress.compressors.CompressorStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * {@link InputStreamFactory} backed by + * <a href="https://commons.apache.org/proper/commons-compress/";>Apache Commons Compress</a>. + * <p> + * The class is compiled against Commons Compress but lives behind an <i>optional</i> + * dependency. At run-time it will only be loaded when the library is present, Review Comment: You don't need two spaces after a `.`; we're not using typewriters anymore and Javadoc is not rendered in mono-space ;-) ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressDecoderFactory.java: ########## @@ -0,0 +1,122 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Locale; + +import org.apache.commons.compress.compressors.CompressorException; +import org.apache.commons.compress.compressors.CompressorStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * {@link InputStreamFactory} backed by + * <a href="https://commons.apache.org/proper/commons-compress/";>Apache Commons Compress</a>. + * <p> + * The class is compiled against Commons Compress but lives behind an <i>optional</i> + * dependency. At run-time it will only be loaded when the library is present, + * therefore callers may rely on it without pulling Commons Compress into every + * downstream build. + * </p> + * + * <h4>Run-time guards</h4> + * Some encodings (e.g. {@code br}, {@code zstd}, {@code xz/lzma}) + * depend on native helper JARs. {@link #runtimeAvailable(String)} performs a + * lightweight {@code Class.forName} probe so we register such codecs only when + * the helper is on the class-path. + * + * @since 5.6 + */ +@Internal +@Contract(threading = ThreadingBehavior.STATELESS) +final class CommonsCompressDecoderFactory implements InputStreamFactory { + + private final String encoding; + + CommonsCompressDecoderFactory(final String encoding) { + this.encoding = encoding.toLowerCase(Locale.ROOT); + } + + @Override + public String getContentEncoding() { + return encoding; + } + + @Override + public InputStream create(final InputStream source) throws IOException { + try { + return new CompressorStreamFactory() + .createCompressorInputStream(encoding, source); + } catch (final CompressorException | LinkageError ex) { + throw new IOException( + "Unable to decode Content-Encoding '" + encoding + '\'', ex); + } + } + + private enum Probe { + BR(ContentCoding.BROTLI.token(), "org.brotli.dec.BrotliInputStream"), + ZSTD(ContentCoding.ZSTD.token(), "com.github.luben.zstd.ZstdInputStream"), + XZ(ContentCoding.XZ.token(), "org.tukaani.xz.XZInputStream"), + LZMA(ContentCoding.LZMA.token(), "org.tukaani.xz.XZInputStream"); + + final String enc, probeClass; + + Probe(final String enc, final String probeClass) { + this.enc = enc; + this.probeClass = probeClass; + } + + static String helperFor(final String enc) { Review Comment: I would stay away from cryptic names like "enc", even in private code, if it's an "encoding", the call it that ;-) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org
