Stub out and test system notifications.
Steve Howard [Sat, 24 Jul 2010 03:32:21 +0000 (20:32 -0700)]
This change abstracts NotificationManager interactions behind
SystemFacade and takes advantage of that to test notifications, to a
limited degree.

It also fixes a silly typo in AbstractDownloadManagerFunctionalTest,
and it introduces an extra sleep between tests to avoid some
flakiness.  I'll look for a better solution to that problem after this
change goes in.

Change-Id: I3a0307f828955cd45b0e3581ad499da28cc0556e

src/com/android/providers/downloads/DownloadNotification.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 3c0b7d9..76bffbc 100644 (file)
@@ -17,7 +17,6 @@
 package com.android.providers.downloads;
 
 import android.app.Notification;
-import android.app.NotificationManager;
 import android.app.PendingIntent;
 import android.content.Context;
 import android.content.Intent;
@@ -38,8 +37,8 @@ import java.util.HashMap;
 class DownloadNotification {
 
     Context mContext;
-    public NotificationManager mNotificationMgr;
     HashMap <String, NotificationItem> mNotifications;
+    private SystemFacade mSystemFacade;
 
     static final String LOGTAG = "DownloadNotification";
     static final String WHERE_RUNNING =
@@ -93,10 +92,9 @@ class DownloadNotification {
      * @param ctx The context to use to obtain access to the
      *            Notification Service
      */
-    DownloadNotification(Context ctx) {
+    DownloadNotification(Context ctx, SystemFacade systemFacade) {
         mContext = ctx;
-        mNotificationMgr = (NotificationManager) mContext
-                .getSystemService(Context.NOTIFICATION_SERVICE);
+        mSystemFacade = systemFacade;
         mNotifications = new HashMap<String, NotificationItem>();
     }
 
@@ -208,7 +206,7 @@ class DownloadNotification {
 
             n.contentIntent = PendingIntent.getBroadcast(mContext, 0, intent, 0);
 
-            mNotificationMgr.notify(item.mId, n);
+            mSystemFacade.postNotification(item.mId, n);
 
         }
     }
@@ -287,7 +285,7 @@ class DownloadNotification {
             intent.setData(contentUri);
             n.deleteIntent = PendingIntent.getBroadcast(mContext, 0, intent, 0);
 
-            mNotificationMgr.notify(c.getInt(idColumn), n);
+            mSystemFacade.postNotification(c.getInt(idColumn), n);
         }
         c.close();
     }
index 98c3710..852c371 100644 (file)
@@ -16,9 +16,6 @@
 
 package com.android.providers.downloads;
 
-import com.google.common.annotations.VisibleForTesting;
-
-import android.app.NotificationManager;
 import android.content.ActivityNotFoundException;
 import android.content.BroadcastReceiver;
 import android.content.ContentUris;
@@ -34,6 +31,8 @@ import android.provider.Downloads;
 import android.util.Config;
 import android.util.Log;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import java.io.File;
 
 /**
@@ -164,11 +163,7 @@ public class DownloadReceiver extends BroadcastReceiver {
                 }
                 cursor.close();
             }
-            NotificationManager notMgr = (NotificationManager) context
-                    .getSystemService(Context.NOTIFICATION_SERVICE);
-            if (notMgr != null) {
-                notMgr.cancel((int) ContentUris.parseId(intent.getData()));
-            }
+            mSystemFacade.cancelNotification((int) ContentUris.parseId(intent.getData()));
         } else if (intent.getAction().equals(Constants.ACTION_HIDE)) {
             if (Constants.LOGVV) {
                 Log.v(Constants.TAG, "Receiver hide for " + intent.getData());
index 472b4c5..a617595 100644 (file)
@@ -218,8 +218,8 @@ public class DownloadService extends Service {
         mMediaScannerConnecting = false;
         mMediaScannerConnection = new MediaScannerConnection();
 
-        mNotifier = new DownloadNotification(this);
-        mNotifier.mNotificationMgr.cancelAll();
+        mNotifier = new DownloadNotification(this, mSystemFacade);
+        mSystemFacade.cancelAllNotifications();
         mNotifier.updateNotification();
 
         trimDatabase();
@@ -641,7 +641,7 @@ public class DownloadService extends Service {
         if (info.mVisibility == Downloads.Impl.VISIBILITY_VISIBLE_NOTIFY_COMPLETED
                 && newVisibility != Downloads.Impl.VISIBILITY_VISIBLE_NOTIFY_COMPLETED
                 && Downloads.Impl.isStatusCompleted(info.mStatus)) {
-            mNotifier.mNotificationMgr.cancel(info.mId);
+            mSystemFacade.cancelNotification(info.mId);
         }
         info.mVisibility = newVisibility;
         synchronized (info) {
@@ -651,7 +651,7 @@ public class DownloadService extends Service {
         int newStatus = cursor.getInt(statusColumn);
         if (!Downloads.Impl.isStatusCompleted(info.mStatus) &&
                     Downloads.Impl.isStatusCompleted(newStatus)) {
-            mNotifier.mNotificationMgr.cancel(info.mId);
+            mSystemFacade.cancelNotification(info.mId);
         }
         info.mStatus = newStatus;
         info.mNumFailed = cursor.getInt(failedColumn);
@@ -724,7 +724,7 @@ public class DownloadService extends Service {
                     && info.mFileName != null) {
             new File(info.mFileName).delete();
         }
-        mNotifier.mNotificationMgr.cancel(info.mId);
+        mSystemFacade.cancelNotification(info.mId);
 
         mDownloads.remove(arrayPos);
     }
index f89f165..1d9e64a 100644 (file)
@@ -1,5 +1,7 @@
 package com.android.providers.downloads;
 
+import android.app.Notification;
+import android.app.NotificationManager;
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageManager.NameNotFoundException;
@@ -10,9 +12,12 @@ import android.util.Log;
 
 class RealSystemFacade implements SystemFacade {
     private Context mContext;
+    private NotificationManager mNotificationManager;
 
     public RealSystemFacade(Context context) {
         mContext = context;
+        mNotificationManager = (NotificationManager)
+                mContext.getSystemService(Context.NOTIFICATION_SERVICE);
     }
 
     public long currentTimeMillis() {
@@ -67,4 +72,19 @@ class RealSystemFacade implements SystemFacade {
     public boolean userOwnsPackage(int uid, String packageName) throws NameNotFoundException {
         return mContext.getPackageManager().getApplicationInfo(packageName, 0).uid == uid;
     }
+
+    @Override
+    public void postNotification(int id, Notification notification) {
+        mNotificationManager.notify(id, notification);
+    }
+
+    @Override
+    public void cancelNotification(int id) {
+        mNotificationManager.cancel(id);
+    }
+
+    @Override
+    public void cancelAllNotifications() {
+        mNotificationManager.cancelAll();
+    }
 }
index d48e6b8..e41644a 100644 (file)
@@ -1,6 +1,7 @@
 
 package com.android.providers.downloads;
 
+import android.app.Notification;
 import android.content.Intent;
 import android.content.pm.PackageManager.NameNotFoundException;
 
@@ -37,4 +38,19 @@ interface SystemFacade {
      * Returns true if the specified UID owns the specified package name.
      */
     public boolean userOwnsPackage(int uid, String pckg) throws NameNotFoundException;
+
+    /**
+     * Post a system notification to the NotificationManager.
+     */
+    public void postNotification(int id, Notification notification);
+
+    /**
+     * Cancel a system notification.
+     */
+    public void cancelNotification(int id);
+
+    /**
+     * Cancel all system notifications.
+     */
+    public void cancelAllNotifications();
 }
