Skip to content

Commit 6c8265a

Browse files
author
Uwe Voigt
committed
[COMPRESS-722] Be more memory efficient when writing one byte, test builder construction without file and fix an inconsistency
1 parent 5bb7daa commit 6c8265a

2 files changed

Lines changed: 33 additions & 30 deletions

File tree

src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java

Lines changed: 14 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,6 @@ public abstract static class AbstractBuilder<T extends ZipArchiveOutputStream, B
8686
*/
8787
protected long zipSplitSize;
8888

89-
/**
90-
* The options specifying how the file is opened.
91-
*/
92-
protected OpenOption[] options;
93-
9489
/**
9590
* Whether this stream automatically compresses entries using registered compressor factories.
9691
*/
@@ -103,19 +98,6 @@ protected AbstractBuilder() {
10398
setCharset(StandardCharsets.UTF_8);
10499
}
105100

106-
/**
107-
* Sets the options specifying how the file is opened.
108-
*
109-
* <p>Null by default.</p>
110-
*
111-
* @param options the opem options.
112-
* @return {@code this} instance.
113-
*/
114-
public B setOptions(OpenOption... options) {
115-
this.options = options;
116-
return asThis();
117-
}
118-
119101
/**
120102
* Sets the maximum size of a single part of the split archive created by this stream. Must be between 64kB and about 4GB.
121103
*
@@ -180,10 +162,12 @@ public ZipArchiveOutputStream get() throws IOException {
180162
* and writes it to the underlying StreamCompressor as raw (already-compressed) bytes.
181163
*/
182164
private class CompressorBridgeOutputStream extends OutputStream {
165+
private final byte[] oneByte = new byte[1];
166+
183167
@Override
184168
public void write(final int b) throws IOException {
185-
final byte[] buf = { (byte) b };
186-
write(buf, 0, 1);
169+
oneByte[0] = (byte) (b & ZipConstants.BYTE_MASK);
170+
write(oneByte, 0, 1);
187171
}
188172

189173
@Override
@@ -552,19 +536,21 @@ protected ZipArchiveOutputStream(final AbstractBuilder<?, ?> builder) throws IOE
552536
this.out = new SeekableChannelRandomAccessOutputStream(builder.getChannel(SeekableByteChannel.class));
553537
this.isSplitZip = false;
554538
} else {
555-
final Path path = builder.getPath();
539+
final Path path;
540+
try {
541+
path = builder.getPath();
542+
} catch (IllegalStateException e) {
543+
throw new IOException("No output stream available", e);
544+
}
556545
if (builder.zipSplitSize > 0) {
557546
this.out = new ZipSplitOutputStream(path, builder.zipSplitSize);
558547
this.isSplitZip = true;
559548
} else {
560-
OpenOption[] options = builder.getOpenOptions();
549+
final OpenOption[] options = builder.getOpenOptions();
561550
this.out = options.length == 0 ? new FileRandomAccessOutputStream(path) : new FileRandomAccessOutputStream(path, options);
562551
this.isSplitZip = false;
563552
}
564553
}
565-
if (this.out == null) {
566-
throw new IOException("No output stream available");
567-
}
568554
this.def = new Deflater(level, true);
569555
this.streamCompressor = StreamCompressor.create(this.out, def);
570556
this.autoCompress = builder.autoCompress;
@@ -655,11 +641,9 @@ public ZipArchiveOutputStream(final Path path, final long zipSplitSize) throws I
655641
* @since 1.21
656642
*/
657643
public ZipArchiveOutputStream(final Path file, final OpenOption... options) throws IOException {
658-
this.def = new Deflater(level, true);
659-
this.out = options.length == 0 ? new FileRandomAccessOutputStream(file) : new FileRandomAccessOutputStream(file, options);
660-
this.streamCompressor = StreamCompressor.create(out, def);
661-
this.isSplitZip = false;
662-
this.autoCompress = false;
644+
this(builder()
645+
.setPath(file)
646+
.setOpenOptions(options));
663647
}
664648

665649
/**

src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStreamTest.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@
2020

2121
import static org.junit.jupiter.api.Assertions.assertEquals;
2222
import static org.junit.jupiter.api.Assertions.assertFalse;
23+
import static org.junit.jupiter.api.Assertions.assertThrows;
2324
import static org.junit.jupiter.api.Assertions.assertTrue;
2425

2526
import java.io.ByteArrayOutputStream;
2627
import java.io.IOException;
28+
import java.nio.channels.NonWritableChannelException;
2729
import java.nio.charset.Charset;
2830
import java.nio.charset.StandardCharsets;
31+
import java.nio.file.StandardOpenOption;
2932
import java.util.zip.Deflater;
3033
import java.util.zip.ZipEntry;
3134

@@ -77,4 +80,20 @@ void testSetEncoding() throws IOException {
7780
assertEquals(Charset.defaultCharset().name(), outputStream.getEncoding());
7881
}
7982
}
83+
84+
@Test
85+
void testOpenFileWithOptions() throws IOException {
86+
ZipArchiveOutputStream out = ZipArchiveOutputStream.builder()
87+
.setFile(createTempFile())
88+
.setOpenOptions(StandardOpenOption.READ, StandardOpenOption.DELETE_ON_CLOSE)
89+
.get();
90+
ZipArchiveEntry entry = new ZipArchiveEntry("test.txt");
91+
assertThrows(NonWritableChannelException.class, () -> out.putArchiveEntry(entry));
92+
}
93+
94+
@Test
95+
void testReportMissingOutputStreamUsingBuilder() {
96+
IOException exception = assertThrows(IOException.class, () -> ZipArchiveOutputStream.builder().get());
97+
assertEquals("No output stream available", exception.getMessage());
98+
}
8099
}

0 commit comments

Comments
 (0)