Partial-work commit of tag migration and improvements to the data model.

plugin-src/com/todoroo/astrid/tags/DataService.java

@@ -0,0 +1,178 @@
+package com.todoroo.astrid.tags;
+
+import java.util.ArrayList;
+
+import android.content.Context;
+import android.database.DatabaseUtils;
+
+import com.thoughtworks.sql.Criterion;
+import com.thoughtworks.sql.Order;
+import com.thoughtworks.sql.Query;
+import com.todoroo.andlib.data.TodorooCursor;
+import com.todoroo.andlib.data.Property.CountProperty;
+import com.todoroo.andlib.service.Autowired;
+import com.todoroo.andlib.service.DependencyInjectionService;
+import com.todoroo.astrid.api.AstridApiConstants;
+import com.todoroo.astrid.api.AstridContentProvider.AstridTask;
+import com.todoroo.astrid.dao.Database;
+import com.todoroo.astrid.dao.MetadataDao;
+import com.todoroo.astrid.dao.MetadataDao.MetadataCriteria;
+import com.todoroo.astrid.model.Metadata;
+import com.todoroo.astrid.service.MetadataService;
+
+/**
+ * Provides operations for working with tags
+ *
+ * @author Tim Su <tim@todoroo.com>
+ *
+ */
+@SuppressWarnings("nls")
+public class DataService {
+
+    /**
+     * Metadata key for tag data
+     */
+    public static final String KEY = "tags-tag";
+
+    @Autowired
+    private Database database;
+
+    @Autowired
+    private MetadataDao metadataDao;
+
+    @Autowired
+    private MetadataService metadataService;
+
+    public DataService(@SuppressWarnings("unused") Context context) {
+        DependencyInjectionService.getInstance().inject(this);
+    }
+
+    /**
+     * Property for retrieving count of aggregated rows
+     */
+    private static final CountProperty COUNT = new CountProperty();
+
+    public static final Order GROUPED_TAGS_BY_ALPHA = Order.asc(Metadata.VALUE);
+    public static final Order GROUPED_TAGS_BY_SIZE = Order.desc(COUNT);
+
+    /**
+     * Helper class for returning a tag/task count pair
+     *
+     * @author Tim Su <tim@todoroo.com>
+     *
+     */
+    public class Tag {
+        String tag;
+        int count;
+
+        @Override
+        public String toString() {
+            return tag;
+        }
+    }
+
+    /**
+     * Return all tags ordered by given clause
+     *
+     * @param taskId
+     * @return empty array if no tags, otherwise array
+     */
+    public Tag[] getGroupedTags(Order order) {
+        TodorooCursor<Metadata> cursor = metadataService.fetchWithCount(
+                COUNT, MetadataCriteria.withKey(KEY), order, true);
+        try {
+            Tag[] array = new Tag[cursor.getCount()];
+            for (int i = 0; i < array.length; i++) {
+                cursor.moveToNext();
+                array[i] = new Tag();
+                array[i].tag = cursor.get(Metadata.VALUE);
+                array[i].count = cursor.get(COUNT);
+            }
+            return array;
+        } finally {
+            cursor.close();
+        }
+    }
+
+    /**
+     * Return tags on the given task
+     *
+     * @param taskId
+     * @return empty array if no tags, otherwise array
+     */
+    public TodorooCursor<Metadata> getTags(long taskId) {
+        Query query = Query.select(Metadata.VALUE).where(Criterion.and(MetadataCriteria.withKey(KEY),
+                MetadataCriteria.byTask(taskId)));
+        return metadataDao.query(query);
+    }
+
+    /**
+     * Return tags as a comma-separated list of strings
+     *
+     * @param taskId
+     * @return empty string if no tags, otherwise string
+     */
+    public String getTagsAsString(long taskId) {
+        StringBuilder tagBuilder = new StringBuilder();
+        String[] tags = getTags(taskId);
+        for (int i = 0; i < tags.length; i++) {
+            tagBuilder.append(tags[i]);
+            if (i < tags.length - 1)
+                tagBuilder.append(", ");
+        }
+        return tagBuilder.toString();
+    }
+
+    private static final String query = String.format("INNER JOIN %s ON %s = "
+            + "%s WHERE %s = 0 AND %s = '%s' AND %s = ",
+            AstridApiConstants.METADATA_TABLE,
+            AstridTask.ID, Metadata.TASK,
+            AstridTask.COMPLETION_DATE,
+            Metadata.KEY, KEY,
+            Metadata.VALUE);
+
+    /**
+     * Return SQL selector query for getting tasks with a given tag
+     *
+     * @param tag
+     * @return
+     */
+    public String getQuery(String tag) {
+        return query + String.format("%s", DatabaseUtils.sqlEscapeString(tag));
+    }
+
+    private static final String newTaskSql = String.format(
+            "INSERT INTO %s (%s, %s, %s) " + "VALUES ($ID,'%s',",
+            AstridApiConstants.METADATA_TABLE,
+            Metadata.TASK.name,
+            Metadata.KEY.name,
+            Metadata.VALUE.name,
+            KEY);
+
+    /**
+     * Return SQL new task creator query
+     * @param tag
+     */
+    public String getNewTaskSql(String tag) {
+        return newTaskSql + String.format("%s)", DatabaseUtils.sqlEscapeString(tag));
+    }
+
+    /**
+     * Save the given array of tags into the database
+     * @param taskId
+     * @param tags
+     */
+    public void synchronizeTags(long taskId, ArrayList<String> tags) {
+        metadataDao.deleteWhere(database, MetadataSql.byTask(taskId) + " AND " +
+                MetadataSql.withKey(KEY));
+
+        Metadata metadata = new Metadata();
+        metadata.setValue(Metadata.KEY, KEY);
+        metadata.setValue(Metadata.TASK, taskId);
+        for(String tag : tags) {
+            metadata.setValue(Metadata.VALUE, tag.trim());
+            metadataDao.save(database, metadata);
+            metadata.clearValue(Metadata.ID);
+        }
+    }
+}

plugin-src/com/todoroo/astrid/tagsold/Tag.java

@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2009, Todoroo Inc
+ * All Rights Reserved
+ * http://www.todoroo.com
+ */
+package com.todoroo.astrid.tagsold;
+
+
+import android.content.ContentValues;
+
+import com.todoroo.andlib.data.AbstractModel;
+import com.todoroo.andlib.data.Property;
+import com.todoroo.andlib.data.Table;
+import com.todoroo.andlib.data.TodorooCursor;
+import com.todoroo.andlib.data.Property.LongProperty;
+import com.todoroo.andlib.data.Property.StringProperty;
+
+/**
+ * Data Model which represents a task users need to accomplish.
+ *
+ * @author Tim Su <tim@todoroo.com>
+ *
+ */
+@SuppressWarnings("nls")
+public class Tag extends AbstractModel {
+
+    // --- table
+
+    public static final Table TABLE = new Table("tags", Tag.class);
+
+    // --- properties
+
+    /** ID */
+    public static final LongProperty ID = new LongProperty(
+            TABLE, ID_PROPERTY_NAME);
+
+    /** Name of Task */
+    public static final StringProperty NAME = new StringProperty(
+            TABLE, "name");
+
+    /** List of all properties for this model */
+    public static final Property<?>[] PROPERTIES = generateProperties(Tag.class);
+
+    // --- defaults
+
+    /** Default values container */
+    private static final ContentValues defaultValues = new ContentValues();
+
+    static {
+        defaultValues.put(NAME.name, "");
+    }
+
+    @Override
+    public ContentValues getDefaultValues() {
+        return defaultValues;
+    }
+
+    // --- data access boilerplate
+
+    public Tag() {
+        super();
+    }
+
+    public Tag(TodorooCursor<Tag> cursor, Property<?>[] properties) {
+        this();
+        readPropertiesFromCursor(cursor, properties);
+    }
+
+    public void readFromCursor(TodorooCursor<Tag> cursor, Property<?>[] properties) {
+        super.readPropertiesFromCursor(cursor, properties);
+    }
+
+    @Override
+    public long getId() {
+        return getIdHelper(ID);
+    }
+
+}

