fix: remote deletions not mirrored when file has no state record
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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,10 +259,22 @@ internal fun syncDecide(
|
||||
}
|
||||
|
||||
!localExists && remoteExists -> when {
|
||||
known == null -> when (direction) {
|
||||
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
|
||||
direction == SyncDirection.UPLOAD_ONLY || direction == SyncDirection.TWO_WAY -> SyncDecision.DELETE_REMOTE
|
||||
|
||||
@@ -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`() =
|
||||
|
||||
+2
-2
@@ -1,2 +1,2 @@
|
||||
VERSION_NAME=1.0.11
|
||||
VERSION_CODE=12
|
||||
VERSION_NAME=1.0.12
|
||||
VERSION_CODE=13
|
||||
|
||||
Reference in New Issue
Block a user