[stable-3.35] fix: upsert single file

app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt

@@ -0,0 +1,158 @@
+/*
+ * Nextcloud - Android Client
+ *
+ * SPDX-FileCopyrightText: 2025 Alper Ozturk <[email protected]>
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+package com.owncloud.android.providers
+
+import android.content.ContentValues
+import android.content.ContentUris
+import android.net.Uri
+import androidx.sqlite.db.SimpleSQLiteQuery
+import androidx.sqlite.db.SupportSQLiteDatabase
+import com.owncloud.android.db.ProviderMeta
+import io.mockk.Runs
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
+import kotlinx.coroutines.coroutineScope
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+
+@Suppress("MagicNumber")
+class FileContentProviderTests {
+
+    private lateinit var provider: FileContentProvider
+    private lateinit var db: SupportSQLiteDatabase
+
+    @Before
+    fun setup() {
+        provider = FileContentProvider()
+        db = mockk()
+    }
+
+    @Test
+    fun insertNewFileShouldReturnNewId() {
+        val values = ContentValues().apply {
+            put(ProviderMeta.ProviderTableMeta.FILE_PATH, "/path/to/file.txt")
+            put(ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER, "[email protected]")
+        }
+
+        // Mock insert to return new ID
+        every { db.insert(any(), any(), any()) } returns 42L
+
+        val result: Uri = provider.upsertSingleFile(
+            db,
+            ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE,
+            values
+        )
+
+        assertEquals(
+            ContentUris.withAppendedId(ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, 42),
+            result
+        )
+    }
+
+    @Test
+    fun updateExistingFileShouldReturnSameId() {
+        val values = ContentValues().apply {
+            put(ProviderMeta.ProviderTableMeta.FILE_PATH, "/path/to/file.txt")
+            put(ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER, "[email protected]")
+        }
+
+        // Simulate insert conflict
+        every { db.insert(ProviderMeta.ProviderTableMeta.FILE_TABLE_NAME, any(), values) } returns -1L
+
+        // Simulate update returning 1 row affected
+        every {
+            db.update(
+                ProviderMeta.ProviderTableMeta.FILE_TABLE_NAME,
+                any(),
+                values,
+                any(),
+                any()
+            )
+        } returns 1
+
+        // Mock cursor to return ID 99
+        val cursor = mockk<android.database.Cursor>()
+        every { cursor.moveToFirst() } returns true
+        every { cursor.getLong(0) } returns 99L
+        every { cursor.close() } just Runs
+
+        every { db.query(any<SimpleSQLiteQuery>()) } returns cursor
+
+        val result: Uri = provider.upsertSingleFile(
+            db,
+            ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE,
+            values
+        )
+
+        assertEquals(ContentUris.withAppendedId(ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, 99), result)
+
+        cursor.close()
+    }
+
+    @Test
+    fun testConcurrentUpserts() = runBlocking {
+        val values = ContentValues().apply {
+            put(ProviderMeta.ProviderTableMeta.FILE_PATH, "/path/to/file.txt")
+            put(ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER, "[email protected]")
+        }
+
+        // shared state to simulate race
+        val inserted = mutableListOf<Long>()
+
+        // mock insert: fail first call, succeed second
+        every { db.insert(any(), any(), any()) } answers {
+            synchronized(inserted) {
+                if (inserted.isEmpty()) {
+                    // first thread "fails" insert which means already existing file id will be returned
+                    inserted.add(-1L)
+                    -1L
+                } else {
+                    // second thread "succeeds" it will update existing one
+                    inserted.add(42L)
+                    42L
+                }
+            }
+        }
+
+        // mock update only one row should be affected
+        every { db.update(any(), any(), any(), any(), any()) } returns 1
+
+        // mock query existing file id will return 99
+        val cursor = mockk<android.database.Cursor>()
+        every { cursor.moveToFirst() } returns true
+        every { cursor.getLong(0) } returns 99L
+        every { cursor.close() } just Runs
+        every { db.query(any<SimpleSQLiteQuery>()) } returns cursor
+
+        // launch two coroutines simulating concurrent threads
+        val results = mutableListOf<Uri>()
+        coroutineScope {
+            val job1 =
+                async {
+                    results.add(provider.upsertSingleFile(db, ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, values))
+                }
+            val job2 =
+                async {
+                    results.add(provider.upsertSingleFile(db, ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, values))
+                }
+            awaitAll(job1, job2)
+        }
+
+        // both URIs should be correct (one updated, one inserted)
+        assertTrue(results.contains(ContentUris.withAppendedId(ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, 99)))
+        assertTrue(results.contains(ContentUris.withAppendedId(ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, 42)))
+
+        cursor.close()
+    }
+}

app/src/main/java/com/owncloud/android/providers/FileContentProvider.java

@@ -42,6 +42,7 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
+import androidx.sqlite.db.SimpleSQLiteQuery;
 import androidx.sqlite.db.SupportSQLiteDatabase;
 import androidx.sqlite.db.SupportSQLiteOpenHelper;
 import androidx.sqlite.db.SupportSQLiteQuery;
@@ -234,34 +235,7 @@ private Uri insert(SupportSQLiteDatabase db, Uri uri, ContentValues values) {
         switch (mUriMatcher.match(uri)) {
             case ROOT_DIRECTORY:
             case SINGLE_FILE:
-                String where = ProviderTableMeta.FILE_PATH + "=? AND " + ProviderTableMeta.FILE_ACCOUNT_OWNER + "=?";
-
-                String remotePath = values.getAsString(ProviderTableMeta.FILE_PATH);
-                String accountName = values.getAsString(ProviderTableMeta.FILE_ACCOUNT_OWNER);
-                String[] whereArgs = {remotePath, accountName};
-
-                Cursor doubleCheck = query(db, uri, PROJECTION_FILE_PATH_AND_OWNER, where, whereArgs, null);
-                // ugly patch; serious refactoring is needed to reduce work in
-                // FileDataStorageManager and bring it to FileContentProvider
-                if (!doubleCheck.moveToFirst()) {
-                    doubleCheck.close();
-                    long rowId = db.insert(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_REPLACE, values);
-                    if (rowId > 0) {
-                        return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, rowId);
-                    } else {
-                        throw new SQLException(ERROR + uri);
-                    }
-                } else {
-                    // file is already inserted; race condition, let's avoid a duplicated entry
-                    Uri insertedFileUri = ContentUris.withAppendedId(
-                        ProviderTableMeta.CONTENT_URI_FILE,
-                        doubleCheck.getLong(doubleCheck.getColumnIndexOrThrow(ProviderTableMeta._ID))
-                    );
-                    doubleCheck.close();
-
-                    return insertedFileUri;
-                }
-
+                return upsertSingleFile(db, uri, values);
             case SHARES:
                 Uri insertedShareUri;
                 long idShares = db.insert(ProviderTableMeta.OCSHARES_TABLE_NAME, SQLiteDatabase.CONFLICT_REPLACE, values);
@@ -343,6 +317,50 @@ private Uri insert(SupportSQLiteDatabase db, Uri uri, ContentValues values) {
         }
     }
 
+    public Uri upsertSingleFile(SupportSQLiteDatabase db, Uri uri, ContentValues values) {
+        String filePath = values.getAsString(ProviderTableMeta.FILE_PATH);
+        String accountOwner = values.getAsString(ProviderTableMeta.FILE_ACCOUNT_OWNER);
+
+        String where = ProviderTableMeta.FILE_PATH + "=? AND " + ProviderTableMeta.FILE_ACCOUNT_OWNER + "=?";
+        String[] whereArgs = {filePath, accountOwner};
+
+        // Try insert first, ignore conflict
+        long rowId = db.insert(
+            ProviderTableMeta.FILE_TABLE_NAME,
+            SQLiteDatabase.CONFLICT_IGNORE,
+            values);
+
+        if (rowId <= 0) {
+            // Already exists: update
+            int count = db.update(
+                ProviderTableMeta.FILE_TABLE_NAME,
+                SQLiteDatabase.CONFLICT_NONE,
+                values,
+                where,
+                whereArgs);
+
+            if (count == 0) {
+                throw new SQLException("Failed to update existing file: " + uri);
+            }
+
+            try (Cursor cursor = db.query(
+                new SimpleSQLiteQuery(
+                    "SELECT " + ProviderTableMeta._ID +
+                        " FROM " + ProviderTableMeta.FILE_TABLE_NAME +
+                        " WHERE " + where,
+                    whereArgs
+                ))) {
+                if (cursor.moveToFirst()) {
+                    rowId = cursor.getLong(0);
+                } else {
+                    throw new SQLException("Failed to fetch ID after update: " + uri);
+                }
+            }
+        }
+
+        return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, rowId);
+    }
+
     private void updateFilesTableAccordingToShareInsertion(SupportSQLiteDatabase db, ContentValues newShare) {
         ContentValues fileValues = new ContentValues();
         Integer shareTypeValue = newShare.getAsInteger(ProviderTableMeta.OCSHARES_SHARE_TYPE);