Simplify dangling metadata cleanup

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

@@ -10,7 +10,6 @@ import com.todoroo.andlib.data.TodorooCursor;
 import com.todoroo.andlib.sql.Query;
 import com.todoroo.astrid.dao.MetadataDao.MetadataCriteria;
 import com.todoroo.astrid.data.Metadata;
-import com.todoroo.astrid.data.Task;
 import com.todoroo.astrid.test.DatabaseTestCase;
 
 import javax.inject.Inject;
@@ -124,70 +123,4 @@ public class MetadataDaoTests extends DatabaseTestCase {
         assertEquals(1, cursor.getCount());
         cursor.close();
     }
-
-    /**
-     * Test metadata deletion
-     */
-    public void disabled_testFetchDangling() throws Exception {
-        // fetch with nothing in db
-        TodorooCursor<Metadata> cursor = metadataDao.fetchDangling(KEYS);
-        assertEquals(0, cursor.getCount());
-        cursor.close();
-
-        Task task1 = new Task();
-        taskDao.persist(task1);
-        Task task2 = new Task();
-        taskDao.persist(task2);
-        Task task3 = new Task();
-        taskDao.persist(task3);
-
-        // fetch with only tasks
-        cursor = metadataDao.fetchDangling(KEYS);
-        assertEquals(0, cursor.getCount());
-        cursor.close();
-
-        Metadata metadata = new Metadata();
-        metadata.setKey("with1");
-        metadata.setTask(task1.getId());
-        assertTrue(metadataDao.persist(metadata));
-
-        metadata = new Metadata();
-        metadata.setKey("with2");
-        metadata.setTask(task2.getId());
-        assertTrue(metadataDao.persist(metadata));
-
-        metadata = new Metadata();
-        metadata.setKey("with3");
-        metadata.setTask(task3.getId());
-        assertTrue(metadataDao.persist(metadata));
-
-        // fetch with tasks and corresponding metadata
-        cursor = metadataDao.fetchDangling(KEYS);
-        assertEquals(0, cursor.getCount());
-        cursor.close();
-
-        long task2Id = task2.getId();
-        taskDao.delete(task2.getId());
-
-        // note: we should not have any dangling, since deleting a task
-        // will automatically delete metadata
-        cursor = metadataDao.fetchDangling(KEYS);
-        assertEquals(0, cursor.getCount());
-        cursor.close();
-
-        metadata = new Metadata();
-        metadata.setKey("with2");
-        metadata.setTask(task2Id);
-        assertTrue(metadataDao.persist(metadata));
-
-        // but if we simulate something bad happening by creating
-        // it manually.. well, what can i say, it should be broken
-        cursor = metadataDao.fetchDangling(KEYS);
-        assertEquals(1, cursor.getCount());
-        cursor.moveToFirst();
-        metadata = new Metadata(cursor);
-        assertEquals("with2", metadata.getKey());
-        cursor.close();
-    }
-
 }

astrid/src/androidTest/java/com/todoroo/astrid/service/MetadataServiceTest.java

@@ -0,0 +1,61 @@
+package com.todoroo.astrid.service;
+
+import com.todoroo.astrid.dao.MetadataDao;
+import com.todoroo.astrid.dao.TaskDao;
+import com.todoroo.astrid.data.Metadata;
+import com.todoroo.astrid.data.Task;
+
+import org.tasks.injection.InjectingTestCase;
+
+import javax.inject.Inject;
+
+public class MetadataServiceTest extends InjectingTestCase {
+
+    @Inject MetadataService metadataService;
+    @Inject MetadataDao metadataDao;
+    @Inject TaskDao taskDao;
+
+    Metadata metadata;
+
+    @Override
+    public void setUp() {
+        super.setUp();
+        metadata = new Metadata();
+    }
+
+    public void testDontSaveMetadataWithoutTaskId() {
+        try {
+            metadataService.save(metadata);
+            fail("expected exception");
+        } catch(IllegalArgumentException e) {
+            assertTrue(e.getMessage().startsWith("metadata needs to be attached to a task"));
+        }
+    }
+
+    public void testSaveMetadata() {
+        metadata.setTask(1L);
+        metadataService.save(metadata);
+
+        assertNotNull(metadataDao.fetch(metadata.getId()));
+    }
+
+    public void testDontDeleteValidMetadata() {
+        final Task task = new Task();
+        taskDao.save(task);
+        metadata.setTask(task.getId());
+        metadataService.save(metadata);
+
+        metadataService.removeDanglingMetadata();
+
+        assertNotNull(metadataDao.fetch(metadata.getId()));
+    }
+
+    public void testDeleteDangling() {
+        metadata.setTask(1L);
+        metadataService.save(metadata);
+
+        metadataService.removeDanglingMetadata();
+
+        assertNull(metadataDao.fetch(1));
+    }
+}

