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

Commit e45d1f0e 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 e910403e

src/com/android/documentsui/dirlist/DirectoryFragment.java

+1 −0
Original line number Diff line number Diff line
@@ -332,6 +332,7 @@ public class DirectoryFragment extends Fragment implements DocumentsAdapter.Envi 
        // into the selection manager.

        mSelectionManager = new MultiSelectManager(

                mRecView,

                mAdapter,

                state.allowMultiple

                    ? MultiSelectManager.MODE_MULTIPLE

                    : MultiSelectManager.MODE_SINGLE);

src/com/android/documentsui/dirlist/Model.java

+8 −2
Original line number Diff line number Diff line
@@ -414,8 +414,14 @@ public class Model implements SiblingProvider { 


        @Override

        protected Void doInBackground(Selection... selected) {

            List<DocumentInfo> toDelete = getDocuments(selected[0]);

            mHadTrouble = false;

            List<DocumentInfo> toDelete = null;

            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) {

                if (!doc.isDeleteSupported()) {

src/com/android/documentsui/dirlist/MultiSelectManager.java

+26 −57
Original line number Diff line number Diff line
@@ -68,21 +68,23 @@ public final class MultiSelectManager implements View.OnKeyListener { 


    private final Selection mSelection = new Selection();



    private Range mRanger;

    private SelectionEnvironment mEnvironment;



    private final SelectionEnvironment mEnvironment;

    private final DocumentsAdapter mAdapter;

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



    private Range mRanger;

    private boolean mSingleSelect;



    @Nullable private BandController mBandManager;





    /**

     * @param recyclerView

     * @param mode Selection mode

     */

    public MultiSelectManager(final RecyclerView recyclerView, int mode) {

        this(new RuntimeSelectionEnvironment(recyclerView), mode);

    public MultiSelectManager(

            final RecyclerView recyclerView, DocumentsAdapter adapter, int mode) {

        this(new RuntimeSelectionEnvironment(recyclerView), adapter, mode);



        if (mode == MODE_MULTIPLE) {

            mBandManager = new BandController();
@@ -133,11 +135,12 @@ public final class MultiSelectManager implements View.OnKeyListener { 
     * @hide

     */

    @VisibleForTesting

    MultiSelectManager(SelectionEnvironment environment, int mode) {

    MultiSelectManager(SelectionEnvironment environment, DocumentsAdapter adapter, int mode) {

        mEnvironment = checkNotNull(environment, "'environment' cannot be null.");

        mAdapter = checkNotNull(adapter, "'adapter' cannot be null.");

        mSingleSelect = mode == MODE_SINGLE;



        mEnvironment.registerDataObserver(

        mAdapter.registerAdapterDataObserver(

                new RecyclerView.AdapterDataObserver() {



                    private List<String> mModelIds;
@@ -146,7 +149,7 @@ public final class MultiSelectManager implements View.OnKeyListener { 
                    public void onChanged() {

                        // TODO: This is causing b/22765812

                        mSelection.clear();

                        mModelIds = mEnvironment.getModelIds();

                        mModelIds = mAdapter.getModelIds();

                    }



                    @Override
@@ -336,7 +339,7 @@ public final class MultiSelectManager implements View.OnKeyListener { 
            if (DEBUG) Log.d(TAG, "Ignoring toggle for element with no position.");

            return;

        }

        String id = mEnvironment.getModelIdFromAdapterPosition(position);

        String id = mAdapter.getModelId(position);

        if (id != null) {

            toggleSelection(id);

        }
@@ -388,7 +391,7 @@ public final class MultiSelectManager implements View.OnKeyListener { 
            return;

        }



        if (mSelection.contains(mEnvironment.getModelIdFromAdapterPosition(position))) {

        if (mSelection.contains(mAdapter.getModelId(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) {

        checkState(end >= begin);

        for (int i = begin; i <= end; i++) {

            String id = mEnvironment.getModelIdFromAdapterPosition(i);

            String id = mAdapter.getModelId(i);

            if (id == null) {

                continue;

            }
@@ -473,7 +476,7 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        for (int i = lastListener; i > -1; i--) {

            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 getVisibleChildCount();

        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. */
@@ -830,11 +823,9 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        private final Drawable mBand;



        private boolean mIsOverlayShown = false;

        private DocumentsAdapter mAdapter;



        RuntimeSelectionEnvironment(RecyclerView rv) {

            mView = rv;

            mAdapter = (DocumentsAdapter) rv.getAdapter();

        RuntimeSelectionEnvironment(RecyclerView view) {

            mView = view;

            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));

        }



        @Override

        public @Nullable String getModelIdFromAdapterPosition(int position) {

            return mAdapter.getModelId(position);

        }



        @Override

        public void addOnScrollListener(RecyclerView.OnScrollListener 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 {
@@ -1049,7 +1015,7 @@ public final class MultiSelectManager implements View.OnKeyListener { 
            mModelBuilder = new Runnable() {

                @Override

                public void run() {

                    mModel = new GridModel(mEnvironment);

                    mModel = new GridModel(mEnvironment, mAdapter);

                    mModel.addOnSelectionChangedListener(BandController.this);

                }

            };
@@ -1100,7 +1066,7 @@ public final class MultiSelectManager implements View.OnKeyListener { 
            return !isActive()

                    && e.isMouseEvent()  // a mouse

                    && e.isActionDown()  // the initial button press

                    && mEnvironment.getItemCount() > 0

                    && mAdapter.getItemCount() > 0

                    && e.getItemPosition() == RecyclerView.NO_ID;  // in empty space

        }
@@ -1177,7 +1143,7 @@ public final class MultiSelectManager implements View.OnKeyListener { 
            mModel.endSelection();

            int firstSelected = mModel.getPositionNearestOrigin();

            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

                    // where the mouse-up occurred.

                    setSelectionRangeBegin(firstSelected);
@@ -1339,6 +1305,8 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        private static final int LOWER_RIGHT = LOWER | RIGHT;



        private final SelectionEnvironment mHelper;

        private final DocumentsAdapter mAdapter;



        private final List<OnSelectionChangedListener> mOnSelectionChangedListeners =

                new ArrayList<>();
@@ -1376,8 +1344,9 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        // should expand from when Shift+click is used.

        private int mPositionNearestOrigin = NOT_SET;



        GridModel(SelectionEnvironment helper) {

        GridModel(SelectionEnvironment helper, DocumentsAdapter adapter) {

            mHelper = helper;

            mAdapter = adapter;

            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.

                    int position = items.get(items.keyAt(row), NOT_SET);

                    if (position != NOT_SET) {

                        String id = mHelper.getModelIdFromAdapterPosition(position);

                        String id = mAdapter.getModelId(position);

                        if (id != null) {

                            // 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.
@@ -1989,7 +1958,7 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        if (keyCode == KeyEvent.KEYCODE_MOVE_HOME) {

            position = 0;

        } else if (keyCode == KeyEvent.KEYCODE_MOVE_END) {

            position = mEnvironment.getItemCount() - 1;

            position = mAdapter.getItemCount() - 1;

        } else {

            // Find a navigation target based on the arrow key that the user pressed.  Ignore

            // navigation targets that aren't items in the recycler view.

tests/src/com/android/documentsui/dirlist/ModelBackedDocumentsAdapterTest.java

+2 −1
Original line number Diff line number Diff line
@@ -84,7 +84,8 @@ public class ModelBackedDocumentsAdapterTest extends AndroidTestCase { 
    // Tests that the items can be hidden and unhidden.

    public void testUnhide_PreservesOrder() {

        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);



        // Finally ensure the restored items are in the original order

tests/src/com/android/documentsui/dirlist/MultiSelectManagerTest.java

+7 −4
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import android.util.SparseBooleanArray; 


import com.android.documentsui.TestInputEvent;

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



import com.google.common.collect.Lists;



import java.util.ArrayList;
@@ -44,11 +45,13 @@ public class MultiSelectManagerTest extends AndroidTestCase { 
    private MultiSelectManager mManager;

    private TestCallback mCallback;

    private TestSelectionEnvironment mEnv;

    private TestDocumentsAdapter mAdapter;



    public void setUp() throws Exception {

        mCallback = new TestCallback();

        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);

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



    public void testSingleSelectMode() {

        mManager = new MultiSelectManager(mEnv, MultiSelectManager.MODE_SINGLE);

        mManager = new MultiSelectManager(mEnv, mAdapter, MultiSelectManager.MODE_SINGLE);

        mManager.addCallback(mCallback);

        longPress(20);

        tap(13);
@@ -172,7 +175,7 @@ public class MultiSelectManagerTest extends AndroidTestCase { 
    }



    public void testSingleSelectMode_ShiftTap() {

        mManager = new MultiSelectManager(mEnv, MultiSelectManager.MODE_SINGLE);

        mManager = new MultiSelectManager(mEnv, mAdapter, MultiSelectManager.MODE_SINGLE);

        mManager.addCallback(mCallback);

        longPress(13);

        shiftTap(20);
@@ -180,7 +183,7 @@ public class MultiSelectManagerTest extends AndroidTestCase { 
    }



    public void testSingleSelectMode_ShiftDoesNotExtendSelection() {

        mManager = new MultiSelectManager(mEnv, MultiSelectManager.MODE_SINGLE);

        mManager = new MultiSelectManager(mEnv, mAdapter, MultiSelectManager.MODE_SINGLE);

        mManager.addCallback(mCallback);

        longPress(20);

        keyToPosition(22, true);