From 1549852dcc7f905057b569836020feed0265cc14 Mon Sep 17 00:00:00 2001 From: Olivier Patry Date: Mon, 28 Oct 2024 09:16:56 +0100 Subject: [PATCH 1/5] Add factories for TaskList & Task to avoid setting output only parameters exposed by data class --- .../net/opatry/google/tasks/model/Task.kt | 22 +++++++++++++++++++ .../net/opatry/google/tasks/model/TaskList.kt | 11 ++++++++++ 2 files changed, 33 insertions(+) diff --git a/google/tasks/src/commonMain/kotlin/net/opatry/google/tasks/model/Task.kt b/google/tasks/src/commonMain/kotlin/net/opatry/google/tasks/model/Task.kt index 0dfc8812..c4cb2d0d 100644 --- a/google/tasks/src/commonMain/kotlin/net/opatry/google/tasks/model/Task.kt +++ b/google/tasks/src/commonMain/kotlin/net/opatry/google/tasks/model/Task.kt @@ -116,3 +116,25 @@ data class Task( val link: String ) } + +/** + * Factory function to create a new [TaskList] exposing only relevant parameters. + * + * @property title Title of the task. Maximum length allowed: 1024 characters. + * @property notes Notes describing the task. Tasks assigned from Google Docs cannot have notes. Optional. Maximum length allowed: 8192 characters. + * @property status Status of the task. This is either [Status.NeedsAction] or [Status.Completed]. + * @property dueDate Due date of the task (as a RFC 3339 timestamp). Optional. The due date only records date information; the time portion of the timestamp is discarded when setting the due date. It isn't possible to read or write the time that a task is due via the API. + * @property completedDate Completion date of the task (as a RFC 3339 timestamp). This field is omitted if the task has not been completed. + */ +fun Task( + title: String, + notes: String? = null, + status: Status = Status.NeedsAction, + dueDate: Instant? = null, + completedDate: Instant? = null +): Task { + require(title.length <= 1024) { "Title length must be at most 1024 characters" } + require(notes == null || notes.length <= 8192) { "Notes length must be at most 8192 characters" } + // need to artificially define an extra parameter to call data class ctor instead of recursive call + return Task(id = "", title = title, notes = notes, status = status, dueDate = dueDate, completedDate = completedDate) +} \ No newline at end of file diff --git a/google/tasks/src/commonMain/kotlin/net/opatry/google/tasks/model/TaskList.kt b/google/tasks/src/commonMain/kotlin/net/opatry/google/tasks/model/TaskList.kt index b71b8e3d..6e39bbd0 100644 --- a/google/tasks/src/commonMain/kotlin/net/opatry/google/tasks/model/TaskList.kt +++ b/google/tasks/src/commonMain/kotlin/net/opatry/google/tasks/model/TaskList.kt @@ -53,3 +53,14 @@ data class TaskList( @SerialName("selfLink") val selfLink: String = "", ) + +/** + * Factory function to create a new [TaskList] exposing only relevant parameters. + * + * @param title Title of the task list. Maximum length allowed: 1024 characters. + */ +fun TaskList(title: String): TaskList { + require(title.length <= 1024) { "Title length must be at most 1024 characters" } + // need to artificially define an extra parameter to call data class ctor instead of recursive call + return TaskList(id = "", title = title) +} \ No newline at end of file From cb453c38ab26e188434c10663ec24bf2adb6898e Mon Sep 17 00:00:00 2001 From: Olivier Patry Date: Mon, 28 Oct 2024 09:58:27 +0100 Subject: [PATCH 2/5] Split insert & upsert for UserDao --- .../src/commonMain/kotlin/net/opatry/tasks/data/UserDao.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/UserDao.kt b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/UserDao.kt index fd196f41..4975fc4a 100644 --- a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/UserDao.kt +++ b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/UserDao.kt @@ -32,9 +32,12 @@ import net.opatry.tasks.data.entity.UserEntity @Dao interface UserDao { - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Insert(onConflict = OnConflictStrategy.ABORT) suspend fun insert(user: UserEntity): Long + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun upsert(user: UserEntity): Long + @Query("SELECT * FROM user WHERE remote_id = :remoteId") suspend fun getByRemoteId(remoteId: String): UserEntity? @@ -61,6 +64,6 @@ interface UserDao { @Transaction suspend fun setSignedInUser(userEntity: UserEntity) { clearAllSignedInStatus() - insert(userEntity.copy(isSignedIn = true)) + upsert(userEntity.copy(isSignedIn = true)) } } From ec3616ecb2c8cf96dd51e7a723ddb7e456d18de9 Mon Sep 17 00:00:00 2001 From: Olivier Patry Date: Mon, 28 Oct 2024 10:08:09 +0100 Subject: [PATCH 3/5] Fix sync of local only tasks (was partial) & split insert & upsert for TaskList & Task --- .../kotlin/net/opatry/tasks/data/TaskDao.kt | 9 +++-- .../net/opatry/tasks/data/TaskListDao.kt | 5 ++- .../net/opatry/tasks/data/TaskRepository.kt | 35 +++++++++---------- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskDao.kt b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskDao.kt index 11ab16c4..8b5c1cc3 100644 --- a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskDao.kt +++ b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskDao.kt @@ -32,17 +32,20 @@ import net.opatry.tasks.data.entity.TaskEntity @Dao interface TaskDao { - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Insert(onConflict = OnConflictStrategy.ABORT) suspend fun insert(item: TaskEntity): Long + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun upsert(item: TaskEntity): Long + @Query("SELECT * FROM task WHERE remote_id = :remoteId") suspend fun getByRemoteId(remoteId: String): TaskEntity? @Query("SELECT * FROM task") fun getAllAsFlow(): Flow> - @Query("SELECT * FROM task WHERE remote_id IS NULL") - suspend fun getLocalOnlyTasks(): List + @Query("SELECT * FROM task WHERE parent_list_local_id = :taskListLocalId AND remote_id IS NULL") + suspend fun getLocalOnlyTasks(taskListLocalId: Long): List // FIXME should be a pending deletion "flag" until sync is done @Query("DELETE FROM task WHERE local_id = :id") diff --git a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskListDao.kt b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskListDao.kt index ad71d1f7..d1138e42 100644 --- a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskListDao.kt +++ b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskListDao.kt @@ -32,9 +32,12 @@ import net.opatry.tasks.data.entity.TaskListEntity @Dao interface TaskListDao { - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Insert(onConflict = OnConflictStrategy.ABORT) suspend fun insert(item: TaskListEntity): Long + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun upsert(item: TaskListEntity): Long + // FIXME should be a pending deletion "flag" until sync is done @Query("DELETE FROM task_list WHERE local_id = :id") suspend fun deleteTaskList(id: Long) diff --git a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt index effa7e08..228cff15 100644 --- a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt +++ b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt @@ -206,18 +206,18 @@ class TaskRepository( // - etc. val existingEntity = taskListDao.getByRemoteId(remoteTaskList.id) val updatedEntity = remoteTaskList.asTaskListEntity(existingEntity?.id, existingEntity?.sorting ?: TaskListEntity.Sorting.UserDefined) - val finalLocalId = taskListDao.insert(updatedEntity) + val finalLocalId = taskListDao.upsert(updatedEntity) taskListIds[finalLocalId] = remoteTaskList.id } taskListDao.deleteStaleTaskLists(remoteTaskLists.map(TaskList::id)) taskListDao.getLocalOnlyTaskLists().onEach { localTaskList -> - val remoteId = try { - taskListsApi.insert(TaskList(title = localTaskList.title)).id + val remoteTaskList = try { + taskListsApi.insert(TaskList(localTaskList.title)) } catch (e: Exception) { null } - if (remoteId != null) { - taskListDao.insert(localTaskList.copy(remoteId = remoteId)) + if (remoteTaskList != null) { + taskListDao.upsert(remoteTaskList.asTaskListEntity(localTaskList.id, localTaskList.sorting)) } } taskListIds.forEach { (localListId, remoteListId) -> @@ -227,17 +227,17 @@ class TaskRepository( val remoteTasks = tasksApi.listAll(remoteListId, showHidden = true, showCompleted = true) remoteTasks.onEach { remoteTask -> val existingEntity = taskDao.getByRemoteId(remoteTask.id) - taskDao.insert(remoteTask.asTaskEntity(localListId, existingEntity?.id)) + taskDao.upsert(remoteTask.asTaskEntity(localListId, existingEntity?.id)) } taskDao.deleteStaleTasks(localListId, remoteTasks.map(Task::id)) - taskDao.getLocalOnlyTasks().onEach { localTask -> - val remoteId = try { - tasksApi.insert(remoteListId, Task(title = localTask.title)).id + taskDao.getLocalOnlyTasks(localListId).onEach { localTask -> + val remoteTask = try { + tasksApi.insert(remoteListId, localTask.asTask()) } catch (e: Exception) { null } - if (remoteId != null) { - taskDao.insert(localTask.copy(remoteId = remoteId)) + if (remoteTask != null) { + taskDao.upsert(remoteTask.asTaskEntity(localListId, localTask.id)) } } } @@ -254,7 +254,7 @@ class TaskRepository( } } if (taskList != null) { - taskListDao.insert(taskList.asTaskListEntity(taskListId, TaskListEntity.Sorting.UserDefined)) + taskListDao.upsert(taskList.asTaskListEntity(taskListId, TaskListEntity.Sorting.UserDefined)) } } @@ -280,7 +280,7 @@ class TaskRepository( title = newTitle, lastUpdateDate = now ) - taskListDao.insert(taskListEntity) + taskListDao.upsert(taskListEntity) if (taskListEntity.remoteId != null) { withContext(Dispatchers.IO) { try { @@ -345,16 +345,13 @@ class TaskRepository( if (taskListEntity.remoteId != null) { val task = withContext(Dispatchers.IO) { try { - tasksApi.insert( - taskListEntity.remoteId, - taskEntity.asTask() - ) + tasksApi.insert(taskListEntity.remoteId, taskEntity.asTask()) } catch (e: Exception) { null } } if (task != null) { - taskDao.insert(task.asTaskEntity(taskListId, taskId)) + taskDao.upsert(task.asTaskEntity(taskListId, taskId)) } } } @@ -386,7 +383,7 @@ class TaskRepository( val task = requireNotNull(taskDao.getById(taskId)) { "Invalid task id $taskId" } val updatedTaskEntity = updateLogic(task, now) ?: return - taskDao.insert(updatedTaskEntity) + taskDao.upsert(updatedTaskEntity) // FIXME should already be available in entity, quick & dirty workaround val taskListRemoteId = updatedTaskEntity.parentTaskRemoteId From 80c43341b598e4c8c5e3f83b49dd27b9599e5e0e Mon Sep 17 00:00:00 2001 From: Olivier Patry Date: Mon, 28 Oct 2024 10:33:02 +0100 Subject: [PATCH 4/5] Enforce proper dispatcher for API calls --- .../net/opatry/tasks/data/TaskRepository.kt | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt index 228cff15..0b5c63da 100644 --- a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt +++ b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt @@ -211,10 +211,12 @@ class TaskRepository( } taskListDao.deleteStaleTaskLists(remoteTaskLists.map(TaskList::id)) taskListDao.getLocalOnlyTaskLists().onEach { localTaskList -> - val remoteTaskList = try { - taskListsApi.insert(TaskList(localTaskList.title)) - } catch (e: Exception) { - null + val remoteTaskList = withContext(Dispatchers.IO) { + try { + taskListsApi.insert(TaskList(localTaskList.title)) + } catch (e: Exception) { + null + } } if (remoteTaskList != null) { taskListDao.upsert(remoteTaskList.asTaskListEntity(localTaskList.id, localTaskList.sorting)) @@ -224,17 +226,21 @@ class TaskRepository( // TODO deal with showDeleted, showHidden, etc. // TODO updatedMin could be used to filter out unchanged tasks since last sync // /!\ this would impact the deleteStaleTasks logic - val remoteTasks = tasksApi.listAll(remoteListId, showHidden = true, showCompleted = true) + val remoteTasks = withContext(Dispatchers.IO) { + tasksApi.listAll(remoteListId, showHidden = true, showCompleted = true) + } remoteTasks.onEach { remoteTask -> val existingEntity = taskDao.getByRemoteId(remoteTask.id) taskDao.upsert(remoteTask.asTaskEntity(localListId, existingEntity?.id)) } taskDao.deleteStaleTasks(localListId, remoteTasks.map(Task::id)) taskDao.getLocalOnlyTasks(localListId).onEach { localTask -> - val remoteTask = try { - tasksApi.insert(remoteListId, localTask.asTask()) - } catch (e: Exception) { - null + val remoteTask = withContext(Dispatchers.IO) { + try { + tasksApi.insert(remoteListId, localTask.asTask()) + } catch (e: Exception) { + null + } } if (remoteTask != null) { taskDao.upsert(remoteTask.asTaskEntity(localListId, localTask.id)) From 0221c30343f56ac2aeec7ad4ff53a1b772812416 Mon Sep 17 00:00:00 2001 From: Olivier Patry Date: Mon, 28 Oct 2024 10:59:28 +0100 Subject: [PATCH 5/5] Enforce hidden status when syncing local only completed tasks --- .../commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt index 0b5c63da..85d22433 100644 --- a/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt +++ b/tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt @@ -118,6 +118,9 @@ private fun TaskEntity.asTask(): Task { updatedDate = lastUpdateDate, status = if (isCompleted) Task.Status.Completed else Task.Status.NeedsAction, completedDate = completionDate, + // doc says it's a read only field, but status is not hidden when syncing local only completed tasks + // forcing the hidden status works and makes everything more consistent (position following 099999... pattern, hidden status) + isHidden = isCompleted, position = position, ) }