Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f712a206 authored by Steve McKay's avatar Steve McKay
Browse files

Don't copy a directory into itself...doesn't go well. 

Minimally deform CopyService such that we can listen
    to the completion of operations in the test.
Add test coverage.
Add equals and hashcode to DocumentInfo...so we can compare
    the heck out of 'em. + a test.
WIP: Expose (@hide style) DocumentsProvider.isChildDocument via
    DocumentsContract. Use that to check for recusive copies.

Bug: 25794511

Change-Id: I05bb329eb10b43540c6806d634e5b96a753e8178
parent 8d45f5a6

src/com/android/documentsui/CopyService.java

+70 −11
Original line number Diff line number Diff line
@@ -39,13 +39,15 @@ import android.os.RemoteException; 
import android.os.SystemClock;

import android.provider.DocumentsContract;

import android.provider.DocumentsContract.Document;

import android.support.annotation.Nullable;

import android.support.annotation.VisibleForTesting;

import android.support.design.widget.Snackbar;

import android.text.format.DateUtils;

import android.util.Log;

import android.widget.Toast;



import com.android.documentsui.model.DocumentInfo;

import com.android.documentsui.model.DocumentStack;

import com.android.documentsui.model.RootInfo;



import libcore.io.IoUtils;
@@ -72,6 +74,12 @@ public class CopyService extends IntentService { 
    // TODO: Move it to a shared file when more operations are implemented.

    public static final int FAILURE_COPY = 1;



    // Parameters of the copy job. Requests to an IntentService are serialized so this code only

    // needs to deal with one job at a time.

    // NOTE: This must be declared by concrete type as the concrete type

    // is required by putParcelableArrayListExtra.

    private final ArrayList<DocumentInfo> mFailedFiles = new ArrayList<>();



    private PowerManager mPowerManager;



    private NotificationManager mNotificationManager;
@@ -80,9 +88,6 @@ public class CopyService extends IntentService { 
    // Jobs are serialized but a job ID is used, to avoid mixing up cancellation requests.

    private String mJobId;

    private volatile boolean mIsCancelled;

    // Parameters of the copy job. Requests to an IntentService are serialized so this code only

    // needs to deal with one job at a time.

    private final ArrayList<DocumentInfo> mFailedFiles;

    private long mBatchSize;

    private long mBytesCopied;

    private long mStartTime;
@@ -97,10 +102,11 @@ public class CopyService extends IntentService { 
    private ContentProviderClient mSrcClient;

    private ContentProviderClient mDstClient;



    // For testing only.

    @Nullable private TestOnlyListener mJobFinishedListener;



    public CopyService() {

        super("CopyService");



        mFailedFiles = new ArrayList<DocumentInfo>();

    }



    /**
@@ -115,7 +121,11 @@ public class CopyService extends IntentService { 
        final Resources res = activity.getResources();

        final Intent copyIntent = new Intent(activity, CopyService.class);

        copyIntent.putParcelableArrayListExtra(

                EXTRA_SRC_LIST, new ArrayList<DocumentInfo>(srcDocs));

                EXTRA_SRC_LIST,

                // Don't create a copy unless absolutely necessary :)

                srcDocs instanceof ArrayList

                    ? (ArrayList<DocumentInfo>) srcDocs

                    : new ArrayList<DocumentInfo>(srcDocs));

        copyIntent.putExtra(Shared.EXTRA_STACK, (Parcelable) dstStack);

        copyIntent.putExtra(EXTRA_TRANSFER_MODE, mode);
@@ -198,6 +208,11 @@ public class CopyService extends IntentService { 
                        .setAutoCancel(true);

                mNotificationManager.notify(mJobId, 0, errorBuilder.build());

            }



            if (mJobFinishedListener != null) {

                mJobFinishedListener.onFinished(mFailedFiles);

            }



            if (DEBUG) Log.d(TAG, "Done cleaning up");

        }

    }
@@ -268,6 +283,26 @@ public class CopyService extends IntentService { 
        // - check MIME types?

    }



    /**

     * Sets a callback to be run when the next run job is finished.

     * This is test ONLY instrumentation. The alternative is for us to add

     * broadcast intents SOLELY for the purpose of testing.

     * @param listener

     */

    @VisibleForTesting

    void addFinishedListener(TestOnlyListener listener) {

        this.mJobFinishedListener = listener;



    }



    /**

     * Only used for testing. Is that obvious enough?

     */

    @VisibleForTesting

    interface TestOnlyListener {

        void onFinished(List<DocumentInfo> failed);

    }



    /**

     * Calculates the cumulative size of all the documents in the list. Directories are recursed

     * into and totaled up.
@@ -279,7 +314,7 @@ public class CopyService extends IntentService { 
    private long calculateFileSizes(List<DocumentInfo> srcs) throws RemoteException {

        long result = 0;

        for (DocumentInfo src : srcs) {

            if (Document.MIME_TYPE_DIR.equals(src.mimeType)) {

            if (src.isDirectory()) {

                // Directories need to be recursed into.

                result += calculateFileSizesHelper(src.derivedUri);

            } else {
@@ -412,8 +447,21 @@ public class CopyService extends IntentService { 
     */

    private void copy(DocumentInfo srcInfo, DocumentInfo dstDirInfo, int mode)

            throws RemoteException {

        if (DEBUG) Log.d(TAG, "Copying " + srcInfo.displayName + " (" + srcInfo.derivedUri + ")" +

            " to " + dstDirInfo.displayName + " (" + dstDirInfo.derivedUri + ")");



        String opDesc = mode == TRANSFER_MODE_COPY ? "copy" : "move";



        // Guard unsupported recursive operation.

        if (dstDirInfo.equals(srcInfo) || isDescendentOf(srcInfo, dstDirInfo)) {

            if (DEBUG) Log.d(TAG,

                    "Skipping recursive " + opDesc + " of directory " + dstDirInfo.derivedUri);

            mFailedFiles.add(srcInfo);

            return;

        }



        if (DEBUG) Log.d(TAG,

                "Performing " + opDesc + " of " + srcInfo.displayName

                + " (" + srcInfo.derivedUri + ")" + " to " + dstDirInfo.displayName

                + " (" + dstDirInfo.derivedUri + ")");



        // When copying within the same provider, try to use optimized copying and moving.

        // If not supported, then fallback to byte-by-byte copy/move.
@@ -450,13 +498,24 @@ public class CopyService extends IntentService { 
            return;

        }



        if (Document.MIME_TYPE_DIR.equals(srcInfo.mimeType)) {

        if (srcInfo.isDirectory()) {

            copyDirectoryHelper(srcInfo.derivedUri, dstUri, mode);

        } else {

            copyFileHelper(srcInfo.derivedUri, dstUri, mode);

        }

    }



    /**

     * Returns true if {@code doc} is a descendant of {@code parentDoc}.

     */

    boolean isDescendentOf(DocumentInfo doc, DocumentInfo parentDoc) throws RemoteException {

        if (parentDoc.isDirectory() && doc.authority.equals(parentDoc.authority)) {

            return DocumentsContract.isChildDocument(

                    mDstClient, doc.derivedUri, parentDoc.derivedUri);

        }

        return false;

    }



    /**

     * 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".

src/com/android/documentsui/FailureDialogFragment.java

+0 −2
Original line number Diff line number Diff line
@@ -37,7 +37,6 @@ public class FailureDialogFragment extends DialogFragment 
        implements DialogInterface.OnClickListener {

    private static final String TAG = "FailureDialogFragment";



    private int mFailure;

    private int mTransferMode;

    private ArrayList<DocumentInfo> mFailedSrcList;
@@ -75,7 +74,6 @@ public class FailureDialogFragment extends DialogFragment 
    public Dialog onCreateDialog(Bundle inState) {

        super.onCreate(inState);



        mFailure = getArguments().getInt(CopyService.EXTRA_FAILURE);

        mTransferMode = getArguments().getInt(CopyService.EXTRA_TRANSFER_MODE);

        mFailedSrcList = getArguments().getParcelableArrayList(CopyService.EXTRA_SRC_LIST);

src/com/android/documentsui/model/DocumentInfo.java

+24 −2
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.os.Parcelable; 
import android.provider.DocumentsContract;

import android.provider.DocumentsContract.Document;

import android.provider.DocumentsProvider;

import android.support.annotation.VisibleForTesting;

import android.text.TextUtils;



import com.android.documentsui.DocumentsApplication;
@@ -204,13 +205,18 @@ public class DocumentInfo implements Durable, Parcelable { 
        }

    }



    private void deriveFields() {

    @VisibleForTesting

    void deriveFields() {

        derivedUri = DocumentsContract.buildDocumentUri(authority, documentId);

    }



    @Override

    public String toString() {

        return "Document{docId=" + documentId + ", name=" + displayName + "}";

        return "Document{"

                + "docId=" + documentId

                + ", name=" + displayName

                + ", isDirectory=" + isDirectory()

                + "}";

    }



    public boolean isCreateSupported() {
@@ -237,6 +243,22 @@ public class DocumentInfo implements Durable, Parcelable { 
        return (flags & Document.FLAG_DIR_HIDE_GRID_TITLES) != 0;

    }



    public int hashCode() {

        return derivedUri.hashCode() + mimeType.hashCode();

    }



    public boolean equals(Object other) {

        if (this == other) {

            return true;

        } else if (!(other instanceof DocumentInfo)) {

            return false;

        }



        DocumentInfo that = (DocumentInfo) other;

        // Uri + mime type should be totally unique.

        return derivedUri.equals(that.derivedUri) && mimeType.equals(that.mimeType);

    }



    public static String getCursorString(Cursor cursor, String columnName) {

        final int index = cursor.getColumnIndex(columnName);

        return (index != -1) ? cursor.getString(index) : null;

tests/src/com/android/documentsui/CopyTest.java→tests/src/com/android/documentsui/CopyServiceTest.java

+90 −42
Original line number Diff line number Diff line
@@ -28,12 +28,10 @@ import android.net.Uri; 
import android.os.Parcelable;

import android.os.RemoteException;

import android.provider.DocumentsContract;

import android.provider.DocumentsContract.Document;

import android.test.MoreAsserts;

import android.test.ServiceTestCase;

import android.test.mock.MockContentResolver;

import android.test.suitebuilder.annotation.MediumTest;

import android.test.suitebuilder.annotation.SmallTest;

import android.util.Log;



import com.android.documentsui.model.DocumentInfo;
@@ -48,6 +46,7 @@ import libcore.io.Streams; 
import java.io.File;

import java.io.FileInputStream;

import java.io.FileNotFoundException;

import java.io.IOException;

import java.util.ArrayList;

import java.util.List;

import java.util.concurrent.CountDownLatch;
@@ -55,9 +54,9 @@ import java.util.concurrent.TimeUnit; 
import java.util.concurrent.TimeoutException;



@MediumTest

public class CopyTest extends ServiceTestCase<CopyService> {

public class CopyServiceTest extends ServiceTestCase<CopyService> {



    public CopyTest() {

    public CopyServiceTest() {

        super(CopyService.class);

    }
@@ -72,11 +71,13 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
    private DocumentsProviderHelper mDocHelper;

    private StubProvider mStorage;

    private Context mSystemContext;

    private CopyJobListener mListener;



    @Override

    protected void setUp() throws Exception {

        super.setUp();



        mListener = new CopyJobListener();

        setupTestContext();

        mClient = mResolver.acquireContentProviderClient(AUTHORITY);
@@ -84,6 +85,8 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        mStorage.clearCacheAndBuildRoots();



        mDocHelper = new DocumentsProviderHelper(AUTHORITY, mClient);



        assertDestFileCount(0);

    }



    @Override
@@ -97,15 +100,13 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        Uri testFile = mStorage.createFile(SRC_ROOT, srcPath, "text/plain",

                "The five boxing wizards jump quickly".getBytes());



        assertDstFileCountEquals(0);



        startService(createCopyIntent(Lists.newArrayList(testFile)));



        // 2 operations: file creation, then writing data.

        mResolver.waitForChanges(2);



        // Verify that one file was copied; check file contents.

        assertDstFileCountEquals(1);

        assertDestFileCount(1);

        assertCopied(srcPath);

    }
@@ -114,8 +115,6 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        String testContent = "The five boxing wizards jump quickly";

        Uri testFile = mStorage.createFile(SRC_ROOT, srcPath, "text/plain", testContent.getBytes());



        assertDstFileCountEquals(0);



        Intent moveIntent = createCopyIntent(Lists.newArrayList(testFile));

        moveIntent.putExtra(CopyService.EXTRA_TRANSFER_MODE, CopyService.TRANSFER_MODE_MOVE);

        startService(moveIntent);
@@ -124,7 +123,7 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        mResolver.waitForChanges(3);



        // Verify that one file was moved; check file contents.

        assertDstFileCountEquals(1);

        assertDestFileCount(1);

        assertDoesNotExist(SRC_ROOT, srcPath);



        byte[] dstContent = readFile(DST_ROOT, srcPath);
@@ -147,15 +146,13 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
                mStorage.createFile(SRC_ROOT, srcPaths[1], "text/plain", testContent[1].getBytes()),

                mStorage.createFile(SRC_ROOT, srcPaths[2], "text/plain", testContent[2].getBytes()));



        assertDstFileCountEquals(0);



        // Copy all the test files.

        startService(createCopyIntent(testFiles));



        // 3 file creations, 3 file writes.

        mResolver.waitForChanges(6);



        assertDstFileCountEquals(3);

        assertDestFileCount(3);

        for (String path : srcPaths) {

            assertCopied(path);

        }
@@ -163,29 +160,54 @@ public class CopyTest extends ServiceTestCase<CopyService> { 


    public void testCopyEmptyDir() throws Exception {

        String srcPath = "/emptyDir";

        Uri testDir = mStorage.createFile(SRC_ROOT, srcPath, DocumentsContract.Document.MIME_TYPE_DIR,

                null);



        assertDstFileCountEquals(0);

        Uri testDir = createTestDirectory(srcPath);



        startService(createCopyIntent(Lists.newArrayList(testDir)));



        // Just 1 operation: Directory creation.

        mResolver.waitForChanges(1);



        assertDstFileCountEquals(1);

        assertDestFileCount(1);



        // Verify that the dst exists and is a directory.

        File dst = mStorage.getFile(DST_ROOT, srcPath);

        assertTrue(dst.isDirectory());

    }



    public void testNoCopyDirToSelf() throws Exception {

        Uri testDir = createTestDirectory("/someDir");



        Intent intent = createCopyIntent(Lists.newArrayList(testDir), testDir);

        startService(intent);



        getService().addFinishedListener(mListener);



        mListener.waitForFinished();

        mListener.assertFailedCount(1);

        mListener.assertFileFailed("someDir");



        assertDestFileCount(0);

    }



    public void testNoCopyDirToDescendent() throws Exception {

        Uri testDir = createTestDirectory("/someDir");

        Uri descDir = createTestDirectory("/someDir/theDescendent");



        Intent intent = createCopyIntent(Lists.newArrayList(testDir), descDir);

        startService(intent);



        getService().addFinishedListener(mListener);



        mListener.waitForFinished();

        mListener.assertFailedCount(1);

        mListener.assertFileFailed("someDir");



        assertDestFileCount(0);

    }



    public void testMoveEmptyDir() throws Exception {

        String srcPath = "/emptyDir";

        Uri testDir = mStorage.createFile(SRC_ROOT, srcPath, DocumentsContract.Document.MIME_TYPE_DIR,

                null);



        assertDstFileCountEquals(0);

        Uri testDir = createTestDirectory(srcPath);



        Intent moveIntent = createCopyIntent(Lists.newArrayList(testDir));

        moveIntent.putExtra(CopyService.EXTRA_TRANSFER_MODE, CopyService.TRANSFER_MODE_MOVE);
@@ -194,7 +216,7 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        // 2 operations: Directory creation, and removal of the original.

        mResolver.waitForChanges(2);



        assertDstFileCountEquals(1);

        assertDestFileCount(1);



        // Verify that the dst exists and is a directory.

        File dst = mStorage.getFile(DST_ROOT, srcPath);
@@ -217,8 +239,7 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
                srcDir + "/test2.txt"

        };

        // Create test dir; put some files in it.

        Uri testDir = mStorage.createFile(SRC_ROOT, srcDir, DocumentsContract.Document.MIME_TYPE_DIR,

                null);

        Uri testDir = createTestDirectory(srcDir);

        mStorage.createFile(SRC_ROOT, srcFiles[0], "text/plain", testContent[0].getBytes());

        mStorage.createFile(SRC_ROOT, srcFiles[1], "text/plain", testContent[1].getBytes());

        mStorage.createFile(SRC_ROOT, srcFiles[2], "text/plain", testContent[2].getBytes());
@@ -252,8 +273,6 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        Uri testFile = mStorage.createFile(SRC_ROOT, srcPath, "text/plain",

                "The five boxing wizards jump quickly".getBytes());



        assertDstFileCountEquals(0);



        mStorage.simulateReadErrorsForFile(testFile);



        startService(createCopyIntent(Lists.newArrayList(testFile)));
@@ -262,7 +281,7 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        mResolver.waitForChanges(3);



        // Verify that the failed copy was cleaned up.

        assertDstFileCountEquals(0);

        assertDestFileCount(0);

    }



    public void testMoveFileWithReadErrors() throws Exception {
@@ -270,8 +289,6 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        Uri testFile = mStorage.createFile(SRC_ROOT, srcPath, "text/plain",

                "The five boxing wizards jump quickly".getBytes());



        assertDstFileCountEquals(0);



        mStorage.simulateReadErrorsForFile(testFile);



        Intent moveIntent = createCopyIntent(Lists.newArrayList(testFile));
@@ -288,7 +305,7 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
            return;

        } finally {

            // Verify that the failed copy was cleaned up, and the src file wasn't removed.

            assertDstFileCountEquals(0);

            assertDestFileCount(0);

            assertExists(SRC_ROOT, srcPath);

        }

        // The asserts above didn't fail, but the CopyService did something unexpected.
@@ -308,8 +325,7 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
                srcDir + "/test2.txt"

        };

        // Create test dir; put some files in it.

        Uri testDir = mStorage.createFile(SRC_ROOT, srcDir, DocumentsContract.Document.MIME_TYPE_DIR,

                null);

        Uri testDir = createTestDirectory(srcDir);

        mStorage.createFile(SRC_ROOT, srcFiles[0], "text/plain", testContent[0].getBytes());

        Uri errFile = mStorage

                .createFile(SRC_ROOT, srcFiles[1], "text/plain", testContent[1].getBytes());
@@ -346,33 +362,37 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        assertExists(SRC_ROOT, srcFiles[1]);

    }



    /**

     * Copies the given files to a pre-determined destination.

     *

     * @throws FileNotFoundException

     */

    private Uri createTestDirectory(String dir) throws IOException {

        return mStorage.createFile(

                SRC_ROOT, dir, DocumentsContract.Document.MIME_TYPE_DIR, null);

    }



    private Intent createCopyIntent(List<Uri> srcs) throws Exception {

        RootInfo root = mDocHelper.getRoot(DST_ROOT);

        final Uri dst = DocumentsContract.buildDocumentUri(AUTHORITY, root.documentId);



        return createCopyIntent(srcs, dst);

    }



    private Intent createCopyIntent(List<Uri> srcs, Uri dst) throws Exception {

        final ArrayList<DocumentInfo> srcDocs = Lists.newArrayList();

        for (Uri src : srcs) {

            srcDocs.add(DocumentInfo.fromUri(mResolver, src));

        }



        RootInfo root = mDocHelper.getRoot(DST_ROOT);

        final Uri dst = DocumentsContract.buildDocumentUri(AUTHORITY, root.documentId);

        DocumentStack stack = new DocumentStack();

        stack.push(DocumentInfo.fromUri(mResolver, dst));

        final Intent copyIntent = new Intent(mContext, CopyService.class);

        copyIntent.putParcelableArrayListExtra(CopyService.EXTRA_SRC_LIST, srcDocs);

        copyIntent.putExtra(Shared.EXTRA_STACK, (Parcelable) stack);



        // startService(copyIntent);

        return copyIntent;

    }



    /**

     * Returns a count of the files in the given directory.

     */

    private void assertDstFileCountEquals(int expected) throws RemoteException {

    private void assertDestFileCount(int expected) throws RemoteException {

        RootInfo dest = mDocHelper.getRoot(DST_ROOT);

        final Uri queryUri = DocumentsContract.buildChildDocumentsUri(AUTHORITY,

                dest.documentId);
@@ -449,6 +469,34 @@ public class CopyTest extends ServiceTestCase<CopyService> { 
        mResolver.addProvider(AUTHORITY, mStorage);

    }



    private final class CopyJobListener implements CopyService.TestOnlyListener {



        final CountDownLatch latch = new CountDownLatch(1);

        final List<DocumentInfo> failedDocs = new ArrayList<>();

        @Override

        public void onFinished(List<DocumentInfo> failed) {

            failedDocs.addAll(failed);

            latch.countDown();

        }



        public void assertFileFailed(String expectedName) {

            for (DocumentInfo failed : failedDocs) {

                if (expectedName.equals(failed.displayName)) {

                    return;

                }

            }

            fail("Couldn't find failed file: " + expectedName);

        }



        public void waitForFinished() throws InterruptedException {

            latch.await(500, TimeUnit.MILLISECONDS);

        }



        public void assertFailedCount(int expected) {

            assertEquals(expected, failedDocs.size());

        }

    }



    /**

     * A test resolver that enables this test suite to listen for notifications that mark when copy

     * operations are done.

tests/src/com/android/documentsui/StubProvider.java

+10 −0
Original line number Diff line number Diff line
@@ -531,6 +531,16 @@ public class StubProvider extends DocumentsProvider { 
            this.rootInfo = rootInfo;

            mStorage.put(this.documentId, this);

        }

        @Override

        public String toString() {

            return "StubDocument{"

                    + "path:" + file.getPath()

                    + ", mimeType:" + mimeType

                    + ", rootInfo:" + rootInfo

                    + ", documentId:" + documentId

                    + ", parentId:" + parentId

                    + "}";

        }

    }



    private static String getDocumentIdForFile(File file) {