[jira] Resolved: (VFS-114) FtpFileObject lists children excessively using ON_RESOLVE cache strategy

["Mario Ivankovits (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/VFS-114?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mario Ivankovits resolved VFS-114.
----------------------------------

       Resolution: Fixed
    Fix Version/s: 1.1

Good catch.
Applied.

Thanks!
Ciao,
Mario

> FtpFileObject lists children excessively using ON_RESOLVE cache strategy
> ------------------------------------------------------------------------
>
>                 Key: VFS-114
>                 URL: https://issues.apache.org/jira/browse/VFS-114
>             Project: Commons VFS
>          Issue Type: Bug
>    Affects Versions: 1.1
>         Environment: Mac OS X 10.4.8, Darwin 8.8.0; Java 1.5.0_06; 
> Libraries: ant-1.6.2.jar, commons-net-1.4.1.jar, commons-logging-1.0.4.jar, commons-collections-3.1.jar, jcifs-0.8.3.jar, jakarta-slide-webdavlib-20050629.161100.jar, jdom-1.0.jar, commons-httpclient-2.0.2.jar, jsch-0.1.23.jar, xml-apis-1.0.b2.jar, oro-2.0.8.jar, maven-xdoc-plugin-1.9.2.jar, 
>            Reporter: K. Lo Shih
>            Priority: Minor
>             Fix For: 1.1
>
>         Attachments: VFS-114.patch
>
>
> When calling FileObject.getChildren() on a org.apache.commons.vfs.provider.ftp.FtpFileObject using the default ON_RESOLVE cache strategy, I would FtpClientWrapper.listFiles() to be called only once. Instead, because FileObject.refresh() is called once for itself and each of its parents for each of the children. For example, if there's a file structure like:
> <pre>
>     ftp://ftp.example.com
>        A/
>           B/ 
>              C/
>                 1
>                 2
>                 3
> </pre>
> and you list files for the URL "ftp://ftp.example.com/A/B/C/". Instead of calling listFiles() once on "C", it calls listFiles() on each of "A", "B", and "C" for each of the children "1", "2" and "3", i.e. 9 times. I discovered this when listing a directory with 709 files--you can imagine my bewilderment at how long it took.
> I fixed this by reusing the FtpFileObject.inRefresh flag within AbstractFileObject.getChildren() to not refresh the listed directory when its children request a refresh. I've used this inRefresh incorrectly, I think. I should rather use a separate flag depth counter like FtpFileObject.refreshDepth, incremented upon entry into AbstractFileObject.getChildren() and decremented upon exit. This approach would fix the implementation for providers other than FTP as well. Anyways, here's my patch:
> Index: core/src/main/java/org/apache/commons/vfs/provider/ftp/FtpFileObject.java
> ===================================================================
> --- core/src/main/java/org/apache/commons/vfs/provider/ftp/FtpFileObject.java   (revision 513805)
> +++ core/src/main/java/org/apache/commons/vfs/provider/ftp/FtpFileObject.java   (working copy)
> @@ -94,7 +94,12 @@
>       */
>      private FTPFile getChildFile(final String name, final boolean flush) throws IOException
>      {
> -        if (flush)
> +        /* If we should flush cached children, clear our children map unless
> +         * we're in the middle of a refresh in which case we've just recently
> +         * refreshed our children. No need to do it again when our children are
> +         * refresh()ed, calling getChildFile() for themselves from within
> +         * getInfo(). See getChildren(). */
> +        if (flush && !inRefresh)
>          {
>                         children = null;
>          }
> @@ -322,6 +327,28 @@
>          return null;
>      }
> +   
> +    /** Returns the file's list of children.
> +     *
> +     *  @return The list of children
> +     *  @throws FileSystemException If there was a problem listing children
> +     *  @see    AbstratFileObject#getChildren()
> +     *  @since  1.0
> +     */
> +    public FileObject[] getChildren() throws FileSystemException {
> +        try {
> +            /* Wrap our parent implementation, noting that we're refreshing so
> +             * that we don't refresh() ourselves and each of our parents for
> +             * each children. Note that refresh() will list children. Meaning,
> +             * if if this file has C children, P parents, there will be (C * P)
> +             * listings made with (C * (P + 1)) refreshes, when there should
> +             * really only be 1 listing and C refreshes. */
> +            this.inRefresh = true;
> +            return super.getChildren();
> +        } finally {
> +            this.inRefresh = false;
> +        }
> +    }
>      /**
>       * Lists the children of the file.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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