From 5f2148077ab80e5e9e1698a6033fb83b6cfe90ee Mon Sep 17 00:00:00 2001
From: Garfield Tan <xutan@google.com>
Date: Wed, 26 Oct 2016 14:52:46 -0700
Subject: [PATCH] Iteration on findPath API. Address Jeff's comment in
 ag/1542773.

* Add @Nullable to rootId in Path constructor.
* Erase rootId if findPath() doesn't return null rootId.
* Use Log.wtf() instead of throwing.

Bug: 30948740
Change-Id: I38354c5ac49aaa7e31a3ae56cd3379ffb19918c9
(Cherry picked from commit 04fdf6e18c97c2bf6ed58f8de9ba6723fa8c1613)
---
 .../android/provider/DocumentsContract.java   |  2 +-
 .../android/provider/DocumentsProvider.java   | 18 ++++++++++++------
 .../provider/DocumentsProviderTest.java       | 19 ++++++-------------
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/core/java/android/provider/DocumentsContract.java b/core/java/android/provider/DocumentsContract.java
index 1faada14e3478..8bf57a7006238 100644
--- a/core/java/android/provider/DocumentsContract.java
+++ b/core/java/android/provider/DocumentsContract.java
@@ -1412,7 +1412,7 @@ public final class DocumentsContract {
          * @param path the list of document ids from the parent document at
          *          position 0 to the child document.
          */
-        public Path(String rootId, List<String> path) {
+        public Path(@Nullable String rootId, List<String> path) {
             checkCollectionNotEmpty(path, "path");
             checkCollectionElementsNotNull(path, "path");
 
diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java
index d75781b792165..4256484b74c25 100644
--- a/core/java/android/provider/DocumentsProvider.java
+++ b/core/java/android/provider/DocumentsProvider.java
@@ -927,14 +927,20 @@ public abstract class DocumentsProvider extends ContentProvider {
                     ? DocumentsContract.getTreeDocumentId(documentUri)
                     : null;
 
-            final Path path = findPath(documentId, parentDocumentId);
+            Path path = findPath(documentId, parentDocumentId);
 
             // Ensure provider doesn't leak information to unprivileged callers.
-            if (isTreeUri
-                    && (path.getRootId() != null
-                        || !Objects.equals(path.getPath().get(0), parentDocumentId))) {
-                throw new IllegalStateException(
-                        "Provider returns an invalid result for findPath.");
+            if (isTreeUri) {
+                if (!Objects.equals(path.getPath().get(0), parentDocumentId)) {
+                    Log.wtf(TAG, "Provider doesn't return path from the tree root. Expected: "
+                            + parentDocumentId + " found: " + path.getPath().get(0));
+                }
+
+                if (path.getRootId() != null) {
+                    Log.wtf(TAG, "Provider returns root id :"
+                            + path.getRootId() + " unexpectedly. Erase root id.");
+                    path = new Path(null, path.getPath());
+                }
             }
 
             out.putParcelable(DocumentsContract.EXTRA_RESULT, path);
diff --git a/core/tests/coretests/src/android/provider/DocumentsProviderTest.java b/core/tests/coretests/src/android/provider/DocumentsProviderTest.java
index 0b4675c756e1c..71546e4b323b8 100644
--- a/core/tests/coretests/src/android/provider/DocumentsProviderTest.java
+++ b/core/tests/coretests/src/android/provider/DocumentsProviderTest.java
@@ -86,25 +86,18 @@ public class DocumentsProviderTest extends ProviderTestCase2<TestDocumentsProvid
         assertNull(DocumentsContract.findPath(mResolver, docUri));
     }
 
-    public void testFindPath_treeUri_throwsOnNonNullRootId() throws Exception {
+    public void testFindPath_treeUri_erasesNonNullRootId() throws Exception {
         mProvider.nextIsChildDocument = true;
 
         mProvider.nextPath = new Path(ROOT_ID, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));
 
         final Uri docUri = buildTreeDocumentUri(
                 TestDocumentsProvider.AUTHORITY, PARENT_DOCUMENT_ID, DOCUMENT_ID);
-        assertNull(DocumentsContract.findPath(mResolver, docUri));
-    }
-
-    public void testFindPath_treeUri_throwsOnDifferentParentDocId() throws Exception {
-        mProvider.nextIsChildDocument = true;
-
-        mProvider.nextPath = new Path(
-                null, Arrays.asList(ANCESTOR_DOCUMENT_ID, PARENT_DOCUMENT_ID, DOCUMENT_ID));
-
-        final Uri docUri = buildTreeDocumentUri(
-                TestDocumentsProvider.AUTHORITY, PARENT_DOCUMENT_ID, DOCUMENT_ID);
-        assertNull(DocumentsContract.findPath(mResolver, docUri));
+        try (ContentProviderClient client =
+                     mResolver.acquireUnstableContentProviderClient(docUri)) {
+            Path path = DocumentsContract.findPath(client, docUri);
+            assertNull(path.getRootId());
+        }
     }
 
     private static Uri buildTreeDocumentUri(String authority, String parentDocId, String docId) {