Parse relative path from docId

Remove the logic of using file path directly.
Parse the docId to get the relative path.

Test: atest ExternalStorageProviderTest
Change-Id: Ic218813acc73e247ee5593ed9e8e7688760e6780
Fix: 144467519

2 files changed, 46 insertions, 22 deletions

diff --git a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
index 4a50210d1a60..7b2922ba961b 100644
--- a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
+++ b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
@@ -50,6 +50,7 @@ import android.util.Log;
 import android.util.Pair;
 
 import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.content.FileSystemProvider;
 import com.android.internal.util.IndentingPrintWriter;
 
@@ -308,37 +309,26 @@ public class ExternalStorageProvider extends FileSystemProvider {
     @Override
     protected boolean shouldBlockFromTree(@NonNull String docId) {
         try {
-            final File dir = getFileForDocId(docId, true /* visible */).getCanonicalFile();
-            if (!dir.isDirectory()) {
+            final File dir = getFileForDocId(docId, false /* visible */);
+
+            // the file is null or it is not a directory
+            if (dir == null || !dir.isDirectory()) {
                 return false;
             }
 
-            final String path = dir.getAbsolutePath();
+            final String path = getPathFromDocId(docId);
 
-            // Block Download folder from tree
-            if (MediaStore.Downloads.isDownloadDir(path)) {
+            // Block the root of the storage
+            if (path.isEmpty()) {
                 return true;
             }
 
-            final ArrayMap<String, RootInfo> roots = new ArrayMap<>();
-
-            synchronized (mRootsLock) {
-                roots.putAll(mRoots);
+            // Block Download folder from tree
+            if (TextUtils.equals(Environment.DIRECTORY_DOWNLOADS.toLowerCase(),
+                    path.toLowerCase())) {
+                return true;
             }
 
-            // block root of storage
-            for (int i = 0; i < roots.size(); i++) {
-                RootInfo rootInfo = roots.valueAt(i);
-                // skip home root
-                if (TextUtils.equals(rootInfo.rootId, ROOT_ID_HOME)) {
-                    continue;
-                }
-
-                // block the root of storage
-                if (TextUtils.equals(path, rootInfo.visiblePath.getAbsolutePath())) {
-                    return true;
-                }
-            }
             return false;
         } catch (IOException e) {
             throw new IllegalArgumentException(
@@ -430,6 +420,23 @@ public class ExternalStorageProvider extends FileSystemProvider {
         return Pair.create(root, buildFile(root, docId, visible, true));
     }
 
+    @VisibleForTesting
+    static String getPathFromDocId(String docId) {
+        final int splitIndex = docId.indexOf(':', 1);
+        final String path = docId.substring(splitIndex + 1);
+
+        if (path.isEmpty()) {
+            return path;
+        }
+
+        // remove trailing "/"
+        if (path.charAt(path.length() - 1) == '/') {
+            return path.substring(0, path.length() - 1);
+        } else {
+            return path;
+        }
+    }
+
     private RootInfo getRootFromDocId(String docId) throws FileNotFoundException {
         final int splitIndex = docId.indexOf(':', 1);
         final String tag = docId.substring(0, splitIndex);
diff --git a/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java b/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java
index fbf2e4b8ff19..ed8320fa7fef 100644
--- a/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java
+++ b/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java
@@ -17,7 +17,10 @@
 package com.android.externalstorage;
 
 import static com.android.externalstorage.ExternalStorageProvider.AUTHORITY;
+import static com.android.externalstorage.ExternalStorageProvider.getPathFromDocId;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
@@ -51,4 +54,18 @@ public class ExternalStorageProviderTest {
 
         verify(spyProvider, atLeast(1)).updateVolumes();
     }
+
+    @Test
+    public void testGetPathFromDocId() throws Exception {
+        final String root = "root";
+        final String path = "abc/def/ghi";
+        String docId = root + ":" + path;
+        assertEquals(getPathFromDocId(docId), path);
+
+        docId = root + ":" + path + "/";
+        assertEquals(getPathFromDocId(docId), path);
+
+        docId = root + ":";
+        assertTrue(getPathFromDocId(docId).isEmpty());
+    }
 }