astrid/src/androidTest/java/org/tasks/injection/TestModule.java

@@ -13,6 +13,7 @@ import com.todoroo.astrid.model.TaskTest;
 import com.todoroo.astrid.provider.Astrid3ProviderTests;
 import com.todoroo.astrid.reminders.ReminderServiceTest;
 import com.todoroo.astrid.repeats.NewRepeatTests;
+import com.todoroo.astrid.service.MetadataServiceTest;
 import com.todoroo.astrid.service.QuickAddMarkupTest;
 import com.todoroo.astrid.service.TitleParserTest;
 import com.todoroo.astrid.subtasks.SubtasksHelperTest;
@@ -43,7 +44,8 @@ import dagger.Provides;
         ReminderServiceTest.class,
         SubtasksHelperTest.class,
         SubtasksMovingTest.class,
-        SyncModelTest.class
+        SyncModelTest.class,
+        MetadataServiceTest.class
 })
 public class TestModule {
     private Context context;

astrid/src/main/java/com/todoroo/astrid/core/OldTaskPreferences.java

@@ -195,7 +195,7 @@ public class OldTaskPreferences extends TodorooPreferenceActivity {
                                     cursor.close();
                                 }
                                 int result = taskDeleter.purgeDeletedTasks();
-                                metadataService.cleanup();
+                                metadataService.removeDanglingMetadata();
                                 showResult(R.string.EPr_manage_purge_deleted_status, result);
                             }
                         });

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

@@ -6,17 +6,13 @@
 package com.todoroo.astrid.dao;
 
 import android.content.ContentValues;
-import android.database.Cursor;
 
 import com.todoroo.andlib.data.DatabaseDao;
-import com.todoroo.andlib.data.Property;
 import com.todoroo.andlib.data.TodorooCursor;
 import com.todoroo.andlib.sql.Criterion;
-import com.todoroo.andlib.sql.Join;
 import com.todoroo.andlib.sql.Query;
 import com.todoroo.andlib.utility.DateUtilities;
 import com.todoroo.astrid.data.Metadata;
-import com.todoroo.astrid.data.Task;
 
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -123,15 +119,5 @@ public class MetadataDao extends DatabaseDao<Metadata> {
 
         return super.persist(item);
     }
-
-    /**
-     * Fetch all metadata that are unattached to the task
-     */
-    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.rawQuery(sql.toString());
-        return new TodorooCursor<>(cursor, properties);
-    }
 }
 

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

@@ -9,11 +9,13 @@ import android.content.ContentValues;
 
 import com.todoroo.andlib.data.TodorooCursor;
 import com.todoroo.andlib.sql.Criterion;
+import com.todoroo.andlib.sql.Join;
 import com.todoroo.andlib.sql.Query;
 import com.todoroo.andlib.utility.DateUtilities;
 import com.todoroo.astrid.dao.MetadataDao;
 import com.todoroo.astrid.dao.MetadataDao.MetadataCriteria;
 import com.todoroo.astrid.data.Metadata;
+import com.todoroo.astrid.data.Task;
 
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -47,20 +49,9 @@ public class MetadataService {
     /**
      * Clean up metadata. Typically called on startup
      */
-    public void cleanup() {
+    public void removeDanglingMetadata() {
-        TodorooCursor<Metadata> cursor = metadataDao.fetchDangling(Metadata.ID);
+        metadataDao.deleteWhere(Metadata.ID.in(Query.select(Metadata.ID).from(Metadata.TABLE).join(Join.left(Task.TABLE,
-        try {
+                Metadata.TASK.eq(Task.ID))).where(Task.TITLE.isNull())));
-            if(cursor.getCount() == 0) {
-                return;
-            }
-
-            for(cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) {
-                long id = cursor.getLong(0);
-                metadataDao.delete(id);
-            }
-        } finally {
-            cursor.close();
-        }
     }
 
     /**