Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5f214807 authored by Garfield Tan's avatar Garfield Tan
Browse files

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)
parent fbee823e
Loading
Loading
Loading
Loading
+1 −1
Original line number Original line Diff line number Diff line
@@ -1412,7 +1412,7 @@ public final class DocumentsContract {
         * @param path the list of document ids from the parent document at
         * @param path the list of document ids from the parent document at
         *          position 0 to the child document.
         *          position 0 to the child document.
         */
         */
        public Path(String rootId, List<String> path) {
        public Path(@Nullable String rootId, List<String> path) {
            checkCollectionNotEmpty(path, "path");
            checkCollectionNotEmpty(path, "path");
            checkCollectionElementsNotNull(path, "path");
            checkCollectionElementsNotNull(path, "path");


+12 −6
Original line number Original line Diff line number Diff line
@@ -927,14 +927,20 @@ public abstract class DocumentsProvider extends ContentProvider {
                    ? DocumentsContract.getTreeDocumentId(documentUri)
                    ? DocumentsContract.getTreeDocumentId(documentUri)
                    : null;
                    : null;


            final Path path = findPath(documentId, parentDocumentId);
            Path path = findPath(documentId, parentDocumentId);


            // Ensure provider doesn't leak information to unprivileged callers.
            // Ensure provider doesn't leak information to unprivileged callers.
            if (isTreeUri
            if (isTreeUri) {
                    && (path.getRootId() != null
                if (!Objects.equals(path.getPath().get(0), parentDocumentId)) {
                        || !Objects.equals(path.getPath().get(0), parentDocumentId))) {
                    Log.wtf(TAG, "Provider doesn't return path from the tree root. Expected: "
                throw new IllegalStateException(
                            + parentDocumentId + " found: " + path.getPath().get(0));
                        "Provider returns an invalid result for findPath.");
                }

                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);
            out.putParcelable(DocumentsContract.EXTRA_RESULT, path);
+6 −13
Original line number Original line Diff line number Diff line
@@ -86,25 +86,18 @@ public class DocumentsProviderTest extends ProviderTestCase2<TestDocumentsProvid
        assertNull(DocumentsContract.findPath(mResolver, docUri));
        assertNull(DocumentsContract.findPath(mResolver, docUri));
    }
    }


    public void testFindPath_treeUri_throwsOnNonNullRootId() throws Exception {
    public void testFindPath_treeUri_erasesNonNullRootId() throws Exception {
        mProvider.nextIsChildDocument = true;
        mProvider.nextIsChildDocument = true;


        mProvider.nextPath = new Path(ROOT_ID, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));
        mProvider.nextPath = new Path(ROOT_ID, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));


        final Uri docUri = buildTreeDocumentUri(
        final Uri docUri = buildTreeDocumentUri(
                TestDocumentsProvider.AUTHORITY, PARENT_DOCUMENT_ID, DOCUMENT_ID);
                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());
        }
        }

    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));
    }
    }


    private static Uri buildTreeDocumentUri(String authority, String parentDocId, String docId) {
    private static Uri buildTreeDocumentUri(String authority, String parentDocId, String docId) {