commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "K. Lo Shih (JIRA)" <j...@apache.org>
Subject [jira] Created: (VFS-114) FtpFileObject lists children excessively using ON_RESOLVE cache strategy
Date Mon, 05 Mar 2007 22:10:50 GMT
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


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


Mime
View raw message