plugin-src/com/todoroo/astrid/tagsold/TagService.java

@@ -0,0 +1,63 @@
+/*
+ * ASTRID: Android's Simple Task Recording Dashboard
+ *
+ * Copyright (c) 2009 Tim Su
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+package com.todoroo.astrid.tagsold;
+
+import android.database.SQLException;
+
+import com.thoughtworks.sql.Join;
+import com.thoughtworks.sql.Query;
+import com.todoroo.andlib.data.GenericDao;
+import com.todoroo.andlib.data.Property;
+import com.todoroo.andlib.data.TodorooCursor;
+import com.todoroo.astrid.model.Task;
+
+/**
+ * Service layer for tags plugin
+ *
+ * @author Tim Su <tim@todoroo.com>
+ *
+ */
+public class TagService {
+
+    private GenericDao<Tag> tagDao;
+    private GenericDao<TagToTaskMapping> tagToTaskDao;
+
+    public TagService() {
+        TagsDatabase tagDatabase = new TagsDatabase();
+        tagDao = new GenericDao<Tag>(Tag.class, tagDatabase);
+        tagToTaskDao = new GenericDao<TagToTaskMapping>(TagToTaskMapping.class,
+                tagDatabase);
+    }
+
+    // --- tag batch operations
+
+    /** Get a list of all tags */
+    public TodorooCursor<Tag> getAllTags(Property<?>... properties) {
+        return tagDao.query(Query.select(properties));
+    }
+
+    /** Get a list of tag identifiers for the given task */
+    public TodorooCursor<Tag> getTaskTags(Task task, Property<?>... properties) throws SQLException {
+        Query query = Query.select(properties).join(Join.inner(TagToTaskMapping.TABLE,
+                Tag.ID.eq(TagToTaskMapping.TAG))).where(TagToTaskMapping.TASK.eq(task.getId()));
+        return tagDao.query(query);
+    }
+
+}

plugin-src/com/todoroo/astrid/tagsold/TagToTaskMapping.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2009, Todoroo Inc
+ * All Rights Reserved
+ * http://www.todoroo.com
+ */
+package com.todoroo.astrid.tagsold;
+
+
+import android.content.ContentValues;
+
+import com.todoroo.andlib.data.AbstractModel;
+import com.todoroo.andlib.data.Property;
+import com.todoroo.andlib.data.Table;
+import com.todoroo.andlib.data.TodorooCursor;
+import com.todoroo.andlib.data.Property.LongProperty;
+
+/**
+ * Data Model which represents a task users need to accomplish.
+ *
+ * @author Tim Su <tim@todoroo.com>
+ *
+ */
+@SuppressWarnings("nls")
+public class TagToTaskMapping extends AbstractModel {
+
+    // --- table
+
+    public static final Table TABLE = new Table("tagTaskMap", TagToTaskMapping.class);
+
+    // --- properties
+
+    /** Tag */
+    public static final LongProperty TAG = new LongProperty(
+            TABLE, "tag");
+
+    /** Task */
+    public static final LongProperty TASK = new LongProperty(
+            TABLE, "task");
+
+    /** List of all properties for this model */
+    public static final Property<?>[] PROPERTIES = generateProperties(TagToTaskMapping.class);
+
+    // --- defaults
+
+    /** Default values container */
+    private static final ContentValues defaultValues = new ContentValues();
+
+    @Override
+    public ContentValues getDefaultValues() {
+        return defaultValues;
+    }
+
+    // --- data access boilerplate
+
+    public TagToTaskMapping() {
+        super();
+    }
+
+    public TagToTaskMapping(TodorooCursor<TagToTaskMapping> cursor, Property<?>[] properties) {
+        this();
+        readPropertiesFromCursor(cursor, properties);
+    }
+
+    public void readFromCursor(TodorooCursor<TagToTaskMapping> cursor, Property<?>[] properties) {
+        super.readPropertiesFromCursor(cursor, properties);
+    }
+
+    @Override
+    public long getId() {
+        return NO_ID;
+    }
+
+}

plugin-src/com/todoroo/astrid/tagsold/TagsDatabase.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2009, Todoroo Inc
+ * All Rights Reserved
+ * http://www.todoroo.com
+ */
+package com.todoroo.astrid.tagsold;
+
+import com.todoroo.andlib.data.AbstractDatabase;
+import com.todoroo.andlib.data.Table;
+
+/**
+ * Database wrapper
+ *
+ * @author Tim Su <tim@todoroo.com>
+ *
+ */
+@SuppressWarnings("nls")
+public class TagsDatabase extends AbstractDatabase {
+
+    // --- constants
+
+    /**
+     * Database version number. This variable must be updated when database
+     * tables are updated, as it determines whether a database needs updating.
+     */
+    public static final int VERSION = 1;
+
+    /**
+     * Database name (must be unique)
+     */
+    private static final String NAME = "tags";
+
+    /**
+     * List of table/ If you're adding a new table, add it to this list and
+     * also make sure that our SQLite helper does the right thing.
+     */
+    public static final Table[] TABLES =  new Table[] {
+        Tag.TABLE,
+        TagToTaskMapping.TABLE,
+    };
+
+    // --- implementation
+
+    @Override
+    protected String getName() {
+        return NAME;
+    }
+
+    @Override
+    protected int getVersion() {
+        return VERSION;
+    }
+
+    @Override
+    public Table[] getTables() {
+        return TABLES;
+    }
+
+    @Override
+    protected void onCreateTables() {
+        StringBuilder sql = new StringBuilder();
+        sql.append("CREATE INDEX IF NOT EXISTS tm_tag ON ").
+            append(TagToTaskMapping.TABLE).append('(').
+                append(TagToTaskMapping.TAG.name).
+            append(')');
+        database.execSQL(sql.toString());
+
+        sql.setLength(0);
+        sql.append("CREATE INDEX IF NOT EXISTS tm_task ON ").
+            append(TagToTaskMapping.TABLE).append('(').
+                append(TagToTaskMapping.TASK.name).
+            append(')');
+        database.execSQL(sql.toString());
+    }
+
+    @Override
+    protected boolean onUpgrade(int oldVersion, int newVersion) {
+        return false;
+    }
+
+}
+

