Merge "Refactor duplicate code handling removed uid entries and add tests."
diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index 550c8fb..5d39d56 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -22,17 +22,14 @@
import static android.provider.Downloads.Impl.COLUMN_MEDIASTORE_URI;
import static android.provider.Downloads.Impl.COLUMN_MEDIA_SCANNED;
import static android.provider.Downloads.Impl.COLUMN_OTHER_UID;
-import static android.provider.Downloads.Impl.DESTINATION_EXTERNAL;
import static android.provider.Downloads.Impl.DESTINATION_FILE_URI;
import static android.provider.Downloads.Impl.DESTINATION_NON_DOWNLOADMANAGER_DOWNLOAD;
import static android.provider.Downloads.Impl.MEDIA_NOT_SCANNABLE;
import static android.provider.Downloads.Impl.MEDIA_NOT_SCANNED;
import static android.provider.Downloads.Impl.MEDIA_SCANNED;
import static android.provider.Downloads.Impl.PERMISSION_ACCESS_ALL;
-import static android.provider.Downloads.Impl._DATA;
import android.annotation.NonNull;
-import android.annotation.Nullable;
import android.app.AppOpsManager;
import android.app.DownloadManager;
import android.app.DownloadManager.Request;
@@ -50,7 +47,6 @@
import android.database.Cursor;
import android.database.DatabaseUtils;
import android.database.SQLException;
-import android.database.TranslatingCursor;
import android.database.sqlite.SQLiteDatabase;
import android.database.sqlite.SQLiteOpenHelper;
import android.database.sqlite.SQLiteQueryBuilder;
@@ -59,7 +55,6 @@
import android.os.Build;
import android.os.Bundle;
import android.os.Environment;
-import android.os.FileUtils;
import android.os.ParcelFileDescriptor;
import android.os.ParcelFileDescriptor.OnCloseListener;
import android.os.Process;
@@ -71,11 +66,7 @@
import android.provider.OpenableColumns;
import android.text.TextUtils;
import android.text.format.DateUtils;
-import android.util.ArrayMap;
import android.util.Log;
-import android.util.LongArray;
-import android.util.LongSparseArray;
-import android.util.SparseArray;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.IndentingPrintWriter;
@@ -91,7 +82,6 @@
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.PrintWriter;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
@@ -565,36 +555,7 @@
}
private void reconcileRemovedUidEntries() {
- // Grant access permissions for all known downloads to the owning apps
- final ArrayList<Long> idsToDelete = new ArrayList<>();
- final ArrayList<Long> idsToOrphan = new ArrayList<>();
- final LongSparseArray<String> idsToGrantPermission = new LongSparseArray<>();
- final SQLiteDatabase db = mOpenHelper.getReadableDatabase();
- try (Cursor cursor = db.query(DB_TABLE,
- new String[] { Downloads.Impl._ID, Constants.UID, COLUMN_DESTINATION, _DATA },
- Constants.UID + " IS NOT NULL", null, null, null, null)) {
- Helpers.handleRemovedUidEntries(getContext(), cursor,
- idsToDelete, idsToOrphan, idsToGrantPermission);
- }
- for (int i = 0; i < idsToGrantPermission.size(); ++i) {
- final long downloadId = idsToGrantPermission.keyAt(i);
- final String ownerPackageName = idsToGrantPermission.valueAt(i);
- grantAllDownloadsPermission(ownerPackageName, downloadId);
- }
- if (idsToOrphan.size() > 0) {
- Log.i(Constants.TAG, "Orphaning downloads with ids "
- + Arrays.toString(idsToOrphan.toArray()) + " as owner package is missing");
- final ContentValues values = new ContentValues();
- values.putNull(Constants.UID);
- update(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, values,
- Helpers.buildQueryWithIds(idsToOrphan), null);
- }
- if (idsToDelete.size() > 0) {
- Log.i(Constants.TAG, "Deleting downloads with ids "
- + Arrays.toString(idsToDelete.toArray()) + " as owner package is missing");
- delete(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
- Helpers.buildQueryWithIds(idsToDelete), null);
- }
+ Helpers.handleRemovedUidEntries(getContext(), this, -1, this::grantAllDownloadsPermission);
}
/**
diff --git a/src/com/android/providers/downloads/DownloadReceiver.java b/src/com/android/providers/downloads/DownloadReceiver.java
index f0b9de7..8038500 100644
--- a/src/com/android/providers/downloads/DownloadReceiver.java
+++ b/src/com/android/providers/downloads/DownloadReceiver.java
@@ -18,8 +18,7 @@
import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED;
import static android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_ONLY_COMPLETION;
-import static android.provider.Downloads.Impl.COLUMN_DESTINATION;
-import static android.provider.Downloads.Impl._DATA;
+import static android.provider.Downloads.Impl.AUTHORITY;
import static com.android.providers.downloads.Constants.TAG;
import static com.android.providers.downloads.Helpers.getAsyncHandler;
@@ -31,6 +30,7 @@
import android.app.DownloadManager;
import android.app.NotificationManager;
import android.content.BroadcastReceiver;
+import android.content.ContentProviderClient;
import android.content.ContentResolver;
import android.content.ContentUris;
import android.content.ContentValues;
@@ -39,16 +39,10 @@
import android.database.Cursor;
import android.net.Uri;
import android.provider.Downloads;
-import android.provider.MediaStore;
import android.text.TextUtils;
import android.util.Log;
-import android.util.Slog;
import android.widget.Toast;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.regex.Pattern;
-
/**
* Receives system broadcasts (boot, network connectivity)
*/
@@ -145,30 +139,14 @@
}
private void handleUidRemoved(Context context, Intent intent) {
- final ContentResolver resolver = context.getContentResolver();
final int uid = intent.getIntExtra(Intent.EXTRA_UID, -1);
-
- final ArrayList<Long> idsToDelete = new ArrayList<>();
- final ArrayList<Long> idsToOrphan = new ArrayList<>();
- try (Cursor cursor = resolver.query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
- new String[] { Downloads.Impl._ID, Constants.UID, COLUMN_DESTINATION, _DATA },
- Constants.UID + "=" + uid, null, null)) {
- Helpers.handleRemovedUidEntries(context, cursor, idsToDelete, idsToOrphan, null);
+ if (uid == -1) {
+ return;
}
- if (idsToOrphan.size() > 0) {
- Log.i(Constants.TAG, "Orphaning downloads with ids "
- + Arrays.toString(idsToOrphan.toArray()) + " as owner package is removed");
- final ContentValues values = new ContentValues();
- values.putNull(Constants.UID);
- resolver.update(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, values,
- Helpers.buildQueryWithIds(idsToOrphan), null);
- }
- if (idsToDelete.size() > 0) {
- Log.i(Constants.TAG, "Deleting downloads with ids "
- + Arrays.toString(idsToDelete.toArray()) + " as owner package is removed");
- resolver.delete(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
- Helpers.buildQueryWithIds(idsToDelete), null);
+ try (ContentProviderClient cpc = context.getContentResolver()
+ .acquireContentProviderClient(AUTHORITY)) {
+ Helpers.handleRemovedUidEntries(context, cpc.getLocalContentProvider(), uid, null);
}
}
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 7b4cb4e..7ce269f 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -20,11 +20,13 @@
import static android.os.Environment.buildExternalStorageAppMediaDirs;
import static android.os.Environment.buildExternalStorageAppObbDirs;
import static android.os.Environment.buildExternalStoragePublicDirs;
+import static android.provider.Downloads.Impl.COLUMN_DESTINATION;
import static android.provider.Downloads.Impl.DESTINATION_EXTERNAL;
import static android.provider.Downloads.Impl.DESTINATION_FILE_URI;
import static android.provider.Downloads.Impl.DESTINATION_NON_DOWNLOADMANAGER_DOWNLOAD;
import static android.provider.Downloads.Impl.FLAG_REQUIRES_CHARGING;
import static android.provider.Downloads.Impl.FLAG_REQUIRES_DEVICE_IDLE;
+import static android.provider.Downloads.Impl._DATA;
import static com.android.providers.downloads.Constants.TAG;
@@ -33,6 +35,8 @@
import android.app.job.JobInfo;
import android.app.job.JobScheduler;
import android.content.ComponentName;
+import android.content.ContentProvider;
+import android.content.ContentValues;
import android.content.Context;
import android.database.Cursor;
import android.net.Uri;
@@ -48,20 +52,19 @@
import android.provider.Downloads;
import android.text.TextUtils;
import android.util.Log;
-import android.util.LongSparseArray;
import android.util.SparseArray;
-import android.util.SparseBooleanArray;
import android.webkit.MimeTypeMap;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.ArrayUtils;
-import com.google.common.annotations.VisibleForTesting;
-
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Random;
import java.util.Set;
+import java.util.function.BiConsumer;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -626,39 +629,63 @@
}
}
- public static void handleRemovedUidEntries(@NonNull Context context, @NonNull Cursor cursor,
- @NonNull ArrayList<Long> idsToDelete, @NonNull ArrayList<Long> idsToOrphan,
- @Nullable LongSparseArray<String> idsToGrantPermission) {
+ @VisibleForTesting
+ public static void handleRemovedUidEntries(@NonNull Context context,
+ @NonNull ContentProvider downloadProvider, int removedUid,
+ @Nullable BiConsumer<String, Long> validEntryConsumer) {
final SparseArray<String> knownUids = new SparseArray<>();
- while (cursor.moveToNext()) {
- final long downloadId = cursor.getLong(0);
- final int uid = cursor.getInt(1);
+ final ArrayList<Long> idsToDelete = new ArrayList<>();
+ final ArrayList<Long> idsToOrphan = new ArrayList<>();
+ final String selection = removedUid == -1 ? Constants.UID + " IS NOT NULL"
+ : Constants.UID + "=" + removedUid;
+ try (Cursor cursor = downloadProvider.query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
+ new String[] { Downloads.Impl._ID, Constants.UID, COLUMN_DESTINATION, _DATA },
+ selection, null, null)) {
+ while (cursor.moveToNext()) {
+ final long downloadId = cursor.getLong(0);
+ final int uid = cursor.getInt(1);
- final String ownerPackageName;
- final int index = knownUids.indexOfKey(uid);
- if (index >= 0) {
- ownerPackageName = knownUids.valueAt(index);
- } else {
- ownerPackageName = getPackageForUid(context, uid);
- knownUids.put(uid, ownerPackageName);
- }
-
- if (ownerPackageName == null) {
- final int destination = cursor.getInt(2);
- final String filePath = cursor.getString(3);
-
- if ((destination == DESTINATION_EXTERNAL
- || destination == DESTINATION_FILE_URI
- || destination == DESTINATION_NON_DOWNLOADMANAGER_DOWNLOAD)
- && isFilenameValidInKnownPublicDir(filePath)) {
- idsToOrphan.add(downloadId);
+ final String ownerPackageName;
+ final int index = knownUids.indexOfKey(uid);
+ if (index >= 0) {
+ ownerPackageName = knownUids.valueAt(index);
} else {
- idsToDelete.add(downloadId);
+ ownerPackageName = getPackageForUid(context, uid);
+ knownUids.put(uid, ownerPackageName);
}
- } else if (idsToGrantPermission != null) {
- idsToGrantPermission.put(downloadId, ownerPackageName);
+
+ if (ownerPackageName == null) {
+ final int destination = cursor.getInt(2);
+ final String filePath = cursor.getString(3);
+
+ if ((destination == DESTINATION_EXTERNAL
+ || destination == DESTINATION_FILE_URI
+ || destination == DESTINATION_NON_DOWNLOADMANAGER_DOWNLOAD)
+ && isFilenameValidInKnownPublicDir(filePath)) {
+ idsToOrphan.add(downloadId);
+ } else {
+ idsToDelete.add(downloadId);
+ }
+ } else if (validEntryConsumer != null) {
+ validEntryConsumer.accept(ownerPackageName, downloadId);
+ }
}
}
+
+ if (idsToOrphan.size() > 0) {
+ Log.i(Constants.TAG, "Orphaning downloads with ids "
+ + Arrays.toString(idsToOrphan.toArray()) + " as owner package is removed");
+ final ContentValues values = new ContentValues();
+ values.putNull(Constants.UID);
+ downloadProvider.update(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, values,
+ Helpers.buildQueryWithIds(idsToOrphan), null);
+ }
+ if (idsToDelete.size() > 0) {
+ Log.i(Constants.TAG, "Deleting downloads with ids "
+ + Arrays.toString(idsToDelete.toArray()) + " as owner package is removed");
+ downloadProvider.delete(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI,
+ Helpers.buildQueryWithIds(idsToDelete), null);
+ }
}
public static String buildQueryWithIds(ArrayList<Long> downloadIds) {
diff --git a/tests/src/com/android/providers/downloads/HelpersTest.java b/tests/src/com/android/providers/downloads/HelpersTest.java
index 65c5d36..9cb99ba 100644
--- a/tests/src/com/android/providers/downloads/HelpersTest.java
+++ b/tests/src/com/android/providers/downloads/HelpersTest.java
@@ -16,14 +16,45 @@
package com.android.providers.downloads;
+import static android.provider.Downloads.Impl.COLUMN_DESTINATION;
+import static android.provider.Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE;
+import static android.provider.Downloads.Impl.DESTINATION_EXTERNAL;
+import static android.provider.Downloads.Impl.DESTINATION_FILE_URI;
+import static android.provider.Downloads.Impl.DESTINATION_NON_DOWNLOADMANAGER_DOWNLOAD;
+import static android.provider.Downloads.Impl._DATA;
+import static android.provider.Downloads.Impl._ID;
+
+import static com.android.internal.util.ArrayUtils.contains;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import android.content.ContentProvider;
+import android.content.Context;
+import android.content.pm.PackageManager;
+import android.database.MatrixCursor;
import android.net.Uri;
+import android.os.Environment;
import android.provider.Downloads;
+
import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.SmallTest;
+import android.util.LongArray;
+import android.util.LongSparseArray;
import libcore.io.IoUtils;
import java.io.File;
+import java.util.Arrays;
+import java.util.function.BiConsumer;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
/**
* This test exercises methods in the {@Helpers} utility class.
@@ -31,8 +62,22 @@
@SmallTest
public class HelpersTest extends AndroidTestCase {
+ private final static int TEST_UID1 = 11111;
+ private final static int TEST_UID2 = 11112;
+ private final static int TEST_UID3 = 11113;
+
+ private final MockitoHelper mMockitoHelper = new MockitoHelper();
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+
+ mMockitoHelper.setUp(getClass());
+ }
+
@Override
protected void tearDown() throws Exception {
+ mMockitoHelper.tearDown();
IoUtils.deleteContents(getContext().getFilesDir());
IoUtils.deleteContents(getContext().getCacheDir());
@@ -117,4 +162,164 @@
assertFalse(Helpers.isFilenameValidInKnownPublicDir(
"/storage/emulated/0/Android/data/com.example/bar.jpg"));
}
+
+ public void testHandleRemovedUidEntries() throws Exception {
+ // Prepare
+ final int[] testUids = {
+ TEST_UID1, TEST_UID2, TEST_UID3
+ };
+ final int[] unknownUids = {
+ TEST_UID1, TEST_UID2
+ };
+ final Context context = mock(Context.class);
+ final PackageManager packageManager = mock(PackageManager.class);
+ when(context.getPackageManager()).thenReturn(packageManager);
+ for (int uid : testUids) {
+ when(packageManager.getPackagesForUid(uid)).thenReturn(
+ contains(unknownUids, uid) ? null : new String[] {"com.example" + uid}
+ );
+ }
+
+ final LongArray idsToRemove = new LongArray();
+ final LongArray idsToOrphan = new LongArray();
+ final LongSparseArray<String> validEntries = new LongSparseArray<>();
+ final MatrixCursor cursor = prepareData(testUids, unknownUids,
+ idsToOrphan, idsToRemove, validEntries);
+
+ final ContentProvider downloadProvider = mock(ContentProvider.class);
+ when(downloadProvider.query(eq(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI),
+ any(String[].class), any(String.class),isNull(), isNull())).thenReturn(cursor);
+ final BiConsumer<String, Long> validEntryConsumer = mock(BiConsumer.class);
+
+ // Call
+ Helpers.handleRemovedUidEntries(context, downloadProvider, -1, validEntryConsumer);
+
+ // Verify
+ verify(downloadProvider).update(eq(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI),
+ argThat(values -> values.get(Constants.UID) == null),
+ argThat(selection -> Arrays.equals(
+ idsToOrphan.toArray(), extractIdsFromSelection(selection))),
+ isNull());
+ verify(downloadProvider).delete(eq(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI),
+ argThat(selection -> Arrays.equals(
+ idsToRemove.toArray(), extractIdsFromSelection(selection))),
+ isNull());
+ for (int i = 0; i < validEntries.size(); ++i) {
+ verify(validEntryConsumer).accept(validEntries.valueAt(i), validEntries.keyAt(i));
+ }
+
+
+ // Reset
+ idsToOrphan.clear();
+ idsToRemove.clear();
+ validEntries.clear();
+ reset(downloadProvider);
+
+ // Prepare
+ final MatrixCursor cursor2 = prepareData(new int[] {TEST_UID2}, unknownUids,
+ idsToOrphan, idsToRemove, validEntries);
+ when(downloadProvider.query(eq(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI),
+ any(String[].class), any(String.class),isNull(), isNull())).thenReturn(cursor2);
+
+ // Call
+ Helpers.handleRemovedUidEntries(context, downloadProvider, TEST_UID2, null);
+
+ // Verify
+ verify(downloadProvider).update(eq(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI),
+ argThat(values -> values.get(Constants.UID) == null),
+ argThat(selection -> Arrays.equals(
+ idsToOrphan.toArray(), extractIdsFromSelection(selection))),
+ isNull());
+ verify(downloadProvider).delete(eq(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI),
+ argThat(selection -> Arrays.equals(
+ idsToRemove.toArray(), extractIdsFromSelection(selection))),
+ isNull());
+ }
+
+ private MatrixCursor prepareData(int[] uids, int[] unknownUids,
+ final LongArray idsToOrphan, final LongArray idsToRemove,
+ LongSparseArray<String> validEntries) {
+ final MatrixCursor cursor = new MatrixCursor(
+ new String[] {_ID, Constants.UID, COLUMN_DESTINATION, _DATA});
+ final int[] destinations = {
+ DESTINATION_EXTERNAL,
+ DESTINATION_FILE_URI,
+ DESTINATION_NON_DOWNLOADMANAGER_DOWNLOAD,
+ DESTINATION_CACHE_PARTITION_PURGEABLE
+ };
+ long counter = 0;
+ for (int uid : uids) {
+ for (int destination : destinations) {
+ final String fileName = uid + "_" + destination + ".txt";
+ switch (destination) {
+ case DESTINATION_EXTERNAL: {
+ final File file = new File(Environment.getExternalStoragePublicDirectory(
+ Environment.DIRECTORY_DOWNLOADS), fileName);
+ cursor.addRow(new Object[]{++counter, uid, destination, file.getPath()});
+ if (contains(unknownUids, uid)) {
+ idsToOrphan.add(counter);
+ } else {
+ validEntries.put(counter, "com.example" + uid);
+ }
+ } break;
+ case DESTINATION_FILE_URI: {
+ final File file1 = new File(Environment.getExternalStoragePublicDirectory(
+ Environment.DIRECTORY_DOCUMENTS), fileName);
+ cursor.addRow(new Object[]{++counter, uid, destination, file1.getPath()});
+ if (contains(unknownUids, uid)) {
+ idsToOrphan.add(counter);
+ } else {
+ validEntries.put(counter, "com.example" + uid);
+ }
+ final File file2 = new File(getContext().getExternalFilesDir(null),
+ fileName);
+ cursor.addRow(new Object[]{++counter, uid, destination, file2.getPath()});
+ if (contains(unknownUids, uid)) {
+ idsToRemove.add(counter);
+ } else {
+ validEntries.put(counter, "com.example" + uid);
+ }
+ } break;
+ case DESTINATION_NON_DOWNLOADMANAGER_DOWNLOAD: {
+ final File file1 = new File(Environment.getExternalStoragePublicDirectory(
+ Environment.DIRECTORY_DOCUMENTS), fileName);
+ cursor.addRow(new Object[]{++counter, uid, destination, file1.getPath()});
+ if (contains(unknownUids, uid)) {
+ idsToOrphan.add(counter);
+ } else {
+ validEntries.put(counter, "com.example" + uid);
+ }
+ final File file2 = new File(getContext().getExternalFilesDir(null),
+ fileName);
+ cursor.addRow(new Object[]{++counter, uid, destination, file2.getPath()});
+ if (contains(unknownUids, uid)) {
+ idsToRemove.add(counter);
+ } else {
+ validEntries.put(counter, "com.example" + uid);
+ }
+ } break;
+ case DESTINATION_CACHE_PARTITION_PURGEABLE: {
+ final File file = new File(getContext().getCacheDir(), fileName);
+ final String filePath = file.getPath().replace(
+ getContext().getPackageName(), "com.android.providers.downloads");
+ cursor.addRow(new Object[]{++counter, uid, destination, filePath});
+ if (contains(unknownUids, uid)) {
+ idsToRemove.add(counter);
+ } else {
+ validEntries.put(counter, "com.example" + uid);
+ }
+ } break;
+ }
+ }
+ }
+ return cursor;
+ }
+
+ private long[] extractIdsFromSelection(String selection) {
+ final Pattern uidsListPattern = Pattern.compile(".*\\((.+)\\)");
+ final Matcher matcher = uidsListPattern.matcher(selection);
+ assertTrue(matcher.matches());
+ return Arrays.stream(matcher.group(1).split(","))
+ .mapToLong(Long::valueOf).sorted().toArray();
+ }
}
