From 1178b8f3e6381ef7cfade13a9344a7b50ad79ff5 Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Fri, 31 Jul 2020 14:01:57 -0500 Subject: [PATCH] Prevent setting parent to self --- .../astrid/adapter/RecursiveLoopTest.kt | 7 ++-- .../java/org/tasks/data/TaskDaoTests.kt | 39 +++++++++++------ .../java/org/tasks/jobs/BackupServiceTests.kt | 42 ++++++++++--------- app/src/main/java/org/tasks/data/TaskDao.kt | 5 ++- 4 files changed, 54 insertions(+), 39 deletions(-) diff --git a/app/src/androidTest/java/com/todoroo/astrid/adapter/RecursiveLoopTest.kt b/app/src/androidTest/java/com/todoroo/astrid/adapter/RecursiveLoopTest.kt index b6ec86230..8630e43a5 100644 --- a/app/src/androidTest/java/com/todoroo/astrid/adapter/RecursiveLoopTest.kt +++ b/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 diff --git a/app/src/androidTest/java/org/tasks/data/TaskDaoTests.kt b/app/src/androidTest/java/org/tasks/data/TaskDaoTests.kt index 612f824b7..7ff251366 100644 --- a/app/src/androidTest/java/org/tasks/data/TaskDaoTests.kt +++ b/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) } } \ No newline at end of file diff --git a/app/src/androidTest/java/org/tasks/jobs/BackupServiceTests.kt b/app/src/androidTest/java/org/tasks/jobs/BackupServiceTests.kt index f1debc608..e95f8a0b6 100644 --- a/app/src/androidTest/java/org/tasks/jobs/BackupServiceTests.kt +++ b/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 diff --git a/app/src/main/java/org/tasks/data/TaskDao.kt b/app/src/main/java/org/tasks/data/TaskDao.kt index a13e77991..1145ceae5 100644 --- a/app/src/main/java/org/tasks/data/TaskDao.kt +++ b/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) = 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) @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 {