OSDN Git Service

Fix event relay to correctly dispatch events.
authorSteve McKay <smckay@google.com>
Tue, 5 Jan 2016 20:53:35 +0000 (12:53 -0800)
committerSteve McKay <smckay@google.com>
Tue, 5 Jan 2016 22:13:57 +0000 (14:13 -0800)
This fixes:
- UI to show selection which was broken in ag/838866
- Delete undo, which throws IOB exception
  when undoing a a full delete of all entries.

Change-Id: Idbb43510974e130d283313602a71ac15ad10aadf

packages/DocumentsUI/src/com/android/documentsui/dirlist/DocumentsAdapter.java
packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java
packages/DocumentsUI/src/com/android/documentsui/dirlist/ModelBackedDocumentsAdapter.java
packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java
packages/DocumentsUI/src/com/android/documentsui/dirlist/SectionBreakDocumentsAdapterWrapper.java
packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelBackedDocumentsAdapterTest.java
packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestModel.java

index 2001b29..43c2f63 100644 (file)
@@ -58,7 +58,7 @@ abstract class DocumentsAdapter
      * Triggers item-change notifications by stable ID (as opposed to position).
      * Passing an unrecognized ID will result in a warning in logcat, but no other error.
      */
-    abstract void notifyItemSelectionChanged(String id);
+    abstract void onItemSelectionChanged(String id);
 
     /**
      * @return The model ID of the item at the given adapter position.
index 864f405..4bef51b 100644 (file)
@@ -86,8 +86,19 @@ public class Model implements SiblingProvider {
     private static String createModelId(Cursor c) {
         // TODO: Maybe more efficient to use just the document ID, in cases where there is only one
         // authority (which should be the majority of cases).
-        return getCursorString(c, RootCursorWrapper.COLUMN_AUTHORITY) +
-                "|" + getCursorString(c, Document.COLUMN_DOCUMENT_ID);
+        return createModelId(
+                getCursorString(c, RootCursorWrapper.COLUMN_AUTHORITY),
+                getCursorString(c, Document.COLUMN_DOCUMENT_ID));
+    }
+
+    /**
+     * Generates a Model ID for a cursor entry that refers to a document. The Model ID is a unique
+     * string that can be used to identify the document referred to by the cursor.
+     *
+     * @param c A cursor that refers to a document.
+     */
+    static String createModelId(String authority, String docId) {
+        return authority + "|" + docId;
     }
 
     private void notifyUpdateListeners() {
index bb0d729..68756a3 100644 (file)
@@ -25,7 +25,6 @@ import static com.android.documentsui.model.DocumentInfo.getCursorString;
 
 import android.database.Cursor;
 import android.provider.DocumentsContract.Document;
-import android.support.v7.widget.GridLayoutManager;
 import android.util.Log;
 import android.util.SparseArray;
 import android.view.ViewGroup;
@@ -187,9 +186,17 @@ final class ModelBackedDocumentsAdapter extends DocumentsAdapter {
     @Override
     public void unhide(SparseArray<String> ids) {
         if (DEBUG) Log.d(TAG, "Un-iding ids: " + ids);
-        // Proceed backwards through the list of items, because each addition causes the
-        // positions of all subsequent items to change.
-        for (int i = ids.size() - 1; i >= 0; --i) {
+
+        // An ArrayList can shrink at runtime...and in fact
+        // it does when we clear it completely.
+        // This means we can't call add(pos, id) without
+        // first checking the list size.
+        List<String> oldIds = mModelIds;
+        mModelIds = new ArrayList<>(oldIds.size() + ids.size());
+        mModelIds.addAll(oldIds);
+
+        // Finally insert the unhidden items.
+        for (int i = 0; i < ids.size(); i++) {
             int pos = ids.keyAt(i);
             String id = ids.get(pos);
             mHiddenIds.remove(id);
@@ -211,7 +218,7 @@ final class ModelBackedDocumentsAdapter extends DocumentsAdapter {
     }
 
     @Override
-    public void notifyItemSelectionChanged(String id) {
+    public void onItemSelectionChanged(String id) {
         int position = mModelIds.indexOf(id);
 
         if (position >= 0) {
index 4b3bf1e..125352f 100644 (file)
@@ -976,7 +976,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
 
         @Override
         public void notifyItemChanged(String id) {
-            mAdapter.notifyItemSelectionChanged(id);
+            mAdapter.onItemSelectionChanged(id);
         }
 
         @Override
index ae6ada9..b4782f0 100644 (file)
@@ -43,40 +43,9 @@ final class SectionBreakDocumentsAdapterWrapper extends DocumentsAdapter {
         mEnv = environment;
         mDelegate = delegate;
 
-        // Events and information flows two ways between recycler view and adapter.
-        // So we need to listen to events on our delegate and forward them
-        // to our listeners with a corrected position.
-        AdapterDataObserver adapterDataObserver = new AdapterDataObserver() {
-            public void onChanged() {
-                throw new UnsupportedOperationException();
-            }
-
-            public void onItemRangeChanged(int positionStart, int itemCount) {
-                checkArgument(itemCount == 1);
-            }
-
-            public void onItemRangeInserted(int positionStart, int itemCount) {
-                checkArgument(itemCount == 1);
-                if (positionStart < mBreakPosition) {
-                    mBreakPosition++;
-                }
-                notifyItemRangeInserted(toViewPosition(positionStart), itemCount);
-            }
-
-            public void onItemRangeRemoved(int positionStart, int itemCount) {
-                checkArgument(itemCount == 1);
-                if (positionStart < mBreakPosition) {
-                    mBreakPosition--;
-                }
-                notifyItemRangeRemoved(toViewPosition(positionStart), itemCount);
-            }
-
-            public void onItemRangeMoved(int fromPosition, int toPosition, int itemCount) {
-                throw new UnsupportedOperationException();
-            }
-        };
-
-        mDelegate.registerAdapterDataObserver(adapterDataObserver);
+        // Relay events published by our delegate to our listeners (presumably RecyclerView)
+        // with adjusted positions.
+        mDelegate.registerAdapterDataObserver(new EventRelay());
     }
 
     public GridLayoutManager.SpanSizeLookup createSpanSizeLookup() {
@@ -205,7 +174,44 @@ final class SectionBreakDocumentsAdapterWrapper extends DocumentsAdapter {
     }
 
     @Override
-    public void notifyItemSelectionChanged(String id) {
-        mDelegate.notifyItemSelectionChanged(id);
+    public void onItemSelectionChanged(String id) {
+        mDelegate.onItemSelectionChanged(id);
+    }
+
+    // Listener we add to our delegate. This allows us to relay events published
+    // by the delegate to our listeners (presumably RecyclerView) with adjusted positions.
+    private final class EventRelay extends AdapterDataObserver {
+        public void onChanged() {
+            throw new UnsupportedOperationException();
+        }
+
+        public void onItemRangeChanged(int positionStart, int itemCount) {
+            throw new UnsupportedOperationException();
+        }
+
+        public void onItemRangeChanged(int positionStart, int itemCount, Object payload) {
+            checkArgument(itemCount == 1);
+            notifyItemRangeChanged(toViewPosition(positionStart), itemCount, payload);
+        }
+
+        public void onItemRangeInserted(int positionStart, int itemCount) {
+            checkArgument(itemCount == 1);
+            if (positionStart < mBreakPosition) {
+                mBreakPosition++;
+            }
+            notifyItemRangeInserted(toViewPosition(positionStart), itemCount);
+        }
+
+        public void onItemRangeRemoved(int positionStart, int itemCount) {
+            checkArgument(itemCount == 1);
+            if (positionStart < mBreakPosition) {
+                mBreakPosition--;
+            }
+            notifyItemRangeRemoved(toViewPosition(positionStart), itemCount);
+        }
+
+        public void onItemRangeMoved(int fromPosition, int toPosition, int itemCount) {
+            throw new UnsupportedOperationException();
+        }
     }
 }
index 92668a8..e2a9094 100644 (file)
@@ -21,6 +21,7 @@ import android.database.Cursor;
 import android.support.v7.widget.RecyclerView;
 import android.test.AndroidTestCase;
 import android.test.suitebuilder.annotation.SmallTest;
+import android.util.SparseArray;
 import android.view.ViewGroup;
 
 import com.android.documentsui.State;
@@ -44,32 +45,53 @@ public class ModelBackedDocumentsAdapterTest extends AndroidTestCase {
             "%$%VD"
     };
 
-    private TestModel model;
-    private ModelBackedDocumentsAdapter adapter;
+    private TestModel mModel;
+    private ModelBackedDocumentsAdapter mAdapter;
 
     public void setUp() {
 
         final Context testContext = TestContext.createStorageTestContext(getContext(), AUTHORITY);
-        model = new TestModel(testContext, AUTHORITY);
-        model.update(NAMES);
+        mModel = new TestModel(testContext, AUTHORITY);
+        mModel.update(NAMES);
 
         DocumentsAdapter.Environment env = new TestEnvironment(testContext);
 
-        adapter = new ModelBackedDocumentsAdapter(
+        mAdapter = new ModelBackedDocumentsAdapter(
                 env, new IconHelper(testContext, State.MODE_GRID));
-        adapter.onModelUpdate(model);
+        mAdapter.onModelUpdate(mModel);
     }
 
     // Tests that the item count is correct.
     public void testItemCount() {
-        assertEquals(model.getItemCount(), adapter.getItemCount());
+        assertEquals(mModel.getItemCount(), mAdapter.getItemCount());
     }
 
     // Tests that the item count is correct.
     public void testHide_ItemCount() {
-        List<String> ids = model.getModelIds();
-        adapter.hide(ids.get(0), ids.get(1));
-        assertEquals(model.getItemCount() - 2, adapter.getItemCount());
+        List<String> ids = mModel.getModelIds();
+        mAdapter.hide(ids.get(0), ids.get(1));
+        assertEquals(mModel.getItemCount() - 2, mAdapter.getItemCount());
+    }
+
+    // Tests that the items can be hidden and unhidden.
+    public void testUnhide_ItemCount() {
+        List<String> ids = mModel.getModelIds();
+        SparseArray<String> hidden = mAdapter.hide(ids.toArray(new String[ids.size()]));
+        mAdapter.unhide(hidden);
+        assertEquals(mModel.getItemCount(), mAdapter.getItemCount());
+    }
+
+    // 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()]));
+        mAdapter.unhide(hidden);
+
+        // Finally ensure the restored items are in the original order
+        // by checking them against the model.
+        for (int i = 0; i < mAdapter.getItemCount(); i++) {
+            assertEquals(mModel.idForPosition(i), mAdapter.getModelId(i));
+        }
     }
 
     private final class TestEnvironment implements DocumentsAdapter.Environment {
@@ -94,7 +116,7 @@ public class ModelBackedDocumentsAdapterTest extends AndroidTestCase {
 
         @Override
         public Model getModel() {
-            return model;
+            return mModel;
         }
 
         @Override
index f861c73..3a537a6 100644 (file)
@@ -70,6 +70,12 @@ public class TestModel extends Model {
         update(r);
     }
 
+    // Note that model id includes authority qualifier and is distinct
+    // WRT documentId because of this.
+    String idForPosition(int p) {
+        return createModelId(mAuthority, Integer.toString(p));
+    }
+
     @Override
     public void delete(Selection selected, DeletionListener listener) {
         for (String id : selected.getAll()) {