[
https://issues.apache.org/jira/browse/IO-651?focusedWorklogId=554950&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-554950
]
ASF GitHub Bot logged work on IO-651:
-------------------------------------
Author: ASF GitHub Bot
Created on: 19/Feb/21 21:44
Start Date: 19/Feb/21 21:44
Worklog Time Spent: 10m
Work Description: garydgregory commented on a change in pull request #206:
URL: https://github.com/apache/commons-io/pull/206#discussion_r579489683
##########
File path:
src/main/java/org/apache/commons/io/output/DeferredFileOutputStream.java
##########
@@ -272,4 +274,31 @@ public void writeTo(final OutputStream outputStream)
throws IOException {
}
}
}
+
+ /**
+ * Gets the current contents of this byte stream as an {@link InputStream}.
+ * If the data for this output stream has been retained in memory, the
+ * returned stream is backed by buffers of {@code this} stream,
+ * avoiding memory allocation and copy, thus saving space and time.<br>
+ * Otherwise, the returned stream will be one that is created from the data
+ * that has been committed to disk.
+ *
+ * @return the current contents of this output stream.
+ * @throws IOException if this stream is not yet closed or an error occurs.
+ * @see org.apache.commons.io.output.ByteArrayOutputStream#toInputStream()
Review comment:
Needs a Javadoc since tag.
##########
File path:
src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java
##########
@@ -365,4 +370,55 @@ private void verifyResultFile(final File testFile) {
fail("Unexpected IOException");
}
}
+
+ /**
+ * Tests the case where the amount of data falls below the threshold, and
is therefore confined to memory.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testBelowThresholdGetInputStream(final int initialBufferSize)
throws IOException {
+ final DeferredFileOutputStream dfos = new
DeferredFileOutputStream(testBytes.length + 42, initialBufferSize,
+ null);
+ try {
+ dfos.write(testBytes, 0, testBytes.length);
+ dfos.close();
+ } catch (final IOException e) {
+ fail("Unexpected IOException");
+ }
+ assertTrue(dfos.isInMemory());
+
+ try(InputStream is = dfos.toInputStream()) {
+ final byte[] resultBytes = IOUtils.toByteArray(is);
+ assertArrayEquals(testBytes, resultBytes);
+ }
+ }
+
+ /**
+ * Tests the case where the amount of data exceeds the threshold, and is
therefore written to disk. The actual data
+ * written to disk is verified, as is the file itself.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testAboveThresholdGetInputStream(final int initialBufferSize,
@TempDir Path tempDir) throws IOException {
+ final File testFile =
tempDir.resolve("testAboveThreshold.dat").toFile();
+
+ final DeferredFileOutputStream dfos = new
DeferredFileOutputStream(testBytes.length - 5, initialBufferSize,
+ testFile);
+ try {
+ dfos.write(testBytes, 0, testBytes.length);
+ dfos.close();
+ } catch (final IOException e) {
+ fail("Unexpected IOException");
+ }
+ assertFalse(dfos.isInMemory());
+
+ try(InputStream is = dfos.toInputStream()) {
Review comment:
Missing space after "try".
##########
File path:
src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java
##########
@@ -365,4 +370,55 @@ private void verifyResultFile(final File testFile) {
fail("Unexpected IOException");
}
}
+
+ /**
+ * Tests the case where the amount of data falls below the threshold, and
is therefore confined to memory.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testBelowThresholdGetInputStream(final int initialBufferSize)
throws IOException {
+ final DeferredFileOutputStream dfos = new
DeferredFileOutputStream(testBytes.length + 42, initialBufferSize,
+ null);
+ try {
+ dfos.write(testBytes, 0, testBytes.length);
+ dfos.close();
+ } catch (final IOException e) {
+ fail("Unexpected IOException");
+ }
+ assertTrue(dfos.isInMemory());
+
+ try(InputStream is = dfos.toInputStream()) {
Review comment:
Missing space after "try".
##########
File path:
src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java
##########
@@ -365,4 +370,55 @@ private void verifyResultFile(final File testFile) {
fail("Unexpected IOException");
}
}
+
+ /**
+ * Tests the case where the amount of data falls below the threshold, and
is therefore confined to memory.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testBelowThresholdGetInputStream(final int initialBufferSize)
throws IOException {
+ final DeferredFileOutputStream dfos = new
DeferredFileOutputStream(testBytes.length + 42, initialBufferSize,
+ null);
+ try {
+ dfos.write(testBytes, 0, testBytes.length);
+ dfos.close();
+ } catch (final IOException e) {
+ fail("Unexpected IOException");
+ }
+ assertTrue(dfos.isInMemory());
+
+ try(InputStream is = dfos.toInputStream()) {
+ final byte[] resultBytes = IOUtils.toByteArray(is);
+ assertArrayEquals(testBytes, resultBytes);
+ }
+ }
+
+ /**
+ * Tests the case where the amount of data exceeds the threshold, and is
therefore written to disk. The actual data
+ * written to disk is verified, as is the file itself.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testAboveThresholdGetInputStream(final int initialBufferSize,
@TempDir Path tempDir) throws IOException {
+ final File testFile =
tempDir.resolve("testAboveThreshold.dat").toFile();
+
+ final DeferredFileOutputStream dfos = new
DeferredFileOutputStream(testBytes.length - 5, initialBufferSize,
+ testFile);
+ try {
+ dfos.write(testBytes, 0, testBytes.length);
+ dfos.close();
+ } catch (final IOException e) {
+ fail("Unexpected IOException");
+ }
+ assertFalse(dfos.isInMemory());
+
+ try(InputStream is = dfos.toInputStream()) {
+ final byte[] resultBytes = IOUtils.toByteArray(is);
+ assertArrayEquals(testBytes, resultBytes);
Review comment:
No need for intermediary local variable.
##########
File path:
src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java
##########
@@ -365,4 +370,55 @@ private void verifyResultFile(final File testFile) {
fail("Unexpected IOException");
}
}
+
+ /**
+ * Tests the case where the amount of data falls below the threshold, and
is therefore confined to memory.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testBelowThresholdGetInputStream(final int initialBufferSize)
throws IOException {
+ final DeferredFileOutputStream dfos = new
DeferredFileOutputStream(testBytes.length + 42, initialBufferSize,
Review comment:
Use try-with-resources.
##########
File path:
src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java
##########
@@ -365,4 +370,55 @@ private void verifyResultFile(final File testFile) {
fail("Unexpected IOException");
}
}
+
+ /**
+ * Tests the case where the amount of data falls below the threshold, and
is therefore confined to memory.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testBelowThresholdGetInputStream(final int initialBufferSize)
throws IOException {
+ final DeferredFileOutputStream dfos = new
DeferredFileOutputStream(testBytes.length + 42, initialBufferSize,
+ null);
+ try {
+ dfos.write(testBytes, 0, testBytes.length);
+ dfos.close();
+ } catch (final IOException e) {
+ fail("Unexpected IOException");
Review comment:
Useless catch.
##########
File path:
src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java
##########
@@ -365,4 +370,55 @@ private void verifyResultFile(final File testFile) {
fail("Unexpected IOException");
}
}
+
+ /**
+ * Tests the case where the amount of data falls below the threshold, and
is therefore confined to memory.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testBelowThresholdGetInputStream(final int initialBufferSize)
throws IOException {
+ final DeferredFileOutputStream dfos = new
DeferredFileOutputStream(testBytes.length + 42, initialBufferSize,
+ null);
+ try {
+ dfos.write(testBytes, 0, testBytes.length);
+ dfos.close();
+ } catch (final IOException e) {
+ fail("Unexpected IOException");
+ }
+ assertTrue(dfos.isInMemory());
+
+ try(InputStream is = dfos.toInputStream()) {
+ final byte[] resultBytes = IOUtils.toByteArray(is);
+ assertArrayEquals(testBytes, resultBytes);
+ }
+ }
+
+ /**
+ * Tests the case where the amount of data exceeds the threshold, and is
therefore written to disk. The actual data
+ * written to disk is verified, as is the file itself.
+ * Testing the getInputStream() method.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
Review comment:
See other comments, which apply here as well. Use final in method
parmeter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 554950)
Time Spent: 2h (was: 1h 50m)
> Provide method to retrieve data from DeferredFileOutputStream as InputStream
> ----------------------------------------------------------------------------
>
> Key: IO-651
> URL: https://issues.apache.org/jira/browse/IO-651
> Project: Commons IO
> Issue Type: Improvement
> Components: Streams/Writers
> Affects Versions: 2.6
> Reporter: John Mark
> Priority: Major
> Labels: easyfix, performance, pull-request-available
> Time Spent: 2h
> Remaining Estimate: 0h
>
> It would be very helpful to have to {{toInputStream()}} method on the
> {{DeferredFileOutputStream}} class. Besides for convenience, it would allow
> for improved efficiency since in the case of in-memory data the byte buffer
> would not need to be copied (as opposed to the current {{getData()}} method).
> The implementation is pretty simple and can be something like the following
> (based on the current {{writeTo(OutputStream)}} method):
> {code:java}
> public InputStream toInputStream() throws IOException {
> if (!closed) {
> throw new IOException("Stream not closed");
> }
> if (isInMemory()) {
> return memoryOutputStream.toInputStream();
> } else {
> return Files.newInputStream(outputFile.toPath());
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
