commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@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:45:22 GMT
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?

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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message