Remove all bad ideas from SelectedConversations

1. Remove references to AbstractActivityController and
   ConversationCursor (yikes!) from the selected conversation set.
2. Create a new interface for delegating all changes to conversation
   state.
3. Consolidate FolderChangesCommitListener with interface created above.

Change-Id: Id1d982a19eb2d26c51aec661cee5dc23ef98c110

9 files changed, 157 insertions, 102 deletions

diff --git a/src/com/android/mail/browse/SelectedConversationsActionMenu.java b/src/com/android/mail/browse/SelectedConversationsActionMenu.java
index 13fd7cb34..294418f55 100644
--- a/src/com/android/mail/browse/SelectedConversationsActionMenu.java
+++ b/src/com/android/mail/browse/SelectedConversationsActionMenu.java
@@ -36,10 +36,11 @@ import com.android.mail.providers.Settings;
 import com.android.mail.providers.UIProvider;
 import com.android.mail.providers.UIProvider.ConversationColumns;
 import com.android.mail.providers.UIProvider.FolderCapabilities;
-import com.android.mail.ui.AbstractActivityController;
 import com.android.mail.ui.AnimatedAdapter;
+import com.android.mail.ui.ControllableActivity;
 import com.android.mail.ui.ConversationSelectionSet;
 import com.android.mail.ui.ConversationSetObserver;
+import com.android.mail.ui.ConversationUpdater;
 import com.android.mail.ui.DestructiveAction;
 import com.android.mail.ui.FoldersSelectionDialog;
 import com.android.mail.ui.RestrictedActivity;
@@ -81,48 +82,28 @@ public class SelectedConversationsActionMenu implements ActionMode.Callback,
     private Menu mMenu;
 
     private AnimatedAdapter mListAdapter;
-    // TODO(viki): Bad idea.  This is a relic of the previous method of having a DestructiveAction.
-    // A better implementation is not to have clients know about destructive actions but rather
-    // request them from the controller directly. Then, you wouldn't need to know when to commit
-    // them.
-    private AbstractActivityController mController;
 
-    private Account mAccount;
+    /** Object that can update conversation state on our behalf. */
+    private final ConversationUpdater mUpdater;
 
-    protected int mCheckedItem = 0;
+    private final Account mAccount;
 
-    private Folder mFolder;
+    private final Folder mFolder;
 
-    private final ConversationCursor mConversationCursor;
-
-    private SwipeableListView mListView;
+    private final SwipeableListView mListView;
 
     public SelectedConversationsActionMenu(RestrictedActivity activity,
-            ConversationSelectionSet selectionSet, AnimatedAdapter adapter,
-            AbstractActivityController controller, Account account,
+            ConversationSelectionSet selectionSet, AnimatedAdapter adapter, Account account,
             Folder folder, SwipeableListView list) {
         mActivity = activity;
         mSelectionSet = selectionSet;
         mListAdapter = adapter;
-        mConversationCursor = (ConversationCursor)adapter.getCursor();
-        mController = controller;
         mAccount = account;
         mFolder = folder;
         mListView = list;
 
         mContext = mActivity.getActivityContext();
-    }
-
-    /**
-     * Registers a destructive action with the controller and returns it.
-     * @param type the resource id of the menu item that corresponds to this action: R.id.delete
-     *  for example.
-     * @return the {@link DestructiveAction} associated with this action.
-     */
-    // TODO(viki): This is a placeholder during the refactoring. Ideally the controller hands
-    // the ID of the action to clients.
-    private final DestructiveAction getAction(int type) {
-        return mController.getBatchDestruction(type);
+        mUpdater = ((ControllableActivity) mActivity).getConversationUpdater();
     }
 
     @Override
@@ -137,10 +118,10 @@ public class SelectedConversationsActionMenu implements ActionMode.Callback,
                 performDestructiveAction(R.id.archive);
                 break;
             case R.id.mute:
-                mListAdapter.delete(conversations, getAction(R.id.mute));
+                mListAdapter.delete(conversations, mUpdater.getBatchAction(R.id.mute));
                 break;
             case R.id.report_spam:
-                mListAdapter.delete(conversations, getAction(R.id.report_spam));
+                mListAdapter.delete(conversations, mUpdater.getBatchAction(R.id.report_spam));
                 break;
             case R.id.read:
                 markConversationsRead(true);
