Merge pull request #1561 from microsoft/feature/downloading-check

Add timeout for download manager

CHANGELOG.md

@@ -6,6 +6,11 @@
 
 * **[Breaking change]** Remove `AppCenter.setCustomProperties` API.
 * **[Fix]** Remove `android.support.test.InstrumentationRegistry` string that caused an error when checking applications on availability of android support libraries.
+
+### App Center Distribute
+
+* **[Feature]** Remove the download manager task if the download doesn't start within 10 seconds.
+
 ___
 
 ## Version 4.3.1

sdk/appcenter-distribute/src/main/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerRequestTask.java

@@ -6,26 +6,36 @@
 package com.microsoft.appcenter.distribute.download.manager;
 
 import android.app.DownloadManager;
+import android.database.Cursor;
 import android.net.Uri;
 import android.os.AsyncTask;
+import android.os.Handler;
+import android.os.Looper;
+
 import androidx.annotation.VisibleForTesting;
 
 import com.microsoft.appcenter.distribute.ReleaseDetails;
 import com.microsoft.appcenter.utils.AppCenterLog;
 
 import static com.microsoft.appcenter.distribute.DistributeConstants.LOG_TAG;
 
+import java.util.concurrent.TimeoutException;
+
 /**
  * The download manager API triggers strict mode exception in UI thread.
  */
 class DownloadManagerRequestTask extends AsyncTask<Void, Void, Void> {
-
+
+    private final int TIMEOUT_LIMIT = 10000;
     private final DownloadManagerReleaseDownloader mDownloader;
     private final String mTitle;
+    private Handler mHandler;
+    private Runnable handlerCallback;
 
     DownloadManagerRequestTask(DownloadManagerReleaseDownloader downloader, String title) {
         mDownloader = downloader;
         mTitle = title;
+        mHandler = new Handler(Looper.getMainLooper());
     }
 
     @Override
@@ -35,7 +45,7 @@ protected Void doInBackground(Void... params) {
         ReleaseDetails releaseDetails = mDownloader.getReleaseDetails();
         Uri downloadUrl = releaseDetails.getDownloadUrl();
         AppCenterLog.debug(LOG_TAG, "Start downloading new release from " + downloadUrl);
-        DownloadManager downloadManager = mDownloader.getDownloadManager();
+        final DownloadManager downloadManager = mDownloader.getDownloadManager();
         DownloadManager.Request request = createRequest(downloadUrl);
         request.setTitle(String.format(mTitle, releaseDetails.getShortVersion(), releaseDetails.getVersion()));
 
@@ -46,12 +56,28 @@ protected Void doInBackground(Void... params) {
         }
         long enqueueTime = System.currentTimeMillis();
         try {
-            long downloadId = downloadManager.enqueue(request);
+            final long downloadId = downloadManager.enqueue(request);
             if (!isCancelled()) {
                 mDownloader.onDownloadStarted(downloadId, enqueueTime);
+                handlerCallback = new Runnable() {
+
+                    @Override
+                    public void run() {
+                        DownloadManager.Query query = new DownloadManager.Query();
+                        query.setFilterByStatus(DownloadManager.STATUS_PENDING);
+                        Cursor c = downloadManager.query(query);
+                        if (c.moveToFirst()) {
+                            downloadManager.remove(downloadId);
+                            mDownloader.onDownloadError(new IllegalStateException("Failed to start downloading file due to timeout exception."));
+                        }
+                    }
+                };
+
+                /* Check that the file started to download. */
+                mHandler.postDelayed(handlerCallback, TIMEOUT_LIMIT);
             }
         } catch (IllegalArgumentException e) {
-            
+
             /*
              * In cases when Download Manager application is disabled,
              * IllegalArgumentException: Unknown URL content://downloads/my_download is thrown.
@@ -63,6 +89,9 @@ protected Void doInBackground(Void... params) {
 
     @VisibleForTesting
     DownloadManager.Request createRequest(Uri Uri) {
+        if (handlerCallback != null) {
+            mHandler.removeCallbacks(handlerCallback);
+        }
         return new DownloadManager.Request(Uri);
     }
 }

sdk/appcenter-distribute/src/test/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerRequestTaskTest.java

@@ -6,30 +6,49 @@
 package com.microsoft.appcenter.distribute.download.manager;
 
 import android.app.DownloadManager;
+import android.database.Cursor;
 import android.net.Uri;
+import android.os.AsyncTask;
+import android.os.Handler;
 
 import com.microsoft.appcenter.distribute.ReleaseDetails;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Matchers;
 import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.rule.PowerMockRule;
 
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyLong;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
+import static org.powermock.api.mockito.PowerMockito.whenNew;
 
+
+@PrepareForTest({
+        DownloadManagerRequestTask.class,
+        AsyncTask.class
+})
 @RunWith(MockitoJUnitRunner.class)
 public class DownloadManagerRequestTaskTest {
 
     private static final long DOWNLOAD_ID = 42;
 
+    @Rule
+    public PowerMockRule mRule = new PowerMockRule();
+
     @Mock
     private ReleaseDetails mReleaseDetails;
 
@@ -42,10 +61,19 @@ public class DownloadManagerRequestTaskTest {
     @Mock
     private DownloadManagerReleaseDownloader mDownloader;
 
+    @Mock
+    private Handler mHandler;
+
+    @Mock
+    private Cursor mCursor;
+
     private DownloadManagerRequestTask mRequestTask;
 
     @Before
-    public void setUp() {
+    public void setUp() throws Exception {
+
+        /* Mock Handler. */
+        whenNew(Handler.class).withAnyArguments().thenReturn(mHandler);
 
         /* Mock DownloadManager. */
         when(mDownloadManager.enqueue(eq(mDownloadManagerRequest))).thenReturn(DOWNLOAD_ID);
@@ -79,7 +107,6 @@ public void downloadStarted() {
     public void hideNotificationOnMandatoryUpdate() {
         when(mReleaseDetails.getVersion()).thenReturn(1);
         when(mReleaseDetails.getShortVersion()).thenReturn("1");
-        when(mReleaseDetails.isMandatoryUpdate()).thenReturn(false);
         when(mReleaseDetails.isMandatoryUpdate()).thenReturn(true);
 
         /* Perform background task. */
@@ -118,4 +145,67 @@ public void enqueueTaskIllegalStateExceptionHandled() {
         verify(mDownloader).onDownloadError(any(IllegalStateException.class));
         verify(mDownloader, never()).onDownloadStarted(anyLong(), anyLong());
     }
+
+    @Test
+    public void noExceptionsWhenDownloadFinishedAfterTimeout() {
+        when(mDownloadManager.query(Matchers.<DownloadManager.Query>any())).thenReturn(mCursor);
+        when(mCursor.moveToFirst()).thenReturn(false);
+        when(mRequestTask.isCancelled()).thenReturn(false);
+
+        /* Run Callback immediately. */
+        when(mHandler.postDelayed(Matchers.<Runnable>any(), anyLong())).thenAnswer(new Answer<Object>() {
+
+            @Override
+            public Object answer(InvocationOnMock invocation) {
+                ((Runnable) invocation.getArguments()[0]).run();
+                return true;
+            }
+        });
+
+        /* Perform background task. */
+        mRequestTask.doInBackground();
+
+        /* Verify. */
+        verify(mDownloader, never()).onDownloadError(any(IllegalStateException.class));
+    }
+
+
+    @Test
+    public void throwExceptionWhenDownloadStillNotFinishedAfterTimeout() {
+        when(mDownloadManager.query(Matchers.<DownloadManager.Query>any())).thenReturn(mCursor);
+        when(mCursor.moveToFirst()).thenReturn(true);
+        when(mCursor.getInt(anyInt())).thenReturn(1);
+        when(mRequestTask.isCancelled()).thenReturn(false);
+
+        /* Run Callback immediately. */
+        when(mHandler.postDelayed(Matchers.<Runnable>any(), anyLong())).thenAnswer(new Answer<Object>() {
+
+            @Override
+            public Object answer(InvocationOnMock invocation) {
+                ((Runnable) invocation.getArguments()[0]).run();
+                return true;
+            }
+        });
+
+        /* Perform background task. */
+        mRequestTask.doInBackground();
+
+        /* Verify that exception was thrown. */
+        verify(mDownloader).onDownloadError(any(IllegalStateException.class));
+    }
+
+    @Test
+    public void oldCallbacksRemovedBeforeCreatingNewDownload() {
+        when(mRequestTask.isCancelled()).thenReturn(false);
+        when(mRequestTask.createRequest(Matchers.<Uri>any())).thenCallRealMethod();
+
+        /* Perform background task. Emulates creating callback, which would not be used */
+        mRequestTask.doInBackground();
+
+        /* Perform background task. Must delete old callbacks */
+        mRequestTask.doInBackground();
+
+        /* Verify that callback was removed. */
+        verify(mHandler).removeCallbacks(Matchers.<Runnable>any());
+    }
 }