Prevent setting parent to self

app/src/androidTest/java/com/todoroo/astrid/adapter/RecursiveLoopTest.kt

@@ -37,13 +37,12 @@ class RecursiveLoopTest : InjectingTestCase() {
     @Test
     @Ignore("infinite loop")
     fun handleSelfLoop() = runBlocking {
-        val id = addTask(with(DUE_DATE, newDateTime()))
-
-        taskDao.setParent(id, listOf(id))
+        addTask(with(DUE_DATE, newDateTime()), with(PARENT, 1L))
 
         val tasks = getTasks()
+
         assertEquals(1, tasks.size)
-        assertEquals(id, tasks[0].id)
+        assertEquals(1L, tasks[0].id)
     }
 
     @Test

app/src/androidTest/java/org/tasks/data/TaskDaoTests.kt

@@ -12,11 +12,11 @@ import com.todoroo.astrid.service.TaskDeleter
 import dagger.hilt.android.testing.HiltAndroidTest
 import dagger.hilt.android.testing.UninstallModules
 import kotlinx.coroutines.runBlocking
-import org.junit.Assert.*
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
 import org.junit.Test
 import org.tasks.injection.InjectingTestCase
 import org.tasks.injection.ProductionModule
-import org.tasks.makers.TaskMaker.ID
 import org.tasks.makers.TaskMaker.PARENT
 import org.tasks.makers.TaskMaker.newTask
 import javax.inject.Inject
@@ -99,24 +99,37 @@ class TaskDaoTests : InjectingTestCase() {
 
     @Test
     fun findChildrenInList() = runBlocking {
-        taskDao.createNew(newTask(with(ID, 1L)))
-        taskDao.createNew(newTask(with(ID, 2L), with(PARENT, 1L)))
-        assertEquals(listOf(2L), taskDao.getChildren(listOf(1L, 2L)))
+        val parent = taskDao.createNew(newTask())
+        val child = taskDao.createNew(newTask(with(PARENT, parent)))
+        assertEquals(listOf(child), taskDao.getChildren(listOf(parent, child)))
     }
 
     @Test
     fun findRecursiveChildrenInList() = runBlocking {
-        taskDao.createNew(newTask(with(ID, 1L)))
-        taskDao.createNew(newTask(with(ID, 2L), with(PARENT, 1L)))
-        taskDao.createNew(newTask(with(ID, 3L), with(PARENT, 2L)))
-        assertEquals(listOf(2L, 3L, 3L), taskDao.getChildren(listOf(1L, 2L, 3L)))
+        val parent = taskDao.createNew(newTask())
+        val child = taskDao.createNew(newTask(with(PARENT, parent)))
+        val grandchild = taskDao.createNew(newTask(with(PARENT, child)))
+        assertEquals(
+                listOf(child, grandchild, grandchild),
+                taskDao.getChildren(listOf(parent, child, grandchild)))
     }
 
     @Test
     fun findRecursiveChildrenInListAfterSkippingParent() = runBlocking {
-        taskDao.createNew(newTask(with(ID, 1L)))
-        taskDao.createNew(newTask(with(ID, 2L), with(PARENT, 1L)))
-        taskDao.createNew(newTask(with(ID, 3L), with(PARENT, 2L)))
-        assertEquals(listOf(2L, 3L), taskDao.getChildren(listOf(1L, 3L)))
+        val parent = taskDao.createNew(newTask())
+        val child = taskDao.createNew(newTask(with(PARENT, parent)))
+        val grandchild = taskDao.createNew(newTask(with(PARENT, child)))
+        assertEquals(listOf(child, grandchild), taskDao.getChildren(listOf(parent, grandchild)))
+    }
+
+    @Test
+    fun dontSetParentToSelf() = runBlocking {
+        val parent = taskDao.createNew(newTask())
+        val child = taskDao.createNew(newTask())
+
+        taskDao.setParent(parent, listOf(parent, child))
+
+        assertEquals(0, taskDao.fetch(parent)!!.parent)
+        assertEquals(parent, taskDao.fetch(child)!!.parent)
     }
 }

app/src/androidTest/java/org/tasks/jobs/BackupServiceTests.kt

@@ -36,27 +36,29 @@ class BackupServiceTests : InjectingTestCase() {
     private lateinit var temporaryDirectory: File
 
     @Before
-    override fun setUp() = runBlocking {
-        super.setUp()
-        temporaryDirectory = try {
-            File.createTempFile("backup", System.nanoTime().toString())
-        } catch (e: IOException) {
-            throw RuntimeException(e)
-        }
-        if (!temporaryDirectory.delete()) {
-            throw RuntimeException(
-                    "Could not delete temp file: " + temporaryDirectory.absolutePath)
-        }
-        if (!temporaryDirectory.mkdir()) {
-            throw RuntimeException(
-                    "Could not create temp directory: " + temporaryDirectory.absolutePath)
-        }
-        preferences.setUri(R.string.p_backup_dir, Uri.fromFile(temporaryDirectory))
+    override fun setUp() {
+        runBlocking {
+            super.setUp()
+            temporaryDirectory = try {
+                File.createTempFile("backup", System.nanoTime().toString())
+            } catch (e: IOException) {
+                throw RuntimeException(e)
+            }
+            if (!temporaryDirectory.delete()) {
+                throw RuntimeException(
+                        "Could not delete temp file: " + temporaryDirectory.absolutePath)
+            }
+            if (!temporaryDirectory.mkdir()) {
+                throw RuntimeException(
+                        "Could not create temp directory: " + temporaryDirectory.absolutePath)
+            }
+            preferences.setUri(R.string.p_backup_dir, Uri.fromFile(temporaryDirectory))
 
-        // make a temporary task
-        val task = Task()
-        task.title = "helicopter"
-        taskDao.createNew(task)
+            // make a temporary task
+            val task = Task()
+            task.title = "helicopter"
+            taskDao.createNew(task)
+        }
     }
 
     @After

app/src/main/java/org/tasks/data/TaskDao.kt

@@ -144,7 +144,7 @@ SELECT EXISTS(SELECT 1 FROM tasks WHERE parent > 0 AND deleted = 0) AS hasSubtas
     suspend fun setParent(parent: Long, tasks: List<Long>) =
             tasks.eachChunk { setParentInternal(parent, it) }
 
-    @Query("UPDATE tasks SET parent = :parent WHERE _id IN (:children)")
+    @Query("UPDATE tasks SET parent = :parent WHERE _id IN (:children) AND _id != :parent")
     internal abstract suspend fun setParentInternal(parent: Long, children: List<Long>)
 
     @Query("UPDATE tasks SET lastNotified = :timestamp WHERE _id = :id AND lastNotified != :timestamp")
@@ -203,7 +203,7 @@ SELECT EXISTS(SELECT 1 FROM tasks WHERE parent > 0 AND deleted = 0) AS hasSubtas
     @Update
     internal abstract suspend fun updateInternal(task: Task): Int
 
-    suspend fun createNew(task: Task) {
+    suspend fun createNew(task: Task): Long {
         task.id = NO_ID
         if (task.creationDate == 0L) {
             task.creationDate = DateUtilities.now()
@@ -216,6 +216,7 @@ SELECT EXISTS(SELECT 1 FROM tasks WHERE parent > 0 AND deleted = 0) AS hasSubtas
         }
         val insert = insert(task)
         task.id = insert
+        return task.id
     }
 
     suspend fun count(filter: Filter): Int {