Transition selection to use Model IDs.
This CL transitions the MultiSelectManager to (mostly) use Model IDs.
- Add the ability to retrieve all model IDs for the current directory,
from the model.
- Add a map in the DocumentsAdapter that maps from adapter position to
model ID.
- Make the adapter listen for model updates, and update its internal map
of positions to model IDs appropriately.
- Use the aforementioned map when binding ViewHolders.
- Get unit tests to compile; get as many tests passing as possible at
this point. Tests related to deleting things won't work right now.
Still to do:
- Add code to the adapter to sort and group items. After this is done,
SortingCursorWrapper will no longer be needed.
- Add code to the adapter to deal with item addition/removal. After
this is done, the pending-deletion code in the model can be removed.
- Rationalize position-based vs model-based selection. Some code in the
MSM (in particular, code dealing with range selection) is still
position-based. It's becoming clear that it doesn't make sense for
range selection to be ID-based, since "range" is a concept that only
makes sense in the context of items that are positioned in the UI.
Will need to iterate more on the position-based code to make sure it
makes sense.
BUG=26024369
Change-Id: I767cde2d888c101aaf83b59155b14634a236164b
diff --git a/src/com/android/documentsui/dirlist/Model.java b/src/com/android/documentsui/dirlist/Model.java
index 9d0bceb..49691d4 100644
--- a/src/com/android/documentsui/dirlist/Model.java
+++ b/src/com/android/documentsui/dirlist/Model.java
@@ -34,7 +34,7 @@
import android.support.annotation.VisibleForTesting;
import android.support.v7.widget.RecyclerView;
import android.util.Log;
-import android.util.SparseBooleanArray;
+import android.util.SparseArray;
import com.android.documentsui.BaseActivity.DocumentContext;
import com.android.documentsui.DirectoryResult;
@@ -45,9 +45,10 @@
import com.android.internal.annotations.GuardedBy;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
/**
* The data model for the current loaded directory.
@@ -62,8 +63,8 @@
@GuardedBy("mPendingDelete")
private Boolean mPendingDelete = false;
@GuardedBy("mPendingDelete")
- private SparseBooleanArray mMarkedForDeletion = new SparseBooleanArray();
- private Model.UpdateListener mUpdateListener;
+ private Set<String> mMarkedForDeletion = new HashSet<>();
+ private List<UpdateListener> mUpdateListeners = new ArrayList<>();
@Nullable private Cursor mCursor;
@Nullable String info;
@Nullable String error;
@@ -74,6 +75,37 @@
mViewAdapter = viewAdapter;
}
+ /**
+ * 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.
+ */
+ public static String createId(Cursor c) {
+ return getCursorString(c, RootCursorWrapper.COLUMN_AUTHORITY) +
+ "|" + getCursorString(c, Document.COLUMN_DOCUMENT_ID);
+ }
+
+ /**
+ * @return Model IDs for all known items in the model. Note that this will include items
+ * pending deletion.
+ */
+ public Set<String> getIds() {
+ return mPositions.keySet();
+ }
+
+ private void notifyUpdateListeners() {
+ for (UpdateListener listener: mUpdateListeners) {
+ listener.onModelUpdate(this);
+ }
+ }
+
+ private void notifyUpdateListeners(Exception e) {
+ for (UpdateListener listener: mUpdateListeners) {
+ listener.onModelUpdateFailed(e);
+ }
+ }
+
void update(DirectoryResult result) {
if (DEBUG) Log.i(TAG, "Updating model with new result set.");
@@ -83,13 +115,13 @@
info = null;
error = null;
mIsLoading = false;
- mUpdateListener.onModelUpdate(this);
+ notifyUpdateListeners();
return;
}
if (result.exception != null) {
Log.e(TAG, "Error while loading directory contents", result.exception);
- mUpdateListener.onModelUpdateFailed(result.exception);
+ notifyUpdateListeners(result.exception);
return;
}
@@ -105,9 +137,10 @@
mIsLoading = extras.getBoolean(DocumentsContract.EXTRA_LOADING, false);
}
- mUpdateListener.onModelUpdate(this);
+ notifyUpdateListeners();
}
+ @VisibleForTesting
int getItemCount() {
synchronized(mPendingDelete) {
return mCursorCount - mMarkedForDeletion.size();
@@ -122,10 +155,7 @@
mCursor.moveToPosition(-1);
for (int pos = 0; pos < mCursorCount; ++pos) {
mCursor.moveToNext();
- // TODO(stable-id): factor the model ID construction code.
- String modelId = getCursorString(mCursor, RootCursorWrapper.COLUMN_AUTHORITY) +
- "|" + getCursorString(mCursor, Document.COLUMN_DOCUMENT_ID);
- mPositions.put(modelId, pos);
+ mPositions.put(Model.createId(mCursor), pos);
}
}
@@ -138,36 +168,6 @@
return null;
}
- Cursor getItem(int position) {
- synchronized(mPendingDelete) {
- // Items marked for deletion are masked out of the UI. To do this, for every marked
- // item whose position is less than the requested item position, advance the requested
- // position by 1.
- final int originalPos = position;
- final int size = mMarkedForDeletion.size();
- for (int i = 0; i < size; ++i) {
- // It'd be more concise, but less efficient, to iterate over positions while calling
- // mMarkedForDeletion.get. Instead, iterate over deleted entries.
- if (mMarkedForDeletion.keyAt(i) <= position && mMarkedForDeletion.valueAt(i)) {
- ++position;
- }
- }
-
- if (DEBUG && position != originalPos) {
- Log.d(TAG, "Item position adjusted for deletion. Original: " + originalPos
- + " Adjusted: " + position);
- }
-
- if (position >= mCursorCount) {
- throw new IndexOutOfBoundsException("Attempt to retrieve " + position + " of " +
- mCursorCount + " items");
- }
-
- mCursor.moveToPosition(position);
- return mCursor;
- }
- }
-
boolean isEmpty() {
return mCursorCount == 0;
}
@@ -180,8 +180,8 @@
final int size = (items != null) ? items.size() : 0;
final List<DocumentInfo> docs = new ArrayList<>(size);
- for (int i = 0; i < size; i++) {
- final Cursor cursor = getItem(items.get(i));
+ for (String modelId: items.getAll()) {
+ final Cursor cursor = getItem(modelId);
checkNotNull(cursor, "Cursor cannot be null.");
final DocumentInfo doc = DocumentInfo.fromDirectoryCursor(cursor);
docs.add(doc);
@@ -198,13 +198,14 @@
}
List<DocumentInfo> getDocumentsMarkedForDeletion() {
+ // TODO(stable-id): This could be just a plain old selection now.
synchronized (mPendingDelete) {
final int size = mMarkedForDeletion.size();
List<DocumentInfo> docs = new ArrayList<>(size);
- for (int i = 0; i < size; ++i) {
- final int position = mMarkedForDeletion.keyAt(i);
- checkState(position < mCursorCount);
+ for (String id: mMarkedForDeletion) {
+ Integer position = mPositions.get(id);
+ checkState(position != null);
mCursor.moveToPosition(position);
final DocumentInfo doc = DocumentInfo.fromDirectoryCursor(mCursor);
docs.add(doc);
@@ -228,15 +229,14 @@
// There should never be more to delete than what exists.
checkState(mCursorCount >= selected.size());
- int[] positions = selected.getAll();
- Arrays.sort(positions);
-
- // Walk backwards through the set, since we're removing positions.
- // Otherwise, positions would change after the first modification.
- for (int p = positions.length - 1; p >= 0; p--) {
- mMarkedForDeletion.append(positions[p], true);
- mViewAdapter.notifyItemRemoved(positions[p]);
- if (DEBUG) Log.d(TAG, "Scheduled " + positions[p] + " for delete.");
+ // Adapter notifications must be sent in reverse order of adapter position. This is
+ // because each removal causes subsequent item adapter positions to change.
+ SparseArray<String> ids = new SparseArray<>();
+ for (int i = ids.size() - 1; i >= 0; i--) {
+ int pos = ids.keyAt(i);
+ mMarkedForDeletion.add(ids.get(pos));
+ mViewAdapter.notifyItemRemoved(pos);
+ if (DEBUG) Log.d(TAG, "Scheduled " + pos + " for delete.");
}
}
}
@@ -249,11 +249,11 @@
synchronized (mPendingDelete) {
// Iterate over deleted items, temporarily marking them false in the deletion list, and
// re-adding them to the UI.
- final int size = mMarkedForDeletion.size();
- for (int i = 0; i < size; ++i) {
- final int position = mMarkedForDeletion.keyAt(i);
- mMarkedForDeletion.put(position, false);
- mViewAdapter.notifyItemInserted(position);
+ for (String id: mMarkedForDeletion) {
+ Integer pos= mPositions.get(id);
+ checkNotNull(pos);
+ mMarkedForDeletion.remove(id);
+ mViewAdapter.notifyItemInserted(pos);
}
resetDeleteData();
}
@@ -359,19 +359,18 @@
}
void addUpdateListener(UpdateListener listener) {
- checkState(mUpdateListener == null);
- mUpdateListener = listener;
+ mUpdateListeners.add(listener);
}
- static class UpdateListener {
+ static interface UpdateListener {
/**
* Called when a successful update has occurred.
*/
- void onModelUpdate(Model model) {}
+ void onModelUpdate(Model model);
/**
* Called when an update has been attempted but failed.
*/
- void onModelUpdateFailed(Exception e) {}
+ void onModelUpdateFailed(Exception e);
}
}