From 3564543e1ece2211a2d5b729fbbf8dd7ac94944c Mon Sep 17 00:00:00 2001 From: Steve McKay Date: Wed, 20 Jan 2016 15:09:35 -0800 Subject: [PATCH] Move delete support into FileOperationService. Refactor Jobs to work with files from multiple providers. Don't shut down threadpool until service#onDestroy is called. Bug: 26696797, 26462789, 26567205, 25162803, 26714663 Change-Id: Id43e8e3dc2294cd07dcd6a3477b19efb298c260f --- packages/DocumentsUI/res/values/strings.xml | 9 +- .../dirlist/DirectoryFragment.java | 54 +++--- .../android/documentsui/dirlist/Model.java | 95 ---------- .../android/documentsui/services/CopyJob.java | 163 ++++++++---------- .../documentsui/services/DeleteJob.java | 96 +++++++++++ .../services/FileOperationService.java | 60 +++++-- .../documentsui/services/FileOperations.java | 34 +++- .../com/android/documentsui/services/Job.java | 57 +++++- .../android/documentsui/services/MoveJob.java | 39 ++--- .../ModelBackedDocumentsAdapterTest.java | 2 +- .../documentsui/dirlist/ModelTest.java | 36 +--- ...ctionBreakDocumentsAdapterWrapperTest.java | 5 +- .../documentsui/dirlist/TestModel.java | 20 +-- ...yJobTest.java => AbstractCopyJobTest.java} | 67 +------ .../documentsui/services/AbstractJobTest.java | 101 +++++++++++ .../documentsui/services/CopyJobTest.java | 17 +- .../documentsui/services/DeleteJobTest.java | 59 +++++++ .../services/FileOperationServiceTest.java | 9 +- .../documentsui/services/MoveJobTest.java | 17 +- .../android/documentsui/services/TestJob.java | 3 - .../TestScheduledExecutorService.java | 2 +- 21 files changed, 542 insertions(+), 403 deletions(-) create mode 100644 packages/DocumentsUI/src/com/android/documentsui/services/DeleteJob.java rename packages/DocumentsUI/tests/src/com/android/documentsui/services/{BaseCopyJobTest.java => AbstractCopyJobTest.java} (71%) create mode 100644 packages/DocumentsUI/tests/src/com/android/documentsui/services/AbstractJobTest.java create mode 100644 packages/DocumentsUI/tests/src/com/android/documentsui/services/DeleteJobTest.java diff --git a/packages/DocumentsUI/res/values/strings.xml b/packages/DocumentsUI/res/values/strings.xml index 016657e616e82..05c43b2fca3fa 100644 --- a/packages/DocumentsUI/res/values/strings.xml +++ b/packages/DocumentsUI/res/values/strings.xml @@ -87,7 +87,7 @@ Dismiss Try Again - + By name @@ -158,6 +158,8 @@ Preparing for copy\u2026 Preparing for move\u2026 + + Preparing for delete\u2026 Couldn\'t copy %1$d file @@ -168,6 +170,11 @@ Couldn\'t move %1$d file Couldn\'t move %1$d files + + + Couldn\'t delete %1$d file + Couldn\'t delete %1$d files + Touch to view details diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java index 580e2d8d97b74..6b9c90c2ae32e 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java +++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java @@ -124,11 +124,13 @@ public class DirectoryFragment extends Fragment implements DocumentsAdapter.Envi public static final int REQUEST_COPY_DESTINATION = 1; - private static final String TAG = "DirectoryFragment"; - - private static final int LOADER_ID = 42; static final boolean DEBUG_ENABLE_DND = true; + private static final String TAG = "DirectoryFragment"; + private static final int LOADER_ID = 42; + private static final int DELETE_UNDO_TIMEOUT = 5000; + private static final int DELETE_JOB_DELAY = 5500; + private static final String EXTRA_TYPE = "type"; private static final String EXTRA_ROOT = "root"; private static final String EXTRA_DOC = "doc"; @@ -339,7 +341,7 @@ public class DirectoryFragment extends Fragment implements DocumentsAdapter.Envi : MultiSelectManager.MODE_SINGLE); mSelectionManager.addCallback(new SelectionModeListener()); - mModel = new Model(context); + mModel = new Model(); mModel.addUpdateListener(mAdapter); mModel.addUpdateListener(mModelUpdateListener); @@ -852,16 +854,32 @@ public class DirectoryFragment extends Fragment implements DocumentsAdapter.Envi } private void deleteDocuments(final Selection selected) { - Context context = getActivity(); - String message = Shared.getQuantityString(context, R.plurals.deleting, selected.size()); - // Hide the files in the UI. - final SparseArray toDelete = mAdapter.hide(selected.getAll()); + checkArgument(!selected.isEmpty()); + new GetDocumentsTask() { + @Override + void onDocumentsReady(List docs) { + // Hide the files in the UI. + final SparseArray hidden = mAdapter.hide(selected.getAll()); + + checkState(DELETE_JOB_DELAY > DELETE_UNDO_TIMEOUT); + String operationId = FileOperations.delete( + getActivity(), docs, getDisplayState().stack, + DELETE_JOB_DELAY); + showDeleteSnackbar(hidden, operationId); + } + }.execute(selected); + } + + private void showDeleteSnackbar(final SparseArray hidden, final String jobId) { + + Context context = getActivity(); + String message = Shared.getQuantityString(context, R.plurals.deleting, hidden.size()); // Show a snackbar informing the user that files will be deleted, and give them an option to // cancel. final Activity activity = getActivity(); - Snackbars.makeSnackbar(activity, message, Snackbar.LENGTH_LONG) + Snackbars.makeSnackbar(activity, message, DELETE_UNDO_TIMEOUT) .setAction( R.string.undo, new View.OnClickListener() { @@ -874,22 +892,8 @@ public class DirectoryFragment extends Fragment implements DocumentsAdapter.Envi public void onDismissed(Snackbar snackbar, int event) { if (event == Snackbar.Callback.DISMISS_EVENT_ACTION) { // If the delete was cancelled, just unhide the files. - mAdapter.unhide(toDelete); - } else { - // Actually kick off the delete. - mModel.delete( - selected, - new Model.DeletionListener() { - @Override - public void onError() { - Snackbars.makeSnackbar( - activity, - R.string.toast_failed_delete, - Snackbar.LENGTH_LONG) - .show(); - - } - }); + FileOperations.cancel(activity, jobId); + mAdapter.unhide(hidden); } } }) diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java index cf21d1547b027..075b3ea9cdfe4 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java +++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java @@ -24,11 +24,7 @@ import static com.android.documentsui.model.DocumentInfo.getCursorLong; import static com.android.documentsui.model.DocumentInfo.getCursorString; import static com.android.internal.util.Preconditions.checkNotNull; -import android.content.ContentProviderClient; -import android.content.ContentResolver; -import android.content.Context; import android.database.Cursor; -import android.os.AsyncTask; import android.os.Bundle; import android.os.Looper; import android.provider.DocumentsContract; @@ -39,7 +35,6 @@ import android.util.Log; import com.android.documentsui.BaseActivity.SiblingProvider; import com.android.documentsui.DirectoryResult; -import com.android.documentsui.DocumentsApplication; import com.android.documentsui.RootCursorWrapper; import com.android.documentsui.dirlist.MultiSelectManager.Selection; import com.android.documentsui.model.DocumentInfo; @@ -56,7 +51,6 @@ import java.util.Map; public class Model implements SiblingProvider { private static final String TAG = "Model"; - private Context mContext; private boolean mIsLoading; private List mUpdateListeners = new ArrayList<>(); @Nullable private Cursor mCursor; @@ -73,10 +67,6 @@ public class Model implements SiblingProvider { @Nullable String info; @Nullable String error; - Model(Context context) { - mContext = context; - } - /** * Generates a Model ID for a cursor entry that refers to a document. The Model ID is a unique * string that can be used to identify the document referred to by the cursor. @@ -406,91 +396,6 @@ public class Model implements SiblingProvider { return mCursor; } - public void delete(Selection selected, DeletionListener listener) { - final ContentResolver resolver = mContext.getContentResolver(); - new DeleteFilesTask(resolver, listener).execute(selected); - } - - /** - * A Task which collects the DocumentInfo for documents that have been marked for deletion, - * and actually deletes them. - */ - private class DeleteFilesTask extends AsyncTask { - private ContentResolver mResolver; - private DeletionListener mListener; - private boolean mHadTrouble; - - /** - * @param resolver A ContentResolver for performing the actual file deletions. - * @param errorCallback A Runnable that is executed in the event that one or more errors - * occurred while copying files. Execution will occur on the UI thread. - */ - public DeleteFilesTask(ContentResolver resolver, DeletionListener listener) { - mResolver = resolver; - mListener = listener; - } - - @Override - protected Void doInBackground(Selection... selected) { - List toDelete = null; - try { - toDelete = getDocuments(selected[0]); - } catch (NullPointerException e) { - Log.w(TAG, "Failed to retrieve documents for delete."); - mHadTrouble = true; - return null; - } - - for (DocumentInfo doc : toDelete) { - if (!doc.isDeleteSupported()) { - Log.w(TAG, doc + " could not be deleted. Skipping..."); - mHadTrouble = true; - continue; - } - - ContentProviderClient client = null; - try { - if (DEBUG) Log.d(TAG, "Deleting: " + doc.displayName); - client = DocumentsApplication.acquireUnstableProviderOrThrow( - mResolver, doc.derivedUri.getAuthority()); - DocumentsContract.deleteDocument(client, doc.derivedUri); - } catch (Exception e) { - Log.w(TAG, "Failed to delete " + doc, e); - mHadTrouble = true; - } finally { - ContentProviderClient.releaseQuietly(client); - } - } - - return null; - } - - @Override - protected void onPostExecute(Void _) { - if (mHadTrouble) { - // TODO show which files failed? b/23720103 - mListener.onError(); - if (DEBUG) Log.d(TAG, "Deletion task completed. Some deletions failed."); - } else { - if (DEBUG) Log.d(TAG, "Deletion task completed successfully."); - } - - mListener.onCompletion(); - } - } - - static class DeletionListener { - /** - * Called when deletion has completed (regardless of whether an error occurred). - */ - void onCompletion() {} - - /** - * Called at the end of a deletion operation that produced one or more errors. - */ - void onError() {} - } - void addUpdateListener(UpdateListener listener) { mUpdateListeners.add(listener); } diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java b/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java index b1932b8efb8f1..f3195a7a0fdf4 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java +++ b/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java @@ -21,7 +21,6 @@ import static android.provider.DocumentsContract.buildChildDocumentsUri; import static android.provider.DocumentsContract.buildDocumentUri; import static android.provider.DocumentsContract.getDocumentId; import static android.provider.DocumentsContract.isChildDocument; -import static com.android.documentsui.DocumentsApplication.acquireUnstableProviderOrThrow; import static com.android.documentsui.Shared.DEBUG; import static com.android.documentsui.model.DocumentInfo.getCursorLong; import static com.android.documentsui.model.DocumentInfo.getCursorString; @@ -62,12 +61,7 @@ import java.util.List; class CopyJob extends Job { private static final String TAG = "CopyJob"; private static final int PROGRESS_INTERVAL_MILLIS = 1000; - final List mSrcFiles; - - // Provider clients are acquired for the duration of each copy job. Note that there is an - // implicit assumption that all srcs come from the same authority. - ContentProviderClient srcClient; - ContentProviderClient dstClient; + final List mSrcs; private long mStartTime = -1; private long mBatchSize; @@ -86,11 +80,11 @@ class CopyJob extends Job { * @param srcs List of files to be copied. */ CopyJob(Context service, Context appContext, Listener listener, - String id, DocumentStack destination, List srcs) { - super(service, appContext, listener, OPERATION_COPY, id, destination); + String id, DocumentStack stack, List srcs) { + super(service, appContext, listener, OPERATION_COPY, id, stack); checkArgument(!srcs.isEmpty()); - this.mSrcFiles = srcs; + this.mSrcs = srcs; } /** @@ -103,7 +97,7 @@ class CopyJob extends Job { super(service, appContext, listener, opType, id, destination); checkArgument(!srcs.isEmpty()); - this.mSrcFiles = srcs; + this.mSrcs = srcs; } @Override @@ -185,21 +179,13 @@ class CopyJob extends Job { void start() throws RemoteException { mStartTime = elapsedRealtime(); - // Acquire content providers. - srcClient = acquireUnstableProviderOrThrow( - getContentResolver(), - mSrcFiles.get(0).authority); - dstClient = acquireUnstableProviderOrThrow( - getContentResolver(), - stack.peek().authority); - // client - mBatchSize = calculateSize(srcClient, mSrcFiles); + mBatchSize = calculateSize(mSrcs); DocumentInfo srcInfo; DocumentInfo dstInfo; - for (int i = 0; i < mSrcFiles.size() && !isCanceled(); ++i) { - srcInfo = mSrcFiles.get(i); + for (int i = 0; i < mSrcs.size() && !isCanceled(); ++i) { + srcInfo = mSrcs.get(i); dstInfo = stack.peek(); // Guard unsupported recursive operation. @@ -233,24 +219,24 @@ class CopyJob extends Job { /** * Copies a the given document to the given location. * - * @param srcInfo DocumentInfos for the documents to copy. + * @param src DocumentInfos for the documents to copy. * @param dstDirInfo The destination directory. * @return True on success, false on failure. * @throws RemoteException */ - boolean processDocument(DocumentInfo srcInfo, DocumentInfo dstDirInfo) throws RemoteException { + boolean processDocument(DocumentInfo src, DocumentInfo dstDirInfo) throws RemoteException { // TODO: When optimized copy kicks in, we'll not making any progress updates. // For now. Local storage isn't using optimized copy. // When copying within the same provider, try to use optimized copying. // If not supported, then fallback to byte-by-byte copy/move. - if (srcInfo.authority.equals(dstDirInfo.authority)) { - if ((srcInfo.flags & Document.FLAG_SUPPORTS_COPY) != 0) { - if (DocumentsContract.copyDocument(srcClient, srcInfo.derivedUri, + if (src.authority.equals(dstDirInfo.authority)) { + if ((src.flags & Document.FLAG_SUPPORTS_COPY) != 0) { + if (DocumentsContract.copyDocument(getClient(src), src.derivedUri, dstDirInfo.derivedUri) == null) { - onFileFailed(srcInfo, - "Provider side copy failed for documents: " + srcInfo.derivedUri + "."); + onFileFailed(src, + "Provider side copy failed for documents: " + src.derivedUri + "."); return false; } return true; @@ -258,44 +244,44 @@ class CopyJob extends Job { } // If we couldn't do an optimized copy...we fall back to vanilla byte copy. - return byteCopyDocument(srcInfo, dstDirInfo); + return byteCopyDocument(src, dstDirInfo); } - boolean byteCopyDocument(DocumentInfo srcInfo, DocumentInfo dstDirInfo) + boolean byteCopyDocument(DocumentInfo src, DocumentInfo dest) throws RemoteException { final String dstMimeType; final String dstDisplayName; - if (DEBUG) Log.d(TAG, "Doing byte copy of document: " + srcInfo); + if (DEBUG) Log.d(TAG, "Doing byte copy of document: " + src); // If the file is virtual, but can be converted to another format, then try to copy it // as such format. Also, append an extension for the target mime type (if known). - if (srcInfo.isVirtualDocument()) { + if (src.isVirtualDocument()) { final String[] streamTypes = getContentResolver().getStreamTypes( - srcInfo.derivedUri, "*/*"); + src.derivedUri, "*/*"); if (streamTypes != null && streamTypes.length > 0) { dstMimeType = streamTypes[0]; final String extension = MimeTypeMap.getSingleton(). getExtensionFromMimeType(dstMimeType); - dstDisplayName = srcInfo.displayName + - (extension != null ? "." + extension : srcInfo.displayName); + dstDisplayName = src.displayName + + (extension != null ? "." + extension : src.displayName); } else { - onFileFailed(srcInfo, "Cannot copy virtual file. No streamable formats available."); + onFileFailed(src, "Cannot copy virtual file. No streamable formats available."); return false; } } else { - dstMimeType = srcInfo.mimeType; - dstDisplayName = srcInfo.displayName; + dstMimeType = src.mimeType; + dstDisplayName = src.displayName; } // Create the target document (either a file or a directory), then copy recursively the // contents (bytes or children). - final Uri dstUri = DocumentsContract.createDocument(dstClient, - dstDirInfo.derivedUri, dstMimeType, dstDisplayName); + final Uri dstUri = DocumentsContract.createDocument( + getClient(dest), dest.derivedUri, dstMimeType, dstDisplayName); if (dstUri == null) { // If this is a directory, the entire subdir will not be copied over. - onFileFailed(srcInfo, + onFileFailed(src, "Couldn't create destination document " + dstDisplayName - + " in directory " + dstDirInfo.displayName + "."); + + " in directory " + dest.displayName + "."); return false; } @@ -303,16 +289,16 @@ class CopyJob extends Job { try { dstInfo = DocumentInfo.fromUri(getContentResolver(), dstUri); } catch (FileNotFoundException e) { - onFileFailed(srcInfo, + onFileFailed(src, "Could not load DocumentInfo for newly created file: " + dstUri + "."); return false; } final boolean success; - if (Document.MIME_TYPE_DIR.equals(srcInfo.mimeType)) { - success = copyDirectoryHelper(srcInfo, dstInfo); + if (Document.MIME_TYPE_DIR.equals(src.mimeType)) { + success = copyDirectoryHelper(src, dstInfo); } else { - success = copyFileHelper(srcInfo, dstInfo, dstMimeType); + success = copyFileHelper(src, dstInfo, dstMimeType); } return success; @@ -322,13 +308,13 @@ class CopyJob extends Job { * Handles recursion into a directory and copying its contents. Note that in linux terms, this * does the equivalent of "cp src/* dst", not "cp -r src dst". * - * @param srcDirInfo Info of the directory to copy from. The routine will copy the directory's + * @param srcDir Info of the directory to copy from. The routine will copy the directory's * contents, not the directory itself. - * @param dstDirInfo Info of the directory to copy to. Must be created beforehand. + * @param destDir Info of the directory to copy to. Must be created beforehand. * @return True on success, false if some of the children failed to copy. * @throws RemoteException */ - private boolean copyDirectoryHelper(DocumentInfo srcDirInfo, DocumentInfo dstDirInfo) + private boolean copyDirectoryHelper(DocumentInfo srcDir, DocumentInfo destDir) throws RemoteException { // Recurse into directories. Copy children into the new subdirectory. final String queryColumns[] = new String[] { @@ -342,13 +328,11 @@ class CopyJob extends Job { boolean success = true; try { // Iterate over srcs in the directory; copy to the destination directory. - final Uri queryUri = DocumentsContract.buildChildDocumentsUri(srcDirInfo.authority, - srcDirInfo.documentId); - cursor = srcClient.query(queryUri, queryColumns, null, null, null); - DocumentInfo srcInfo; + final Uri queryUri = buildChildDocumentsUri(srcDir.authority, srcDir.documentId); + cursor = getClient(srcDir).query(queryUri, queryColumns, null, null, null); while (cursor.moveToNext() && !isCanceled()) { - srcInfo = DocumentInfo.fromCursor(cursor, srcDirInfo.authority); - success &= processDocument(srcInfo, dstDirInfo); + DocumentInfo src = DocumentInfo.fromCursor(cursor, srcDir.authority); + success &= processDocument(src, destDir); } } finally { IoUtils.closeQuietly(cursor); @@ -366,50 +350,49 @@ class CopyJob extends Job { * @return True on success, false on error. * @throws RemoteException */ - private boolean copyFileHelper(DocumentInfo srcInfo, DocumentInfo dstInfo, String mimeType) + private boolean copyFileHelper(DocumentInfo src, DocumentInfo dest, String mimeType) throws RemoteException { - // Copy an individual file. CancellationSignal canceller = new CancellationSignal(); ParcelFileDescriptor srcFile = null; ParcelFileDescriptor dstFile = null; - InputStream src = null; - OutputStream dst = null; + InputStream in = null; + OutputStream out = null; boolean success = true; try { // If the file is virtual, but can be converted to another format, then try to copy it // as such format. - if (srcInfo.isVirtualDocument()) { + if (src.isVirtualDocument()) { final AssetFileDescriptor srcFileAsAsset = - srcClient.openTypedAssetFileDescriptor( - srcInfo.derivedUri, mimeType, null, canceller); + getClient(src).openTypedAssetFileDescriptor( + src.derivedUri, mimeType, null, canceller); srcFile = srcFileAsAsset.getParcelFileDescriptor(); - src = new AssetFileDescriptor.AutoCloseInputStream(srcFileAsAsset); + in = new AssetFileDescriptor.AutoCloseInputStream(srcFileAsAsset); } else { - srcFile = srcClient.openFile(srcInfo.derivedUri, "r", canceller); - src = new ParcelFileDescriptor.AutoCloseInputStream(srcFile); + srcFile = getClient(src).openFile(src.derivedUri, "r", canceller); + in = new ParcelFileDescriptor.AutoCloseInputStream(srcFile); } - dstFile = dstClient.openFile(dstInfo.derivedUri, "w", canceller); - dst = new ParcelFileDescriptor.AutoCloseOutputStream(dstFile); + dstFile = getClient(dest).openFile(dest.derivedUri, "w", canceller); + out = new ParcelFileDescriptor.AutoCloseOutputStream(dstFile); byte[] buffer = new byte[32 * 1024]; int len; - while ((len = src.read(buffer)) != -1) { + while ((len = in.read(buffer)) != -1) { if (isCanceled()) { if (DEBUG) Log.d(TAG, "Canceled copy mid-copy. Id:" + id); success = false; break; } - dst.write(buffer, 0, len); + out.write(buffer, 0, len); makeCopyProgress(len); } srcFile.checkError(); } catch (IOException e) { success = false; - onFileFailed(srcInfo, "Exception thrown while copying from " - + srcInfo.derivedUri + " to " + dstInfo.derivedUri + "."); + onFileFailed(src, "Exception thrown while copying from " + + src.derivedUri + " to " + dest.derivedUri + "."); if (dstFile != null) { try { @@ -420,20 +403,20 @@ class CopyJob extends Job { } } finally { // This also ensures the file descriptors are closed. - IoUtils.closeQuietly(src); - IoUtils.closeQuietly(dst); + IoUtils.closeQuietly(in); + IoUtils.closeQuietly(out); } if (!success) { if (DEBUG) Log.d(TAG, "Cleaning up failed operation leftovers."); canceller.cancel(); try { - DocumentsContract.deleteDocument(dstClient, dstInfo.derivedUri); + DocumentsContract.deleteDocument(getClient(dest), dest.derivedUri); } catch (RemoteException e) { // RemoteExceptions usually signal that the connection is dead, so there's no // point attempting to continue. Propagate the exception up so the copy job is // cancelled. - Log.w(TAG, "Failed to cleanup after copy error: " + srcInfo.derivedUri, e); + Log.w(TAG, "Failed to cleanup after copy error: " + src.derivedUri, e); throw e; } } @@ -449,14 +432,14 @@ class CopyJob extends Job { * @return Size in bytes. * @throws RemoteException */ - private static long calculateSize(ContentProviderClient client, List srcs) + private long calculateSize(List srcs) throws RemoteException { long result = 0; for (DocumentInfo src : srcs) { if (src.isDirectory()) { // Directories need to be recursed into. - result += calculateFileSizesRecursively(client, src.derivedUri); + result += calculateFileSizesRecursively(getClient(src), src.derivedUri); } else { result += src.size; } @@ -503,20 +486,14 @@ class CopyJob extends Job { return result; } - @Override - void cleanup() { - ContentProviderClient.releaseQuietly(srcClient); - ContentProviderClient.releaseQuietly(dstClient); - } - /** * Returns true if {@code doc} is a descendant of {@code parentDoc}. * @throws RemoteException */ - boolean isDescendentOf(DocumentInfo doc, DocumentInfo parentDoc) + boolean isDescendentOf(DocumentInfo doc, DocumentInfo parent) throws RemoteException { - if (parentDoc.isDirectory() && doc.authority.equals(parentDoc.authority)) { - return isChildDocument(dstClient, doc.derivedUri, parentDoc.derivedUri); + if (parent.isDirectory() && doc.authority.equals(parent.authority)) { + return isChildDocument(getClient(doc), doc.derivedUri, parent.derivedUri); } return false; } @@ -525,4 +502,16 @@ class CopyJob extends Job { Log.w(TAG, msg); onFileFailed(file); } + + @Override + public String toString() { + return new StringBuilder() + .append("CopyJob") + .append("{") + .append("id=" + id) + .append("srcs=" + mSrcs) + .append(", destination=" + stack) + .append("}") + .toString(); + } } diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/DeleteJob.java b/packages/DocumentsUI/src/com/android/documentsui/services/DeleteJob.java new file mode 100644 index 0000000000000..6a2a7946d99c1 --- /dev/null +++ b/packages/DocumentsUI/src/com/android/documentsui/services/DeleteJob.java @@ -0,0 +1,96 @@ +/* + * 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.Shared.DEBUG; +import static com.android.documentsui.services.FileOperationService.OPERATION_DELETE; + +import android.app.Notification; +import android.app.Notification.Builder; +import android.content.Context; +import android.os.RemoteException; +import android.util.Log; + +import com.android.documentsui.R; +import com.android.documentsui.model.DocumentInfo; +import com.android.documentsui.model.DocumentStack; + +import java.util.List; + +final class DeleteJob extends Job { + + private static final String TAG = "DeleteJob"; + private List mSrcs; + + /** + * Moves files to a destination identified by {@code destination}. + * Performs most work by delegating to CopyJob, then deleting + * a file after it has been copied. + * + * @see @link {@link Job} constructor for most param descriptions. + * + * @param srcs List of files to delete + */ + DeleteJob(Context service, Context appContext, Listener listener, + String id, DocumentStack stack, List srcs) { + super(service, appContext, listener, OPERATION_DELETE, id, stack); + this.mSrcs = srcs; + } + + @Override + Builder createProgressBuilder() { + return super.createProgressBuilder( + service.getString(R.string.move_notification_title), + R.drawable.ic_menu_copy, + service.getString(android.R.string.cancel), + R.drawable.ic_cab_cancel); + } + + @Override + public Notification getSetupNotification() { + return getSetupNotification(service.getString(R.string.delete_preparing)); + } + + @Override + Notification getFailureNotification() { + return getFailureNotification( + R.plurals.delete_error_notification_title, R.drawable.ic_menu_delete); + } + + @Override + void start() throws RemoteException { + for (DocumentInfo doc : mSrcs) { + if (DEBUG) Log.d(TAG, "Deleting document @ " + doc.derivedUri); + if (!deleteDocument(doc)) { + Log.w(TAG, "Failed to delete document @ " + doc.derivedUri); + onFileFailed(doc); + } + } + } + + @Override + public String toString() { + return new StringBuilder() + .append("DeleteJob") + .append("{") + .append("id=" + id) + .append("srcs=" + mSrcs) + .append(", location=" + stack) + .append("}") + .toString(); + } +} diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/FileOperationService.java b/packages/DocumentsUI/src/com/android/documentsui/services/FileOperationService.java index 1df20ac15cfa8..d6479507f56b9 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/services/FileOperationService.java +++ b/packages/DocumentsUI/src/com/android/documentsui/services/FileOperationService.java @@ -27,6 +27,7 @@ import android.app.Service; import android.content.Intent; import android.os.IBinder; import android.os.PowerManager; +import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; import android.util.Log; @@ -92,7 +93,7 @@ public class FileOperationService extends Service implements Job.Listener { @GuardedBy("mRunning") private Map mRunning = new HashMap<>(); - private int mLastStarted; + private int mLastServiceId; @Override public void onCreate() { @@ -111,7 +112,18 @@ public class FileOperationService extends Service implements Job.Listener { } @Override - public int onStartCommand(Intent intent, int flags, int startTime) { + public void onDestroy() { + if (DEBUG) Log.d(TAG, "Shutting down executor."); + List unfinished = executor.shutdownNow(); + if (!unfinished.isEmpty()) { + Log.w(TAG, "Shutting down, but executor reports running jobs: " + unfinished); + } + executor = null; + if (DEBUG) Log.d(TAG, "Destroyed."); + } + + @Override + public int onStartCommand(Intent intent, int flags, int serviceId) { // TODO: Ensure we're not being called with retry or redeliver. // checkArgument(flags == 0); // retry and redeliver are not supported. @@ -123,17 +135,17 @@ public class FileOperationService extends Service implements Job.Listener { handleCancel(intent); } else { checkArgument(operationType != OPERATION_UNKNOWN); - handleOperation(intent, startTime, jobId, operationType); + handleOperation(intent, serviceId, jobId, operationType); } return START_NOT_STICKY; } - private void handleOperation(Intent intent, int startTime, String jobId, int operationType) { - if (DEBUG) Log.d(TAG, "onStartCommand: " + jobId + " with start time " + startTime); + private void handleOperation(Intent intent, int serviceId, String jobId, int operationType) { + if (DEBUG) Log.d(TAG, "onStartCommand: " + jobId + " with serviceId " + serviceId); - // Track start time so we can stop the service once we're out of work to do. - mLastStarted = startTime; + // Track the service supplied id so we can stop the service once we're out of work to do. + mLastServiceId = serviceId; Job job = null; synchronized (mRunning) { @@ -147,12 +159,18 @@ public class FileOperationService extends Service implements Job.Listener { job = createJob(operationType, jobId, srcs, stack); + if (job == null) { + return; + } + mWakeLock.acquire(); } checkState(job != null); int delay = intent.getIntExtra(EXTRA_DELAY, DEFAULT_DELAY); checkArgument(delay <= MAX_DELAY); + if (DEBUG) Log.d( + TAG, "Scheduling job " + job.id + " to run in " + delay + " milliseconds."); ScheduledFuture future = executor.schedule(job, delay, TimeUnit.MILLISECONDS); mRunning.put(jobId, new JobRecord(job, future)); } @@ -196,11 +214,19 @@ public class FileOperationService extends Service implements Job.Listener { // TODO: Guarantee the job is being finalized } + /** + * Creates a new job. Returns null if a job with {@code id} already exists. + * @return + */ @GuardedBy("mRunning") - private Job createJob( + private @Nullable Job createJob( @OpType int operationType, String id, List srcs, DocumentStack stack) { - checkArgument(!mRunning.containsKey(id)); + if (mRunning.containsKey(id)) { + Log.w(TAG, "Duplicate job id: " + id + + ". Ignoring job request for srcs: " + srcs + ", stack: " + stack + "."); + return null; + } Job job = null; switch (operationType) { @@ -211,7 +237,8 @@ public class FileOperationService extends Service implements Job.Listener { job = jobFactory.createMove(this, getApplicationContext(), this, id, stack, srcs); break; case OPERATION_DELETE: - throw new UnsupportedOperationException(); + job = jobFactory.createDelete(this, getApplicationContext(), this, id, stack, srcs); + break; default: throw new UnsupportedOperationException(); } @@ -234,20 +261,21 @@ public class FileOperationService extends Service implements Job.Listener { /** * Most likely shuts down. Won't shut down if service has a pending - * message. + * message. Thread pool is deal with in onDestroy. */ private void shutdown() { - if (DEBUG) Log.d(TAG, "Shutting down. Last start time: " + mLastStarted); + if (DEBUG) Log.d(TAG, "Shutting down. Last serviceId was " + mLastServiceId); mWakeLock.release(); mWakeLock = null; - boolean gonnaStop = stopSelfResult(mLastStarted); + + // Turns out, for us, stopSelfResult always returns false in tests, + // so we can't guard executor shutdown. For this reason we move + // executor shutdown to #onDestroy. + boolean gonnaStop = stopSelfResult(mLastServiceId); if (DEBUG) Log.d(TAG, "Stopping service: " + gonnaStop); if (!gonnaStop) { Log.w(TAG, "Service should be stopping, but reports otherwise."); } - // Sadly "gonnaStop" is always false in tests, so we can't guard executor shutdown. - List unfinished = executor.shutdownNow(); - checkState(unfinished.isEmpty()); } @VisibleForTesting diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/FileOperations.java b/packages/DocumentsUI/src/com/android/documentsui/services/FileOperations.java index 0f1730a3e8bbe..f59a32ad60ea5 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/services/FileOperations.java +++ b/packages/DocumentsUI/src/com/android/documentsui/services/FileOperations.java @@ -22,6 +22,7 @@ import static com.android.documentsui.Shared.EXTRA_STACK; import static com.android.documentsui.Shared.asArrayList; import static com.android.documentsui.Shared.getQuantityString; import static com.android.documentsui.services.FileOperationService.EXTRA_CANCEL; +import static com.android.documentsui.services.FileOperationService.EXTRA_DELAY; import static com.android.documentsui.services.FileOperationService.EXTRA_JOB_ID; import static com.android.documentsui.services.FileOperationService.EXTRA_OPERATION; import static com.android.documentsui.services.FileOperationService.EXTRA_SRC_LIST; @@ -52,10 +53,12 @@ public final class FileOperations { private static final String TAG = "FileOperations"; + private static final IdBuilder idBuilder = new IdBuilder(); + private FileOperations() {} public static String createJobId() { - return String.valueOf(elapsedRealtime()); + return idBuilder.getNext(); } /** @@ -73,7 +76,7 @@ public final class FileOperations { case OPERATION_MOVE: return FileOperations.move(activity, srcDocs, stack); case OPERATION_DELETE: - return FileOperations.delete(activity, srcDocs, stack); + throw new UnsupportedOperationException("Delete isn't currently supported."); default: throw new UnsupportedOperationException("Unknown operation: " + operationType); } @@ -151,14 +154,17 @@ public final class FileOperations { * @param jobId A unique jobid for this job. * Use {@link #createJobId} if you don't have one handy. * @param srcDocs A list of src files to copy. + * @param delay Number of milliseconds to wait before executing the job. * @return Id of the job. */ public static String delete( - Activity activity, List srcDocs, DocumentStack location) { + Activity activity, List srcDocs, DocumentStack location, int delay) { String jobId = createJobId(); - if (DEBUG) Log.d(TAG, "Initiating 'delete' operation id: " + jobId); + if (DEBUG) Log.d(TAG, "Initiating 'delete' operation id " + jobId + + " delayed by " + delay + " milliseconds."); Intent intent = createBaseIntent(OPERATION_DELETE, activity, jobId, srcDocs, location); + intent.putExtra(EXTRA_DELAY, delay); activity.startService(intent); return jobId; @@ -193,4 +199,24 @@ public final class FileOperations { getQuantityString(activity, contentId, fileCount), Snackbar.LENGTH_SHORT); } + + private static final class IdBuilder { + + // Remember last job time so we can guard against collisions. + private long mLastJobTime; + + // If we detect a collision, use subId to make distinct. + private int mSubId; + + public synchronized String getNext() { + long time = elapsedRealtime(); + if (time == mLastJobTime) { + mSubId++; + } else { + mSubId = 0; + } + mLastJobTime = time; + return String.valueOf(mLastJobTime) + "-" + String.valueOf(mSubId); + } + } } diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/Job.java b/packages/DocumentsUI/src/com/android/documentsui/services/Job.java index c7939ebd8b185..f351df94a0d8c 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/services/Job.java +++ b/packages/DocumentsUI/src/com/android/documentsui/services/Job.java @@ -16,6 +16,7 @@ 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_FAILURE; import static com.android.documentsui.services.FileOperationService.EXTRA_JOB_ID; @@ -24,18 +25,21 @@ import static com.android.documentsui.services.FileOperationService.EXTRA_SRC_LI import static com.android.documentsui.services.FileOperationService.FAILURE_COPY; import static com.android.documentsui.services.FileOperationService.OPERATION_UNKNOWN; import static com.android.internal.util.Preconditions.checkArgument; +import static com.android.internal.util.Preconditions.checkNotNull; import android.annotation.DrawableRes; import android.annotation.PluralsRes; 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.os.Parcelable; import android.os.RemoteException; import android.provider.DocumentsContract; +import android.util.Log; import com.android.documentsui.FilesActivity; import com.android.documentsui.R; @@ -45,7 +49,9 @@ import com.android.documentsui.model.DocumentStack; import com.android.documentsui.services.FileOperationService.OpType; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; /** * A mashup of work item and ui progress update factory. Used by {@link FileOperationService} @@ -53,6 +59,7 @@ import java.util.List; */ abstract class Job implements Runnable { + private static final String TAG = "Job"; final Context service; final Context appContext; final Listener listener; @@ -64,6 +71,7 @@ abstract class Job implements Runnable { final ArrayList failedFiles = new ArrayList<>(); final Notification.Builder mProgressBuilder; + private final Map mClients = new HashMap<>(); private volatile boolean mCanceled; /** @@ -118,14 +126,31 @@ abstract class Job implements Runnable { abstract void start() throws RemoteException; - // Service will call this when it is done with the job. - abstract void cleanup(); - abstract Notification getSetupNotification(); // TODO: Progress notification for deletes. // abstract Notification getProgressNotification(long bytesCopied); abstract Notification getFailureNotification(); + ContentProviderClient getClient(DocumentInfo doc) throws RemoteException { + ContentProviderClient client = mClients.get(doc.authority); + if (client == null) { + // Acquire content providers. + client = acquireUnstableProviderOrThrow( + getContentResolver(), + doc.authority); + + mClients.put(doc.authority, client); + } + + return checkNotNull(client); + } + + final void cleanup() { + for (ContentProviderClient client : mClients.values()) { + ContentProviderClient.releaseQuietly(client); + } + } + final void cancel() { mCanceled = true; } @@ -146,6 +171,17 @@ abstract class Job implements Runnable { return !failedFiles.isEmpty(); } + final boolean deleteDocument(DocumentInfo doc) { + try { + DocumentsContract.deleteDocument(getClient(doc), doc.derivedUri); + } catch (RemoteException e) { + Log.w(TAG, "Failed to delete file: " + doc.derivedUri, e); + return false; + } + + return true; // victory dance! + } + Notification getSetupNotification(String content) { mProgressBuilder.setProgress(0, 0, true); mProgressBuilder.setContentText(content); @@ -215,6 +251,16 @@ abstract class Job implements Runnable { return cancelIntent; } + @Override + public String toString() { + return new StringBuilder() + .append("Job") + .append("{") + .append("id=" + id) + .append("}") + .toString(); + } + /** * Factory class that facilitates our testing FileOperationService. */ @@ -231,6 +277,11 @@ abstract class Job implements Runnable { String id, DocumentStack stack, List srcs) { return new MoveJob(service, appContext, listener, id, stack, srcs); } + + Job createDelete(Context service, Context appContext, Listener listener, + String id, DocumentStack stack, List srcs) { + return new DeleteJob(service, appContext, listener, id, stack, srcs); + } } /** diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/MoveJob.java b/packages/DocumentsUI/src/com/android/documentsui/services/MoveJob.java index f46f23478ef29..7f6b1e1d3abd6 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/services/MoveJob.java +++ b/packages/DocumentsUI/src/com/android/documentsui/services/MoveJob.java @@ -24,7 +24,6 @@ import android.content.Context; import android.os.RemoteException; import android.provider.DocumentsContract; import android.provider.DocumentsContract.Document; -import android.util.Log; import com.android.documentsui.R; import com.android.documentsui.model.DocumentInfo; @@ -76,17 +75,17 @@ final class MoveJob extends CopyJob { } @Override - boolean processDocument(DocumentInfo srcInfo, DocumentInfo dstDirInfo) throws RemoteException { + boolean processDocument(DocumentInfo src, DocumentInfo dest) throws RemoteException { // TODO: When optimized move kicks in, we're not making any progress updates. FIX IT! // When moving within the same provider, try to use optimized moving. // If not supported, then fallback to byte-by-byte copy/move. - if (srcInfo.authority.equals(dstDirInfo.authority)) { - if ((srcInfo.flags & Document.FLAG_SUPPORTS_MOVE) != 0) { - if (DocumentsContract.moveDocument(srcClient, srcInfo.derivedUri, - dstDirInfo.derivedUri) == null) { - onFileFailed(srcInfo); + if (src.authority.equals(dest.authority)) { + if ((src.flags & Document.FLAG_SUPPORTS_MOVE) != 0) { + if (DocumentsContract.moveDocument(getClient(src), src.derivedUri, + dest.derivedUri) == null) { + onFileFailed(src); return false; } return true; @@ -94,22 +93,20 @@ final class MoveJob extends CopyJob { } // If we couldn't do an optimized copy...we fall back to vanilla byte copy. - boolean copied = byteCopyDocument(srcInfo, dstDirInfo); + boolean copied = byteCopyDocument(src, dest); - return copied && !isCanceled() && deleteSrcDocument(srcInfo); + return copied && !isCanceled() && deleteDocument(src); } - private boolean deleteSrcDocument(DocumentInfo srcInfo) { - // This is racey. We should make sure that we never delete a directory after - // it changed, so we don't remove a file which had not been copied earlier - // to the target location. - try { - DocumentsContract.deleteDocument(srcClient, srcInfo.derivedUri); - } catch (RemoteException e) { - Log.w(TAG, "Failed to delete source after copy: " + srcInfo.derivedUri, e); - return false; - } - - return true; // victory dance! + @Override + public String toString() { + return new StringBuilder() + .append("MoveJob") + .append("{") + .append("id=" + id) + .append("srcs=" + mSrcs) + .append(", destination=" + stack) + .append("}") + .toString(); } } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelBackedDocumentsAdapterTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelBackedDocumentsAdapterTest.java index 5ce1823d6a21e..2244be9fa8cc5 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelBackedDocumentsAdapterTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelBackedDocumentsAdapterTest.java @@ -51,7 +51,7 @@ public class ModelBackedDocumentsAdapterTest extends AndroidTestCase { public void setUp() { final Context testContext = TestContext.createStorageTestContext(getContext(), AUTHORITY); - mModel = new TestModel(testContext, AUTHORITY); + mModel = new TestModel(AUTHORITY); mModel.update(NAMES); DocumentsAdapter.Environment env = new TestEnvironment(testContext); diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelTest.java index a5f065638c96d..83299f0677cfe 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelTest.java @@ -38,7 +38,6 @@ import java.util.HashSet; import java.util.List; import java.util.Random; import java.util.Set; -import java.util.concurrent.CountDownLatch; @SmallTest public class ModelTest extends AndroidTestCase { @@ -96,7 +95,7 @@ public class ModelTest extends AndroidTestCase { r.cursor = cursor; // Instantiate the model with a dummy view adapter and listener that (for now) do nothing. - model = new Model(context); + model = new Model(); model.addUpdateListener(new DummyListener()); model.update(r); } @@ -303,16 +302,6 @@ public class ModelTest extends AndroidTestCase { } } - // Tests that Model.delete works correctly. - public void testDelete() throws Exception { - // Simulate deleting 2 files. - List docsBefore = getDocumentInfo(2, 3); - delete(2, 3); - - provider.assertWasDeleted(docsBefore.get(0)); - provider.assertWasDeleted(docsBefore.get(1)); - } - private void setupTestContext() { final MockContentResolver resolver = new MockContentResolver(); context = new ContextWrapper(getContext()) { @@ -335,29 +324,6 @@ public class ModelTest extends AndroidTestCase { return s; } - private void delete(int... positions) throws InterruptedException { - Selection s = positionToSelection(positions); - final CountDownLatch latch = new CountDownLatch(1); - - model.delete( - s, - new Model.DeletionListener() { - @Override - public void onError() { - latch.countDown(); - } - @Override - void onCompletion() { - latch.countDown(); - } - }); - latch.await(); - } - - private List getDocumentInfo(int... positions) { - return model.getDocuments(positionToSelection(positions)); - } - private static class DummyListener implements Model.UpdateListener { public void onModelUpdate(Model model) {} public void onModelUpdateFailed(Exception e) {} diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/SectionBreakDocumentsAdapterWrapperTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/SectionBreakDocumentsAdapterWrapperTest.java index 398885cbdb848..7c324e77242c0 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/SectionBreakDocumentsAdapterWrapperTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/SectionBreakDocumentsAdapterWrapperTest.java @@ -23,15 +23,12 @@ import android.provider.DocumentsContract.Document; import android.support.v7.widget.RecyclerView; import android.test.AndroidTestCase; import android.test.suitebuilder.annotation.SmallTest; -import android.util.SparseArray; import android.view.ViewGroup; import com.android.documentsui.DirectoryResult; import com.android.documentsui.RootCursorWrapper; import com.android.documentsui.State; -import java.util.List; - @SmallTest public class SectionBreakDocumentsAdapterWrapperTest extends AndroidTestCase { @@ -57,7 +54,7 @@ public class SectionBreakDocumentsAdapterWrapperTest extends AndroidTestCase { final Context testContext = TestContext.createStorageTestContext(getContext(), AUTHORITY); DocumentsAdapter.Environment env = new TestEnvironment(testContext); - mModel = new TestModel(testContext, AUTHORITY); + mModel = new TestModel(AUTHORITY); mAdapter = new SectionBreakDocumentsAdapterWrapper( env, new ModelBackedDocumentsAdapter( diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestModel.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestModel.java index f9cd3b2c0c251..d8c29db81a77f 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestModel.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestModel.java @@ -16,16 +16,13 @@ package com.android.documentsui.dirlist; -import android.content.Context; import android.database.MatrixCursor; import android.provider.DocumentsContract.Document; import com.android.documentsui.DirectoryResult; import com.android.documentsui.RootCursorWrapper; -import com.android.documentsui.dirlist.MultiSelectManager.Selection; import java.util.Random; -import java.util.Set; public class TestModel extends Model { @@ -39,14 +36,9 @@ public class TestModel extends Model { }; private final String mAuthority; - private Set mDeleted; - /** - * Creates a new context. context must be configured with provider for authority. - * @see TestContext#createStorageTestContext(Context, String). - */ - public TestModel(Context context, String authority) { - super(context); + public TestModel(String authority) { + super(); mAuthority = authority; } @@ -75,12 +67,4 @@ public class TestModel extends Model { String idForPosition(int p) { return createModelId(mAuthority, Integer.toString(p)); } - - @Override - public void delete(Selection selected, DeletionListener listener) { - for (String id : selected.getAll()) { - mDeleted.add(id); - } - listener.onCompletion(); - } } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/BaseCopyJobTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/AbstractCopyJobTest.java similarity index 71% rename from packages/DocumentsUI/tests/src/com/android/documentsui/services/BaseCopyJobTest.java rename to packages/DocumentsUI/tests/src/com/android/documentsui/services/AbstractCopyJobTest.java index f57ce53291683..ec21150412f5c 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/services/BaseCopyJobTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/AbstractCopyJobTest.java @@ -16,72 +16,18 @@ package com.android.documentsui.services; -import static com.android.documentsui.StubProvider.ROOT_0_ID; -import static com.android.documentsui.StubProvider.ROOT_1_ID; import static com.google.common.collect.Lists.newArrayList; -import android.content.ContentProviderClient; -import android.content.ContentResolver; -import android.content.Context; import android.net.Uri; -import android.os.RemoteException; import android.provider.DocumentsContract; -import android.test.AndroidTestCase; import android.test.suitebuilder.annotation.MediumTest; -import com.android.documentsui.DocumentsProviderHelper; -import com.android.documentsui.StubProvider; import com.android.documentsui.model.DocumentInfo; -import com.android.documentsui.model.RootInfo; import java.util.List; @MediumTest -public abstract class BaseCopyJobTest extends AndroidTestCase { - - static String AUTHORITY = StubProvider.DEFAULT_AUTHORITY; - static final byte[] HAM_BYTES = "ham and cheese".getBytes(); - static final byte[] FRUITY_BYTES = "I love fruit cakes!".getBytes(); - - Context mContext; - ContentResolver mResolver; - ContentProviderClient mClient; - DocumentsProviderHelper mDocs; - TestJobListener mJobListener; - RootInfo mSrcRoot; - RootInfo mDestRoot; - - @Override - protected void setUp() throws Exception { - super.setUp(); - - mJobListener = new TestJobListener(); - - // NOTE: Must be the "target" context, else security checks in content provider will fail. - mContext = getContext(); - mResolver = mContext.getContentResolver(); - - mClient = mResolver.acquireContentProviderClient(AUTHORITY); - mDocs = new DocumentsProviderHelper(AUTHORITY, mClient); - - initTestFiles(); - } - - @Override - protected void tearDown() throws Exception { - resetStorage(); - mClient.release(); - super.tearDown(); - } - - private void resetStorage() throws RemoteException { - mClient.call("clear", null, null); - } - - private void initTestFiles() throws RemoteException { - mSrcRoot = mDocs.getRoot(ROOT_0_ID); - mDestRoot = mDocs.getRoot(ROOT_1_ID); - } +public abstract class AbstractCopyJobTest extends AbstractJobTest { public void runCopyFilesTest() throws Exception { Uri testFile1 = mDocs.createDocument(mSrcRoot, "text/plain", "test1.txt"); @@ -174,9 +120,9 @@ public abstract class BaseCopyJobTest extends AndroidTestCase { public void runNoCopyDirToDescendentTest() throws Exception { Uri testDir = mDocs.createFolder(mSrcRoot, "someDir"); - Uri descDir = mDocs.createFolder(testDir, "theDescendent"); + Uri destDir = mDocs.createFolder(testDir, "theDescendent"); - createJob(newArrayList(testDir), descDir).run(); + createJob(newArrayList(testDir), destDir).run(); mJobListener.waitForFinished(); mJobListener.assertFailed(); @@ -201,10 +147,11 @@ public abstract class BaseCopyJobTest extends AndroidTestCase { mDocs.assertChildCount(mDestRoot, 0); } - final CopyJob createJob(List srcs) throws Exception { + /** + * Creates a job with a stack consisting to the default destination. + */ + final T createJob(List srcs) throws Exception { Uri destination = DocumentsContract.buildDocumentUri(AUTHORITY, mDestRoot.documentId); return createJob(srcs, destination); } - - abstract CopyJob createJob(List srcs, Uri destination) throws Exception; } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/AbstractJobTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/AbstractJobTest.java new file mode 100644 index 0000000000000..691af6ac03e81 --- /dev/null +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/AbstractJobTest.java @@ -0,0 +1,101 @@ +/* + * 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.StubProvider.ROOT_0_ID; +import static com.android.documentsui.StubProvider.ROOT_1_ID; + +import android.content.ContentProviderClient; +import android.content.ContentResolver; +import android.content.Context; +import android.net.Uri; +import android.os.RemoteException; +import android.provider.DocumentsContract; +import android.test.AndroidTestCase; +import android.test.suitebuilder.annotation.MediumTest; + +import com.android.documentsui.DocumentsProviderHelper; +import com.android.documentsui.StubProvider; +import com.android.documentsui.model.DocumentInfo; +import com.android.documentsui.model.DocumentStack; +import com.android.documentsui.model.RootInfo; + +import com.google.common.collect.Lists; + +import java.util.List; + +@MediumTest +public abstract class AbstractJobTest extends AndroidTestCase { + + static String AUTHORITY = StubProvider.DEFAULT_AUTHORITY; + static final byte[] HAM_BYTES = "ham and cheese".getBytes(); + static final byte[] FRUITY_BYTES = "I love fruit cakes!".getBytes(); + + Context mContext; + ContentResolver mResolver; + ContentProviderClient mClient; + DocumentsProviderHelper mDocs; + TestJobListener mJobListener; + RootInfo mSrcRoot; + RootInfo mDestRoot; + + @Override + protected void setUp() throws Exception { + super.setUp(); + + mJobListener = new TestJobListener(); + + // NOTE: Must be the "target" context, else security checks in content provider will fail. + mContext = getContext(); + mResolver = mContext.getContentResolver(); + + mClient = mResolver.acquireContentProviderClient(AUTHORITY); + mDocs = new DocumentsProviderHelper(AUTHORITY, mClient); + + initTestFiles(); + } + + @Override + protected void tearDown() throws Exception { + resetStorage(); + mClient.release(); + super.tearDown(); + } + + private void resetStorage() throws RemoteException { + mClient.call("clear", null, null); + } + + private void initTestFiles() throws RemoteException { + mSrcRoot = mDocs.getRoot(ROOT_0_ID); + mDestRoot = mDocs.getRoot(ROOT_1_ID); + } + + final T createJob(List srcs, Uri destination) throws Exception { + DocumentStack stack = new DocumentStack(); + stack.push(DocumentInfo.fromUri(mResolver, destination)); + + List srcDocs = Lists.newArrayList(); + for (Uri src : srcs) { + srcDocs.add(DocumentInfo.fromUri(mResolver, src)); + } + + return createJob(srcDocs, stack); + } + + abstract T createJob(List srcs, DocumentStack destination) throws Exception; +} diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/CopyJobTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/CopyJobTest.java index c0ce993f8d98e..1acf2dc5f4cfa 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/services/CopyJobTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/CopyJobTest.java @@ -16,18 +16,15 @@ package com.android.documentsui.services; -import android.net.Uri; import android.test.suitebuilder.annotation.MediumTest; import com.android.documentsui.model.DocumentInfo; import com.android.documentsui.model.DocumentStack; -import com.google.common.collect.Lists; - import java.util.List; @MediumTest -public class CopyJobTest extends BaseCopyJobTest { +public class CopyJobTest extends AbstractCopyJobTest { public void testCopyFiles() throws Exception { runCopyFilesTest(); @@ -62,16 +59,8 @@ public class CopyJobTest extends BaseCopyJobTest { } @Override - CopyJob createJob(List srcs, Uri destination) throws Exception { - DocumentStack stack = new DocumentStack(); - stack.push(DocumentInfo.fromUri(mResolver, destination)); - - List srcDocs = Lists.newArrayList(); - for (Uri src : srcs) { - srcDocs.add(DocumentInfo.fromUri(mResolver, src)); - } - + CopyJob createJob(List srcs, DocumentStack stack) throws Exception { return new CopyJob( - mContext, mContext, mJobListener, FileOperations.createJobId(), stack, srcDocs); + mContext, mContext, mJobListener, FileOperations.createJobId(), stack, srcs); } } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/DeleteJobTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/DeleteJobTest.java new file mode 100644 index 0000000000000..d6d1023972805 --- /dev/null +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/DeleteJobTest.java @@ -0,0 +1,59 @@ +/* + * 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.google.common.collect.Lists.newArrayList; + +import android.net.Uri; +import android.provider.DocumentsContract; +import android.test.suitebuilder.annotation.MediumTest; + +import com.android.documentsui.model.DocumentInfo; +import com.android.documentsui.model.DocumentStack; + +import java.util.List; + +@MediumTest +public class DeleteJobTest extends AbstractJobTest { + + public void testDeleteFiles() throws Exception { + Uri testFile1 = mDocs.createDocument(mSrcRoot, "text/plain", "test1.txt"); + mDocs.writeDocument(testFile1, HAM_BYTES); + + Uri testFile2 = mDocs.createDocument(mSrcRoot, "text/plain", "test2.txt"); + mDocs.writeDocument(testFile2, FRUITY_BYTES); + + createJob(newArrayList(testFile1, testFile2)).run(); + mJobListener.waitForFinished(); + + mDocs.assertChildCount(mSrcRoot, 0); + } + + /** + * Creates a job with a stack consisting to the default src directory. + */ + private final DeleteJob createJob(List srcs) throws Exception { + Uri stack = DocumentsContract.buildDocumentUri(AUTHORITY, mSrcRoot.documentId); + return createJob(srcs, stack); + } + + @Override + DeleteJob createJob(List srcs, DocumentStack stack) throws Exception { + return new DeleteJob( + mContext, mContext, mJobListener, FileOperations.createJobId(), stack, srcs); + } +} diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/FileOperationServiceTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/FileOperationServiceTest.java index d55b6f0985481..4d5392ee9432a 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/services/FileOperationServiceTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/FileOperationServiceTest.java @@ -114,8 +114,11 @@ public class FileOperationServiceTest extends ServiceTestCase { public void testMoveFiles() throws Exception { runCopyFilesTest(); @@ -82,16 +79,8 @@ public class MoveJobTest extends BaseCopyJobTest { } @Override - CopyJob createJob(List srcs, Uri destination) throws Exception { - DocumentStack stack = new DocumentStack(); - stack.push(DocumentInfo.fromUri(mResolver, destination)); - - List srcDocs = Lists.newArrayList(); - for (Uri src : srcs) { - srcDocs.add(DocumentInfo.fromUri(mResolver, src)); - } - + MoveJob createJob(List srcs, DocumentStack stack) throws Exception { return new MoveJob( - mContext, mContext, mJobListener, FileOperations.createJobId(), stack, srcDocs); + mContext, mContext, mJobListener, FileOperations.createJobId(), stack, srcs); } } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/TestJob.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/TestJob.java index 72da9a1b9e25a..9c58780239ab7 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/services/TestJob.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/TestJob.java @@ -46,9 +46,6 @@ public class TestJob extends Job { assertTrue(mStarted); } - @Override - void cleanup() {} - void fail(DocumentInfo doc) { onFileFailed(doc); } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/TestScheduledExecutorService.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/TestScheduledExecutorService.java index 5c39b78cb60b2..4d417cfc095c0 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/services/TestScheduledExecutorService.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/TestScheduledExecutorService.java @@ -146,7 +146,7 @@ public class TestScheduledExecutorService implements ScheduledExecutorService { scheduled.get(taskIndex).runnable.run(); } - public void isAlive() { + public void assertAlive() { assertFalse(isShutdown()); }