From 303c1ee85d3d8983554e8db8853935cef6238755 Mon Sep 17 00:00:00 2001
From: cketti <cketti@gmail.com>
Date: Mon, 29 Oct 2012 02:27:34 +0100
Subject: [PATCH] Extract generation of SQL queries for searches to
 SqlQueryBuilder

---
 .../fsck/k9/fragment/MessageListFragment.java |  69 +-----
 src/com/fsck/k9/mail/store/LocalStore.java    |  29 +--
 .../fsck/k9/search/ConditionsTreeNode.java    |  12 -
 src/com/fsck/k9/search/LocalSearch.java       |   1 +
 .../fsck/k9/search/SearchSpecification.java   |  80 ++-----
 src/com/fsck/k9/search/SqlQueryBuilder.java   | 225 ++++++++++++++++++
 6 files changed, 272 insertions(+), 144 deletions(-)
 create mode 100644 src/com/fsck/k9/search/SqlQueryBuilder.java

diff --git a/src/com/fsck/k9/fragment/MessageListFragment.java b/src/com/fsck/k9/fragment/MessageListFragment.java
index 1438c33fe..ad668d339 100644
--- a/src/com/fsck/k9/fragment/MessageListFragment.java
+++ b/src/com/fsck/k9/fragment/MessageListFragment.java
@@ -85,10 +85,9 @@ import com.fsck.k9.mail.store.LocalStore.LocalFolder;
 import com.fsck.k9.provider.EmailProvider;
 import com.fsck.k9.provider.EmailProvider.MessageColumns;
 import com.fsck.k9.provider.EmailProvider.SpecialColumns;
-import com.fsck.k9.search.ConditionsTreeNode;
 import com.fsck.k9.search.LocalSearch;
 import com.fsck.k9.search.SearchSpecification;
-import com.fsck.k9.search.SearchSpecification.SearchCondition;
+import com.fsck.k9.search.SqlQueryBuilder;
 import com.handmark.pulltorefresh.library.PullToRefreshBase;
 import com.handmark.pulltorefresh.library.PullToRefreshListView;
 
@@ -2667,11 +2666,18 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick
 
         StringBuilder query = new StringBuilder();
         List<String> queryArgs = new ArrayList<String>();
-        buildQuery(account, mSearch.getConditions(), query, queryArgs);
+        SqlQueryBuilder.buildWhereClause(account, mSearch.getConditions(), query, queryArgs);
 
         String selection = query.toString();
         String[] selectionArgs = queryArgs.toArray(new String[0]);
 