index cb4ad8c..a401a5b 100644 (file)
@@ -165,12 +165,12 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
 
     @Override
     protected void tearDown() throws Exception {
-        waitForUpdateThread();
+        waitForThreads();
         cleanUpDownloads();
         super.tearDown();
     }
 
-    private void waitForUpdateThread() throws InterruptedException {
+    private void waitForThreads() throws InterruptedException {
         DownloadService service = getService();
         if (service == null) {
             return;
@@ -181,6 +181,10 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
                 && System.currentTimeMillis() < startTimeMillis + 1000) {
             Thread.sleep(50);
         }
+
+        // We can't explicitly wait for DownloadThreads, so just throw this last sleep in.  Ugly,
+        // but necessary to avoid unbearable flakiness until I can find a better solution.
+        Thread.sleep(50);
     }
 
     private boolean isDatabaseEmpty() {
@@ -289,7 +293,7 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
             status = reader.getStatus();
         }
 
-        long delta = startTimeMillis - startTimeMillis;
+        long delta = System.currentTimeMillis() - startTimeMillis;
         Log.d(LOG_TAG, "Status " + status + " reached after " + delta + "ms");
     }
 
index 0f8a980..d35b558 100644 (file)
@@ -1,11 +1,15 @@
 package com.android.providers.downloads;
 
