diff options
8 files changed, 181 insertions, 69 deletions
@@ -8,6 +8,7 @@ LOCAL_SRC_FILES := $(call all-java-files-under, src) LOCAL_PACKAGE_NAME := DownloadProvider LOCAL_CERTIFICATE := media +LOCAL_STATIC_JAVA_LIBRARIES := guava include $(BUILD_PACKAGE) diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java index 9e890ea0..f3bc9586 100644 --- a/src/com/android/providers/downloads/DownloadService.java +++ b/src/com/android/providers/downloads/DownloadService.java @@ -17,6 +17,7 @@ package com.android.providers.downloads; import com.google.android.collect.Lists; +import com.google.common.annotations.VisibleForTesting; import android.app.AlarmManager; import android.app.PendingIntent; @@ -62,7 +63,7 @@ public class DownloadService extends Service { /** Observer to get notified when the content observer's data changes */ private DownloadManagerContentObserver mObserver; - + /** Class to handle Notification Manager updates */ private DownloadNotification mNotifier; @@ -109,6 +110,9 @@ public class DownloadService extends Service { */ private CharArrayBuffer mNewChars; + @VisibleForTesting + SystemFacade mSystemFacade; + /* ------------ Inner Classes ------------ */ /** @@ -200,6 +204,10 @@ public class DownloadService extends Service { Log.v(Constants.TAG, "Service onCreate"); } + if (mSystemFacade == null) { + mSystemFacade = new RealSystemFacade(); + } + mDownloads = Lists.newArrayList(); mObserver = new DownloadManagerContentObserver(); @@ -209,7 +217,7 @@ public class DownloadService extends Service { mMediaScannerService = null; mMediaScannerConnecting = false; mMediaScannerConnection = new MediaScannerConnection(); - + mNotifier = new DownloadNotification(this); mNotifier.mNotificationMgr.cancelAll(); mNotifier.updateNotification(); @@ -259,10 +267,10 @@ public class DownloadService extends Service { public UpdateThread() { super("Download Service"); } - + public void run() { Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); - + boolean keepService = false; // for each update from the database, remember which download is // supposed to get restarted soonest in the future @@ -292,7 +300,7 @@ public class DownloadService extends Service { DownloadReceiver.class.getName()); alarms.set( AlarmManager.RTC_WAKEUP, - System.currentTimeMillis() + wakeUp, + mSystemFacade.currentTimeMillis() + wakeUp, PendingIntent.getBroadcast(DownloadService.this, 0, intent, PendingIntent.FLAG_ONE_SHOT)); } @@ -305,7 +313,7 @@ public class DownloadService extends Service { } boolean networkAvailable = Helpers.isNetworkAvailable(DownloadService.this); boolean networkRoaming = Helpers.isNetworkRoaming(DownloadService.this); - long now = System.currentTimeMillis(); + long now = mSystemFacade.currentTimeMillis(); Cursor cursor = getContentResolver().query(Downloads.Impl.CONTENT_URI, null, null, null, Downloads.Impl._ID); @@ -666,7 +674,7 @@ public class DownloadService extends Service { ContentUris.withAppendedId(Downloads.Impl.CONTENT_URI, info.mId), values, null, null); } - DownloadThread downloader = new DownloadThread(this, info); + DownloadThread downloader = new DownloadThread(this, mSystemFacade, info); info.mHasActiveThread = true; downloader.start(); } @@ -757,7 +765,7 @@ public class DownloadService extends Service { getContentResolver().update( ContentUris.withAppendedId(Downloads.Impl.CONTENT_URI, info.mId), values, null, null); - DownloadThread downloader = new DownloadThread(this, info); + DownloadThread downloader = new DownloadThread(this, mSystemFacade, info); info.mHasActiveThread = true; downloader.start(); } diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java index d2bd3220..f1296b8f 100644 --- a/src/com/android/providers/downloads/DownloadThread.java +++ b/src/com/android/providers/downloads/DownloadThread.java @@ -56,9 +56,11 @@ public class DownloadThread extends Thread { private Context mContext; private DownloadInfo mInfo; + private SystemFacade mSystemFacade; - public DownloadThread(Context context, DownloadInfo info) { + public DownloadThread(Context context, SystemFacade systemFacade, DownloadInfo info) { mContext = context; + mSystemFacade = systemFacade; mInfo = info; } @@ -131,7 +133,7 @@ public class DownloadThread extends Thread { // Tough luck, that's not a resumable download if (Config.LOGD) { Log.d(Constants.TAG, - "can't resume interrupted non-resumable download"); + "can't resume interrupted non-resumable download"); } f.delete(); finalStatus = Downloads.Impl.STATUS_PRECONDITION_FAILED; @@ -363,7 +365,7 @@ http_request_loop: if (mimeType == null) { header = response.getFirstHeader("Content-Type"); if (header != null) { - mimeType = sanitizeMimeType(header.getValue()); + mimeType = sanitizeMimeType(header.getValue()); } } header = response.getFirstHeader("ETag"); @@ -593,7 +595,7 @@ http_request_loop: } } bytesSoFar += bytesRead; - long now = System.currentTimeMillis(); + long now = mSystemFacade.currentTimeMillis(); if (bytesSoFar - bytesNotified > Constants.MIN_PROGRESS_STEP && now - timeLastNotification > Constants.MIN_PROGRESS_TIME) { @@ -678,7 +680,7 @@ http_request_loop: } else if (Downloads.Impl.isStatusSuccess(finalStatus) && DrmRawContent.DRM_MIMETYPE_MESSAGE_STRING .equalsIgnoreCase(mimeType)) { - // transfer the file to the DRM content provider + // transfer the file to the DRM content provider File file = new File(filename); Intent item = DrmStore.addDrmFile(mContext.getContentResolver(), file, null); if (item == null) { @@ -688,7 +690,7 @@ http_request_loop: filename = item.getDataString(); mimeType = item.getType(); } - + file.delete(); } else if (Downloads.Impl.isStatusSuccess(finalStatus)) { // make sure the file is readable @@ -748,7 +750,7 @@ http_request_loop: values.put(Downloads.Impl.COLUMN_URI, uri); } values.put(Downloads.Impl.COLUMN_MIME_TYPE, mimeType); - values.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, System.currentTimeMillis()); + values.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, mSystemFacade.currentTimeMillis()); values.put(Constants.RETRY_AFTER_X_REDIRECT_COUNT, retryAfter + (redirectCount << 28)); if (!countRetry) { values.put(Constants.FAILED_CONNECTIONS, 0); diff --git a/src/com/android/providers/downloads/RealSystemFacade.java b/src/com/android/providers/downloads/RealSystemFacade.java new file mode 100644 index 00000000..88f10d8c --- /dev/null +++ b/src/com/android/providers/downloads/RealSystemFacade.java @@ -0,0 +1,7 @@ +package com.android.providers.downloads; + +class RealSystemFacade implements SystemFacade { + public long currentTimeMillis() { + return System.currentTimeMillis(); + } +} diff --git a/src/com/android/providers/downloads/SystemFacade.java b/src/com/android/providers/downloads/SystemFacade.java new file mode 100644 index 00000000..4498877d --- /dev/null +++ b/src/com/android/providers/downloads/SystemFacade.java @@ -0,0 +1,6 @@ + +package com.android.providers.downloads; + +interface SystemFacade { + public long currentTimeMillis(); +} diff --git a/tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java b/tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java index 76b3d589..8d4655bb 100644 --- a/tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java @@ -26,7 +26,9 @@ import android.database.Cursor; import android.net.ConnectivityManager; import android.net.NetworkInfo; import android.net.Uri; +import android.os.Environment; import android.provider.Downloads; +import android.test.MoreAsserts; import android.test.RenamingDelegatingContext; import android.test.ServiceTestCase; import android.test.mock.MockContentResolver; @@ -55,18 +57,23 @@ import java.util.Set; */ @LargeTest public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadService> { - private static final int HTTP_PARTIAL_CONTENT = 206; + private static final String LOG_TAG = "DownloadManagerFunctionalTest"; + private static final String PROVIDER_AUTHORITY = "downloads"; + private static final int RETRY_DELAY_MILLIS = 61 * 1000; + private static final long REQUEST_TIMEOUT_MILLIS = 10 * 1000; + private static final String FILE_CONTENT = "hello world"; + private static final int HTTP_OK = 200; - private static final String LOG_TAG = "DownloadManagerFunctionalTest"; + private static final int HTTP_PARTIAL_CONTENT = 206; private static final int HTTP_NOT_FOUND = 404; - private static final String FILE_CONTENT = "hello world"; - private static final long REQUEST_TIMEOUT_MILLIS = 5000; + private static final int HTTP_SERVICE_UNAVAILABLE = 503; private MockWebServer mServer; // resolves requests to the DownloadProvider we set up private MockContentResolver mResolver; private TestContext mTestContext; + private FakeSystemFacade mSystemFacade; /** * Context passed to the provider and the service. Allows most methods to pass through to the @@ -148,6 +155,9 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi mTestContext.setResolver(mResolver); setContext(mTestContext); + setupService(); + mSystemFacade = new FakeSystemFacade(); + getService().mSystemFacade = mSystemFacade; mServer = new MockWebServer(); mServer.play(); @@ -205,26 +215,55 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi assertEquals(Downloads.STATUS_PENDING, getDownloadStatus(downloadUri)); assertTrue(mTestContext.mHasServiceBeenStarted); - startService(null); - - RecordedRequest request = takeRequest(); + RecordedRequest request = runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS); assertEquals("GET", request.getMethod()); assertEquals(path, request.getPath()); + assertEquals(FILE_CONTENT, getDownloadContents(downloadUri)); + assertStartsWith(Environment.getExternalStorageDirectory().getPath(), + getDownloadFilename(downloadUri)); + } - waitForDownloadToStop(downloadUri, Downloads.STATUS_SUCCESS); + public void testDownloadToCache() throws Exception { + enqueueResponse(HTTP_OK, FILE_CONTENT); + Uri downloadUri = requestDownload("/path"); + updateDownload(downloadUri, Downloads.COLUMN_DESTINATION, + Integer.toString(Downloads.DESTINATION_CACHE_PARTITION)); + runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS); assertEquals(FILE_CONTENT, getDownloadContents(downloadUri)); - checkForUnexpectedRequests(); + assertStartsWith(Environment.getDownloadCacheDirectory().getPath(), + getDownloadFilename(downloadUri)); } public void testFileNotFound() throws Exception { enqueueEmptyResponse(HTTP_NOT_FOUND); Uri downloadUri = requestDownload("/nonexistent_path"); assertEquals(Downloads.STATUS_PENDING, getDownloadStatus(downloadUri)); + runUntilStatus(downloadUri, HTTP_NOT_FOUND); + } - startService(null); - takeRequest(); - waitForDownloadToStop(downloadUri, HTTP_NOT_FOUND); - checkForUnexpectedRequests(); + public void testRetryAfter() throws Exception { + final int delay = 120; + enqueueEmptyResponse(HTTP_SERVICE_UNAVAILABLE).addHeader("Retry-after", delay); + Uri downloadUri = requestDownload("/path"); + runUntilStatus(downloadUri, Downloads.STATUS_RUNNING_PAUSED); + + // download manager adds random 0-30s offset + mSystemFacade.incrementTimeMillis((delay + 31) * 1000); + + enqueueResponse(HTTP_OK, FILE_CONTENT); + runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS); + } + + public void testRedirect() throws Exception { + enqueueEmptyResponse(301).addHeader("Location", mServer.getUrl("/other_path").toString()); + enqueueResponse(HTTP_OK, FILE_CONTENT); + Uri downloadUri = requestDownload("/path"); + RecordedRequest request = runUntilStatus(downloadUri, Downloads.STATUS_RUNNING_PAUSED); + assertEquals("/path", request.getPath()); + + mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS); + request = runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS); + assertEquals("/other_path", request.getPath()); } public void testBasicConnectivityChanges() throws Exception { @@ -235,18 +274,13 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi mTestContext.mFakeIConnectivityManager.setNetworkState(NetworkInfo.State.DISCONNECTED); startService(null); waitForDownloadToStop(downloadUri, Downloads.STATUS_RUNNING_PAUSED); - checkForUnexpectedRequests(); // connecting should start the download mTestContext.mFakeIConnectivityManager.setNetworkState(NetworkInfo.State.CONNECTED); - startService(null); // normally done by DownloadReceiver - takeRequest(); - waitForDownloadToStop(downloadUri, Downloads.STATUS_SUCCESS); - checkForUnexpectedRequests(); + runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS); } - // disabled due to excessive sleep - public void disabledTestInterruptedDownload() throws Exception { + public void testInterruptedDownload() throws Exception { int initialLength = 5; String etag = "my_etag"; int totalLength = FILE_CONTENT.length(); @@ -257,12 +291,9 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi .setCloseConnectionAfter(true); Uri downloadUri = requestDownload("/path"); - startService(null); - takeRequest(); - waitForDownloadToStop(downloadUri, Downloads.STATUS_RUNNING_PAUSED); + runUntilStatus(downloadUri, Downloads.STATUS_RUNNING_PAUSED); - Thread.sleep(61 * 1000); // TODO: avoid this by stubbing the system clock - mServer.drainRequests(); + mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS); // the second response returns partial content for the rest of the data enqueueResponse(HTTP_PARTIAL_CONTENT, FILE_CONTENT.substring(initialLength)) .addHeader("Content-range", @@ -270,17 +301,18 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi .addHeader("Etag", etag); // TODO: ideally we wouldn't need to call startService again, but there's a bug where the // service won't retry a download until an intent comes in - startService(null); - waitForDownloadToStop(downloadUri, Downloads.STATUS_SUCCESS); + RecordedRequest request = runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS); - RecordedRequest request = takeRequest(); List<String> headers = request.getHeaders(); assertTrue("No Range header: " + headers, headers.contains("Range: bytes=" + initialLength + "-")); assertTrue("No ETag header: " + headers, headers.contains("If-Match: " + etag)); - assertEquals(FILE_CONTENT, getDownloadContents(downloadUri)); - checkForUnexpectedRequests(); + } + + private void assertStartsWith(String expectedPrefix, String actual) { + String regex = "^" + expectedPrefix + ".*"; + MoreAsserts.assertMatchesRegex(regex, actual); } /** @@ -295,8 +327,19 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi return response; } - private void enqueueEmptyResponse(int status) { - enqueueResponse(status, ""); + private MockResponse enqueueEmptyResponse(int status) { + return enqueueResponse(status, ""); + } + + /** + * Run the service and wait for a request and for the download to reach the given status. + * @return the request received + */ + private RecordedRequest runUntilStatus(Uri downloadUri, int status) throws Exception { + startService(null); + RecordedRequest request = takeRequest(); + waitForDownloadToStop(downloadUri, status); + return request; } /** @@ -324,7 +367,7 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi * Wait for a download to given a given status, with a timeout. Fails if the download reaches * any other final status. */ - private void waitForDownloadToStop(Uri downloadUri, int expectedStatus) { + private void waitForDownloadToStop(Uri downloadUri, int expectedStatus) throws Exception { // TODO(showard): find a better way to accomplish this long startTimeMillis = System.currentTimeMillis(); int status = getDownloadStatus(downloadUri); @@ -335,11 +378,8 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi if (System.currentTimeMillis() > startTimeMillis + REQUEST_TIMEOUT_MILLIS) { fail("Download timed out with status " + status); } - try { - Thread.sleep(100); - } catch (InterruptedException exc) { - // no problem - } + Thread.sleep(100); + mServer.checkForExceptions(); status = getDownloadStatus(downloadUri); } @@ -348,12 +388,20 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi } private int getDownloadStatus(Uri downloadUri) { - final String[] columns = new String[] {Downloads.COLUMN_STATUS}; + return Integer.valueOf(getDownloadField(downloadUri, Downloads.COLUMN_STATUS)); + } + + private String getDownloadFilename(Uri downloadUri) { + return getDownloadField(downloadUri, Downloads._DATA); + } + + private String getDownloadField(Uri downloadUri, String column) { + final String[] columns = new String[] {column}; Cursor cursor = mResolver.query(downloadUri, columns, null, null, null); try { assertEquals(1, cursor.getCount()); cursor.moveToFirst(); - return cursor.getInt(0); + return cursor.getString(0); } finally { cursor.close(); } @@ -369,6 +417,16 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi return mResolver.insert(Downloads.CONTENT_URI, values); } + /** + * Update one field of a download in the provider. + */ + private void updateDownload(Uri downloadUri, String column, String value) { + ContentValues values = new ContentValues(); + values.put(column, value); + int numChanged = mResolver.update(downloadUri, values, null, null); + assertEquals(1, numChanged); + } + private String readStream(InputStream inputStream) throws IOException { BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); try { @@ -380,16 +438,4 @@ public class DownloadManagerFunctionalTest extends ServiceTestCase<DownloadServi reader.close(); } } - - /** - * Check for any extra requests made to the MockWebServer that weren't consumed with - * {@link #takeRequest()}. - */ - private void checkForUnexpectedRequests() { - if (mServer == null) { - return; - } - List<RecordedRequest> extraRequests = mServer.drainRequests(); - assertEquals("Invalid requests: " + extraRequests.toString(), 0, extraRequests.size()); - } } diff --git a/tests/src/com/android/providers/downloads/FakeSystemFacade.java b/tests/src/com/android/providers/downloads/FakeSystemFacade.java new file mode 100644 index 00000000..b75e663a --- /dev/null +++ b/tests/src/com/android/providers/downloads/FakeSystemFacade.java @@ -0,0 +1,13 @@ +package com.android.providers.downloads; + +public class FakeSystemFacade implements SystemFacade { + private long mTimeMillis = 0; + + void incrementTimeMillis(long delta) { + mTimeMillis += delta; + } + + public long currentTimeMillis() { + return mTimeMillis; + } +} diff --git a/tests/src/tests/http/MockWebServer.java b/tests/src/tests/http/MockWebServer.java index b2cb8d7b..11c8063e 100644 --- a/tests/src/tests/http/MockWebServer.java +++ b/tests/src/tests/http/MockWebServer.java @@ -27,13 +27,18 @@ import java.net.ServerSocket; import java.net.Socket; import java.net.URL; import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; +import java.util.Queue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; /** * A scriptable web server. Callers supply canned responses and the server @@ -50,6 +55,8 @@ public final class MockWebServer { = new LinkedBlockingQueue<MockResponse>(); private int bodyLimit = Integer.MAX_VALUE; private final ExecutorService executor = Executors.newCachedThreadPool(); + // keep Futures around so we can rethrow any exceptions thrown by Callables + private final Queue<Future<?>> futures = new LinkedList<Future<?>>(); private int port = -1; @@ -107,7 +114,7 @@ public final class MockWebServer { final ServerSocket ss = new ServerSocket(0); ss.setReuseAddress(true); port = ss.getLocalPort(); - executor.submit(new Callable<Void>() { + submitCallable(new Callable<Void>() { public Void call() throws Exception { int count = 0; while (true) { @@ -125,7 +132,7 @@ public final class MockWebServer { } private void serveConnection(final Socket s) { - executor.submit(new Callable<Void>() { + submitCallable(new Callable<Void>() { public Void call() throws Exception { InputStream in = new BufferedInputStream(s.getInputStream()); OutputStream out = new BufferedOutputStream(s.getOutputStream()); @@ -156,6 +163,28 @@ public final class MockWebServer { }); } + private void submitCallable(Callable<?> callable) { + Future<?> future = executor.submit(callable); + futures.add(future); + } + + /** + * Check for and raise any exceptions that have been thrown by child threads. Will not block on + * children still running. + * @throws ExecutionException for the first child thread that threw an exception + */ + public void checkForExceptions() throws ExecutionException, InterruptedException { + final int originalSize = futures.size(); + for (int i = 0; i < originalSize; i++) { + Future<?> future = futures.remove(); + try { + future.get(0, TimeUnit.SECONDS); + } catch (TimeoutException e) { + futures.add(future); // still running + } + } + } + /** * @param sequenceNumber the index of this request on this connection. */ |