@@ -183,7 +164,7 @@ public class SelectedConversationsActionMenu implements ActionMode.Callback,
                     }
                 }
                 if (!cantMove) {
-                    new FoldersSelectionDialog(mContext, acct, mController,
+                    new FoldersSelectionDialog(mContext, acct, mUpdater,
                             mSelectionSet.values(), true).show();
                 }
                 break;
@@ -226,7 +207,7 @@ public class SelectedConversationsActionMenu implements ActionMode.Callback,
     }
 
     private void performDestructiveAction(final int id) {
-        final DestructiveAction action = getAction(id);
+        final DestructiveAction action = mUpdater.getBatchAction(id);
         final Settings settings = mActivity.getSettings();
         final Collection<Conversation> conversations = mSelectionSet.values();
         final boolean showDialog = (settings != null
@@ -262,17 +243,14 @@ public class SelectedConversationsActionMenu implements ActionMode.Callback,
     }
 
     private void markConversationsRead(boolean read) {
-        final Collection<Conversation> conversations = mSelectionSet.values();
-        mConversationCursor.updateBoolean(mContext, conversations, ConversationColumns.READ, read);
+        mUpdater.updateConversation(mSelectionSet.values(), ConversationColumns.READ, read);
         updateSelection();
     }
 
     private void markConversationsImportant(boolean important) {
-        final Collection<Conversation> conversations = mSelectionSet.values();
         final int priority = important ? UIProvider.ConversationPriority.HIGH
                 : UIProvider.ConversationPriority.LOW;
-        mConversationCursor.updateInt(mContext, conversations, ConversationColumns.PRIORITY,
-                priority);
+        mUpdater.updateConversation(mSelectionSet.values(), ConversationColumns.PRIORITY, priority);
         updateSelection();
     }
 
@@ -282,11 +260,7 @@ public class SelectedConversationsActionMenu implements ActionMode.Callback,
      * stars from all conversations
      */
     private void starConversations(boolean star) {
-        final Collection<Conversation> conversations = mSelectionSet.values();
-        if (conversations.size() > 0) {
-            mConversationCursor.updateBoolean(mContext, conversations, ConversationColumns.STARRED,
-                    star);
-        }
+        mUpdater.updateConversation(mSelectionSet.values(), ConversationColumns.STARRED, star);
         updateSelection();
     }
 
diff --git a/src/com/android/mail/providers/UIProvider.java b/src/com/android/mail/providers/UIProvider.java
index d5ef6317a..c9b1c10f4 100644
--- a/src/com/android/mail/providers/UIProvider.java
+++ b/src/com/android/mail/providers/UIProvider.java
@@ -740,6 +740,9 @@ public class UIProvider {
         public static final int CALENDAR_INVITE = 1<<4;
     }
 
+    /**
+     * Names of columns representing fields in a Conversation.
+     */
     public static final class ConversationColumns {
         public static final String URI = "conversationUri";
         /**
diff --git a/src/com/android/mail/ui/AbstractActivityController.java b/src/com/android/mail/ui/AbstractActivityController.java
index 6de5ce348..0acbc0fdf 100644
--- a/src/com/android/mail/ui/AbstractActivityController.java
+++ b/src/com/android/mail/ui/AbstractActivityController.java
@@ -53,7 +53,6 @@ import android.widget.Toast;
 import com.android.mail.ConversationListContext;
 import com.android.mail.R;
 import com.android.mail.browse.ConversationCursor;
-import com.android.mail.browse.ConversationCursor.ConversationListener;
 import com.android.mail.browse.ConversationPagerController;
 import com.android.mail.browse.SelectedConversationsActionMenu;
 import com.android.mail.compose.ComposeActivity;
@@ -96,8 +95,7 @@ import java.util.TimerTask;
  * In the Gmail codebase, this was called BaseActivityController
  * </p>
  */
-public abstract class AbstractActivityController implements ActivityController,
-        ConversationListener, OnScrollListener {
+public abstract class AbstractActivityController implements ActivityController {
     // Keys for serialization of various information in Bundles.
     /** Tag for {@link #mAccount} */
     private static final String SAVED_ACCOUNT = "saved-account";
@@ -617,12 +615,12 @@ public abstract class AbstractActivityController implements ActivityController,
                 break;
             }
             case R.id.mark_important:
-                updateCurrentConversation(ConversationColumns.PRIORITY,
-                        UIProvider.ConversationPriority.HIGH);
+                updateConversation(Conversation.listOf(mCurrentConversation),
+                        ConversationColumns.PRIORITY, UIProvider.ConversationPriority.HIGH);
                 break;
             case R.id.mark_not_important:
-                updateCurrentConversation(ConversationColumns.PRIORITY,
-                        UIProvider.ConversationPriority.LOW);
+                updateConversation(Conversation.listOf(mCurrentConversation),
+                        ConversationColumns.PRIORITY, UIProvider.ConversationPriority.LOW);
                 break;
             case R.id.mute:
                 requestDelete(target, getAction(R.id.mute, target));
@@ -631,7 +629,10 @@ public abstract class AbstractActivityController implements ActivityController,
                 requestDelete(target, getAction(R.id.report_spam, target));
                 break;
             case R.id.inside_conversation_unread:
-                updateCurrentConversation(ConversationColumns.READ, false);
+                // TODO(viki): This is strange, and potentially incorrect. READ is an int column
+                // in the provider.
+                updateConversation(Conversation.listOf(mCurrentConversation),
+                        ConversationColumns.READ, false);
                 mViewMode.enterConversationListMode();
                 break;
             case android.R.id.home:
@@ -673,31 +674,23 @@ public abstract class AbstractActivityController implements ActivityController,
         return handled;
     }
 
-    /**
-     * Update the specified column name in conversation for a boolean value.
-     * @param columnName
-     * @param value
-     */
-    protected void updateCurrentConversation(String columnName, boolean value) {
-        mConversationListCursor.updateBoolean(mContext, Conversation.listOf(mCurrentConversation),
-                columnName, value);
+    @Override
+    public void updateConversation(Collection <Conversation> target, String columnName,
+            boolean value) {
+        mConversationListCursor.updateBoolean(mContext, target, columnName, value);
         refreshConversationList();
     }
 
-    /**
-     * Update the specified column name in conversation for an integer value.
-     * @param columnName
-     * @param value
-     */
-    protected void updateCurrentConversation(String columnName, int value) {
-        mConversationListCursor.updateInt(mContext, Conversation.listOf(mCurrentConversation),
-                columnName, value);
+    @Override
+    public void updateConversation(Collection <Conversation> target, String columnName, int value) {
+        mConversationListCursor.updateInt(mContext, target, columnName, value);
         refreshConversationList();
     }
 
-    protected void updateCurrentConversation(String columnName, String value) {
-        mConversationListCursor.updateString(mContext, Conversation.listOf(mCurrentConversation),
-                columnName, value);
+    @Override
+    public void updateConversation(Collection <Conversation> target, String columnName,
+            String value) {
+        mConversationListCursor.updateString(mContext, target, columnName, value);
         refreshConversationList();
     }
 
@@ -1426,7 +1419,7 @@ public abstract class AbstractActivityController implements ActivityController,
      * clients should only require {@link DestructiveAction}s, not specific implementations of the.
      * Only the controllers should know what kind of destructive actions are being created.
      */
-    protected class ConversationAction implements DestructiveAction {
+    private class ConversationAction implements DestructiveAction {
         /**
          * The action to be performed. This is specified as the resource ID of the menu item
          * corresponding to this action: R.id.delete, R.id.report_spam, etc.
@@ -1547,7 +1540,7 @@ public abstract class AbstractActivityController implements ActivityController,
      * @param target the conversations to act upon.
      * @return a {@link DestructiveAction} that performs the specified action.
      */
-    protected final DestructiveAction getAction(int action, Collection<Conversation> target) {
+    private final DestructiveAction getAction(int action, Collection<Conversation> target) {
         final DestructiveAction da = new ConversationAction(action, target, false);
         registerDestructiveAction(da);
         return da;
@@ -1556,7 +1549,7 @@ public abstract class AbstractActivityController implements ActivityController,
     // Called from the FolderSelectionDialog after a user is done selecting folders to assign the
     // conversations to.
     @Override
-    public final void onFolderChangesCommit(
+    public final void assignFolder(
             Collection<Folder> folders, Collection<Conversation> target, boolean batch) {
         final boolean isDestructive = !Folder.containerIncludes(folders, mFolder);
         LogUtils.d(LOG_TAG, "onFolderChangesCommit: isDestructive = %b", isDestructive);
@@ -1689,8 +1682,8 @@ public abstract class AbstractActivityController implements ActivityController,
             return;
         }
         mCabActionMenu = new SelectedConversationsActionMenu(mActivity, set,
-                convList.getAnimatedAdapter(), this,
-                mAccount, mFolder, (SwipeableListView) convList.getListView());
+                convList.getAnimatedAdapter(), mAccount, mFolder,
+                (SwipeableListView) convList.getListView());
         enableCabMode();
     }
 
@@ -1912,12 +1905,8 @@ public abstract class AbstractActivityController implements ActivityController,
         return;
     }
 
-    /**
-     * Get a destructive action for selected conversations.
-     * @param action
-     * @return
-     */
-    public final DestructiveAction getBatchDestruction(int action) {
+    @Override
+    public final DestructiveAction getBatchAction(int action) {
         final DestructiveAction da = new ConversationAction(action, mSelectedSet.values(), true);
         registerDestructiveAction(da);
         return da;
diff --git a/src/com/android/mail/ui/ActivityController.java b/src/com/android/mail/ui/ActivityController.java
index 1eef46e73..ede3a5ee4 100644
--- a/src/com/android/mail/ui/ActivityController.java
+++ b/src/com/android/mail/ui/ActivityController.java
@@ -27,13 +27,14 @@ import android.view.KeyEvent;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.MotionEvent;
+import android.widget.AbsListView.OnScrollListener;
 
 import com.android.mail.ConversationListContext;
+import com.android.mail.browse.ConversationCursor.ConversationListener;
 import com.android.mail.providers.Account;
 import com.android.mail.providers.Conversation;
 import com.android.mail.providers.Folder;
 import com.android.mail.providers.Settings;
-import com.android.mail.ui.FoldersSelectionDialog.FolderChangeCommitListener;
 import com.android.mail.ui.ViewMode.ModeChangeListener;
 
 /**
@@ -42,10 +43,11 @@ import com.android.mail.ui.ViewMode.ModeChangeListener;
  * or respond to user action.
  */
 public interface ActivityController extends LayoutListener, SubjectDisplayChanger,
-        ModeChangeListener, ConversationListCallbacks, FolderChangeCommitListener,
+        ModeChangeListener, ConversationListCallbacks,
         FolderChangeListener, AccountChangeListener, LoaderManager.LoaderCallbacks<Cursor>,
-        ConversationSetObserver,
-        FolderListFragment.FolderListSelectionListener, HelpCallback, UndoBarView.UndoListener {
+        ConversationSetObserver, ConversationListener, OnScrollListener,
+        FolderListFragment.FolderListSelectionListener, HelpCallback, UndoBarView.UndoListener,
+        ConversationUpdater {
 
     // As far as possible, the methods here that correspond to Activity lifecycle have the same name
     // as their counterpart in the Activity lifecycle.
diff --git a/src/com/android/mail/ui/ControllableActivity.java b/src/com/android/mail/ui/ControllableActivity.java
index ce573beab..689627bce 100644
--- a/src/com/android/mail/ui/ControllableActivity.java
+++ b/src/com/android/mail/ui/ControllableActivity.java
@@ -67,6 +67,11 @@ public interface ControllableActivity extends HelpCallback, RestrictedActivity,
      */
     boolean shouldShowFirstConversation();
 
+    /**
+     * Get the set of currently selected conversations. This method returns a non-null value.
+     * In case no conversation is currently selected, it returns an empty selection set.
+     * @return
+     */
     ConversationSelectionSet getSelectedSet();
 
     /**
@@ -82,4 +87,12 @@ public interface ControllableActivity extends HelpCallback, RestrictedActivity,
      * Get the folder currently being accessed by the activity.
      */
     Folder getCurrentFolder();
+
+    /**
+     * Returns an object that can update conversation state. Holding a reference to the
+     * ConversationUpdater is safe since the ConversationUpdater is guaranteed to persist across
+     * changes to the conversation cursor.
+     * @return
+     */
+    ConversationUpdater getConversationUpdater();
 }
diff --git a/src/com/android/mail/ui/ConversationUpdater.java b/src/com/android/mail/ui/ConversationUpdater.java
new file mode 100644
index 000000000..60cb4eefc
--- /dev/null
+++ b/src/com/android/mail/ui/ConversationUpdater.java
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2012 Google Inc.
+ * Licensed to 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.mail.ui;
+
+import com.android.mail.providers.Conversation;
+import com.android.mail.providers.Folder;
+import com.android.mail.providers.UIProvider;
+
+import java.util.Collection;
+
+/**
+ * Classes that can update conversations implement this interface.
+ */
+public interface ConversationUpdater {
+    /**
+     * Modify the given conversation by changing the column provided here to contain the value
+     * provided. Column names are listed in {@link UIProvider.ConversationColumns}, for example
+     * {@link UIProvider.ConversationColumns#FOLDER_LIST}
+     * @param target
+     * @param columnName
+     * @param value
+     */
+    void updateConversation(Collection <Conversation> target, String columnName, String value);
+
+    /**
+     * Modify the given conversation by changing the column provided here to contain the value
+     * provided. Column names are listed in {@link UIProvider.ConversationColumns}, for example
+     * {@link UIProvider.ConversationColumns#READ}
+     * @param target
+     * @param columnName
+     * @param value
+     */
+    void updateConversation(Collection <Conversation> target, String columnName, int value);
+
+    /**
+     * Modify the given conversation by changing the column provided here to contain the value
+     * provided. Column names are listed in {@link UIProvider.ConversationColumns}, for example
+     * {@link UIProvider.ConversationColumns#HAS_ATTACHMENTS}
+     * @param target
+     * @param columnName
+     * @param value
+     */
+    void updateConversation(Collection <Conversation> target, String columnName, boolean value);
+
+    /**
+     * Get a destructive action for selected conversations. The action corresponds to Menu item
+     * identifiers, for example R.id.unread, or R.id.delete.
+     * @param action
+     * @return
+     */
+    public DestructiveAction getBatchAction(int action);
+
+    /**
+     * Assign the target conversations to the given folders, and remove them from all other
+     * folders that they might be assigned to.
+     * @param folders the folders to assign the conversations to.
+     * @param target the conversations to act upon.
+     * @param batch whether this is a batch operation
+     */
+    public void assignFolder(
+            Collection<Folder> folders, Collection<Conversation> target, boolean batch);
+}
diff --git a/src/com/android/mail/ui/FolderSelectionActivity.java b/src/com/android/mail/ui/FolderSelectionActivity.java
index bcf8c6f62..6f3892569 100644
--- a/src/com/android/mail/ui/FolderSelectionActivity.java
+++ b/src/com/android/mail/ui/FolderSelectionActivity.java
@@ -319,4 +319,9 @@ public class FolderSelectionActivity extends Activity implements OnClickListener
     public Folder getCurrentFolder() {
         return null;
     }
+
+    @Override
+    public ConversationUpdater getConversationUpdater() {
+        return null;
+    }
 }
diff --git a/src/com/android/mail/ui/FoldersSelectionDialog.java b/src/com/android/mail/ui/FoldersSelectionDialog.java
index b971c4c62..21ad75e81 100644
--- a/src/com/android/mail/ui/FoldersSelectionDialog.java
+++ b/src/com/android/mail/ui/FoldersSelectionDialog.java
@@ -42,29 +42,16 @@ import java.util.Map.Entry;
 
 public class FoldersSelectionDialog implements OnClickListener, OnMultiChoiceClickListener {
     private AlertDialog mDialog;
-    private FolderChangeCommitListener mCommitListener;
+    private ConversationUpdater mUpdater;
     private HashMap<Folder, Boolean> mCheckedState;
     private boolean mSingle = false;
     private FolderSelectorAdapter mAdapter;
     private final Collection<Conversation> mTarget;
     private boolean mBatch;
 
-    public interface FolderChangeCommitListener {
-        /**
-         * Assign the target conversations to the given folders, and remove them from all other
-         * folders that they might be assigned to.
-         * @param folders the folders to assign the conversations to.
-         * @param target the conversations to act upon.
-         * @param batch whether this is a batch operation
-         */
-        public void onFolderChangesCommit(
-                Collection<Folder> folders, Collection<Conversation> target, boolean batch);
-    }
-
     public FoldersSelectionDialog(final Context context, Account account,
-            final FolderChangeCommitListener commitListener,
-            Collection<Conversation> target, boolean isBatch) {
-        mCommitListener = commitListener;
+            final ConversationUpdater updater, Collection<Conversation> target, boolean isBatch) {
+        mUpdater = updater;
         mTarget = target;
         mBatch = isBatch;
 
@@ -152,8 +139,8 @@ public class FoldersSelectionDialog implements OnClickListener, OnMultiChoiceCli
                         folders.add(entry.getKey());
                     }
                 }
-                if (mCommitListener != null) {
-                    mCommitListener.onFolderChangesCommit(folders, mTarget, mBatch);
+                if (mUpdater != null) {
+                    mUpdater.assignFolder(folders, mTarget, mBatch);
                 }
                 break;
             case DialogInterface.BUTTON_NEGATIVE:
diff --git a/src/com/android/mail/ui/MailActivity.java b/src/com/android/mail/ui/MailActivity.java
index d09bb71f0..af2681792 100644
--- a/src/com/android/mail/ui/MailActivity.java
+++ b/src/com/android/mail/ui/MailActivity.java
@@ -300,4 +300,9 @@ public class MailActivity extends AbstractMailActivity implements ControllableAc
     public Folder getCurrentFolder() {
         return mController.getFolder();
     }
+
+    @Override
+    public ConversationUpdater getConversationUpdater() {
+        return mController;
+    }
 }