Merge "Improve error handling." into nyc-andromeda-dev
diff --git a/src/com/android/documentsui/Metrics.java b/src/com/android/documentsui/Metrics.java
index 15cf902..cc3597e 100644
--- a/src/com/android/documentsui/Metrics.java
+++ b/src/com/android/documentsui/Metrics.java
@@ -392,7 +392,9 @@
@OpType int operationType,
List<DocumentInfo> srcs,
@Nullable DocumentInfo dst) {
- ProviderCounts counts = countProviders(srcs, dst);
+
+ ProviderCounts counts = new ProviderCounts();
+ countProviders(counts, srcs, dst);
if (counts.intraProvider > 0) {
logIntraProviderFileOps(context, dst.authority, operationType);
@@ -438,8 +440,13 @@
* @param failedFiles
*/
public static void logFileOperationErrors(Context context, @OpType int operationType,
- List<DocumentInfo> failedFiles) {
- ProviderCounts counts = countProviders(failedFiles, null);
+ List<DocumentInfo> failedFiles, List<Uri> failedUris) {
+
+ ProviderCounts counts = new ProviderCounts();
+ countProviders(counts, failedFiles, null);
+
+ // TODO: Report URI errors separate from file operation errors.
+ countProviders(counts, failedUris);
@FileOp int opCode = FILEOP_OTHER_ERROR;
switch (operationType) {
@@ -826,19 +833,33 @@
* the dst document (if a dst is provided), how many come from system providers, and how many
* come from external 3rd-party providers.
*/
- private static ProviderCounts countProviders(
- List<DocumentInfo> srcs, @Nullable DocumentInfo dst) {
- ProviderCounts counts = new ProviderCounts();
+ private static void countProviders(
+ ProviderCounts counts, List<DocumentInfo> srcs, @Nullable DocumentInfo dst) {
for (DocumentInfo doc: srcs) {
- if (dst != null && doc.authority.equals(dst.authority)) {
- counts.intraProvider++;
- } else if (isSystemProvider(doc.authority)){
- counts.systemProvider++;
- } else {
- counts.externalProvider++;
- }
+ countForAuthority(counts, doc.authority, dst);
}
- return counts;
+ }
+
+ /**
+ * Count the given uris and provide a tally of how many come from the same provider as
+ * the dst document (if a dst is provided), how many come from system providers, and how many
+ * come from external 3rd-party providers.
+ */
+ private static void countProviders(ProviderCounts counts, List<Uri> uris) {
+ for (Uri uri: uris) {
+ countForAuthority(counts, uri.getAuthority(), null);
+ }
+ }
+
+ private static void countForAuthority(
+ ProviderCounts counts, String authority, @Nullable DocumentInfo dst) {
+ if (dst != null && authority.equals(dst.authority)) {
+ counts.intraProvider++;
+ } else if (isSystemProvider(authority)){
+ counts.systemProvider++;
+ } else {
+ counts.externalProvider++;
+ }
}
private static class ProviderCounts {
diff --git a/src/com/android/documentsui/OperationDialogFragment.java b/src/com/android/documentsui/OperationDialogFragment.java
index 19730b3..6a76312 100644
--- a/src/com/android/documentsui/OperationDialogFragment.java
+++ b/src/com/android/documentsui/OperationDialogFragment.java
@@ -17,13 +17,14 @@
package com.android.documentsui;
import android.annotation.IntDef;
-import android.app.AlertDialog;
import android.app.Dialog;
import android.app.DialogFragment;
import android.app.FragmentManager;
import android.app.FragmentTransaction;
import android.content.DialogInterface;
+import android.net.Uri;
import android.os.Bundle;
+import android.support.v7.app.AlertDialog;
import android.text.Html;
import com.android.documentsui.base.DocumentInfo;
@@ -34,7 +35,6 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
-
/**
* Alert dialog for operation dialogs.
*/
@@ -55,13 +55,18 @@
private static final String TAG = "OperationDialogFragment";
- public static void show(FragmentManager fm, @DialogType int dialogType,
- ArrayList<DocumentInfo> failedSrcList, DocumentStack dstStack,
+ public static void show(
+ FragmentManager fm,
+ @DialogType int dialogType,
+ ArrayList<DocumentInfo> failedSrcList,
+ ArrayList<DocumentInfo> uriList,
+ DocumentStack dstStack,
@OpType int operationType) {
+
final Bundle args = new Bundle();
args.putInt(FileOperationService.EXTRA_DIALOG_TYPE, dialogType);
args.putInt(FileOperationService.EXTRA_OPERATION_TYPE, operationType);
- args.putParcelableArrayList(FileOperationService.EXTRA_SRC_LIST, failedSrcList);
+ args.putParcelableArrayList(FileOperationService.EXTRA_FAILED_DOCS, failedSrcList);
final FragmentTransaction ft = fm.beginTransaction();
final OperationDialogFragment fragment = new OperationDialogFragment();
@@ -79,8 +84,10 @@
getArguments().getInt(FileOperationService.EXTRA_DIALOG_TYPE);
final @OpType int operationType =
getArguments().getInt(FileOperationService.EXTRA_OPERATION_TYPE);
- final ArrayList<DocumentInfo> srcList = getArguments().getParcelableArrayList(
- FileOperationService.EXTRA_SRC_LIST);
+ final ArrayList<Uri> uriList = getArguments().getParcelableArrayList(
+ FileOperationService.EXTRA_FAILED_URIS);
+ final ArrayList<DocumentInfo> docList = getArguments().getParcelableArrayList(
+ FileOperationService.EXTRA_FAILED_DOCS);
final AlertDialog.Builder builder = new AlertDialog.Builder(getActivity());
String messageFormat;
@@ -111,10 +118,14 @@
}
final StringBuilder list = new StringBuilder("<p>");
- for (DocumentInfo documentInfo : srcList) {
- list.append(String.format("• %s<br>", Html.escapeHtml(documentInfo.displayName)));
+ for (DocumentInfo documentInfo : docList) {
+ list.append("• " + Html.escapeHtml(documentInfo.displayName) + "<br>");
+ }
+ for (Uri uri : uriList) {
+ list.append("• " + uri.toSafeString() + "<br>");
}
list.append("</p>");
+
builder.setMessage(Html.fromHtml(String.format(messageFormat, list.toString())));
builder.setPositiveButton(
R.string.close,
diff --git a/src/com/android/documentsui/files/FilesActivity.java b/src/com/android/documentsui/files/FilesActivity.java
index bbeb623..0d9cfed 100644
--- a/src/com/android/documentsui/files/FilesActivity.java
+++ b/src/com/android/documentsui/files/FilesActivity.java
@@ -154,12 +154,15 @@
final int opType = intent.getIntExtra(
FileOperationService.EXTRA_OPERATION_TYPE,
FileOperationService.OPERATION_COPY);
- final ArrayList<DocumentInfo> srcList =
- intent.getParcelableArrayListExtra(FileOperationService.EXTRA_SRC_LIST);
+ final ArrayList<DocumentInfo> docList =
+ intent.getParcelableArrayListExtra(FileOperationService.EXTRA_FAILED_DOCS);
+ final ArrayList<DocumentInfo> uriList =
+ intent.getParcelableArrayListExtra(FileOperationService.EXTRA_FAILED_URIS);
OperationDialogFragment.show(
getFragmentManager(),
dialogType,
- srcList,
+ docList,
+ uriList,
mState.stack,
opType);
}
diff --git a/src/com/android/documentsui/services/CopyJob.java b/src/com/android/documentsui/services/CopyJob.java
index 30bbbf1..e001ee3 100644
--- a/src/com/android/documentsui/services/CopyJob.java
+++ b/src/com/android/documentsui/services/CopyJob.java
@@ -35,7 +35,6 @@
import android.app.Notification.Builder;
import android.app.PendingIntent;
import android.content.ContentProviderClient;
-import android.content.ContentResolver;
import android.content.Context;
import android.content.Intent;
import android.content.res.AssetFileDescriptor;
@@ -67,18 +66,16 @@
import java.io.InputStream;
import java.text.NumberFormat;
import java.util.ArrayList;
-import java.util.List;
-class CopyJob extends Job {
+class CopyJob extends ResolvedResourcesJob {
private static final String TAG = "CopyJob";
- final List<DocumentInfo> mSrcs;
final ArrayList<DocumentInfo> convertedFiles = new ArrayList<>();
private long mStartTime = -1;
- private long mBatchSize;
+ private long mBytesRequired;
private volatile long mBytesCopied;
// Speed estimation
private long mBytesCopiedSample;
@@ -99,9 +96,6 @@
super(service, listener, id, opType, destination, srcs);
assert(srcs.getItemCount() > 0);
-
- // delay the initialization of it to setUp() because it may be IO extensive.
- mSrcs = new ArrayList<>(srcs.getItemCount());
}
@Override
@@ -121,8 +115,8 @@
Notification getProgressNotification(@StringRes int msgId) {
updateRemainingTimeEstimate();
- if (mBatchSize >= 0) {
- double completed = (double) this.mBytesCopied / mBatchSize;
+ if (mBytesRequired >= 0) {
+ double completed = (double) this.mBytesCopied / mBytesRequired;
mProgressBuilder.setProgress(100, (int) (completed * 100), false);
mProgressBuilder.setSubText(
NumberFormat.getPercentInstance().format(completed));
@@ -171,7 +165,7 @@
}
if (mSampleTime > 0 && mSpeed > 0) {
- mRemainingTime = ((mBatchSize - bytesCopied) * 1000) / mSpeed;
+ mRemainingTime = ((mBytesRequired - bytesCopied) * 1000) / mSpeed;
} else {
mRemainingTime = 0;
}
@@ -211,10 +205,7 @@
@Override
boolean setUp() {
- try {
- buildDocumentList();
- } catch (ResourceException e) {
- Log.e(TAG, "Failed to get the list of docs.", e);
+ if (!super.setUp()) {
return false;
}
@@ -224,10 +215,10 @@
}
try {
- mBatchSize = calculateSize(mSrcs);
+ mBytesRequired = calculateBytesRequired();
} catch (ResourceException e) {
Log.w(TAG, "Failed to calculate total size. Copying without progress.", e);
- mBatchSize = -1;
+ mBytesRequired = -1;
}
// Check if user has canceled this task. We should check it again here as user cancels
@@ -246,8 +237,8 @@
mStartTime = elapsedRealtime();
DocumentInfo srcInfo;
DocumentInfo dstInfo = stack.peek();
- for (int i = 0; i < mSrcs.size() && !isCanceled(); ++i) {
- srcInfo = mSrcs.get(i);
+ for (int i = 0; i < mResolvedDocs.size() && !isCanceled(); ++i) {
+ srcInfo = mResolvedDocs.get(i);
if (DEBUG) Log.d(TAG,
"Copying " + srcInfo.displayName + " (" + srcInfo.derivedUri + ")"
@@ -265,39 +256,12 @@
onFileFailed(srcInfo);
}
}
- Metrics.logFileOperation(service, operationType, mSrcs, dstInfo);
+
+ Metrics.logFileOperation(service, operationType, mResolvedDocs, dstInfo);
}
- private void buildDocumentList() throws ResourceException {
- try {
- final ContentResolver resolver = appContext.getContentResolver();
- final Iterable<Uri> uris = srcs.getUris(appContext);
-
- int docProcessed = 0;
- for (Uri uri : uris) {
- DocumentInfo doc = DocumentInfo.fromUri(resolver, uri);
- if (canCopy(doc, stack.getRoot())) {
- mSrcs.add(doc);
- } else {
- onFileFailed(doc);
- }
- ++docProcessed;
-
- if (isCanceled()) {
- return;
- }
- }
-
- // If docProcessed is different than the count claimed by UrisSupplier, add the number
- // to failedFileCount.
- failedFileCount += (srcs.getItemCount() - docProcessed);
- } catch(IOException e) {
- failedFileCount += srcs.getItemCount();
- throw new ResourceException("Failed to open the list of docs to copy.", e);
- }
- }
-
- private static boolean canCopy(DocumentInfo doc, RootInfo root) {
+ @Override
+ boolean isEligibleDoc(DocumentInfo doc, RootInfo root) {
// Can't copy folders to downloads, because we don't show folders there.
return !root.isDownloads() || !doc.isDirectory();
}
@@ -307,7 +271,7 @@
* @return true if the root has enough space or doesn't provide free space info; otherwise false
*/
boolean checkSpace() {
- return checkSpace(mBatchSize);
+ return verifySpaceAvailable(mBytesRequired);
}
/**
@@ -315,10 +279,10 @@
* @param batchSize the total size of files
* @return true if the root has enough space or doesn't provide free space info; otherwise false
*/
- final boolean checkSpace(long batchSize) {
+ final boolean verifySpaceAvailable(long batchSize) {
// Default to be true because if batchSize or available space is invalid, we still let the
// copy start anyway.
- boolean result = true;
+ boolean available = true;
if (batchSize >= 0) {
RootsCache cache = DocumentsApplication.getRootsCache(appContext);
@@ -327,18 +291,18 @@
// stale.
root = cache.getRootOneshot(root.authority, root.rootId, true);
if (root.availableBytes >= 0) {
- result = (batchSize <= root.availableBytes);
+ available = (batchSize <= root.availableBytes);
} else {
Log.w(TAG, root.toString() + " doesn't provide available bytes.");
}
}
- if (!result) {
- failedFileCount += mSrcs.size();
- failedFiles.addAll(mSrcs);
+ if (!available) {
+ failureCount = mResolvedDocs.size();
+ failedDocs.addAll(mResolvedDocs);
}
- return result;
+ return available;
}
@Override
@@ -626,14 +590,13 @@
* Calculates the cumulative size of all the documents in the list. Directories are recursed
* into and totaled up.
*
- * @param srcs
* @return Size in bytes.
* @throws ResourceException
*/
- private long calculateSize(List<DocumentInfo> srcs) throws ResourceException {
+ private long calculateBytesRequired() throws ResourceException {
long result = 0;
- for (DocumentInfo src : srcs) {
+ for (DocumentInfo src : mResolvedDocs) {
if (src.isDirectory()) {
// Directories need to be recursed into.
try {
@@ -719,7 +682,8 @@
.append("CopyJob")
.append("{")
.append("id=" + id)
- .append(", docs=" + srcs)
+ .append(", uris=" + mResourceUris)
+ .append(", docs=" + mResolvedDocs)
.append(", destination=" + stack)
.append("}")
.toString();
diff --git a/src/com/android/documentsui/services/DeleteJob.java b/src/com/android/documentsui/services/DeleteJob.java
index 5d32b70..4031c1f 100644
--- a/src/com/android/documentsui/services/DeleteJob.java
+++ b/src/com/android/documentsui/services/DeleteJob.java
@@ -26,22 +26,21 @@
import android.net.Uri;
import android.util.Log;
-import com.android.documentsui.clipping.UrisSupplier;
import com.android.documentsui.Metrics;
import com.android.documentsui.R;
import com.android.documentsui.base.DocumentInfo;
import com.android.documentsui.base.DocumentStack;
+import com.android.documentsui.clipping.UrisSupplier;
+
+import java.io.FileNotFoundException;
import javax.annotation.Nullable;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-final class DeleteJob extends Job {
+final class DeleteJob extends ResolvedResourcesJob {
private static final String TAG = "DeleteJob";
- private final Uri mSrcParent;
+ private final Uri mParentUri;
private volatile int mDocsProcessed = 0;
/**
@@ -54,7 +53,7 @@
DeleteJob(Context service, Listener listener, String id, DocumentStack stack,
UrisSupplier srcs, @Nullable Uri srcParent) {
super(service, listener, id, OPERATION_DELETE, stack, srcs);
- mSrcParent = srcParent;
+ mParentUri = srcParent;
}
@Override
@@ -73,9 +72,10 @@
@Override
public Notification getProgressNotification() {
- mProgressBuilder.setProgress(srcs.getItemCount(), mDocsProcessed, false);
+ mProgressBuilder.setProgress(mResourceUris.getItemCount(), mDocsProcessed, false);
String format = service.getString(R.string.delete_progress);
- mProgressBuilder.setSubText(String.format(format, mDocsProcessed, srcs.getItemCount()));
+ mProgressBuilder.setSubText(
+ String.format(format, mDocsProcessed, mResourceUris.getItemCount()));
mProgressBuilder.setContentText(null);
@@ -95,44 +95,35 @@
@Override
void start() {
+ ContentResolver resolver = appContext.getContentResolver();
+
+ DocumentInfo parentDoc;
try {
- final List<DocumentInfo> srcs = new ArrayList<>(this.srcs.getItemCount());
+ parentDoc = mParentUri != null
+ ? DocumentInfo.fromUri(resolver, mParentUri)
+ : null;
+ } catch (FileNotFoundException e) {
+ Log.e(TAG, "Failed to resolve parent from Uri: " + mParentUri + ". Cannot continue.", e);
+ failureCount += this.mResourceUris.getItemCount();
+ return;
+ }
- final Iterable<Uri> uris = this.srcs.getUris(appContext);
-
- final ContentResolver resolver = appContext.getContentResolver();
- final DocumentInfo srcParent =
- mSrcParent != null
- ? DocumentInfo.fromUri(resolver, mSrcParent)
- : null;
- for (Uri uri : uris) {
- DocumentInfo doc = DocumentInfo.fromUri(resolver, uri);
- srcs.add(doc);
-
- if (DEBUG) Log.d(TAG, "Deleting document @ " + doc.derivedUri);
- try {
- deleteDocument(doc, srcParent);
-
- if (isCanceled()) {
- // Canceled, dump the rest of the work. Deleted docs are not recoverable.
- return;
- }
- } catch (ResourceException e) {
- Log.e(TAG, "Failed to delete document @ " + doc.derivedUri, e);
- onFileFailed(doc);
- }
-
- ++mDocsProcessed;
+ for (DocumentInfo doc : mResolvedDocs) {
+ if (DEBUG) Log.d(TAG, "Deleting document @ " + doc.derivedUri);
+ try {
+ deleteDocument(doc, parentDoc);
+ } catch (ResourceException e) {
+ Log.e(TAG, "Failed to delete document @ " + doc.derivedUri, e);
+ onFileFailed(doc);
}
- // If mDocProcessed is different than the count claimed by UrisSupplier, add the number
- // to failedFileCount.
- failedFileCount += (this.srcs.getItemCount() - mDocsProcessed);
- Metrics.logFileOperation(service, operationType, srcs, null);
- } catch(IOException e) {
- Log.e(TAG, "Failed to get list of docs or parent source.", e);
- failedFileCount += srcs.getItemCount();
+ mDocsProcessed++;
+ if (isCanceled()) {
+ return;
+ }
}
+
+ Metrics.logFileOperation(service, operationType, mResolvedDocs, null);
}
@Override
@@ -141,8 +132,9 @@
.append("DeleteJob")
.append("{")
.append("id=" + id)
- .append(", docs=" + srcs)
- .append(", srcParent=" + mSrcParent)
+ .append(", uris=" + mResourceUris)
+ .append(", docs=" + mResolvedDocs)
+ .append(", srcParent=" + mParentUri)
.append(", location=" + stack)
.append("}")
.toString();
diff --git a/src/com/android/documentsui/services/FileOperationService.java b/src/com/android/documentsui/services/FileOperationService.java
index d28a578..d8485ba 100644
--- a/src/com/android/documentsui/services/FileOperationService.java
+++ b/src/com/android/documentsui/services/FileOperationService.java
@@ -56,6 +56,9 @@
public static final String EXTRA_DIALOG_TYPE = "com.android.documentsui.DIALOG_TYPE";
public static final String EXTRA_SRC_LIST = "com.android.documentsui.SRC_LIST";
+ public static final String EXTRA_FAILED_URIS = "com.android.documentsui.FAILED_URIS";
+ public static final String EXTRA_FAILED_DOCS = "com.android.documentsui.FAILED_DOCS";
+
// Extras used to start or cancel a file operation...
public static final String EXTRA_JOB_ID = "com.android.documentsui.JOB_ID";
public static final String EXTRA_OPERATION = "com.android.documentsui.OPERATION";
@@ -296,7 +299,12 @@
mNotificationManager.cancel(job.id, NOTIFICATION_ID_PROGRESS);
if (job.hasFailures()) {
- Log.e(TAG, "Job failed on files: " + job.failedFileCount + ".");
+ if (!job.failedUris.isEmpty()) {
+ Log.e(TAG, "Job failed to resolve uris: " + job.failedUris + ".");
+ }
+ if (!job.failedDocs.isEmpty()) {
+ Log.e(TAG, "Job failed to process docs: " + job.failedDocs + ".");
+ }
mNotificationManager.notify(
job.id, NOTIFICATION_ID_FAILURE, job.getFailureNotification());
}
diff --git a/src/com/android/documentsui/services/Job.java b/src/com/android/documentsui/services/Job.java
index 3cec4a4..dff360a 100644
--- a/src/com/android/documentsui/services/Job.java
+++ b/src/com/android/documentsui/services/Job.java
@@ -19,9 +19,10 @@
import static com.android.documentsui.DocumentsApplication.acquireUnstableProviderOrThrow;
import static com.android.documentsui.services.FileOperationService.EXTRA_CANCEL;
import static com.android.documentsui.services.FileOperationService.EXTRA_DIALOG_TYPE;
+import static com.android.documentsui.services.FileOperationService.EXTRA_FAILED_DOCS;
+import static com.android.documentsui.services.FileOperationService.EXTRA_FAILED_URIS;
import static com.android.documentsui.services.FileOperationService.EXTRA_JOB_ID;
import static com.android.documentsui.services.FileOperationService.EXTRA_OPERATION_TYPE;
-import static com.android.documentsui.services.FileOperationService.EXTRA_SRC_LIST;
import static com.android.documentsui.services.FileOperationService.OPERATION_UNKNOWN;
import android.annotation.DrawableRes;
@@ -40,23 +41,24 @@
import android.provider.DocumentsContract;
import android.util.Log;
-import com.android.documentsui.clipping.UrisSupplier;
-import com.android.documentsui.files.FilesActivity;
import com.android.documentsui.Metrics;
import com.android.documentsui.OperationDialogFragment;
import com.android.documentsui.R;
import com.android.documentsui.base.DocumentInfo;
import com.android.documentsui.base.DocumentStack;
import com.android.documentsui.base.Shared;
+import com.android.documentsui.clipping.UrisSupplier;
+import com.android.documentsui.files.FilesActivity;
import com.android.documentsui.services.FileOperationService.OpType;
-import javax.annotation.Nullable;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
+import javax.annotation.Nullable;
+
/**
* A mashup of work item and ui progress update factory. Used by {@link FileOperationService}
* to do work and show progress relating to this work.
@@ -89,10 +91,13 @@
final @OpType int operationType;
final String id;
final DocumentStack stack;
- final UrisSupplier srcs;
- int failedFileCount = 0;
- final ArrayList<DocumentInfo> failedFiles = new ArrayList<>();
+ final UrisSupplier mResourceUris;
+
+ int failureCount = 0;
+ final ArrayList<DocumentInfo> failedDocs = new ArrayList<>();
+ final ArrayList<Uri> failedUris = new ArrayList<>();
+
final Notification.Builder mProgressBuilder;
private final Map<String, ContentProviderClient> mClients = new HashMap<>();
@@ -121,7 +126,7 @@
this.id = id;
this.stack = stack;
- this.srcs = srcs;
+ this.mResourceUris = srcs;
mProgressBuilder = createProgressBuilder();
}
@@ -135,6 +140,7 @@
mState = STATE_STARTED;
listener.onStart(this);
+
try {
boolean result = setUp();
if (result && !isCanceled()) {
@@ -145,20 +151,21 @@
// No exceptions should be thrown here, as all calls to the provider must be
// handled within Job implementations. However, just in case catch them here.
Log.e(TAG, "Operation failed due to an unhandled runtime exception.", e);
- Metrics.logFileOperationErrors(service, operationType, failedFiles);
+ Metrics.logFileOperationErrors(service, operationType, failedDocs, failedUris);
} finally {
mState = (mState == STATE_STARTED || mState == STATE_SET_UP) ? STATE_COMPLETED : mState;
listener.onFinished(this);
// NOTE: If this details is a JumboClipDetails, and it's still referred in primary clip
// at this point, user won't be able to paste it to anywhere else because the underlying
- srcs.dispose();
+ mResourceUris.dispose();
}
}
boolean setUp() {
return true;
}
+
abstract void start();
abstract Notification getSetupNotification();
@@ -214,12 +221,17 @@
}
void onFileFailed(DocumentInfo file) {
- ++failedFileCount;
- failedFiles.add(file);
+ failureCount++;
+ failedDocs.add(file);
+ }
+
+ void onResolveFailed(Uri uri) {
+ failureCount++;
+ failedUris.add(uri);
}
final boolean hasFailures() {
- return failedFileCount > 0;
+ return failureCount > 0;
}
boolean hasWarnings() {
@@ -234,8 +246,8 @@
} else if (doc.isDeleteSupported()) {
DocumentsContract.deleteDocument(getClient(doc), doc.derivedUri);
} else {
- throw new ResourceException("Unable to delete source document as the file is " +
- "not deletable nor removable: %s.", doc.derivedUri);
+ throw new ResourceException("Unable to delete source document. "
+ + "File is not deletable or removable: %s.", doc.derivedUri);
}
} catch (RemoteException | RuntimeException e) {
throw new ResourceException("Failed to delete file %s due to an exception.",
@@ -253,11 +265,12 @@
final Intent navigateIntent = buildNavigateIntent(INTENT_TAG_FAILURE);
navigateIntent.putExtra(EXTRA_DIALOG_TYPE, OperationDialogFragment.DIALOG_TYPE_FAILURE);
navigateIntent.putExtra(EXTRA_OPERATION_TYPE, operationType);
- navigateIntent.putParcelableArrayListExtra(EXTRA_SRC_LIST, failedFiles);
+ navigateIntent.putParcelableArrayListExtra(EXTRA_FAILED_DOCS, failedDocs);
+ navigateIntent.putParcelableArrayListExtra(EXTRA_FAILED_URIS, failedUris);
final Notification.Builder errorBuilder = new Notification.Builder(service)
.setContentTitle(service.getResources().getQuantityString(titleId,
- failedFileCount, failedFileCount))
+ failureCount, failureCount))
.setContentText(service.getString(R.string.notification_touch_for_details))
.setContentIntent(PendingIntent.getActivity(appContext, 0, navigateIntent,
PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_ONE_SHOT))
diff --git a/src/com/android/documentsui/services/MoveJob.java b/src/com/android/documentsui/services/MoveJob.java
index a315579..c7e9fc9 100644
--- a/src/com/android/documentsui/services/MoveJob.java
+++ b/src/com/android/documentsui/services/MoveJob.java
@@ -34,9 +34,10 @@
import com.android.documentsui.base.DocumentStack;
import com.android.documentsui.clipping.UrisSupplier;
-import javax.annotation.Nullable;
import java.io.FileNotFoundException;
+import javax.annotation.Nullable;
+
// TODO: Stop extending CopyJob.
final class MoveJob extends CopyJob {
@@ -93,7 +94,7 @@
mSrcParent = DocumentInfo.fromUri(resolver, mSrcParentUri);
} catch (FileNotFoundException e) {
Log.e(TAG, "Failed to create srcParent.", e);
- failedFileCount += srcs.getItemCount();
+ failureCount = mResourceUris.getItemCount();
return false;
}
}
@@ -112,7 +113,7 @@
@Override
boolean checkSpace() {
long size = 0;
- for (DocumentInfo src : mSrcs) {
+ for (DocumentInfo src : mResolvedDocs) {
if (!src.authority.equals(stack.getRoot().authority)) {
if (src.isDirectory()) {
try {
@@ -129,7 +130,7 @@
}
}
- return checkSpace(size);
+ return verifySpaceAvailable(size);
}
void processDocument(DocumentInfo src, DocumentInfo srcParent, DocumentInfo dest)
@@ -179,7 +180,8 @@
.append("MoveJob")
.append("{")
.append("id=" + id)
- .append(", srcs=" + mSrcs)
+ .append(", uris=" + mResourceUris)
+ .append(", docs=" + mResolvedDocs)
.append(", srcParent=" + mSrcParent)
.append(", destination=" + stack)
.append("}")
diff --git a/src/com/android/documentsui/services/ResolvedResourcesJob.java b/src/com/android/documentsui/services/ResolvedResourcesJob.java
new file mode 100644
index 0000000..145fcee
--- /dev/null
+++ b/src/com/android/documentsui/services/ResolvedResourcesJob.java
@@ -0,0 +1,123 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.documentsui.services;
+
+import android.content.ContentResolver;
+import android.content.Context;
+import android.net.Uri;
+import android.util.Log;
+
+import com.android.documentsui.base.DocumentInfo;
+import com.android.documentsui.base.DocumentStack;
+import com.android.documentsui.base.RootInfo;
+import com.android.documentsui.clipping.UrisSupplier;
+import com.android.documentsui.services.FileOperationService.OpType;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Abstract job that resolves all resource URIs into mResolvedDocs. This provides
+ * uniform error handling and reporting on resource resolution failures, as well
+ * as an easy path for sub-classes to recover and continue past partial failures.
+ */
+public abstract class ResolvedResourcesJob extends Job {
+ private static final String TAG = "ResolvedResourcesJob";
+
+ final List<DocumentInfo> mResolvedDocs;
+
+ ResolvedResourcesJob(Context service, Listener listener, String id, @OpType int opType,
+ DocumentStack destination, UrisSupplier srcs) {
+ super(service, listener, id, opType, destination, srcs);
+
+ assert(srcs.getItemCount() > 0);
+
+ // delay the initialization of it to setUp() because it may be IO extensive.
+ mResolvedDocs = new ArrayList<>(srcs.getItemCount());
+
+ }
+
+ boolean setUp() {
+ if (!super.setUp()) {
+ return false;
+ }
+
+ int docsResolved = buildDocumentList();
+ if (!isCanceled() && docsResolved < mResourceUris.getItemCount()) {
+ if (docsResolved == 0) {
+ Log.e(TAG, "Failed to load any documents. Aborting.");
+ return false;
+ } else {
+ Log.e(TAG, "Failed to load some documents. Processing loaded documents only.");
+ }
+ }
+
+ return true;
+ }
+
+ /**
+ * Allows sub-classes to exclude files from processing.
+ * By default all files are eligible.
+ */
+ boolean isEligibleDoc(DocumentInfo doc, RootInfo root) {
+ return true;
+ }
+
+ /**
+ * @return number of docs successfully loaded.
+ */
+ protected int buildDocumentList() {
+ final ContentResolver resolver = appContext.getContentResolver();
+ Iterable<Uri> uris;
+ try {
+ uris = mResourceUris.getUris(appContext);
+ } catch (IOException e) {
+ Log.e(TAG, "Failed to read list of target resource Uris. Cannot continue.", e);
+ failureCount = this.mResourceUris.getItemCount();
+ return 0;
+ }
+
+ int docsLoaded = 0;
+ for (Uri uri : uris) {
+
+ DocumentInfo doc;
+ try {
+ doc = DocumentInfo.fromUri(resolver, uri);
+ } catch (FileNotFoundException e) {
+ Log.e(TAG, "Failed to resolve content from Uri: " + uri
+ + ". Skipping to next resource.", e);
+ onResolveFailed(uri);
+ continue;
+ }
+
+ if (isEligibleDoc(doc, stack.getRoot())) {
+ mResolvedDocs.add(doc);
+ } else {
+ onFileFailed(doc);
+ }
+ docsLoaded++;
+
+ if (isCanceled()) {
+ break;
+ }
+ }
+
+ return docsLoaded;
+ }
+}
diff --git a/tests/common/com/android/documentsui/services/TestJobListener.java b/tests/common/com/android/documentsui/services/TestJobListener.java
index 3488650..6977806 100644
--- a/tests/common/com/android/documentsui/services/TestJobListener.java
+++ b/tests/common/com/android/documentsui/services/TestJobListener.java
@@ -19,6 +19,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
+import android.net.Uri;
import android.support.annotation.Nullable;
import com.android.documentsui.base.DocumentInfo;
@@ -64,12 +65,12 @@
}
}
- public void assertFilesFailed(ArrayList<String> names) {
+ public void assertFilesFailed(List<String> names) {
if (finished == null || !finished.hasFailures()) {
fail("Can't test failed documetns. Job didn't fail.");
}
- assertEquals(finished.failedFiles.size(), names.size());
+ assertEquals(finished.failedDocs.size(), names.size());
for (String name : names) {
assertFileFailed(name);
}
@@ -80,7 +81,7 @@
fail("Can't test failed documetns. Job didn't fail.");
}
- for (DocumentInfo failed : finished.failedFiles) {
+ for (DocumentInfo failed : finished.failedDocs) {
if (name.equals(failed.displayName)) {
return;
}
@@ -88,6 +89,38 @@
fail("Couldn't find failed file: " + name);
}
+ public void assertUrisFailed(List<Uri> uris) {
+ if (finished == null || !finished.hasFailures()) {
+ fail("Can't test failed documetns. Job didn't fail.");
+ }
+
+ assertEquals(finished.failedDocs.size(), uris.size());
+ for (Uri uri : uris) {
+ assertUriFailed(uri);
+ }
+ }
+
+ public void assertUriFailed(Uri uri) {
+ if (finished == null || !finished.hasFailures()) {
+ fail("Can't test failed documetns. Job didn't fail.");
+ }
+
+ for (Uri failed : finished.failedUris) {
+ if (uri.equals(failed)) {
+ return;
+ }
+ }
+ fail("Couldn't find failed uri: " + uri);
+ }
+
+ public void assertFailureCount(int expected) {
+ if (finished == null) {
+ fail("No job to test.");
+ }
+
+ assertEquals(expected, finished.failureCount);
+ }
+
public void assertCanceled() {
if (finished == null) {
fail("Can't determine if job was canceled. Job didn't finish.");
diff --git a/tests/unit/com/android/documentsui/services/AbstractJobTest.java b/tests/unit/com/android/documentsui/services/AbstractJobTest.java
index d6b1a70..8fbba19 100644
--- a/tests/unit/com/android/documentsui/services/AbstractJobTest.java
+++ b/tests/unit/com/android/documentsui/services/AbstractJobTest.java
@@ -24,15 +24,15 @@
import android.content.Context;
import android.net.Uri;
import android.os.RemoteException;
+import android.support.test.filters.MediumTest;
import android.test.AndroidTestCase;
-import android.test.suitebuilder.annotation.MediumTest;
-import com.android.documentsui.clipping.UrisSupplier;
import com.android.documentsui.DocumentsProviderHelper;
import com.android.documentsui.StubProvider;
import com.android.documentsui.base.DocumentInfo;
import com.android.documentsui.base.DocumentStack;
import com.android.documentsui.base.RootInfo;
+import com.android.documentsui.clipping.UrisSupplier;
import com.android.documentsui.services.FileOperationService.OpType;
import com.android.documentsui.testing.DocsProviders;
diff --git a/tests/unit/com/android/documentsui/services/CopyJobTest.java b/tests/unit/com/android/documentsui/services/CopyJobTest.java
index 64211c2..a6b9562 100644
--- a/tests/unit/com/android/documentsui/services/CopyJobTest.java
+++ b/tests/unit/com/android/documentsui/services/CopyJobTest.java
@@ -22,7 +22,7 @@
import android.net.Uri;
import android.provider.DocumentsContract.Document;
-import android.test.suitebuilder.annotation.MediumTest;
+import android.support.test.filters.MediumTest;
@MediumTest
public class CopyJobTest extends AbstractCopyJobTest<CopyJob> {
diff --git a/tests/unit/com/android/documentsui/services/DeleteJobTest.java b/tests/unit/com/android/documentsui/services/DeleteJobTest.java
index f97a360..0d8d39b 100644
--- a/tests/unit/com/android/documentsui/services/DeleteJobTest.java
+++ b/tests/unit/com/android/documentsui/services/DeleteJobTest.java
@@ -22,7 +22,7 @@
import android.net.Uri;
import android.provider.DocumentsContract;
-import android.test.suitebuilder.annotation.MediumTest;
+import android.support.test.filters.MediumTest;
import java.util.List;
diff --git a/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java b/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java
index 306be3c..3c4d74a 100644
--- a/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java
+++ b/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java
@@ -18,10 +18,8 @@
import static com.android.documentsui.services.FileOperationService.OPERATION_COPY;
import static com.android.documentsui.services.FileOperationService.OPERATION_DELETE;
-import static com.android.documentsui.services.FileOperationService.OpType;
import static com.android.documentsui.services.FileOperations.createBaseIntent;
import static com.android.documentsui.services.FileOperations.createJobId;
-
import static com.google.android.collect.Lists.newArrayList;
import android.content.Context;
@@ -29,12 +27,13 @@
import android.net.Uri;
import android.os.Parcel;
import android.os.Parcelable;
+import android.support.test.filters.MediumTest;
import android.test.ServiceTestCase;
-import android.test.suitebuilder.annotation.MediumTest;
import com.android.documentsui.base.DocumentInfo;
import com.android.documentsui.base.DocumentStack;
import com.android.documentsui.clipping.UrisSupplier;
+import com.android.documentsui.services.FileOperationService.OpType;
import com.android.documentsui.testing.DocsProviders;
import com.android.documentsui.testing.TestHandler;
import com.android.documentsui.testing.TestScheduledExecutorService;
diff --git a/tests/unit/com/android/documentsui/services/JobErrorHandlingTest.java b/tests/unit/com/android/documentsui/services/JobErrorHandlingTest.java
new file mode 100644
index 0000000..89b34bc
--- /dev/null
+++ b/tests/unit/com/android/documentsui/services/JobErrorHandlingTest.java
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.documentsui.services;
+
+import static com.android.documentsui.services.FileOperationService.OPERATION_DELETE;
+import static com.google.common.collect.Lists.newArrayList;
+
+import android.net.Uri;
+import android.provider.DocumentsContract;
+import android.support.test.filters.MediumTest;
+
+import java.util.List;
+
+/**
+ * With this test we're interested in testing a common ancestor class shared by DeleteJob, CopyJob,
+ * and MoveJob. Since DeleteJob is simplest, we use that.
+ */
+@MediumTest
+public class JobErrorHandlingTest extends AbstractJobTest<DeleteJob> {
+
+ public void testRecoversFromInvalidUri() throws Exception {
+ Uri invalidUri1 = Uri.parse("content://poodles/chuckleberry/ham");
+ Uri validUri = mDocs.createDocument(mSrcRoot, "text/plain", "test2.txt");
+ Uri invalidUri2 = Uri.parse("content://poodles/chuckleberry/ham");
+ mDocs.writeDocument(validUri, FRUITY_BYTES);
+
+ createJob(newArrayList(invalidUri1, validUri, invalidUri2),
+ DocumentsContract.buildDocumentUri(AUTHORITY, mSrcRoot.documentId)).run();
+ mJobListener.waitForFinished();
+ mJobListener.assertFinished();
+
+ // verify that the document associated with the one valid uri was deleted.
+ mDocs.assertChildCount(mSrcRoot, 0);
+ }
+
+ public void testRecordsInvalidUris() throws Exception {
+ Uri invalidUri1 = Uri.parse("content://poodles/chuckleberry/ham");
+ Uri validUri = mDocs.createDocument(mSrcRoot, "text/plain", "test2.txt");
+ Uri invalidUri2 = Uri.parse("content://poodles/chuckleberry/ham");
+ mDocs.writeDocument(validUri, FRUITY_BYTES);
+
+ createJob(newArrayList(invalidUri1, validUri, invalidUri2),
+ DocumentsContract.buildDocumentUri(AUTHORITY, mSrcRoot.documentId)).run();
+ mJobListener.waitForFinished();
+
+ mJobListener.assertUriFailed(invalidUri1);
+ mJobListener.assertUriFailed(invalidUri2);
+ }
+
+ public void testReportsCorrectFailureCount() throws Exception {
+ Uri invalidUri1 = Uri.parse("content://poodles/chuckleberry/ham");
+ Uri validUri = mDocs.createDocument(mSrcRoot, "text/plain", "test2.txt");
+ Uri invalidUri2 = Uri.parse("content://poodles/chuckleberry/ham");
+ mDocs.writeDocument(validUri, FRUITY_BYTES);
+
+ createJob(newArrayList(invalidUri1, validUri, invalidUri2),
+ DocumentsContract.buildDocumentUri(AUTHORITY, mSrcRoot.documentId)).run();
+ mJobListener.waitForFinished();
+
+ mJobListener.assertFailureCount(2);
+ }
+
+ /**
+ * Creates a job with a stack consisting to the default src directory.
+ */
+ private final DeleteJob createJob(List<Uri> srcs, Uri srcParent) throws Exception {
+ Uri stack = DocumentsContract.buildDocumentUri(AUTHORITY, mSrcRoot.documentId);
+ return createJob(OPERATION_DELETE, srcs, srcParent, stack);
+ }
+}
diff --git a/tests/unit/com/android/documentsui/services/MoveJobTest.java b/tests/unit/com/android/documentsui/services/MoveJobTest.java
index 6bdd9ed..8607576 100644
--- a/tests/unit/com/android/documentsui/services/MoveJobTest.java
+++ b/tests/unit/com/android/documentsui/services/MoveJobTest.java
@@ -17,12 +17,11 @@
package com.android.documentsui.services;
import static com.android.documentsui.services.FileOperationService.OPERATION_MOVE;
-
import static com.google.common.collect.Lists.newArrayList;
import android.net.Uri;
import android.provider.DocumentsContract.Document;
-import android.test.suitebuilder.annotation.MediumTest;
+import android.support.test.filters.MediumTest;
@MediumTest
public class MoveJobTest extends AbstractCopyJobTest<MoveJob> {