+import android.app.Notification;
 import android.content.Intent;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.net.ConnectivityManager;
+import android.test.AssertionFailedError;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 public class FakeSystemFacade implements SystemFacade {
     long mTimeMillis = 0;
@@ -13,6 +17,8 @@ public class FakeSystemFacade implements SystemFacade {
     boolean mIsRoaming = false;
     Integer mMaxBytesOverMobile = null;
     List<Intent> mBroadcastsSent = new ArrayList<Intent>();
+    Map<Integer,Notification> mActiveNotifications = new HashMap<Integer,Notification>();
+    List<Notification> mCanceledNotifications = new ArrayList<Notification>();
 
     void incrementTimeMillis(long delta) {
         mTimeMillis += delta;
@@ -43,4 +49,27 @@ public class FakeSystemFacade implements SystemFacade {
     public boolean userOwnsPackage(int uid, String pckg) throws NameNotFoundException {
         return true;
     }
+
+    @Override
+    public void postNotification(int id, Notification notification) {
+        if (notification == null) {
+            throw new AssertionFailedError("Posting null notification");
+        }
+        mActiveNotifications.put(id, notification);
+    }
+
+    @Override
+    public void cancelNotification(int id) {
+        Notification notification = mActiveNotifications.remove(id);
+        if (notification != null) {
+            mCanceledNotifications.add(notification);
+        }
+    }
+
+    @Override
+    public void cancelAllNotifications() {
+        for (int id : mActiveNotifications.keySet()) {
+            cancelNotification(id);
+        }
+    }
 }
index 3d32ae3..00419a3 100644 (file)
@@ -485,6 +485,24 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         assertTrue(mResolver.mNotifyWasCalled);
     }
 
+    public void testNotifications() throws Exception {
+        enqueueEmptyResponse(HTTP_OK);
+        Download download = enqueueRequest(getRequest()); // no visibility requested
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        assertEquals(0, mSystemFacade.mActiveNotifications.size());
+        assertEquals(0, mSystemFacade.mCanceledNotifications.size());
+        
+        enqueueEmptyResponse(HTTP_OK);
+        download = enqueueRequest(
+                getRequest()
+                .setShowNotification(DownloadManager.Request.NOTIFICATION_WHEN_RUNNING));
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        assertEquals(1, mSystemFacade.mActiveNotifications.size());
+        // The notification doesn't actually get canceled until the UpdateThread runs again, which
+        // gets triggered by the DownloadThread updating the status in the provider.  This is
+        // tough to test right now, so I'll leave it until the overall structure is changed.
+    }
+
     private void runSimpleFailureTest(int expectedErrorCode) throws Exception {
         Download download = enqueueRequest(getRequest());
         download.runUntilStatus(DownloadManager.STATUS_FAILED);