[ 
https://issues.apache.org/jira/browse/IO-849?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17830163#comment-17830163
 ] 

Gary D. Gregory edited comment on IO-849 at 3/23/24 8:29 PM:
-------------------------------------------------------------

There might be a misunderstanding here: The caller is always responsible for 
deleting the target file, whether it is caller-supplied in the builder (or 
constructor), or created by the class from a name prefix.

When you use the (deprecated) DeferredFileOutputStream(int, String, String, 
File), you don't know the exact file name you'll end up with, so you must call 
getFile() or getPath() to find out, use that file, and delete it when you are 
done. 

The file created with a prefix is not always in the directory but can be in a 
user supplier directory.

Once you have the stream, even after it is closed, you have access to the 
File/Path (the caller's file or the one created by the class).

I touched up the Javadoc in git master.

For example, see 
{{org.apache.commons.io.output.DeferredFileOutputStreamTest.testTempFileAboveThresholdPrefixOnly(int)}}:

{code:java}
    /**
     * Tests specifying a temporary file and the threshold is reached.
     * @throws IOException
     */
    @ParameterizedTest(name = "initialBufferSize = {0}")
    @MethodSource("data")
    public void testTempFileAboveThresholdPrefixOnly(final int 
initialBufferSize) throws IOException {

        final String prefix = "commons-io-test";
        final String suffix = null;
        final DeferredFileOutputStream dfos = DeferredFileOutputStream.builder()
                .setThreshold(testBytes.length - 5)
                .setBufferSize(initialBufferSize)
                .setPrefix(prefix)
                .setSuffix(suffix)
                .setDirectory((Path) null)
                .get();
        try {
            assertNull(dfos.getFile(), "Check File is null-A");
            assertNull(dfos.getPath(), "Check Path is null-A");
            dfos.write(testBytes, 0, testBytes.length);
            dfos.close();
            assertFalse(dfos.isInMemory());
            assertNull(dfos.getData());
            assertEquals(testBytes.length, dfos.getByteCount());
            assertNotNull(dfos.getFile(), "Check file not null");
            assertTrue(dfos.getFile().exists(), "Check file exists");
            assertTrue(dfos.getFile().getName().startsWith(prefix), "Check 
prefix");
            assertTrue(dfos.getFile().getName().endsWith(".tmp"), "Check 
suffix"); // ".tmp" is default
            verifyResultFile(dfos.getFile());
        } finally {
            // Delete the temporary file.
            dfos.getFile().delete();
        }
    }
{code}
Where the pattern is:
{code:java}
        final DeferredFileOutputStream dfos = DeferredFileOutputStream.builder()
                .set...
                .set...
                .get();
        try {
            dfos.write(...);
            dfos.close();
        } finally {
            // Use, then delete the temporary file.
            dfos.getFile().delete();
        }
    }
{code}

HTH.


was (Author: garydgregory):
There might be a misunderstanding here: The caller is always responsible for 
deleting the target file, whether it is caller-supplied in the builder (or 
constructor), or created by the class from a name prefix.

When you use the (deprecated) DeferredFileOutputStream(int, String, String, 
File), you don't know the exact file name you'll end up with, so you must call 
getFile() or getPath() to find out, use that file, and delete it when you are 
done. 

The file created with a prefix is not always in the directory but can be in a 
user supplier directory.

Once you have the stream, even after it is closed, you have access to the 
File/Path (the caller's file or the one created by the class).

For example, see 
{{org.apache.commons.io.output.DeferredFileOutputStreamTest.testTempFileAboveThresholdPrefixOnly(int)}}:

{code:java}
    /**
     * Tests specifying a temporary file and the threshold is reached.
     * @throws IOException
     */
    @ParameterizedTest(name = "initialBufferSize = {0}")
    @MethodSource("data")
    public void testTempFileAboveThresholdPrefixOnly(final int 
initialBufferSize) throws IOException {

        final String prefix = "commons-io-test";
        final String suffix = null;
        final DeferredFileOutputStream dfos = DeferredFileOutputStream.builder()
                .setThreshold(testBytes.length - 5)
                .setBufferSize(initialBufferSize)
                .setPrefix(prefix)
                .setSuffix(suffix)
                .setDirectory((Path) null)
                .get();
        try {
            assertNull(dfos.getFile(), "Check File is null-A");
            assertNull(dfos.getPath(), "Check Path is null-A");
            dfos.write(testBytes, 0, testBytes.length);
            dfos.close();
            assertFalse(dfos.isInMemory());
            assertNull(dfos.getData());
            assertEquals(testBytes.length, dfos.getByteCount());
            assertNotNull(dfos.getFile(), "Check file not null");
            assertTrue(dfos.getFile().exists(), "Check file exists");
            assertTrue(dfos.getFile().getName().startsWith(prefix), "Check 
prefix");
            assertTrue(dfos.getFile().getName().endsWith(".tmp"), "Check 
suffix"); // ".tmp" is default
            verifyResultFile(dfos.getFile());
        } finally {
            // Delete the temporary file.
            dfos.getFile().delete();
        }
    }
{code}
Where the pattern is:
{code:java}
        final DeferredFileOutputStream dfos = DeferredFileOutputStream.builder()
                .set...
                .set...
                .get();
        try {
            dfos.write(...);
            dfos.close();
        } finally {
            // Use, then delete the temporary file.
            dfos.getFile().delete();
        }
    }
{code}

HTH.

> DeferredFileOutputStream does not delete the temporary file created
> -------------------------------------------------------------------
>
>                 Key: IO-849
>                 URL: https://issues.apache.org/jira/browse/IO-849
>             Project: Commons IO
>          Issue Type: Improvement
>            Reporter: Sheung Chi Chan
>            Priority: Trivial
>
> The {{DeferredFileOutputStream}} class is a custom {{OutputStream}} object 
> from the Apache Commons IO library which will not write data directly to 
> disk. It will only write data to disk when the configured threshold is 
> reached. During the initialisation of the {{DeferredFileOutputStream}} object 
> through its builder class, the user could specify a custom file path or 
> provide a prefix and suffix for temporary file creation. The provided custom 
> file path or the temporary file created will be used for storing the data on 
> disk when the configured threshold is reached. When using the prefix/suffix 
> approach, the temporary file is created using the 
> {{java.nio.file.Files::createTempFile}} method only when the threshold is 
> reached. The temporary file created by the 
> {{java.nio.file.Files::createTempFile}} method will not be removed 
> automatically, thus when the stream is closed after the threshold is reached 
> and the prefix/suffix approach is used, there will be an unexpected file 
> stored in the disk persistently. Although it should not be accessible by 
> other users since the {{java.nio.file.Files::createTempFile}} method creates 
> a temporary file only for the current user to access, it still poses a 
> problem when the {{DeferredFileOutputStream}} object is being flooded with a 
> large amount of data. This could use up the disk space and cause possible 
> out-of-disk space problems.
> Although the flooding of data could also be a problem when using the 
> user-provided file, since it is the user who creates the file, thus the user 
> is responsible to remove or clean up that file when it is no longer used. But 
> if the prefix/suffix approach is used, the user does not have control of the 
> file and when the {{DeferredFileOutputStream}} is closed, it is assumed that 
> the temporary file created during the processing of 
> {{DeferredFileOutputStream}} is removed or cleaned up. It is a general 
> practice for Java OutputStream to clean up its process and temporary objects 
> when its close method is called. Thus the missing that could result in 
> unexpectedly large files staying in the disk unawared.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to