fix(sync): heal skipped notes' folder & false deleted-flag (no-ETag servers)

inventory69 · inventory69 · commit 1d3a5a13e3ac · 2026-06-06T22:08:35.000+02:00
On WebDAV servers that never return ETags the timestamp fallback skips
unchanged notes forever. Two stale local states could therefore never
correct themselves on the skip path:

1. folderName: the server-path -&gt; folderName mapping was only applied on
   actual downloads, so a stale local folderName (e.g. null on a freshly
   enrolled device) was never corrected — the same note appeared in a
   folder on one device and outside it on another.

2. DELETED_ON_SERVER: during an older folder move a note could drop out
   of the PROPFIND listing for a single sync pass and get flagged
   DELETED_ON_SERVER locally; nothing ever cleared that flag, so the note
   stayed mis-filed even though it still existed on the server.

Replace reconcileFolderIfStale with reconcileSkippedNote(), called in
both skip branches (ETag-match and timestamp-fallback):
- SYNCED: align folderName with the authoritative server path.
- DELETED_ON_SERVER: note is present in the current server listing, so
  the flag is false — restore to SYNCED and adopt the server folder,
  keeping local content (no download).
- PENDING / LOCAL_ONLY / CONFLICT: real local state, left untouched.

updatedAt is never bumped and status never becomes PENDING, so the note
stays invisible to NoteUploader — no re-upload loop. The heal branch only
runs while a server file is being processed, so genuinely deleted notes
(absent from the listing) are never touched. Both skip logs now carry
[localStatus=...] for field diagnosis.

Propagate folderReconciledCount through DownloadResult -&gt; SyncResult
(foldersReconciled flag); MainViewModel reloads notes and refreshes
folders when set, so the UI heals on the next sync with no manual action.
Covered by NoteDownloaderFolderReconcileTest.
diff --git a/android/app/src/main/java/dev/dettmer/simplenotes/sync/NoteDownloader.kt b/android/app/src/main/java/dev/dettmer/simplenotes/sync/NoteDownloader.kt
@@ -28,6 +28,7 @@ internal data class DownloadResult(
     val downloadedCount: Int,
     val conflictCount: Int,
     val deletedOnServerCount: Int = 0,
+    val folderReconciledCount: Int = 0,
     val downloadFailed: Boolean = false,
     val downloadError: String? = null
 )
