From 1008a11233c9edb6544a1e4d65e291bb16066fa3 Mon Sep 17 00:00:00 2001 From: Tomasz Mikolajewski Date: Wed, 2 Mar 2016 17:41:40 +0900 Subject: [PATCH] Fallback to byte-by-byte copy/move if optimized fails. Sometimes providers may refuse to do a provider-side copy/move even though the documents had the SUPPORTS_COPY/MOVE flags set. This is because optimized copy/move may be only supported within the same root. Since there is no easy way to check if documents are on the same roots from DocumentsUI, as well as whether the provider can actually copy/move efficiently between different roots, we should tolerate the failure and do a fallback to byte-by-byte operation. Bug: 27436368 Change-Id: Ia1a0fbdba26c06b2151afc25d8513c01d60d31db --- .../android/documentsui/services/CopyJob.java | 15 ++++++--------- .../android/documentsui/services/MoveJob.java | 19 +++++++++++-------- .../documentsui/DocumentsProviderHelper.java | 4 +++- .../com/android/documentsui/StubProvider.java | 16 +++++++++------- .../documentsui/services/CopyJobTest.java | 19 +++++++++++++++++++ .../documentsui/services/MoveJobTest.java | 15 +++++++++++++++ 6 files changed, 63 insertions(+), 25 deletions(-) diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java b/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java index ad48a70c40754..9ed2abf9c9b8e 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java +++ b/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java @@ -299,18 +299,15 @@ class CopyJob extends Job { if ((src.flags & Document.FLAG_SUPPORTS_COPY) != 0) { try { if (DocumentsContract.copyDocument(getClient(src), src.derivedUri, - dstDirInfo.derivedUri) == null) { - throw new ResourceException("Provider side copy failed for document %s.", - src.derivedUri); + dstDirInfo.derivedUri) != null) { + return; } - } catch (ResourceException e) { - throw e; } catch (RemoteException | RuntimeException e) { - throw new ResourceException( - "Provider side copy failed for document %s due to an exception.", - src.derivedUri, e); + Log.e(TAG, "Provider side copy failed for: " + src.derivedUri + + " due to an exception: " + e); } - return; + // If optimized copy fails, then fallback to byte-by-byte copy. + if (DEBUG) Log.d(TAG, "Fallback to byte-by-byte copy for: " + src.derivedUri); } } diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/MoveJob.java b/packages/DocumentsUI/src/com/android/documentsui/services/MoveJob.java index dc39235b35c51..ceae50ca7b0b0 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/services/MoveJob.java +++ b/packages/DocumentsUI/src/com/android/documentsui/services/MoveJob.java @@ -16,6 +16,7 @@ package com.android.documentsui.services; +import static com.android.documentsui.Shared.DEBUG; import static com.android.documentsui.services.FileOperationService.OPERATION_MOVE; import android.app.Notification; @@ -24,6 +25,7 @@ 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; @@ -34,6 +36,8 @@ import java.util.List; // TODO: Stop extending CopyJob. final class MoveJob extends CopyJob { + private static final String TAG = "MoveJob"; + final DocumentInfo mSrcParent; /** @@ -89,16 +93,15 @@ final class MoveJob extends CopyJob { try { if (DocumentsContract.moveDocument(getClient(src), src.derivedUri, srcParent != null ? srcParent.derivedUri : mSrcParent.derivedUri, - dest.derivedUri) == null) { - throw new ResourceException("Provider side move failed for document %s.", - src.derivedUri); + dest.derivedUri) != null) { + return; } - } catch (RuntimeException | RemoteException e) { - throw new ResourceException( - "Provider side move failed for document %s due to an exception.", - src.derivedUri, e); + } catch (RemoteException | RuntimeException e) { + Log.e(TAG, "Provider side move failed for: " + src.derivedUri + + " due to an exception: " + e); } - return; + // If optimized move fails, then fallback to byte-by-byte copy. + if (DEBUG) Log.d(TAG, "Fallback to byte-by-byte move for: " + src.derivedUri); } } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/DocumentsProviderHelper.java b/packages/DocumentsUI/tests/src/com/android/documentsui/DocumentsProviderHelper.java index af478eafc38e5..16ed2d9dee3ca 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/DocumentsProviderHelper.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/DocumentsProviderHelper.java @@ -106,13 +106,15 @@ public class DocumentsProviderHelper { return createDocument(root.documentId, mimeType, name); } - public Uri createDocumentWithFlags(String documentId, String mimeType, String name, int flags) + public Uri createDocumentWithFlags(String documentId, String mimeType, String name, int flags, + String... streamTypes) throws RemoteException { Bundle in = new Bundle(); in.putInt(StubProvider.EXTRA_FLAGS, flags); in.putString(StubProvider.EXTRA_PARENT_ID, documentId); in.putString(Document.COLUMN_MIME_TYPE, mimeType); in.putString(Document.COLUMN_DISPLAY_NAME, name); + in.putStringArrayList(StubProvider.EXTRA_STREAM_TYPES, Lists.newArrayList(streamTypes)); Bundle out = mClient.call("createDocumentWithFlags", null, in); Uri uri = out.getParcelable(DocumentsContract.EXTRA_URI); diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/StubProvider.java b/packages/DocumentsUI/tests/src/com/android/documentsui/StubProvider.java index 25276505ee431..f71ce5d1b9300 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/StubProvider.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/StubProvider.java @@ -518,28 +518,29 @@ public class StubProvider extends DocumentsProvider { String rootId = extras.getString(EXTRA_PARENT_ID); String mimeType = extras.getString(Document.COLUMN_MIME_TYPE); String name = extras.getString(Document.COLUMN_DISPLAY_NAME); + List streamTypes = extras.getStringArrayList(EXTRA_STREAM_TYPES); int flags = extras.getInt(EXTRA_FLAGS); Bundle out = new Bundle(); String documentId = null; try { - documentId = createDocument(rootId, mimeType, name, flags); + documentId = createDocument(rootId, mimeType, name, flags, streamTypes); Uri uri = DocumentsContract.buildDocumentUri(mAuthority, documentId); out.putParcelable(DocumentsContract.EXTRA_URI, uri); } catch (FileNotFoundException e) { - Log.d(TAG, "Cretaing document with flags failed" + name); + Log.d(TAG, "Creating document with flags failed" + name); } return out; } - public String createDocument(String parentId, String mimeType, String displayName, int flags) - throws FileNotFoundException { + public String createDocument(String parentId, String mimeType, String displayName, int flags, + List streamTypes) throws FileNotFoundException { StubDocument parent = mStorage.get(parentId); File file = createFile(parent, mimeType, displayName); final StubDocument document = StubDocument.createDocumentWithFlags(file, mimeType, parent, - flags); + flags, streamTypes); mStorage.put(document.documentId, document); Log.d(TAG, "Created document " + document.documentId); notifyParentChanged(document.parentId); @@ -787,8 +788,9 @@ public class StubProvider extends DocumentsProvider { } public static StubDocument createDocumentWithFlags( - File file, String mimeType, StubDocument parent, int flags) { - return new StubDocument(file, mimeType, new ArrayList(), flags, parent); + File file, String mimeType, StubDocument parent, int flags, + List streamTypes) { + return new StubDocument(file, mimeType, streamTypes, flags, parent); } public static StubDocument createVirtualDocument( 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 543396eec7b56..bb7c01afb65e7 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/services/CopyJobTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/CopyJobTest.java @@ -16,6 +16,10 @@ package com.android.documentsui.services; +import static com.google.common.collect.Lists.newArrayList; + +import android.net.Uri; +import android.provider.DocumentsContract.Document; import android.test.suitebuilder.annotation.MediumTest; import com.android.documentsui.model.DocumentInfo; @@ -38,6 +42,21 @@ public class CopyJobTest extends AbstractCopyJobTest { runCopyVirtualNonTypedFileTest(); } + public void testCopy_BackendSideVirtualTypedFile_Fallback() throws Exception { + mDocs.assertChildCount(mDestRoot, 0); + + Uri testFile = mDocs.createDocumentWithFlags( + mSrcRoot.documentId, "virtual/mime-type", "tokyo.sth", + Document.FLAG_VIRTUAL_DOCUMENT | Document.FLAG_SUPPORTS_COPY + | Document.FLAG_SUPPORTS_MOVE, "application/pdf"); + + createJob(newArrayList(testFile)).run(); + + mJobListener.waitForFinished(); + mDocs.assertChildCount(mDestRoot, 1); + mDocs.assertHasFile(mDestRoot, "tokyo.sth.pdf"); // Copy should convert file to PDF. + } + public void testCopyEmptyDir() throws Exception { runCopyEmptyDirTest(); } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/MoveJobTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/MoveJobTest.java index 749264a9ada80..24181d62e208e 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/services/MoveJobTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/MoveJobTest.java @@ -59,6 +59,21 @@ public class MoveJobTest extends AbstractCopyJobTest { mDocs.assertChildCount(mSrcRoot, 1); } + public void testMove_BackendSideVirtualTypedFile_Fallback() throws Exception { + Uri testFile = mDocs.createDocumentWithFlags( + mSrcRoot.documentId, "virtual/mime-type", "tokyo.sth", + Document.FLAG_VIRTUAL_DOCUMENT | Document.FLAG_SUPPORTS_COPY + | Document.FLAG_SUPPORTS_MOVE, "application/pdf"); + + createJob(newArrayList(testFile)).run(); + mJobListener.waitForFinished(); + + // Should have failed, source not deleted. Moving by bytes for virtual files + // is not supported. + mDocs.assertChildCount(mDestRoot, 0); + mDocs.assertChildCount(mSrcRoot, 1); + } + public void testMoveEmptyDir() throws Exception { runCopyEmptyDirTest();