Skip to content

Commit 13b0bad

Browse files
alperozturk96alperozturk96
authored
Merge pull request #16174 from nextcloud/backport/16146/stable-3.35
[stable-3.35] fix: upsert single file
2 parents 3eb6cc0 + a6916c5 commit 13b0bad

File tree

2 files changed

+204
-28
lines changed

2 files changed

+204
-28
lines changed

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

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2025 Alper Ozturk <[email protected]>
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
package com.owncloud.android.providers
9+
10+
import android.content.ContentValues
11+
import android.content.ContentUris
12+
import android.net.Uri
13+
import androidx.sqlite.db.SimpleSQLiteQuery
14+
import androidx.sqlite.db.SupportSQLiteDatabase
15+
import com.owncloud.android.db.ProviderMeta
16+
import io.mockk.Runs
17+
import io.mockk.every
18+
import io.mockk.just
19+
import io.mockk.mockk
20+
import kotlinx.coroutines.async
21+
import kotlinx.coroutines.awaitAll
22+
import kotlinx.coroutines.coroutineScope
23+
import kotlinx.coroutines.runBlocking
24+
import org.junit.Assert.assertEquals
25+
import org.junit.Assert.assertTrue
26+
import org.junit.Before
27+
import org.junit.Test
28+
29+
@Suppress("MagicNumber")
30+
class FileContentProviderTests {
31+
32+
private lateinit var provider: FileContentProvider
33+
private lateinit var db: SupportSQLiteDatabase
34+
35+
@Before
36+
fun setup() {
37+
provider = FileContentProvider()
38+
db = mockk()
39+
}
40+
41+
@Test
42+
fun insertNewFileShouldReturnNewId() {
43+
val values = ContentValues().apply {
44+
put(ProviderMeta.ProviderTableMeta.FILE_PATH, "/path/to/file.txt")
45+
put(ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER, "[email protected]")
46+
}
47+
48+
// Mock insert to return new ID
49+
every { db.insert(any(), any(), any()) } returns 42L
50+
51+
val result: Uri = provider.upsertSingleFile(
52+
db,
53+
ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE,
54+
values
55+
)
56+
57+
assertEquals(
58+
ContentUris.withAppendedId(ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, 42),
59+
result
60+
)
61+
}
62+
63+
@Test
64+
fun updateExistingFileShouldReturnSameId() {
65+
val values = ContentValues().apply {
66+
put(ProviderMeta.ProviderTableMeta.FILE_PATH, "/path/to/file.txt")
67+
put(ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER, "[email protected]")
68+
}
69+
70+
// Simulate insert conflict
71+
every { db.insert(ProviderMeta.ProviderTableMeta.FILE_TABLE_NAME, any(), values) } returns -1L
72+
73+
// Simulate update returning 1 row affected
74+
every {
75+
db.update(
76+
ProviderMeta.ProviderTableMeta.FILE_TABLE_NAME,
77+
any(),
78+
values,
79+
any(),
80+
any()
81+
)
82+
} returns 1
83+
84+
// Mock cursor to return ID 99
85+
val cursor = mockk<android.database.Cursor>()
86+
every { cursor.moveToFirst() } returns true
87+
every { cursor.getLong(0) } returns 99L
88+
every { cursor.close() } just Runs
89+
90+
every { db.query(any<SimpleSQLiteQuery>()) } returns cursor
91+
92+
val result: Uri = provider.upsertSingleFile(
93+
db,
94+
ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE,
95+
values
96+
)
97+
98+
assertEquals(ContentUris.withAppendedId(ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, 99), result)
99+
100+
cursor.close()
101+
}
102+
103+
@Test
104+
fun testConcurrentUpserts() = runBlocking {
105+
val values = ContentValues().apply {
106+
put(ProviderMeta.ProviderTableMeta.FILE_PATH, "/path/to/file.txt")
107+
put(ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER, "[email protected]")
108+
}
109+
110+
// shared state to simulate race
111+
val inserted = mutableListOf<Long>()
112+
113+
// mock insert: fail first call, succeed second
114+
every { db.insert(any(), any(), any()) } answers {
115+
synchronized(inserted) {
116+
if (inserted.isEmpty()) {
117+
// first thread "fails" insert which means already existing file id will be returned
118+
inserted.add(-1L)
119+
-1L
120+
} else {
121+
// second thread "succeeds" it will update existing one
122+
inserted.add(42L)
123+
42L
124+
}
125+
}
126+
}
127+
128+
// mock update only one row should be affected
129+
every { db.update(any(), any(), any(), any(), any()) } returns 1
130+
131+
// mock query existing file id will return 99
132+
val cursor = mockk<android.database.Cursor>()
133+
every { cursor.moveToFirst() } returns true
134+
every { cursor.getLong(0) } returns 99L
135+
every { cursor.close() } just Runs
136+
every { db.query(any<SimpleSQLiteQuery>()) } returns cursor
137+
138+
// launch two coroutines simulating concurrent threads
139+
val results = mutableListOf<Uri>()
140+
coroutineScope {
141+
val job1 =
142+
async {
143+
results.add(provider.upsertSingleFile(db, ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, values))
144+
}
145+
val job2 =
146+
async {
147+
results.add(provider.upsertSingleFile(db, ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, values))
148+
}
149+
awaitAll(job1, job2)
150+
}
151+
152+
// both URIs should be correct (one updated, one inserted)
153+
assertTrue(results.contains(ContentUris.withAppendedId(ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, 99)))
154+
assertTrue(results.contains(ContentUris.withAppendedId(ProviderMeta.ProviderTableMeta.CONTENT_URI_FILE, 42)))
155+
156+
cursor.close()
157+
}
158+
}

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

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import androidx.annotation.NonNull;
4343
import androidx.annotation.Nullable;
4444
import androidx.annotation.VisibleForTesting;
45+
import androidx.sqlite.db.SimpleSQLiteQuery;
4546
import androidx.sqlite.db.SupportSQLiteDatabase;
4647
import androidx.sqlite.db.SupportSQLiteOpenHelper;
4748
import androidx.sqlite.db.SupportSQLiteQuery;
@@ -234,34 +235,7 @@ private Uri insert(SupportSQLiteDatabase db, Uri uri, ContentValues values) {
234235
switch (mUriMatcher.match(uri)) {
235236
case ROOT_DIRECTORY:
236237
case SINGLE_FILE:
237-
String where = ProviderTableMeta.FILE_PATH + "=? AND " + ProviderTableMeta.FILE_ACCOUNT_OWNER + "=?";
238-
239-
String remotePath = values.getAsString(ProviderTableMeta.FILE_PATH);
240-
String accountName = values.getAsString(ProviderTableMeta.FILE_ACCOUNT_OWNER);
241-
String[] whereArgs = {remotePath, accountName};
242-
243-
Cursor doubleCheck = query(db, uri, PROJECTION_FILE_PATH_AND_OWNER, where, whereArgs, null);
244-
// ugly patch; serious refactoring is needed to reduce work in
245-
// FileDataStorageManager and bring it to FileContentProvider
246-
if (!doubleCheck.moveToFirst()) {
247-
doubleCheck.close();
248-
long rowId = db.insert(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_REPLACE, values);
249-
if (rowId > 0) {
250-
return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, rowId);
251-
} else {
252-
throw new SQLException(ERROR + uri);
253-
}
254-
} else {
255-
// file is already inserted; race condition, let's avoid a duplicated entry
256-
Uri insertedFileUri = ContentUris.withAppendedId(
257-
ProviderTableMeta.CONTENT_URI_FILE,
258-
doubleCheck.getLong(doubleCheck.getColumnIndexOrThrow(ProviderTableMeta._ID))
259-
);
260-
doubleCheck.close();
261-
262-
return insertedFileUri;
263-
}
264-
238+
return upsertSingleFile(db, uri, values);
265239
case SHARES:
266240
Uri insertedShareUri;
267241
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) {
343317
}
344318
}
345319

