Iteration on findPath API.
* Extend this API to take tree URI
* Add toString(), equals() and hashCode() to Path
* Address Jeff's comments in ag/1513538
* Add unit tests for findPath
Bug: 30948740
Change-Id: Iaf852d0e40fae37623e9bb9ffa1c6fbe334c1b21
(cherry picked from commit d4ab7ade7171a4382ef4f61f2a5f078a17800e83)
diff --git a/core/java/android/provider/DocumentsContract.java b/core/java/android/provider/DocumentsContract.java
index 98371f4..a858324 100644
--- a/core/java/android/provider/DocumentsContract.java
+++ b/core/java/android/provider/DocumentsContract.java
@@ -19,6 +19,10 @@
import static android.net.TrafficStats.KB_IN_BYTES;
import static android.system.OsConstants.SEEK_SET;
+import static com.android.internal.util.Preconditions.checkArgument;
+import static com.android.internal.util.Preconditions.checkCollectionElementsNotNull;
+import static com.android.internal.util.Preconditions.checkCollectionNotEmpty;
+
import android.annotation.Nullable;
import android.content.ContentProviderClient;
import android.content.ContentResolver;
@@ -55,6 +59,7 @@
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.List;
+import java.util.Objects;
/**
* Defines the contract between a documents provider and the platform.
@@ -1311,21 +1316,26 @@
}
/**
- * Finds the canonical path to the root. Document id should be unique across
- * roots.
+ * 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.
*
- * @param documentUri uri of the document which path is requested.
- * @return the path to the root of the document, or {@code null} if failed.
- * @see DocumentsProvider#findPath(String)
+ * Document id should be unique across roots.
+ *
+ * @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.
+ * @see DocumentsProvider#findPath(String, String)
*
* {@hide}
*/
- public static Path findPath(ContentResolver resolver, Uri documentUri)
- throws RemoteException {
+ public static List<String> findPath(ContentResolver resolver, Uri treeUri) {
+ checkArgument(isTreeUri(treeUri), treeUri + " is not a tree uri.");
+
final ContentProviderClient client = resolver.acquireUnstableContentProviderClient(
- documentUri.getAuthority());
+ treeUri.getAuthority());
try {
- return findPath(client, documentUri);
+ return findPath(client, treeUri).getPath();
} catch (Exception e) {
Log.w(TAG, "Failed to find path", e);
return null;
@@ -1334,11 +1344,24 @@
}
}
- /** {@hide} */
- public static Path findPath(ContentProviderClient client, Uri documentUri)
- throws RemoteException {
+ /**
+ * 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.
+ *
+ * Document id should be unique across roots.
+ *
+ * @param uri uri of the document which path is requested. It can be either a
+ * plain document uri or a tree uri.
+ * @return the path of the document.
+ * @see DocumentsProvider#findPath(String, String)
+ *
+ * {@hide}
+ */
+ public static Path findPath(ContentProviderClient client, Uri uri) throws RemoteException {
final Bundle in = new Bundle();
- in.putParcelable(DocumentsContract.EXTRA_URI, documentUri);
+ in.putParcelable(DocumentsContract.EXTRA_URI, uri);
final Bundle out = client.call(METHOD_FIND_PATH, null, in);
@@ -1392,20 +1415,71 @@
*/
public static final class Path implements Parcelable {
- public final String mRootId;
- public final List<String> mPath;
+ private final @Nullable String mRootId;
+ private final List<String> mPath;
/**
* Creates a Path.
- * @param rootId the id of the root
- * @param path the list of document ids from the root document
- * at position 0 to the target document
+ *
+ * @param rootId the ID of the root. May be null.
+ * @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) {
+ checkCollectionNotEmpty(path, "path");
+ checkCollectionElementsNotNull(path, "path");
+
mRootId = rootId;
mPath = path;
}
+ /**
+ * Returns the root id or null if the calling package doesn't have
+ * permission to access root information.
+ */
+ public @Nullable String getRootId() {
+ return mRootId;
+ }
+
+ /**
+ * Returns the path. The path is trimmed to the top of tree if
+ * calling package doesn't have permission to access those
+ * documents.
+ */
+ public List<String> getPath() {
+ return mPath;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || !(o instanceof Path)) {
+ return false;
+ }
+ Path path = (Path) o;
+ return Objects.equals(mRootId, path.mRootId) &&
+ Objects.equals(mPath, path.mPath);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(mRootId, mPath);
+ }
+
+ @Override
+ public String toString() {
+ return new StringBuilder()
+ .append("DocumentsContract.Path{")
+ .append("rootId=")
+ .append(mRootId)
+ .append(", path=")
+ .append(mPath)
+ .append("}")
+ .toString();
+ }
+
@Override
public void writeToParcel(Parcel dest, int flags) {
dest.writeString(mRootId);
diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java
index 6234f6a..d75781b 100644
--- a/core/java/android/provider/DocumentsProvider.java
+++ b/core/java/android/provider/DocumentsProvider.java
@@ -36,6 +36,7 @@
import android.Manifest;
import android.annotation.CallSuper;
+import android.annotation.Nullable;
import android.content.ClipDescription;
import android.content.ContentProvider;
import android.content.ContentResolver;
@@ -54,8 +55,8 @@
import android.os.ParcelFileDescriptor;
import android.os.ParcelFileDescriptor.OnCloseListener;
import android.provider.DocumentsContract.Document;
-import android.provider.DocumentsContract.Root;
import android.provider.DocumentsContract.Path;
+import android.provider.DocumentsContract.Root;
import android.util.Log;
import libcore.io.IoUtils;
@@ -154,17 +155,7 @@
*/
@Override
public void attachInfo(Context context, ProviderInfo info) {
- mAuthority = info.authority;
-
- mMatcher = new UriMatcher(UriMatcher.NO_MATCH);
- mMatcher.addURI(mAuthority, "root", MATCH_ROOTS);
- mMatcher.addURI(mAuthority, "root/*", MATCH_ROOT);
- mMatcher.addURI(mAuthority, "root/*/recent", MATCH_RECENT);
- mMatcher.addURI(mAuthority, "root/*/search", MATCH_SEARCH);
- mMatcher.addURI(mAuthority, "document/*", MATCH_DOCUMENT);
- mMatcher.addURI(mAuthority, "document/*/children", MATCH_CHILDREN);
- mMatcher.addURI(mAuthority, "tree/*/document/*", MATCH_DOCUMENT_TREE);
- mMatcher.addURI(mAuthority, "tree/*/document/*/children", MATCH_CHILDREN_TREE);
+ registerAuthority(info.authority);
// Sanity check our setup
if (!info.exported) {
@@ -181,6 +172,28 @@
super.attachInfo(context, info);
}
+ /** {@hide} */
+ @Override
+ public void attachInfoForTesting(Context context, ProviderInfo info) {
+ registerAuthority(info.authority);
+
+ super.attachInfoForTesting(context, info);
+ }
+
+ private void registerAuthority(String authority) {
+ mAuthority = authority;
+
+ mMatcher = new UriMatcher(UriMatcher.NO_MATCH);
+ mMatcher.addURI(mAuthority, "root", MATCH_ROOTS);
+ mMatcher.addURI(mAuthority, "root/*", MATCH_ROOT);
+ mMatcher.addURI(mAuthority, "root/*/recent", MATCH_RECENT);
+ mMatcher.addURI(mAuthority, "root/*/search", MATCH_SEARCH);
+ mMatcher.addURI(mAuthority, "document/*", MATCH_DOCUMENT);
+ mMatcher.addURI(mAuthority, "document/*/children", MATCH_CHILDREN);
+ mMatcher.addURI(mAuthority, "tree/*/document/*", MATCH_DOCUMENT_TREE);
+ mMatcher.addURI(mAuthority, "tree/*/document/*/children", MATCH_CHILDREN_TREE);
+ }
+
/**
* Test if a document is descendant (child, grandchild, etc) from the given
* parent. For example, providers must implement this to support
@@ -326,23 +339,28 @@
}
/**
- * Finds the canonical path to the root for the requested document. If there are
- * more than one path to this document, return the most typical one.
+ * Finds the canonical path for the requested document. The path must start
+ * from the parent document if parentDocumentId is not null or the root document
+ * if parentDocumentId is null. If there are more than one path to this document,
+ * return the most typical one. Include both the parent document or root document
+ * and the requested document in the returned path.
*
- * <p>This API assumes that document id has enough info to infer the root.
- * Different roots should use different document id to refer to the same
+ * <p>This API assumes that document ID has enough info to infer the root.
+ * Different roots should use different document ID to refer to the same
* document.
*
- * @param documentId the document which path is requested.
- * @return the path of the requested document to the root, or null if
- * such operation is not supported.
+ * @param childDocumentId the document which path is requested.
+ * @param parentDocumentId the document with which path starts if not null, or
+ * null to indicate path to root 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.
*
* @hide
*/
- public Path findPath(String documentId)
+ public Path findPath(String childDocumentId, @Nullable String parentDocumentId)
throws FileNotFoundException {
- Log.w(TAG, "findPath is called on an unsupported provider.");
- return null;
+ throw new UnsupportedOperationException("findPath not supported.");
}
/**
@@ -897,9 +915,27 @@
// It's responsibility of the provider to revoke any grants, as the document may be
// still attached to another parents.
} else if (METHOD_FIND_PATH.equals(method)) {
- getContext().enforceCallingPermission(Manifest.permission.MANAGE_DOCUMENTS, null);
+ final boolean isTreeUri = isTreeUri(documentUri);
- final Path path = findPath(documentId);
+ if (isTreeUri) {
+ enforceReadPermissionInner(documentUri, getCallingPackage(), null);
+ } else {
+ getContext().enforceCallingPermission(Manifest.permission.MANAGE_DOCUMENTS, null);
+ }
+
+ final String parentDocumentId = isTreeUri
+ ? DocumentsContract.getTreeDocumentId(documentUri)
+ : null;
+
+ final 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.");
+ }
out.putParcelable(DocumentsContract.EXTRA_RESULT, path);
} else {
diff --git a/core/tests/coretests/src/android/provider/DocumentsProviderTest.java b/core/tests/coretests/src/android/provider/DocumentsProviderTest.java
new file mode 100644
index 0000000..0b4675c
--- /dev/null
+++ b/core/tests/coretests/src/android/provider/DocumentsProviderTest.java
@@ -0,0 +1,114 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.provider;
+
+import android.content.ContentProviderClient;
+import android.content.ContentResolver;
+import android.net.Uri;
+import android.provider.DocumentsContract.Path;
+import android.support.test.filters.SmallTest;
+import android.test.ProviderTestCase2;
+
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Unit tests for {@link DocumentsProvider}.
+ */
+@SmallTest
+public class DocumentsProviderTest extends ProviderTestCase2<TestDocumentsProvider> {
+
+ private static final String ROOT_ID = "rootId";
+ private static final String DOCUMENT_ID = "docId";
+ private static final String PARENT_DOCUMENT_ID = "parentDocId";
+ private static final String ANCESTOR_DOCUMENT_ID = "ancestorDocId";
+
+ private TestDocumentsProvider mProvider;
+
+ private ContentResolver mResolver;
+
+ public DocumentsProviderTest() {
+ super(TestDocumentsProvider.class, TestDocumentsProvider.AUTHORITY);
+ }
+
+ public void setUp() throws Exception {
+ super.setUp();
+
+ mProvider = getProvider();
+ mResolver = getMockContentResolver();
+ }
+
+ public void testFindPath_docUri() throws Exception {
+ final Path expected = new Path(ROOT_ID, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));
+ mProvider.nextPath = expected;
+
+ final Uri docUri =
+ DocumentsContract.buildDocumentUri(TestDocumentsProvider.AUTHORITY, DOCUMENT_ID);
+ try (ContentProviderClient client =
+ mResolver.acquireUnstableContentProviderClient(docUri)) {
+ final Path actual = DocumentsContract.findPath(client, docUri);
+ assertEquals(expected, actual);
+ }
+ }
+
+ public void testFindPath_treeUri() throws Exception {
+ mProvider.nextIsChildDocument = true;
+
+ final Path expected = new Path(null, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));
+ mProvider.nextPath = expected;
+
+ final Uri docUri = buildTreeDocumentUri(
+ TestDocumentsProvider.AUTHORITY, PARENT_DOCUMENT_ID, DOCUMENT_ID);
+ final List<String> actual = DocumentsContract.findPath(mResolver, docUri);
+
+ assertEquals(expected.getPath(), actual);
+ }
+
+ public void testFindPath_treeUri_throwsOnNonChildDocument() throws Exception {
+ mProvider.nextPath = new Path(null, 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_throwsOnNonNullRootId() 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));
+ }
+
+ private static Uri buildTreeDocumentUri(String authority, String parentDocId, String docId) {
+ final Uri treeUri = DocumentsContract.buildTreeDocumentUri(authority, parentDocId);
+ return DocumentsContract.buildDocumentUriUsingTree(treeUri, docId);
+ }
+}
diff --git a/core/tests/coretests/src/android/provider/TestDocumentsProvider.java b/core/tests/coretests/src/android/provider/TestDocumentsProvider.java
new file mode 100644
index 0000000..8dcf566
--- /dev/null
+++ b/core/tests/coretests/src/android/provider/TestDocumentsProvider.java
@@ -0,0 +1,125 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.provider;
+
+import android.annotation.Nullable;
+import android.app.AppOpsManager;
+import android.content.Context;
+import android.content.ContextWrapper;
+import android.content.pm.ProviderInfo;
+import android.database.Cursor;
+import android.net.Uri;
+import android.os.CancellationSignal;
+import android.os.IBinder;
+import android.os.ParcelFileDescriptor;
+import android.provider.DocumentsContract.Path;
+
+import org.mockito.Mockito;
+
+import java.io.FileNotFoundException;
+
+/**
+ * Provides a test double of {@link DocumentsProvider}.
+ */
+public class TestDocumentsProvider extends DocumentsProvider {
+ public static final String AUTHORITY = "android.provider.TestDocumentsProvider";
+
+ public Path nextPath;
+
+ public boolean nextIsChildDocument;
+
+ public String lastDocumentId;
+ public String lastParentDocumentId;
+
+ @Override
+ public void attachInfoForTesting(Context context, ProviderInfo info) {
+ context = new TestContext(context);
+ super.attachInfoForTesting(context, info);
+ }
+
+ @Override
+ public boolean onCreate() {
+ return true;
+ }
+
+ @Override
+ public Cursor queryRoots(String[] projection) throws FileNotFoundException {
+ return null;
+ }
+
+ @Override
+ public Cursor queryDocument(String documentId, String[] projection)
+ throws FileNotFoundException {
+ return null;
+ }
+
+ @Override
+ public Cursor queryChildDocuments(String parentDocumentId, String[] projection,
+ String sortOrder) throws FileNotFoundException {
+ return null;
+ }
+
+ @Override
+ public ParcelFileDescriptor openDocument(String documentId, String mode,
+ CancellationSignal signal) throws FileNotFoundException {
+ return null;
+ }
+
+ @Override
+ public boolean isChildDocument(String parentDocumentId, String documentId) {
+ return nextIsChildDocument;
+ }
+
+ @Override
+ public Path findPath(String documentId, @Nullable String parentDocumentId) {
+ lastDocumentId = documentId;
+ lastParentDocumentId = parentDocumentId;
+
+ return nextPath;
+ }
+
+ @Override
+ protected int enforceReadPermissionInner(Uri uri, String callingPkg, IBinder callerToken) {
+ return AppOpsManager.MODE_ALLOWED;
+ }
+
+ @Override
+ protected int enforceWritePermissionInner(Uri uri, String callingPkg, IBinder callerToken) {
+ return AppOpsManager.MODE_ALLOWED;
+ }
+
+ private static class TestContext extends ContextWrapper {
+
+ private TestContext(Context context) {
+ super(context);
+ }
+
+ @Override
+ public void enforceCallingPermission(String permission, String message) {
+ // Always granted
+ }
+
+ @Override
+ public Object getSystemService(String name) {
+ if (Context.APP_OPS_SERVICE.equals(name)) {
+ return Mockito.mock(AppOpsManager.class);
+ }
+
+ return super.getSystemService(name);
+ }
+ }
+}
diff --git a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
index 3b575a8..662a1cd 100644
--- a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
+++ b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
@@ -16,6 +16,7 @@
package com.android.externalstorage;
+import android.annotation.Nullable;
import android.content.ContentResolver;
import android.content.Context;
import android.content.Intent;
@@ -40,8 +41,8 @@
import android.os.storage.VolumeInfo;
import android.provider.DocumentsContract;
import android.provider.DocumentsContract.Document;
-import android.provider.DocumentsContract.Root;
import android.provider.DocumentsContract.Path;
+import android.provider.DocumentsContract.Root;
import android.provider.DocumentsProvider;
import android.provider.MediaStore;
import android.provider.Settings;
@@ -325,14 +326,19 @@
}
private File getFileForDocId(String docId, boolean visible) throws FileNotFoundException {
- return resolveDocId(docId, visible).second;
+ RootInfo root = getRootFromDocId(docId);
+ return buildFile(root, docId, visible);
}
private Pair<RootInfo, File> resolveDocId(String docId, boolean visible)
throws FileNotFoundException {
+ RootInfo root = getRootFromDocId(docId);
+ return Pair.create(root, buildFile(root, docId, visible));
+ }
+
+ private RootInfo getRootFromDocId(String docId) throws FileNotFoundException {
final int splitIndex = docId.indexOf(':', 1);
final String tag = docId.substring(0, splitIndex);
- final String path = docId.substring(splitIndex + 1);
RootInfo root;
synchronized (mRootsLock) {
@@ -342,6 +348,14 @@
throw new FileNotFoundException("No root for " + tag);
}
+ return root;
+ }
+
+ private File buildFile(RootInfo root, String docId, boolean visible)
+ throws FileNotFoundException {
+ final int splitIndex = docId.indexOf(':', 1);
+ final String path = docId.substring(splitIndex + 1);
+
File target = visible ? root.visiblePath : root.path;
if (target == null) {
return null;
@@ -353,7 +367,7 @@
if (!target.exists()) {
throw new FileNotFoundException("Missing file for " + docId + " at " + target);
}
- return Pair.create(root, target);
+ return target;
}
private void includeFile(MatrixCursor result, String docId, File file)
@@ -430,25 +444,33 @@
}
@Override
- public Path findPath(String documentId)
+ public Path findPath(String childDocId, @Nullable String parentDocId)
throws FileNotFoundException {
LinkedList<String> path = new LinkedList<>();
- final Pair<RootInfo, File> resolvedDocId = resolveDocId(documentId, false);
- RootInfo root = resolvedDocId.first;
- File file = resolvedDocId.second;
+ final Pair<RootInfo, File> resolvedDocId = resolveDocId(childDocId, false);
+ final RootInfo root = resolvedDocId.first;
+ File child = resolvedDocId.second;
- if (!file.exists()) {
- throw new FileNotFoundException();
+ final File parent = TextUtils.isEmpty(parentDocId)
+ ? root.path
+ : getFileForDocId(parentDocId);
+
+ if (!child.exists()) {
+ throw new FileNotFoundException(childDocId + " is not found.");
}
- while (file != null && file.getAbsolutePath().startsWith(root.path.getAbsolutePath())) {
- path.addFirst(getDocIdForFile(file));
-
- file = file.getParentFile();
+ if (!child.getAbsolutePath().startsWith(parent.getAbsolutePath())) {
+ throw new FileNotFoundException(childDocId + " is not found under " + parentDocId);
}
- return new Path(root.rootId, path);
+ while (child != null && child.getAbsolutePath().startsWith(parent.getAbsolutePath())) {
+ path.addFirst(getDocIdForFile(child));
+
+ child = child.getParentFile();
+ }
+
+ return new Path(parentDocId == null ? root.rootId : null, path);
}
@Override