commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1480300 - in /commons/proper/io/trunk/src: changes/changes.xml main/java/org/apache/commons/io/FileUtils.java
Date Wed, 08 May 2013 16:14:33 GMT
On 8 May 2013 15:51, Gary Gregory <garydgregory@gmail.com> wrote:
> Sebb,
>
> Are you suggesting making copyInputStreamToFile(InputStream, File, boolean)
> private and adding a public method like copyToFile(InputStream, File) that
> calls it with false?

Not exactly.

I'm saying that it does not make sense to me for that method to be public.
Instead we should have a new method that provides the no close behaviour.

Whether the old and new behaviour have their own code or delegate to a
3rd private method is another matter.
In fact it occurs to me that:

copyInputStreamToFile(InputStream, File)
could just call
copyToFile(InputStream, File)
and then close the input.

> Gary
>
>
>
> On Wed, May 8, 2013 at 10:45 AM, sebb <sebbaz@gmail.com> wrote:
>
>> On 8 May 2013 15:36,  <ggregory@apache.org> wrote:
>> > Author: ggregory
>> > Date: Wed May  8 14:36:32 2013
>> > New Revision: 1480300
>> >
>> > URL: http://svn.apache.org/r1480300
>> > Log:
>> > <action issue="IO-381" dev="ggregory" type="add">
>> >         Add FileUtils.copyInputStreamToFile API with option to leave the
>> source open.
>> >         See copyInputStreamToFile(final InputStream source, final File
>> destination, boolean closeSource)
>> >       </action>
>> >
>> > Modified:
>> >     commons/proper/io/trunk/src/changes/changes.xml
>> >
>> commons/proper/io/trunk/src/main/java/org/apache/commons/io/FileUtils.java
>> >
>> > Modified: commons/proper/io/trunk/src/changes/changes.xml
>> > URL:
>> http://svn.apache.org/viewvc/commons/proper/io/trunk/src/changes/changes.xml?rev=1480300&r1=1480299&r2=1480300&view=diff
>> >
>> ==============================================================================
>> > --- commons/proper/io/trunk/src/changes/changes.xml (original)
>> > +++ commons/proper/io/trunk/src/changes/changes.xml Wed May  8 14:36:32
>> 2013
>> > @@ -47,7 +47,11 @@ The <action> type attribute can be add,u
>> >    <body>
>> >      <!-- The release date is the date RC is cut -->
>> >      <release version="2.5" date="2013-??-??" description="New features
>> and bug fixes.">
>> > -      <action issue="IO-380" dev="sebb" type="fix">
>> > +      <action issue="IO-381" dev="ggregory" type="add">
>> > +        Add FileUtils.copyInputStreamToFile API with option to leave
>> the source open.
>> > +        See copyInputStreamToFile(final InputStream source, final File
>> destination, boolean closeSource)
>> > +      </action>
>> > +      <action issue="IO-380" dev="sebb" type="fix" due-to="claudio_ch">
>> >          FileUtils.copyInputStreamToFile should document it closes the
>> input source
>> >        </action>
>> >        <action issue="IO-279" dev="sebb" type="fix">
>> >
>> > Modified:
>> commons/proper/io/trunk/src/main/java/org/apache/commons/io/FileUtils.java
>> > URL:
>> http://svn.apache.org/viewvc/commons/proper/io/trunk/src/main/java/org/apache/commons/io/FileUtils.java?rev=1480300&r1=1480299&r2=1480300&view=diff
>> >
>> ==============================================================================
>> > ---
>> commons/proper/io/trunk/src/main/java/org/apache/commons/io/FileUtils.java
>> (original)
>> > +++
>> commons/proper/io/trunk/src/main/java/org/apache/commons/io/FileUtils.java
>> Wed May  8 14:36:32 2013
>> > @@ -1459,8 +1459,7 @@ public class FileUtils {
>> >       * @throws IOException if an IO error occurs during copying
>> >       */
>> >      public static void copyURLToFile(final URL source, final File
>> destination) throws IOException {
>> > -        final InputStream input = source.openStream();
>> > -        copyInputStreamToFile(input, destination);
>> > +        copyInputStreamToFile(source.openStream(), destination);
>>
>> That really belonged in a separate commit.
>>
>> >      }
>> >
>> >      /**
>> > @@ -1488,8 +1487,7 @@ public class FileUtils {
>> >          final URLConnection connection = source.openConnection();
>> >          connection.setConnectTimeout(connectionTimeout);
>> >          connection.setReadTimeout(readTimeout);
>> > -        final InputStream input = connection.getInputStream();
>> > -        copyInputStreamToFile(input, destination);
>> > +        copyInputStreamToFile(connection.getInputStream(), destination);
>>
>> Ditto.
>>
>> >      }
>> >
>> >      /**
>> > @@ -1509,6 +1507,27 @@ public class FileUtils {
>> >       * @since 2.0
>> >       */
>> >      public static void copyInputStreamToFile(final InputStream source,
>> final File destination) throws IOException {
>> > +        copyInputStreamToFile(source, destination, true);
>> > +    }
>> > +
>> > +    /**
>> > +     * Copies bytes from an {@link InputStream} <code>source</code>
to
>> a file
>> > +     * <code>destination</code>. The directories up to
>> <code>destination</code>
>> > +     * will be created if they don't already exist.
>> <code>destination</code>
>> > +     * will be overwritten if it already exists.
>> > +     *
>> > +     * @param source  the <code>InputStream</code> to copy bytes
from,
>> must not be {@code null}, will be closed
>>
>> That's wrong.
>>
>> > +     * @param destination  the non-directory <code>File</code>
to write
>> bytes to
>> > +     *  (possibly overwriting), must not be {@code null}
>> > +     * @param closeSource If true, closes the <code>source</code>
>> > +     * @throws IOException if <code>destination</code> is a directory
>> > +     * @throws IOException if <code>destination</code> cannot be
written
>> > +     * @throws IOException if <code>destination</code> needs creating
>> but can't be
>> > +     * @throws IOException if an IO error occurs during copying
>> > +     * @since 2.5
>> > +     */
>> > +    public static void copyInputStreamToFile(final InputStream source,
>> final File destination, boolean closeSource)
>> > +            throws IOException {
>>
>> -1
>>
>> I don't like this new method; as explained in  IO-381 there is no
>> reason for anyone to use
>>
>> copyInputStreamToFile(source, destination, true);
>>
>> as that is the same as
>>
>> copyInputStreamToFile(source, destination);
>>
>> >          try {
>> >              final FileOutputStream output =
>> openOutputStream(destination);
>> >              try {
>> > @@ -1518,7 +1537,9 @@ public class FileUtils {
>> >                  IOUtils.closeQuietly(output);
>> >              }
>> >          } finally {
>> > -            IOUtils.closeQuietly(source);
>> > +            if (closeSource) {
>> > +                IOUtils.closeQuietly(source);
>> > +            }
>> >          }
>> >      }
>> >
>> >
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message