From b690b4de06385a821aed3442e10058986c03badc Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Wed, 1 Mar 2017 16:05:23 -0800 Subject: Address comments from API council. Test: Code builds and tests pass. Also some manual tests around ESP. Bug: 35813037 Bug: 35812990 Change-Id: Ia9d3a3964e9a83d0c1c08e5db4c2e231504aa99a --- api/current.txt | 4 +-- api/system-current.txt | 4 +-- api/test-current.txt | 4 +-- core/java/android/provider/DocumentsContract.java | 29 ++++++++++--------- core/java/android/provider/DocumentsProvider.java | 13 +++++++-- .../android/provider/DocumentsProviderTest.java | 33 ++++++++++++++-------- .../android/provider/TestDocumentsProvider.java | 2 +- .../externalstorage/ExternalStorageProvider.java | 17 +---------- .../src/com/android/mtp/MtpDocumentsProvider.java | 2 +- .../com/android/mtp/MtpDocumentsProviderTest.java | 9 +++--- 10 files changed, 61 insertions(+), 56 deletions(-) diff --git a/api/current.txt b/api/current.txt index 0872b2d7d707..aff5e8e7ba73 100644 --- a/api/current.txt +++ b/api/current.txt @@ -30125,12 +30125,12 @@ package android.os { field public static final int BATTERY_PLUGGED_AC = 1; // 0x1 field public static final int BATTERY_PLUGGED_USB = 2; // 0x2 field public static final int BATTERY_PLUGGED_WIRELESS = 4; // 0x4 - field public static final int BATTERY_PROPERTY_STATUS = 6; // 0x6 field public static final int BATTERY_PROPERTY_CAPACITY = 4; // 0x4 field public static final int BATTERY_PROPERTY_CHARGE_COUNTER = 1; // 0x1 field public static final int BATTERY_PROPERTY_CURRENT_AVERAGE = 3; // 0x3 field public static final int BATTERY_PROPERTY_CURRENT_NOW = 2; // 0x2 field public static final int BATTERY_PROPERTY_ENERGY_COUNTER = 5; // 0x5 + field public static final int BATTERY_PROPERTY_STATUS = 6; // 0x6 field public static final int BATTERY_STATUS_CHARGING = 2; // 0x2 field public static final int BATTERY_STATUS_DISCHARGING = 3; // 0x3 field public static final int BATTERY_STATUS_FULL = 5; // 0x5 @@ -33862,7 +33862,7 @@ package android.provider { method public static android.net.Uri createDocument(android.content.ContentResolver, android.net.Uri, java.lang.String, java.lang.String); method public static android.content.IntentSender createWebLinkIntent(android.content.ContentResolver, android.net.Uri, android.os.Bundle); method public static boolean deleteDocument(android.content.ContentResolver, android.net.Uri); - method public static java.util.List findDocumentPath(android.content.ContentResolver, android.net.Uri); + method public static android.provider.DocumentsContract.Path findDocumentPath(android.content.ContentResolver, android.net.Uri); method public static java.lang.String getDocumentId(android.net.Uri); method public static android.graphics.Bitmap getDocumentThumbnail(android.content.ContentResolver, android.net.Uri, android.graphics.Point, android.os.CancellationSignal); method public static java.lang.String getRootId(android.net.Uri); diff --git a/api/system-current.txt b/api/system-current.txt index c321e08619c8..f6a9eda0041b 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -32726,12 +32726,12 @@ package android.os { field public static final int BATTERY_PLUGGED_AC = 1; // 0x1 field public static final int BATTERY_PLUGGED_USB = 2; // 0x2 field public static final int BATTERY_PLUGGED_WIRELESS = 4; // 0x4 - field public static final int BATTERY_PROPERTY_STATUS = 6; // 0x6 field public static final int BATTERY_PROPERTY_CAPACITY = 4; // 0x4 field public static final int BATTERY_PROPERTY_CHARGE_COUNTER = 1; // 0x1 field public static final int BATTERY_PROPERTY_CURRENT_AVERAGE = 3; // 0x3 field public static final int BATTERY_PROPERTY_CURRENT_NOW = 2; // 0x2 field public static final int BATTERY_PROPERTY_ENERGY_COUNTER = 5; // 0x5 + field public static final int BATTERY_PROPERTY_STATUS = 6; // 0x6 field public static final int BATTERY_STATUS_CHARGING = 2; // 0x2 field public static final int BATTERY_STATUS_DISCHARGING = 3; // 0x3 field public static final int BATTERY_STATUS_FULL = 5; // 0x5 @@ -36657,7 +36657,7 @@ package android.provider { method public static android.net.Uri createDocument(android.content.ContentResolver, android.net.Uri, java.lang.String, java.lang.String); method public static android.content.IntentSender createWebLinkIntent(android.content.ContentResolver, android.net.Uri, android.os.Bundle); method public static boolean deleteDocument(android.content.ContentResolver, android.net.Uri); - method public static java.util.List findDocumentPath(android.content.ContentResolver, android.net.Uri); + method public static android.provider.DocumentsContract.Path findDocumentPath(android.content.ContentResolver, android.net.Uri); method public static java.lang.String getDocumentId(android.net.Uri); method public static android.graphics.Bitmap getDocumentThumbnail(android.content.ContentResolver, android.net.Uri, android.graphics.Point, android.os.CancellationSignal); method public static java.lang.String getRootId(android.net.Uri); diff --git a/api/test-current.txt b/api/test-current.txt index 584903293669..db8fb0100b67 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -30221,12 +30221,12 @@ package android.os { field public static final int BATTERY_PLUGGED_AC = 1; // 0x1 field public static final int BATTERY_PLUGGED_USB = 2; // 0x2 field public static final int BATTERY_PLUGGED_WIRELESS = 4; // 0x4 - field public static final int BATTERY_PROPERTY_STATUS = 6; // 0x6 field public static final int BATTERY_PROPERTY_CAPACITY = 4; // 0x4 field public static final int BATTERY_PROPERTY_CHARGE_COUNTER = 1; // 0x1 field public static final int BATTERY_PROPERTY_CURRENT_AVERAGE = 3; // 0x3 field public static final int BATTERY_PROPERTY_CURRENT_NOW = 2; // 0x2 field public static final int BATTERY_PROPERTY_ENERGY_COUNTER = 5; // 0x5 + field public static final int BATTERY_PROPERTY_STATUS = 6; // 0x6 field public static final int BATTERY_STATUS_CHARGING = 2; // 0x2 field public static final int BATTERY_STATUS_DISCHARGING = 3; // 0x3 field public static final int BATTERY_STATUS_FULL = 5; // 0x5 @@ -33984,7 +33984,7 @@ package android.provider { method public static android.net.Uri createDocument(android.content.ContentResolver, android.net.Uri, java.lang.String, java.lang.String); method public static android.content.IntentSender createWebLinkIntent(android.content.ContentResolver, android.net.Uri, android.os.Bundle); method public static boolean deleteDocument(android.content.ContentResolver, android.net.Uri); - method public static java.util.List findDocumentPath(android.content.ContentResolver, android.net.Uri); + method public static android.provider.DocumentsContract.Path findDocumentPath(android.content.ContentResolver, android.net.Uri); method public static java.lang.String getDocumentId(android.net.Uri); method public static android.graphics.Bitmap getDocumentThumbnail(android.content.ContentResolver, android.net.Uri, android.graphics.Point, android.os.CancellationSignal); method public static java.lang.String getRootId(android.net.Uri); diff --git a/core/java/android/provider/DocumentsContract.java b/core/java/android/provider/DocumentsContract.java index 71a0349bce54..c1bd0320f4f2 100644 --- a/core/java/android/provider/DocumentsContract.java +++ b/core/java/android/provider/DocumentsContract.java @@ -1364,24 +1364,25 @@ public final class DocumentsContract { } /** - * Finds the canonical path to the top of the tree. The return value starts - * from the top of the tree or the root document to the requested document, - * both inclusive. + * Finds the canonical path from the top of the document tree. + * + * The {@link Path#getPath()} of the return value contains the document ID + * of all documents along the path from the top the document tree to the + * requested document, both inclusive. * - * Document ID should be unique across roots. + * The {@link Path#getRootId()} of the return value returns {@code null}. * * @param treeUri treeUri of the document which path is requested. - * @return a list of documents ID starting from the top of the tree to the - * requested document, or {@code null} if failed. + * @return the path of the document, or {@code null} if failed. * @see DocumentsProvider#findDocumentPath(String, String) */ - public static List findDocumentPath(ContentResolver resolver, Uri treeUri) { + public static Path findDocumentPath(ContentResolver resolver, Uri treeUri) { checkArgument(isTreeUri(treeUri), treeUri + " is not a tree uri."); final ContentProviderClient client = resolver.acquireUnstableContentProviderClient( treeUri.getAuthority()); try { - return findDocumentPath(client, treeUri).getPath(); + return findDocumentPath(client, treeUri); } catch (Exception e) { Log.w(TAG, "Failed to find path", e); return null; @@ -1391,12 +1392,14 @@ public final class DocumentsContract { } /** - * Finds the canonical path. If uri is a document uri returns path to a root and - * its associated root id. If uri is a tree uri returns the path to the top of - * the tree. The {@link Path#getPath()} in the return value starts from the top of - * the tree or the root document to the requested document, both inclusive. + * Finds the canonical path. If uri is a document uri returns path from a root and + * its associated root id. If uri is a tree uri returns the path from the top of + * the tree. The {@link Path#getPath()} of the return value contains document ID + * starts from the top of the tree or the root document to the requested document, + * both inclusive. * - * Document id should be unique across roots. + * Callers can expect the root ID returned from multiple calls to this method is + * consistent. * * @param uri uri of the document which path is requested. It can be either a * plain document uri or a tree uri. diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java index f5e558a01c56..89a80f054ded 100644 --- a/core/java/android/provider/DocumentsProvider.java +++ b/core/java/android/provider/DocumentsProvider.java @@ -65,6 +65,7 @@ import android.util.Log; import libcore.io.IoUtils; import java.io.FileNotFoundException; +import java.util.LinkedList; import java.util.Objects; /** @@ -352,14 +353,14 @@ public abstract class DocumentsProvider extends ContentProvider { * Different roots should use different document ID to refer to the same * document. * - * @param childDocumentId the document which path is requested. * @param parentDocumentId the document from which the path starts if not null, * or null to indicate a path from the root is requested. + * @param childDocumentId the document which path is requested. * @return the path of the requested document. If parentDocumentId is null * returned root ID must not be null. If parentDocumentId is not null * returned root ID must be null. */ - public Path findDocumentPath(String childDocumentId, @Nullable String parentDocumentId) + public Path findDocumentPath(@Nullable String parentDocumentId, String childDocumentId) throws FileNotFoundException { throw new UnsupportedOperationException("findDocumentPath not supported."); } @@ -1048,13 +1049,19 @@ public abstract class DocumentsProvider extends ContentProvider { ? DocumentsContract.getTreeDocumentId(documentUri) : null; - Path path = findDocumentPath(documentId, parentDocumentId); + Path path = findDocumentPath(parentDocumentId, documentId); // Ensure provider doesn't leak information to unprivileged callers. 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)); + + LinkedList docs = new LinkedList<>(path.getPath()); + while (docs.size() > 1 && !Objects.equals(docs.getFirst(), parentDocumentId)) { + docs.removeFirst(); + } + path = new Path(null, docs); } if (path.getRootId() != null) { diff --git a/core/tests/coretests/src/android/provider/DocumentsProviderTest.java b/core/tests/coretests/src/android/provider/DocumentsProviderTest.java index d1c68a92b6e7..09cbbff9077f 100644 --- a/core/tests/coretests/src/android/provider/DocumentsProviderTest.java +++ b/core/tests/coretests/src/android/provider/DocumentsProviderTest.java @@ -52,7 +52,7 @@ public class DocumentsProviderTest extends ProviderTestCase2 actual = DocumentsContract.findDocumentPath(mResolver, docUri); + final Path actual = DocumentsContract.findDocumentPath(mResolver, docUri); - assertEquals(expected.getPath(), actual); + assertNull(actual.getRootId()); + assertEquals(expected.getPath(), actual.getPath()); } - public void testFindPath_treeUri_throwsOnNonChildDocument() throws Exception { + public void testFindDocumentPath_treeUri_throwsOnNonChildDocument() throws Exception { mProvider.nextPath = new Path(null, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID)); final Uri docUri = buildTreeDocumentUri( @@ -86,18 +87,28 @@ public class DocumentsProviderTest extends ProviderTestCase2 resolvedDocId = resolveDocId(childDocId, false); final RootInfo root = resolvedDocId.first; diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java index b8c10a6a3366..e60b5a9a373a 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java @@ -435,7 +435,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { } @Override - public Path findDocumentPath(String childDocumentId, String parentDocumentId) + public Path findDocumentPath(String parentDocumentId, String childDocumentId) throws FileNotFoundException { final LinkedList ids = new LinkedList<>(); final Identifier childIdentifier = mDatabase.createIdentifier(childDocumentId); diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java index 491e24deefa2..29783e4165c7 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java @@ -35,7 +35,6 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.util.Arrays; import java.util.LinkedList; -import java.util.Objects; import java.util.Queue; import java.util.concurrent.TimeoutException; @@ -802,7 +801,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { setupRoots(0, new MtpRoot[] { new MtpRoot(0, 0, "Storage", 1000, 1000, "") }); setupHierarchyDocuments("1"); - final Path path = mProvider.findDocumentPath("15", null); + final Path path = mProvider.findDocumentPath(null, "15"); assertEquals("1", path.getRootId()); assertEquals(4, path.getPath().size()); assertEquals("1", path.getPath().get(0)); @@ -816,7 +815,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { setupRoots(0, new MtpRoot[] { new MtpRoot(0, 0, "Storage", 1000, 1000, "") }); setupHierarchyDocuments("1"); - final Path path = mProvider.findDocumentPath("18", "3"); + final Path path = mProvider.findDocumentPath("3", "18"); assertNull(path.getRootId()); assertEquals(3, path.getPath().size()); assertEquals("3", path.getPath().get(0)); @@ -831,7 +830,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { new MtpRoot(0, 1, "Storage B", 1000, 1000, "") }); setupHierarchyDocuments("2"); - final Path path = mProvider.findDocumentPath("16", null); + final Path path = mProvider.findDocumentPath(null, "16"); assertEquals("2", path.getRootId()); assertEquals(4, path.getPath().size()); assertEquals("2", path.getPath().get(0)); @@ -847,7 +846,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { new MtpRoot(0, 1, "Storage B", 1000, 1000, "") }); setupHierarchyDocuments("2"); - final Path path = mProvider.findDocumentPath("19", "4"); + final Path path = mProvider.findDocumentPath("4", "19"); assertNull(path.getRootId()); assertEquals(3, path.getPath().size()); assertEquals("4", path.getPath().get(0)); -- cgit v1.2.3-59-g8ed1b