Use task modification date for lastSync value

Fix race condition that caused Tasks to not push changes after
rescheduling a remotely completed recurring task
pull/1264/head
Alex Baker 5 years ago
parent 892ce990a4
commit 5eb9370294

@ -34,7 +34,6 @@ import org.tasks.data.CaldavAccount.Companion.ERROR_UNAUTHORIZED
import org.tasks.data.CaldavCalendar import org.tasks.data.CaldavCalendar
import org.tasks.data.CaldavDao import org.tasks.data.CaldavDao
import org.tasks.data.CaldavTask import org.tasks.data.CaldavTask
import org.tasks.time.DateTimeUtils
import timber.log.Timber import timber.log.Timber
import java.io.IOException import java.io.IOException
import java.net.ConnectException import java.net.ConnectException
@ -295,7 +294,7 @@ class CaldavSynchronizer @Inject constructor(
Timber.e(e) Timber.e(e)
return return
} }
caldavTask.lastSync = DateTimeUtils.currentTimeMillis() caldavTask.lastSync = task.modificationDate
caldavDao.update(caldavTask) caldavDao.update(caldavTask)
Timber.d("SENT %s", caldavTask) Timber.d("SENT %s", caldavTask)
} }

@ -2,7 +2,6 @@ package org.tasks.caldav
import at.bitfire.ical4android.Task import at.bitfire.ical4android.Task
import at.bitfire.ical4android.Task.Companion.tasksFromReader import at.bitfire.ical4android.Task.Companion.tasksFromReader
import com.todoroo.andlib.utility.DateUtilities
import com.todoroo.astrid.dao.TaskDao import com.todoroo.astrid.dao.TaskDao
import com.todoroo.astrid.helper.UUIDHelper import com.todoroo.astrid.helper.UUIDHelper
import com.todoroo.astrid.service.TaskCreator import com.todoroo.astrid.service.TaskCreator
@ -205,7 +204,7 @@ class iCalendar @Inject constructor(
taskDao.save(task) taskDao.save(task)
caldavTask.vtodo = vtodo caldavTask.vtodo = vtodo
caldavTask.etag = eTag caldavTask.etag = eTag
caldavTask.lastSync = DateUtilities.now() + 1000L caldavTask.lastSync = task.modificationDate
caldavTask.remoteParent = remote.getParent() caldavTask.remoteParent = remote.getParent()
if (caldavTask.id == com.todoroo.astrid.data.Task.NO_ID) { if (caldavTask.id == com.todoroo.astrid.data.Task.NO_ID) {
caldavTask.id = caldavDao.insert(caldavTask) caldavTask.id = caldavDao.insert(caldavTask)

@ -96,6 +96,9 @@ class GoogleTask {
return "GoogleTask(id=$id, task=$task, remoteId='$remoteId', listId='$listId', parent=$parent, remoteParent=$remoteParent, isMoved=$isMoved, order=$order, remoteOrder=$remoteOrder, lastSync=$lastSync, deleted=$deleted)" return "GoogleTask(id=$id, task=$task, remoteId='$remoteId', listId='$listId', parent=$parent, remoteParent=$remoteParent, isMoved=$isMoved, order=$order, remoteOrder=$remoteOrder, lastSync=$lastSync, deleted=$deleted)"
} }
val isNew: Boolean
get() = id == 0L
companion object { companion object {
const val KEY = "gtasks" const val KEY = "gtasks"
@JvmField val TABLE = Table("google_tasks") @JvmField val TABLE = Table("google_tasks")

@ -68,7 +68,7 @@ class EtebaseClient(
task.`object` = uid task.`object` = uid
caldavDao.update(task) caldavDao.update(task)
} }
item.meta = updateMtime(item.meta) item.meta = updateMtime(item.meta, task.lastSync)
item.content = content item.content = content
return item return item
} }
@ -83,8 +83,8 @@ class EtebaseClient(
} }
} }
private fun updateMtime(meta: ItemMetadata): ItemMetadata { private fun updateMtime(meta: ItemMetadata, mtime: Long = currentTimeMillis()): ItemMetadata {
meta.mtime = currentTimeMillis() meta.mtime = mtime
return meta return meta
} }