320+
public Uri upsertSingleFile(SupportSQLiteDatabase db, Uri uri, ContentValues values) {
321+
String filePath = values.getAsString(ProviderTableMeta.FILE_PATH);
322+
String accountOwner = values.getAsString(ProviderTableMeta.FILE_ACCOUNT_OWNER);
323+
324+
String where = ProviderTableMeta.FILE_PATH + "=? AND " + ProviderTableMeta.FILE_ACCOUNT_OWNER + "=?";
325+
String[] whereArgs = {filePath, accountOwner};
326+
327+
// Try insert first, ignore conflict
328+
long rowId = db.insert(
329+
ProviderTableMeta.FILE_TABLE_NAME,
330+
SQLiteDatabase.CONFLICT_IGNORE,
331+
values);
332+
333+
if (rowId <= 0) {
334+
// Already exists: update
335+
int count = db.update(
336+
ProviderTableMeta.FILE_TABLE_NAME,
337+
SQLiteDatabase.CONFLICT_NONE,
338+
values,
339+
where,
340+
whereArgs);
341+
342+
if (count == 0) {
343+
throw new SQLException("Failed to update existing file: " + uri);
344+
}
345+
346+
try (Cursor cursor = db.query(
347+
new SimpleSQLiteQuery(
348+
"SELECT " + ProviderTableMeta._ID +
349+
" FROM " + ProviderTableMeta.FILE_TABLE_NAME +
350+
" WHERE " + where,
351+
whereArgs
352+
))) {
353+
if (cursor.moveToFirst()) {
354+
rowId = cursor.getLong(0);
355+
} else {
356+
throw new SQLException("Failed to fetch ID after update: " + uri);
357+
}
358+
}
359+
}
360+
361+
return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, rowId);
362+
}
363+
346364
private void updateFilesTableAccordingToShareInsertion(SupportSQLiteDatabase db, ContentValues newShare) {
347365
ContentValues fileValues = new ContentValues();
348366
Integer shareTypeValue = newShare.getAsInteger(ProviderTableMeta.OCSHARES_SHARE_TYPE);

0 commit comments

Comments
 (0)