diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 86ef475..c2d3b13 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -2,8 +2,10 @@ name: Build & Release APK on: push: + branches: ['**'] tags: - 'v*' + pull_request: jobs: build: @@ -20,10 +22,29 @@ jobs: - uses: android-actions/setup-android@v3 - - name: Build debug APK + - name: Run unit tests run: | chmod +x gradlew - ./gradlew assembleDebug --no-daemon + ./gradlew testDebugUnitTest --no-daemon + + - name: Decode release keystore + env: + KEYSTORE_BASE64: ${{ secrets.KEYSTORE_BASE64 }} + run: | + if [ -n "$KEYSTORE_BASE64" ]; then + echo "$KEYSTORE_BASE64" | base64 -d > "$RUNNER_TEMP/release.keystore" + echo "KEYSTORE_PATH=$RUNNER_TEMP/release.keystore" >> "$GITHUB_ENV" + echo "Release keystore decoded — building signed release." + else + echo "::warning::KEYSTORE_BASE64 secret not set — release APK will be debug-signed." + fi + + - name: Build release APK + env: + KEYSTORE_PASSWORD: ${{ secrets.KEYSTORE_PASSWORD }} + KEY_ALIAS: ${{ secrets.KEY_ALIAS }} + KEY_PASSWORD: ${{ secrets.KEY_PASSWORD }} + run: ./gradlew assembleRelease --no-daemon - name: Get version name id: ver @@ -32,10 +53,11 @@ jobs: - name: Rename APK run: | mkdir dist - cp app/build/outputs/apk/debug/app-debug.apk \ + cp app/build/outputs/apk/release/app-release.apk \ dist/SyncFlow-v${{ steps.ver.outputs.name }}.apk - name: Attach APK to release + if: startsWith(github.ref, 'refs/tags/v') env: TOKEN: ${{ secrets.GITHUB_TOKEN }} TAG: ${{ github.ref_name }} diff --git a/app/build.gradle.kts b/app/build.gradle.kts index bd7488b..3f7d2a0 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -18,9 +18,15 @@ val localProps = Properties().apply { if (f.exists()) load(f.inputStream()) } +// Release signing is read from local.properties (local builds) or environment variables +// (CI). When no keystore is available the release build falls back to the debug key so the +// build still succeeds — it just isn't a distributable, properly-signed APK. +val keystorePath = (localProps["KEYSTORE_PATH"] as String?) ?: System.getenv("KEYSTORE_PATH") +val hasReleaseKeystore = keystorePath != null && file(keystorePath).exists() + android { namespace = "com.syncflow" - compileSdk = 34 + compileSdk = 35 defaultConfig { applicationId = "com.syncflow" @@ -38,19 +44,28 @@ android { signingConfigs { create("release") { - storeFile = localProps["KEYSTORE_PATH"]?.toString()?.let { file(it) } - storePassword = localProps["KEYSTORE_PASSWORD"]?.toString() - keyAlias = localProps["KEY_ALIAS"]?.toString() - keyPassword = localProps["KEY_PASSWORD"]?.toString() + if (hasReleaseKeystore) { + storeFile = file(keystorePath!!) + storePassword = (localProps["KEYSTORE_PASSWORD"] as String?) ?: System.getenv("KEYSTORE_PASSWORD") + keyAlias = (localProps["KEY_ALIAS"] as String?) ?: System.getenv("KEY_ALIAS") + keyPassword = (localProps["KEY_PASSWORD"] as String?) ?: System.getenv("KEY_PASSWORD") + } } } buildTypes { release { - isMinifyEnabled = true - isShrinkResources = true + // R8/minify has never been exercised by CI (it only built debug), so leave it off + // to keep the signed release behaving identically to the debug builds in use today. + // Re-enable with proper keep rules and an on-device smoke test if APK size matters. + isMinifyEnabled = false + isShrinkResources = false proguardFiles(getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro") - signingConfig = signingConfigs.getByName("release") + signingConfig = if (hasReleaseKeystore) { + signingConfigs.getByName("release") + } else { + signingConfigs.getByName("debug") + } } } diff --git a/app/src/androidTest/kotlin/com/syncflow/FullSyncEngineTest.kt b/app/src/androidTest/kotlin/com/syncflow/FullSyncEngineTest.kt new file mode 100644 index 0000000..31b1b8a --- /dev/null +++ b/app/src/androidTest/kotlin/com/syncflow/FullSyncEngineTest.kt @@ -0,0 +1,246 @@ +package com.syncflow + +import androidx.room.Room +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.syncflow.data.db.SyncDatabase +import com.syncflow.data.db.entities.CloudAccountEntity +import com.syncflow.data.db.entities.SyncPairEntity +import com.syncflow.data.db.entities.toDomain +import com.syncflow.data.providers.nextcloud.NextcloudProvider +import com.syncflow.domain.model.* +import com.syncflow.domain.sync.SyncEngine +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.* +import org.junit.Assume.assumeTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import java.io.ByteArrayInputStream +import java.io.ByteArrayOutputStream +import java.io.File +import java.time.Instant + +/** + * Full-matrix end-to-end test of the REAL SyncEngine against a live Nextcloud, on the device: + * in-memory Room DB, a real local folder per test, and the real NextcloudProvider over TLS. + * Covers every direction, every delete behavior, updates, nested/recursive, filters, and conflicts. + * + * Creds via instrumentation args: -e ncUrl ... -e ncUser ... -e ncPass ... + */ +@RunWith(AndroidJUnit4::class) +class FullSyncEngineTest { + + private val ctx = InstrumentationRegistry.getInstrumentation().targetContext + private val args = InstrumentationRegistry.getArguments() + private val url get() = args.getString("ncUrl") + private val user get() = args.getString("ncUser") + private val pass get() = args.getString("ncPass") + + private lateinit var db: SyncDatabase + private lateinit var engine: SyncEngine + private lateinit var provider: NextcloudProvider + private var accountId = 0L + private val remoteRoot = "SyncFlowFull_${System.currentTimeMillis()}" + private val localDirs = mutableListOf() + + @Before fun setUp() = runBlocking { + assumeTrue("ncUrl/ncUser/ncPass required", url != null && user != null && pass != null) + db = Room.inMemoryDatabaseBuilder(ctx, SyncDatabase::class.java).build() + val account = CloudAccount(0, "IT", user, ProviderType.NEXTCLOUD, + """{"username":"$user","password":"$pass"}""", url, null) + accountId = db.cloudAccountDao().insert( + CloudAccountEntity(0, account.displayName, account.email, account.providerType, + account.credentialJson, account.serverUrl, account.port)) + provider = NextcloudProvider(account) + engine = SyncEngine(db.syncPairDao(), db.syncFileStateDao(), db.syncConflictDao(), db.syncEventDao(), ctx) + provider.createDirectory(remoteRoot).getOrThrow() + } + + @After fun tearDown() = runBlocking { + runCatching { provider.deleteFile(remoteRoot) } + localDirs.forEach { it.deleteRecursively() } + if (::db.isInitialized) db.close() + } + + // ── helpers ────────────────────────────────────────────────────────────── + + private suspend fun newPair( + name: String, + dir: SyncDirection = SyncDirection.TWO_WAY, + delete: DeleteBehavior = DeleteBehavior.MIRROR, + conflict: ConflictStrategy = ConflictStrategy.KEEP_NEWEST, + recursive: Boolean = true, + excludeExtensions: String = "", + excludePatterns: String = "", + skipHidden: Boolean = false, + maxKb: Long = 0L, + ): Triple { + val local = File(ctx.cacheDir, "synctest_${name}_${System.currentTimeMillis()}").apply { mkdirs() } + localDirs += local + val remote = "$remoteRoot/$name" + provider.createDirectory(remote).getOrThrow() + val id = db.syncPairDao().insert(SyncPairEntity( + id = 0, name = name, localPath = local.absolutePath, remotePath = remote, accountId = accountId, + syncDirection = dir, conflictStrategy = conflict, deleteBehavior = delete, recursive = recursive, + scheduleType = ScheduleType.MANUAL, scheduleIntervalMinutes = 30, scheduleDailyTime = null, scheduleWeekdays = 0, + wifiOnly = false, wifiSsid = "", chargingOnly = false, minBatteryPct = 0, + excludePatterns = excludePatterns, includeExtensions = "", excludeExtensions = excludeExtensions, + skipHiddenFiles = skipHidden, minFileSizeKb = 0, maxFileSizeKb = maxKb, + notifyOnComplete = false, notifyOnError = false, + isEnabled = true, lastSyncAt = null, lastSyncResult = SyncStatus.IDLE, pendingConflicts = 0, + )) + return Triple(db.syncPairDao().getById(id)!!.toDomain(), local, remote) + } + + private suspend fun sync(pair: SyncPair) = engine.sync(pair, provider) + private fun write(dir: File, rel: String, content: String) = + File(dir, rel).apply { parentFile?.mkdirs() }.writeText(content) + private suspend fun remoteNames(remote: String) = + provider.listFiles(remote).getOrThrow().map { it.name } + private suspend fun remoteText(path: String): String { + val out = ByteArrayOutputStream(); provider.downloadFile(path, out).getOrThrow(); return out.toString("UTF-8") + } + private suspend fun putRemote(remote: String, name: String, content: String) { + val b = content.toByteArray() + provider.uploadFile(ByteArrayInputStream(b), "$remote/$name", b.size.toLong()).getOrThrow() + } + + // ── 1. Upload-only backup: delete on phone keeps the cloud copy (KEEP) ──── + @Test fun uploadOnly_keep_localDeleteKeepsCloud() = runBlocking { + val (pair, local, remote) = newPair("ul_keep", SyncDirection.UPLOAD_ONLY, DeleteBehavior.KEEP) + write(local, "a.txt", "hello") + assertEquals(1, sync(pair).uploaded) + assertTrue("a.txt" in remoteNames(remote)) + File(local, "a.txt").delete() + val r = sync(pair) + assertEquals("KEEP must not delete remotely", 0, r.deleted) + assertTrue("cloud copy must survive", "a.txt" in remoteNames(remote)) + } + + // ── 2. Upload-only + MIRROR: local delete removes the cloud copy ────────── + @Test fun uploadOnly_mirror_localDeleteRemovesCloud() = runBlocking { + val (pair, local, remote) = newPair("ul_mirror", SyncDirection.UPLOAD_ONLY, DeleteBehavior.MIRROR) + write(local, "a.txt", "hello"); assertEquals(1, sync(pair).uploaded) + File(local, "a.txt").delete() + assertEquals(1, sync(pair).deleted) + assertFalse("a.txt" in remoteNames(remote)) + } + + // ── 3. Upload-only + ARCHIVE: deleted file moved to _Deleted/ ───────────── + @Test fun uploadOnly_archive_movesToDeleted() = runBlocking { + val (pair, local, remote) = newPair("ul_archive", SyncDirection.UPLOAD_ONLY, DeleteBehavior.ARCHIVE) + write(local, "a.txt", "keepme"); sync(pair) + File(local, "a.txt").delete(); sync(pair) + assertFalse("a.txt" in remoteNames(remote)) + assertTrue("archived copy expected", "a.txt" in remoteNames("$remote/_Deleted")) + } + + // ── 4. Two-way initial sync: each side gets the other's files ───────────── + @Test fun twoWay_initial_mergesBothSides() = runBlocking { + val (pair, local, remote) = newPair("tw_init", SyncDirection.TWO_WAY) + write(local, "local.txt", "L") + putRemote(remote, "remote.txt", "R") + val r = sync(pair) + assertEquals(1, r.uploaded); assertEquals(1, r.downloaded) + assertTrue(File(local, "remote.txt").exists()) + assertTrue("local.txt" in remoteNames(remote) && "remote.txt" in remoteNames(remote)) + } + + // ── 5. Two-way: a local edit propagates to the remote ───────────────────── + @Test fun twoWay_localEdit_updatesRemote() = runBlocking { + val (pair, local, remote) = newPair("tw_edit", SyncDirection.TWO_WAY) + write(local, "f.txt", "v1"); sync(pair) + Thread.sleep(1100) // cross the 1s mtime resolution + write(local, "f.txt", "v2-updated"); val r = sync(pair) + assertEquals(1, r.uploaded) + assertEquals("v2-updated", remoteText("$remote/f.txt")) + } + + // ── 6. Two-way + MIRROR: deleting locally removes it remotely ───────────── + @Test fun twoWay_mirror_localDeletePropagates() = runBlocking { + val (pair, local, remote) = newPair("tw_mirror", SyncDirection.TWO_WAY, DeleteBehavior.MIRROR) + write(local, "f.txt", "x"); sync(pair) + File(local, "f.txt").delete() + assertEquals(1, sync(pair).deleted) + assertFalse("f.txt" in remoteNames(remote)) + } + + // ── 7. Download-only: pulls remote, never uploads local-only files ──────── + @Test fun downloadOnly_pullsRemoteIgnoresLocal() = runBlocking { + val (pair, local, remote) = newPair("dl_only", SyncDirection.DOWNLOAD_ONLY) + putRemote(remote, "cloud.txt", "from-cloud") + write(local, "phoneonly.txt", "P") + val r = sync(pair) + assertEquals(1, r.downloaded); assertEquals(0, r.uploaded) + assertEquals("from-cloud", File(local, "cloud.txt").readText()) + assertFalse("local-only file must NOT upload", "phoneonly.txt" in remoteNames(remote)) + } + + // ── 8. Recursive: nested directory structure is preserved ───────────────── + @Test fun recursive_uploadsNestedTree() = runBlocking { + val (pair, local, remote) = newPair("rec_on", SyncDirection.UPLOAD_ONLY, recursive = true) + write(local, "sub/deep/n.txt", "nested") + assertEquals(1, sync(pair).uploaded) + assertTrue("n.txt" in remoteNames("$remote/sub/deep")) + } + + // ── 9. recursive=false: subfolders are skipped ──────────────────────────── + @Test fun nonRecursive_skipsSubfolders() = runBlocking { + val (pair, local, remote) = newPair("rec_off", SyncDirection.UPLOAD_ONLY, recursive = false) + write(local, "top.txt", "T") + write(local, "sub/deep.txt", "D") + assertEquals(1, sync(pair).uploaded) + assertTrue("top.txt" in remoteNames(remote)) + assertTrue("subfolder must be skipped", remoteNames(remote).none { it == "sub" }) + } + + // ── 10. Filters: excluded extension + hidden file are not uploaded ───────── + @Test fun filters_excludeExtensionAndHidden() = runBlocking { + val (pair, local, remote) = newPair("filters", SyncDirection.UPLOAD_ONLY, + excludeExtensions = "tmp", skipHidden = true) + write(local, "keep.txt", "k") + write(local, "skip.tmp", "s") + write(local, ".hidden", "h") + sync(pair) + val names = remoteNames(remote) + assertTrue("keep.txt" in names) + assertFalse("skip.tmp" in names) + assertFalse(".hidden" in names) + } + + // ── 11. Conflict: both sides changed (ASK) → conflict recorded, no clobber ─ + @Test fun twoWay_bothChanged_recordsConflict() = runBlocking { + val (pair, local, remote) = newPair("conflict", SyncDirection.TWO_WAY, conflict = ConflictStrategy.ASK) + write(local, "c.txt", "base"); sync(pair) // upload + sync(pair) // reconcile: record remote baseline (etag/mtime) + Thread.sleep(1100) + write(local, "c.txt", "LOCAL-change") // change local + putRemote(remote, "c.txt", "REMOTE-change") // change remote out-of-band + val r = sync(pair) + assertEquals("a conflict must be detected", 1, r.conflicts) + // ASK must not silently overwrite either side + assertEquals("LOCAL-change", File(local, "c.txt").readText()) + assertEquals("REMOTE-change", remoteText("$remote/c.txt")) + } + + // ── 12. Conflict KEEP_NEWEST: newer local wins and uploads ──────────────── + @Test fun twoWay_keepNewest_newerLocalWins() = runBlocking { + val (pair, local, remote) = newPair("newest", SyncDirection.TWO_WAY, conflict = ConflictStrategy.KEEP_NEWEST) + write(local, "n.txt", "base"); sync(pair) + putRemote(remote, "n.txt", "remote-older") + Thread.sleep(1100) + write(local, "n.txt", "local-newer") // local is newer than remote + sync(pair) + assertEquals("newer local must win", "local-newer", remoteText("$remote/n.txt")) + } + + // ── 13. Content integrity: binary-ish bytes round-trip exactly ──────────── + @Test fun contentIntegrity_roundTrip() = runBlocking { + val (pair, local, remote) = newPair("integrity", SyncDirection.TWO_WAY) + val payload = (0..5000).joinToString("") { "Ω$it·" } + write(local, "big.txt", payload); sync(pair) + assertEquals(payload, remoteText("$remote/big.txt")) + } +} diff --git a/app/src/androidTest/kotlin/com/syncflow/NextcloudIntegrationTest.kt b/app/src/androidTest/kotlin/com/syncflow/NextcloudIntegrationTest.kt new file mode 100644 index 0000000..7fa129c --- /dev/null +++ b/app/src/androidTest/kotlin/com/syncflow/NextcloudIntegrationTest.kt @@ -0,0 +1,117 @@ +package com.syncflow + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.syncflow.data.db.entities.SyncFileStateEntity +import com.syncflow.data.providers.nextcloud.NextcloudProvider +import com.syncflow.domain.model.CloudAccount +import com.syncflow.domain.model.ConflictStrategy +import com.syncflow.domain.model.DeleteBehavior +import com.syncflow.domain.model.ProviderType +import com.syncflow.domain.model.SyncDirection +import com.syncflow.domain.sync.LocalFileInfo +import com.syncflow.domain.sync.SyncDecision +import com.syncflow.domain.sync.syncDecide +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Assume.assumeTrue +import org.junit.Test +import org.junit.runner.RunWith +import java.io.ByteArrayInputStream +import java.io.ByteArrayOutputStream +import java.time.Instant + +/** + * Real end-to-end test against a live Nextcloud, run ON the device. Exercises the actual + * NextcloudProvider (WebDAV over real TLS, including the atomic temp+MOVE upload) and proves + * the backup guarantee: with Upload-only + KEEP, "deleted on phone" leaves the cloud copy. + * + * Credentials are passed as instrumentation args (never committed): + * adb shell am instrument -w \ + * -e ncUrl https://nextcloud.khodak.me -e ncUser syncflow-test -e ncPass \ + * com.syncflow.test/androidx.test.runner.AndroidJUnitRunner + */ +@RunWith(AndroidJUnit4::class) +class NextcloudIntegrationTest { + + private val args = InstrumentationRegistry.getArguments() + private val url = args.getString("ncUrl") + private val user = args.getString("ncUser") + private val pass = args.getString("ncPass") + + private fun provider(): NextcloudProvider { + val account = CloudAccount( + id = 1L, + displayName = "IT", + email = user, // Nextcloud dav path uses this + providerType = ProviderType.NEXTCLOUD, + credentialJson = """{"username":"$user","password":"$pass"}""", + serverUrl = url, + port = null, + ) + return NextcloudProvider(account) + } + + @Test + fun fullBackupRoundTrip() = runBlocking { + assumeTrue("ncUrl/ncUser/ncPass instrumentation args required", url != null && user != null && pass != null) + val p = provider() + val dir = "SyncFlowITest_${System.currentTimeMillis()}" + val remoteFile = "$dir/hello.txt" + val content = "SyncFlow integration test — 0 to 100 — ${System.currentTimeMillis()}".toByteArray() + + try { + // 1. Connect + assertTrue("testConnection failed", p.testConnection().isSuccess) + + // 2. Create the backup folder + assertTrue("createDirectory failed", p.createDirectory(dir).isSuccess) + + // 3. Upload (exercises atomic temp-file + MOVE) + val uploaded = p.uploadFile(ByteArrayInputStream(content), remoteFile, content.size.toLong()) + assertTrue("upload failed: ${uploaded.exceptionOrNull()}", uploaded.isSuccess) + + // 4. List — the file is on the cloud with the right size + val listed = p.listFiles(dir).getOrThrow() + val entry = listed.firstOrNull { it.name == "hello.txt" } + assertNotNull("uploaded file not found in listing", entry) + assertEquals("remote size mismatch", content.size.toLong(), entry!!.sizeBytes) + + // 5. Download — bytes round-trip intact + val out = ByteArrayOutputStream() + assertTrue("download failed", p.downloadFile(remoteFile, out).isSuccess) + assertEquals("downloaded content mismatch", String(content), out.toString("UTF-8")) + + // 6. THE backup guarantee. Phone copy deleted, state record exists, Upload-only + KEEP. + val known = SyncFileStateEntity( + syncPairId = 1L, relativePath = "hello.txt", + localModifiedAt = Instant.now(), localSizeBytes = content.size.toLong(), localHash = null, + remoteModifiedAt = entry.modifiedAt, remoteSizeBytes = entry.sizeBytes, remoteEtag = entry.etag, + lastSyncedAt = Instant.now(), syncedHash = null, + ) + val keepDecision = syncDecide( + SyncDirection.UPLOAD_ONLY, ConflictStrategy.KEEP_NEWEST, DeleteBehavior.KEEP, + local = null, remote = entry, known = known, hasPriorSyncState = true, + ) + assertEquals("KEEP must not delete the cloud copy", SyncDecision.SKIP, keepDecision) + + // ...and the engine would do nothing, so the file is verifiably STILL on the cloud: + val stillThere = p.listFiles(dir).getOrThrow().any { it.name == "hello.txt" } + assertTrue("cloud copy must survive a local delete under KEEP", stillThere) + + // 7. Contrast: MIRROR would delete it — prove the real DELETE works (also cleanup). + val mirrorDecision = syncDecide( + SyncDirection.UPLOAD_ONLY, ConflictStrategy.KEEP_NEWEST, DeleteBehavior.MIRROR, + local = null, remote = entry, known = known, hasPriorSyncState = true, + ) + assertEquals(SyncDecision.DELETE_REMOTE, mirrorDecision) + assertTrue("deleteFile failed", p.deleteFile(remoteFile).isSuccess) + val goneAfterDelete = p.listFiles(dir).getOrThrow().none { it.name == "hello.txt" } + assertTrue("file should be gone after explicit remote delete", goneAfterDelete) + } finally { + runCatching { p.deleteFile(dir) } // best-effort cleanup of the test folder + } + } +} diff --git a/app/src/main/kotlin/com/syncflow/data/providers/sftp/SftpProvider.kt b/app/src/main/kotlin/com/syncflow/data/providers/sftp/SftpProvider.kt index ebd8c00..853569e 100644 --- a/app/src/main/kotlin/com/syncflow/data/providers/sftp/SftpProvider.kt +++ b/app/src/main/kotlin/com/syncflow/data/providers/sftp/SftpProvider.kt @@ -43,17 +43,21 @@ class SftpProvider(private val account: CloudAccount, private val credentialStor override suspend fun listFiles(remotePath: String): Result> = runCatching { withSftp { sftp -> - sftp.ls(remotePath).map { entry -> - RemoteFile( - path = "$remotePath/${entry.name}".replace("//", "/"), - name = entry.name, - isDirectory = entry.isDirectory, - sizeBytes = entry.attributes.size, - modifiedAt = Instant.ofEpochSecond(entry.attributes.mtime.toLong()), - etag = null, - mimeType = null, - ) - } + sftp.ls(remotePath) + // Drop "."/".." and any name with a path separator so a hostile server can't + // smuggle a traversal segment into a local/remote path. + .filter { it.name != "." && it.name != ".." && !it.name.contains('/') && !it.name.contains('\\') } + .map { entry -> + RemoteFile( + path = "$remotePath/${entry.name}".replace("//", "/"), + name = entry.name, + isDirectory = entry.isDirectory, + sizeBytes = entry.attributes.size, + modifiedAt = Instant.ofEpochSecond(entry.attributes.mtime.toLong()), + etag = null, + mimeType = null, + ) + } } } @@ -63,12 +67,25 @@ class SftpProvider(private val account: CloudAccount, private val credentialStor sizeBytes: Long, onProgress: (Long) -> Unit, ): Result = runCatching { + // Upload to a hidden temp sibling, then rename onto the destination so an interrupted + // transfer never leaves a truncated file at the real path. + val dir = remotePath.substringBeforeLast('/', "") + val name = remotePath.substringAfterLast('/') + val tmpPath = if (dir.isEmpty()) ".$name.sfpart" else "$dir/.$name.sfpart" withSftp { sftp -> sftp.put(object : InMemorySourceFile() { - override fun getName() = remotePath.substringAfterLast('/') + override fun getName() = name override fun getLength() = sizeBytes override fun getInputStream() = localStream - }, remotePath) + }, tmpPath) + // SFTP rename fails if the target exists on servers without the POSIX-rename + // extension, so fall back to removing the destination first. + try { + sftp.rename(tmpPath, remotePath) + } catch (e: Exception) { + runCatching { sftp.rm(remotePath) } + sftp.rename(tmpPath, remotePath) + } } getFileMetadata(remotePath).getOrThrow() } diff --git a/app/src/main/kotlin/com/syncflow/data/providers/webdav/WebDavProvider.kt b/app/src/main/kotlin/com/syncflow/data/providers/webdav/WebDavProvider.kt index 73bc075..9851d5e 100644 --- a/app/src/main/kotlin/com/syncflow/data/providers/webdav/WebDavProvider.kt +++ b/app/src/main/kotlin/com/syncflow/data/providers/webdav/WebDavProvider.kt @@ -93,15 +93,30 @@ open class WebDavProvider(protected val account: CloudAccount) : CloudProvider { localStream.source().use { source -> sink.writeAll(source) } } } - val req = Request.Builder().url(url(remotePath)).put(body).build() + // Upload to a hidden temp sibling first, then MOVE it onto the destination. A + // failed PUT leaves the real file untouched instead of overwriting it with a + // truncated body; the MOVE is a server-side atomic-ish swap. + val tmpPath = tempPathFor(remotePath) + val req = Request.Builder().url(url(tmpPath)).put(body).build() client.newCall(req).execute().use { resp -> if (!resp.isSuccessful) throw Exception("Upload HTTP ${resp.code}") } + moveFile(tmpPath, remotePath).getOrElse { e -> + runCatching { deleteFile(tmpPath) } + throw e + } onProgress(sizeBytes) getFileMetadata(remotePath).getOrThrow() } } + private fun tempPathFor(remotePath: String): String { + val dir = remotePath.substringBeforeLast('/', "") + val name = remotePath.substringAfterLast('/') + val tmp = ".$name.sfpart" + return if (dir.isEmpty()) tmp else "$dir/$tmp" + } + override suspend fun downloadFile(remotePath: String, destination: OutputStream, onProgress: (Long) -> Unit): Result = runCatching { withContext(Dispatchers.IO) { val req = Request.Builder().url(url(remotePath)).get().build() diff --git a/app/src/main/kotlin/com/syncflow/domain/sync/LocalAccessor.kt b/app/src/main/kotlin/com/syncflow/domain/sync/LocalAccessor.kt index 1146922..273a438 100644 --- a/app/src/main/kotlin/com/syncflow/domain/sync/LocalAccessor.kt +++ b/app/src/main/kotlin/com/syncflow/domain/sync/LocalAccessor.kt @@ -5,6 +5,8 @@ import android.net.Uri import android.provider.DocumentsContract import com.syncflow.domain.model.SyncPair import java.io.File +import java.io.FileOutputStream +import java.io.IOException import java.io.InputStream import java.io.OutputStream @@ -14,10 +16,17 @@ sealed class LocalAccessor { abstract fun walkFiles(pair: SyncPair): Map abstract fun openInputStream(relativePath: String): InputStream? - abstract fun createOutputStream(relativePath: String): OutputStream? abstract fun delete(relativePath: String): Boolean abstract fun lastModifiedMs(relativePath: String): Long + /** + * Write [relativePath] atomically: stream into a temp sibling first, then swap it into + * place only after [write] completes without throwing. An interrupted transfer (network + * drop, process death) leaves the existing destination untouched instead of truncating it. + * On failure the temp is removed and the exception is rethrown. + */ + abstract suspend fun writeAtomically(relativePath: String, write: suspend (OutputStream) -> Unit) + // ── java.io.File backend (regular /storage/... paths) ──────────────────── class JavaFile(private val root: File) : LocalAccessor() { @@ -48,10 +57,30 @@ sealed class LocalAccessor { override fun openInputStream(relativePath: String): InputStream = File(root, relativePath).inputStream() - override fun createOutputStream(relativePath: String): OutputStream { + override suspend fun writeAtomically(relativePath: String, write: suspend (OutputStream) -> Unit) { val dest = File(root, relativePath) dest.parentFile?.mkdirs() - return dest.outputStream() + val tmp = File(dest.parentFile, ".${dest.name}.sfpart") + try { + FileOutputStream(tmp).use { os -> + write(os) + os.flush() + os.fd.sync() // durably persist bytes before the rename swaps the file in + } + } catch (e: Throwable) { + tmp.delete() + throw e + } + // Same-directory rename is atomic on POSIX/Android and replaces the destination. + if (!tmp.renameTo(dest)) { + try { + tmp.copyTo(dest, overwrite = true) + } catch (e: Throwable) { + tmp.delete() + throw e + } + tmp.delete() + } } override fun delete(relativePath: String): Boolean = File(root, relativePath).delete() @@ -131,7 +160,7 @@ sealed class LocalAccessor { return resolver.openInputStream(docUri) } - override fun createOutputStream(relativePath: String): OutputStream? { + override suspend fun writeAtomically(relativePath: String, write: suspend (OutputStream) -> Unit) { val parts = relativePath.replace('\\', '/').split('/') var currentId = DocumentsContract.getTreeDocumentId(treeUri) @@ -141,7 +170,7 @@ sealed class LocalAccessor { val parentUri = DocumentsContract.buildDocumentUriUsingTree(treeUri, currentId) val newDir = DocumentsContract.createDocument( resolver, parentUri, DocumentsContract.Document.MIME_TYPE_DIR, parts[i] - ) ?: return null + ) ?: throw IOException("Cannot create directory ${parts[i]} for $relativePath") DocumentsContract.getDocumentId(newDir) } } @@ -149,19 +178,47 @@ sealed class LocalAccessor { val fileName = parts.last() val childrenUri = DocumentsContract.buildChildDocumentsUriUsingTree(treeUri, currentId) val parentUri = DocumentsContract.buildDocumentUriUsingTree(treeUri, currentId) + val tmpName = ".$fileName.sfpart" - // Delete existing to allow overwrite - findChildId(childrenUri, fileName)?.let { existingId -> - DocumentsContract.deleteDocument( - resolver, - DocumentsContract.buildDocumentUriUsingTree(treeUri, existingId) - ) + // Clear any leftover temp document from a previously interrupted write. + findChildId(childrenUri, tmpName)?.let { staleId -> + runCatching { + DocumentsContract.deleteDocument( + resolver, DocumentsContract.buildDocumentUriUsingTree(treeUri, staleId) + ) + } } - val newUri = DocumentsContract.createDocument( - resolver, parentUri, "application/octet-stream", fileName - ) ?: return null - return resolver.openOutputStream(newUri) + val tmpUri = DocumentsContract.createDocument( + resolver, parentUri, "application/octet-stream", tmpName + ) ?: throw IOException("Cannot create temp document for $relativePath") + + try { + (resolver.openOutputStream(tmpUri) + ?: throw IOException("Cannot open temp stream for $relativePath")).use { os -> + write(os) + os.flush() + } + } catch (e: Throwable) { + runCatching { DocumentsContract.deleteDocument(resolver, tmpUri) } + throw e + } + + // Commit: remove the existing destination, then rename the fully-written temp into + // place. If interrupted between the two steps the temp still holds the complete data + // (recoverable by hand), which is strictly safer than truncating the destination. + findChildId(childrenUri, fileName)?.let { existingId -> + DocumentsContract.deleteDocument( + resolver, DocumentsContract.buildDocumentUriUsingTree(treeUri, existingId) + ) + } + val renamed = DocumentsContract.renameDocument(resolver, tmpUri, fileName) + if (renamed == null) { + runCatching { DocumentsContract.deleteDocument(resolver, tmpUri) } + throw IOException("Cannot finalize $relativePath") + } + // Drop the stale cache entry so the next read re-resolves the new document id. + docIdCache.remove(relativePath) } override fun delete(relativePath: String): Boolean { 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 339f382..0a5b7c6 100644 --- a/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt +++ b/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt @@ -111,6 +111,14 @@ class SyncEngine @Inject constructor( allPaths.map { rel -> async { semaphore.withPermit { + // Defense-in-depth against a malicious/compromised remote returning a + // path that escapes the sync root (e.g. "../../evil"). Skip rather than + // write outside pair.localPath / pair.remotePath. + if (isUnsafeSyncPath(rel)) { + Timber.w("SyncEngine: skipping unsafe path for pair ${pair.id}: $rel") + logEvent(pair.id, SyncEventType.FILE_SKIPPED, rel, "unsafe path", 0) + return@withPermit FileOutcome(skipped = 1) + } val local = localFiles[rel] val remote = remoteFiles[rel] val known = knownStates[rel] @@ -142,8 +150,8 @@ class SyncEngine @Inject constructor( } SyncDecision.DOWNLOAD -> { val bytes = runCatching { - accessor.createOutputStream(rel)?.use { stream -> - provider.downloadFile("${pair.remotePath}/$rel", stream) { } + accessor.writeAtomically(rel) { stream -> + provider.downloadFile("${pair.remotePath}/$rel", stream) { }.getOrThrow() } remote!!.sizeBytes }.getOrElse { e -> @@ -178,6 +186,10 @@ class SyncEngine @Inject constructor( if (pair.deleteBehavior == DeleteBehavior.ARCHIVE) { val archivePath = "${pair.remotePath}/_Deleted/$rel" runCatching { + // Create the _Deleted base itself first — ensureRemoteDirs only + // makes sub-parents of rel, so for a top-level file the MOVE + // would otherwise fail with a missing-parent error. + provider.createDirectory("${pair.remotePath}/_Deleted") ensureRemoteDirs(provider, "${pair.remotePath}/_Deleted", rel) provider.moveFile("${pair.remotePath}/$rel", archivePath).getOrThrow() }.onFailure { e -> Timber.e(e, "SyncEngine: ARCHIVE failed for $rel") } @@ -363,6 +375,19 @@ internal fun syncDecide( enum class SyncDecision { UPLOAD, DOWNLOAD, DELETE_LOCAL, DELETE_REMOTE, CONFLICT, SKIP } +/** + * True if a relative sync path is unsafe to act on — empty, absolute, or containing a ".." + * segment that would let a hostile remote escape the sync root via path traversal. Applied to + * every path before any file operation as defense-in-depth (WebDAV already filters names at the + * parser; SFTP and any future provider are covered here). + */ +internal fun isUnsafeSyncPath(rel: String): Boolean { + if (rel.isBlank()) return true + val normalized = rel.replace('\\', '/') + if (normalized.startsWith("/")) return true + return normalized.split('/').any { it == ".." } +} + data class SyncResult( val uploaded: Int = 0, val downloaded: Int = 0, diff --git a/app/src/main/kotlin/com/syncflow/ui/addpair/AddPairScreen.kt b/app/src/main/kotlin/com/syncflow/ui/addpair/AddPairScreen.kt index 0692431..4583c0f 100644 --- a/app/src/main/kotlin/com/syncflow/ui/addpair/AddPairScreen.kt +++ b/app/src/main/kotlin/com/syncflow/ui/addpair/AddPairScreen.kt @@ -5,6 +5,8 @@ import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.clickable +import androidx.compose.foundation.selection.selectable +import androidx.compose.ui.semantics.Role import androidx.compose.foundation.layout.* import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.text.KeyboardOptions @@ -163,7 +165,7 @@ fun AddPairScreen(onDone: () -> Unit, vm: AddPairViewModel = hiltViewModel()) { label = "Direction", options = SyncDirection.entries, selected = s.syncDirection, - onSelect = { vm.update { copy(syncDirection = it) } }, + onSelect = { vm.setDirection(it) }, itemLabel = { "${it.label} — ${it.description}" }, ) Spacer(Modifier.height(8.dp)) @@ -179,7 +181,7 @@ fun AddPairScreen(onDone: () -> Unit, vm: AddPairViewModel = hiltViewModel()) { label = "Deletion behaviour", options = DeleteBehavior.entries, selected = s.deleteBehavior, - onSelect = { vm.update { copy(deleteBehavior = it) } }, + onSelect = { vm.setDeleteBehavior(it) }, itemLabel = { "${it.label} — ${it.description}" }, ) } @@ -382,10 +384,17 @@ private fun RadioGroup( } options.forEach { option -> Row( - modifier = Modifier.fillMaxWidth(), + modifier = Modifier + .fillMaxWidth() + .selectable( + selected = option == selected, + role = Role.RadioButton, + onClick = { onSelect(option) }, + ), verticalAlignment = Alignment.CenterVertically, ) { - RadioButton(selected = option == selected, onClick = { onSelect(option) }) + // onClick = null: the whole row handles selection (bigger tap target + a11y). + RadioButton(selected = option == selected, onClick = null) Text(itemLabel(option), style = MaterialTheme.typography.bodyMedium) } } diff --git a/app/src/main/kotlin/com/syncflow/ui/addpair/AddPairViewModel.kt b/app/src/main/kotlin/com/syncflow/ui/addpair/AddPairViewModel.kt index 29f88b3..e87485f 100644 --- a/app/src/main/kotlin/com/syncflow/ui/addpair/AddPairViewModel.kt +++ b/app/src/main/kotlin/com/syncflow/ui/addpair/AddPairViewModel.kt @@ -28,7 +28,10 @@ data class AddPairUiState( // ── Sync type ──────────────────────────────────────────────────────────── val syncDirection: SyncDirection = SyncDirection.TWO_WAY, val conflictStrategy: ConflictStrategy = ConflictStrategy.KEEP_NEWEST, - val deleteBehavior: DeleteBehavior = DeleteBehavior.MIRROR, + val deleteBehavior: DeleteBehavior = recommendedDeleteBehavior(SyncDirection.TWO_WAY), + // True once the user explicitly picks a deletion behaviour, so changing direction stops + // auto-overriding their choice. + val deleteBehaviorTouched: Boolean = false, val recursive: Boolean = true, // ── Schedule ───────────────────────────────────────────────────────────── val scheduleType: ScheduleType = ScheduleType.INTERVAL, @@ -56,6 +59,16 @@ data class AddPairUiState( val done: Boolean = false, ) +/** + * Safe default deletion behaviour for a given direction. One-way backups must NOT propagate a + * local deletion to the cloud (the whole point of a backup), so they default to KEEP; two-way + * sync defaults to MIRROR. The user can always override — all three options stay selectable. + */ +internal fun recommendedDeleteBehavior(direction: SyncDirection): DeleteBehavior = when (direction) { + SyncDirection.UPLOAD_ONLY, SyncDirection.DOWNLOAD_ONLY -> DeleteBehavior.KEEP + SyncDirection.TWO_WAY -> DeleteBehavior.MIRROR +} + @HiltViewModel class AddPairViewModel @Inject constructor( private val syncPairDao: SyncPairDao, @@ -93,6 +106,7 @@ class AddPairViewModel @Inject constructor( syncDirection = pair.syncDirection, conflictStrategy = pair.conflictStrategy, deleteBehavior = pair.deleteBehavior, + deleteBehaviorTouched = true, // preserve the saved choice when editing recursive = pair.recursive, scheduleType = pair.scheduleType, intervalMinutes = pair.scheduleIntervalMinutes, @@ -119,6 +133,18 @@ class AddPairViewModel @Inject constructor( fun update(transform: AddPairUiState.() -> AddPairUiState) = _state.update(transform) + /** Changing direction re-applies the safe deletion default unless the user already chose one. */ + fun setDirection(direction: SyncDirection) = _state.update { s -> + s.copy( + syncDirection = direction, + deleteBehavior = if (s.deleteBehaviorTouched) s.deleteBehavior else recommendedDeleteBehavior(direction), + ) + } + + fun setDeleteBehavior(behavior: DeleteBehavior) = _state.update { + it.copy(deleteBehavior = behavior, deleteBehaviorTouched = true) + } + fun save() { val s = _state.value val errors = buildList { diff --git a/app/src/test/kotlin/com/syncflow/domain/sync/PathSafetyTest.kt b/app/src/test/kotlin/com/syncflow/domain/sync/PathSafetyTest.kt new file mode 100644 index 0000000..3e8be49 --- /dev/null +++ b/app/src/test/kotlin/com/syncflow/domain/sync/PathSafetyTest.kt @@ -0,0 +1,36 @@ +package com.syncflow.domain.sync + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Path-traversal guard: a hostile/compromised remote must not be able to make the engine read + * or write outside the sync root via "..", absolute, or separator-smuggled paths. + */ +class PathSafetyTest { + + @Test fun `normal relative paths are allowed`() { + assertFalse(isUnsafeSyncPath("photo.jpg")) + assertFalse(isUnsafeSyncPath("sub/dir/photo.jpg")) + assertFalse(isUnsafeSyncPath("a.b..c/file.txt")) // ".." only inside a name, not a segment + } + + @Test fun `parent-dir traversal is rejected`() { + assertTrue(isUnsafeSyncPath("..")) + assertTrue(isUnsafeSyncPath("../evil")) + assertTrue(isUnsafeSyncPath("a/../../etc/passwd")) + assertTrue(isUnsafeSyncPath("sub/../../escape")) + } + + @Test fun `backslash traversal is rejected`() { + assertTrue(isUnsafeSyncPath("..\\evil")) + assertTrue(isUnsafeSyncPath("a\\..\\..\\escape")) + } + + @Test fun `absolute and empty paths are rejected`() { + assertTrue(isUnsafeSyncPath("/etc/passwd")) + assertTrue(isUnsafeSyncPath("")) + assertTrue(isUnsafeSyncPath(" ")) + } +} 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 d1b544e..391cfe7 100644 --- a/app/src/test/kotlin/com/syncflow/domain/sync/SyncDecideTest.kt +++ b/app/src/test/kotlin/com/syncflow/domain/sync/SyncDecideTest.kt @@ -91,10 +91,19 @@ class SyncDecideTest { decide(local(ts), remote(ts, etag = "e"), state(localMs = ts, remoteMs = ts, etag = "e"))) } - @Test fun `1ms difference detected as local change`() { + @Test fun `sub-second mtime difference treated as unchanged`() { + // Second-precision comparison is intentional: FAT32 has 2s mtime resolution and WebDAV + // 1s, so sub-second deltas are phantom changes that caused rewrite loops. A 1ms diff + // within the same second must NOT be treated as a change. + val ts = 1_716_393_136_789L + assertEquals(SyncDecision.SKIP, + decide(local(ts + 1), remote(ts, etag = "e"), state(localMs = ts, remoteMs = ts, etag = "e"))) + } + + @Test fun `mtime change of a full second detected as local change`() { val ts = 1_716_393_136_789L assertEquals(SyncDecision.UPLOAD, - decide(local(ts + 1), remote(ts, etag = "e"), state(localMs = ts, remoteMs = ts, etag = "e"))) + decide(local(ts + 1000), remote(ts, etag = "e"), state(localMs = ts, remoteMs = ts, etag = "e"))) } @Test fun `epoch-second stored value differs from millis comparison`() { @@ -127,12 +136,14 @@ class SyncDecideTest { assertEquals(SyncDecision.SKIP, decide(null, remote(), state(), dir = SyncDirection.DOWNLOAD_ONLY)) - // ── local deleted, no state record (uploaded in broken version) ────────── + // ── remote exists, no state record: never delete on ambiguity ──────────── - @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, + @Test fun `remote exists with no state record downloads rather than deleting`() = + // known=null can mean a brand-new remote file OR one whose state was lost. The engine + // cannot tell them apart, so it downloads rather than risk deleting a real file — + // worst case is a re-downloaded file, never a lost one. A file the user genuinely + // deleted locally still has its state record, which routes to DELETE_REMOTE. + assertEquals(SyncDecision.DOWNLOAD, decide(null, remote(), known = null, delete = DeleteBehavior.MIRROR, hasPriorState = true)) @Test fun `initial sync remote only no prior state downloads`() = diff --git a/app/src/test/kotlin/com/syncflow/domain/sync/UploadBackupLifecycleTest.kt b/app/src/test/kotlin/com/syncflow/domain/sync/UploadBackupLifecycleTest.kt new file mode 100644 index 0000000..8fba158 --- /dev/null +++ b/app/src/test/kotlin/com/syncflow/domain/sync/UploadBackupLifecycleTest.kt @@ -0,0 +1,117 @@ +package com.syncflow.domain.sync + +import com.syncflow.data.db.entities.SyncFileStateEntity +import com.syncflow.domain.model.ConflictStrategy +import com.syncflow.domain.model.DeleteBehavior +import com.syncflow.domain.model.RemoteFile +import com.syncflow.domain.model.SyncDirection +import org.junit.Assert.assertEquals +import org.junit.Test +import java.time.Instant + +/** + * End-to-end decision lifecycle for the backup scenario: + * + * "I back up my phone to the cloud, then I delete the file on the phone. + * It must stay in the cloud." + * + * These walk the exact multi-cycle state the SyncEngine produces: + * - a successful UPLOAD saves state with the local mtime known and remote metadata null + * (SyncEngine.buildState(..., remoteAfterTransfer = null)), + * - the next sync sees both sides present and unchanged, returns SKIP, and the SKIP branch + * reconciles the record by filling in the remote metadata, + * - then the local file is deleted and we assert what happens to the cloud copy. + * + * The decision is driven entirely by deleteBehavior, so each terminal case is asserted for + * KEEP, MIRROR, and ARCHIVE. + */ +class UploadBackupLifecycleTest { + + private val T0 = 1_716_393_136_000L // exact second boundary + + private fun local(ms: Long = T0, size: Long = 100L) = + LocalFileInfo("photo.jpg", size, ms) + + private fun remote(ms: Long = T0, etag: String? = "etag1", size: Long = 100L) = + RemoteFile("backup/photo.jpg", "photo.jpg", false, size, Instant.ofEpochMilli(ms), etag, null) + + /** Mirrors SyncEngine.buildState right after a successful UPLOAD: remote metadata still null. */ + private fun stateAfterUpload(ms: Long = T0) = SyncFileStateEntity( + syncPairId = 1L, relativePath = "photo.jpg", + localModifiedAt = Instant.ofEpochMilli(ms), localSizeBytes = 100L, localHash = null, + remoteModifiedAt = null, remoteSizeBytes = 0L, remoteEtag = null, + lastSyncedAt = Instant.now(), syncedHash = null, + ) + + /** Mirrors the record after the next sync's SKIP reconciliation fills in remote metadata. */ + private fun stateReconciled(ms: Long = T0, etag: String? = "etag1") = SyncFileStateEntity( + syncPairId = 1L, relativePath = "photo.jpg", + localModifiedAt = Instant.ofEpochMilli(ms), localSizeBytes = 100L, localHash = null, + remoteModifiedAt = Instant.ofEpochMilli(ms), remoteSizeBytes = 100L, remoteEtag = etag, + lastSyncedAt = Instant.now(), syncedHash = null, + ) + + private fun decide( + local: LocalFileInfo?, + remote: RemoteFile?, + known: SyncFileStateEntity?, + delete: DeleteBehavior, + ) = syncDecide( + SyncDirection.UPLOAD_ONLY, ConflictStrategy.KEEP_NEWEST, delete, + local, remote, known, hasPriorSyncState = known != null, + ) + + // ── Cycle 1: first backup uploads the file ─────────────────────────────── + + @Test fun `cycle 1 - first backup uploads regardless of delete behavior`() { + assertEquals(SyncDecision.UPLOAD, decide(local(), null, null, DeleteBehavior.KEEP)) + assertEquals(SyncDecision.UPLOAD, decide(local(), null, null, DeleteBehavior.MIRROR)) + assertEquals(SyncDecision.UPLOAD, decide(local(), null, null, DeleteBehavior.ARCHIVE)) + } + + // ── Cycle 2: file present on both sides, unchanged -> SKIP (no deletion) ── + + @Test fun `cycle 2 - unchanged file skips right after upload (remote metadata still null)`() { + assertEquals(SyncDecision.SKIP, decide(local(), remote(), stateAfterUpload(), DeleteBehavior.KEEP)) + assertEquals(SyncDecision.SKIP, decide(local(), remote(), stateAfterUpload(), DeleteBehavior.MIRROR)) + } + + @Test fun `cycle 2 - unchanged file skips once state is reconciled`() { + assertEquals(SyncDecision.SKIP, decide(local(), remote(), stateReconciled(), DeleteBehavior.KEEP)) + assertEquals(SyncDecision.SKIP, decide(local(), remote(), stateReconciled(), DeleteBehavior.MIRROR)) + } + + // ── Cycle 3: deleted on the phone — THE scenario ───────────────────────── + + @Test fun `KEEP - deleting on phone leaves the cloud copy (correct backup behavior)`() { + // Reconciled steady state: + assertEquals(SyncDecision.SKIP, decide(null, remote(), stateReconciled(), DeleteBehavior.KEEP)) + // And even if deletion happens before the reconcile pass ran: + assertEquals(SyncDecision.SKIP, decide(null, remote(), stateAfterUpload(), DeleteBehavior.KEEP)) + } + + @Test fun `MIRROR - deleting on phone DELETES the cloud copy (wrong for a backup)`() { + assertEquals(SyncDecision.DELETE_REMOTE, decide(null, remote(), stateReconciled(), DeleteBehavior.MIRROR)) + assertEquals(SyncDecision.DELETE_REMOTE, decide(null, remote(), stateAfterUpload(), DeleteBehavior.MIRROR)) + } + + @Test fun `ARCHIVE - deleting on phone moves the cloud copy to _Deleted (preserved)`() { + // syncDecide returns DELETE_REMOTE; the engine's DELETE_REMOTE branch MOVEs the file to + // /_Deleted/ instead of removing it when deleteBehavior == ARCHIVE. + assertEquals(SyncDecision.DELETE_REMOTE, decide(null, remote(), stateReconciled(), DeleteBehavior.ARCHIVE)) + } + + // ── After deletion: a brand-new remote file is NOT pulled down (upload-only) ─ + + @Test fun `KEEP - a new remote file never comes down to the phone (upload-only)`() { + // Remote-only, no state record: in upload-only this must SKIP, not DOWNLOAD. + assertEquals(SyncDecision.SKIP, decide(null, remote(), null, DeleteBehavior.KEEP)) + } + + // ── Re-adding / changing a file after a KEEP deletion still uploads ─────── + + @Test fun `KEEP - modifying the file locally still uploads the change`() { + val newer = local(T0 + 5_000) + assertEquals(SyncDecision.UPLOAD, decide(newer, remote(), stateReconciled(), DeleteBehavior.KEEP)) + } +} diff --git a/app/src/test/kotlin/com/syncflow/ui/addpair/RecommendedDeleteBehaviorTest.kt b/app/src/test/kotlin/com/syncflow/ui/addpair/RecommendedDeleteBehaviorTest.kt new file mode 100644 index 0000000..4fefa70 --- /dev/null +++ b/app/src/test/kotlin/com/syncflow/ui/addpair/RecommendedDeleteBehaviorTest.kt @@ -0,0 +1,23 @@ +package com.syncflow.ui.addpair + +import com.syncflow.domain.model.DeleteBehavior +import com.syncflow.domain.model.SyncDirection +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * The Add-Pair screen's default deletion behaviour must never wipe a backup. One-way directions + * default to KEEP so deleting a file on the phone leaves the cloud copy intact; two-way defaults + * to MIRROR. (The user can still override to any of the three options.) + */ +class RecommendedDeleteBehaviorTest { + + @Test fun `upload-only defaults to KEEP so backups are never deleted`() = + assertEquals(DeleteBehavior.KEEP, recommendedDeleteBehavior(SyncDirection.UPLOAD_ONLY)) + + @Test fun `download-only defaults to KEEP`() = + assertEquals(DeleteBehavior.KEEP, recommendedDeleteBehavior(SyncDirection.DOWNLOAD_ONLY)) + + @Test fun `two-way defaults to MIRROR`() = + assertEquals(DeleteBehavior.MIRROR, recommendedDeleteBehavior(SyncDirection.TWO_WAY)) +}