garydgregory commented on code in PR #634:
URL: https://github.com/apache/commons-compress/pull/634#discussion_r1939506624
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipMethod.java:
##########
@@ -124,6 +124,24 @@ public enum ZipMethod {
*/
LZMA(14),
+ /**
+ * Compression Method 20 for Zstd.
+ *
+ * @see <a
href="https://github.com/facebook/zstd";>https://github.com/facebook/zstd</a>
+ * @see <a
href="https://pkwaredownloads.blob.core.windows.net/pkware-general/Documentation/APPNOTE-6.3.7.TXT";>deprecated
zstd compression method id</a>
+ * @see <a
href="https://www.pkware.com/documents/casestudies/APPNOTE.TXT";>Explanation of
fields: compression method: (2 bytes)</a>
+ */
Review Comment:
All new public and protected elements should carry a Javadoc `@since` tag,
in this case 1.28.0. Done in git master.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -293,6 +295,8 @@ public static boolean matches(final byte[] signature, final
int length) {
private int entriesRead;
+ private ZstdCompressorInputStream zstdInputStream;
Review Comment:
This is not needed, removed in git master.
##########
src/main/java/org/apache/commons/compress/compressors/zstandard/ZstdUtils.java:
##########
@@ -134,4 +157,21 @@ public static void setCacheZstdAvailablity(final boolean
doCache) {
/** Private constructor to prevent instantiation of this utility class. */
private ZstdUtils() {
}
+
+ /**
+ * Reads uncompressed data stream and writes it compressed to the output
+ *
+ * @param input the data stream with uncompressed data
+ * @param output the data stream for compressed output
+ * @return the compressed size
+ * @throws IOException throws the exception which could be got from from
IOUtils.copyLarge()
+ * or ZstdCompressorOutputStream constructor
+ */
+ public static long readAndCompressWrite(InputStream input, OutputStream
output) throws IOException {
Review Comment:
The return value is never used, the method returns void in git master.
In addition, the method is only used from the new test and therefore moved
to the test.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -444,6 +448,9 @@ public void close() throws IOException {
closed = true;
try {
in.close();
+ if (zstdInputStream != null) {
Review Comment:
This is not needed, removed in git master.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipMethod.java:
##########
@@ -124,6 +124,24 @@ public enum ZipMethod {
*/
LZMA(14),
+ /**
+ * Compression Method 20 for Zstd.
+ *
+ * @see <a
href="https://github.com/facebook/zstd";>https://github.com/facebook/zstd</a>
+ * @see <a
href="https://pkwaredownloads.blob.core.windows.net/pkware-general/Documentation/APPNOTE-6.3.7.TXT";>deprecated
zstd compression method id</a>
+ * @see <a
href="https://www.pkware.com/documents/casestudies/APPNOTE.TXT";>Explanation of
fields: compression method: (2 bytes)</a>
+ */
+ ZSTD_DEPRECATED(20),
+
+ /**
+ * Compression Method 93 for Zstd.
+ *
+ * @see <a
href="https://github.com/facebook/zstd";>https://github.com/facebook/zstd</a>
+ * @see <a
href="https://pkwaredownloads.blob.core.windows.net/pkware-general/Documentation/APPNOTE-6.3.8.TXT";>changed
zstd compression method id</a>
+ * @see <a
href="https://www.pkware.com/documents/casestudies/APPNOTE.TXT";>Explanation of
fields: compression method: (2 bytes)</a>
+ */
+ ZSTD(93),
Review Comment:
All new public and protected elements should carry a Javadoc `@since` tag,
in this case 1.28.0. Done in git master.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -931,6 +942,19 @@ public int read(final byte[] buffer, final int offset,
final int length) throws
} else if (current.entry.getMethod() ==
ZipMethod.UNSHRINKING.getCode() || current.entry.getMethod() ==
ZipMethod.IMPLODING.getCode()
|| current.entry.getMethod() ==
ZipMethod.ENHANCED_DEFLATED.getCode() || current.entry.getMethod() ==
ZipMethod.BZIP2.getCode()) {
read = current.inputStream.read(buffer, offset, length);
+ } else if (current.entry.getMethod() == ZipMethod.ZSTD.getCode() ||
current.entry.getMethod() == ZipMethod.ZSTD_DEPRECATED.getCode()) {
Review Comment:
This can be done the same as deflate, removing the need for the one-off
instance variable, done in git master.
More importantly, this code does not appear to be tested, as putting
breakpoints in the debugger on lines 947, 951, and 954 are never hit when
running the new test.
##########
src/main/java/org/apache/commons/compress/compressors/zstandard/ZstdUtils.java:
##########
@@ -28,6 +34,23 @@
*/
public class ZstdUtils {
+ private static final class InnerNotClosingOutputStream extends
CountingOutputStream {
Review Comment:
Not needed, removed in git master.
##########
src/main/java/org/apache/commons/compress/compressors/zstandard/ZstdUtils.java:
##########
@@ -134,4 +157,21 @@ public static void setCacheZstdAvailablity(final boolean
doCache) {
/** Private constructor to prevent instantiation of this utility class. */
private ZstdUtils() {
}
+
+ /**
+ * Reads uncompressed data stream and writes it compressed to the output
+ *
+ * @param input the data stream with uncompressed data
+ * @param output the data stream for compressed output
+ * @return the compressed size
+ * @throws IOException throws the exception which could be got from from
IOUtils.copyLarge()
+ * or ZstdCompressorOutputStream constructor
+ */
+ public static long readAndCompressWrite(InputStream input, OutputStream
output) throws IOException {
+ final InnerNotClosingOutputStream outStream = new
InnerNotClosingOutputStream(output);
+ final ZstdCompressorOutputStream outputStream = new
ZstdCompressorOutputStream(outStream, 3, true);
Review Comment:
The value `3` looks like a magic number. A comment should state the reason
to use 3 instead of another number.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
