diff --git a/app/src/androidTest/java/com/todoroo/astrid/service/TaskMoverTest.kt b/app/src/androidTest/java/com/todoroo/astrid/service/TaskMoverTest.kt index 325afa5c1..0f01808d7 100644 --- a/app/src/androidTest/java/com/todoroo/astrid/service/TaskMoverTest.kt +++ b/app/src/androidTest/java/com/todoroo/astrid/service/TaskMoverTest.kt @@ -89,7 +89,7 @@ class TaskMoverTest : InjectingTestCase() { createTasks(1) caldavDao.insert(newCaldavTask(with(CaldavTaskMaker.TASK, 1L), with(CALENDAR, "1"))) moveToCaldavList("2", 1) - val deleted = caldavDao.getDeleted("1") + val deleted = caldavDao.getMoved("1") assertEquals(1, deleted.size.toLong()) assertEquals(1, deleted[0].task) assertTrue(deleted[0].deleted > 0) @@ -114,7 +114,7 @@ class TaskMoverTest : InjectingTestCase() { with(CALENDAR, "1"), with(REMOTE_PARENT, "b")))) moveToCaldavList("2", 1) - val deleted = caldavDao.getDeleted("1") + val deleted = caldavDao.getMoved("1") assertEquals(3, deleted.size.toLong()) val task = caldavDao.getTask(3) assertEquals("2", task!!.calendar) @@ -249,7 +249,7 @@ class TaskMoverTest : InjectingTestCase() { createTasks(1) caldavDao.insert(newCaldavTask(with(CaldavTaskMaker.TASK, 1L), with(CALENDAR, "1"))) moveToCaldavList("1", 1) - assertTrue(caldavDao.getDeleted("1").isEmpty()) + assertTrue(caldavDao.getMoved("1").isEmpty()) assertEquals(1, caldavDao.getTasks(1).size.toLong()) } diff --git a/app/src/main/java/org/tasks/caldav/CaldavSynchronizer.kt b/app/src/main/java/org/tasks/caldav/CaldavSynchronizer.kt index 455bd37e6..8f7b19a9b 100644 --- a/app/src/main/java/org/tasks/caldav/CaldavSynchronizer.kt +++ b/app/src/main/java/org/tasks/caldav/CaldavSynchronizer.kt @@ -226,7 +226,7 @@ class CaldavSynchronizer @Inject constructor( private suspend fun pushLocalChanges( caldavCalendar: CaldavCalendar, httpClient: OkHttpClient, httpUrl: HttpUrl) { - for (task in caldavDao.getDeleted(caldavCalendar.uuid!!)) { + for (task in caldavDao.getMoved(caldavCalendar.uuid!!)) { deleteRemoteResource(httpClient, httpUrl, task) } for (task in taskDao.getCaldavTasksToPush(caldavCalendar.uuid!!)) { diff --git a/app/src/main/java/org/tasks/data/CaldavDao.kt b/app/src/main/java/org/tasks/data/CaldavDao.kt index 80d6bd5f8..1c63a77f0 100644 --- a/app/src/main/java/org/tasks/data/CaldavDao.kt +++ b/app/src/main/java/org/tasks/data/CaldavDao.kt @@ -107,8 +107,11 @@ abstract class CaldavDao { @Delete abstract suspend fun delete(caldavTask: CaldavTask) + @Delete + abstract suspend fun delete(caldavTasks: List) + @Query("SELECT * FROM caldav_tasks WHERE cd_deleted > 0 AND cd_calendar = :calendar") - abstract suspend fun getDeleted(calendar: String): List + abstract suspend fun getMoved(calendar: String): List @Query("UPDATE caldav_tasks SET cd_deleted = :now WHERE cd_task IN (:tasks)") abstract suspend fun markDeleted(tasks: List, now: Long = currentTimeMillis()) @@ -138,8 +141,11 @@ SELECT EXISTS(SELECT 1 """) abstract suspend fun isAccountType(id: Long, type: Int): Boolean + suspend fun getTasks(taskIds: List): List = + taskIds.chunkedMap { getTasksInternal(it) } + @Query("SELECT * FROM caldav_tasks WHERE cd_task in (:taskIds) AND cd_deleted = 0") - abstract suspend fun getTasks(taskIds: List): List + internal abstract suspend fun getTasksInternal(taskIds: List): List @Query("SELECT task.*, caldav_task.* FROM tasks AS task " + "INNER JOIN caldav_tasks AS caldav_task ON _id = cd_task " diff --git a/app/src/main/java/org/tasks/data/OpenTaskDao.kt b/app/src/main/java/org/tasks/data/OpenTaskDao.kt index b4086c646..033b6dd70 100644 --- a/app/src/main/java/org/tasks/data/OpenTaskDao.kt +++ b/app/src/main/java/org/tasks/data/OpenTaskDao.kt @@ -1,5 +1,7 @@ package org.tasks.data +import android.content.ContentProviderOperation +import android.content.ContentProviderOperation.* import android.content.ContentValues import android.content.Context import android.database.Cursor @@ -21,6 +23,8 @@ class OpenTaskDao @Inject constructor(@ApplicationContext context: Context) { private val cr = context.contentResolver val authority = context.getString(R.string.opentasks_authority) + private val tasks = Tasks.getContentUri(authority) + private val properties = Properties.getContentUri(authority) suspend fun accounts(): List = getLists().map { it.account!! }.distinct() @@ -53,7 +57,7 @@ class OpenTaskDao @Inject constructor(@ApplicationContext context: Context) { suspend fun getEtags(listId: Long): List> = withContext(Dispatchers.IO) { val items = ArrayList>() cr.query( - Tasks.getContentUri(authority), + tasks, arrayOf(Tasks._SYNC_ID, "version"), "${Tasks.LIST_ID} = $listId", null, @@ -65,37 +69,58 @@ class OpenTaskDao @Inject constructor(@ApplicationContext context: Context) { items } - suspend fun delete(listId: Long, item: String): Int = withContext(Dispatchers.IO) { - cr.delete( - Tasks.getContentUri(authority), - "${Tasks.LIST_ID} = $listId AND ${Tasks._SYNC_ID} = '$item'", - null) - } - - suspend fun getId(uid: String?): Long = - uid?.let { - withContext(Dispatchers.IO) { - cr.query( - Tasks.getContentUri(authority), - arrayOf(Tasks._ID), - "${Tasks._UID} = '$uid'", - null, - null)?.use { - if (it.moveToFirst()) { - it.getLong(Tasks._ID) - } else { - Timber.e("No task with uid=$uid") - null + fun delete(listId: Long, item: String): ContentProviderOperation = + newDelete(tasks) + .withSelection( + "${Tasks.LIST_ID} = $listId AND ${Tasks._SYNC_ID} = '$item'", + null) + .build() + + fun insert(values: ContentValues): ContentProviderOperation = + newInsert(tasks) + .withValues(values) + .build() + + fun update(listId: Long, item: String, values: ContentValues): ContentProviderOperation = + newUpdate(tasks) + .withSelection( + "${Tasks.LIST_ID} = $listId AND ${Tasks._SYNC_ID} = '$item'", + null) + .withValues(values) + .build() + + suspend fun getId(listId: Long, uid: String?): Long? = + uid + ?.takeIf { it.isNotBlank() } + ?.let { + withContext(Dispatchers.IO) { + cr.query( + tasks, + arrayOf(Tasks._ID), + "${Tasks.LIST_ID} = $listId AND ${Tasks._UID} = '$uid'", + null, + null)?.use { + if (it.moveToFirst()) { + it.getLong(Tasks._ID) + } else { + null + } + } } } - } - } ?: 0L + ?: Timber.e("No task with uid=$uid").let { null } + + suspend fun batch(operations: List) = withContext(Dispatchers.IO) { + operations.chunked(OPENTASK_BATCH_LIMIT).forEach { + cr.applyBatch(authority, ArrayList(it)) + } + } - suspend fun getTags(caldavTask: CaldavTask): List = withContext(Dispatchers.IO) { - val id = getId(caldavTask.remoteId) + suspend fun getTags(listId: Long, caldavTask: CaldavTask): List = withContext(Dispatchers.IO) { + val id = getId(listId, caldavTask.remoteId) val tags = ArrayList() cr.query( - Properties.getContentUri(authority), + properties, arrayOf(Properties.DATA1), "${Properties.TASK_ID} = $id AND ${Properties.MIMETYPE} = '${Category.CONTENT_ITEM_TYPE}'", null, @@ -107,25 +132,29 @@ class OpenTaskDao @Inject constructor(@ApplicationContext context: Context) { return@withContext tags } - suspend fun setTags(caldavTask: CaldavTask, tags: List) = withContext(Dispatchers.IO) { - val id = getId(caldavTask.remoteId) - cr.delete( - Properties.getContentUri(authority), - "${Properties.TASK_ID} = $id AND ${Properties.MIMETYPE} = '${Category.CONTENT_ITEM_TYPE}'", - null) - tags.forEach { - cr.insert(Properties.getContentUri(authority), ContentValues().apply { - put(Category.MIMETYPE, Category.CONTENT_ITEM_TYPE) - put(Category.TASK_ID, id) - put(Category.CATEGORY_NAME, it) - }) + fun setTags(id: Long, tags: List): List { + val delete = listOf( + newDelete(properties) + .withSelection( + "${Properties.TASK_ID} = $id AND ${Properties.MIMETYPE} = '${Category.CONTENT_ITEM_TYPE}'", + null) + .build()) + val inserts = tags.map { + newInsert(properties) + .withValues(ContentValues().apply { + put(Category.MIMETYPE, Category.CONTENT_ITEM_TYPE) + put(Category.TASK_ID, id) + put(Category.CATEGORY_NAME, it) + }) + .build() } + return delete + inserts } - suspend fun getRemoteOrder(caldavTask: CaldavTask): Long? = withContext(Dispatchers.IO) { - val id = getId(caldavTask.remoteId) + suspend fun getRemoteOrder(listId: Long, caldavTask: CaldavTask): Long? = withContext(Dispatchers.IO) { + val id = getId(listId, caldavTask.remoteId) ?: return@withContext null cr.query( - Properties.getContentUri(authority), + properties, arrayOf(Properties.DATA0), "${Properties.TASK_ID} = $id AND ${Properties.MIMETYPE} = '${UnknownProperty.CONTENT_ITEM_TYPE}' AND ${Properties.DATA0} LIKE '%$APPLE_SORT_ORDER%'", null, @@ -142,37 +171,53 @@ class OpenTaskDao @Inject constructor(@ApplicationContext context: Context) { return@withContext null } - suspend fun setRemoteOrder(caldavTask: CaldavTask) = withContext(Dispatchers.IO) { - val id = getId(caldavTask.remoteId) - cr.delete( - Properties.getContentUri(authority), - "${Properties.TASK_ID} = $id AND ${Properties.MIMETYPE} = '${UnknownProperty.CONTENT_ITEM_TYPE}' AND ${Properties.DATA0} LIKE '%$APPLE_SORT_ORDER%'", - null) + fun setRemoteOrder(id: Long, caldavTask: CaldavTask): List { + val operations = ArrayList() + operations.add( + newDelete(properties) + .withSelection( + "${Properties.TASK_ID} = $id AND ${Properties.MIMETYPE} = '${UnknownProperty.CONTENT_ITEM_TYPE}' AND ${Properties.DATA0} LIKE '%$APPLE_SORT_ORDER%'", + null) + .build()) caldavTask.order?.let { - cr.insert(Properties.getContentUri(authority), ContentValues().apply { - put(Properties.MIMETYPE, UnknownProperty.CONTENT_ITEM_TYPE) - put(Properties.TASK_ID, id) - put(Properties.DATA0, UnknownProperty.toJsonString(XProperty(APPLE_SORT_ORDER, it.toString()))) - }) + operations.add( + newInsert(properties) + .withValues(ContentValues().apply { + put(Properties.MIMETYPE, UnknownProperty.CONTENT_ITEM_TYPE) + put(Properties.TASK_ID, id) + put(Properties.DATA0, UnknownProperty.toJsonString(XProperty(APPLE_SORT_ORDER, it.toString()))) + }) + .build()) } + return operations } - suspend fun updateParent(caldavTask: CaldavTask) = withContext(Dispatchers.IO) { - caldavTask.remoteParent - ?.takeIf { it.isNotBlank() } - ?.let { - cr.insert(Properties.getContentUri(authority), ContentValues().apply { - put(Relation.MIMETYPE, Relation.CONTENT_ITEM_TYPE) - put(Relation.TASK_ID, getId(caldavTask.remoteId)) - put(Relation.RELATED_TYPE, Relation.RELTYPE_PARENT) - put(Relation.RELATED_ID, getId(caldavTask.remoteParent)) - }) - } + fun updateParent(id: Long, parent: Long?): List { + val operations = ArrayList() + operations.add( + newDelete(properties) + .withSelection( + "${Properties.TASK_ID} = $id AND ${Properties.MIMETYPE} = '${Relation.CONTENT_ITEM_TYPE}' AND ${Relation.RELATED_TYPE} = ${Relation.RELTYPE_PARENT}", + null + ) + .build()) + parent?.let { + operations.add( + newInsert(properties) + .withValues(ContentValues().apply { + put(Relation.MIMETYPE, Relation.CONTENT_ITEM_TYPE) + put(Relation.TASK_ID, id) + put(Relation.RELATED_TYPE, Relation.RELTYPE_PARENT) + put(Relation.RELATED_ID, parent) + }) + .build()) + } + return operations } suspend fun getParent(id: Long): String? = withContext(Dispatchers.IO) { cr.query( - Properties.getContentUri(authority), + properties, arrayOf(Relation.RELATED_UID), "${Relation.TASK_ID} = $id AND ${Properties.MIMETYPE} = '${Relation.CONTENT_ITEM_TYPE}' AND ${Relation.RELATED_TYPE} = ${Relation.RELTYPE_PARENT}", null, @@ -186,6 +231,7 @@ class OpenTaskDao @Inject constructor(@ApplicationContext context: Context) { } companion object { + private const val OPENTASK_BATCH_LIMIT = 500 const val ACCOUNT_TYPE_DAVx5 = "bitfire.at.davdroid" const val ACCOUNT_TYPE_ETESYNC = "com.etesync.syncadapter" diff --git a/app/src/main/java/org/tasks/data/TaskDao.kt b/app/src/main/java/org/tasks/data/TaskDao.kt index 9414bfb6e..114a23eed 100644 --- a/app/src/main/java/org/tasks/data/TaskDao.kt +++ b/app/src/main/java/org/tasks/data/TaskDao.kt @@ -68,6 +68,7 @@ abstract class TaskDao(private val database: Database) { FROM tasks INNER JOIN caldav_tasks ON tasks._id = caldav_tasks.cd_task WHERE caldav_tasks.cd_calendar = :calendar + AND cd_deleted = 0 AND (tasks.modified > caldav_tasks.cd_last_sync OR caldav_tasks.cd_last_sync = 0) ORDER BY created""") abstract suspend fun getCaldavTasksToPush(calendar: String): List diff --git a/app/src/main/java/org/tasks/etesync/EteSynchronizer.kt b/app/src/main/java/org/tasks/etesync/EteSynchronizer.kt index 2a18807a8..d572054cd 100644 --- a/app/src/main/java/org/tasks/etesync/EteSynchronizer.kt +++ b/app/src/main/java/org/tasks/etesync/EteSynchronizer.kt @@ -144,7 +144,7 @@ class EteSynchronizer @Inject constructor( Timber.d("%s up to date", caldavCalendar.name) } val changes: MutableList = ArrayList() - for (task in caldavDao.getDeleted(caldavCalendar.uuid!!)) { + for (task in caldavDao.getMoved(caldavCalendar.uuid!!)) { val vtodo = task.vtodo if (!isNullOrEmpty(vtodo)) { changes.add(SyncEntry(vtodo!!, SyncEntry.Actions.DELETE)) diff --git a/app/src/main/java/org/tasks/opentasks/OpenTasksSynchronizer.kt b/app/src/main/java/org/tasks/opentasks/OpenTasksSynchronizer.kt index 2ace6a4b3..9b7b31905 100644 --- a/app/src/main/java/org/tasks/opentasks/OpenTasksSynchronizer.kt +++ b/app/src/main/java/org/tasks/opentasks/OpenTasksSynchronizer.kt @@ -1,5 +1,6 @@ package org.tasks.opentasks +import android.content.ContentProviderOperation import android.content.ContentValues import android.content.Context import android.database.Cursor @@ -13,8 +14,6 @@ import com.todoroo.astrid.helper.UUIDHelper import com.todoroo.astrid.service.TaskCreator import com.todoroo.astrid.service.TaskDeleter import dagger.hilt.android.qualifiers.ApplicationContext -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.withContext import net.fortuna.ical4j.model.Date import net.fortuna.ical4j.model.property.Geo import net.fortuna.ical4j.model.property.RRule @@ -104,22 +103,38 @@ class OpenTasksSynchronizer @Inject constructor( private suspend fun sync(calendar: CaldavCalendar, ctag: String?, listId: Long) { Timber.d("SYNC $calendar") - caldavDao.getDeleted(calendar.uuid!!).forEach { - openTaskDao.delete(listId, it.`object`!!) - caldavDao.delete(it) - } + val moved = caldavDao.getMoved(calendar.uuid!!) + val (deleted, updated) = + taskDao.getCaldavTasksToPush(calendar.uuid!!).partition { it.isDeleted } + + (moved + deleted.map(Task::id).let { caldavDao.getTasks(it) }) + .mapNotNull { it.`object` } + .map { openTaskDao.delete(listId, it) } + .let { openTaskDao.batch(it) } + caldavDao.delete(moved) + taskDeleter.delete(deleted.map { it.id }) - taskDao - .getCaldavTasksToPush(calendar.uuid!!) - .mapNotNull { push(it, listId) } - .forEach { - val tags = tagDataDao.getTagDataForTask(it.task).mapNotNull(TagData::name) - openTaskDao.setTags(it, tags) - openTaskDao.setRemoteOrder(it) - openTaskDao.updateParent(it) - it.lastSync = currentTimeMillis() + openTaskDao.batch(updated.mapNotNull { toOperation(it, listId) }) + + val caldavTasks = updated.let { caldavDao.getTasks(it.map(Task::id)) } + openTaskDao.batch(caldavTasks.flatMap { + val id = openTaskDao + .getId(listId, it.remoteId) + ?: return@flatMap emptyList() + val tags = tagDataDao.getTagDataForTask(it.task).mapNotNull(TagData::name) + val parent = openTaskDao.getId(listId, it.remoteParent) + openTaskDao.setTags(id, tags) + .plus(openTaskDao.setRemoteOrder(id, it)) + .plus(openTaskDao.updateParent(id, parent)) + }) + + caldavTasks + .takeIf { it.isNotEmpty() } + ?.let { + val lastSync = currentTimeMillis() + caldavTasks.forEach { t -> t.lastSync = lastSync } caldavDao.update(it) - Timber.d("SENT $it") + Timber.d("SENT ${it.joinToString("\n")}") } ctag?.let { @@ -132,7 +147,9 @@ class OpenTasksSynchronizer @Inject constructor( val etags = openTaskDao.getEtags(listId) etags.forEach { (syncId, etag) -> val caldavTask = caldavDao.getTask(calendar.uuid!!, syncId) - applyChanges(calendar, listId, syncId, etag, caldavTask) + if (caldavTask?.etag == null || caldavTask.etag != etag) { + applyChanges(calendar, listId, syncId, etag, caldavTask) + } } removeDeleted(calendar.uuid!!, etags.map { it.first }) @@ -163,13 +180,8 @@ class OpenTasksSynchronizer @Inject constructor( } } - private suspend fun push(task: Task, listId: Long): CaldavTask? = withContext(Dispatchers.IO) { - val caldavTask = caldavDao.getTask(task.id) ?: return@withContext null - if (task.isDeleted) { - openTaskDao.delete(listId, caldavTask.`object`!!) - taskDeleter.delete(task) - return@withContext null - } + private suspend fun toOperation(task: Task, listId: Long): ContentProviderOperation? { + val caldavTask = caldavDao.getTask(task.id) ?: return null val values = ContentValues() values.put(Tasks._SYNC_ID, caldavTask.`object`) values.put(Tasks.LIST_ID, listId) @@ -213,26 +225,17 @@ class OpenTasksSynchronizer @Inject constructor( values.put(Tasks.PRIORITY, toRemote(it.getInt(Tasks.PRIORITY), task.priority)) true } ?: false - try { + return try { if (existing) { - val updated = cr.update( - Tasks.getContentUri(openTaskDao.authority), - values, - "${Tasks.LIST_ID} = $listId AND ${Tasks._SYNC_ID} = '${caldavTask.`object`}'", - null) - if (updated <= 0) { - throw Exception("update failed") - } + openTaskDao.update(listId, caldavTask.`object`!!, values) } else { values.put(Tasks._UID, caldavTask.remoteId) values.put(Tasks.PRIORITY, toRemote(task.priority, task.priority)) - cr.insert(Tasks.getContentUri(openTaskDao.authority), values) - ?: throw Exception("insert returned null") + openTaskDao.insert(values) } - caldavTask } catch (e: Exception) { firebase.reportException(e) - return@withContext null + null } } @@ -262,32 +265,30 @@ class OpenTasksSynchronizer @Inject constructor( task = taskDao.fetch(existing.task)!! caldavTask = existing } - if (caldavTask.etag == null || caldavTask.etag != etag) { - task.title = it.getString(Tasks.TITLE) - task.priority = CaldavConverter.fromRemote(it.getInt(Tasks.PRIORITY)) - task.completionDate = it.getLong(Tasks.COMPLETED) - task.notes = it.getString(Tasks.DESCRIPTION) - task.modificationDate = currentTimeMillis() - task.creationDate = it.getLong(Tasks.CREATED).toLocal() - task.setDueDateAdjustingHideUntil(it.getLong(Tasks.DUE).let { due -> - when { - due == 0L -> 0 - it.getBoolean(Tasks.IS_ALLDAY) -> - Task.createDueDate(URGENCY_SPECIFIC_DAY, due - DateTime(due).offset) - else -> Task.createDueDate(URGENCY_SPECIFIC_DAY_TIME, due) - } - }) - iCalendar.setPlace(task.id, it.getString(Tasks.GEO).toGeo()) - task.setRecurrence(it.getString(Tasks.RRULE).toRRule()) - task.suppressSync() - task.suppressRefresh() - taskDao.save(task) - caldavTask.lastSync = DateUtilities.now() + 1000L - caldavTask.etag = etag - } - val tags = openTaskDao.getTags(caldavTask) + task.title = it.getString(Tasks.TITLE) + task.priority = CaldavConverter.fromRemote(it.getInt(Tasks.PRIORITY)) + task.completionDate = it.getLong(Tasks.COMPLETED) + task.notes = it.getString(Tasks.DESCRIPTION) + task.modificationDate = currentTimeMillis() + task.creationDate = it.getLong(Tasks.CREATED).toLocal() + task.setDueDateAdjustingHideUntil(it.getLong(Tasks.DUE).let { due -> + when { + due == 0L -> 0 + it.getBoolean(Tasks.IS_ALLDAY) -> + Task.createDueDate(URGENCY_SPECIFIC_DAY, due - DateTime(due).offset) + else -> Task.createDueDate(URGENCY_SPECIFIC_DAY_TIME, due) + } + }) + iCalendar.setPlace(task.id, it.getString(Tasks.GEO).toGeo()) + task.setRecurrence(it.getString(Tasks.RRULE).toRRule()) + task.suppressSync() + task.suppressRefresh() + taskDao.save(task) + caldavTask.lastSync = DateUtilities.now() + 1000L + caldavTask.etag = etag + val tags = openTaskDao.getTags(listId, caldavTask) tagDao.applyTags(task, tagDataDao, iCalendar.getTags(tags)) - caldavTask.order = openTaskDao.getRemoteOrder(caldavTask) + caldavTask.order = openTaskDao.getRemoteOrder(listId, caldavTask) caldavTask.remoteParent = openTaskDao.getParent(it.getLong(Tasks._ID)) if (caldavTask.id == Task.NO_ID) { caldavTask.id = caldavDao.insert(caldavTask)