+        String sortOrder = buildSortOrder();
+
+        return new CursorLoader(getActivity(), uri, projection, selection, selectionArgs,
+                sortOrder);
+    }
+
+    private String buildSortOrder() {
         String sortColumn = MessageColumns.ID;
         switch (mSortType) {
             case SORT_ARRIVAL: {
@@ -2715,62 +2721,7 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick
 
         String sortOrder = sortColumn + sortDirection + ", " + secondarySort +
                 MessageColumns.ID + " DESC";
-
-        return new CursorLoader(getActivity(), uri, projection, selection, selectionArgs,
-                sortOrder);
-    }
-
-    private void buildQuery(Account account, ConditionsTreeNode node, StringBuilder query,
-            List<String> selectionArgs) {
-
-        if (node == null) {
-            return;
-        }
-
-        if (node.mLeft == null && node.mRight == null) {
-            SearchCondition condition = node.mCondition;
-            switch (condition.field) {
-                case FOLDER: {
-                    String folderName;
-                    //TODO: Fix the search condition used by the Unified Inbox
-                    if (LocalSearch.GENERIC_INBOX_NAME.equals(condition.value) ||
-                            "1".equals(condition.value)) {
-                        folderName = account.getInboxFolderName();
-                    } else {
-                        folderName = condition.value;
-                    }
-                    long folderId = getFolderId(account, folderName);
-                    query.append("folder_id = ?");
-                    selectionArgs.add(Long.toString(folderId));
-                    break;
-                }
-                default: {
-                    query.append(condition.toString());
-                }
-            }
-        } else {
-            query.append("(");
-            buildQuery(account, node.mLeft, query, selectionArgs);
-            query.append(") ");
-            query.append(node.mValue.name());
-            query.append(" (");
-            buildQuery(account, node.mRight, query, selectionArgs);
-            query.append(")");
-        }
-    }
-
-    private long getFolderId(Account account, String folderName) {
-        long folderId = 0;
-        try {
-            LocalFolder folder = (LocalFolder) getFolder(folderName, account).folder;
-            folder.open(OpenMode.READ_ONLY);
-            folderId = folder.getId();
-        } catch (MessagingException e) {
-            //FIXME
-            e.printStackTrace();
-        }
-
-        return folderId;
+        return sortOrder;
     }
 
     @Override
diff --git a/src/com/fsck/k9/mail/store/LocalStore.java b/src/com/fsck/k9/mail/store/LocalStore.java
index db9e52495..0a25a2b52 100644
--- a/src/com/fsck/k9/mail/store/LocalStore.java
+++ b/src/com/fsck/k9/mail/store/LocalStore.java
@@ -46,6 +46,7 @@ import com.fsck.k9.activity.Search;
 import com.fsck.k9.controller.MessageRemovalListener;
 import com.fsck.k9.controller.MessageRetrievalListener;
 import com.fsck.k9.helper.HtmlConverter;
+import com.fsck.k9.helper.StringUtils;
 import com.fsck.k9.helper.Utility;
 import com.fsck.k9.mail.Address;
 import com.fsck.k9.mail.Body;
@@ -71,9 +72,8 @@ import com.fsck.k9.mail.store.LockableDatabase.WrappedException;
 import com.fsck.k9.mail.store.StorageManager.StorageProvider;
 import com.fsck.k9.provider.AttachmentProvider;
 import com.fsck.k9.provider.EmailProvider;
-import com.fsck.k9.search.ConditionsTreeNode;
 import com.fsck.k9.search.LocalSearch;
-import com.fsck.k9.search.SearchSpecification.Searchfield;
+import com.fsck.k9.search.SqlQueryBuilder;
 
 /**
  * <pre>
@@ -960,32 +960,23 @@ public class LocalStore extends Store implements Serializable {
     public Message[] searchForMessages(MessageRetrievalListener retrievalListener,
                                         LocalSearch search) throws MessagingException {
 
-        // update some references in the search that have to be bound to this one store
-        for (ConditionsTreeNode node : search.getLeafSet()) {
-            if (node.mCondition.field == Searchfield.FOLDER) {
-                // TODO find better solution
-                if (isFolderId(node.mCondition.value)) {
-                    continue;
-                }
+        StringBuilder query = new StringBuilder();
+        List<String> queryArgs = new ArrayList<String>();
+        SqlQueryBuilder.buildWhereClause(mAccount, search.getConditions(), query, queryArgs);
 
-                if (node.mCondition.value.equals(LocalSearch.GENERIC_INBOX_NAME)) {
-                    node.mCondition.value = mAccount.getInboxFolderName();
-                }
-                node.mCondition.value = String.valueOf(getFolderId(node.mCondition.value));
-            }
-        }
+        String where = query.toString();
+        String[] selectionArgs = queryArgs.toArray(EMPTY_STRING_ARRAY);
 
-        // build sql query
-        ConditionsTreeNode conditions = search.getConditions();
         String sqlQuery = "SELECT " + GET_MESSAGES_COLS + "FROM messages WHERE " +
                 "((empty IS NULL OR empty != 1) AND deleted = 0)" +
-                ((conditions != null) ? " AND (" + conditions + ")" : "") + " ORDER BY date DESC";
+                ((!StringUtils.isNullOrEmpty(where)) ? " AND (" + where + ")" : "") +
+                " ORDER BY date DESC";
 
         if (K9.DEBUG) {
             Log.d(K9.LOG_TAG, "Query = " + sqlQuery);
         }
 
-        return getMessages(retrievalListener, null, sqlQuery, new String[] {});
+        return getMessages(retrievalListener, null, sqlQuery, selectionArgs);
     }
 
     /*
diff --git a/src/com/fsck/k9/search/ConditionsTreeNode.java b/src/com/fsck/k9/search/ConditionsTreeNode.java
index 52bc6bb58..1a1be85d7 100644
--- a/src/com/fsck/k9/search/ConditionsTreeNode.java
+++ b/src/com/fsck/k9/search/ConditionsTreeNode.java
@@ -239,18 +239,6 @@ public class ConditionsTreeNode implements Parcelable {
         return mCondition;
     }
 
-
-    /**
-     * This will traverse the tree inorder and call toString recursively resulting
-     * in a valid SQL where clause.
-     */
-    @Override
-    public String toString() {
-        return (mLeft == null ? "" : "(" + mLeft + ")")
-                + " " + ( mCondition == null ? mValue.name() : mCondition ) + " "
-                + (mRight == null ? "" : "(" + mRight + ")") ;
-    }
-
     /**
      * Get a set of all the leaves in the tree.
      * @return Set of all the leaves.
diff --git a/src/com/fsck/k9/search/LocalSearch.java b/src/com/fsck/k9/search/LocalSearch.java
index 444e9adde..ac3463258 100644
--- a/src/com/fsck/k9/search/LocalSearch.java
+++ b/src/com/fsck/k9/search/LocalSearch.java
@@ -112,6 +112,7 @@ public class LocalSearch implements SearchSpecification {
     public void addAccountUuid(String uuid) {
         if (uuid.equals(ALL_ACCOUNTS)) {
             mAccountUuids.clear();
+            return;
         }
         mAccountUuids.add(uuid);
     }
diff --git a/src/com/fsck/k9/search/SearchSpecification.java b/src/com/fsck/k9/search/SearchSpecification.java
index a953a5d4d..f40f1dfba 100644
--- a/src/com/fsck/k9/search/SearchSpecification.java
+++ b/src/com/fsck/k9/search/SearchSpecification.java
@@ -35,40 +35,17 @@ public interface SearchSpecification extends Parcelable {
     // ATTRIBUTE enum
     ///////////////////////////////////////////////////////////////
     public enum Attribute {
-        CONTAINS(false), EQUALS(false), STARTSWITH(false), ENDSWITH(false),
-        NOT_CONTAINS(true), NOT_EQUALS(true), NOT_STARTSWITH(true), NOT_ENDSWITH(true);
+        CONTAINS,
+        NOT_CONTAINS,
 
-        private boolean mNegation;
+        EQUALS,
+        NOT_EQUALS,
 
-        private Attribute(boolean negation) {
-            this.mNegation = negation;
-        }
+        STARTSWITH,
+        NOT_STARTSWITH,
 
-        public String formQuery(String value) {
-            String queryPart = "";
-
-            switch (this) {
-            case NOT_CONTAINS:
-            case CONTAINS:
-                queryPart = "'%"+value+"%'";
-                break;
-            case NOT_EQUALS:
-            case EQUALS:
-                queryPart = "'"+value+"'";
-                break;
-            case NOT_STARTSWITH:
-            case STARTSWITH:
-                queryPart = "'%"+value+"'";
-                break;
-            case NOT_ENDSWITH:
-            case ENDSWITH:
-                queryPart = "'"+value+"%'";
-                break;
-            default: queryPart = "'"+value+"'";
-            }
-
-            return (mNegation ? " NOT LIKE " : " LIKE ") + queryPart;
-        }
+        ENDSWITH,
+        NOT_ENDSWITH
     }
 
     ///////////////////////////////////////////////////////////////
@@ -87,21 +64,21 @@ public interface SearchSpecification extends Parcelable {
      *
      */
     public enum Searchfield {
-        SUBJECT("subject"), DATE("date"), UID("uid"), FLAG("flags"),
-        SENDER("sender_list"), TO("to_list"), CC("cc_list"), FOLDER("folder_id"),
-        BCC("bcc_list"), REPLY_TO("reply_to_list"), MESSAGE("text_content"),
-        ATTACHMENT_COUNT("attachment_count"), DELETED("deleted"), THREAD_ROOT("thread_root"),
-        ID("id");
-
-        private String dbName;
-
-        private Searchfield(String dbName) {
-            this.dbName = dbName;
-        }
-
-        public String getDatabaseName() {
-            return dbName;
-        }
+        SUBJECT,
+        DATE,
+        UID,
+        FLAG,
+        SENDER,
+        TO,
+        CC,
+        FOLDER,
+        BCC,
+        REPLY_TO,
+        MESSAGE_CONTENTS,
+        ATTACHMENT_COUNT,
+        DELETED,
+        THREAD_ROOT,
+        ID
     }
 
 
@@ -118,9 +95,9 @@ public interface SearchSpecification extends Parcelable {
      * @author dzan
      */
     public class SearchCondition implements Parcelable {
-        public String value;
-        public Attribute attribute;
-        public Searchfield field;
+        public final String value;
+        public final Attribute attribute;
+        public final Searchfield field;
 
         public SearchCondition(Searchfield field, Attribute attribute, String value) {
             this.value = value;
@@ -143,11 +120,6 @@ public interface SearchSpecification extends Parcelable {
             return field.toString() + attribute.toString();
         }
 
-        @Override
-        public String toString() {
-            return field.getDatabaseName() + attribute.formQuery(value);
-        }
-
         @Override
         public boolean equals(Object o) {
             if (o instanceof SearchCondition) {
diff --git a/src/com/fsck/k9/search/SqlQueryBuilder.java b/src/com/fsck/k9/search/SqlQueryBuilder.java
new file mode 100644
index 000000000..3749c3507
--- /dev/null
+++ b/src/com/fsck/k9/search/SqlQueryBuilder.java
@@ -0,0 +1,225 @@
+package com.fsck.k9.search;
+
+import java.util.List;
+
+import com.fsck.k9.Account;
+import com.fsck.k9.mail.MessagingException;
+import com.fsck.k9.mail.Folder.OpenMode;
+import com.fsck.k9.mail.store.LocalStore;
+import com.fsck.k9.mail.store.LocalStore.LocalFolder;
+import com.fsck.k9.search.SearchSpecification.SearchCondition;
+import com.fsck.k9.search.SearchSpecification.Searchfield;
+
+
+public class SqlQueryBuilder {
+    public static void buildWhereClause(Account account, ConditionsTreeNode node,
+            StringBuilder query, List<String> selectionArgs) {
+        buildWhereClauseInternal(account, node, query, selectionArgs);
+    }
+
+    private static void buildWhereClauseInternal(Account account, ConditionsTreeNode node,
+            StringBuilder query, List<String> selectionArgs) {
+        if (node == null) {
+            return;
+        }
+
+        if (node.mLeft == null && node.mRight == null) {
+            SearchCondition condition = node.mCondition;
+            switch (condition.field) {
+                case FOLDER: {
+                    String folderName;
+                    //TODO: Fix the search condition used by the Unified Inbox (we search all
+                    // folders with an enabled "Unify" folder setting).
+                    if (LocalSearch.GENERIC_INBOX_NAME.equals(condition.value)) {
+                        folderName = account.getInboxFolderName();
+                    } else {
+                        folderName = condition.value;
+                    }
+                    long folderId = getFolderId(account, folderName);
+                    query.append("folder_id = ?");
+                    selectionArgs.add(Long.toString(folderId));
+                    break;
+                }
+                default: {
+                    appendCondition(condition, query, selectionArgs);
+                }
+            }
+        } else {
+            query.append("(");
+            buildWhereClauseInternal(account, node.mLeft, query, selectionArgs);
+            query.append(") ");
+            query.append(node.mValue.name());
+            query.append(" (");
+            buildWhereClauseInternal(account, node.mRight, query, selectionArgs);
+            query.append(")");
+        }
+    }
+
+    private static void appendCondition(SearchCondition condition, StringBuilder query,
+            List<String> selectionArgs) {
+        query.append(getColumnName(condition));
+        appendExprRight(condition, query, selectionArgs);
+    }
+
+    private static long getFolderId(Account account, String folderName) {
+        long folderId = 0;
+        try {
+            LocalStore localStore = account.getLocalStore();
+            LocalFolder folder = localStore.getFolder(folderName);
+            folder.open(OpenMode.READ_ONLY);
+            folderId = folder.getId();
+        } catch (MessagingException e) {
+            //FIXME
+            e.printStackTrace();
+        }
+
+        return folderId;
+    }
+
+    private static String getColumnName(SearchCondition condition) {
+        String columnName = null;
+        switch (condition.field) {
+            case ATTACHMENT_COUNT: {
+                columnName = "attachment_count";
+                break;
+            }
+            case BCC: {
+                columnName = "bcc_list";
+                break;
+            }
+            case CC: {
+                columnName = "cc_list";
+                break;
+            }
+            case DATE: {
+                columnName = "date";
+                break;
+            }
+            case DELETED: {
+                columnName = "deleted";
+                break;
+            }
+            case FLAG: {
+                columnName = "flags";
+                break;
+            }
+            case FOLDER: {
+                columnName = "folder_id";
+                break;
+            }
+            case ID: {
+                columnName = "id";
+                break;
+            }
+            case MESSAGE_CONTENTS: {
+                columnName = "text_content";
+                break;
+            }
+            case REPLY_TO: {
+                columnName = "reply_to_list";
+                break;
+            }
+            case SENDER: {
+                columnName = "sender_list";
+                break;
+            }
+            case SUBJECT: {
+                columnName = "subject";
+                break;
+            }
+            case THREAD_ROOT: {
+                columnName = "thread_root";
+                break;
+            }
+            case TO: {
+                columnName = "to_list";
+                break;
+            }
+            case UID: {
+                columnName = "uid";
+                break;
+            }
+        }
+
+        if (columnName == null) {
+            throw new RuntimeException("Unhandled case");
+        }
+
+        return columnName;
+    }
+
+    private static void appendExprRight(SearchCondition condition, StringBuilder query,
+            List<String> selectionArgs) {
+        String value = condition.value;
+        Searchfield field = condition.field;
+
+        query.append(" ");
+        String selectionArg = null;
+        switch (condition.attribute) {
+            case NOT_CONTAINS:
+                query.append("NOT ");
+                //$FALL-THROUGH$
+            case CONTAINS: {
+                query.append("LIKE ?");
+                selectionArg = "%" + value + "%";
+                break;
+            }
+            case NOT_STARTSWITH:
+                query.append("NOT ");
+                //$FALL-THROUGH$
+            case STARTSWITH: {
+                query.append("LIKE ?");
+                selectionArg = "%" + value;
+                break;
+            }
+            case NOT_ENDSWITH:
+                query.append("NOT ");
+                //$FALL-THROUGH$
+            case ENDSWITH: {
+                query.append("LIKE ?");
+                selectionArg = value + "%";
+                break;
+            }
+            case NOT_EQUALS: {
+                if (isNumberColumn(field)) {
+                    query.append("!= ?");
+                } else {
+                    query.append("NOT LIKE ?");
+                }
+                selectionArg = value;
+                break;
+            }
+            case EQUALS: {
+                if (isNumberColumn(field)) {
+                    query.append("== ?");
+                } else {
+                    query.append("LIKE ?");
+                }
+                selectionArg = value;
+                break;
+            }
+        }
+
+        if (selectionArg == null) {
+            throw new RuntimeException("Unhandled case");
+        }
+
+        selectionArgs.add(selectionArg);
+    }
+
+    private static boolean isNumberColumn(Searchfield field) {
+        switch (field) {
+            case ATTACHMENT_COUNT:
+            case DATE:
+            case DELETED:
+            case FOLDER:
+            case ID:
+            case THREAD_ROOT: {
+                return true;
+            }
+            default: {
+                return false;
+            }
+        }
+    }
+}