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

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

Don't proxy calls to DocumentsAdapter in MultiSelManager.

We were bouncing calls out to the adapter pulled from RecyclerView,
but the adapter is easy to write a test double for and
we can readily inject the adapter when needed at runtime.

Eliminates unnecessary indirection.
Also, protect against failure when documents can't be loaded for delete.

Change-Id: Ief6585bf2e3e4fd407d801d485a9d7cd888b8500
parent ab6fb6fc
Loading
Loading
Loading
Loading
+1 −0
Original line number Original line Diff line number Diff line
@@ -332,6 +332,7 @@ public class DirectoryFragment extends Fragment implements DocumentsAdapter.Envi
        // into the selection manager.
        // into the selection manager.
        mSelectionManager = new MultiSelectManager(
        mSelectionManager = new MultiSelectManager(
                mRecView,
                mRecView,
                mAdapter,
                state.allowMultiple
                state.allowMultiple
                    ? MultiSelectManager.MODE_MULTIPLE
                    ? MultiSelectManager.MODE_MULTIPLE
                    : MultiSelectManager.MODE_SINGLE);
                    : MultiSelectManager.MODE_SINGLE);
+8 −2
Original line number Original line Diff line number Diff line
@@ -414,8 +414,14 @@ public class Model implements SiblingProvider {


        @Override
        @Override
        protected Void doInBackground(Selection... selected) {
        protected Void doInBackground(Selection... selected) {
            List<DocumentInfo> toDelete = getDocuments(selected[0]);
            List<DocumentInfo> toDelete = null;
            mHadTrouble = false;
            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) {
            for (DocumentInfo doc : toDelete) {
                if (!doc.isDeleteSupported()) {
                if (!doc.isDeleteSupported()) {
+26 −57
Original line number Original line Diff line number Diff line
@@ -68,21 +68,23 @@ public final class MultiSelectManager implements View.OnKeyListener {


    private final Selection mSelection = new Selection();
    private final Selection mSelection = new Selection();


    private Range mRanger;
    private final SelectionEnvironment mEnvironment;
    private SelectionEnvironment mEnvironment;
    private final DocumentsAdapter mAdapter;

    private final List<MultiSelectManager.Callback> mCallbacks = new ArrayList<>(1);
    private final List<MultiSelectManager.Callback> mCallbacks = new ArrayList<>(1);


    private Range mRanger;
    private boolean mSingleSelect;
    private boolean mSingleSelect;


    @Nullable private BandController mBandManager;
    @Nullable private BandController mBandManager;



    /**
    /**
     * @param recyclerView
     * @param recyclerView
     * @param mode Selection mode
     * @param mode Selection mode
     */
     */
    public MultiSelectManager(final RecyclerView recyclerView, int mode) {
    public MultiSelectManager(
        this(new RuntimeSelectionEnvironment(recyclerView), mode);
            final RecyclerView recyclerView, DocumentsAdapter adapter, int mode) {
        this(new RuntimeSelectionEnvironment(recyclerView), adapter, mode);


        if (mode == MODE_MULTIPLE) {
        if (mode == MODE_MULTIPLE) {
            mBandManager = new BandController();
            mBandManager = new BandController();
@@ -133,11 +135,12 @@ public final class MultiSelectManager implements View.OnKeyListener {
     * @hide
     * @hide
     */
     */
    @VisibleForTesting
    @VisibleForTesting
    MultiSelectManager(SelectionEnvironment environment, int mode) {
    MultiSelectManager(SelectionEnvironment environment, DocumentsAdapter adapter, int mode) {
        mEnvironment = checkNotNull(environment, "'environment' cannot be null.");
        mEnvironment = checkNotNull(environment, "'environment' cannot be null.");
        mAdapter = checkNotNull(adapter, "'adapter' cannot be null.");
        mSingleSelect = mode == MODE_SINGLE;
        mSingleSelect = mode == MODE_SINGLE;


        mEnvironment.registerDataObserver(
        mAdapter.registerAdapterDataObserver(
                new RecyclerView.AdapterDataObserver() {
                new RecyclerView.AdapterDataObserver() {


                    private List<String> mModelIds;
                    private List<String> mModelIds;
@@ -146,7 +149,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
                    public void onChanged() {
                    public void onChanged() {
                        // TODO: This is causing b/22765812
                        // TODO: This is causing b/22765812
                        mSelection.clear();
                        mSelection.clear();
                        mModelIds = mEnvironment.getModelIds();
                        mModelIds = mAdapter.getModelIds();
                    }
                    }


                    @Override
                    @Override
@@ -336,7 +339,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
            if (DEBUG) Log.d(TAG, "Ignoring toggle for element with no position.");
            if (DEBUG) Log.d(TAG, "Ignoring toggle for element with no position.");
            return;
            return;
        }
        }
        String id = mEnvironment.getModelIdFromAdapterPosition(position);
        String id = mAdapter.getModelId(position);
        if (id != null) {
        if (id != null) {
            toggleSelection(id);
            toggleSelection(id);
        }
        }
@@ -388,7 +391,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
            return;
            return;
        }
        }


        if (mSelection.contains(mEnvironment.getModelIdFromAdapterPosition(position))) {
        if (mSelection.contains(mAdapter.getModelId(position))) {
            mRanger = new Range(position);
            mRanger = new Range(position);
        }
        }
    }
    }
@@ -405,7 +408,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
    private void updateRange(int begin, int end, boolean selected) {
    private void updateRange(int begin, int end, boolean selected) {
        checkState(end >= begin);
        checkState(end >= begin);
        for (int i = begin; i <= end; i++) {
        for (int i = begin; i <= end; i++) {
            String id = mEnvironment.getModelIdFromAdapterPosition(i);
            String id = mAdapter.getModelId(i);
            if (id == null) {
            if (id == null) {
                continue;
                continue;
            }
            }
@@ -473,7 +476,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
        for (int i = lastListener; i > -1; i--) {
        for (int i = lastListener; i > -1; i--) {
            mCallbacks.get(i).onItemStateChanged(id, selected);
            mCallbacks.get(i).onItemStateChanged(id, selected);
        }
        }
        mEnvironment.notifyItemChanged(id);
        mAdapter.onItemSelectionChanged(id);
    }
    }


    /**
    /**
@@ -811,16 +814,6 @@ public final class MultiSelectManager implements View.OnKeyListener {
        int getChildCount();
        int getChildCount();
        int getVisibleChildCount();
        int getVisibleChildCount();
        void focusItem(int position);
        void focusItem(int position);
        /**
         * Returns null if non-useful item.
         * @param position
         * @return
         */
        @Nullable String getModelIdFromAdapterPosition(int position);
        int getItemCount();
        List<String> getModelIds();
        void notifyItemChanged(String id);
        void registerDataObserver(RecyclerView.AdapterDataObserver observer);
    }
    }


    /** Recycler view facade implementation backed by good ol' RecyclerView. */
    /** Recycler view facade implementation backed by good ol' RecyclerView. */
@@ -830,11 +823,9 @@ public final class MultiSelectManager implements View.OnKeyListener {
        private final Drawable mBand;
        private final Drawable mBand;


        private boolean mIsOverlayShown = false;
        private boolean mIsOverlayShown = false;
        private DocumentsAdapter mAdapter;


        RuntimeSelectionEnvironment(RecyclerView rv) {
        RuntimeSelectionEnvironment(RecyclerView view) {
            mView = rv;
            mView = view;
            mAdapter = (DocumentsAdapter) rv.getAdapter();
            mBand = mView.getContext().getTheme().getDrawable(R.drawable.band_select_overlay);
            mBand = mView.getContext().getTheme().getDrawable(R.drawable.band_select_overlay);
        }
        }


@@ -852,11 +843,6 @@ public final class MultiSelectManager implements View.OnKeyListener {
            return getAdapterPositionForChildView(mView.getChildAt(index));
            return getAdapterPositionForChildView(mView.getChildAt(index));
        }
        }


        @Override
        public @Nullable String getModelIdFromAdapterPosition(int position) {
            return mAdapter.getModelId(position);
        }

        @Override
        @Override
        public void addOnScrollListener(RecyclerView.OnScrollListener listener) {
        public void addOnScrollListener(RecyclerView.OnScrollListener listener) {
            mView.addOnScrollListener(listener);
            mView.addOnScrollListener(listener);
@@ -973,26 +959,6 @@ public final class MultiSelectManager implements View.OnKeyListener {
                    });
                    });
            }
            }
        }
        }

        @Override
        public void notifyItemChanged(String id) {
            mAdapter.onItemSelectionChanged(id);
        }

        @Override
        public int getItemCount() {
            return mAdapter.getItemCount();
        }

        @Override
        public void registerDataObserver(RecyclerView.AdapterDataObserver observer) {
            mAdapter.registerAdapterDataObserver(observer);
        }

        @Override
        public List<String> getModelIds() {
            return mAdapter.getModelIds();
        }
    }
    }


    public interface Callback {
    public interface Callback {
@@ -1049,7 +1015,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
            mModelBuilder = new Runnable() {
            mModelBuilder = new Runnable() {
                @Override
                @Override
                public void run() {
                public void run() {
                    mModel = new GridModel(mEnvironment);
                    mModel = new GridModel(mEnvironment, mAdapter);
                    mModel.addOnSelectionChangedListener(BandController.this);
                    mModel.addOnSelectionChangedListener(BandController.this);
                }
                }
            };
            };
@@ -1100,7 +1066,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
            return !isActive()
            return !isActive()
                    && e.isMouseEvent()  // a mouse
                    && e.isMouseEvent()  // a mouse
                    && e.isActionDown()  // the initial button press
                    && e.isActionDown()  // the initial button press
                    && mEnvironment.getItemCount() > 0
                    && mAdapter.getItemCount() > 0
                    && e.getItemPosition() == RecyclerView.NO_ID;  // in empty space
                    && e.getItemPosition() == RecyclerView.NO_ID;  // in empty space
        }
        }


@@ -1177,7 +1143,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
            mModel.endSelection();
            mModel.endSelection();
            int firstSelected = mModel.getPositionNearestOrigin();
            int firstSelected = mModel.getPositionNearestOrigin();
            if (firstSelected != NOT_SET) {
            if (firstSelected != NOT_SET) {
                if (mSelection.contains(mEnvironment.getModelIdFromAdapterPosition(firstSelected))) {
                if (mSelection.contains(mAdapter.getModelId(firstSelected))) {
                    // TODO: firstSelected should really be lastSelected, we want to anchor the item
                    // TODO: firstSelected should really be lastSelected, we want to anchor the item
                    // where the mouse-up occurred.
                    // where the mouse-up occurred.
                    setSelectionRangeBegin(firstSelected);
                    setSelectionRangeBegin(firstSelected);
@@ -1339,6 +1305,8 @@ public final class MultiSelectManager implements View.OnKeyListener {
        private static final int LOWER_RIGHT = LOWER | RIGHT;
        private static final int LOWER_RIGHT = LOWER | RIGHT;


        private final SelectionEnvironment mHelper;
        private final SelectionEnvironment mHelper;
        private final DocumentsAdapter mAdapter;

        private final List<OnSelectionChangedListener> mOnSelectionChangedListeners =
        private final List<OnSelectionChangedListener> mOnSelectionChangedListeners =
                new ArrayList<>();
                new ArrayList<>();


@@ -1376,8 +1344,9 @@ public final class MultiSelectManager implements View.OnKeyListener {
        // should expand from when Shift+click is used.
        // should expand from when Shift+click is used.
        private int mPositionNearestOrigin = NOT_SET;
        private int mPositionNearestOrigin = NOT_SET;


        GridModel(SelectionEnvironment helper) {
        GridModel(SelectionEnvironment helper, DocumentsAdapter adapter) {
            mHelper = helper;
            mHelper = helper;
            mAdapter = adapter;
            mHelper.addOnScrollListener(this);
            mHelper.addOnScrollListener(this);
        }
        }


@@ -1580,7 +1549,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
                    // position. Use a sentry value to prevent erroneously selecting item 0.
                    // position. Use a sentry value to prevent erroneously selecting item 0.
                    int position = items.get(items.keyAt(row), NOT_SET);
                    int position = items.get(items.keyAt(row), NOT_SET);
                    if (position != NOT_SET) {
                    if (position != NOT_SET) {
                        String id = mHelper.getModelIdFromAdapterPosition(position);
                        String id = mAdapter.getModelId(position);
                        if (id != null) {
                        if (id != null) {
                            // The adapter inserts items for UI layout purposes that aren't associated
                            // The adapter inserts items for UI layout purposes that aren't associated
                            // with files.  Those will have a null model ID.  Don't select them.
                            // with files.  Those will have a null model ID.  Don't select them.
@@ -1989,7 +1958,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
        if (keyCode == KeyEvent.KEYCODE_MOVE_HOME) {
        if (keyCode == KeyEvent.KEYCODE_MOVE_HOME) {
            position = 0;
            position = 0;
        } else if (keyCode == KeyEvent.KEYCODE_MOVE_END) {
        } else if (keyCode == KeyEvent.KEYCODE_MOVE_END) {
            position = mEnvironment.getItemCount() - 1;
            position = mAdapter.getItemCount() - 1;
        } else {
        } else {
            // Find a navigation target based on the arrow key that the user pressed.  Ignore
            // Find a navigation target based on the arrow key that the user pressed.  Ignore
            // navigation targets that aren't items in the recycler view.
            // navigation targets that aren't items in the recycler view.
+2 −1
Original line number Original line Diff line number Diff line
@@ -84,7 +84,8 @@ public class ModelBackedDocumentsAdapterTest extends AndroidTestCase {
    // Tests that the items can be hidden and unhidden.
    // Tests that the items can be hidden and unhidden.
    public void testUnhide_PreservesOrder() {
    public void testUnhide_PreservesOrder() {
        List<String> ids = mModel.getModelIds();
        List<String> ids = mModel.getModelIds();
        SparseArray<String> hidden = mAdapter.hide(ids.toArray(new String[ids.size()]));
        SparseArray<String> hidden = mAdapter.hide(
                ids.get(0), ids.get(1), ids.get(5), ids.get(9));
        mAdapter.unhide(hidden);
        mAdapter.unhide(hidden);


        // Finally ensure the restored items are in the original order
        // Finally ensure the restored items are in the original order
+7 −4
Original line number Original line Diff line number Diff line
@@ -23,6 +23,7 @@ import android.util.SparseBooleanArray;


import com.android.documentsui.TestInputEvent;
import com.android.documentsui.TestInputEvent;
import com.android.documentsui.dirlist.MultiSelectManager.Selection;
import com.android.documentsui.dirlist.MultiSelectManager.Selection;

import com.google.common.collect.Lists;
import com.google.common.collect.Lists;


import java.util.ArrayList;
import java.util.ArrayList;
@@ -44,11 +45,13 @@ public class MultiSelectManagerTest extends AndroidTestCase {
    private MultiSelectManager mManager;
    private MultiSelectManager mManager;
    private TestCallback mCallback;
    private TestCallback mCallback;
    private TestSelectionEnvironment mEnv;
    private TestSelectionEnvironment mEnv;
    private TestDocumentsAdapter mAdapter;


    public void setUp() throws Exception {
    public void setUp() throws Exception {
        mCallback = new TestCallback();
        mCallback = new TestCallback();
        mEnv = new TestSelectionEnvironment(items);
        mEnv = new TestSelectionEnvironment(items);
        mManager = new MultiSelectManager(mEnv, MultiSelectManager.MODE_MULTIPLE);
        mAdapter = new TestDocumentsAdapter(items);
        mManager = new MultiSelectManager(mEnv, mAdapter, MultiSelectManager.MODE_MULTIPLE);
        mManager.addCallback(mCallback);
        mManager.addCallback(mCallback);
    }
    }


@@ -164,7 +167,7 @@ public class MultiSelectManagerTest extends AndroidTestCase {
    }
    }


    public void testSingleSelectMode() {
    public void testSingleSelectMode() {
        mManager = new MultiSelectManager(mEnv, MultiSelectManager.MODE_SINGLE);
        mManager = new MultiSelectManager(mEnv, mAdapter, MultiSelectManager.MODE_SINGLE);
        mManager.addCallback(mCallback);
        mManager.addCallback(mCallback);
        longPress(20);
        longPress(20);
        tap(13);
        tap(13);
@@ -172,7 +175,7 @@ public class MultiSelectManagerTest extends AndroidTestCase {
    }
    }


    public void testSingleSelectMode_ShiftTap() {
    public void testSingleSelectMode_ShiftTap() {
        mManager = new MultiSelectManager(mEnv, MultiSelectManager.MODE_SINGLE);
        mManager = new MultiSelectManager(mEnv, mAdapter, MultiSelectManager.MODE_SINGLE);
        mManager.addCallback(mCallback);
        mManager.addCallback(mCallback);
        longPress(13);
        longPress(13);
        shiftTap(20);
        shiftTap(20);
@@ -180,7 +183,7 @@ public class MultiSelectManagerTest extends AndroidTestCase {
    }
    }


    public void testSingleSelectMode_ShiftDoesNotExtendSelection() {
    public void testSingleSelectMode_ShiftDoesNotExtendSelection() {
        mManager = new MultiSelectManager(mEnv, MultiSelectManager.MODE_SINGLE);
        mManager = new MultiSelectManager(mEnv, mAdapter, MultiSelectManager.MODE_SINGLE);
        mManager.addCallback(mCallback);
        mManager.addCallback(mCallback);
        longPress(20);
        longPress(20);
        keyToPosition(22, true);
        keyToPosition(22, true);
Loading