@@ -88,6 +89,7 @@ internal class NoteDownloader(
         var downloadedCount = 0
         var conflictCount = 0
         var skippedDeleted = 0 // Track skipped deleted notes
+        var folderReconciledCount = 0
         val processedIds = mutableSetOf<String>() // 🆕 v1.2.2: Track already loaded notes
 
         Logger.d(TAG, "📥 downloadAll() called:")
@@ -239,7 +241,8 @@ internal class NoteDownloader(
                     // PRIMARY: E-Tag check — erkennt Inhaltsänderungen zuverlässig
                     if (!forceOverwrite && fileExistsLocally && serverETag != null && serverETag == cachedETag) {
                         skippedUnchanged++
-                        Logger.d(TAG, "   ⏭️ Skipping $noteId: E-Tag match (content unchanged)")
+                        Logger.d(TAG, "   ⏭️ Skipping $noteId: E-Tag match (content unchanged) [localStatus=${localNote?.syncStatus}]")
+                        if (reconcileSkippedNote(localNote, folderByNoteId[noteId])) folderReconciledCount++
                         processedIds.add(noteId)
                         continue
                     }
@@ -253,7 +256,12 @@ internal class NoteDownloader(
                         serverModified <= lastSyncTime
                     if (noETagAndTimestampUnchanged) {
                         skippedUnchanged++
-                        Logger.d(TAG, "   ⏭️ Skipping $noteId: No E-Tag, timestamp unchanged (fallback)")
+                        Logger.d(
+                            TAG,
+                            "   ⏭️ Skipping $noteId: No E-Tag, timestamp unchanged (fallback)" +
+                                " [localStatus=${localNote?.syncStatus}]"
+                        )
+                        if (reconcileSkippedNote(localNote, folderByNoteId[noteId])) folderReconciledCount++
                         processedIds.add(noteId)
                         continue
                     }
@@ -615,6 +623,7 @@ internal class NoteDownloader(
             downloadedCount = downloadedCount,
             conflictCount = conflictCount,
             deletedOnServerCount = deletedOnServerCount,
+            folderReconciledCount = folderReconciledCount,
             downloadFailed = downloadException != null,
             downloadError = downloadException?.message
         )
@@ -810,6 +819,52 @@ internal class NoteDownloader(
         }
     }
 
+    /**
+     * Heilt eine übersprungene, lokal vorhandene Notiz, deren Server-File gerade
+     * verarbeitet wird (→ die Notiz ist nachweislich auf dem Server vorhanden), ohne
+     * Datei-Download:
+     *
+     * - DELETED_ON_SERVER: Die Notiz liegt in der aktuellen Server-Liste, wurde aber
+     *   lokal fälschlich als „gelöscht" markiert (z. B. durch einen v2.7.0-Ordner-Move-
+     *   Race auf E-Tag-losen Servern, der den Eintrag für einen Sync-Durchlauf aus der
+     *   PROPFIND-Liste fallen ließ; nichts setzt dieses Flag je zurück). Falsch-Flag
+     *   löschen: zurück auf SYNCED + autoritativen Server-Ordner übernehmen. Lokaler
+     *   Inhalt bleibt erhalten. Echt gelöschte Notizen sind NICHT in der Liste → ihr
+     *   File wird nie verarbeitet → dieser Zweig läuft für sie nie.
+     * - SYNCED: veraltetes folderName an den Server-Pfad angleichen.
+     * - PENDING / LOCAL_ONLY / CONFLICT: echter lokaler Zustand → unangetastet.
+     *
+     * @return true wenn auf Platte geschrieben wurde (→ UI muss neu laden).
+     */
+    private suspend fun reconcileSkippedNote(localNote: Note?, serverFolder: String?): Boolean {
+        if (localNote == null) return false
+        return when (localNote.syncStatus) {
+            SyncStatus.DELETED_ON_SERVER -> {
+                // updatedAt NICHT bumpen, Status nicht PENDING → kein Re-Upload, keine Schleife
+                storage.saveNote(
+                    localNote.copy(syncStatus = SyncStatus.SYNCED, folderName = serverFolder)
+                )
+                Logger.d(
+                    TAG,
+                    "   🔧 Cleared false DELETED_ON_SERVER for ${localNote.id}: " +
+                        "present in server listing → SYNCED, folder '$serverFolder'"
+                )
+                true
+            }
+            SyncStatus.SYNCED -> {
+                if (localNote.folderName == serverFolder) return false // kein unnötiger Write
+                storage.saveNote(localNote.copy(folderName = serverFolder))
+                Logger.d(
+                    TAG,
+                    "   📁 Reconciled folder for ${localNote.id}: " +
+                        "'${localNote.folderName}' → '$serverFolder'"
+                )
+                true
+            }
+            else -> false // PENDING / LOCAL_ONLY / CONFLICT → lokaler Zustand gewinnt
+        }
+    }
+
     /**
      * Returns true if the user-visible content of [a] and [b] differs.
      * Intentionally excludes [Note.updatedAt] and [Note.syncStatus] — only the
diff --git a/android/app/src/main/java/dev/dettmer/simplenotes/sync/SyncResult.kt b/android/app/src/main/java/dev/dettmer/simplenotes/sync/SyncResult.kt
@@ -13,6 +13,7 @@ data class SyncResult(
     val conflictCount: Int = 0,
     val deletedOnServerCount: Int = 0, // 🆕 v1.8.0
     val foldersChanged: Boolean = false, // 🆕 v2.7.0 (Folders): Ordner-Metadaten haben sich geändert
+    val foldersReconciled: Boolean = false, // 🆕 v2.7.2: folderName einer Notiz an Server-Pfad geheilt
     val errorMessage: String? = null,
     val infoMessage: String? = null // 🆕 v1.9.0 Issue #21: Non-error status info
 ) {
diff --git a/android/app/src/main/java/dev/dettmer/simplenotes/sync/WebDavSyncService.kt b/android/app/src/main/java/dev/dettmer/simplenotes/sync/WebDavSyncService.kt
@@ -671,6 +671,7 @@ class WebDavSyncService(private val context: Context, private val ioDispatcher:
                 Logger.d(TAG, "📍 Step 5: Downloading remote notes")
                 // Download remote notes
                 var deletedOnServerCount = 0 // 🆕 v1.8.0
+                var folderReconciledCount = 0 // 🆕 v2.7.2
                 try {
                     Logger.d(TAG, "⬇️ Downloading remote notes...")
                     val downloadResult = downloadRemoteNotes(
@@ -695,6 +696,7 @@ class WebDavSyncService(private val context: Context, private val ioDispatcher:
                     syncedCount += downloadResult.downloadedCount
                     conflictCount += downloadResult.conflictCount
                     deletedOnServerCount = downloadResult.deletedOnServerCount // 🆕 v1.8.0
+                    folderReconciledCount = downloadResult.folderReconciledCount // 🆕 v2.7.2
                     Logger.d(
                         TAG,
                         "✅ Downloaded: ${downloadResult.downloadedCount} notes, " +
@@ -809,7 +811,8 @@ class WebDavSyncService(private val context: Context, private val ioDispatcher:
                     syncedCount = effectiveSyncedCount,
                     conflictCount = conflictCount,
                     deletedOnServerCount = deletedOnServerCount, // 🆕 v1.8.0
-                    foldersChanged = foldersChanged // 🆕 v2.7.0 (Folders)
+                    foldersChanged = foldersChanged, // 🆕 v2.7.0 (Folders)
+                    foldersReconciled = folderReconciledCount > 0 // 🆕 v2.7.2
                 )
             } catch (e: Exception) {
                 Logger.e(TAG, "═══════════════════════════════════════")
diff --git a/android/app/src/main/java/dev/dettmer/simplenotes/ui/main/MainViewModel.kt b/android/app/src/main/java/dev/dettmer/simplenotes/ui/main/MainViewModel.kt
@@ -1032,8 +1032,10 @@ class MainViewModel(application: Application) : AndroidViewModel(application) {
                 } else if (result.isSuccess) {
                     Logger.d(TAG, "ℹ️ Auto-sync ($source): No changes")
                     SyncStateManager.markCompleted() // Silent → geht direkt auf IDLE
+                    // 🆕 v2.7.2: Ordner-Zuordnung wurde lokal geheilt → Notenliste neu laden
+                    if (result.foldersReconciled) loadNotes(forceReload = true)
                     // 🆕 v2.7.0 (Folders): Farbe leerer Ordner auch ohne Note-Sync ins UI laden
-                    if (result.foldersChanged) refreshFolders()
+                    if (result.foldersChanged || result.foldersReconciled) refreshFolders()
                 } else {
                     Logger.e(TAG, "❌ Auto-sync failed ($source): ${result.errorMessage}")
                     // Fehler werden IMMER angezeigt (auch bei Silent-Sync)
diff --git a/android/app/src/test/java/dev/dettmer/simplenotes/sync/NoteDownloaderFolderReconcileTest.kt b/android/app/src/test/java/dev/dettmer/simplenotes/sync/NoteDownloaderFolderReconcileTest.kt
@@ -0,0 +1,130 @@
+package dev.dettmer.simplenotes.sync
+
+import android.content.Context
+import android.content.SharedPreferences
+import com.thegrizzlylabs.sardineandroid.DavResource
+import com.thegrizzlylabs.sardineandroid.Sardine
+import dev.dettmer.simplenotes.models.Note
+import dev.dettmer.simplenotes.models.SyncStatus
+import dev.dettmer.simplenotes.storage.FolderStore
+import dev.dettmer.simplenotes.storage.NotesStorage
+import dev.dettmer.simplenotes.utils.Constants
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.verify
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.test.runTest
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import java.io.File
+import java.nio.file.Files
+import java.util.Date
+
+class NoteDownloaderFolderReconcileTest {
+    private lateinit var tmpDir: File
+    private lateinit var prefs: SharedPreferences
+    private lateinit var storage: NotesStorage
+    private lateinit var downloader: NoteDownloader
+
+    private val serverUrl = "http://server:8080"
+    private val noteId = "8aeff7d6-4aa0-447a-960d-572206faeaf5"
+    private val lastSync = 2_000_000L
+    private val serverModified = 1_000_000L // <= lastSync → Timestamp-Fallback-Skip
+
+    @Before fun setUp() {
+        tmpDir = Files.createTempDirectory("notedl-test").toFile()
+        prefs = mockk(relaxed = true)
+        every { prefs.getLong("last_sync_timestamp", 0L) } returns lastSync
+        every { prefs.getString(Constants.KEY_SYNC_FOLDER_NAME, any()) } returns "notes"
+        val context = mockk<Context> {
+            every { filesDir } returns tmpDir
+            every { getSharedPreferences(any(), any()) } returns prefs
+        }
+        storage = NotesStorage(context)
+        downloader = NoteDownloader(
+            prefs = prefs,
+            storage = storage,
+            eTagCache = ETagCache(prefs),
+            urlBuilder = SyncUrlBuilder(prefs),
+            connectionManager = mockk(relaxed = true),
+            markdownSyncManager = mockk(relaxed = true),
+            ioDispatcher = Dispatchers.Unconfined,
+            folderStore = FolderStore(context)
+        )
+    }
+
+    @After fun tearDown() { tmpDir.deleteRecursively() }
+
+    private fun folderDir(dirName: String): DavResource = mockk(relaxed = true) {
+        every { isDirectory } returns true
+        every { name } returns dirName
+    }
+
+    private fun noteFile(id: String, etagValue: String? = null): DavResource = mockk(relaxed = true) {
+        every { isDirectory } returns false
+        every { name } returns "$id.json"
+        every { etag } returns etagValue
+        every { modified } returns Date(serverModified)
+    }
+
+    private fun mockSardine(): Sardine = mockk(relaxed = true) {
+        every { list(match { it.endsWith("/notes/") }) } returns listOf(folderDir("Noltenius"))
+        every { list(match { it.contains("Noltenius") }) } returns listOf(noteFile(noteId))
+    }
+
+    @Test fun `stale folderName is healed to server path without download`() = runTest {
+        storage.saveNote(
+            Note(id = noteId, title = "T", content = "C", deviceId = "", syncStatus = SyncStatus.SYNCED, folderName = null)
+        )
+        val sardine = mockSardine()
+
+        val result = downloader.downloadAll(sardine, serverUrl)
+
+        assertEquals(0, result.downloadedCount)
+        verify(exactly = 0) { sardine.get(any<String>()) }
+        assertEquals(1, result.folderReconciledCount)
+        val healed = storage.loadNote(noteId)!!
+        assertEquals("Noltenius", healed.folderName)
+        assertEquals(SyncStatus.SYNCED, healed.syncStatus)
+    }
+
+    @Test fun `pending note is not reconciled`() = runTest {
+        storage.saveNote(
+            Note(id = noteId, title = "T", content = "C", deviceId = "", syncStatus = SyncStatus.PENDING, folderName = null)
+        )
+        val result = downloader.downloadAll(mockSardine(), serverUrl)
+        assertEquals(0, result.folderReconciledCount)
+        assertNull(storage.loadNote(noteId)!!.folderName)
+    }
+
+    @Test fun `already correct folder is a no-op`() = runTest {
+        storage.saveNote(
+            Note(id = noteId, title = "T", content = "C", deviceId = "", syncStatus = SyncStatus.SYNCED, folderName = "Noltenius")
+        )
+        val result = downloader.downloadAll(mockSardine(), serverUrl)
+        assertEquals(0, result.folderReconciledCount)
+        assertEquals("Noltenius", storage.loadNote(noteId)!!.folderName)
+    }
+
+    @Test fun `false DELETED_ON_SERVER is cleared when note is still present on server`() = runTest {
+        storage.saveNote(
+            Note(
+                id = noteId, title = "T", content = "C", deviceId = "",
+                syncStatus = SyncStatus.DELETED_ON_SERVER, folderName = null
+            )
+        )
+        val sardine = mockSardine()
+
+        val result = downloader.downloadAll(sardine, serverUrl)
+
+        assertEquals(0, result.downloadedCount)
+        verify(exactly = 0) { sardine.get(any<String>()) } // kein Download — lokaler Inhalt bleibt
+        assertEquals(1, result.folderReconciledCount)
+        val healed = storage.loadNote(noteId)!!
+        assertEquals(SyncStatus.SYNCED, healed.syncStatus)
+        assertEquals("Noltenius", healed.folderName)
+    }
+}
