fix: prevent and clean up duplicate file entries from concurrent folder refreshes

app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java

@@ -112,6 +112,13 @@ public class FileDataStorageManager {
     private static final String JSON_EMPTY_ARRAY = "[]";
     private static final long INTERNAL_TWO_WAY_SYNC_NEVER_SYNCED = 0L;
 
+    /**
+     * Serializes the "update if the row exists, insert otherwise" sequences of {@link #saveFile(OCFile)} and
+     * {@link #saveFolder(OCFile, List, Collection)}. The file table has no unique constraint on the remote path, so
+     * two concurrent writers (e.g. two refreshes of the same folder) would otherwise both insert the same new file.
+     */
+    private static final Object FILE_TABLE_WRITE_LOCK = new Object();
+
     private final ContentResolver contentResolver;
     private final ContentProviderClient contentProviderClient;
     private final User user;
@@ -572,6 +579,12 @@ public List<OCFile> getFolderImagesAndVideos(OCFile folder, boolean onlyOnDevice
     }
 
     public boolean saveFile(OCFile ocFile) {
+        synchronized (FILE_TABLE_WRITE_LOCK) {
+            return saveFileLocked(ocFile);
+        }
+    }
+
+    private boolean saveFileLocked(OCFile ocFile) {
         Log_OC.d(TAG, "saving file " + ocFile.getFileName() + " into " + ocFile.getRemotePath());
 
         boolean overridden = false;
@@ -754,6 +767,12 @@ public void saveNewFile(OCFile newFile) {
      * @param filesToRemove
      */
     public void saveFolder(OCFile folder, List<OCFile> updatedFiles, Collection<OCFile> filesToRemove) {
+        synchronized (FILE_TABLE_WRITE_LOCK) {
+            saveFolderLocked(folder, updatedFiles, filesToRemove);
+        }
+    }
+
+    private void saveFolderLocked(OCFile folder, List<OCFile> updatedFiles, Collection<OCFile> filesToRemove) {
         Log_OC.d(TAG, "Saving folder " + folder.getRemotePath() + " with " + updatedFiles.size()
             + " children and " + filesToRemove.size() + " files to remove");
 
@@ -770,6 +789,7 @@ public void saveFolder(OCFile folder, List<OCFile> updatedFiles, Collection<OCFi
                     fileId = ocFile.getFileId();
                 } else {
                     fileId = getFileByPath(ocFile.getRemotePath()).getFileId();
+                    ocFile.setFileId(fileId);
                 }
                 // updating an existing file
                 operations.add(ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI)
@@ -1065,6 +1085,17 @@ private boolean removeFolderInDb(OCFile folder) {
         return fileDao.deleteFolderWithDescendants(user.getAccountName(), folder.getFileId()) > 0;
     }
 
+    /**
+     * Removes duplicate database entries, with the entries of their contents for folders. Local files are not
+     * touched: they belong to the entry that is kept for the same remote path.
+     */
+    public void removeDuplicateEntries(Collection<OCFile> duplicates) {
+        for (OCFile duplicate : duplicates) {
+            Log_OC.w(TAG, "Removing duplicate entry " + duplicate.getFileId() + " of " + duplicate.getRemotePath());
+            fileDao.deleteFolderWithDescendants(user.getAccountName(), duplicate.getFileId());
+        }
+    }
+
     private boolean removeLocalFolder(OCFile folder) {
         if (folder == null) {
             Log_OC.d(TAG, "removeLocalFolder: folder is null");

app/src/main/java/com/owncloud/android/operations/RefreshFolderOperation.java

@@ -58,6 +58,7 @@
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 import androidx.localbroadcastmanager.content.LocalBroadcastManager;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
@@ -557,16 +558,24 @@ private void synchronizeData(List<Object> folderAndFiles) {
 
         // get current data about local contents of the folder to synchronize
         Map<String, OCFile> localFilesMap;
+        List<OCFile> duplicateLocalFiles = new ArrayList<>();
         E2EVersion e2EVersion = fileDataStorageManager.getE2EEVersionObject(user);
         if (object instanceof DecryptedFolderMetadataFileV1 metadataFileV1) {
-            localFilesMap = prefillLocalFilesMap(metadataFileV1, fileDataStorageManager.getFolderContent(mLocalFolder, false));
+            localFilesMap = prefillLocalFilesMap(metadataFileV1,
+                                                 fileDataStorageManager.getFolderContent(mLocalFolder, false),
+                                                 duplicateLocalFiles);
         } else {
-            localFilesMap = prefillLocalFilesMap(object, fileDataStorageManager.getFolderContent(mLocalFolder, false));
+            localFilesMap = prefillLocalFilesMap(object,
+                                                 fileDataStorageManager.getFolderContent(mLocalFolder, false),
+                                                 duplicateLocalFiles);
             if (object instanceof DecryptedFolderMetadataFile metadataFile) {
                 mLocalFolder.setE2eCounter(metadataFile.getMetadata().getCounter());
             }
         }
 
+        // duplicate entries are never matched with a remote file, so they would stay forever
+        fileDataStorageManager.removeDuplicateEntries(duplicateLocalFiles);
+
         // loop to update every child
         OCFile remoteFile;
         OCFile localFile;
@@ -792,6 +801,19 @@ private void setLocalFileDataOnUpdatedFile(OCFile remoteFile, OCFile localFile,
     @NonNull
     @SuppressFBWarnings("OCP")
     public static Map<String, OCFile> prefillLocalFilesMap(Object metadata, List<OCFile> localFiles) {
+        return prefillLocalFilesMap(metadata, localFiles, new ArrayList<>());
+    }
+
+    /**
+     * Maps the remote path of every local file to the file. Local files with the same remote path as another one are
+     * duplicate database entries: only the one chosen by {@link #preferredLocalFile(OCFile, OCFile)} is mapped, the
+     * others are added to {@code duplicates}.
+     */
+    @NonNull
+    @SuppressFBWarnings("OCP")
+    public static Map<String, OCFile> prefillLocalFilesMap(Object metadata,
+                                                           List<OCFile> localFiles,
+                                                           List<OCFile> duplicates) {
         Map<String, OCFile> localFilesMap = Maps.newHashMapWithExpectedSize(localFiles.size());
 
         for (OCFile file : localFiles) {
@@ -803,11 +825,32 @@ public static Map<String, OCFile> prefillLocalFilesMap(Object metadata, List<OCF
                     remotePath = remotePath + PATH_SEPARATOR;
                 }
             }
-            localFilesMap.put(remotePath, file);
+
+            OCFile mappedFile = localFilesMap.get(remotePath);
+            if (mappedFile != null && mappedFile.getRemotePath().equals(file.getRemotePath())) {
+                OCFile keptFile = preferredLocalFile(mappedFile, file);
+                localFilesMap.put(remotePath, keptFile);
+                duplicates.add(keptFile == mappedFile ? file : mappedFile);
+            } else {
+                localFilesMap.put(remotePath, file);
+            }
         }
         return localFilesMap;
     }
 
+    /**
+     * Chooses which of two database entries of the same remote file to keep: the one whose local copy exists on the
+     * device, else the oldest one (lowest id).
+     */
+    @VisibleForTesting
+    static OCFile preferredLocalFile(OCFile first, OCFile second) {
+        boolean firstExistsOnDevice = first.existsOnDevice();
+        if (firstExistsOnDevice != second.existsOnDevice()) {
+            return firstExistsOnDevice ? first : second;
+        }
+        return first.getFileId() <= second.getFileId() ? first : second;
+    }
+
     /**
      * Performs a list of synchronization operations, determining if a download or upload is needed or if exists
      * conflict due to changes both in local and remote contents of the each file.

app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java

@@ -310,7 +310,13 @@ private void synchronizeData(List<Object> folderAndFiles) throws OperationCancel
         }
 
         // get current data about local contents of the folder to synchronize
-        Map<String, OCFile> localFilesMap = RefreshFolderOperation.prefillLocalFilesMap(object,storageManager.getFolderContent(mLocalFolder, false));
+        List<OCFile> duplicateLocalFiles = new ArrayList<>();
+        Map<String, OCFile> localFilesMap = RefreshFolderOperation.prefillLocalFilesMap(object,
+                                                                                        storageManager.getFolderContent(mLocalFolder, false),
+                                                                                        duplicateLocalFiles);
+
+        // duplicate entries are never matched with a remote file, so they would stay forever
+        storageManager.removeDuplicateEntries(duplicateLocalFiles);
 
         // loop to synchronize every child
         List<OCFile> updatedFiles = new ArrayList<>(folderAndFiles.size() - 1);

app/src/test/java/com/owncloud/android/operations/PrefillLocalFilesMapTest.kt

@@ -0,0 +1,90 @@
+/*
+ * Nextcloud - Android Client
+ *
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+package com.owncloud.android.operations
+
+import com.owncloud.android.datamodel.OCFile
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertSame
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.mockito.kotlin.doReturn
+import org.mockito.kotlin.mock
+
+class PrefillLocalFilesMapTest {
+
+    private fun localFile(id: Long, path: String, onDevice: Boolean = false): OCFile = mock {
+        on { fileId } doReturn id
+        on { remotePath } doReturn path
+        on { existsOnDevice() } doReturn onDevice
+    }
+
+    @Test
+    fun withoutDuplicates_mapsEveryFile() {
+        val photos = localFile(1, "/Photos/")
+        val notes = localFile(2, "/Notes/")
+        val duplicates = mutableListOf<OCFile>()
+
+        val map = RefreshFolderOperation.prefillLocalFilesMap(null, listOf(photos, notes), duplicates)
+
+        assertEquals(mapOf("/Photos/" to photos, "/Notes/" to notes), map)
+        assertTrue(duplicates.isEmpty())
+    }
+
+    @Test
+    fun withDuplicates_keepsLowestIdAndReportsTheOthers() {
+        val newer = localFile(7, "/Photos/")
+        val oldest = localFile(3, "/Photos/")
+        val middle = localFile(5, "/Photos/")
+        val duplicates = mutableListOf<OCFile>()
+
+        val map = RefreshFolderOperation.prefillLocalFilesMap(null, listOf(newer, oldest, middle), duplicates)
+
+        assertSame(oldest, map["/Photos/"])
+        assertEquals(setOf(newer, middle), duplicates.toSet())
+        assertEquals(2, duplicates.size)
+    }
+
+    @Test
+    fun withDuplicates_keepsEntryWithLocalCopy() {
+        val oldest = localFile(3, "/Documents/report.pdf")
+        val downloaded = localFile(9, "/Documents/report.pdf", onDevice = true)
+        val duplicates = mutableListOf<OCFile>()
+
+        val map = RefreshFolderOperation.prefillLocalFilesMap(null, listOf(oldest, downloaded), duplicates)
+
+        assertSame(downloaded, map["/Documents/report.pdf"])
+        assertEquals(listOf(oldest), duplicates)
+    }
+
+    @Test
+    fun encryptedFolder_sameKeyButDifferentRemotePath_isNoDuplicate() {
+        val first: OCFile = mock {
+            on { remotePath } doReturn "/secret/aaa"
+            on { parentRemotePath } doReturn "/secret/"
+        }
+        val second: OCFile = mock {
+            on { remotePath } doReturn "/secret/bbb"
+            on { parentRemotePath } doReturn "/secret/"
+        }
+        val duplicates = mutableListOf<OCFile>()
+
+        RefreshFolderOperation.prefillLocalFilesMap(Any(), listOf(first, second), duplicates)
+
+        assertTrue(duplicates.isEmpty())
+    }
+
+    @Test
+    fun withoutDuplicatesList_mapsTheSameEntry() {
+        val newer = localFile(7, "/zotero/")
+        val oldest = localFile(3, "/zotero/")
+
+        val map = RefreshFolderOperation.prefillLocalFilesMap(null, listOf(newer, oldest))
+
+        assertSame(oldest, map["/zotero/"])
+    }
+}