src-legacy/com/timsu/astrid/data/tag/TagToTaskMapping.java

@@ -40,8 +40,8 @@ public class TagToTaskMapping extends AbstractModel {
 
     // field names
 
-    static final String                TASK                = "task";
+    public static final String                TASK                = "task";
-    static final String                TAG                 = "tag";
+    public static final String                TAG                 = "tag";
 
     /** Default values container */
     private static final ContentValues defaultValues       = new ContentValues();

src/com/todoroo/astrid/dao/Database.java

@@ -5,17 +5,8 @@
  */
 package com.todoroo.astrid.dao;
 
-import android.content.Context;
-import android.database.sqlite.SQLiteDatabase;
-import android.database.sqlite.SQLiteOpenHelper;
-import android.database.sqlite.SQLiteDatabase.CursorFactory;
-import android.util.Log;
-
 import com.todoroo.andlib.data.AbstractDatabase;
-import com.todoroo.andlib.data.AbstractModel;
-import com.todoroo.andlib.data.Property;
 import com.todoroo.andlib.data.Table;
-import com.todoroo.andlib.service.ContextManager;
 import com.todoroo.astrid.model.Metadata;
 import com.todoroo.astrid.model.Task;
 
@@ -25,6 +16,7 @@ import com.todoroo.astrid.model.Task;
  * @author Tim Su <tim@todoroo.com>
  *
  */
+@SuppressWarnings("nls")
 public class Database extends AbstractDatabase {
 
     // --- constants
@@ -36,13 +28,13 @@ public class Database extends AbstractDatabase {
     public static final int VERSION = 1;
 
     /**
-     * Database name
+     * Database name (must be unique)
      */
-    private static final String NAME = "database"; //$NON-NLS-1$
+    private static final String NAME = "database";
 
     /**
      * List of table/ If you're adding a new table, add it to this list and
-     * also make sure that BenteSQLiteOpenHelper does the right thing.
+     * also make sure that our SQLite helper does the right thing.
      */
     public static final Table[] TABLES =  new Table[] {
         Task.TABLE,
@@ -51,18 +43,12 @@ public class Database extends AbstractDatabase {
 
     // --- implementation
 
-    /**
-     * Creates a database wrapper
-     */
-    public Database() {
-        //
-    }
-
     @Override
     protected String getName() {
         return NAME;
     }
 
+    @Override
     protected int getVersion() {
         return VERSION;
     }
@@ -72,101 +58,20 @@ public class Database extends AbstractDatabase {
         return TABLES;
     }
 
-    /**
-     * Default implementation of Astrid database helper
-     */
-    @SuppressWarnings("nls")
-    private static class AstridSQLiteOpenHelper extends SQLiteOpenHelper {
-
-        public AstridSQLiteOpenHelper(Context context, String name,
-                CursorFactory factory, int version) {
-            super(context, name, factory, version);
-        }
-
-        /**
-         * Called to create the database tables
-         */
     @Override
-        public synchronized void onCreate(SQLiteDatabase db) {
+    protected void onCreateTables() {
         StringBuilder sql = new StringBuilder();
-            SqlConstructorVisitor sqlVisitor = new SqlConstructorVisitor();
-
-            // create tables
-            for(Table table : TABLES) {
-                sql.append("CREATE TABLE IF NOT EXISTS ").append(table.name).append('(').
-                append(AbstractModel.ID_PROPERTY).append(" INTEGER PRIMARY KEY AUTOINCREMENT");
-                for(Property<?> property : table.getProperties()) {
-                    if(AbstractModel.ID_PROPERTY.name.equals(property.name))
-                        continue;
-                    sql.append(',').append(property.accept(sqlVisitor, null));
-                }
-                sql.append(')');
-                db.execSQL(sql.toString());
-                sql.setLength(0);
-            }
-
-            // create indices
         sql.append("CREATE INDEX IF NOT EXISTS md_tid ON ").
             append(Metadata.TABLE).append('(').
                 append(Metadata.TASK.name).
             append(')');
-            db.execSQL(sql.toString());
+        database.execSQL(sql.toString());
     }
 
-        /**
-         * Called to upgrade the database to a new version
-         */
     @Override
-        public synchronized void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
+    protected boolean onUpgrade(int oldVersion, int newVersion) {
-            Log.w("database-" + NAME, String.format("Upgrading database from version %d to %d.",
+        return false;
-                    oldVersion, newVersion));
-
-            switch(oldVersion) {
-            default:
-                // We don't know how to handle this case because someone forgot to
-                // implement the upgrade. We can't drop tables, we can only
-                // throw a nasty exception at this time
-
-                Log.e("database", "Unsupported migration, tables dropped!");
-                throw new IllegalStateException("Missing database migration " +
-                		"from " + oldVersion + " to " + newVersion);
-            }
-        }
-    }
-
-    /**
-     * Gets the underlying database
-     *
-     * @return database object
-     * @throws IllegalStateException if database hasn't been opened
-     */
-    @Override
-    public SQLiteDatabase getDatabase() {
-        if(database == null)
-            throw new IllegalStateException("Tried to access an unopened database"); //$NON-NLS-1$
-        return database;
     }
 
-    /**
-     * Get SQLite Helper
-     */
-    @Override
-    protected SQLiteOpenHelper getHelper() {
-        if(helper == null) {
-            helper = new AstridSQLiteOpenHelper(ContextManager.getContext(),
-                    NAME, null, VERSION);
-        }
-        return helper;
-    }
-
-    /**
-     * Close the database. This should be used with caution, as there is
-     * usually only one global connection to the database
-     */
-    @Override
-    public synchronized void close() {
-        super.close();
-        helper = null;
-    }
 }
 

src/com/todoroo/astrid/dao/MetadataDao.java

@@ -10,10 +10,11 @@ import android.database.Cursor;
 import com.thoughtworks.sql.Criterion;
 import com.thoughtworks.sql.Join;
 import com.thoughtworks.sql.Query;
-import com.todoroo.andlib.data.AbstractDao;
+import com.todoroo.andlib.data.GenericDao;
-import com.todoroo.andlib.data.AbstractDatabase;
 import com.todoroo.andlib.data.Property;
 import com.todoroo.andlib.data.TodorooCursor;
+import com.todoroo.andlib.service.Autowired;
+import com.todoroo.andlib.service.DependencyInjectionService;
 import com.todoroo.astrid.model.Metadata;
 import com.todoroo.astrid.model.Task;
 
@@ -23,10 +24,15 @@ import com.todoroo.astrid.model.Task;
  * @author Tim Su <tim@todoroo.com>
  *
  */
-public class MetadataDao extends AbstractDao<Metadata> {
+public class MetadataDao extends GenericDao<Metadata> {
+
+    @Autowired
+    Database database;
 
 	public MetadataDao() {
         super(Metadata.class);
+        DependencyInjectionService.getInstance().inject(this);
+        setDatabase(database);
     }
 
     // --- SQL clause generators
@@ -56,7 +62,7 @@ public class MetadataDao extends AbstractDao<Metadata> {
      * @param properties
      * @return
      */
-    public TodorooCursor<Metadata> fetchDangling(AbstractDatabase database, Property<?>[] properties) {
+    public TodorooCursor<Metadata> fetchDangling(Property<?>[] properties) {
         Query sql = Query.select(properties).from(Metadata.TABLE).join(Join.left(Task.TABLE,
                 Metadata.TASK.eq(Task.ID))).where(Task.TITLE.isNull());
         Cursor cursor = database.getDatabase().rawQuery(sql.toString(), null);

src/com/todoroo/astrid/dao/TaskDao.java

@@ -10,8 +10,7 @@ import android.content.Context;
 import android.content.Intent;
 
 import com.thoughtworks.sql.Criterion;
-import com.todoroo.andlib.data.AbstractDao;
+import com.todoroo.andlib.data.GenericDao;
-import com.todoroo.andlib.data.AbstractDatabase;
 import com.todoroo.andlib.service.Autowired;
 import com.todoroo.andlib.service.ContextManager;
 import com.todoroo.andlib.service.DependencyInjectionService;
@@ -26,14 +25,18 @@ import com.todoroo.astrid.model.Task;
  * @author Tim Su <tim@todoroo.com>
  *
  */
-public class TaskDao extends AbstractDao<Task> {
+public class TaskDao extends GenericDao<Task> {
 
     @Autowired
     MetadataDao metadataDao;
 
+    @Autowired
+    Database database;
+
 	public TaskDao() {
         super(Task.class);
         DependencyInjectionService.getInstance().inject(this);
+        setDatabase(database);
     }
 
     // --- SQL clause generators
@@ -100,13 +103,13 @@ public class TaskDao extends AbstractDao<Task> {
      * @return true if delete was successful
      */
     @Override
-    public boolean delete(AbstractDatabase database, long id) {
+    public boolean delete(long id) {
-        boolean result = super.delete(database, id);
+        boolean result = super.delete(id);
         if(!result)
             return false;
 
         // delete all metadata
-        metadataDao.deleteWhere(database, MetadataCriteria.byTask(id));
+        metadataDao.deleteWhere(MetadataCriteria.byTask(id));
 
         return true;
     }
@@ -119,21 +122,21 @@ public class TaskDao extends AbstractDao<Task> {
      *
      * @param duringSync whether this save occurs as part of a sync
      */
-    public boolean save(Database database, Task task, boolean duringSync) {
+    public boolean save(Task task, boolean duringSync) {
         boolean saveSuccessful;
 
         if (task.getId() == Task.NO_ID) {
             task.setValue(Task.CREATION_DATE, DateUtilities.now());
             task.setValue(Task.MODIFICATION_DATE, DateUtilities.now());
-            saveSuccessful = createItem(database, task);
+            saveSuccessful = createItem(task);
         } else {
             ContentValues values = task.getSetValues();
             if(values.size() == 0)
                 return true;
             task.setValue(Task.MODIFICATION_DATE, DateUtilities.now());
-            beforeSave(database, task, values, duringSync);
+            beforeSave(task, values, duringSync);
-            saveSuccessful = saveItem(database, task);
+            saveSuccessful = saveItem(task);
-            afterSave(database, task, values, duringSync);
+            afterSave(task, values, duringSync);
         }
 
         return saveSuccessful;
@@ -153,7 +156,7 @@ public class TaskDao extends AbstractDao<Task> {
      * @param duringSync
      *            whether this save occurs as part of a sync
      */
-    private void beforeSave(Database database, Task task, ContentValues values, boolean duringSync) {
+    private void beforeSave(Task task, ContentValues values, boolean duringSync) {
         //
     }
 
@@ -168,7 +171,7 @@ public class TaskDao extends AbstractDao<Task> {
      * @param values values to be persisted to the database
      * @param duringSync whether this save occurs as part of a sync
      */
-    private void afterSave(Database database, Task task, ContentValues values, boolean duringSync) {
+    private void afterSave(Task task, ContentValues values, boolean duringSync) {
         if(duringSync)
             return;
 

src/com/todoroo/astrid/service/MetadataService.java

@@ -6,10 +6,8 @@ import com.thoughtworks.sql.Query;
 import com.todoroo.andlib.data.Property;
 import com.todoroo.andlib.data.TodorooCursor;
 import com.todoroo.andlib.data.Property.CountProperty;
-import com.todoroo.andlib.data.Property.IntegerProperty;
 import com.todoroo.andlib.service.Autowired;
 import com.todoroo.andlib.service.DependencyInjectionService;
-import com.todoroo.astrid.dao.Database;
 import com.todoroo.astrid.dao.MetadataDao;
 import com.todoroo.astrid.model.Metadata;
 
@@ -21,9 +19,6 @@ import com.todoroo.astrid.model.Metadata;
  */
 public class MetadataService {
 
-    @Autowired
-    private Database database;
-
     @Autowired
     private MetadataDao metadataDao;
 
@@ -53,14 +48,14 @@ public class MetadataService {
      * Clean up metadata. Typically called on startup
      */
     public void cleanup() {
-        TodorooCursor<Metadata> cursor = metadataDao.fetchDangling(database, idProperty());
+        TodorooCursor<Metadata> cursor = metadataDao.fetchDangling(idProperty());
         try {
             if(cursor.getCount() == 0)
                 return;
 
             for(cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) {
                 long id = cursor.getLong(0);
-                metadataDao.delete(database, id);
+                metadataDao.delete(id);
             }
         } finally {
             cursor.close();
@@ -72,14 +67,13 @@ public class MetadataService {
      * @param where SQL where clause
      * @param onlyCountsGreaterThanZero only include items where count > 0
      */
-    public TodorooCursor<Metadata> fetchWithCount(Criterion where, Order order,
+    public TodorooCursor<Metadata> fetchWithCount(CountProperty count,
-            boolean onlyCountsGreaterThanZero) {
+            Criterion where, Order order, boolean onlyCountsGreaterThanZero) {
-        IntegerProperty count = new CountProperty();
         Query query = Query.select(Metadata.VALUE, count).
             where(where).orderBy(order);
         if(onlyCountsGreaterThanZero)
             query.having(count.gt(0));
-        TodorooCursor<Metadata> cursor = metadataDao.query(database, query);
+        TodorooCursor<Metadata> cursor = metadataDao.query(query);
         return cursor;
     }
 
@@ -88,6 +82,6 @@ public class MetadataService {
      * @param where
      */
     public void deleteWhere(Criterion where) {
-        metadataDao.deleteWhere(database, where);
+        metadataDao.deleteWhere(where);
     }
 }

src/com/todoroo/astrid/service/TaskService.java

@@ -6,8 +6,6 @@ import com.todoroo.andlib.data.TodorooCursor;
 import com.todoroo.andlib.service.Autowired;
 import com.todoroo.andlib.service.DependencyInjectionService;
 import com.todoroo.andlib.utility.DateUtilities;
-import com.todoroo.astrid.api.Filter;
-import com.todoroo.astrid.dao.Database;
 import com.todoroo.astrid.dao.TaskDao;
 import com.todoroo.astrid.dao.TaskDao.TaskCriteria;
 import com.todoroo.astrid.model.Task;
@@ -20,9 +18,6 @@ import com.todoroo.astrid.model.Task;
  */
 public class TaskService {
 
-    @Autowired
-    private Database database;
-
     @Autowired
     private TaskDao taskDao;
 
@@ -30,15 +25,6 @@ public class TaskService {
         DependencyInjectionService.getInstance().inject(this);
     }
 
-    // --- property list
-
-    /**
-     * @return property list containing just task id's
-     */
-    public static Property<?>[] idProperties() {
-        return new Property<?>[] { Task.ID };
-    }
-
     // --- service layer
 
     /**
@@ -47,9 +33,8 @@ public class TaskService {
      * @param id id
      * @return item, or null if it doesn't exist
      */
-    public Task fetchById(Property<?>[] properties,
+    public Task fetchById(long id, Property<?>... properties) {
-            long id) {
+        return taskDao.fetch(id, properties);
-        return taskDao.fetch(database, properties, id);
     }
 
     /**
@@ -63,7 +48,7 @@ public class TaskService {
         else
             item.setValue(Task.COMPLETION_DATE, 0);
 
-        taskDao.save(database, item, false);
+        taskDao.save(item, false);
     }
 
     /**
@@ -75,7 +60,7 @@ public class TaskService {
      *            determines which pre and post save hooks get run
      */
     public boolean save(Task item, boolean isDuringSync) {
-        return taskDao.save(database, item, isDuringSync);
+        return taskDao.save(item, isDuringSync);
     }
 
     /**
@@ -84,37 +69,26 @@ public class TaskService {
      * @param model
      */
     public void delete(long itemId) {
-        taskDao.delete(database, itemId);
+        taskDao.delete(itemId);
     }
 
     /**
      * Clean up tasks. Typically called on startup
      */
     public void cleanup() {
-        TodorooCursor<Task> cursor = taskDao.query(database,
+        TodorooCursor<Task> cursor = taskDao.query(
-                Query.select(idProperties()).where(TaskCriteria.hasNoTitle()));
+                Query.select(Task.ID).where(TaskCriteria.hasNoTitle()));
         try {
             if(cursor.getCount() == 0)
                 return;
 
             for(cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) {
                 long id = cursor.getLong(0);
-                taskDao.delete(database, id);
+                taskDao.delete(id);
             }
         } finally {
             cursor.close();
         }
     }
 
-    /**
-     * Invoke the sql in the filter for sqlforNewTasks
-     *
-     * @param filter
-     * @param task
-     */
-    public void invokeSqlForNewTask(Filter filter, Task task) {
-        String sql = filter.sqlForNewTasks.replace("$ID", //$NON-NLS-1$
-                Long.toString(task.getId()));
-        database.getDatabase().execSQL(sql);
-    }
 }

src/com/todoroo/astrid/service/UpgradeService.java

@@ -11,29 +11,37 @@ import android.database.sqlite.SQLiteDatabase.CursorFactory;
 
 import com.timsu.astrid.data.AbstractController;
 import com.timsu.astrid.data.task.AbstractTaskModel;
-import com.todoroo.andlib.data.AbstractDao;
 import com.todoroo.andlib.data.AbstractModel;
+import com.todoroo.andlib.data.GenericDao;
 import com.todoroo.andlib.data.Property;
 import com.todoroo.andlib.data.Property.PropertyVisitor;
 import com.todoroo.andlib.service.Autowired;
 import com.todoroo.andlib.service.ContextManager;
 import com.todoroo.andlib.service.DependencyInjectionService;
-import com.todoroo.astrid.dao.Database;
+import com.todoroo.astrid.dao.MetadataDao;
 import com.todoroo.astrid.dao.TaskDao;
+import com.todoroo.astrid.model.Metadata;
 import com.todoroo.astrid.model.Task;
+import com.todoroo.astrid.tags.DataService;
 
 
 public final class UpgradeService {
 
     @Autowired
-    private Database database;
+    private TaskDao taskDao;
 
     @Autowired
-    private TaskDao taskDao;
+    private MetadataDao metadataDao;
 
     @Autowired
     private String tasksTable;
 
+    @Autowired
+    private String tagsTable;
+
+    @Autowired
+    private String tagTaskTable;
+
     // --- implementation
 
     public UpgradeService() {
@@ -62,7 +70,7 @@ public final class UpgradeService {
     /**
      * Upgrade helper class that reads a database
      */
-    private class Astrid2UpgradeHelper extends SQLiteOpenHelper {
+    private static class Astrid2UpgradeHelper extends SQLiteOpenHelper {
 
         private String name;
 
@@ -117,14 +125,18 @@ public final class UpgradeService {
         propertyMap.put(AbstractTaskModel.COMPLETION_DATE, Task.COMPLETION_DATE);
         propertyMap.put(AbstractTaskModel.CALENDAR_URI, Task.CALENDAR_URI);
         propertyMap.put(AbstractTaskModel.FLAGS, Task.FLAGS);
-        upgradeTasksTable(context, tasksTable,
+        upgradeTable(context, tasksTable,
                 propertyMap, new Task(), taskDao);
 
-        // --- upgrade tags table
+        // --- upgrade tags tables
+        migrateTagsToMetadata();
+
 
     }
 
-    protected static class UpgradeVisitorContainer {
+    // --- database upgrade helpers
+
+    protected static final class UpgradeVisitorContainer {
         public int columnIndex;
         public Cursor cursor;
         public AbstractModel model;
@@ -135,7 +147,7 @@ public final class UpgradeService {
      * @author Tim Su <tim@todoroo.com>
      *
      */
-    protected class ColumnUpgradeVisitor implements PropertyVisitor<Void, UpgradeVisitorContainer> {
+    protected static final class ColumnUpgradeVisitor implements PropertyVisitor<Void, UpgradeVisitorContainer> {
         @Override
         public Void visitDouble(Property<Double> property, UpgradeVisitorContainer data) {
             double value = data.cursor.getDouble(data.columnIndex);
@@ -190,9 +202,9 @@ public final class UpgradeService {
      * @param dao
      */
     @SuppressWarnings("nls")
-    private <TYPE extends AbstractModel> void upgradeTasksTable(Context context, String legacyTable,
+    private static final <TYPE extends AbstractModel> void upgradeTable(Context context, String legacyTable,
             HashMap<String, Property<?>> propertyMap, TYPE model,
-            AbstractDao<TYPE> dao) {
+            GenericDao<TYPE> dao) {
 
         SQLiteDatabase upgradeDb = new Astrid2UpgradeHelper(context, legacyTable,
                 null, 1).getReadableDatabase();
@@ -208,11 +220,58 @@ public final class UpgradeService {
                 container.columnIndex = cursor.getColumnIndex(entry.getKey());
                 entry.getValue().accept(visitor, container);
             }
-            dao.createItem(database, container.model);
+            dao.createItem(container.model);
         }
 
         upgradeDb.close();
-        context.deleteDatabase(legacyTable);
+    }
+
+    /**
+     * Move data from tags tables into metadata table. We do this by looping
+     * through both the tags and tagTaskMap databases, reading data from
+     * both and adding to the Metadata table. This way, we are able to
+     * do everything in one pass without loading too much into memory
+     */
+    @SuppressWarnings("nls")
+    private void migrateTagsToMetadata() {
+        Context context = ContextManager.getContext();
+        SQLiteDatabase tagsDb = new Astrid2UpgradeHelper(context, tagsTable,
+                null, 1).getReadableDatabase();
+        SQLiteDatabase tagTaskDb = new Astrid2UpgradeHelper(context, tagTaskTable,
+                null, 1).getReadableDatabase();
+
+        Cursor tagCursor = tagsDb.rawQuery("SELECT _id, name FROM " + tagsTable +
+                " ORDER BY _id ASC", null);
+        Cursor mapCursor = tagTaskDb.rawQuery("SELECT tag, task FROM " + tagTaskTable +
+                " ORDER BY tag ASC", null);
+
+        if(tagCursor.getCount() == 0)
+            return;
+
+        Metadata metadata = new Metadata();
+        metadata.setValue(Metadata.KEY, DataService.KEY);
+        long tagId = -1;
+        String tag = null;
+        for(mapCursor.moveToFirst(); !mapCursor.isAfterLast(); mapCursor.moveToNext()) {
+            long mapTagId = mapCursor.getLong(1);
+
+            while(mapTagId > tagId && !tagCursor.isLast()) {
+                tagCursor.moveToNext();
+                tagId = tagCursor.getLong(1);
+            }
+
+            if(mapTagId == tagId) {
+                if(tag == null)
+                    tag = tagCursor.getString(2);
+                long task = mapCursor.getLong(2);
+                metadata.setValue(Metadata.TASK, task);
+                metadata.setValue(Metadata.VALUE, tag);
+                metadataDao.createItem(metadata);
+            }
+        }
+
+        tagCursor.close();
+        mapCursor.close();
     }
 
 }

tests/src/com/todoroo/astrid/dao/MetadataDaoTests.java

@@ -25,7 +25,7 @@ public class MetadataDaoTests extends DatabaseTestCase {
      * Test basic creation, fetch, and save
      */
     public void testCrud() throws Exception {
-        TodorooCursor<Metadata> cursor = metadataDao.query(database,
+        TodorooCursor<Metadata> cursor = metadataDao.query(
                 Query.select(MetadataService.idProperty()));
         assertEquals(0, cursor.getCount());
         cursor.close();
@@ -33,21 +33,21 @@ public class MetadataDaoTests extends DatabaseTestCase {
         // create "happy"
         Metadata metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "happy");
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
-        cursor = metadataDao.query(database,
+        cursor = metadataDao.query(
                 Query.select(MetadataService.idProperty()));
         assertEquals(1, cursor.getCount());
         cursor.close();
         long happyId = metadata.getId();
         assertNotSame(Metadata.NO_ID, happyId);
-        metadata = metadataDao.fetch(database, KEYS, happyId);
+        metadata = metadataDao.fetch(happyId, KEYS);
         assertEquals("happy", metadata.getValue(Metadata.KEY));
 
         // create "sad"
         metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "sad");
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
-        cursor = metadataDao.query(database, Query.select(MetadataService.idProperty()));
+        cursor = metadataDao.query(Query.select(MetadataService.idProperty()));
         assertEquals(2, cursor.getCount());
         cursor.close();
 
@@ -55,21 +55,21 @@ public class MetadataDaoTests extends DatabaseTestCase {
         long sadId = metadata.getId();
         assertNotSame(Metadata.NO_ID, sadId);
         metadata.setValue(Metadata.KEY, "melancholy");
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
-        cursor = metadataDao.query(database,
+        cursor = metadataDao.query(
                 Query.select(MetadataService.idProperty()));
         assertEquals(2, cursor.getCount());
         cursor.close();
 
         // check state
-        metadata = metadataDao.fetch(database, KEYS, happyId);
+        metadata = metadataDao.fetch(happyId, KEYS);
         assertEquals("happy", metadata.getValue(Metadata.KEY));
-        metadata = metadataDao.fetch(database, KEYS, sadId);
+        metadata = metadataDao.fetch(sadId, KEYS);
         assertEquals("melancholy", metadata.getValue(Metadata.KEY));
 
         // delete sad
-        assertTrue(metadataDao.delete(database, sadId));
+        assertTrue(metadataDao.delete(sadId));
-        cursor = metadataDao.query(database,
+        cursor = metadataDao.query(
                 Query.select(KEYS));
         assertEquals(1, cursor.getCount());
         cursor.moveToFirst();
@@ -86,20 +86,20 @@ public class MetadataDaoTests extends DatabaseTestCase {
         Metadata metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "with1");
         metadata.setValue(Metadata.TASK, 1L);
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
 
         metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "with2");
         metadata.setValue(Metadata.TASK, 2L);
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
 
         metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "with1");
         metadata.setValue(Metadata.TASK, 1L);
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
 
 
-        TodorooCursor<Metadata> cursor = metadataDao.query(database,
+        TodorooCursor<Metadata> cursor = metadataDao.query(
                 Query.select(KEYS).where(MetadataCriteria.byTask(1)));
         assertEquals(2, cursor.getCount());
         cursor.moveToFirst();
@@ -110,14 +110,14 @@ public class MetadataDaoTests extends DatabaseTestCase {
         assertEquals("with1", metadata.getValue(Metadata.KEY));
         cursor.close();
 
-        cursor = metadataDao.query(database,
+        cursor = metadataDao.query(
                 Query.select(KEYS).where(MetadataCriteria.byTask(3)));
         assertEquals(0, cursor.getCount());
         cursor.close();
 
-        int deleted = metadataDao.deleteWhere(database, MetadataCriteria.byTask(1));
+        int deleted = metadataDao.deleteWhere(MetadataCriteria.byTask(1));
         assertEquals(2, deleted);
-        cursor = metadataDao.query(database,
+        cursor = metadataDao.query(
                 Query.select(KEYS));
         assertEquals(1, cursor.getCount());
         cursor.close();
@@ -128,59 +128,59 @@ public class MetadataDaoTests extends DatabaseTestCase {
      */
     public void testFetchDangling() throws Exception {
         // fetch with nothing in db
-        TodorooCursor<Metadata> cursor = metadataDao.fetchDangling(database, KEYS);
+        TodorooCursor<Metadata> cursor = metadataDao.fetchDangling(KEYS);
         assertEquals(0, cursor.getCount());
         cursor.close();
 
         Task task1 = new Task();
-        taskDao.save(database, task1);
+        taskDao.save(task1);
         Task task2 = new Task();
-        taskDao.save(database, task2);
+        taskDao.save(task2);
         Task task3 = new Task();
-        taskDao.save(database, task3);
+        taskDao.save(task3);
 
         // fetch with only tasks
-        cursor = metadataDao.fetchDangling(database, KEYS);
+        cursor = metadataDao.fetchDangling(KEYS);
         assertEquals(0, cursor.getCount());
         cursor.close();
 
         Metadata metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "with1");
         metadata.setValue(Metadata.TASK, task1.getId());
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
 
         metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "with2");
         metadata.setValue(Metadata.TASK, task2.getId());
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
 
         metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "with3");
         metadata.setValue(Metadata.TASK, task3.getId());
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
 
         // fetch with tasks and corresponding metadata
-        cursor = metadataDao.fetchDangling(database, KEYS);
+        cursor = metadataDao.fetchDangling(KEYS);
         assertEquals(0, cursor.getCount());
         cursor.close();
 
         long task2Id = task2.getId();
-        taskDao.delete(database, task2.getId());
+        taskDao.delete(task2.getId());
 
         // note: we should not have any dangling, since deleting a task
         // will automatically delete metadata
-        cursor = metadataDao.fetchDangling(database, KEYS);
+        cursor = metadataDao.fetchDangling(KEYS);
         assertEquals(0, cursor.getCount());
         cursor.close();
 
         metadata = new Metadata();
         metadata.setValue(Metadata.KEY, "with2");
         metadata.setValue(Metadata.TASK, task2Id);
-        assertTrue(metadataDao.save(database, metadata));
+        assertTrue(metadataDao.save(metadata));
 
         // but if we simulate something bad happening by creating
         // it manually.. well, what can i say, it should be broken
-        cursor = metadataDao.fetchDangling(database, KEYS);
+        cursor = metadataDao.fetchDangling(KEYS);
         assertEquals(1, cursor.getCount());
         cursor.moveToFirst();
         metadata.readFromCursor(cursor, KEYS);

tests/src/com/todoroo/astrid/dao/TaskDaoTests.java

@@ -24,7 +24,7 @@ public class TaskDaoTests extends DatabaseTestCase {
      * Test basic task creation, fetch, and save
      */
     public void testTaskCreation() throws Exception {
-        TodorooCursor<Task> cursor = taskDao.query(database,
+        TodorooCursor<Task> cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(0, cursor.getCount());
         cursor.close();
@@ -32,21 +32,21 @@ public class TaskDaoTests extends DatabaseTestCase {
         // create task "happy"
         Task task = new Task();
         task.setValue(Task.TITLE, "happy");
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
-        cursor = taskDao.query(database,
+        cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(1, cursor.getCount());
         cursor.close();
         long happyId = task.getId();
         assertNotSame(Task.NO_ID, happyId);
-        task = taskDao.fetch(database, TITLES, happyId);
+        task = taskDao.fetch(happyId, TITLES);
         assertEquals("happy", task.getValue(Task.TITLE));
 
         // create task "sad"
         task = new Task();
         task.setValue(Task.TITLE, "sad");
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
-        cursor = taskDao.query(database,
+        cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(2, cursor.getCount());
         cursor.close();
@@ -55,16 +55,16 @@ public class TaskDaoTests extends DatabaseTestCase {
         long sadId = task.getId();
         assertNotSame(Task.NO_ID, sadId);
         task.setValue(Task.TITLE, "melancholy");
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
-        cursor = taskDao.query(database,
+        cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(2, cursor.getCount());
         cursor.close();
 
         // check state
-        task = taskDao.fetch(database, TITLES, happyId);
+        task = taskDao.fetch(happyId, TITLES);
         assertEquals("happy", task.getValue(Task.TITLE));
-        task = taskDao.fetch(database, TITLES, sadId);
+        task = taskDao.fetch(sadId,TITLES);
         assertEquals("melancholy", task.getValue(Task.TITLE));
     }
 
@@ -75,38 +75,38 @@ public class TaskDaoTests extends DatabaseTestCase {
         // create normal task
         Task task = new Task();
         task.setValue(Task.TITLE, "normal");
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
 
         // create blank task
         task = new Task();
         task.setValue(Task.TITLE, "");
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
 
         // create hidden task
         task = new Task();
         task.setValue(Task.TITLE, "hidden");
         task.setValue(Task.HIDDEN_UNTIL, DateUtilities.now() + 10000);
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
 
         // create task with deadlines
         task = new Task();
         task.setValue(Task.TITLE, "deadlineInFuture");
         task.setValue(Task.DUE_DATE, DateUtilities.now() + 10000);
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
 
         task = new Task();
         task.setValue(Task.TITLE, "deadlineInPast");
         task.setValue(Task.DUE_DATE, DateUtilities.now() - 10000);
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
 
         // create completed task
         task = new Task();
         task.setValue(Task.TITLE, "completed");
         task.setValue(Task.COMPLETION_DATE, DateUtilities.now() - 10000);
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
 
         // check has no name
-        TodorooCursor<Task> cursor = taskDao.query(database,
+        TodorooCursor<Task> cursor = taskDao.query(
                 Query.select(TITLES).where(TaskCriteria.hasNoTitle()));
         assertEquals(1, cursor.getCount());
         cursor.moveToNext();
@@ -114,7 +114,7 @@ public class TaskDaoTests extends DatabaseTestCase {
         cursor.close();
 
         // check has deadlines
-        cursor = taskDao.query(database, Query.select(TITLES).where(
+        cursor = taskDao.query(Query.select(TITLES).where(
                 TaskCriteria.hasDeadlines()).orderBy(Order.asc(Task.DUE_DATE)));
         assertEquals(2, cursor.getCount());
         cursor.moveToNext();
@@ -124,30 +124,30 @@ public class TaskDaoTests extends DatabaseTestCase {
         cursor.close();
 
         // check is active
-        cursor = taskDao.query(database, Query.select(TITLES).where(TaskCriteria.
+        cursor = taskDao.query(Query.select(TITLES).where(TaskCriteria.
                 isActive()));
         assertEquals(5, cursor.getCount());
         cursor.close();
 
         // check due before / after
-        cursor = taskDao.query(database, Query.select(TITLES).where(TaskCriteria.
+        cursor = taskDao.query(Query.select(TITLES).where(TaskCriteria.
                 dueBefore(DateUtilities.now())));
         cursor.moveToNext();
         assertEquals(1, cursor.getCount());
         cursor.close();
-        cursor = taskDao.query(database, Query.select(TITLES).where(TaskCriteria.
+        cursor = taskDao.query(Query.select(TITLES).where(TaskCriteria.
                 dueAfter(DateUtilities.now())));
         assertEquals(1, cursor.getCount());
         cursor.close();
 
         // check completed before
-        cursor = taskDao.query(database, Query.select(TITLES).where(TaskCriteria.
+        cursor = taskDao.query(Query.select(TITLES).where(TaskCriteria.
                 completedBefore(DateUtilities.now())));
         assertEquals(1, cursor.getCount());
         cursor.close();
 
         // check is visible
-        cursor = taskDao.query(database, Query.select(TITLES).where(TaskCriteria.
+        cursor = taskDao.query(Query.select(TITLES).where(TaskCriteria.
                 isVisible(DateUtilities.now())));
         assertEquals(5, cursor.getCount());
         cursor.close();
@@ -157,7 +157,7 @@ public class TaskDaoTests extends DatabaseTestCase {
      * Test task deletion
      */
     public void testTDeletion() throws Exception {
-        TodorooCursor<Task> cursor = taskDao.query(database,
+        TodorooCursor<Task> cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(0, cursor.getCount());
         cursor.close();
@@ -165,16 +165,16 @@ public class TaskDaoTests extends DatabaseTestCase {
         // create task "happy"
         Task task = new Task();
         task.setValue(Task.TITLE, "happy");
-        assertTrue(taskDao.save(database, task, false));
+        assertTrue(taskDao.save(task, false));
-        cursor = taskDao.query(database,
+        cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(1, cursor.getCount());
         cursor.close();
 
         // delete
         long happyId = task.getId();
-        assertTrue(taskDao.delete(database, happyId));
+        assertTrue(taskDao.delete(happyId));
-        cursor = taskDao.query(database,
+        cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(0, cursor.getCount());
         cursor.close();
@@ -191,9 +191,9 @@ public class TaskDaoTests extends DatabaseTestCase {
         task.setValue(Task.TITLE, "happy");
         task.setValue(Task.ID, 1L);
 
-        assertFalse(taskDao.save(database, task, false));
+        assertFalse(taskDao.save(task, false));
 
-        cursor = taskDao.query(database,
+        cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(0, cursor.getCount());
         cursor.close();
@@ -205,17 +205,17 @@ public class TaskDaoTests extends DatabaseTestCase {
     public void testInvalidIndex() throws Exception {
         TodorooCursor<Task> cursor;
 
-        cursor = taskDao.query(database,
+        cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(0, cursor.getCount());
         cursor.close();
 
-        assertNull(taskDao.fetch(database, IDS, 1));
+        assertNull(taskDao.fetch(1, IDS));
 
-        assertFalse(taskDao.delete(database, 1));
+        assertFalse(taskDao.delete(1));
 
         // make sure db still works
-        cursor = taskDao.query(database,
+        cursor = taskDao.query(
                 Query.select(IDS));
         assertEquals(0, cursor.getCount());
         cursor.close();

tests/src/com/todoroo/astrid/test/DatabaseTestCase.java

@@ -5,6 +5,7 @@ import com.todoroo.andlib.service.TestDependencyInjector;
 import com.todoroo.andlib.test.TodorooTestCase;
 import com.todoroo.astrid.dao.Database;
 import com.todoroo.astrid.service.AstridDependencyInjector;
+import com.todoroo.astrid.tagsold.TagsDatabase;
 
 /**
  * Test case that automatically sets up and tears down a test database
@@ -19,6 +20,7 @@ public class DatabaseTestCase extends TodorooTestCase {
     private static final String TAG_TASK_TEST = "tagtasktest";
     private static final String TAGS_TEST = "tagstest";
     private static final String TASKS_TEST = "taskstest";
+
     @Autowired
 	public Database database;
 
@@ -42,6 +44,10 @@ public class DatabaseTestCase extends TodorooTestCase {
         database.clear();
 		database.openForWriting();
 
+		// and plugin databases too
+		TagsDatabase tagsDatabase = new TagsDatabase();
+		tagsDatabase.clear();
+
 		// clear legacy databases
 		getContext().deleteDatabase(TASKS_TEST);
 		getContext().deleteDatabase(TAGS_TEST);
@@ -55,7 +61,7 @@ public class DatabaseTestCase extends TodorooTestCase {
 		database.close();
 	}
 
-	public static class TestDatabase extends Database {
+	public static class TestDatabase extends TagsDatabase {
 	    private static final String NAME = "databasetest";
 
         @Override

tests/src/com/todoroo/astrid/upgrade/Astrid2To3UpgradeTests.java

@@ -8,11 +8,15 @@ import com.todoroo.andlib.service.Autowired;
 import com.todoroo.astrid.dao.TaskDao;
 import com.todoroo.astrid.legacy.data.enums.Importance;
 import com.todoroo.astrid.legacy.data.enums.RepeatInterval;
+import com.todoroo.astrid.legacy.data.tag.TagController;
+import com.todoroo.astrid.legacy.data.tag.TagIdentifier;
 import com.todoroo.astrid.legacy.data.task.TaskController;
 import com.todoroo.astrid.legacy.data.task.TaskModelForEdit;
 import com.todoroo.astrid.legacy.data.task.AbstractTaskModel.RepeatInfo;
 import com.todoroo.astrid.model.Task;
 import com.todoroo.astrid.service.UpgradeService;
+import com.todoroo.astrid.tagsold.Tag;
+import com.todoroo.astrid.tagsold.TagService;
 import com.todoroo.astrid.test.DatabaseTestCase;
 
 public class Astrid2To3UpgradeTests extends DatabaseTestCase {
@@ -39,7 +43,7 @@ public class Astrid2To3UpgradeTests extends DatabaseTestCase {
         taskController.close();
         upgrade2To3();
 
-        TodorooCursor<Task> tasks = taskDao.query(database, Query.select(Task.PROPERTIES));
+        TodorooCursor<Task> tasks = taskDao.query(Query.select(Task.PROPERTIES));
         assertEquals(0, tasks.getCount());
     }
 
@@ -72,13 +76,8 @@ public class Astrid2To3UpgradeTests extends DatabaseTestCase {
         // upgrade
         upgrade2To3();
 
-        // verify that it ain't no more in the legacy table
-        taskController.open();
-        assertEquals(0, taskController.getAllTaskIdentifiers().size());
-        taskController.close();
-
         // verify that data exists in our new table
-        TodorooCursor<Task> tasks = taskDao.query(database, Query.select(Task.PROPERTIES));
+        TodorooCursor<Task> tasks = taskDao.query(Query.select(Task.PROPERTIES));
         assertEquals(2, tasks.getCount());
 
         tasks.moveToFirst();
@@ -106,5 +105,40 @@ public class Astrid2To3UpgradeTests extends DatabaseTestCase {
     }
 
 
+    public void testTagTableUpgrade() {
+        TaskController taskController = new TaskController(getContext());
+        taskController.open();
+        TagController tagController = new TagController(getContext());
+        tagController.open();
+
+        // create some ish
+        TagIdentifier tasty = tagController.createTag("tasty");
+        TagIdentifier salty = tagController.createTag("salty");
+
+        TaskModelForEdit peanut = new TaskModelForEdit();
+        TaskModelForEdit icecream = new TaskModelForEdit();
+        TaskModelForEdit pickle = new TaskModelForEdit();
+        taskController.saveTask(peanut, false);
+        taskController.saveTask(icecream, false);
+        taskController.saveTask(pickle, false);
+        tagController.addTag(peanut.getTaskIdentifier(), tasty);
+        tagController.addTag(peanut.getTaskIdentifier(), salty);
+        tagController.addTag(icecream.getTaskIdentifier(), tasty);
+
+        // assert created
+        assertEquals(2, tagController.getAllTags().size());
+
+        // upgrade
+        upgrade2To3();
+
+        // verify that data exists in our new table
+        TagService tagService = new TagService();
+        TodorooCursor<Tag> tags = tagService.getAllTags(Tag.NAME);
+        assertEquals(2, tags.getCount());
+
+    }
+
 
 }
+
+