@ -6,7 +6,6 @@ import at.bitfire.ical4android.ICalendar.Companion.prodId
import com.etebase.client.Collection import com.etebase.client.Collection
import com.etebase.client.Item import com.etebase.client.Item
import com.etebase.client.exceptions.* import com.etebase.client.exceptions.*
import com.todoroo.andlib.utility.DateUtilities.now
import com.todoroo.astrid.helper.UUIDHelper import com.todoroo.astrid.helper.UUIDHelper
import com.todoroo.astrid.service.TaskDeleter import com.todoroo.astrid.service.TaskDeleter
import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.qualifiers.ApplicationContext
@ -145,10 +144,10 @@ class EtebaseSynchronizer @Inject constructor(
?.let { changes.add(it) } ?.let { changes.add(it) }
?: caldavDao.delete(caldavTask) ?: caldavDao.delete(caldavTask)
} }
val syncTime = now()
for (change in caldavDao.getCaldavTasksToPush(caldavCalendar.uuid!!)) { for (change in caldavDao.getCaldavTasksToPush(caldavCalendar.uuid!!)) {
val task = change.task val task = change.task
val caldavTask = change.caldavTask val caldavTask = change.caldavTask
caldavTask.lastSync = task.modificationDate
if (task.isDeleted) { if (task.isDeleted) {
client.deleteItem(collection, caldavTask) client.deleteItem(collection, caldavTask)
?.let { changes.add(it) } ?.let { changes.add(it) }
@ -161,7 +160,7 @@ class EtebaseSynchronizer @Inject constructor(
} }
if (changes.isNotEmpty()) { if (changes.isNotEmpty()) {
client.uploadChanges(collection, changes) client.uploadChanges(collection, changes)
applyEntries(caldavCalendar, changes, syncTime = syncTime, isLocalChange = true) applyEntries(caldavCalendar, changes, isLocalChange = true)
client.updateCache(collection, changes) client.updateCache(collection, changes)
} }
} }
@ -170,7 +169,6 @@ class EtebaseSynchronizer @Inject constructor(
caldavCalendar: CaldavCalendar, caldavCalendar: CaldavCalendar,
items: List<Item>, items: List<Item>,
stoken: String? = null, stoken: String? = null,
syncTime: Long = currentTimeMillis(),
isLocalChange: Boolean = false) { isLocalChange: Boolean = false) {
for (item in items) { for (item in items) {
val vtodo = item.contentString val vtodo = item.contentString
@ -188,7 +186,7 @@ class EtebaseSynchronizer @Inject constructor(
} else if (isLocalChange) { } else if (isLocalChange) {
caldavTask?.let { caldavTask?.let {
it.vtodo = vtodo it.vtodo = vtodo
it.lastSync = syncTime it.lastSync = item.meta.mtime ?: currentTimeMillis()
caldavDao.update(it) caldavDao.update(it)
} }
} else { } else {

@ -269,16 +269,8 @@ class GoogleTaskSynchronizer @Inject constructor(
return return
} }
} }
task.modificationDate = DateUtilities.now()
gtasksMetadata.isMoved = false gtasksMetadata.isMoved = false
gtasksMetadata.lastSync = DateUtilities.now() + 1000L write(task, gtasksMetadata)
if (gtasksMetadata.id == com.todoroo.astrid.data.Task.NO_ID) {
googleTaskDao.insert(gtasksMetadata)
} else {
googleTaskDao.update(gtasksMetadata)
}
task.suppressSync()
taskDao.save(task)
} }
@Synchronized @Synchronized
@ -347,8 +339,9 @@ class GoogleTaskSynchronizer @Inject constructor(
mergeDates(createDueDate(com.todoroo.astrid.data.Task.URGENCY_SPECIFIC_DAY, dueDate), task) mergeDates(createDueDate(com.todoroo.astrid.data.Task.URGENCY_SPECIFIC_DAY, dueDate), task)
task.notes = getTruncatedValue(task.notes, gtask.notes, MAX_DESCRIPTION_LENGTH) task.notes = getTruncatedValue(task.notes, gtask.notes, MAX_DESCRIPTION_LENGTH)
googleTask.listId = listId googleTask.listId = listId
googleTask.lastSync = DateUtilities.now() + 1000L if (task.title?.isNotBlank() == true || task.notes?.isNotBlank() == true) {
write(task, googleTask) write(task, googleTask)
}
} }
list.lastSync = lastSyncDate list.lastSync = lastSyncDate
googleTaskListDao.insertOrReplace(list) googleTaskListDao.insertOrReplace(list)
@ -360,20 +353,19 @@ class GoogleTaskSynchronizer @Inject constructor(
googleTask.parent = googleTask.remoteParent?.let { googleTaskDao.getTask(it) } ?: 0L googleTask.parent = googleTask.remoteParent?.let { googleTaskDao.getTask(it) } ?: 0L
} }
private suspend fun write(task: com.todoroo.astrid.data.Task?, googleTask: GoogleTask) { private suspend fun write(task: com.todoroo.astrid.data.Task, googleTask: GoogleTask) {
if (!(isNullOrEmpty(task!!.title) && isNullOrEmpty(task.notes))) { task.suppressSync()
task.suppressSync() task.suppressRefresh()
task.suppressRefresh() if (task.isNew) {
if (task.isNew) { taskDao.createNew(task)
taskDao.createNew(task) }
} taskDao.save(task)
taskDao.save(task) googleTask.lastSync = task.modificationDate
googleTask.task = task.id googleTask.task = task.id
if (googleTask.id == 0L) { if (googleTask.isNew) {
googleTaskDao.insert(googleTask) googleTaskDao.insert(googleTask)
} else { } else {
googleTaskDao.update(googleTask) googleTaskDao.update(googleTask)
}
} }
} }

