Add test for many interruptions to a single download.
Steve Howard [Tue, 27 Jul 2010 23:28:40 +0000 (16:28 -0700)]
Adding a new test case for downloads that undergo many interruptions
(as may happen with a very large download that takes many hours).
Includes some refactoring in the test suite.

Early on, this test exposed a race condition in which the download
manager got some I/O exception while reading from the MockWebServer.
I went in and improved/refactored much of the error logging code in
DownloadThread to try and track this down.  Unfortunately, once I
finished, the race condition no longer seems to be reproducible, even
with hundreds of runs of the test case.  So I've given up on it for
now.  In any event, error logging is better and much duplicate code
has been eliminated.

src/com/android/providers/downloads/DownloadThread.java
tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java
tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java

index 736bbce..be02e3e 100644 (file)
@@ -116,7 +116,13 @@ public class DownloadThread extends Thread {
      * Raised from methods called by run() to indicate that the current request should be stopped
      * immediately.
      */
-    private class StopRequest extends Exception {}
+    private class StopRequest extends Exception {
+        public StopRequest() {}
+
+        public StopRequest(Throwable throwable) {
+            super(throwable);
+        }
+    }
 
     /**
      * Raised from methods called by executeDownload() to indicate that the download should be
@@ -165,6 +171,9 @@ public class DownloadThread extends Thread {
             }
             state.mFinalStatus = Downloads.Impl.STATUS_SUCCESS;
         } catch (StopRequest error) {
+            if (Constants.LOGV) {
+                Log.v(Constants.TAG, "Aborting request for " + mInfo.mUri, error);
+            }
             // fall through to finally block
         } catch (FileNotFoundException ex) {
             Log.d(Constants.TAG, "FileNotFoundException for " + state.mFilename + " : " +  ex);
@@ -424,7 +433,7 @@ public class DownloadThread extends Thread {
             } catch (IOException ex) {
                 if (!Helpers.discardPurgeableFiles(mContext, Constants.BUFFER_SIZE)) {
                     state.mFinalStatus = Downloads.Impl.STATUS_FILE_ERROR;
-                    throw new StopRequest();
+                    throw new StopRequest(ex);
                 }
             }
         }
@@ -454,19 +463,8 @@ public class DownloadThread extends Thread {
                             mInfo.mId);
                 }
                 state.mFinalStatus = Downloads.Impl.STATUS_LENGTH_REQUIRED;
-            } else if (!Helpers.isNetworkAvailable(mSystemFacade)) {
-                state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
-            } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
-                state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
-                state.mCountRetry = true;
             } else {
-                if (Constants.LOGV) {
-                    Log.v(Constants.TAG, "closed socket for " + mInfo.mUri);
-                } else if (Config.LOGD) {
-                    Log.d(Constants.TAG, "closed socket for download " +
-                            mInfo.mId);
-                }
-                state.mFinalStatus = Downloads.Impl.STATUS_HTTP_DATA_ERROR;
+                handleHttpError(state, "closed socket");
             }
             throw new StopRequest();
         }
@@ -483,30 +481,18 @@ public class DownloadThread extends Thread {
         try {
             return entityStream.read(data);
         } catch (IOException ex) {
-            if (Constants.LOGX) {
-                if (Helpers.isNetworkAvailable(mSystemFacade)) {
-                    Log.i(Constants.TAG, "Read Failed " + mInfo.mId + ", Net Up");
-                } else {
-                    Log.i(Constants.TAG, "Read Failed " + mInfo.mId + ", Net Down");
-                }
-            }
+            logNetworkState();
             ContentValues values = new ContentValues();
             values.put(Downloads.Impl.COLUMN_CURRENT_BYTES, innerState.mBytesSoFar);
             mContext.getContentResolver().update(state.mContentUri, values, null, null);
             if (!mInfo.mNoIntegrity && innerState.mHeaderETag == null) {
-                Log.d(Constants.TAG, "download IOException for download " + mInfo.mId + " : " + ex);
+                Log.d(Constants.TAG, "download IOException for download " + mInfo.mId, ex);
                 Log.d(Constants.TAG, "can't resume interrupted download with no ETag");
                 state.mFinalStatus = Downloads.Impl.STATUS_PRECONDITION_FAILED;
-            } else if (!Helpers.isNetworkAvailable(mSystemFacade)) {
-                state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
-            } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
-                state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
-                state.mCountRetry = true;
             } else {
-                Log.d(Constants.TAG, "download IOException for download " + mInfo.mId + " : " + ex);
-                state.mFinalStatus = Downloads.Impl.STATUS_HTTP_DATA_ERROR;
+                handleHttpError(state, "download IOException");
             }
-            throw new StopRequest();
+            throw new StopRequest(ex);
         }
     }
 
@@ -519,32 +505,16 @@ public class DownloadThread extends Thread {
         try {
             return response.getEntity().getContent();
         } catch (IOException ex) {
-            if (Constants.LOGX) {
-                if (Helpers.isNetworkAvailable(mSystemFacade)) {
-                    Log.i(Constants.TAG, "Get Failed " + mInfo.mId + ", Net Up");
-                } else {
-                    Log.i(Constants.TAG, "Get Failed " + mInfo.mId + ", Net Down");
-                }
-            }
-            if (!Helpers.isNetworkAvailable(mSystemFacade)) {
-                state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
-            } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
-                state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
-                state.mCountRetry = true;
-            } else {
-                if (Constants.LOGV) {
-                    Log.d(Constants.TAG,
-                            "IOException getting entity for " +
-                            mInfo.mUri +
-                            " : " +
-                            ex);
-                } else if (Config.LOGD) {
-                    Log.d(Constants.TAG, "IOException getting entity for download " +
-                            mInfo.mId + " : " + ex);
-                }
-                state.mFinalStatus = Downloads.Impl.STATUS_HTTP_DATA_ERROR;
-            }
-            throw new StopRequest();
+            logNetworkState();
+            handleHttpError(state, "IOException getting entity");
+            throw new StopRequest(ex);
+        }
+    }
+
+    private void logNetworkState() {
+        if (Constants.LOGX) {
+            Log.i(Constants.TAG,
+                    "Net " + (Helpers.isNetworkAvailable(mSystemFacade) ? "Up" : "Down"));
         }
     }
 
@@ -806,31 +776,27 @@ public class DownloadThread extends Thread {
                         mInfo.mId + " : " +  ex);
             }
             state.mFinalStatus = Downloads.Impl.STATUS_BAD_REQUEST;
-            throw new StopRequest();
+            throw new StopRequest(ex);
         } catch (IOException ex) {
-            if (Constants.LOGX) {
-                if (Helpers.isNetworkAvailable(mSystemFacade)) {
-                    Log.i(Constants.TAG, "Execute Failed " + mInfo.mId + ", Net Up");
-                } else {
-                    Log.i(Constants.TAG, "Execute Failed " + mInfo.mId + ", Net Down");
-                }
-            }
-            if (!Helpers.isNetworkAvailable(mSystemFacade)) {
-                state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
-            } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
-                state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
-                state.mCountRetry = true;
-            } else {
-                if (Constants.LOGV) {
-                    Log.d(Constants.TAG, "IOException trying to execute request for " +
-                            mInfo.mUri + " : " + ex);
-                } else if (Config.LOGD) {
-                    Log.d(Constants.TAG, "IOException trying to execute request for " +
-                            mInfo.mId + " : " + ex);
-                }
-                state.mFinalStatus = Downloads.Impl.STATUS_HTTP_DATA_ERROR;
-            }
-            throw new StopRequest();
+            logNetworkState();
+            handleHttpError(state, "IOException trying to execute request");
+            throw new StopRequest(ex);
+        }
+    }
+
+    private void handleHttpError(State state, String message) {
+        if (Constants.LOGV) {
+            Log.d(Constants.TAG, message + " for " + mInfo.mUri);
+        }
+
+        if (!Helpers.isNetworkAvailable(mSystemFacade)) {
+            state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
+        } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
+            state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
+            state.mCountRetry = true;
+        } else {
+            Log.d(Constants.TAG, "reached max retries: " + message + " for " + mInfo.mId);
+            state.mFinalStatus = Downloads.Impl.STATUS_HTTP_DATA_ERROR;
         }
     }
 
index 9e7ef0f..3e4bccc 100644 (file)
@@ -49,7 +49,7 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
     protected static final String LOG_TAG = "DownloadManagerFunctionalTest";
     private static final String PROVIDER_AUTHORITY = "downloads";
     protected static final long RETRY_DELAY_MILLIS = 61 * 1000;
-    protected static final String FILE_CONTENT = "hello world";
+    protected static final String FILE_CONTENT = "hello world hello world hello world hello world";
     protected static final int HTTP_OK = 200;
     protected static final int HTTP_PARTIAL_CONTENT = 206;
     protected static final int HTTP_NOT_FOUND = 404;
@@ -209,8 +209,9 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
     MockResponse enqueueResponse(int status, String body) {
         MockResponse response = new MockResponse()
                                 .setResponseCode(status)
-                                .setBody(body);
-        response.addHeader("Content-type", "text/plain");
+                                .setBody(body)
+                                .addHeader("Content-type", "text/plain")
+                                .setCloseConnectionAfter(true);
         mServer.enqueue(response);
         return response;
     }
index c1a015a..d33e458 100644 (file)
@@ -102,7 +102,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
         assertEquals(mSystemFacade.currentTimeMillis(),
                      download.getLongField(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP));
 
-        assertEquals(FILE_CONTENT, download.getContents());
+        checkCompleteDownload(download);
     }
 
     public void testTitleAndDescription() throws Exception {
@@ -138,9 +138,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
 
         mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
         download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
-        assertEquals(FILE_CONTENT.length(),
-                     download.getLongField(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));
-        assertEquals(FILE_CONTENT, download.getContents());
+        checkCompleteDownload(download);
 
         List<String> headers = takeRequest().getHeaders();
         assertTrue("No Range header: " + headers,
@@ -150,30 +148,32 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
 
     public void testInterruptedExternalDownload() throws Exception {
         enqueueInterruptedDownloadResponses(5);
-        Uri destination = getExternalUri();
-        Download download = enqueueRequest(getRequest().setDestinationUri(destination));
+        Download download = enqueueRequest(getRequest().setDestinationUri(getExternalUri()));
         download.runUntilStatus(DownloadManager.STATUS_PAUSED);
         mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
         download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
-        assertEquals(FILE_CONTENT, download.getContents());
+        checkCompleteDownload(download);
     }
 
     private void enqueueInterruptedDownloadResponses(int initialLength) {
-        int totalLength = FILE_CONTENT.length();
         // the first response has normal headers but unexpectedly closes after initialLength bytes
-        enqueuePartialResponse(initialLength);
+        enqueuePartialResponse(0, initialLength);
         // the second response returns partial content for the rest of the data
-        enqueueResponse(HTTP_PARTIAL_CONTENT, FILE_CONTENT.substring(initialLength))
-                .addHeader("Content-range",
-                           "bytes " + initialLength + "-" + totalLength + "/" + totalLength)
-                .addHeader("Etag", ETAG);
+        enqueuePartialResponse(initialLength, FILE_CONTENT.length());
     }
 
-    private MockResponse enqueuePartialResponse(int initialLength) {
-        return enqueueResponse(HTTP_OK, FILE_CONTENT.substring(0, initialLength))
-                               .addHeader("Content-length", FILE_CONTENT.length())
-                               .addHeader("Etag", ETAG)
-                               .setCloseConnectionAfter(true);
+    private MockResponse enqueuePartialResponse(int start, int end) {
+        int totalLength = FILE_CONTENT.length();
+        boolean isFirstResponse = (start == 0);
+        int status = isFirstResponse ? HTTP_OK : HTTP_PARTIAL_CONTENT;
+        MockResponse response = enqueueResponse(status, FILE_CONTENT.substring(start, end))
+                               .addHeader("Content-length", totalLength)
+                               .addHeader("Etag", ETAG);
+        if (!isFirstResponse) {
+            response.addHeader("Content-range",
+                    "bytes " + start + "-" + totalLength + "/" + totalLength);
+        }
+        return response;
     }
 
     public void testFiltering() throws Exception {
@@ -304,7 +304,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
     }
 
     public void testNoEtag() throws Exception {
-        enqueuePartialResponse(5).removeHeader("Etag");
+        enqueuePartialResponse(0, 5).removeHeader("Etag");
         runSimpleFailureTest(HTTP_LENGTH_REQUIRED);
     }
 
@@ -334,7 +334,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
     }
 
     public void testCancel() throws Exception {
-        enqueuePartialResponse(5);
+        enqueuePartialResponse(0, 5);
         Download download = enqueueRequest(getRequest());
         download.runUntilStatus(DownloadManager.STATUS_PAUSED);
 
@@ -463,6 +463,30 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
         download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
     }
 
+    public void testManyInterruptions() throws Exception {
+        int bytesPerResponse = 1;
+        int start = 0;
+
+        Download download = enqueueRequest(getRequest());
+        while (start + bytesPerResponse < FILE_CONTENT.length()) {
+            enqueuePartialResponse(start, start + bytesPerResponse);
+            download.runUntilStatus(DownloadManager.STATUS_PAUSED);
+            takeRequest();
+            start += bytesPerResponse;
+            mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
+        }
+
+        enqueuePartialResponse(start, FILE_CONTENT.length());
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        checkCompleteDownload(download);
+    }
+
+    private void checkCompleteDownload(Download download) throws Exception {
+        assertEquals(FILE_CONTENT.length(),
+                     download.getLongField(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));
+        assertEquals(FILE_CONTENT, download.getContents());
+    }
+
     private void runSimpleFailureTest(int expectedErrorCode) throws Exception {
         Download download = enqueueRequest(getRequest());
         download.runUntilStatus(DownloadManager.STATUS_FAILED);