summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSteve Howard <showard@google.com>2010-07-30 18:55:38 -0700
committerSteve Howard <showard@google.com>2010-08-16 14:29:33 -0700
commitadb6887d3270d180c94eaf90878d5b67d74a8f28 (patch)
tree8d0af2656dbf21cbb5b25a19d25e2c86f743e33a
parente61798da80558450f580ed948d0d469bd6423d8e (diff)
downloadandroid_packages_providers_DownloadProvider-adb6887d3270d180c94eaf90878d5b67d74a8f28.tar.gz
android_packages_providers_DownloadProvider-adb6887d3270d180c94eaf90878d5b67d74a8f28.tar.bz2
android_packages_providers_DownloadProvider-adb6887d3270d180c94eaf90878d5b67d74a8f28.zip
Clean up error codes returned by download manager.
This set of changes cleans up the error codes returned by the download manager in various failure cases, aiming for improved consistency. Error codes are part of the public API so it's important to get this right now. The main changes here are: * Refactoring the flow of error status information throughout DownloadThread to make it more explicit, by having StopRequest accept a status code in its constructor and eliminating State.mFinaStatus. * Eliminating the use of valid HTTP 4xx statuses when those statuses weren't actually returned by the server. Now, if the returned error code is a valid HTTP status code, that means it was returned by the server. These cases have been replaced with more sensible artificial error codes, including a new ERROR_CANNOT_RESUME when an interrupted download can't be resumed. * Improvements to some of the error handling code paths -- ensuring we don't clear the cache for external downloads, ensuring we don't fail with CANNOT_RESUME when the download hasn't actually started yet, removing the restriction on acceptable mime types for public API downloads. Change-Id: I0d825845fe0fe7ed5df74bad26e8d34ac0d1cc4e
-rw-r--r--src/com/android/providers/downloads/DownloadInfo.java6
-rw-r--r--src/com/android/providers/downloads/DownloadThread.java171
-rw-r--r--src/com/android/providers/downloads/Helpers.java12
-rw-r--r--tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java22
4 files changed, 112 insertions, 99 deletions
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 4c08b156..92d8d503 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -371,4 +371,10 @@ public class DownloadInfo {
mHasActiveThread = true;
mSystemFacade.startThread(downloader);
}
+
+ public boolean isOnCache() {
+ return (mDestination == Downloads.Impl.DESTINATION_CACHE_PARTITION
+ || mDestination == Downloads.Impl.DESTINATION_CACHE_PARTITION_NOROAMING
+ || mDestination == Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE);
+ }
}
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index be02e3ec..f0aed3f5 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -79,7 +79,6 @@ public class DownloadThread extends Thread {
*/
private static class State {
public String mFilename;
- public int mFinalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
public FileOutputStream mStream;
public String mMimeType;
public boolean mCountRetry = false;
@@ -95,6 +94,7 @@ public class DownloadThread extends Thread {
mRedirectCount = info.mRedirectCount;
mContentUri = Uri.parse(Downloads.Impl.CONTENT_URI + "/" + info.mId);
mRequestUri = info.mUri;
+ mFilename = info.mFileName;
}
}
@@ -116,11 +116,16 @@ 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 {
- public StopRequest() {}
+ private class StopRequest extends Throwable {
+ public int mFinalStatus;
- public StopRequest(Throwable throwable) {
+ public StopRequest(int finalStatus) {
+ mFinalStatus = finalStatus;
+ }
+
+ public StopRequest(int finalStatus, Throwable throwable) {
super(throwable);
+ mFinalStatus = finalStatus;
}
}
@@ -128,7 +133,7 @@ public class DownloadThread extends Thread {
* Raised from methods called by executeDownload() to indicate that the download should be
* retried immediately.
*/
- private class RetryDownload extends Exception {}
+ private class RetryDownload extends Throwable {}
/**
* Executes the download in a separate thread
@@ -139,6 +144,7 @@ public class DownloadThread extends Thread {
State state = new State(mInfo);
AndroidHttpClient client = null;
PowerManager.WakeLock wakeLock = null;
+ int finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
try {
PowerManager pm = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE);
@@ -169,15 +175,17 @@ public class DownloadThread extends Thread {
if (Constants.LOGV) {
Log.v(Constants.TAG, "download completed for " + mInfo.mUri);
}
- state.mFinalStatus = Downloads.Impl.STATUS_SUCCESS;
+ finalizeDestinationFile(state);
+ finalStatus = Downloads.Impl.STATUS_SUCCESS;
} catch (StopRequest error) {
if (Constants.LOGV) {
Log.v(Constants.TAG, "Aborting request for " + mInfo.mUri, error);
}
+ finalStatus = error.mFinalStatus;
// fall through to finally block
} catch (FileNotFoundException ex) {
Log.d(Constants.TAG, "FileNotFoundException for " + state.mFilename + " : " + ex);
- state.mFinalStatus = Downloads.Impl.STATUS_FILE_ERROR;
+ finalStatus = Downloads.Impl.STATUS_FILE_ERROR;
// falls through to the code that reports an error
} catch (RuntimeException ex) { //sometimes the socket code throws unchecked exceptions
if (Constants.LOGV) {
@@ -185,7 +193,7 @@ public class DownloadThread extends Thread {
} else if (Config.LOGD) {
Log.d(Constants.TAG, "Exception for id " + mInfo.mId, ex);
}
- state.mFinalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
+ finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
// falls through to the code that reports an error
} finally {
mInfo.mHasActiveThread = false;
@@ -197,9 +205,8 @@ public class DownloadThread extends Thread {
client.close();
client = null;
}
- closeDestination(state);
- finalizeDestinationFile(state);
- notifyDownloadCompleted(state.mFinalStatus, state.mCountRetry, state.mRetryAfter,
+ cleanupDestination(state, finalStatus);
+ notifyDownloadCompleted(finalStatus, state.mCountRetry, state.mRetryAfter,
state.mRedirectCount, state.mGotData, state.mFilename,
state.mNewUri, state.mMimeType);
}
@@ -237,8 +244,7 @@ public class DownloadThread extends Thread {
*/
private void checkConnectivity(State state) throws StopRequest {
if (!mInfo.canUseNetwork()) {
- state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED);
}
}
@@ -271,33 +277,28 @@ public class DownloadThread extends Thread {
}
/**
- * Called after a download transfer has just completed to take any necessary action on the
- * downloaded file.
+ * Called after a successful completion to take any necessary action on the downloaded file.
*/
- private void finalizeDestinationFile(State state) {
- if (state.mFilename == null) {
- return;
+ private void finalizeDestinationFile(State state) throws StopRequest {
+ if (isDrmFile(state)) {
+ transferToDrm(state);
+ } else {
+ // make sure the file is readable
+ FileUtils.setPermissions(state.mFilename, 0644, -1, -1);
+ syncDestination(state);
}
+ }
- if (Downloads.Impl.isStatusError(state.mFinalStatus)) {
+ /**
+ * Called just before the thread finishes, regardless of status, to take any necessary action on
+ * the downloaded file.
+ */
+ private void cleanupDestination(State state, int finalStatus) {
+ closeDestination(state);
+ if (state.mFilename != null && Downloads.Impl.isStatusError(finalStatus)) {
new File(state.mFilename).delete();
state.mFilename = null;
- return;
- }
-
- if (!Downloads.Impl.isStatusSuccess(state.mFinalStatus)) {
- // not yet complete
- return;
- }
-
- if (isDrmFile(state)) {
- transferToDrm(state);
- return;
}
-
- // make sure the file is readable
- FileUtils.setPermissions(state.mFilename, 0644, -1, -1);
- syncDestination(state);
}
/**
@@ -339,18 +340,18 @@ public class DownloadThread extends Thread {
/**
* Transfer the downloaded destination file to the DRM store.
*/
- private void transferToDrm(State state) {
+ private void transferToDrm(State state) throws StopRequest {
File file = new File(state.mFilename);
Intent item = DrmStore.addDrmFile(mContext.getContentResolver(), file, null);
+ file.delete();
+
if (item == null) {
Log.w(Constants.TAG, "unable to add file " + state.mFilename + " to DrmProvider");
- state.mFinalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
+ throw new StopRequest(Downloads.Impl.STATUS_UNKNOWN_ERROR);
} else {
state.mFilename = item.getDataString();
state.mMimeType = item.getType();
}
-
- file.delete();
}
/**
@@ -381,16 +382,14 @@ public class DownloadThread extends Thread {
if (Constants.LOGV) {
Log.v(Constants.TAG, "paused " + mInfo.mUri);
}
- state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED);
}
}
if (mInfo.mStatus == Downloads.Impl.STATUS_CANCELED) {
if (Constants.LOGV) {
Log.d(Constants.TAG, "canceled " + mInfo.mUri);
}
- state.mFinalStatus = Downloads.Impl.STATUS_CANCELED;
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_CANCELED);
}
}
@@ -431,10 +430,11 @@ public class DownloadThread extends Thread {
}
return;
} catch (IOException ex) {
- if (!Helpers.discardPurgeableFiles(mContext, Constants.BUFFER_SIZE)) {
- state.mFinalStatus = Downloads.Impl.STATUS_FILE_ERROR;
- throw new StopRequest(ex);
+ if (mInfo.isOnCache()
+ && Helpers.discardPurgeableFiles(mContext, Constants.BUFFER_SIZE)) {
+ continue;
}
+ throw new StopRequest(Downloads.Impl.STATUS_FILE_ERROR, ex);
}
}
}
@@ -454,7 +454,7 @@ public class DownloadThread extends Thread {
boolean lengthMismatched = (innerState.mHeaderContentLength != null)
&& (innerState.mBytesSoFar != Integer.parseInt(innerState.mHeaderContentLength));
if (lengthMismatched) {
- if (!mInfo.mNoIntegrity && innerState.mHeaderETag == null) {
+ if (cannotResume(innerState)) {
if (Constants.LOGV) {
Log.d(Constants.TAG, "mismatched content length " +
mInfo.mUri);
@@ -462,14 +462,17 @@ public class DownloadThread extends Thread {
Log.d(Constants.TAG, "mismatched content length for " +
mInfo.mId);
}
- state.mFinalStatus = Downloads.Impl.STATUS_LENGTH_REQUIRED;
+ throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME);
} else {
- handleHttpError(state, "closed socket");
+ throw new StopRequest(handleHttpError(state, "closed socket"));
}
- throw new StopRequest();
}
}
+ private boolean cannotResume(InnerState innerState) {
+ return innerState.mBytesSoFar > 0 && !mInfo.mNoIntegrity && innerState.mHeaderETag == null;
+ }
+
/**
* Read some data from the HTTP response stream, handling I/O errors.
* @param data buffer to use to read data
@@ -485,14 +488,13 @@ public class DownloadThread extends Thread {
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) {
+ if (cannotResume(innerState)) {
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;
+ throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME, ex);
} else {
- handleHttpError(state, "download IOException");
+ throw new StopRequest(handleHttpError(state, "download IOException"), ex);
}
- throw new StopRequest(ex);
}
}
@@ -506,8 +508,7 @@ public class DownloadThread extends Thread {
return response.getEntity().getContent();
} catch (IOException ex) {
logNetworkState();
- handleHttpError(state, "IOException getting entity");
- throw new StopRequest(ex);
+ throw new StopRequest(handleHttpError(state, "IOException getting entity"), ex);
}
}
@@ -540,10 +541,10 @@ public class DownloadThread extends Thread {
state.mMimeType,
mInfo.mDestination,
(innerState.mHeaderContentLength != null) ?
- Long.parseLong(innerState.mHeaderContentLength) : 0);
+ Long.parseLong(innerState.mHeaderContentLength) : 0,
+ mInfo.mIsPublicApi);
if (fileInfo.mFileName == null) {
- state.mFinalStatus = fileInfo.mStatus;
- throw new StopRequest();
+ throw new StopRequest(fileInfo.mStatus);
}
state.mFilename = fileInfo.mFileName;
state.mStream = fileInfo.mStream;
@@ -629,8 +630,7 @@ public class DownloadThread extends Thread {
|| !headerTransferEncoding.equalsIgnoreCase("chunked"));
if (!mInfo.mNoIntegrity && noSizeInfo) {
Log.d(Constants.TAG, "can't know size of download, giving up");
- state.mFinalStatus = Downloads.Impl.STATUS_LENGTH_REQUIRED;
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR);
}
}
@@ -664,16 +664,17 @@ public class DownloadThread extends Thread {
Log.d(Constants.TAG, "http error " + statusCode + " for download " +
mInfo.mId);
}
+ int finalStatus;
if (Downloads.Impl.isStatusError(statusCode)) {
- state.mFinalStatus = statusCode;
+ finalStatus = statusCode;
} else if (statusCode >= 300 && statusCode < 400) {
- state.mFinalStatus = Downloads.Impl.STATUS_UNHANDLED_REDIRECT;
+ finalStatus = Downloads.Impl.STATUS_UNHANDLED_REDIRECT;
} else if (innerState.mContinuingDownload && statusCode == Downloads.Impl.STATUS_SUCCESS) {
- state.mFinalStatus = Downloads.Impl.STATUS_PRECONDITION_FAILED;
+ finalStatus = Downloads.Impl.STATUS_CANNOT_RESUME;
} else {
- state.mFinalStatus = Downloads.Impl.STATUS_UNHANDLED_HTTP_CODE;
+ finalStatus = Downloads.Impl.STATUS_UNHANDLED_HTTP_CODE;
}
- throw new StopRequest();
+ throw new StopRequest(finalStatus);
}
/**
@@ -691,8 +692,7 @@ public class DownloadThread extends Thread {
} else if (Config.LOGD) {
Log.d(Constants.TAG, "too many redirects for download " + mInfo.mId);
}
- state.mFinalStatus = Downloads.Impl.STATUS_TOO_MANY_REDIRECTS;
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_TOO_MANY_REDIRECTS);
}
Header header = response.getFirstHeader("Location");
if (header == null) {
@@ -714,8 +714,7 @@ public class DownloadThread extends Thread {
"Couldn't resolve redirect URI for download " +
mInfo.mId);
}
- state.mFinalStatus = Downloads.Impl.STATUS_BAD_REQUEST;
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR);
}
++state.mRedirectCount;
state.mRequestUri = newUri;
@@ -733,7 +732,6 @@ public class DownloadThread extends Thread {
if (Constants.LOGVV) {
Log.v(Constants.TAG, "got HTTP response code 503");
}
- state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
state.mCountRetry = true;
Header header = response.getFirstHeader("Retry-After");
if (header != null) {
@@ -757,7 +755,7 @@ public class DownloadThread extends Thread {
// ignored - retryAfter stays 0 in this case.
}
}
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_RUNNING_PAUSED);
}
/**
@@ -775,28 +773,30 @@ public class DownloadThread extends Thread {
Log.d(Constants.TAG, "Arg exception trying to execute request for " +
mInfo.mId + " : " + ex);
}
- state.mFinalStatus = Downloads.Impl.STATUS_BAD_REQUEST;
- throw new StopRequest(ex);
+ throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR, ex);
} catch (IOException ex) {
logNetworkState();
- handleHttpError(state, "IOException trying to execute request");
- throw new StopRequest(ex);
+ throw new StopRequest(handleHttpError(state, "IOException trying to execute request"),
+ ex);
}
}
- private void handleHttpError(State state, String message) {
+ /**
+ * @return the final status for this attempt
+ */
+ private int 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;
+ return Downloads.Impl.STATUS_RUNNING_PAUSED;
} else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
- state.mFinalStatus = Downloads.Impl.STATUS_RUNNING_PAUSED;
state.mCountRetry = true;
+ return Downloads.Impl.STATUS_RUNNING_PAUSED;
} else {
Log.d(Constants.TAG, "reached max retries: " + message + " for " + mInfo.mId);
- state.mFinalStatus = Downloads.Impl.STATUS_HTTP_DATA_ERROR;
+ return Downloads.Impl.STATUS_HTTP_DATA_ERROR;
}
}
@@ -806,11 +806,9 @@ public class DownloadThread extends Thread {
*/
private void setupDestinationFile(State state, InnerState innerState)
throws StopRequest, FileNotFoundException {
- state.mFilename = mInfo.mFileName;
- if (state.mFilename != null) {
+ if (state.mFilename != null) { // only true if we've already run a thread for this download
if (!Helpers.isFilenameValid(state.mFilename)) {
- state.mFinalStatus = Downloads.Impl.STATUS_FILE_ERROR;
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_FILE_ERROR);
}
// We're resuming a download that got interrupted
File f = new File(state.mFilename);
@@ -821,11 +819,10 @@ public class DownloadThread extends Thread {
f.delete();
state.mFilename = null;
} else if (mInfo.mETag == null && !mInfo.mNoIntegrity) {
- // Tough luck, that's not a resumable download
- Log.d(Constants.TAG, "can't resume interrupted non-resumable download");
+ // This should've been caught upon failure
+ Log.wtf(Constants.TAG, "Trying to resume a download that can't be resumed");
f.delete();
- state.mFinalStatus = Downloads.Impl.STATUS_PRECONDITION_FAILED;
- throw new StopRequest();
+ throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME);
} else {
// All right, we'll be able to resume this download
state.mStream = new FileOutputStream(state.mFilename, true);
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index f2988954..5d546ff6 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -94,9 +94,10 @@ public class Helpers {
String contentLocation,
String mimeType,
int destination,
- long contentLength) throws FileNotFoundException {
+ long contentLength,
+ boolean isPublicApi) throws FileNotFoundException {
- if (!canHandleDownload(context, mimeType, destination)) {
+ if (!canHandleDownload(context, mimeType, destination, isPublicApi)) {
return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_NOT_ACCEPTABLE);
}
@@ -156,7 +157,12 @@ public class Helpers {
return chooseUniqueFilename(destination, filename, extension, recoveryDir);
}
- private static boolean canHandleDownload(Context context, String mimeType, int destination) {
+ private static boolean canHandleDownload(Context context, String mimeType, int destination,
+ boolean isPublicApi) {
+ if (isPublicApi) {
+ return true;
+ }
+
if (destination == Downloads.Impl.DESTINATION_EXTERNAL
|| destination == Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE) {
if (mimeType == null) {
diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
index 840b20ac..6d604778 100644
--- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
+++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
@@ -35,8 +35,6 @@ import java.util.List;
@LargeTest
public class PublicApiFunctionalTest extends AbstractPublicApiTest {
- private static final int HTTP_NOT_ACCEPTABLE = 406;
- private static final int HTTP_LENGTH_REQUIRED = 411;
private static final String REDIRECTED_PATH = "/other_path";
private static final String ETAG = "my_etag";
@@ -305,7 +303,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
public void testNoEtag() throws Exception {
enqueuePartialResponse(0, 5).removeHeader("Etag");
- runSimpleFailureTest(HTTP_LENGTH_REQUIRED);
+ runSimpleFailureTest(DownloadManager.ERROR_CANNOT_RESUME);
}
public void testSanitizeMediaType() throws Exception {
@@ -317,12 +315,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
public void testNoContentLength() throws Exception {
enqueueEmptyResponse(HTTP_OK).removeHeader("Content-Length");
- runSimpleFailureTest(HTTP_LENGTH_REQUIRED);
- }
-
- public void testNoContentType() throws Exception {
- enqueueResponse(HTTP_OK, "").removeHeader("Content-Type");
- runSimpleFailureTest(HTTP_NOT_ACCEPTABLE);
+ runSimpleFailureTest(DownloadManager.ERROR_HTTP_DATA_ERROR);
}
public void testInsufficientSpace() throws Exception {
@@ -478,6 +471,17 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
checkCompleteDownload(download);
}
+ public void testExistingFile() throws Exception {
+ Uri destination = getExternalUri();
+ new File(destination.getSchemeSpecificPart()).createNewFile();
+
+ enqueueEmptyResponse(HTTP_OK);
+ Download download = enqueueRequest(getRequest().setDestinationUri(destination));
+ download.runUntilStatus(DownloadManager.STATUS_FAILED);
+ assertEquals(DownloadManager.ERROR_FILE_ERROR,
+ download.getLongField(DownloadManager.COLUMN_ERROR_CODE));
+ }
+
private void checkCompleteDownload(Download download) throws Exception {
assertEquals(FILE_CONTENT.length(),
download.getLongField(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));