diff options
-rw-r--r-- | src/com/android/providers/downloads/DownloadThread.java | 151 | ||||
-rw-r--r-- | tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java | 12 |
2 files changed, 94 insertions, 69 deletions
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java index 56894feb..736bbce5 100644 --- a/src/com/android/providers/downloads/DownloadThread.java +++ b/src/com/android/providers/downloads/DownloadThread.java @@ -88,16 +88,18 @@ public class DownloadThread extends Thread { public String mNewUri; public Uri mContentUri; public boolean mGotData = false; + public String mRequestUri; public State(DownloadInfo info) { mMimeType = sanitizeMimeType(info.mMimeType); mRedirectCount = info.mRedirectCount; mContentUri = Uri.parse(Downloads.Impl.CONTENT_URI + "/" + info.mId); + mRequestUri = info.mUri; } } /** - * State within the outer try block of the run() method. + * State within executeDownload() */ private static class InnerState { public int mBytesSoFar = 0; @@ -117,6 +119,12 @@ public class DownloadThread extends Thread { private class StopRequest extends Exception {} /** + * Raised from methods called by executeDownload() to indicate that the download should be + * retried immediately. + */ + private class RetryDownload extends Exception {} + + /** * Executes the download in a separate thread */ public void run() { @@ -125,12 +133,8 @@ public class DownloadThread extends Thread { State state = new State(mInfo); AndroidHttpClient client = null; PowerManager.WakeLock wakeLock = null; - HttpGet request = null; try { - InnerState innerState = new InnerState(); - byte data[] = new byte[Constants.BUFFER_SIZE]; - PowerManager pm = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE); wakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, Constants.TAG); wakeLock.acquire(); @@ -139,36 +143,29 @@ public class DownloadThread extends Thread { if (Constants.LOGV) { Log.v(Constants.TAG, "initiating download for " + mInfo.mUri); } - setupDestinationFile(state, innerState); - client = AndroidHttpClient.newInstance(userAgent(), mContext); - request = new HttpGet(mInfo.mUri); - addRequestHeaders(innerState, request); - - // check connectivity just before sending - if (!mInfo.canUseNetwork()) { - state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED; - return; - } - HttpResponse response = sendRequest(state, client, request); - handleExceptionalStatus(state, innerState, response); + client = AndroidHttpClient.newInstance(userAgent(), mContext); - if (Constants.LOGV) { - Log.v(Constants.TAG, "received response for " + mInfo.mUri); + boolean finished = false; + while(!finished) { + HttpGet request = new HttpGet(state.mRequestUri); + try { + executeDownload(state, client, request); + finished = true; + } catch (RetryDownload exc) { + // fall through + } finally { + request.abort(); + request = null; + } } - processResponseHeaders(state, innerState, response); - InputStream entityStream = openResponseEntity(state, response); - transferData(state, innerState, data, entityStream); - if (Constants.LOGV) { Log.v(Constants.TAG, "download completed for " + mInfo.mUri); } state.mFinalStatus = Downloads.Impl.STATUS_SUCCESS; } catch (StopRequest error) { - if (request != null) { - request.abort(); - } + // fall through to finally block } catch (FileNotFoundException ex) { Log.d(Constants.TAG, "FileNotFoundException for " + state.mFilename + " : " + ex); state.mFinalStatus = Downloads.Impl.STATUS_FILE_ERROR; @@ -200,6 +197,43 @@ public class DownloadThread extends Thread { } /** + * Fully execute a single download request - setup and send the request, handle the response, + * and transfer the data to the destination file. + */ + private void executeDownload(State state, AndroidHttpClient client, HttpGet request) + throws StopRequest, RetryDownload, FileNotFoundException { + InnerState innerState = new InnerState(); + byte data[] = new byte[Constants.BUFFER_SIZE]; + + setupDestinationFile(state, innerState); + addRequestHeaders(innerState, request); + + // check just before sending the request to avoid using an invalid connection at all + checkConnectivity(state); + + HttpResponse response = sendRequest(state, client, request); + handleExceptionalStatus(state, innerState, response); + + if (Constants.LOGV) { + Log.v(Constants.TAG, "received response for " + mInfo.mUri); + } + + processResponseHeaders(state, innerState, response); + InputStream entityStream = openResponseEntity(state, response); + transferData(state, innerState, data, entityStream); + } + + /** + * Check if current connectivity is valid for this request. + */ + private void checkConnectivity(State state) throws StopRequest { + if (!mInfo.canUseNetwork()) { + state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED; + throw new StopRequest(); + } + } + + /** * Transfer as much data as possible from the HTTP response to the destination file. * @param data buffer to use to read data * @param entityStream stream for reading the HTTP response entity @@ -548,12 +582,8 @@ public class DownloadThread extends Thread { } updateDatabaseFromHeaders(state, innerState); - // check connectivity again now that we know the total size - if (!mInfo.canUseNetwork()) { - state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED; - throw new StopRequest(); - } + checkConnectivity(state); } /** @@ -638,7 +668,7 @@ public class DownloadThread extends Thread { * Check the HTTP response status and handle anything unusual (e.g. not 200/206). */ private void handleExceptionalStatus(State state, InnerState innerState, HttpResponse response) - throws StopRequest { + throws StopRequest, RetryDownload { int statusCode = response.getStatusLine().getStatusCode(); if (statusCode == 503 && mInfo.mNumFailed < Constants.MAX_RETRIES) { handleServiceUnavailable(state, response); @@ -680,7 +710,7 @@ public class DownloadThread extends Thread { * Handle a 3xx redirect status. */ private void handleRedirect(State state, HttpResponse response, int statusCode) - throws StopRequest { + throws StopRequest, RetryDownload { if (Constants.LOGVV) { Log.v(Constants.TAG, "got HTTP redirect " + statusCode); } @@ -695,33 +725,35 @@ public class DownloadThread extends Thread { throw new StopRequest(); } Header header = response.getFirstHeader("Location"); - if (header != null) { - if (Constants.LOGVV) { - Log.v(Constants.TAG, "Location :" + header.getValue()); - } - try { - state.mNewUri = new URI(mInfo.mUri). - resolve(new URI(header.getValue())). - toString(); - } catch(URISyntaxException ex) { - if (Constants.LOGV) { - Log.d(Constants.TAG, - "Couldn't resolve redirect URI " + - header.getValue() + - " for " + - mInfo.mUri); - } else if (Config.LOGD) { - Log.d(Constants.TAG, - "Couldn't resolve redirect URI for download " + - mInfo.mId); - } - state.mFinalStatus = Downloads.Impl.STATUS_BAD_REQUEST; - throw new StopRequest(); + if (header == null) { + return; + } + if (Constants.LOGVV) { + Log.v(Constants.TAG, "Location :" + header.getValue()); + } + + String newUri; + try { + newUri = new URI(mInfo.mUri).resolve(new URI(header.getValue())).toString(); + } catch(URISyntaxException ex) { + if (Constants.LOGV) { + Log.d(Constants.TAG, "Couldn't resolve redirect URI " + header.getValue() + + " for " + mInfo.mUri); + } else if (Config.LOGD) { + Log.d(Constants.TAG, + "Couldn't resolve redirect URI for download " + + mInfo.mId); } - ++state.mRedirectCount; - state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED; + state.mFinalStatus = Downloads.Impl.STATUS_BAD_REQUEST; throw new StopRequest(); } + ++state.mRedirectCount; + state.mRequestUri = newUri; + if (statusCode == 301 || statusCode == 303) { + // use the new URI for all future requests (should a retry/resume be necessary) + state.mNewUri = newUri; + } + throw new RetryDownload(); } /** @@ -824,10 +856,7 @@ public class DownloadThread extends Thread { state.mFilename = null; } else if (mInfo.mETag == null && !mInfo.mNoIntegrity) { // Tough luck, that's not a resumable download - if (Config.LOGD) { - Log.d(Constants.TAG, - "can't resume interrupted non-resumable download"); - } + Log.d(Constants.TAG, "can't resume interrupted non-resumable download"); f.delete(); state.mFinalStatus = Downloads.Impl.STATUS_PRECONDITION_FAILED; throw new StopRequest(); diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java index b02844fa..c1a015af 100644 --- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java @@ -293,14 +293,13 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { public void testRedirect301() throws Exception { RecordedRequest lastRequest = runRedirectionTest(301); - // for 301, upon retry, we reuse the redirected URI + // for 301, upon retry/resume, we reuse the redirected URI assertEquals(REDIRECTED_PATH, lastRequest.getPath()); } - // TODO: currently fails - public void disabledTestRedirect302() throws Exception { + public void testRedirect302() throws Exception { RecordedRequest lastRequest = runRedirectionTest(302); - // for 302, upon retry, we use the original URI + // for 302, upon retry/resume, we use the original URI assertEquals(REQUEST_PATH, lastRequest.getPath()); } @@ -485,11 +484,8 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { enqueueInterruptedDownloadResponses(5); Download download = enqueueRequest(getRequest()); - download.runUntilStatus(DownloadManager.STATUS_PAUSED); + runService(); assertEquals(REQUEST_PATH, takeRequest().getPath()); - - mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS); - download.runUntilStatus(DownloadManager.STATUS_PAUSED); assertEquals(REDIRECTED_PATH, takeRequest().getPath()); mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS); |