garydgregory commented on code in PR #655:
URL: https://github.com/apache/commons-compress/pull/655#discussion_r2008758134
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java:
##########
@@ -167,7 +169,26 @@ public ZipFile get() throws IOException {
actualDescription = path.toString();
}
final boolean closeOnError = seekableByteChannel != null;
- return new ZipFile(actualChannel, actualDescription, getCharset(),
useUnicodeExtraFields, closeOnError, ignoreLocalFileHeader);
+ return new ZipFile(actualChannel, actualDescription, getCharset(),
useUnicodeExtraFields, closeOnError, ignoreLocalFileHeader) {
+ @Override
+ protected InputStream createZstdInputStream(InputStream is)
throws IOException {
+ return zstdInputStream.apply(is);
+ }
+ };
+ }
+
+ /**
+ * @param zstdInpStreamFactory the IOFunction which gives the ability
to return a different InputStream for Zstd compression.
Review Comment:
This method is missing a description.
##########
src/test/java/org/apache/commons/compress/archivers/zip/ZipFileTest.java:
##########
@@ -66,8 +66,36 @@
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Test;
+import io.airlift.compress.zstd.ZstdInputStream;
+
public class ZipFileTest extends AbstractTest {
+ /**
+ * This Class simulates the case where the Zip File uses the
aircompressors {@link ZstdInputStream}
+ */
+ private final class AirliftZstdZipFile extends ZipFile {
+ private boolean used;
+
+ private AirliftZstdZipFile(File file) throws IOException {
+ super(file);
+ }
+
+ protected InputStream createZstdInputStream(InputStream is) throws
IOException {
Review Comment:
Parameter: Use `final`.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java:
##########
@@ -167,7 +169,26 @@ public ZipFile get() throws IOException {
actualDescription = path.toString();
}
final boolean closeOnError = seekableByteChannel != null;
- return new ZipFile(actualChannel, actualDescription, getCharset(),
useUnicodeExtraFields, closeOnError, ignoreLocalFileHeader);
+ return new ZipFile(actualChannel, actualDescription, getCharset(),
useUnicodeExtraFields, closeOnError, ignoreLocalFileHeader) {
+ @Override
Review Comment:
Hello @mehmet-karaman
Thank you for updating this pull request.
The point of parameterizing the builder is to pass the `zstdInputStream` to
the `ZipFile` constructor which can then be recoded to apply the function in
`createZstdInputStream()`. Change the `ZipFile`'s `createZstdInputStream()`
implementation to use `zstdInputStream`, then it's the builder that has the
only hard-coded reference to `ZstdCompressorInputStream`. The `ZipFile`
constructor to edit is private, so we are keeping binary compatibility.
##########
src/test/java/org/apache/commons/compress/archivers/zip/ZipFileTest.java:
##########
@@ -66,8 +66,36 @@
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Test;
+import io.airlift.compress.zstd.ZstdInputStream;
+
public class ZipFileTest extends AbstractTest {
+ /**
+ * This Class simulates the case where the Zip File uses the
aircompressors {@link ZstdInputStream}
+ */
+ private final class AirliftZstdZipFile extends ZipFile {
+ private boolean used;
+
+ private AirliftZstdZipFile(File file) throws IOException {
+ super(file);
+ }
+
+ protected InputStream createZstdInputStream(InputStream is) throws
IOException {
+ return new ZstdInputStream(is) {
+
+ @Override
+ public int read(byte[] outputBuffer, int outputOffset, int
outputLength) throws IOException {
Review Comment:
Parameters: Use final.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java:
##########
@@ -167,7 +169,26 @@ public ZipFile get() throws IOException {
actualDescription = path.toString();
}
final boolean closeOnError = seekableByteChannel != null;
- return new ZipFile(actualChannel, actualDescription, getCharset(),
useUnicodeExtraFields, closeOnError, ignoreLocalFileHeader);
+ return new ZipFile(actualChannel, actualDescription, getCharset(),
useUnicodeExtraFields, closeOnError, ignoreLocalFileHeader) {
+ @Override
+ protected InputStream createZstdInputStream(InputStream is)
throws IOException {
+ return zstdInputStream.apply(is);
+ }
+ };
+ }
+
+ /**
+ * @param zstdInpStreamFactory the IOFunction which gives the ability
to return a different InputStream for Zstd compression.
+ * @return {@code this} instance
+ * @since 1.28.0
+ */
+ public Builder setZstdInputStreamFactory(IOFunction<InputStream,
InputStream> zstdInpStreamFactory) {
+ if (zstdInpStreamFactory == null) {
+ this.zstdInputStream = ZstdCompressorInputStream::new;
+ } else {
+ this.zstdInputStream = zstdInpStreamFactory;
Review Comment:
Simplify: Use a ternary expression.
##########
src/test/java/org/apache/commons/compress/archivers/zip/ZipFileTest.java:
##########
@@ -66,8 +66,36 @@
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Test;
+import io.airlift.compress.zstd.ZstdInputStream;
+
public class ZipFileTest extends AbstractTest {
+ /**
+ * This Class simulates the case where the Zip File uses the
aircompressors {@link ZstdInputStream}
+ */
+ private final class AirliftZstdZipFile extends ZipFile {
+ private boolean used;
+
+ private AirliftZstdZipFile(File file) throws IOException {
Review Comment:
Parameter: Use `final`.
--
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: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
