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

Commit 82d71afd authored by Garfield Tan's avatar Garfield Tan
Browse files

Fix 2 bugs that fail to update item listener right.

* Reset SelectionMetadata on model update.
* Stop updating item listener if an item is selected when we try to add
  it to provisional selection.
* Add tests to ensure consistency between SelectionManager and
  ItemCallbacks

Test: Manual tests & auto tests.
Bug: 37083128
Change-Id: I15149c71fa56f38ae5867a13d9026146a969c525
(cherry picked from commit b8874fd0)
parent 0b3f300a
Loading
Loading
Loading
Loading
+18 −2
Original line number Original line Diff line number Diff line
@@ -426,7 +426,12 @@ public final class SelectionManager {
    }
    }


    private void notifyDataChanged() {
    private void notifyDataChanged() {
        int lastListener = mItemCallbacks.size() - 1;
        final int lastListener = mItemCallbacks.size() - 1;

        for (int i = lastListener; i >= 0; i--) {
            mItemCallbacks.get(i).onSelectionReset();
        }

        for (String id : mSelection) {
        for (String id : mSelection) {
            if (!canSetState(id, true)) {
            if (!canSetState(id, true)) {
                attemptDeselect(id);
                attemptDeselect(id);
@@ -513,21 +518,32 @@ public final class SelectionManager {
            if (id == null) {
            if (id == null) {
                continue;
                continue;
            }
            }

            boolean changedState = false;
            if (selected) {
            if (selected) {
                boolean canSelect = canSetState(id, true);
                boolean canSelect = canSetState(id, true);
                if (canSelect && !mSelection.mSelection.contains(id)) {
                if (canSelect && !mSelection.mSelection.contains(id)) {
                    mSelection.mProvisionalSelection.add(id);
                    mSelection.mProvisionalSelection.add(id);
                    changedState = true;
                }
                }
            } else {
            } else {
                mSelection.mProvisionalSelection.remove(id);
                mSelection.mProvisionalSelection.remove(id);
                changedState = true;
            }
            }

            // Only notify item callbacks when something's state is actually changed in provisional
            // selection.
            if (changedState) {
                notifyItemStateChanged(id, selected);
                notifyItemStateChanged(id, selected);
            }
            }
        }
        notifySelectionChanged();
        notifySelectionChanged();
    }
    }


    public interface ItemCallback {
    public interface ItemCallback {
        void onItemStateChanged(String id, boolean selected);
        void onItemStateChanged(String id, boolean selected);

        void onSelectionReset();
    }
    }


    public interface Callback {
    public interface Callback {
+10 −9
Original line number Original line Diff line number Diff line
@@ -58,15 +58,6 @@ public class SelectionMetadata
        mDocFinder = docFinder;
        mDocFinder = docFinder;
    }
    }


    public void reset(){
        mFileCount = 0;
        mDirectoryCount = 0;
        mPartialCount = 0;
        mWritableDirectoryCount = 0;
        mNoDeleteCount = 0;
        mNoRenameCount = 0;
    }

    @Override
    @Override
    public void onItemStateChanged(String modelId, boolean selected) {
    public void onItemStateChanged(String modelId, boolean selected) {
        final Cursor cursor = mDocFinder.apply(modelId);
        final Cursor cursor = mDocFinder.apply(modelId);
@@ -111,6 +102,16 @@ public class SelectionMetadata
        }
        }
    }
    }


    @Override
    public void onSelectionReset() {
        mFileCount = 0;
        mDirectoryCount = 0;
        mPartialCount = 0;
        mWritableDirectoryCount = 0;
        mNoDeleteCount = 0;
        mNoRenameCount = 0;
    }

    @Override
    @Override
    public boolean containsDirectories() {
    public boolean containsDirectories() {
        return mDirectoryCount > 0;
        return mDirectoryCount > 0;
+2 −0
Original line number Original line Diff line number Diff line
@@ -73,5 +73,7 @@ public class TestDocumentsAdapter extends DocumentsAdapter {


    public void updateTestModelIds(List<String> modelIds) {
    public void updateTestModelIds(List<String> modelIds) {
        mModelIds = modelIds;
        mModelIds = modelIds;

        notifyDataSetChanged();
    }
    }
}
}
+22 −1
Original line number Original line Diff line number Diff line
@@ -24,14 +24,25 @@ import com.android.documentsui.selection.SelectionManager;
import com.android.documentsui.selection.Selection;
import com.android.documentsui.selection.Selection;


/**
/**
 * Helper class for making assertions against the state of a MultiSelectManager instance.
 * Helper class for making assertions against the state of a {@link SelectionManager} instance and
 * the consistency of states between {@link SelectionManager} and
 * {@link SelectionManager.ItemCallback}.
 */
 */
public final class SelectionProbe {
public final class SelectionProbe {


    private final SelectionManager mMgr;
    private final SelectionManager mMgr;
    private final TestItemSelectionListener mTestCallback;


    public SelectionProbe(SelectionManager mgr) {
    public SelectionProbe(SelectionManager mgr) {
        mMgr = mgr;
        mMgr = mgr;
        mTestCallback = new TestItemSelectionListener();

        mMgr.addItemCallback(mTestCallback);
    }

    public SelectionProbe(SelectionManager mgr, TestItemSelectionListener testCallback) {
        mMgr = mgr;
        mTestCallback = testCallback;
    }
    }


    public void assertRangeSelected(int begin, int end) {
    public void assertRangeSelected(int begin, int end) {
@@ -54,15 +65,21 @@ public final class SelectionProbe {
    public void assertSelectionSize(int expected) {
    public void assertSelectionSize(int expected) {
        Selection selection = mMgr.getSelection();
        Selection selection = mMgr.getSelection();
        assertEquals(selection.toString(), expected, selection.size());
        assertEquals(selection.toString(), expected, selection.size());

        mTestCallback.assertSelectionSize(expected);
    }
    }


    public void assertNoSelection() {
    public void assertNoSelection() {
        assertSelectionSize(0);
        assertSelectionSize(0);

        mTestCallback.assertNoSelection();
    }
    }


    public void assertSelection(int... ids) {
    public void assertSelection(int... ids) {
        assertSelected(ids);
        assertSelected(ids);
        assertEquals(ids.length, mMgr.getSelection().size());
        assertEquals(ids.length, mMgr.getSelection().size());

        mTestCallback.assertSelectionSize(ids.length);
    }
    }


    public void assertSelected(int... ids) {
    public void assertSelected(int... ids) {
@@ -70,6 +87,8 @@ public final class SelectionProbe {
        for (int id : ids) {
        for (int id : ids) {
            String sid = String.valueOf(id);
            String sid = String.valueOf(id);
            assertTrue(sid + " is not in selection " + sel, sel.contains(sid));
            assertTrue(sid + " is not in selection " + sel, sel.contains(sid));

            mTestCallback.assertSelected(sid);
        }
        }
    }
    }


@@ -78,6 +97,8 @@ public final class SelectionProbe {
        for (int id : ids) {
        for (int id : ids) {
            String sid = String.valueOf(id);
            String sid = String.valueOf(id);
            assertFalse(sid + " is in selection " + sel, sel.contains(sid));
            assertFalse(sid + " is in selection " + sel, sel.contains(sid));

            mTestCallback.assertNotSelected(sid);
        }
        }
    }
    }


+61 −0
Original line number Original line Diff line number Diff line
/*
 * Copyright (C) 2017 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.documentsui.selection;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;

import java.util.HashSet;
import java.util.Set;

public class TestItemSelectionListener implements SelectionManager.ItemCallback {

    private final Set<String> mSelected = new HashSet<>();

    @Override
    public void onItemStateChanged(String id, boolean selected) {
        if (selected) {
            assertNotSelected(id);
            mSelected.add(id);
        } else {
            assertSelected(id);
            mSelected.remove(id);
        }
    }

    @Override
    public void onSelectionReset() {
        mSelected.clear();
    }

    void assertNoSelection() {
        assertTrue(mSelected.isEmpty());
    }

    void assertSelectionSize(int expected) {
        assertEquals(expected, mSelected.size());
    }

    void assertSelected(String id) {
        assertTrue(id + " is not selected.", mSelected.contains(id));
    }

    void assertNotSelected(String id) {
        assertFalse(id + " is already selected", mSelected.contains(id));
    }
}
Loading