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

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

Fix roots filtering for move operations.

Add test coverage for the Downloads case.
This was a regression to my recent cleanup of RootInfo.
Fixed logic to not exclude basically *everything* when a
    directory is in the set of files to be copied.

Bug: 27303346
Change-Id: I4d5608e0d8d95448b027b20107a11a7a5e46d45e
parent 78614f77
Loading
Loading
Loading
Loading
+50 −17
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@
package com.android.documentsui;

import static com.android.documentsui.Shared.DEBUG;
import static com.android.documentsui.Shared.TAG;

import android.content.ContentProviderClient;
import android.content.ContentResolver;
@@ -62,6 +61,8 @@ public class RootsCache {
    public static final Uri sNotificationUri = Uri.parse(
            "content://com.android.documentsui.roots/");

    private static final String TAG = "RootsCache";

    private final Context mContext;
    private final ContentObserver mObserver;
    private OnCacheUpdateListener mCacheUpdateListener;
@@ -412,25 +413,56 @@ public class RootsCache {
    static List<RootInfo> getMatchingRoots(Collection<RootInfo> roots, State state) {
        final List<RootInfo> matching = new ArrayList<>();
        for (RootInfo root : roots) {
            // Exclude read-only devices when creating
            if (state.action == State.ACTION_CREATE && !root.supportsCreate()) continue;

            if (DEBUG) Log.d(TAG, "Evaluating " + root);

            if (state.action == State.ACTION_CREATE && !root.supportsCreate()) {
                if (DEBUG) Log.d(TAG, "Excluding read-only root because: ACTION_CREATE.");
                continue;
            }

            if (state.action == State.ACTION_PICK_COPY_DESTINATION
                    && !root.supportsCreate()) continue;
            // Exclude roots that don't support directory picking
            if (state.action == State.ACTION_OPEN_TREE && !root.supportsChildren()) continue;
            // Exclude advanced devices when not requested
            if (!state.showAdvanced && root.isAdvanced()) continue;
                    && !root.supportsCreate()) {
                if (DEBUG) Log.d(
                        TAG, "Excluding read-only root because: ACTION_PICK_COPY_DESTINATION.");
                continue;
            }

            if (state.action == State.ACTION_OPEN_TREE && !root.supportsChildren()) {
                if (DEBUG) Log.d(
                        TAG, "Excluding root !supportsChildren because: ACTION_OPEN_TREE.");
                continue;
            }

            if (!state.showAdvanced && root.isAdvanced()) {
                if (DEBUG) Log.d(TAG, "Excluding root because: unwanted advanced device.");
                continue;
            }

            // Exclude non-local devices when local only
            if (state.localOnly && !root.isLocalOnly()) continue;
            if (state.localOnly && !root.isLocalOnly()) {
                if (DEBUG) Log.d(TAG, "Excluding root because: unwanted non-local device.");
                continue;
            }

            // Exclude downloads roots as it doesn't support directory creation (actually
            // we just don't show them).
            // TODO: Add flag to check the root supports directory creation.
            if (state.directoryCopy && !root.isDownloads()) continue;
            if (state.directoryCopy && root.isDownloads()) {
                if (DEBUG) Log.d(
                        TAG, "Excluding downloads root because: unsupported directory copy.");
                continue;
            }

            // Only show empty roots when creating, or in browse mode.
            if (root.isEmpty() && (state.action == State.ACTION_OPEN
                    || state.action == State.ACTION_GET_CONTENT)) {
                if (DEBUG) Log.i(TAG, "Skipping empty root: " + root);
            if (state.action == State.ACTION_OPEN && root.isEmpty()) {
                if (DEBUG) Log.d(TAG, "Excluding empty root because: ACTION_OPEN.");
                continue;
            }

            // Only show empty roots when creating, or in browse mode.
            if (state.action == State.ACTION_GET_CONTENT && root.isEmpty()) {
                if (DEBUG) Log.d(TAG, "Excluding empty root because: ACTION_GET_CONTENT.");
                continue;
            }

@@ -439,18 +471,19 @@ public class RootsCache {
                    MimePredicate.mimeMatches(root.derivedMimeTypes, state.acceptMimes) ||
                    MimePredicate.mimeMatches(state.acceptMimes, root.derivedMimeTypes);
            if (!overlap) {
                if (DEBUG) Log.d(
                        TAG, "Excluding root because: unsupported content types > "
                        + state.acceptMimes);
                continue;
            }

            // Exclude roots from the calling package.
            if (state.excludedAuthorities.contains(root.authority)) {
                if (DEBUG) Log.d(
                        TAG, "Excluding root " + root.authority + " from calling package.");
                if (DEBUG) Log.d(TAG, "Excluding root because: calling package.");
                continue;
            }

            if (DEBUG) Log.d(
                    TAG, "Including root " + root + " in roots list.");
            if (DEBUG) Log.d(TAG, "Including " + root);
            matching.add(root);
        }
        return matching;
+64 −43
Original line number Diff line number Diff line
@@ -16,6 +16,9 @@

package com.android.documentsui;

import static com.android.documentsui.RootsCache.getMatchingRoots;
import static com.google.common.collect.Lists.newArrayList;

import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.SmallTest;

@@ -28,23 +31,18 @@ import java.util.List;
@SmallTest
public class RootsCacheTest extends AndroidTestCase {

    private static RootInfo buildForMimeTypes(String... mimeTypes) {
        final RootInfo root = new RootInfo();
        root.derivedMimeTypes = mimeTypes;
        return root;
    }
    private static RootInfo mNull = new RootInfo();
    private static RootInfo mEmpty = buildForMimeTypes();
    private static RootInfo mWild = buildForMimeTypes("*/*");
    private static RootInfo mImages = buildForMimeTypes("image/*");
    private static RootInfo mAudio = buildForMimeTypes(
            "audio/*", "application/ogg", "application/x-flac");
    private static RootInfo mDocs = buildForMimeTypes(
            "application/msword", "application/vnd.ms-excel");
    private static RootInfo mMalformed1 = buildForMimeTypes("meow");
    private static RootInfo mMalformed2 = buildForMimeTypes("*/meow");

    private RootInfo mNull = new RootInfo();
    private RootInfo mEmpty = buildForMimeTypes();
    private RootInfo mWild = buildForMimeTypes("*/*");
    private RootInfo mImages = buildForMimeTypes("image/*");
    private RootInfo mAudio = buildForMimeTypes("audio/*", "application/ogg", "application/x-flac");
    private RootInfo mDocs = buildForMimeTypes("application/msword", "application/vnd.ms-excel");
    private RootInfo mMalformed1 = buildForMimeTypes("meow");
    private RootInfo mMalformed2 = buildForMimeTypes("*/meow");

    private List<RootInfo> mRoots = Lists.newArrayList(
            mNull, mWild, mEmpty, mImages, mAudio, mDocs, mMalformed1, mMalformed2);
    private List<RootInfo> mRoots;

    private State mState;

@@ -52,70 +50,87 @@ public class RootsCacheTest extends AndroidTestCase {
    protected void setUp() throws Exception {
        super.setUp();

        mRoots = Lists.newArrayList(
                mNull, mWild, mEmpty, mImages, mAudio, mDocs, mMalformed1, mMalformed2);

        mState = new State();
        mState.action = State.ACTION_OPEN;
        mState.showAdvanced = true;
        mState.localOnly = false;
    }

    public void testMatchingRootsEverything() throws Exception {
    public void testMatchingRoots_Everything() throws Exception {
        mState.acceptMimes = new String[] { "*/*" };
        assertContainsExactly(
                newArrayList(mNull, mWild, mImages, mAudio, mDocs, mMalformed1, mMalformed2),
                getMatchingRoots(mRoots, mState));
    }

    public void testMatchingRoots_DirectoryCopy() throws Exception {
        RootInfo downloads = buildForMimeTypes("*/*");
        downloads.authority = "com.android.providers.downloads.documents";
        mRoots.add(downloads);

        mState.acceptMimes = new String[] { "*/*" };
        mState.directoryCopy = true;

        // basically we're asserting that the results don't contain downloads
        assertContainsExactly(
                Lists.newArrayList(mNull, mWild, mImages, mAudio, mDocs, mMalformed1, mMalformed2),
                RootsCache.getMatchingRoots(mRoots, mState));
                newArrayList(mNull, mWild, mImages, mAudio, mDocs, mMalformed1, mMalformed2),
                getMatchingRoots(mRoots, mState));
    }

    public void testMatchingRootsPngOrWild() throws Exception {
    public void testMatchingRoots_PngOrWild() throws Exception {
        mState.acceptMimes = new String[] { "image/png", "*/*" };
        assertContainsExactly(
                Lists.newArrayList(mNull, mWild, mImages, mAudio, mDocs, mMalformed1, mMalformed2),
                RootsCache.getMatchingRoots(mRoots, mState));
                newArrayList(mNull, mWild, mImages, mAudio, mDocs, mMalformed1, mMalformed2),
                getMatchingRoots(mRoots, mState));
    }

    public void testMatchingRootsAudioWild() throws Exception {
    public void testMatchingRoots_AudioWild() throws Exception {
        mState.acceptMimes = new String[] { "audio/*" };
        assertContainsExactly(
                Lists.newArrayList(mNull, mWild, mAudio),
                RootsCache.getMatchingRoots(mRoots, mState));
                newArrayList(mNull, mWild, mAudio),
                getMatchingRoots(mRoots, mState));
    }

    public void testMatchingRootsAudioWildOrImageWild() throws Exception {
    public void testMatchingRoots_AudioWildOrImageWild() throws Exception {
        mState.acceptMimes = new String[] { "audio/*", "image/*" };
        assertContainsExactly(
                Lists.newArrayList(mNull, mWild, mAudio, mImages),
                RootsCache.getMatchingRoots(mRoots, mState));
                newArrayList(mNull, mWild, mAudio, mImages),
                getMatchingRoots(mRoots, mState));
    }

    public void testMatchingRootsAudioSpecific() throws Exception {
    public void testMatchingRoots_AudioSpecific() throws Exception {
        mState.acceptMimes = new String[] { "audio/mpeg" };
        assertContainsExactly(
                Lists.newArrayList(mNull, mWild, mAudio),
                RootsCache.getMatchingRoots(mRoots, mState));
                newArrayList(mNull, mWild, mAudio),
                getMatchingRoots(mRoots, mState));
    }

    public void testMatchingRootsDocument() throws Exception {
    public void testMatchingRoots_Document() throws Exception {
        mState.acceptMimes = new String[] { "application/msword" };
        assertContainsExactly(
                Lists.newArrayList(mNull, mWild, mDocs),
                RootsCache.getMatchingRoots(mRoots, mState));
                newArrayList(mNull, mWild, mDocs),
                getMatchingRoots(mRoots, mState));
    }

    public void testMatchingRootsApplication() throws Exception {
    public void testMatchingRoots_Application() throws Exception {
        mState.acceptMimes = new String[] { "application/*" };
        assertContainsExactly(
                Lists.newArrayList(mNull, mWild, mAudio, mDocs),
                RootsCache.getMatchingRoots(mRoots, mState));
                newArrayList(mNull, mWild, mAudio, mDocs),
                getMatchingRoots(mRoots, mState));
    }

    public void testMatchingRootsFlacOrPng() throws Exception {
    public void testMatchingRoots_FlacOrPng() throws Exception {
        mState.acceptMimes = new String[] { "application/x-flac", "image/png" };
        assertContainsExactly(
                Lists.newArrayList(mNull, mWild, mAudio, mImages),
                RootsCache.getMatchingRoots(mRoots, mState));
                newArrayList(mNull, mWild, mAudio, mImages),
                getMatchingRoots(mRoots, mState));
    }

    public void testExcludedAuthorities() throws Exception {
        final List<RootInfo> roots = Lists.newArrayList();
        final List<RootInfo> roots = newArrayList();

        // Set up some roots
        for (int i = 0; i < 5; ++i) {
@@ -124,7 +139,7 @@ public class RootsCacheTest extends AndroidTestCase {
            roots.add(root);
        }
        // Make some allowed authorities
        List<RootInfo> allowedRoots = Lists.newArrayList(
        List<RootInfo> allowedRoots = newArrayList(
            roots.get(0), roots.get(2), roots.get(4));
        // Set up the excluded authority list
        for (RootInfo root: roots) {
@@ -136,7 +151,7 @@ public class RootsCacheTest extends AndroidTestCase {

        assertContainsExactly(
            allowedRoots,
            RootsCache.getMatchingRoots(roots, mState));
            getMatchingRoots(roots, mState));
    }

    private static void assertContainsExactly(List<?> expected, List<?> actual) {
@@ -145,4 +160,10 @@ public class RootsCacheTest extends AndroidTestCase {
            assertTrue(actual.contains(o));
        }
    }

    private static RootInfo buildForMimeTypes(String... mimeTypes) {
        final RootInfo root = new RootInfo();
        root.derivedMimeTypes = mimeTypes;
        return root;
    }
}