@ -4,7 +4,6 @@ import android.content.ContentProviderOperation
import android.content.ContentValues import android.content.ContentValues
import android.content.Context import android.content.Context
import android.database.Cursor import android.database.Cursor
import com.todoroo.andlib.utility.DateUtilities
import com.todoroo.astrid.dao.TaskDao import com.todoroo.astrid.dao.TaskDao
import com.todoroo.astrid.data.Task import com.todoroo.astrid.data.Task
import com.todoroo.astrid.data.Task.Companion.URGENCY_SPECIFIC_DAY import com.todoroo.astrid.data.Task.Companion.URGENCY_SPECIFIC_DAY
@ -138,8 +137,9 @@ class OpenTasksSynchronizer @Inject constructor(
val isEteSync = account.uuid?.isEteSync() == true val isEteSync = account.uuid?.isEteSync() == true
val moved = caldavDao.getMoved(calendar.uuid!!) val moved = caldavDao.getMoved(calendar.uuid!!)
val (deleted, updated) = val (deleted, updated) = taskDao
taskDao.getCaldavTasksToPush(calendar.uuid!!).partition { it.isDeleted } .getCaldavTasksToPush(calendar.uuid!!)
.partition { it.isDeleted }
(moved + deleted.map(Task::id).let { caldavDao.getTasks(it) }) (moved + deleted.map(Task::id).let { caldavDao.getTasks(it) })
.mapNotNull { it.remoteId } .mapNotNull { it.remoteId }
@ -148,12 +148,11 @@ class OpenTasksSynchronizer @Inject constructor(
caldavDao.delete(moved) caldavDao.delete(moved)
taskDeleter.delete(deleted.map { it.id }) taskDeleter.delete(deleted.map { it.id })
openTaskDao.batch(updated.mapNotNull { toOperation(it, listId, isEteSync) }) val operations = updated.mapNotNull { toOperation(it, listId, isEteSync) }
val caldavTasks = operations.map { it.first }
val caldavTasks = updated.let { caldavDao.getTasks(it.map(Task::id)) } openTaskDao.batch(operations.map { it.second })
openTaskDao.batch(caldavTasks.flatMap { openTaskDao.batch(caldavTasks.flatMap {
val id = openTaskDao val id = openTaskDao.getId(listId, it.remoteId)
.getId(listId, it.remoteId)
?: return@flatMap emptyList<ContentProviderOperation>() ?: return@flatMap emptyList<ContentProviderOperation>()
val tags = tagDataDao.getTagDataForTask(it.task).mapNotNull(TagData::name) val tags = tagDataDao.getTagDataForTask(it.task).mapNotNull(TagData::name)
val parent = openTaskDao.getId(listId, it.remoteParent) val parent = openTaskDao.getId(listId, it.remoteParent)
@ -165,9 +164,7 @@ class OpenTasksSynchronizer @Inject constructor(
caldavTasks caldavTasks
.takeIf { it.isNotEmpty() } .takeIf { it.isNotEmpty() }
?.let { ?.let {
val lastSync = currentTimeMillis() caldavDao.update(it) // apply task modification date
caldavTasks.forEach { t -> t.lastSync = lastSync }
caldavDao.update(it)
Timber.d("SENT ${it.joinToString("\n")}") Timber.d("SENT ${it.joinToString("\n")}")
} }
@ -219,8 +216,9 @@ class OpenTasksSynchronizer @Inject constructor(
task: Task, task: Task,
listId: Long, listId: Long,
isEteSync: Boolean isEteSync: Boolean
): ContentProviderOperation? { ): Pair<CaldavTask, ContentProviderOperation>? {
val caldavTask = caldavDao.getTask(task.id) ?: return null val caldavTask = caldavDao.getTask(task.id) ?: return null
caldavTask.lastSync = task.modificationDate
val values = ContentValues() val values = ContentValues()
values.put(Tasks.LIST_ID, listId) values.put(Tasks.LIST_ID, listId)
values.put(Tasks.TITLE, task.title) values.put(Tasks.TITLE, task.title)
@ -263,7 +261,7 @@ class OpenTasksSynchronizer @Inject constructor(
values.put(Tasks.PRIORITY, toRemote(it.getInt(Tasks.PRIORITY), task.priority)) values.put(Tasks.PRIORITY, toRemote(it.getInt(Tasks.PRIORITY), task.priority))
true true
} ?: false } ?: false
return try { val operation = try {
if (existing) { if (existing) {
openTaskDao.update(listId, caldavTask.remoteId!!, values) openTaskDao.update(listId, caldavTask.remoteId!!, values)
} else { } else {
@ -278,6 +276,7 @@ class OpenTasksSynchronizer @Inject constructor(
firebase.reportException(e) firebase.reportException(e)
null null
} }
return operation?.let { Pair(caldavTask, it) }
} }
private suspend fun applyChanges( private suspend fun applyChanges(
@ -321,7 +320,7 @@ class OpenTasksSynchronizer @Inject constructor(
task.suppressSync() task.suppressSync()
task.suppressRefresh() task.suppressRefresh()
taskDao.save(task) taskDao.save(task)
caldavTask.lastSync = DateUtilities.now() + 1000L caldavTask.lastSync = task.modificationDate
caldavTask.etag = etag caldavTask.etag = etag
val tags = openTaskDao.getTags(listId, caldavTask) val tags = openTaskDao.getTags(listId, caldavTask)
tagDao.applyTags(task, tagDataDao, iCalendar.getTags(tags)) tagDao.applyTags(task, tagDataDao, iCalendar.getTags(tags))

Loading…
Cancel
Save