Эх сурвалжийг харах

Improve performance of DocumentsProvider#isChildDocument()

The old solution iterates through all parents of the child until it
finds the given parent or the storage root.
This has been show to be very expensive in empirical tests.

Therefore, this commit introduces a more efficient solution that simply
compares the file paths of child and given parent.
It also ensures that parent and child belong to the same account.

Reviewers need to take extra care that this change does not introduce
security issues by claiming a document is a child of a parent when it is
really not.

Signed-off-by: Torsten Grote <t@grobox.de>
Torsten Grote 4 жил өмнө
parent
commit
e8870334d2

+ 1 - 1
src/main/java/com/owncloud/android/operations/DownloadFileOperation.java

@@ -171,7 +171,7 @@ public class DownloadFileOperation extends RemoteOperation {
             modificationTimestamp = downloadOperation.getModificationTimestamp();
             etag = downloadOperation.getEtag();
             newFile = new File(getSavePath());
-            if (!newFile.getParentFile().mkdirs()) {
+            if (!newFile.getParentFile().exists() && !newFile.getParentFile().mkdirs()) {
                 Log_OC.e(TAG, "Unable to create parent folder " + newFile.getParentFile().getAbsolutePath());
             }
 

+ 21 - 7
src/main/java/com/owncloud/android/providers/DocumentsStorageProvider.java

@@ -618,15 +618,29 @@ public class DocumentsStorageProvider extends DocumentsProvider {
         Log.d(TAG, "isChildDocument(), parent=" + parentDocumentId + ", id=" + documentId);
 
         try {
-            Document currentDocument = toDocument(documentId);
+            // get file for parent document
             Document parentDocument = toDocument(parentDocumentId);
-
-            while (!ROOT_PATH.equals(currentDocument.getRemotePath())) {
-                currentDocument = currentDocument.getParent();
-                if (parentDocument.getFile().equals(currentDocument.getFile())) {
-                    return true;
-                }
+            OCFile parentFile = parentDocument.getFile();
+            if (parentFile == null) {
+                throw new FileNotFoundException("No parent file with ID " + parentDocumentId);
             }
+            // get file for child candidate document
+            Document currentDocument = toDocument(documentId);
+            OCFile childFile = currentDocument.getFile();
+            if (childFile == null) {
+                throw new FileNotFoundException("No child file with ID " + documentId);
+            }
+
+            String parentPath = parentFile.getDecryptedRemotePath();
+            String childPath = childFile.getDecryptedRemotePath();
+
+            // The alternative is to go up the folder hierarchy from currentDocument with getParent()
+            // until we arrive at parentDocument or the storage root.
+            // However, especially for long paths this is expensive and can take substantial time.
+            // The solution below uses paths and is faster by a factor of 2-10 depending on the nesting level of child.
+            // So far, the same document with its unique ID can never be in two places at once.
+            // If this assumption ever changes, this code would need to be adapted.
+            return parentDocument.getAccount() == currentDocument.getAccount() && childPath.startsWith(parentPath);
 
         } catch (FileNotFoundException e) {
             Log.e(TAG, "failed to check for child document", e);