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

Commit d3afdeeb 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 2829e996

core/java/android/provider/DocumentsContract.java

+27 −1
Original line number Diff line number Diff line
@@ -67,7 +67,7 @@ import java.util.List; 
 * @see DocumentsProvider

 */

public final class DocumentsContract {

    private static final String TAG = "Documents";

    private static final String TAG = "DocumentsContract";



    // content://com.example/root/

    // content://com.example/root/sdcard/
@@ -591,6 +591,12 @@ public final class DocumentsContract { 
     */

    public static final String EXTRA_ERROR = "error";



    /**

     * Optional result (I'm thinking boolean) answer to a question.

     * {@hide}

     */

    public static final String EXTRA_RESULT = "result";



    /** {@hide} */

    public static final String METHOD_CREATE_DOCUMENT = "android:createDocument";

    /** {@hide} */
@@ -601,6 +607,8 @@ public final class DocumentsContract { 
    public static final String METHOD_COPY_DOCUMENT = "android:copyDocument";

    /** {@hide} */

    public static final String METHOD_MOVE_DOCUMENT = "android:moveDocument";

    /** {@hide} */

    public static final String METHOD_IS_CHILD_DOCUMENT = "android:isChildDocument";



    /** {@hide} */

    public static final String EXTRA_URI = "uri";
@@ -1025,6 +1033,24 @@ public final class DocumentsContract { 
        return out.getParcelable(DocumentsContract.EXTRA_URI);

    }



    /** {@hide} */

    public static boolean isChildDocument(ContentProviderClient client, Uri parentDocumentUri,

            Uri childDocumentUri) throws RemoteException {



        final Bundle in = new Bundle();

        in.putParcelable(DocumentsContract.EXTRA_URI, parentDocumentUri);

        in.putParcelable(DocumentsContract.EXTRA_TARGET_URI, childDocumentUri);



        final Bundle out = client.call(METHOD_IS_CHILD_DOCUMENT, null, in);

        if (out == null) {

            throw new RemoteException("Failed to get a reponse from isChildDocument query.");

        }

        if (!out.containsKey(DocumentsContract.EXTRA_RESULT)) {

            throw new RemoteException("Response did not include result field..");

        }

        return out.getBoolean(DocumentsContract.EXTRA_RESULT);

    }



    /**

     * Change the display name of an existing document.

     * <p>

core/java/android/provider/DocumentsProvider.java

+96 −74
Original line number Diff line number Diff line
@@ -16,11 +16,12 @@ 


package android.provider;



import static android.provider.DocumentsContract.METHOD_COPY_DOCUMENT;

import static android.provider.DocumentsContract.METHOD_CREATE_DOCUMENT;

import static android.provider.DocumentsContract.METHOD_DELETE_DOCUMENT;

import static android.provider.DocumentsContract.METHOD_RENAME_DOCUMENT;

import static android.provider.DocumentsContract.METHOD_COPY_DOCUMENT;

import static android.provider.DocumentsContract.METHOD_IS_CHILD_DOCUMENT;

import static android.provider.DocumentsContract.METHOD_MOVE_DOCUMENT;

import static android.provider.DocumentsContract.METHOD_RENAME_DOCUMENT;

import static android.provider.DocumentsContract.buildDocumentUri;

import static android.provider.DocumentsContract.buildDocumentUriMaybeUsingTree;

import static android.provider.DocumentsContract.buildTreeDocumentUri;
@@ -688,6 +689,16 @@ public abstract class DocumentsProvider extends ContentProvider { 
            return super.call(method, arg, extras);

        }



        try {

            return callUnchecked(method, arg, extras);

        } catch (FileNotFoundException e) {

            throw new IllegalStateException("Failed call " + method, e);

        }

    }



    private Bundle callUnchecked(String method, String arg, Bundle extras)

            throws FileNotFoundException {



        final Context context = getContext();

        final Uri documentUri = extras.getParcelable(DocumentsContract.EXTRA_URI);

        final String authority = documentUri.getAuthority();
@@ -697,11 +708,25 @@ public abstract class DocumentsProvider extends ContentProvider { 
            throw new SecurityException(

                    "Requested authority " + authority + " doesn't match provider " + mAuthority);

        }

        enforceTree(documentUri);



        final Bundle out = new Bundle();

        try {

            if (METHOD_CREATE_DOCUMENT.equals(method)) {



        // If the URI is a tree URI performs some validation.

        enforceTree(documentUri);



        if (METHOD_IS_CHILD_DOCUMENT.equals(method)) {

            enforceReadPermissionInner(documentUri, getCallingPackage(), null);



            final Uri childUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI);

            final String childAuthority = childUri.getAuthority();

            final String childId = DocumentsContract.getDocumentId(childUri);



            out.putBoolean(

                    DocumentsContract.EXTRA_RESULT,

                    mAuthority.equals(childAuthority)

                            && isChildDocument(documentId, childId));



        } else if (METHOD_CREATE_DOCUMENT.equals(method)) {

            enforceWritePermissionInner(documentUri, getCallingPackage(), null);



            final String mimeType = extras.getString(Document.COLUMN_MIME_TYPE);
@@ -775,7 +800,6 @@ public abstract class DocumentsProvider extends ContentProvider { 
            enforceReadPermissionInner(documentUri, getCallingPackage(), null);

            enforceWritePermissionInner(targetUri, getCallingPackage(), null);



                final String displayName = extras.getString(Document.COLUMN_DISPLAY_NAME);

            final String newDocumentId = moveDocument(documentId, targetId);



            if (newDocumentId != null) {
@@ -797,9 +821,7 @@ public abstract class DocumentsProvider extends ContentProvider { 
        } else {

            throw new UnsupportedOperationException("Method not supported " + method);

        }

        } catch (FileNotFoundException e) {

            throw new IllegalStateException("Failed call " + method, e);

        }



        return out;

    }

packages/DocumentsUI/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".

packages/DocumentsUI/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);

packages/DocumentsUI/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;