Public API support for broadcasts and connectivity control.
Steve Howard [Wed, 21 Jul 2010 18:41:30 +0000 (11:41 -0700)]
* 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

src/com/android/providers/downloads/DownloadInfo.java
src/com/android/providers/downloads/DownloadProvider.java
src/com/android/providers/downloads/DownloadReceiver.java
src/com/android/providers/downloads/DownloadService.java
src/com/android/providers/downloads/RealSystemFacade.java
src/com/android/providers/downloads/SystemFacade.java
tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java
tests/src/com/android/providers/downloads/FakeSystemFacade.java
tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java

index c99a378..29c2d49 100644 (file)
@@ -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.
      */
index c210063..bb205ad 100644 (file)
@@ -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<String> sAppReadableColumnsSet;
@@ -182,12 +182,33 @@ 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.
          */
         private void createDownloadsTable(SQLiteDatabase db) {
@@ -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));
index 0b4a12d..98c3710 100644 (file)
@@ -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);
                         }
                     }
                 }
index f870954..472b4c5 100644 (file)
@@ -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
index 89cf3b1..f89f165 100644 (file)
@@ -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;
+    }
 }
index 2addbf8..d48e6b8 100644 (file)
@@ -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;
 }
index 92678fe..7af98c1 100644 (file)
@@ -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 {
index 4ff313a..0f8a980 100644 (file)
@@ -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<Intent> mBroadcastsSent = new ArrayList<Intent>();
 
     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;
+    }
 }
index b1ccc7a..7982ef4 100644 (file)
 
 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 {