Throw IllegalArgumentException for getContentResolverForUserId
Throwing FileNotFoundException was not the best choice, as all callers
of getContentResolverForUserId would have to handle it in their own way.
IllegalArgumentException makes sense as we are unable to find a content
resolver corresponding to the user-id embedded in the picker uri.
Make sure we are consistent in error handling for Picker uris with
MediaProvider
Bug: 202388743
Test: atest PickerUriResolverTest
Change-Id: I0d28f0f653d8a3a2fe3e23442f073e58a64c149a
diff --git a/src/com/android/providers/media/PickerUriResolver.java b/src/com/android/providers/media/PickerUriResolver.java
index a104068..c9bd138 100644
--- a/src/com/android/providers/media/PickerUriResolver.java
+++ b/src/com/android/providers/media/PickerUriResolver.java
@@ -93,7 +93,14 @@
checkUriPermission(uri, callingPid, callingUid);
- final ContentResolver resolver = getContentResolverForUserId(uri);
+ final ContentResolver resolver;
+ try {
+ resolver = getContentResolverForUserId(uri);
+ } catch (IllegalArgumentException e) {
+ // This is to be consistent with MediaProvider's response when a file is not found.
+ Log.e(TAG, "No item at " + uri, e);
+ throw new FileNotFoundException("No item at " + uri);
+ }
if (PickerDbFacade.isPickerDbEnabled()) {
if (canHandleUriInUser(uri)) {
return openPickerFile(uri);
@@ -115,7 +122,14 @@
throws FileNotFoundException {
checkUriPermission(uri, callingPid, callingUid);
- final ContentResolver resolver = getContentResolverForUserId(uri);
+ final ContentResolver resolver;
+ try {
+ resolver = getContentResolverForUserId(uri);
+ } catch (IllegalArgumentException e) {
+ // This is to be consistent with MediaProvider's response when a file is not found.
+ Log.e(TAG, "No item at " + uri, e);
+ throw new FileNotFoundException("No item at " + uri);
+ }
if (PickerDbFacade.isPickerDbEnabled()) {
if (canHandleUriInUser(uri)) {
return new AssetFileDescriptor(openPickerFile(uri), 0,
@@ -139,8 +153,10 @@
try {
return queryInternal(uri, projection, queryArgs, signal);
- } catch (FileNotFoundException e) {
- Log.d(TAG, "File not found for uri: " + uri, e);
+ } catch (IllegalArgumentException e) {
+ // This is to be consistent with MediaProvider, it returns an empty cursor if the row
+ // does not exist.
+ Log.e(TAG, "File not found for uri: " + uri, e);
return new MatrixCursor(projection == null ? new String[] {} : projection);
}
}
@@ -148,7 +164,7 @@
// TODO(b/191362529): Restrict projection values when we start querying picker db.
// Add PickerColumns and add checks for projection.
private Cursor queryInternal(Uri uri, String[] projection, Bundle queryArgs,
- CancellationSignal signal) throws FileNotFoundException {
+ CancellationSignal signal) {
final ContentResolver resolver = getContentResolverForUserId(uri);
if (PickerDbFacade.isPickerDbEnabled()) {
@@ -179,8 +195,6 @@
return getCursorString(cursor,
CloudMediaProviderContract.MediaColumns.MIME_TYPE);
}
- } catch (FileNotFoundException e) {
- throw new IllegalArgumentException(e.getMessage());
}
throw new IllegalArgumentException("Failed to getType for uri: " + uri);
@@ -334,13 +348,12 @@
}
@VisibleForTesting
- ContentResolver getContentResolverForUserId(Uri uri) throws FileNotFoundException {
- final UserId userId = UserId.of(UserHandle.of(getUserId(uri)));
+ ContentResolver getContentResolverForUserId(Uri uri) {
+ final UserId userId = UserId.of(UserHandle.of(getUserId(uri)));
try {
return userId.getContentResolver(mContext);
} catch (NameNotFoundException e) {
- throw new FileNotFoundException("File not found due to unavailable content resolver "
- + "for uri: " + uri + " ; error: " + e);
+ throw new IllegalArgumentException("Cannot find content resolver for uri: " + uri, e);
}
}
}
diff --git a/tests/src/com/android/providers/media/PickerUriResolverTest.java b/tests/src/com/android/providers/media/PickerUriResolverTest.java
index fb78ef3..491d9f2 100644
--- a/tests/src/com/android/providers/media/PickerUriResolverTest.java
+++ b/tests/src/com/android/providers/media/PickerUriResolverTest.java
@@ -392,9 +392,7 @@
fail("Invalid user specified in the picker uri: " + uri);
} catch (FileNotFoundException expected) {
// expected
- assertThat(expected.getMessage()).isEqualTo("File not found due to unavailable content"
- + " resolver for uri: " + uri
- + " ; error: android.content.pm.PackageManager$NameNotFoundException");
+ assertThat(expected.getMessage()).isEqualTo("No item at " + uri);
}
}
@@ -405,9 +403,7 @@
fail("Invalid user specified in the picker uri: " + uri);
} catch (FileNotFoundException expected) {
// expected
- assertThat(expected.getMessage()).isEqualTo("File not found due to unavailable content"
- + " resolver for uri: " + uri
- + " ; error: android.content.pm.PackageManager$NameNotFoundException");
+ assertThat(expected.getMessage()).isEqualTo("No item at " + uri);
}
}
@@ -424,9 +420,8 @@
fail("Invalid user specified in the picker uri: " + uri);
} catch (IllegalArgumentException expected) {
// expected
- assertThat(expected.getMessage()).isEqualTo("File not found due to unavailable "
- + "content resolver for uri: " + uri
- + " ; error: android.content.pm.PackageManager$NameNotFoundException");
+ assertThat(expected.getMessage()).isEqualTo("Cannot find content resolver for uri: "
+ + uri);
}
}