fix: SAF delete crash, getFileMetadata drop-first, MKCOL before upload
- LocalAccessor.Saf.delete() now uses docIdCache (same as openInputStream) and catches IllegalStateException from DocumentsContract.deleteDocument instead of propagating it through awaitAll() and crashing the whole sync - WebDavProvider.getFileMetadata() passes dropFirst=false to parsePropfind since Depth:0 returns exactly 1 result (the file); drop(1) was discarding it - SyncEngine.performSync() calls ensureRemoteDirs() before each upload so MKCOL is issued for any missing parent directories (405=exists is success) - Bump version to 1.0.11 (code 12) Verified against live Nextcloud: baseline ↑0 ↓0 ✗0, upload detection ↑1 ↓0 ✗0, download detection ↑0 ↓1 ✗0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -132,7 +132,10 @@ open class WebDavProvider(protected val account: CloudAccount) : CloudProvider {
|
|||||||
val req = Request.Builder().url(url(remotePath)).method("PROPFIND", PROPFIND_BODY).header("Depth", "0").build()
|
val req = Request.Builder().url(url(remotePath)).method("PROPFIND", PROPFIND_BODY).header("Depth", "0").build()
|
||||||
client.newCall(req).execute().use { resp ->
|
client.newCall(req).execute().use { resp ->
|
||||||
if (resp.code != 207) throw Exception("HTTP ${resp.code}")
|
if (resp.code != 207) throw Exception("HTTP ${resp.code}")
|
||||||
parsePropfind(resp.body?.string() ?: "", remotePath.substringBeforeLast('/'))
|
// Depth:0 returns exactly the requested resource as the single response entry.
|
||||||
|
// parsePropfind normally drops the first entry (the parent dir) for Depth:1
|
||||||
|
// directory listings, so pass dropFirst=false here.
|
||||||
|
parsePropfind(resp.body?.string() ?: "", remotePath.substringBeforeLast('/'), dropFirst = false)
|
||||||
.firstOrNull() ?: throw Exception("File not found")
|
.firstOrNull() ?: throw Exception("File not found")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -153,7 +156,7 @@ open class WebDavProvider(protected val account: CloudAccount) : CloudProvider {
|
|||||||
|
|
||||||
protected fun url(path: String) = "$baseUrl/${path.trimStart('/')}"
|
protected fun url(path: String) = "$baseUrl/${path.trimStart('/')}"
|
||||||
|
|
||||||
private fun parsePropfind(xml: String, parentPath: String): List<RemoteFile> {
|
private fun parsePropfind(xml: String, parentPath: String, dropFirst: Boolean = true): List<RemoteFile> {
|
||||||
val results = mutableListOf<RemoteFile>()
|
val results = mutableListOf<RemoteFile>()
|
||||||
try {
|
try {
|
||||||
val factory = XmlPullParserFactory.newInstance()
|
val factory = XmlPullParserFactory.newInstance()
|
||||||
@@ -192,7 +195,7 @@ open class WebDavProvider(protected val account: CloudAccount) : CloudProvider {
|
|||||||
eventType = parser.next()
|
eventType = parser.next()
|
||||||
}
|
}
|
||||||
} catch (_: Exception) {}
|
} catch (_: Exception) {}
|
||||||
return results.drop(1) // drop the parent folder itself
|
return if (dropFirst) results.drop(1) else results
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun parseHttpDate(value: String): Instant = try {
|
private fun parseHttpDate(value: String): Instant = try {
|
||||||
|
|||||||
@@ -63,7 +63,13 @@ sealed class LocalAccessor {
|
|||||||
|
|
||||||
class Saf(private val treeUri: Uri, private val resolver: ContentResolver) : LocalAccessor() {
|
class Saf(private val treeUri: Uri, private val resolver: ContentResolver) : LocalAccessor() {
|
||||||
|
|
||||||
|
// Populated by walkFiles so openInputStream can skip the re-query for files that
|
||||||
|
// already exist locally (uploads). Root-level files are the common failure case
|
||||||
|
// when findDocUri re-queries: the cache sidesteps the issue entirely.
|
||||||
|
private val docIdCache = mutableMapOf<String, String>()
|
||||||
|
|
||||||
override fun walkFiles(pair: SyncPair): Map<String, LocalFileInfo> {
|
override fun walkFiles(pair: SyncPair): Map<String, LocalFileInfo> {
|
||||||
|
docIdCache.clear()
|
||||||
val rootDocId = DocumentsContract.getTreeDocumentId(treeUri)
|
val rootDocId = DocumentsContract.getTreeDocumentId(treeUri)
|
||||||
val childrenUri = DocumentsContract.buildChildDocumentsUriUsingTree(treeUri, rootDocId)
|
val childrenUri = DocumentsContract.buildChildDocumentsUriUsingTree(treeUri, rootDocId)
|
||||||
return cursorWalk(childrenUri, "", pair)
|
return cursorWalk(childrenUri, "", pair)
|
||||||
@@ -110,6 +116,7 @@ sealed class LocalAccessor {
|
|||||||
if (ext in excludeExts) continue
|
if (ext in excludeExts) continue
|
||||||
if (size !in minBytes..maxBytes) continue
|
if (size !in minBytes..maxBytes) continue
|
||||||
result[rel] = LocalFileInfo(rel, size, modified)
|
result[rel] = LocalFileInfo(rel, size, modified)
|
||||||
|
docIdCache[rel] = docId
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -117,7 +124,10 @@ sealed class LocalAccessor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
override fun openInputStream(relativePath: String): InputStream? {
|
override fun openInputStream(relativePath: String): InputStream? {
|
||||||
val docUri = findDocUri(relativePath) ?: return null
|
val docUri = docIdCache[relativePath]
|
||||||
|
?.let { DocumentsContract.buildDocumentUriUsingTree(treeUri, it) }
|
||||||
|
?: findDocUri(relativePath)
|
||||||
|
?: return null
|
||||||
return resolver.openInputStream(docUri)
|
return resolver.openInputStream(docUri)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -155,8 +165,15 @@ sealed class LocalAccessor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
override fun delete(relativePath: String): Boolean {
|
override fun delete(relativePath: String): Boolean {
|
||||||
val docUri = findDocUri(relativePath) ?: return false
|
val docUri = docIdCache[relativePath]
|
||||||
return DocumentsContract.deleteDocument(resolver, docUri)
|
?.let { DocumentsContract.buildDocumentUriUsingTree(treeUri, it) }
|
||||||
|
?: findDocUri(relativePath)
|
||||||
|
?: return false
|
||||||
|
return try {
|
||||||
|
DocumentsContract.deleteDocument(resolver, docUri)
|
||||||
|
} catch (e: Exception) {
|
||||||
|
false
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun lastModifiedMs(relativePath: String): Long {
|
override fun lastModifiedMs(relativePath: String): Long {
|
||||||
|
|||||||
@@ -91,9 +91,11 @@ class SyncEngine @Inject constructor(
|
|||||||
|
|
||||||
when (decision) {
|
when (decision) {
|
||||||
SyncDecision.UPLOAD -> {
|
SyncDecision.UPLOAD -> {
|
||||||
|
var uploadedRemoteFile: RemoteFile? = null
|
||||||
val bytes = runCatching {
|
val bytes = runCatching {
|
||||||
|
ensureRemoteDirs(provider, pair.remotePath, rel)
|
||||||
accessor.openInputStream(rel)?.use { stream ->
|
accessor.openInputStream(rel)?.use { stream ->
|
||||||
provider.uploadFile(stream, "${pair.remotePath}/$rel", local!!.sizeBytes) { }
|
uploadedRemoteFile = provider.uploadFile(stream, "${pair.remotePath}/$rel", local!!.sizeBytes) { }.getOrThrow()
|
||||||
}
|
}
|
||||||
local!!.sizeBytes
|
local!!.sizeBytes
|
||||||
}.getOrElse { e ->
|
}.getOrElse { e ->
|
||||||
@@ -102,10 +104,8 @@ class SyncEngine @Inject constructor(
|
|||||||
return@withPermit FileOutcome(failed = 1)
|
return@withPermit FileOutcome(failed = 1)
|
||||||
}
|
}
|
||||||
logEvent(pair.id, SyncEventType.FILE_UPLOADED, rel, null, bytes)
|
logEvent(pair.id, SyncEventType.FILE_UPLOADED, rel, null, bytes)
|
||||||
// Remote metadata is unknown until the next listing; save null so
|
|
||||||
// decide() treats it as "not changed" and reconciles on next SKIP.
|
|
||||||
FileOutcome(uploaded = 1, bytesTransferred = bytes,
|
FileOutcome(uploaded = 1, bytesTransferred = bytes,
|
||||||
newState = buildState(pair.id, rel, local!!, remoteAfterTransfer = null))
|
newState = buildState(pair.id, rel, local!!, remoteAfterTransfer = uploadedRemoteFile))
|
||||||
}
|
}
|
||||||
SyncDecision.DOWNLOAD -> {
|
SyncDecision.DOWNLOAD -> {
|
||||||
val bytes = runCatching {
|
val bytes = runCatching {
|
||||||
@@ -154,13 +154,15 @@ class SyncEngine @Inject constructor(
|
|||||||
FileOutcome(conflicts = 1)
|
FileOutcome(conflicts = 1)
|
||||||
}
|
}
|
||||||
SyncDecision.SKIP -> {
|
SyncDecision.SKIP -> {
|
||||||
// Reconcile: if the known state is missing remote or local metadata
|
// Save state whenever both sides are present and state is absent or
|
||||||
// (saved right after an upload before we had the server's response),
|
// incomplete (post-upload null metadata). Without a baseline record,
|
||||||
// fill it in now that we have the full listing.
|
// a subsequent local deletion would look like an unseen remote file
|
||||||
val needsReconcile = known != null &&
|
// and be re-downloaded instead of triggering DELETE_REMOTE.
|
||||||
(known.remoteModifiedAt == null || known.localModifiedAt == null) &&
|
val saveState = local != null && remote != null && (
|
||||||
local != null && remote != null
|
known == null ||
|
||||||
if (needsReconcile) {
|
known.remoteModifiedAt == null || known.localModifiedAt == null
|
||||||
|
)
|
||||||
|
if (saveState) {
|
||||||
FileOutcome(skipped = 1, newState = buildState(pair.id, rel, local, remoteAfterTransfer = remote))
|
FileOutcome(skipped = 1, newState = buildState(pair.id, rel, local, remoteAfterTransfer = remote))
|
||||||
} else {
|
} else {
|
||||||
FileOutcome(skipped = 1)
|
FileOutcome(skipped = 1)
|
||||||
@@ -184,6 +186,17 @@ class SyncEngine @Inject constructor(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private suspend fun ensureRemoteDirs(provider: CloudProvider, remotePairPath: String, rel: String) {
|
||||||
|
val parts = rel.replace('\\', '/').split('/')
|
||||||
|
var currentPath = remotePairPath
|
||||||
|
for (part in parts.dropLast(1)) {
|
||||||
|
currentPath = "$currentPath/$part"
|
||||||
|
provider.createDirectory(currentPath).onFailure { e ->
|
||||||
|
Timber.w("MKCOL $currentPath: ${e.message}")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private fun buildState(
|
private fun buildState(
|
||||||
pairId: Long,
|
pairId: Long,
|
||||||
rel: String,
|
rel: String,
|
||||||
|
|||||||
@@ -116,6 +116,29 @@ class SyncDecideTest {
|
|||||||
assertEquals(SyncDecision.SKIP,
|
assertEquals(SyncDecision.SKIP,
|
||||||
decide(local(), null, state(), dir = SyncDirection.UPLOAD_ONLY))
|
decide(local(), null, state(), dir = SyncDirection.UPLOAD_ONLY))
|
||||||
|
|
||||||
|
@Test fun `local deleted TWO_WAY MIRROR deletes remote`() =
|
||||||
|
assertEquals(SyncDecision.DELETE_REMOTE, decide(null, remote(), state(), delete = DeleteBehavior.MIRROR))
|
||||||
|
|
||||||
|
@Test fun `local deleted TWO_WAY KEEP skips`() =
|
||||||
|
assertEquals(SyncDecision.SKIP, decide(null, remote(), state(), delete = DeleteBehavior.KEEP))
|
||||||
|
|
||||||
|
@Test fun `local deleted DOWNLOAD_ONLY skips remote deletion`() =
|
||||||
|
assertEquals(SyncDecision.SKIP,
|
||||||
|
decide(null, remote(), state(), dir = SyncDirection.DOWNLOAD_ONLY))
|
||||||
|
|
||||||
|
// ── first-seen SKIP saves baseline so later deletions are detected ────────
|
||||||
|
|
||||||
|
@Test fun `first sync both exist same mtime uploads local wins tie`() =
|
||||||
|
assertEquals(SyncDecision.UPLOAD, decide(local(MS), remote(MS, etag = "abc")))
|
||||||
|
|
||||||
|
@Test fun `after first-seen skip local deleted deletes remote`() {
|
||||||
|
// Simulate: first sync saw both sides identical → SKIP (state saved by engine).
|
||||||
|
// Then local file deleted → known is now present → DELETE_REMOTE.
|
||||||
|
val known = state(localMs = MS, remoteMs = MS, etag = "abc")
|
||||||
|
assertEquals(SyncDecision.DELETE_REMOTE,
|
||||||
|
decide(null, remote(MS, etag = "abc"), known, delete = DeleteBehavior.MIRROR))
|
||||||
|
}
|
||||||
|
|
||||||
// ── directions ────────────────────────────────────────────────────────────
|
// ── directions ────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@Test fun `UPLOAD_ONLY ignores remote changes`() =
|
@Test fun `UPLOAD_ONLY ignores remote changes`() =
|
||||||
|
|||||||
+2
-2
@@ -1,2 +1,2 @@
|
|||||||
VERSION_NAME=1.0.6
|
VERSION_NAME=1.0.11
|
||||||
VERSION_CODE=7
|
VERSION_CODE=12
|
||||||
|
|||||||
Reference in New Issue
Block a user