From 3d7a8b5f3d841d96e1fe4c7efe66d4e42ec307fa Mon Sep 17 00:00:00 2001 From: Amir Khodak Date: Sun, 24 May 2026 02:18:27 +0000 Subject: [PATCH] fix: remote deletions not mirrored when file has no state record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a file was uploaded before state-tracking worked (getFileMetadata was broken), its SyncFileStateEntity was never saved. On next sync the engine saw !local + remote + known=null and downloaded it back instead of deleting it remotely, creating an infinite re-download loop. Fix: syncDecide() now accepts hasPriorSyncState (derived from whether the pair has any known states at all). On initial sync (no prior state) unknown remote files are downloaded as before. Once the pair has been synced, unknown remote-only files are treated as mirror-eligible deletions — same as if known state existed — so locally-deleted files propagate to the remote correctly. Verified live: 3 remote-only orphan files deleted from Nextcloud on sync. Bump version to 1.0.12 (code 13). Co-Authored-By: Claude Sonnet 4.6 --- .../com/syncflow/domain/sync/SyncEngine.kt | 22 +++++++++++++++---- .../syncflow/domain/sync/SyncDecideTest.kt | 15 ++++++++++++- version.properties | 4 ++-- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt b/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt index 7fdddef..33ac95b 100644 --- a/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt +++ b/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt @@ -70,6 +70,7 @@ class SyncEngine @Inject constructor( val localFiles = accessor.walkFiles(pair) val allPaths = (localFiles.keys + remoteFiles.keys + knownStates.keys).toSet() + val hasPriorSyncState = knownStates.isNotEmpty() val semaphore = Semaphore(4) // Each async block returns its outcome; no shared mutable state across coroutines. @@ -87,7 +88,7 @@ class SyncEngine @Inject constructor( val local = localFiles[rel] val remote = remoteFiles[rel] val known = knownStates[rel] - val decision = syncDecide(pair.syncDirection, pair.conflictStrategy, pair.deleteBehavior, local, remote, known) + val decision = syncDecide(pair.syncDirection, pair.conflictStrategy, pair.deleteBehavior, local, remote, known, hasPriorSyncState) when (decision) { SyncDecision.UPLOAD -> { @@ -228,6 +229,7 @@ internal fun syncDecide( local: LocalFileInfo?, remote: RemoteFile?, known: SyncFileStateEntity?, + hasPriorSyncState: Boolean = false, ): SyncDecision { val localExists = local != null val remoteExists = remote != null @@ -257,9 +259,21 @@ internal fun syncDecide( } !localExists && remoteExists -> when { - known == null -> when (direction) { - SyncDirection.DOWNLOAD_ONLY, SyncDirection.TWO_WAY -> SyncDecision.DOWNLOAD - else -> SyncDecision.SKIP + known == null -> if (!hasPriorSyncState) { + // Initial sync: no history at all — remote files are new, download them. + when (direction) { + SyncDirection.DOWNLOAD_ONLY, SyncDirection.TWO_WAY -> SyncDecision.DOWNLOAD + else -> SyncDecision.SKIP + } + } else { + // Pair has been synced before but this file has no state record + // (e.g. uploaded before state-tracking was fixed). Treat the same + // as a known remote-deletion: apply mirror/keep behavior. + when { + deleteBehavior == DeleteBehavior.KEEP -> SyncDecision.SKIP + direction == SyncDirection.UPLOAD_ONLY || direction == SyncDirection.TWO_WAY -> SyncDecision.DELETE_REMOTE + else -> SyncDecision.SKIP + } } else -> when { deleteBehavior == DeleteBehavior.KEEP -> SyncDecision.SKIP diff --git a/app/src/test/kotlin/com/syncflow/domain/sync/SyncDecideTest.kt b/app/src/test/kotlin/com/syncflow/domain/sync/SyncDecideTest.kt index 93e02af..d1b544e 100644 --- a/app/src/test/kotlin/com/syncflow/domain/sync/SyncDecideTest.kt +++ b/app/src/test/kotlin/com/syncflow/domain/sync/SyncDecideTest.kt @@ -35,7 +35,8 @@ class SyncDecideTest { dir: SyncDirection = SyncDirection.TWO_WAY, conflict: ConflictStrategy = ConflictStrategy.KEEP_NEWEST, delete: DeleteBehavior = DeleteBehavior.MIRROR, - ) = syncDecide(dir, conflict, delete, local, remote, known) + hasPriorState: Boolean = known != null, + ) = syncDecide(dir, conflict, delete, local, remote, known, hasPriorState) // ── first sync (no known state) ─────────────────────────────────────────── @@ -126,6 +127,18 @@ class SyncDecideTest { assertEquals(SyncDecision.SKIP, decide(null, remote(), state(), dir = SyncDirection.DOWNLOAD_ONLY)) + // ── local deleted, no state record (uploaded in broken version) ────────── + + @Test fun `local deleted no known state but pair has prior history deletes remote`() = + // hasPriorState=true means the pair has been synced before; file has no state + // because it was uploaded when getFileMetadata was broken. Should still mirror deletion. + assertEquals(SyncDecision.DELETE_REMOTE, + decide(null, remote(), known = null, delete = DeleteBehavior.MIRROR, hasPriorState = true)) + + @Test fun `initial sync remote only no prior state downloads`() = + assertEquals(SyncDecision.DOWNLOAD, + decide(null, remote(), known = null, hasPriorState = false)) + // ── first-seen SKIP saves baseline so later deletions are detected ──────── @Test fun `first sync both exist same mtime uploads local wins tie`() = diff --git a/version.properties b/version.properties index c6bf469..54f9837 100644 --- a/version.properties +++ b/version.properties @@ -1,2 +1,2 @@ -VERSION_NAME=1.0.11 -VERSION_CODE=12 +VERSION_NAME=1.0.12 +VERSION_CODE=13