From 0a77c62a82503b38c484e0079648f0231dd85d53 Mon Sep 17 00:00:00 2001 From: Steve Howard Date: Wed, 21 Jul 2010 11:41:30 -0700 Subject: Public API support for broadcasts and connectivity control. * Three new DB fields, one indicating whether a download was initiated by the public API or not, two to hold connectivity control info. DB migration to add these fields and code in DownloadProvider.insert() to handle them. * Change broadcast intent code to match public API spec, for public API downloads only. (Legacy code can go away once existing clients are converted over to the new API.) * Introduce SystemFacade methods for sending broadcasts and checking package ownership; this facilitates new tests of broadcast code. * Change DownloadInfo.canUseNetwork() to obey new connectivity controls available in public API, but again, retain legacy behavior for downloads initiated directly through DownloadProvider * New test cases to cover the new behavior Also made a couple changes to reduce some test flakiness I was observing: * in tearDown(), wait for any running UpdateThread to complete * in PublicApiFunctionalTest.setUp(), if the test directory already exists, remove it rather than aborting DB changes for broadcast + roaming support Change-Id: I60f39fc133f678f3510880ea6eb9f639358914b4 --- .../android/providers/downloads/DownloadInfo.java | 75 +++++++++++++++++-- .../providers/downloads/DownloadProvider.java | 44 ++++++++++-- .../providers/downloads/DownloadReceiver.java | 34 ++++++--- .../providers/downloads/DownloadService.java | 3 +- .../providers/downloads/RealSystemFacade.java | 12 ++++ .../android/providers/downloads/SystemFacade.java | 13 ++++ .../AbstractDownloadManagerFunctionalTest.java | 14 ++++ .../providers/downloads/FakeSystemFacade.java | 16 +++++ .../downloads/PublicApiFunctionalTest.java | 84 ++++++++++++++++++++-- 9 files changed, 268 insertions(+), 27 deletions(-) diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java index c99a3784..29c2d490 100644 --- a/src/com/android/providers/downloads/DownloadInfo.java +++ b/src/com/android/providers/downloads/DownloadInfo.java @@ -22,6 +22,7 @@ import android.content.Context; import android.content.Intent; import android.database.Cursor; import android.net.ConnectivityManager; +import android.net.DownloadManager; import android.net.Uri; import android.provider.Downloads; import android.provider.Downloads.Impl; @@ -59,6 +60,9 @@ public class DownloadInfo { public long mCurrentBytes; public String mETag; public boolean mMediaScanned; + public boolean mIsPublicApi; + public int mAllowedNetworkTypes; + public boolean mAllowRoaming; public int mFuzz; @@ -109,6 +113,12 @@ public class DownloadInfo { cursor.getInt(cursor.getColumnIndexOrThrow(Downloads.Impl.COLUMN_CURRENT_BYTES)); mETag = cursor.getString(cursor.getColumnIndexOrThrow(Constants.ETAG)); mMediaScanned = cursor.getInt(cursor.getColumnIndexOrThrow(Constants.MEDIA_SCANNED)) == 1; + mIsPublicApi = cursor.getInt( + cursor.getColumnIndexOrThrow(Downloads.Impl.COLUMN_IS_PUBLIC_API)) != 0; + mAllowedNetworkTypes = cursor.getInt( + cursor.getColumnIndexOrThrow(Downloads.Impl.COLUMN_ALLOWED_NETWORK_TYPES)); + mAllowRoaming = cursor.getInt( + cursor.getColumnIndexOrThrow(Downloads.Impl.COLUMN_ALLOW_ROAMING)) != 0; mFuzz = Helpers.sRandom.nextInt(1001); readRequestHeaders(mId); @@ -144,8 +154,20 @@ public class DownloadInfo { } public void sendIntentIfRequested(Uri contentUri) { - if (mPackage != null && mClass != null) { - Intent intent = new Intent(Downloads.Impl.ACTION_DOWNLOAD_COMPLETED); + if (mPackage == null) { + return; + } + + Intent intent; + if (mIsPublicApi) { + intent = new Intent(DownloadManager.ACTION_DOWNLOAD_COMPLETE); + intent.setPackage(mPackage); + intent.putExtra(DownloadManager.EXTRA_DOWNLOAD_ID, (long) mId); + } else { // legacy behavior + if (mClass == null) { + return; + } + intent = new Intent(Downloads.Impl.ACTION_DOWNLOAD_COMPLETED); intent.setClassName(mPackage, mClass); if (mExtras != null) { intent.putExtra(Downloads.Impl.COLUMN_NOTIFICATION_EXTRAS, mExtras); @@ -154,8 +176,8 @@ public class DownloadInfo { // applications would have an easier time spoofing download results by // sending spoofed intents. intent.setData(contentUri); - mContext.sendBroadcast(intent); } + mSystemFacade.sendBroadcast(intent); } /** @@ -262,16 +284,57 @@ public class DownloadInfo { if (networkType == null) { return false; } - if (!isSizeAllowedForNetwork(networkType)) { + if (!isNetworkTypeAllowed(networkType)) { return false; } - if (mDestination == Downloads.Impl.DESTINATION_CACHE_PARTITION_NOROAMING - && mSystemFacade.isNetworkRoaming()) { + if (!isRoamingAllowed() && mSystemFacade.isNetworkRoaming()) { return false; } return true; } + private boolean isRoamingAllowed() { + if (mIsPublicApi) { + return mAllowRoaming; + } else { // legacy behavior + return mDestination != Downloads.Impl.DESTINATION_CACHE_PARTITION_NOROAMING; + } + } + + /** + * Check if this download can proceed over the given network type. + * @param networkType a constant from ConnectivityManager.TYPE_*. + */ + private boolean isNetworkTypeAllowed(int networkType) { + if (mIsPublicApi) { + int flag = translateNetworkTypeToApiFlag(networkType); + if ((flag & mAllowedNetworkTypes) == 0) { + return false; + } + } + return isSizeAllowedForNetwork(networkType); + } + + /** + * Translate a ConnectivityManager.TYPE_* constant to the corresponding + * DownloadManager.Request.NETWORK_* bit flag. + */ + private int translateNetworkTypeToApiFlag(int networkType) { + switch (networkType) { + case ConnectivityManager.TYPE_MOBILE: + return DownloadManager.Request.NETWORK_MOBILE; + + case ConnectivityManager.TYPE_WIFI: + return DownloadManager.Request.NETWORK_WIFI; + + case ConnectivityManager.TYPE_WIMAX: + return DownloadManager.Request.NETWORK_WIMAX; + + default: + return 0; + } + } + /** * Check if the download's size prohibits it from running over the current network. */ diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index c210063a..bb205ad4 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -56,7 +56,7 @@ public final class DownloadProvider extends ContentProvider { /** Database filename */ private static final String DB_NAME = "downloads.db"; /** Current database version */ - private static final int DB_VERSION = 101; + private static final int DB_VERSION = 102; /** Name of table in the database */ private static final String DB_TABLE = "downloads"; @@ -95,7 +95,7 @@ public final class DownloadProvider extends ContentProvider { Downloads.Impl.COLUMN_TOTAL_BYTES, Downloads.Impl.COLUMN_CURRENT_BYTES, Downloads.Impl.COLUMN_TITLE, - Downloads.Impl.COLUMN_DESCRIPTION + Downloads.Impl.COLUMN_DESCRIPTION, }; private static HashSet sAppReadableColumnsSet; @@ -182,11 +182,32 @@ public final class DownloadProvider extends ContentProvider { createHeadersTable(db); break; + case 102: + addColumn(db, DB_TABLE, Downloads.Impl.COLUMN_IS_PUBLIC_API, + "INTEGER NOT NULL DEFAULT 0"); + addColumn(db, DB_TABLE, Downloads.Impl.COLUMN_ALLOW_ROAMING, + "INTEGER NOT NULL DEFAULT 0"); + addColumn(db, DB_TABLE, Downloads.Impl.COLUMN_ALLOWED_NETWORK_TYPES, + "INTEGER NOT NULL DEFAULT 0"); + break; + default: throw new IllegalStateException("Don't know how to upgrade to " + version); } } + /** + * Add a column to a table using ALTER TABLE. + * @param dbTable name of the table + * @param columnName name of the column to add + * @param columnDefinition SQL for the column definition + */ + private void addColumn(SQLiteDatabase db, String dbTable, String columnName, + String columnDefinition) { + db.execSQL("ALTER TABLE " + dbTable + " ADD COLUMN " + columnName + " " + + columnDefinition); + } + /** * Creates the table that'll hold the download information. */ @@ -346,15 +367,21 @@ public final class DownloadProvider extends ContentProvider { filteredValues.put(Downloads.Impl.COLUMN_STATUS, Downloads.Impl.STATUS_PENDING); filteredValues.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, mSystemFacade.currentTimeMillis()); + + copyBoolean(Downloads.Impl.COLUMN_IS_PUBLIC_API, values, filteredValues); + boolean isPublicApi = + values.getAsBoolean(Downloads.Impl.COLUMN_IS_PUBLIC_API) == Boolean.TRUE; + String pckg = values.getAsString(Downloads.Impl.COLUMN_NOTIFICATION_PACKAGE); String clazz = values.getAsString(Downloads.Impl.COLUMN_NOTIFICATION_CLASS); - if (pckg != null && clazz != null) { + if (pckg != null && (clazz != null || isPublicApi)) { int uid = Binder.getCallingUid(); try { - if (uid == 0 || - getContext().getPackageManager().getApplicationInfo(pckg, 0).uid == uid) { + if (uid == 0 || mSystemFacade.userOwnsPackage(uid, pckg)) { filteredValues.put(Downloads.Impl.COLUMN_NOTIFICATION_PACKAGE, pckg); - filteredValues.put(Downloads.Impl.COLUMN_NOTIFICATION_CLASS, clazz); + if (clazz != null) { + filteredValues.put(Downloads.Impl.COLUMN_NOTIFICATION_CLASS, clazz); + } } } catch (PackageManager.NameNotFoundException ex) { /* ignored for now */ @@ -376,6 +403,11 @@ public final class DownloadProvider extends ContentProvider { copyString(Downloads.Impl.COLUMN_DESCRIPTION, values, filteredValues); filteredValues.put(Downloads.Impl.COLUMN_TOTAL_BYTES, -1); + if (isPublicApi) { + copyInteger(Downloads.Impl.COLUMN_ALLOWED_NETWORK_TYPES, values, filteredValues); + copyBoolean(Downloads.Impl.COLUMN_ALLOW_ROAMING, values, filteredValues); + } + if (Constants.LOGVV) { Log.v(Constants.TAG, "initiating download with UID " + filteredValues.getAsInteger(Constants.UID)); diff --git a/src/com/android/providers/downloads/DownloadReceiver.java b/src/com/android/providers/downloads/DownloadReceiver.java index 0b4a12da..98c37103 100644 --- a/src/com/android/providers/downloads/DownloadReceiver.java +++ b/src/com/android/providers/downloads/DownloadReceiver.java @@ -16,6 +16,8 @@ package com.android.providers.downloads; +import com.google.common.annotations.VisibleForTesting; + import android.app.NotificationManager; import android.content.ActivityNotFoundException; import android.content.BroadcastReceiver; @@ -25,6 +27,7 @@ import android.content.Context; import android.content.Intent; import android.database.Cursor; import android.net.ConnectivityManager; +import android.net.DownloadManager; import android.net.NetworkInfo; import android.net.Uri; import android.provider.Downloads; @@ -37,7 +40,7 @@ import java.io.File; * Receives system broadcasts (boot, network connectivity) */ public class DownloadReceiver extends BroadcastReceiver { - + @VisibleForTesting SystemFacade mSystemFacade = null; public void onReceive(Context context, Intent intent) { @@ -133,18 +136,29 @@ public class DownloadReceiver extends BroadcastReceiver { Downloads.Impl.COLUMN_NOTIFICATION_PACKAGE); int classColumn = cursor.getColumnIndexOrThrow( Downloads.Impl.COLUMN_NOTIFICATION_CLASS); + int isPublicApiColumn = cursor.getColumnIndex( + Downloads.Impl.COLUMN_IS_PUBLIC_API); String pckg = cursor.getString(packageColumn); String clazz = cursor.getString(classColumn); - if (pckg != null && clazz != null) { - Intent appIntent = - new Intent(Downloads.Impl.ACTION_NOTIFICATION_CLICKED); - appIntent.setClassName(pckg, clazz); - if (intent.getBooleanExtra("multiple", true)) { - appIntent.setData(Downloads.Impl.CONTENT_URI); - } else { - appIntent.setData(intent.getData()); + boolean isPublicApi = cursor.getInt(isPublicApiColumn) != 0; + + if (pckg != null) { + Intent appIntent = null; + if (isPublicApi) { + appIntent = new Intent(DownloadManager.ACTION_NOTIFICATION_CLICKED); + appIntent.setPackage(pckg); + } else if (clazz != null) { // legacy behavior + appIntent = new Intent(Downloads.Impl.ACTION_NOTIFICATION_CLICKED); + appIntent.setClassName(pckg, clazz); + if (intent.getBooleanExtra("multiple", true)) { + appIntent.setData(Downloads.Impl.CONTENT_URI); + } else { + appIntent.setData(intent.getData()); + } + } + if (appIntent != null) { + mSystemFacade.sendBroadcast(appIntent); } - context.sendBroadcast(appIntent); } } } diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java index f870954b..472b4c5f 100644 --- a/src/com/android/providers/downloads/DownloadService.java +++ b/src/com/android/providers/downloads/DownloadService.java @@ -78,7 +78,8 @@ public class DownloadService extends Service { * The thread that updates the internal download list from the content * provider. */ - private UpdateThread mUpdateThread; + @VisibleForTesting + UpdateThread mUpdateThread; /** * Whether the internal download list should be updated from the content diff --git a/src/com/android/providers/downloads/RealSystemFacade.java b/src/com/android/providers/downloads/RealSystemFacade.java index 89cf3b1d..f89f1659 100644 --- a/src/com/android/providers/downloads/RealSystemFacade.java +++ b/src/com/android/providers/downloads/RealSystemFacade.java @@ -1,6 +1,8 @@ package com.android.providers.downloads; import android.content.Context; +import android.content.Intent; +import android.content.pm.PackageManager.NameNotFoundException; import android.net.ConnectivityManager; import android.net.NetworkInfo; import android.telephony.TelephonyManager; @@ -55,4 +57,14 @@ class RealSystemFacade implements SystemFacade { public Integer getMaxBytesOverMobile() { return null; } + + @Override + public void sendBroadcast(Intent intent) { + mContext.sendBroadcast(intent); + } + + @Override + public boolean userOwnsPackage(int uid, String packageName) throws NameNotFoundException { + return mContext.getPackageManager().getApplicationInfo(packageName, 0).uid == uid; + } } diff --git a/src/com/android/providers/downloads/SystemFacade.java b/src/com/android/providers/downloads/SystemFacade.java index 2addbf82..d48e6b80 100644 --- a/src/com/android/providers/downloads/SystemFacade.java +++ b/src/com/android/providers/downloads/SystemFacade.java @@ -1,6 +1,9 @@ package com.android.providers.downloads; +import android.content.Intent; +import android.content.pm.PackageManager.NameNotFoundException; + interface SystemFacade { /** @@ -24,4 +27,14 @@ interface SystemFacade { * there's no limit */ public Integer getMaxBytesOverMobile(); + + /** + * Send a broadcast intent. + */ + public void sendBroadcast(Intent intent); + + /** + * Returns true if the specified UID owns the specified package name. + */ + public boolean userOwnsPackage(int uid, String pckg) throws NameNotFoundException; } diff --git a/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java b/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java index 92678fe3..7af98c17 100644 --- a/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java @@ -148,10 +148,24 @@ public abstract class AbstractDownloadManagerFunctionalTest extends @Override protected void tearDown() throws Exception { + waitForUpdateThread(); cleanUpDownloads(); super.tearDown(); } + private void waitForUpdateThread() throws InterruptedException { + DownloadService service = getService(); + if (service == null) { + return; + } + + long startTimeMillis = System.currentTimeMillis(); + while (service.mUpdateThread != null + && System.currentTimeMillis() < startTimeMillis + 1000) { + Thread.sleep(50); + } + } + private boolean isDatabaseEmpty() { Cursor cursor = mResolver.query(Downloads.CONTENT_URI, null, null, null, null); try { diff --git a/tests/src/com/android/providers/downloads/FakeSystemFacade.java b/tests/src/com/android/providers/downloads/FakeSystemFacade.java index 4ff313ab..0f8a9801 100644 --- a/tests/src/com/android/providers/downloads/FakeSystemFacade.java +++ b/tests/src/com/android/providers/downloads/FakeSystemFacade.java @@ -1,12 +1,18 @@ package com.android.providers.downloads; +import android.content.Intent; +import android.content.pm.PackageManager.NameNotFoundException; import android.net.ConnectivityManager; +import java.util.ArrayList; +import java.util.List; + public class FakeSystemFacade implements SystemFacade { long mTimeMillis = 0; Integer mActiveNetworkType = ConnectivityManager.TYPE_WIFI; boolean mIsRoaming = false; Integer mMaxBytesOverMobile = null; + List mBroadcastsSent = new ArrayList(); void incrementTimeMillis(long delta) { mTimeMillis += delta; @@ -27,4 +33,14 @@ public class FakeSystemFacade implements SystemFacade { public Integer getMaxBytesOverMobile() { return mMaxBytesOverMobile ; } + + @Override + public void sendBroadcast(Intent intent) { + mBroadcastsSent.add(intent); + } + + @Override + public boolean userOwnsPackage(int uid, String pckg) throws NameNotFoundException { + return true; + } } diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java index b1ccc7ae..7982ef4a 100644 --- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java @@ -16,12 +16,14 @@ package com.android.providers.downloads; +import android.content.Intent; import android.database.Cursor; import android.net.ConnectivityManager; import android.net.DownloadManager; import android.net.Uri; import android.os.Environment; import android.os.ParcelFileDescriptor; +import android.provider.Downloads; import android.test.suitebuilder.annotation.LargeTest; import tests.http.MockResponse; import tests.http.RecordedRequest; @@ -34,6 +36,7 @@ import java.util.List; @LargeTest public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTest { + private static final String PACKAGE_NAME = "my.package.name"; private static final int HTTP_NOT_ACCEPTABLE = 406; private static final int HTTP_LENGTH_REQUIRED = 411; private static final String REQUEST_PATH = "/path"; @@ -102,13 +105,12 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe @Override protected void setUp() throws Exception { super.setUp(); - mManager = new DownloadManager(mResolver); + mManager = new DownloadManager(mResolver, PACKAGE_NAME); mTestDirectory = new File(Environment.getExternalStorageDirectory() + File.separator + "download_manager_functional_test"); if (mTestDirectory.exists()) { - throw new RuntimeException( - "Test directory on external storage already exists, cannot run"); + mTestDirectory.delete(); } if (!mTestDirectory.mkdir()) { throw new RuntimeException("Couldn't create test directory: " @@ -328,7 +330,7 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe enqueueResponse(HTTP_OK, FILE_CONTENT); download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); } - + /** * Test for race conditions when the service is flooded with startService() calls while running * a download. @@ -394,6 +396,80 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS); startService(null); Thread.sleep(500); // TODO: eliminate this when we can run the service synchronously + // if the cancel didn't work, we should get an unexpected request to the HTTP server + } + + public void testDownloadCompleteBroadcast() throws Exception { + enqueueEmptyResponse(HTTP_OK); + Download download = enqueueRequest(getRequest()); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); + + long startTimeMillis = System.currentTimeMillis(); + while (mSystemFacade.mBroadcastsSent.isEmpty()) { + Thread.sleep(100); + if (System.currentTimeMillis() > startTimeMillis + 500) { + fail("Timed out waiting for broadcast intent"); + } + } + assertEquals(1, mSystemFacade.mBroadcastsSent.size()); + Intent broadcast = mSystemFacade.mBroadcastsSent.get(0); + assertEquals(DownloadManager.ACTION_DOWNLOAD_COMPLETE, broadcast.getAction()); + assertEquals(PACKAGE_NAME, broadcast.getPackage()); + long intentId = broadcast.getExtras().getLong(DownloadManager.EXTRA_DOWNLOAD_ID); + assertEquals(download.mId, intentId); + } + + public void testNotificationClickedBroadcast() throws Exception { + Download download = enqueueRequest(getRequest().setShowNotification( + DownloadManager.Request.NOTIFICATION_WHEN_RUNNING)); + + DownloadReceiver receiver = new DownloadReceiver(); + receiver.mSystemFacade = mSystemFacade; + Intent intent = new Intent(Constants.ACTION_LIST); + intent.setData(Uri.parse(Downloads.Impl.CONTENT_URI + "/" + download.mId)); + receiver.onReceive(mContext, intent); + + assertEquals(1, mSystemFacade.mBroadcastsSent.size()); + Intent broadcast = mSystemFacade.mBroadcastsSent.get(0); + assertEquals(DownloadManager.ACTION_NOTIFICATION_CLICKED, broadcast.getAction()); + assertEquals(PACKAGE_NAME, broadcast.getPackage()); + } + + public void testAllowedNetworkTypes() throws Exception { + mSystemFacade.mActiveNetworkType = ConnectivityManager.TYPE_MOBILE; + + // by default, use any connection + enqueueEmptyResponse(HTTP_OK); + Download download = enqueueRequest(getRequest()); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); + + // restrict a download to wifi... + download = enqueueRequest(getRequest() + .setAllowedNetworkTypes(DownloadManager.Request.NETWORK_WIFI)); + startService(null); + waitForDownloadToStop(download, DownloadManager.STATUS_PAUSED); + // ...then enable wifi + mSystemFacade.mActiveNetworkType = ConnectivityManager.TYPE_WIFI; + enqueueEmptyResponse(HTTP_OK); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); + } + + public void testRoaming() throws Exception { + mSystemFacade.mIsRoaming = true; + + // by default, allow roaming + enqueueEmptyResponse(HTTP_OK); + Download download = enqueueRequest(getRequest()); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); + + // disallow roaming for a download... + download = enqueueRequest(getRequest().setAllowedOverRoaming(false)); + startService(null); + waitForDownloadToStop(download, DownloadManager.STATUS_PAUSED); + // ...then turn off roaming + mSystemFacade.mIsRoaming = false; + enqueueEmptyResponse(HTTP_OK); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); } private void runSimpleFailureTest(int expectedErrorCode) throws Exception { -- cgit v1.2.3