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 22:51:03 GMT
On 8 May 2013 17:45, Gary Gregory <garydgregory@gmail.com> wrote:
> On Wed, May 8, 2013 at 12:14 PM, sebb <sebbaz@gmail.com> wrote:
>
>> 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.
>>
>
> OK, like revision 1480346 then?

Yes, except for the Javadoc copy/paste error, which I have just corrected.

> Gary
>
>>
>> > 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
>>
>>
>
>
> --
> 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