summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Steve McKay <smckay@google.com> 2016-12-29 16:02:01 -0800
committer Steve McKay <smckay@google.com> 2017-01-10 02:43:02 +0000
commit99f1dc3da6defec0596864934bfe76adc96a1d62 (patch)
treea7a9b6bb709d9969c5a1584ae2784a727ea0b5dd
parent1dfad7f207dac2c8f623ec54ca161e7a8e807596 (diff)
Improve error handling.
Recover gracefully when a uri cannot be resolved. Include Uris in information presented in error dialog. Update DeleteJob and CopyJob to have shared uri resolution code. Break out error handling to provide clearer and more granular error info. Update Metrics to be sure we including error information about URIs we failed to resolve. Bug: 33938336 Test: New coverage in JobErrorHandlingTest. Change-Id: Idf34882a561ec5cb90170f291683bdc752188b57
-rw-r--r--src/com/android/documentsui/Metrics.java49
-rw-r--r--src/com/android/documentsui/OperationDialogFragment.java29
-rw-r--r--src/com/android/documentsui/files/FilesActivity.java9
-rw-r--r--src/com/android/documentsui/services/CopyJob.java86
-rw-r--r--src/com/android/documentsui/services/DeleteJob.java82
-rw-r--r--src/com/android/documentsui/services/FileOperationService.java10
-rw-r--r--src/com/android/documentsui/services/Job.java47
-rw-r--r--src/com/android/documentsui/services/MoveJob.java12
-rw-r--r--src/com/android/documentsui/services/ResolvedResourcesJob.java123
-rw-r--r--tests/common/com/android/documentsui/services/TestJobListener.java39
-rw-r--r--tests/unit/com/android/documentsui/services/AbstractJobTest.java4
-rw-r--r--tests/unit/com/android/documentsui/services/CopyJobTest.java2
-rw-r--r--tests/unit/com/android/documentsui/services/DeleteJobTest.java2
-rw-r--r--tests/unit/com/android/documentsui/services/FileOperationServiceTest.java5
-rw-r--r--tests/unit/com/android/documentsui/services/JobErrorHandlingTest.java84
-rw-r--r--tests/unit/com/android/documentsui/services/MoveJobTest.java3
16 files changed, 419 insertions, 167 deletions
diff --git a/src/com/android/documentsui/Metrics.java b/src/com/android/documentsui/Metrics.java
index 15cf90219..cc3597eb2 100644
--- a/src/com/android/documentsui/Metrics.java
+++ b/src/com/android/documentsui/Metrics.java
@@ -392,7 +392,9 @@ public final class Metrics {
@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 @@ public final class Metrics {
* @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 @@ public final class Metrics {
* 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);
+ }
+ }
+
+ /**
+ * 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++;
}
- return counts;
}
private static class ProviderCounts {
diff --git a/src/com/android/documentsui/OperationDialogFragment.java b/src/com/android/documentsui/OperationDialogFragment.java
index 19730b348..6a763128d 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 com.android.documentsui.services.FileOperationService.OpType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
-
/**
* Alert dialog for operation dialogs.
*/
@@ -55,13 +55,18 @@ public class OperationDialogFragment extends DialogFragment {
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 @@ public class OperationDialogFragment extends DialogFragment {
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 @@ public class OperationDialogFragment extends DialogFragment {
}
final StringBuilder list = new StringBuilder("<p>");
- for (DocumentInfo documentInfo : srcList) {
- list.append(String.format("&#8226; %s<br>", Html.escapeHtml(documentInfo.displayName)));
+ for (DocumentInfo documentInfo : docList) {
+ list.append("&#8226; " + Html.escapeHtml(documentInfo.displayName) + "<br>");
+ }
+ for (Uri uri : uriList) {
+ list.append("&#8226; " + 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 85c140aff..e53e58817 100644
--- a/src/com/android/documentsui/files/FilesActivity.java
+++ b/src/com/android/documentsui/files/FilesActivity.java
@@ -154,12 +154,15 @@ public class FilesActivity extends BaseActivity implements ActionHandler.Addons
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 30bbbf14b..e001ee373 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;
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.IOException;
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 @@ class CopyJob extends Job {
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 @@ class CopyJob extends Job {
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 @@ class CopyJob extends Job {
}
if (mSampleTime > 0 && mSpeed > 0) {
- mRemainingTime = ((mBatchSize - bytesCopied) * 1000) / mSpeed;
+ mRemainingTime = ((mBytesRequired - bytesCopied) * 1000) / mSpeed;
} else {
mRemainingTime = 0;
}
@@ -211,10 +205,7 @@ class CopyJob extends Job {
@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 @@ class CopyJob extends Job {
}
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 @@ class CopyJob extends Job {
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 @@ class CopyJob extends Job {
onFileFailed(srcInfo);
}
}
- Metrics.logFileOperation(service, operationType, mSrcs, 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);
- }
+ Metrics.logFileOperation(service, operationType, mResolvedDocs, dstInfo);
}
- 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 @@ class CopyJob extends Job {
* @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 @@ class CopyJob extends Job {
* @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 @@ class CopyJob extends Job {
// 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 @@ class CopyJob extends Job {
* 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 @@ class CopyJob extends Job {
.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 5d32b705a..4031c1fdb 100644
--- a/src/com/android/documentsui/services/DeleteJob.java
+++ b/src/com/android/documentsui/services/DeleteJob.java
@@ -26,22 +26,21 @@ import android.content.Context;
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 @@ final class DeleteJob extends Job {
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 @@ final class DeleteJob extends Job {
@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 @@ final class DeleteJob extends Job {
@Override
void start() {
+ ContentResolver resolver = appContext.getContentResolver();
+
+ DocumentInfo parentDoc;
try {
- final List<DocumentInfo> srcs = new ArrayList<>(this.srcs.getItemCount());
-
- 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;
+ 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;
+ }
+
+ 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 @@ final class DeleteJob extends Job {
.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 d28a578f1..d8485ba17 100644
--- a/src/com/android/documentsui/services/FileOperationService.java
+++ b/src/com/android/documentsui/services/FileOperationService.java
@@ -56,6 +56,9 @@ public class FileOperationService extends Service implements Job.Listener {
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 @@ public class FileOperationService extends Service implements Job.Listener {
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 3cec4a47e..dff360af5 100644
--- a/src/com/android/documentsui/services/Job.java
+++ b/src/com/android/documentsui/services/Job.java
@@ -19,9 +19,10 @@ package com.android.documentsui.services;
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.os.RemoteException;
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 @@ abstract public class Job implements Runnable {
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 @@ abstract public class Job implements Runnable {
this.id = id;
this.stack = stack;
- this.srcs = srcs;
+ this.mResourceUris = srcs;
mProgressBuilder = createProgressBuilder();
}
@@ -135,6 +140,7 @@ abstract public class Job implements Runnable {
mState = STATE_STARTED;
listener.onStart(this);
+
try {
boolean result = setUp();
if (result && !isCanceled()) {
@@ -145,20 +151,21 @@ abstract public class Job implements Runnable {
// 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 @@ abstract public class Job implements Runnable {
}
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 @@ abstract public class Job implements Runnable {
} 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 @@ abstract public class Job implements Runnable {
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 a31557984..c7e9fc961 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.DocumentInfo;
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 @@ final class MoveJob extends CopyJob {
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 @@ final class MoveJob extends CopyJob {
@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 @@ final class MoveJob extends CopyJob {
}
}
- return checkSpace(size);
+ return verifySpaceAvailable(size);
}
void processDocument(DocumentInfo src, DocumentInfo srcParent, DocumentInfo dest)
@@ -179,7 +180,8 @@ final class MoveJob extends CopyJob {
.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 000000000..145fceef9
--- /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 3488650f4..6977806c5 100644
--- a/tests/common/com/android/documentsui/services/TestJobListener.java
+++ b/tests/common/com/android/documentsui/services/TestJobListener.java
@@ -19,6 +19,7 @@ package com.android.documentsui.services;
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 class TestJobListener implements Job.Listener {
}
}
- 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 @@ public class TestJobListener implements Job.Listener {
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 @@ public class TestJobListener implements Job.Listener {
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 d6b1a7058..8fbba1967 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.ContentResolver;
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 64211c20e..a6b9562c0 100644
--- a/tests/unit/com/android/documentsui/services/CopyJobTest.java
+++ b/tests/unit/com/android/documentsui/services/CopyJobTest.java
@@ -22,7 +22,7 @@ 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 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 f97a3606d..0d8d39be8 100644
--- a/tests/unit/com/android/documentsui/services/DeleteJobTest.java
+++ b/tests/unit/com/android/documentsui/services/DeleteJobTest.java
@@ -22,7 +22,7 @@ import static com.google.common.collect.Lists.newArrayList;
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 306be3ca1..3c4d74a3d 100644
--- a/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java
+++ b/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java
@@ -18,10 +18,8 @@ package com.android.documentsui.services;
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.content.Intent;
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 000000000..89b34bc42
--- /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 6bdd9ed6e..8607576b1 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> {