From a9d7451ae47086c423359233757ceb990b34e1fb Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 3 Nov 2026 13:04:59 +0300 Subject: [PATCH 1/4] fix upsert Signed-off-by: alperozturk96 --- ...FileDataStorageManagerContentResolverIT.kt | 1 + .../providers/FileContentProviderTests.kt | 69 +--- .../FileDataStorageManagerExtensions.kt | 128 ++++++- .../datamodel/FileDataStorageManager.java | 138 +------- .../operations/RefreshFolderOperation.java | 6 +- .../SynchronizeFolderOperation.java | 3 +- .../providers/FileContentProvider.java | 43 +-- .../utils/extensions/FileUpsertTest.kt | 327 ++++++++++++++++++ 8 files changed, 484 insertions(+), 231 deletions(-) create mode 100644 app/src/test/java/com/nextcloud/utils/extensions/FileUpsertTest.kt diff --git a/app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt b/app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt index 6c800da513e1..0499fc821fbf 100644 --- a/app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt +++ b/app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt @@ -7,6 +7,7 @@ */ package com.owncloud.android.datamodel +import com.nextcloud.utils.extensions.saveFolder import org.junit.Assert import org.junit.Test diff --git a/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt b/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt index 8027feb9bd08..52ac4b45ca54 100644 --- a/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt +++ b/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt @@ -17,12 +17,7 @@ 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 @@ -45,6 +40,11 @@ class FileContentProviderTests { put(ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER, "user@example.com") } + val cursor = mockk() + every { cursor.moveToFirst() } returns false + every { cursor.close() } just Runs + every { db.query(any()) } returns cursor + // Mock insert to return new ID every { db.insert(any(), any(), any()) } returns 42L @@ -67,9 +67,6 @@ class FileContentProviderTests { put(ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER, "user@example.com") } - // Simulate insert conflict - every { db.insert(ProviderMeta.ProviderTableMeta.FILE_TABLE_NAME, any(), values) } returns -1L - // Simulate update returning 1 row affected every { db.update( @@ -99,60 +96,4 @@ class FileContentProviderTests { 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, "user@example.com") - } - - // shared state to simulate race - val inserted = mutableListOf() - - // 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() - every { cursor.moveToFirst() } returns true - every { cursor.getLong(0) } returns 99L - every { cursor.close() } just Runs - every { db.query(any()) } returns cursor - - // launch two coroutines simulating concurrent threads - val results = mutableListOf() - 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() - } } diff --git a/app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt b/app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt index 4b374782de3f..26a260b34368 100644 --- a/app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt +++ b/app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt @@ -7,18 +7,23 @@ package com.nextcloud.utils.extensions +import android.content.ContentProviderOperation +import android.content.ContentUris +import android.content.OperationApplicationException +import android.database.sqlite.SQLiteDatabase +import android.os.RemoteException import com.nextcloud.client.database.dao.FileDao import com.nextcloud.client.database.entity.model.ShareeKey -import com.nextcloud.client.database.entity.toOCCapability import com.nextcloud.model.HTTPStatusCodes +import com.owncloud.android.MainApp import com.owncloud.android.datamodel.FileDataStorageManager import com.owncloud.android.datamodel.OCFile +import com.owncloud.android.db.ProviderMeta.ProviderTableMeta import com.owncloud.android.lib.common.OwnCloudClient import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.lib.resources.files.ExistenceCheckRemoteOperation import com.owncloud.android.lib.resources.files.model.RemoteFile import com.owncloud.android.lib.resources.shares.OCShare -import com.owncloud.android.lib.resources.status.OCCapability import com.owncloud.android.operations.upload.RemoteFileExistence import com.owncloud.android.utils.FileStorageUtils import com.owncloud.android.utils.MimeType @@ -125,9 +130,6 @@ fun FileDataStorageManager.getNonEncryptedSubfolders(id: Long, accountName: Stri createFileInstance(it) } -suspend fun FileDataStorageManager.getCapabilitiesByAccountName(accountName: String): OCCapability = - capabilityDao.getByAccountName(accountName).toOCCapability() - fun FileDataStorageManager.moveFiles(ocFile: OCFile?, targetPath: String, targetParentPath: String) { Log_OC.d( FileDataStorageManager.TAG, @@ -357,3 +359,119 @@ private fun FileDataStorageManager.saveFolder(remotePath: String, parent: OCFile saveFile(this) Log_OC.d(FileDataStorageManager.TAG, "createDirectoryTree: created $remotePath in Database") } + +/** + * Inserts or updates the content of [folder] and removes [filesToRemove] that are its direct children. + * + * Caller is responsible for granting the right to update the information; this function only keeps the data + * consistent. + */ +fun FileDataStorageManager.saveFolder(folder: OCFile, updatedFiles: List, filesToRemove: Collection) { + Log_OC.d( + FileDataStorageManager.TAG, + "Saving folder ${folder.remotePath} with ${updatedFiles.size} children and " + + "${filesToRemove.size} files to remove" + ) + + upsertFolderContent(folder, updatedFiles) + + val removedChildren = filesToRemove.filter { it.parentId == folder.fileId } + removedChildren.forEach { deleteLocalCopy(it) } + + val operations = removedChildren.map { deleteOperation(it) } + updateFolderOperation(folder) + applyFileOperations(ArrayList(operations)) +} + +private fun FileDataStorageManager.upsertFolderContent(folder: OCFile, files: List) { + db.runInTransaction { + val writableDatabase = db.openHelper.writableDatabase + files.forEach { file -> + val contentValues = createContentValuesForFile(file).apply { + put(ProviderTableMeta.FILE_PARENT, folder.fileId) + } + + val existingFileId = findExistingFileId(file) + if (existingFileId == null) { + file.fileId = writableDatabase.insert( + ProviderTableMeta.FILE_TABLE_NAME, + SQLiteDatabase.CONFLICT_NONE, + contentValues + ) + return@forEach + } + + writableDatabase.update( + ProviderTableMeta.FILE_TABLE_NAME, + SQLiteDatabase.CONFLICT_NONE, + contentValues, + "${ProviderTableMeta._ID} = ?", + arrayOf(existingFileId) + ) + } + } +} + +private fun FileDataStorageManager.findExistingFileId(file: OCFile): Long? { + val fileById = fileDao.getFileById(file.fileId) + if (fileById != null) { + return fileById.id + } + + return fileDao.getFileByEncryptedRemotePath(file.remotePath, user.accountName)?.id +} + +private fun FileDataStorageManager.deleteLocalCopy(file: OCFile) { + if (file.isFolder) { + val localFolder = File(FileStorageUtils.getDefaultSavePathFor(user.accountName, file)) + if (localFolder.exists()) { + removeLocalFolder(localFolder) + } + return + } + + if (!file.isDown) { + return + } + + val storagePath = file.storagePath + if (File(storagePath).delete() && MimeTypeUtil.isMedia(file.mimeType)) { + FileDataStorageManager.triggerMediaScan(storagePath, file) + } +} + +private fun FileDataStorageManager.deleteOperation(file: OCFile): ContentProviderOperation { + val contentUri = if (file.isFolder) ProviderTableMeta.CONTENT_URI_DIR else ProviderTableMeta.CONTENT_URI_FILE + return ContentProviderOperation.newDelete(ContentUris.withAppendedId(contentUri, file.fileId)) + .withSelection( + "${ProviderTableMeta.FILE_ACCOUNT_OWNER} = ? AND ${ProviderTableMeta.FILE_PATH} = ?", + arrayOf(user.accountName, file.remotePath) + ) + .build() +} + +private fun FileDataStorageManager.updateFolderOperation(folder: OCFile): ContentProviderOperation = + ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI) + .withValues(createContentValuesForFolder(folder)) + .withSelection("${ProviderTableMeta._ID} = ?", arrayOf(folder.fileId.toString())) + .build() + +private fun FileDataStorageManager.applyFileOperations(operations: ArrayList) { + Log_OC.d(FileDataStorageManager.TAG, "Sending ${operations.size} operations to FileContentProvider") + + try { + val resolver = contentResolver + if (resolver != null) { + resolver.applyBatch(MainApp.getAuthority(), operations) + } else { + contentProviderClient.applyBatch(operations) + } + } catch (e: OperationApplicationException) { + logFileOperationsFailure(e) + } catch (e: RemoteException) { + logFileOperationsFailure(e) + } +} + +private fun logFileOperationsFailure(e: Exception) { + Log_OC.e(FileDataStorageManager.TAG, "Exception in batch of operations ${e.message}", e) +} diff --git a/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java b/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java index 04055f375549..e1617a4e21ae 100644 --- a/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java +++ b/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java @@ -16,9 +16,7 @@ import android.annotation.SuppressLint; import android.content.ContentProviderClient; import android.content.ContentProviderOperation; -import android.content.ContentProviderResult; import android.content.ContentResolver; -import android.content.ContentUris; import android.content.ContentValues; import android.content.Context; import android.content.OperationApplicationException; @@ -84,11 +82,9 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Set; @@ -116,11 +112,12 @@ public class FileDataStorageManager { private final ContentProviderClient contentProviderClient; private final User user; - public final RecommendedFileDao recommendedFileDao = NextcloudDatabase.getInstance(MainApp.getAppContext()).recommendedFileDao(); - public final OfflineOperationDao offlineOperationDao = NextcloudDatabase.getInstance(MainApp.getAppContext()).offlineOperationDao(); - public final FileDao fileDao = NextcloudDatabase.getInstance(MainApp.getAppContext()).fileDao(); - public final ShareDao shareDao = NextcloudDatabase.getInstance(MainApp.getAppContext()).shareDao(); - public final CapabilityDao capabilityDao = NextcloudDatabase.instance().capabilityDao(); + public final NextcloudDatabase db = NextcloudDatabase.instance(); + public final RecommendedFileDao recommendedFileDao = db.recommendedFileDao(); + public final OfflineOperationDao offlineOperationDao = db.offlineOperationDao(); + public final FileDao fileDao = db.fileDao(); + public final ShareDao shareDao = db.shareDao(); + public final CapabilityDao capabilityDao = db.capabilityDao(); private final Gson gson = new Gson(); public final OfflineOperationsRepositoryType offlineOperationsRepository; @@ -743,123 +740,6 @@ public void saveNewFile(OCFile newFile) { } - /** - * Inserts or updates the list of files contained in a given folder. - *

- * CALLER IS RESPONSIBLE FOR GRANTING RIGHT UPDATE OF INFORMATION, NOT THIS METHOD. HERE ONLY DATA CONSISTENCY - * SHOULD BE GRANTED - * - * @param folder - * @param updatedFiles - * @param filesToRemove - */ - public void saveFolder(OCFile folder, List updatedFiles, Collection filesToRemove) { - Log_OC.d(TAG, "Saving folder " + folder.getRemotePath() + " with " + updatedFiles.size() - + " children and " + filesToRemove.size() + " files to remove"); - - ArrayList operations = new ArrayList<>(updatedFiles.size()); - - // prepare operations to insert or update files to save in the given folder - for (OCFile ocFile : updatedFiles) { - ContentValues contentValues = createContentValuesForFile(ocFile); - contentValues.put(ProviderTableMeta.FILE_PARENT, folder.getFileId()); - - if (fileExists(ocFile.getFileId()) || fileExists(ocFile.getRemotePath())) { - long fileId; - if (ocFile.getFileId() != -1) { - fileId = ocFile.getFileId(); - } else { - fileId = getFileByPath(ocFile.getRemotePath()).getFileId(); - } - // updating an existing file - operations.add(ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI) - .withValues(contentValues) - .withSelection(ProviderTableMeta._ID + " = ?", new String[]{String.valueOf(fileId)}) - .build()); - } else { - // adding a new file - operations.add(ContentProviderOperation.newInsert(ProviderTableMeta.CONTENT_URI) - .withValues(contentValues) - .build()); - } - } - - // prepare operations to remove files in the given folder - String where = ProviderTableMeta.FILE_ACCOUNT_OWNER + AND + ProviderTableMeta.FILE_PATH + " = ?"; - String[] whereArgs = new String[2]; - whereArgs[0] = user.getAccountName(); - for (OCFile ocFile : filesToRemove) { - if (ocFile.getParentId() == folder.getFileId()) { - whereArgs[1] = ocFile.getRemotePath(); - if (ocFile.isFolder()) { - operations.add(ContentProviderOperation.newDelete( - ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_DIR, ocFile.getFileId())) - .withSelection(where, whereArgs).build()); - - File localFolder = new File(FileStorageUtils.getDefaultSavePathFor(user.getAccountName(), ocFile)); - if (localFolder.exists()) { - removeLocalFolder(localFolder); - } - } else { - operations.add(ContentProviderOperation.newDelete( - ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, ocFile.getFileId())) - .withSelection(where, whereArgs).build()); - - if (ocFile.isDown()) { - String path = ocFile.getStoragePath(); - if (new File(path).delete() && MimeTypeUtil.isMedia(ocFile.getMimeType())) { - triggerMediaScan(path, ocFile); // notify MediaScanner about removed file - } - } - } - } - } - - // update metadata of folder - ContentValues contentValues = createContentValuesForFolder(folder); - - operations.add(ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI) - .withValues(contentValues) - .withSelection(ProviderTableMeta._ID + " = ?", new String[]{String.valueOf(folder.getFileId())}) - .build()); - - // apply operations in batch - ContentProviderResult[] results = null; - Log_OC.d(TAG, String.format(Locale.ENGLISH, SENDING_TO_FILECONTENTPROVIDER_MSG, operations.size())); - - try { - if (getContentResolver() != null) { - results = getContentResolver().applyBatch(MainApp.getAuthority(), operations); - - } else { - results = getContentProviderClient().applyBatch(operations); - } - - } catch (OperationApplicationException | RemoteException e) { - Log_OC.e(TAG, EXCEPTION_MSG + e.getMessage(), e); - } - - // update new id in file objects for insertions - if (results != null) { - long newId; - Iterator fileIterator = updatedFiles.iterator(); - OCFile ocFile; - for (ContentProviderResult result : results) { - if (fileIterator.hasNext()) { - ocFile = fileIterator.next(); - } else { - ocFile = null; - } - if (result.uri != null) { - newId = Long.parseLong(result.uri.getPathSegments().get(1)); - if (ocFile != null) { - ocFile.setFileId(newId); - } - } - } - } - } - /** * Returns a {@link ContentValues} filled with values that are common to both files and folders * @@ -910,7 +790,7 @@ private ContentValues createContentValuesBase(OCFile fileOrFolder) { * @see #createContentValuesForFile(OCFile) * @see #createContentValuesBase(OCFile) */ - private ContentValues createContentValuesForFolder(OCFile folder) { + public ContentValues createContentValuesForFolder(OCFile folder) { final ContentValues cv = createContentValuesBase(folder); cv.put(ProviderTableMeta.FILE_CONTENT_LENGTH, 0); return cv; @@ -923,7 +803,7 @@ private ContentValues createContentValuesForFolder(OCFile folder) { * @see #createContentValuesBase(OCFile) */ @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE") - private ContentValues createContentValuesForFile(OCFile file) { + public ContentValues createContentValuesForFile(OCFile file) { final ContentValues cv = createContentValuesBase(file); cv.put(ProviderTableMeta.FILE_CONTENT_LENGTH, file.getFileLength()); cv.put(ProviderTableMeta.FILE_ENCRYPTED_NAME, file.getEncryptedFileName()); @@ -1127,7 +1007,7 @@ private boolean removeLocalFolder(OCFile folder) { return success; } - private boolean removeLocalFolder(File localFolder) { + public boolean removeLocalFolder(File localFolder) { if (localFolder == null) { Log_OC.d(TAG, "removeLocalFolder(File): folder is null"); return false; diff --git a/app/src/main/java/com/owncloud/android/operations/RefreshFolderOperation.java b/app/src/main/java/com/owncloud/android/operations/RefreshFolderOperation.java index eecd9468489c..497cc1d51571 100644 --- a/app/src/main/java/com/owncloud/android/operations/RefreshFolderOperation.java +++ b/app/src/main/java/com/owncloud/android/operations/RefreshFolderOperation.java @@ -19,6 +19,7 @@ import com.nextcloud.utils.ResultParser; import com.nextcloud.utils.e2ee.E2EVersionHelper; import com.nextcloud.utils.share.UnifiedShareSharees; +import com.nextcloud.utils.extensions.FileDataStorageManagerExtensionsKt; import com.nextcloud.utils.extensions.OCFileExtensionsKt; import com.nextcloud.utils.extensions.StringExtensionsKt; import com.owncloud.android.datamodel.ArbitraryDataProvider; @@ -657,7 +658,10 @@ private void synchronizeData(List folderAndFiles) { UnifiedShareSharees.fillBlocking(user, updatedFiles); - fileDataStorageManager.saveFolder(remoteFolder, updatedFiles, localFilesMap.values()); + FileDataStorageManagerExtensionsKt.saveFolder(fileDataStorageManager, + remoteFolder, + updatedFiles, + localFilesMap.values()); mChildren = updatedFiles; } diff --git a/app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java b/app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java index 62299ebb96a5..5d2f43defa7a 100644 --- a/app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java +++ b/app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java @@ -20,6 +20,7 @@ import com.nextcloud.client.jobs.download.FileDownloadHelper; import com.nextcloud.client.jobs.folderDownload.FolderDownloadWorkerNotificationManager; import com.nextcloud.utils.extensions.ExtensionsKt; +import com.nextcloud.utils.extensions.FileDataStorageManagerExtensionsKt; import com.owncloud.android.datamodel.FileDataStorageManager; import com.owncloud.android.datamodel.OCFile; import com.owncloud.android.datamodel.e2e.v1.decrypted.DecryptedFolderMetadataFileV1; @@ -368,7 +369,7 @@ private void synchronizeData(List folderAndFiles) throws OperationCancel // save updated contents in local database UnifiedShareSharees.fillBlocking(user, updatedFiles); - storageManager.saveFolder(remoteFolder, updatedFiles, localFilesMap.values()); + FileDataStorageManagerExtensionsKt.saveFolder(storageManager, remoteFolder, updatedFiles, localFilesMap.values()); mLocalFolder.setLastSyncDateForData(System.currentTimeMillis()); storageManager.saveFile(mLocalFolder); diff --git a/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java b/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java index 80ddd5f02a00..6061de24d1d2 100644 --- a/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java +++ b/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java @@ -340,40 +340,21 @@ public Uri upsertSingleFile(SupportSQLiteDatabase db, Uri uri, ContentValues val 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); - } + try (Cursor cursor = db.query( + new SimpleSQLiteQuery( + "SELECT " + ProviderTableMeta._ID + + " FROM " + ProviderTableMeta.FILE_TABLE_NAME + + " WHERE " + where, + whereArgs + ))) { + if (cursor.moveToFirst()) { + long existingRowId = cursor.getLong(0); + db.update(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_NONE, values, where, whereArgs); + return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, existingRowId); } } + long rowId = db.insert(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_NONE, values); return ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, rowId); } diff --git a/app/src/test/java/com/nextcloud/utils/extensions/FileUpsertTest.kt b/app/src/test/java/com/nextcloud/utils/extensions/FileUpsertTest.kt new file mode 100644 index 000000000000..cf2f27cd9492 --- /dev/null +++ b/app/src/test/java/com/nextcloud/utils/extensions/FileUpsertTest.kt @@ -0,0 +1,327 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Alper Ozturk + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package com.nextcloud.utils.extensions + +import android.content.ContentProviderOperation +import android.content.ContentResolver +import android.content.ContentUris +import android.content.ContentValues +import android.database.Cursor +import android.database.sqlite.SQLiteDatabase +import android.net.Uri +import android.text.TextUtils +import androidx.sqlite.db.SimpleSQLiteQuery +import androidx.sqlite.db.SupportSQLiteDatabase +import com.nextcloud.client.account.User +import com.nextcloud.client.database.NextcloudDatabase +import com.nextcloud.client.database.dao.FileDao +import com.nextcloud.client.database.entity.FileEntity +import com.nextcloud.client.jobs.offlineOperations.repository.OfflineOperationsRepository +import com.owncloud.android.MainApp +import com.owncloud.android.datamodel.FileDataStorageManager +import com.owncloud.android.datamodel.OCFile +import com.owncloud.android.db.ProviderMeta.ProviderTableMeta +import com.owncloud.android.providers.FileContentProvider +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import io.mockk.verify +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Before +import org.junit.Test +import java.io.File +import java.nio.file.Files +import java.util.concurrent.Callable + +@Suppress("TooManyFunctions") +class FileUpsertTest { + + companion object { + private const val ACCOUNT_NAME = "user@nextcloud.example.com" + private const val AUTHORITY = "org.nextcloud" + private const val FOLDER_ID = 3L + private const val OTHER_FOLDER_ID = 4L + private const val FOLDER_PATH = "/folder/" + private const val FILE_PATH = "/folder/file.txt" + private const val REMOVED_FILE_PATH = "/folder/removed.txt" + private const val UNKNOWN_FILE_ID = -1L + private const val EXISTING_ROW_ID = 7L + private const val REMOVED_ROW_ID = 8L + private const val NEW_ROW_ID = 42L + private const val TRANSACTION = "transaction" + private const val BATCH = "batch" + } + + private lateinit var fileDao: FileDao + private lateinit var database: NextcloudDatabase + private lateinit var writableDatabase: SupportSQLiteDatabase + private lateinit var contentResolver: ContentResolver + private lateinit var storageManager: FileDataStorageManager + private val events = mutableListOf() + private val deleteBuilder = operationBuilder() + private val updateBuilder = operationBuilder() + private val folder = OCFile(FOLDER_PATH).apply { fileId = FOLDER_ID } + + @Before + fun setUp() { + fileDao = mockk(relaxed = true) + every { fileDao.getFileById(any()) } returns null + every { fileDao.getFileByEncryptedRemotePath(any(), any()) } returns null + + writableDatabase = mockk(relaxed = true) + database = mockk(relaxed = true) + every { database.openHelper.writableDatabase } returns writableDatabase + every { database.fileDao() } returns fileDao + runTransactionsImmediately() + + contentResolver = mockk(relaxed = true) + every { contentResolver.applyBatch(any(), any()) } answers { + events.add(BATCH) + emptyArray() + } + + mockkStatic(MainApp::class) + every { MainApp.getAppContext() } returns mockk(relaxed = true) + every { MainApp.getAuthority() } returns AUTHORITY + mockkObject(NextcloudDatabase.Companion) + every { NextcloudDatabase.getInstance(any()) } returns database + every { NextcloudDatabase.instance() } returns database + mockkConstructor(OfflineOperationsRepository::class) + mockkConstructor(ContentValues::class) + mockkStatic(TextUtils::class) + every { TextUtils.isEmpty(any()) } answers { arg(0).isNullOrEmpty() } + mockkStatic(ContentUris::class) + every { ContentUris.withAppendedId(any(), any()) } returns mockk() + mockkStatic(ContentProviderOperation::class) + every { ContentProviderOperation.newDelete(any()) } returns deleteBuilder + every { ContentProviderOperation.newUpdate(any()) } returns updateBuilder + + val user = mockk(relaxed = true) + every { user.accountName } returns ACCOUNT_NAME + storageManager = FileDataStorageManager(user, contentResolver) + } + + @After + fun tearDown() { + unmockkAll() + } + + private fun operationBuilder(): ContentProviderOperation.Builder = mockk { + every { withSelection(any(), any()) } returns this + every { withValues(any()) } returns this + every { build() } returns mockk() + } + + private fun runTransactionsImmediately() { + every { database.runInTransaction(any()) } answers { + events.add(TRANSACTION) + firstArg().run() + } + every { database.runInTransaction(any>()) } answers { + events.add(TRANSACTION) + firstArg>().call() + } + } + + private fun skipTransactions() { + every { database.runInTransaction(any()) } returns Unit + every { database.runInTransaction(any>()) } returns null + } + + private fun existingRow(): FileEntity = mockk { every { id } returns EXISTING_ROW_ID } + + private fun stubInsert() { + every { + writableDatabase.insert(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_NONE, any()) + } returns NEW_ROW_ID + } + + private fun saveFolder(updatedFiles: List = emptyList(), filesToRemove: List = emptyList()) { + storageManager.saveFolder(folder, updatedFiles, filesToRemove) + } + + private fun verifyUpdatedRow(rowId: Long) { + verify(exactly = 1) { + writableDatabase.update( + ProviderTableMeta.FILE_TABLE_NAME, + SQLiteDatabase.CONFLICT_NONE, + any(), + "${ProviderTableMeta._ID} = ?", + match { it.contentEquals(arrayOf(rowId)) } + ) + } + } + + @Test + fun `saveFolder inserts file and assigns generated id when no row matches id or path`() { + stubInsert() + val file = OCFile(FILE_PATH) + + saveFolder(updatedFiles = listOf(file)) + + assertEquals(NEW_ROW_ID, file.fileId) + verify(exactly = 0) { writableDatabase.update(any(), any(), any(), any(), any()) } + } + + @Test + fun `saveFolder updates row found by file id without inserting`() { + every { fileDao.getFileById(EXISTING_ROW_ID) } returns existingRow() + + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH).apply { fileId = EXISTING_ROW_ID })) + + verifyUpdatedRow(EXISTING_ROW_ID) + verify(exactly = 0) { writableDatabase.insert(any(), any(), any()) } + } + + @Test + fun `saveFolder updates row found by remote path when file id is unknown`() { + every { fileDao.getFileByEncryptedRemotePath(FILE_PATH, ACCOUNT_NAME) } returns existingRow() + + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH).apply { fileId = UNKNOWN_FILE_ID })) + + verifyUpdatedRow(EXISTING_ROW_ID) + verify(exactly = 0) { writableDatabase.insert(any(), any(), any()) } + } + + @Test + fun `saveFolder updates instead of inserting when the same folder is saved twice`() { + stubInsert() + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) + every { fileDao.getFileByEncryptedRemotePath(FILE_PATH, ACCOUNT_NAME) } returns existingRow() + + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) + + verify(exactly = 1) { writableDatabase.insert(any(), any(), any()) } + verifyUpdatedRow(EXISTING_ROW_ID) + } + + @Test + fun `saveFolder stores children under the saved folder`() { + stubInsert() + + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) + + verify { anyConstructed().put(ProviderTableMeta.FILE_PARENT, FOLDER_ID) } + } + + @Test + fun `saveFolder reads and writes children only inside a transaction`() { + skipTransactions() + + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) + + verify(exactly = 0) { fileDao.getFileById(any()) } + verify(exactly = 0) { fileDao.getFileByEncryptedRemotePath(any(), any()) } + verify(exactly = 0) { writableDatabase.insert(any(), any(), any()) } + verify(exactly = 0) { writableDatabase.update(any(), any(), any(), any(), any()) } + } + + @Test + fun `saveFolder commits children before applying removals and folder update`() { + stubInsert() + + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) + + assertEquals(listOf(TRANSACTION, BATCH), events) + } + + @Test + fun `saveFolder deletes only removed files whose parent is the saved folder`() { + val removedChild = OCFile(REMOVED_FILE_PATH).apply { + fileId = REMOVED_ROW_ID + parentId = FOLDER_ID + } + val fileOfOtherFolder = OCFile("/other/removed.txt").apply { parentId = OTHER_FOLDER_ID } + + saveFolder(filesToRemove = listOf(removedChild, fileOfOtherFolder)) + + verify(exactly = 1) { ContentProviderOperation.newDelete(any()) } + verify(exactly = 1) { ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, REMOVED_ROW_ID) } + verify(exactly = 1) { + deleteBuilder.withSelection( + "${ProviderTableMeta.FILE_ACCOUNT_OWNER} = ? AND ${ProviderTableMeta.FILE_PATH} = ?", + match { it.contentEquals(arrayOf(ACCOUNT_NAME, REMOVED_FILE_PATH)) } + ) + } + verify(exactly = 1) { contentResolver.applyBatch(AUTHORITY, match { it.size == 2 }) } + } + + @Test + fun `saveFolder deletes local copy of a removed downloaded file`() { + val localFile = Files.createTempFile("removed", ".txt").toFile() + val removedChild = OCFile(REMOVED_FILE_PATH).apply { + parentId = FOLDER_ID + storagePath = localFile.absolutePath + mimeType = "text/plain" + } + + saveFolder(filesToRemove = listOf(removedChild)) + + assertFalse(localFile.exists()) + } + + @Test + fun `saveFolder updates folder row by its id`() { + saveFolder() + + verify(exactly = 1) { ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI) } + verify(exactly = 1) { + updateBuilder.withSelection( + "${ProviderTableMeta._ID} = ?", + match { it.contentEquals(arrayOf(FOLDER_ID.toString())) } + ) + } + } + + @Test + fun `upsertSingleFile updates existing row and never inserts when path already exists`() { + val cursor = mockk(relaxed = true) + every { cursor.moveToFirst() } returns true + every { cursor.getLong(0) } returns EXISTING_ROW_ID + every { writableDatabase.query(any()) } returns cursor + val rowIds = captureAppendedRowIds() + val values = mockk(relaxed = true) + + FileContentProvider().upsertSingleFile(writableDatabase, ProviderTableMeta.CONTENT_URI_FILE, values) + + assertEquals(listOf(EXISTING_ROW_ID), rowIds) + verify(exactly = 1) { writableDatabase.update(any(), any(), values, any(), any()) } + verify(exactly = 0) { writableDatabase.insert(any(), any(), any()) } + } + + @Test + fun `upsertSingleFile inserts when no row matches path and owner`() { + val cursor = mockk(relaxed = true) + every { cursor.moveToFirst() } returns false + every { writableDatabase.query(any()) } returns cursor + stubInsert() + val rowIds = captureAppendedRowIds() + + FileContentProvider().upsertSingleFile( + writableDatabase, + ProviderTableMeta.CONTENT_URI_FILE, + mockk(relaxed = true) + ) + + assertEquals(listOf(NEW_ROW_ID), rowIds) + verify(exactly = 0) { writableDatabase.update(any(), any(), any(), any(), any()) } + } + + private fun captureAppendedRowIds(): MutableList { + val rowIds = mutableListOf() + every { ContentUris.withAppendedId(any(), any()) } answers { + rowIds.add(secondArg()) + mockk() + } + return rowIds + } +} From d6796e31b6d87b0c4314aa549d431587780170a4 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 3 Nov 2026 13:05:23 +0300 Subject: [PATCH 2/4] optimize imports Signed-off-by: alperozturk96 --- .../owncloud/android/datamodel/FileDataStorageManager.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java b/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java index e1617a4e21ae..fc037104f64d 100644 --- a/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java +++ b/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java @@ -13,7 +13,6 @@ */ package com.owncloud.android.datamodel; -import android.annotation.SuppressLint; import android.content.ContentProviderClient; import android.content.ContentProviderOperation; import android.content.ContentResolver; @@ -45,9 +44,7 @@ import com.nextcloud.model.OfflineOperationType; import com.nextcloud.model.ShareeEntry; import com.nextcloud.utils.AppConstants; -import com.nextcloud.utils.date.DateFormatPattern; import com.nextcloud.utils.e2ee.E2EVersionHelper; -import com.nextcloud.utils.extensions.DateExtensionsKt; import com.nextcloud.utils.extensions.FileDataStorageManagerExtensionsKt; import com.nextcloud.utils.extensions.FileExtensionsKt; import com.nextcloud.utils.extensions.OCFileExtensionsKt; @@ -71,7 +68,6 @@ import com.owncloud.android.lib.resources.status.OCCapability; import com.owncloud.android.lib.resources.tags.Tag; import com.owncloud.android.operations.RemoteOperationFailedException; -import com.owncloud.android.operations.UploadFileOperation; import com.owncloud.android.utils.FileStorageUtils; import com.owncloud.android.utils.MimeType; import com.owncloud.android.utils.MimeTypeUtil; @@ -83,7 +79,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Locale; From 8ad26c9f5ed9409a1e5844282c2a50959e102502 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 3 Nov 2026 13:46:32 +0300 Subject: [PATCH 3/4] fix deletion Signed-off-by: alperozturk96 --- ...FileDataStorageManagerContentResolverIT.kt | 2 +- .../providers/FileContentProviderTests.kt | 135 +++++++++++++++++- .../nextcloud/client/database/dao/FileDao.kt | 3 + .../datamodel/FileDataStorageManager.java | 14 +- .../providers/FileContentProvider.java | 44 +++--- ...sertTest.kt => FileStorageManagerTests.kt} | 10 +- 6 files changed, 170 insertions(+), 38 deletions(-) rename app/src/test/java/com/nextcloud/utils/extensions/{FileUpsertTest.kt => FileStorageManagerTests.kt} (97%) diff --git a/app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt b/app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt index 0499fc821fbf..721d5bec5483 100644 --- a/app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt +++ b/app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt @@ -42,7 +42,7 @@ class FileDataStorageManagerContentResolverIT : FileDataStorageManagerIT() { val storedFile = sut.getFileByDecryptedRemotePath("/folderA/file$it") Assert.assertNotNull(storedFile) - storedFile + requireNotNull(storedFile) } // save files in folder diff --git a/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt b/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt index 52ac4b45ca54..2b2db82eb0f2 100644 --- a/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt +++ b/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt @@ -7,25 +7,42 @@ package com.owncloud.android.providers -import android.content.ContentValues import android.content.ContentUris +import android.content.ContentValues +import android.content.pm.ProviderInfo import android.net.Uri +import androidx.room.Room import androidx.sqlite.db.SimpleSQLiteQuery import androidx.sqlite.db.SupportSQLiteDatabase +import androidx.test.platform.app.InstrumentationRegistry +import com.nextcloud.client.database.NextcloudDatabase +import com.nextcloud.client.database.typeConverter.OfflineOperationTypeConverter +import com.owncloud.android.R import com.owncloud.android.db.ProviderMeta +import com.owncloud.android.db.ProviderMeta.ProviderTableMeta +import com.owncloud.android.utils.MimeType import io.mockk.Runs import io.mockk.every import io.mockk.just import io.mockk.mockk +import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test -@Suppress("MagicNumber") +@Suppress("MagicNumber", "TooManyFunctions") class FileContentProviderTests { + companion object { + private const val OWNER = "owner@example.com" + private const val OTHER_OWNER = "other@example.com" + private const val NO_PARENT = 0L + } + private lateinit var provider: FileContentProvider private lateinit var db: SupportSQLiteDatabase + private var inMemoryDatabase: NextcloudDatabase? = null @Before fun setup() { @@ -33,6 +50,11 @@ class FileContentProviderTests { db = mockk() } + @After + fun tearDown() { + inMemoryDatabase?.close() + } + @Test fun insertNewFileShouldReturnNewId() { val values = ContentValues().apply { @@ -96,4 +118,113 @@ class FileContentProviderTests { cursor.close() } + + private fun createProviderWithInMemoryDatabase(): FileContentProvider { + val context = InstrumentationRegistry.getInstrumentation().targetContext + val database = Room.inMemoryDatabaseBuilder(context, NextcloudDatabase::class.java) + .addTypeConverter(OfflineOperationTypeConverter()) + .allowMainThreadQueries() + .build() + inMemoryDatabase = database + + val provider = FileContentProvider() + provider.attachInfo(context, ProviderInfo().apply { authority = context.getString(R.string.authority) }) + FileContentProvider::class.java.getDeclaredField("mDbHelper") + .apply { isAccessible = true } + .set(provider, database.openHelper) + return provider + } + + private fun FileContentProvider.insertFile( + path: String, + parentId: Long, + owner: String = OWNER, + isFolder: Boolean = false + ): Long { + val values = ContentValues().apply { + put(ProviderTableMeta.FILE_PATH, path) + put(ProviderTableMeta.FILE_ACCOUNT_OWNER, owner) + put(ProviderTableMeta.FILE_PARENT, parentId) + put(ProviderTableMeta.FILE_CONTENT_TYPE, if (isFolder) MimeType.DIRECTORY else "text/plain") + } + val uri = insert(ProviderTableMeta.CONTENT_URI_FILE, values) + return ContentUris.parseId(requireNotNull(uri)) + } + + private fun storedPaths(owner: String = OWNER): List = + requireNotNull(inMemoryDatabase).fileDao().getAllFiles(owner).map { it.path } + + @Test + fun queryReturnsChildrenOfDirectoryWhenNoSelectionIsGiven() { + val provider = createProviderWithInMemoryDatabase() + val rootId = provider.insertFile("/", NO_PARENT, isFolder = true) + provider.insertFile("/a.txt", rootId) + provider.insertFile("/b.txt", rootId) + + val cursor = provider.query( + ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_DIR, rootId), + null, + null, + null, + null + ) + + assertEquals(2, requireNotNull(cursor).use { it.count }) + } + + @Test + fun deletingDirectoryRemovesAllOfItsDescendants() { + val provider = createProviderWithInMemoryDatabase() + val rootId = provider.insertFile("/", NO_PARENT, isFolder = true) + val folderId = provider.insertFile("/folder/", rootId, isFolder = true) + val subFolderId = provider.insertFile("/folder/sub/", folderId, isFolder = true) + provider.insertFile("/folder/a.txt", folderId) + provider.insertFile("/folder/sub/b.txt", subFolderId) + + provider.delete(ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_DIR, folderId), null, null) + + assertEquals(listOf("/"), storedPaths()) + } + + @Test + fun deletingDirectoriesWithoutIdRemovesOnlyMatchingDirectoriesAndTheirDescendants() { + val provider = createProviderWithInMemoryDatabase() + val ownerRootId = provider.insertFile("/", NO_PARENT, isFolder = true) + provider.insertFile("/a.txt", ownerRootId) + val otherRootId = provider.insertFile("/", NO_PARENT, owner = OTHER_OWNER, isFolder = true) + provider.insertFile("/b.txt", otherRootId, owner = OTHER_OWNER) + + provider.delete( + ProviderTableMeta.CONTENT_URI_DIR, + "${ProviderTableMeta.FILE_ACCOUNT_OWNER} = ? AND ${ProviderTableMeta.FILE_PATH} = ?", + arrayOf(OWNER, "/") + ) + + assertTrue(storedPaths().isEmpty()) + assertEquals(listOf("/", "/b.txt"), storedPaths(OTHER_OWNER).sortedBy { it }) + } + + @Test + fun insertingExistingPathUpdatesRowInsteadOfAddingDuplicate() { + val provider = createProviderWithInMemoryDatabase() + val rootId = provider.insertFile("/", NO_PARENT, isFolder = true) + val firstId = provider.insertFile("/a.txt", rootId) + + val secondId = provider.insertFile("/a.txt", rootId) + + assertEquals(firstId, secondId) + assertEquals(listOf("/", "/a.txt"), storedPaths().sortedBy { it }) + } + + @Test + fun fileDaoDeleteAllFilesRemovesOnlyRowsOfGivenOwner() { + val provider = createProviderWithInMemoryDatabase() + provider.insertFile("/", NO_PARENT, isFolder = true) + provider.insertFile("/", NO_PARENT, owner = OTHER_OWNER, isFolder = true) + + requireNotNull(inMemoryDatabase).fileDao().deleteAllFiles(OWNER) + + assertTrue(storedPaths().isEmpty()) + assertEquals(listOf("/"), storedPaths(OTHER_OWNER)) + } } diff --git a/app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt b/app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt index 7cecfe6fdef3..86976a5c3088 100644 --- a/app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt +++ b/app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt @@ -199,6 +199,9 @@ interface FileDao { @Query("DELETE FROM filelist WHERE file_owner = :fileOwner AND path = :remotePath") fun deleteFileByRemotePath(fileOwner: String, remotePath: String): Int + @Query("DELETE FROM filelist WHERE file_owner = :fileOwner") + fun deleteAllFiles(fileOwner: String): Int + @Query("UPDATE filelist SET is_read_only = :readOnly WHERE file_owner = :fileOwner AND path = :path") fun setReadOnly(fileOwner: String, path: String, readOnly: Int): Int diff --git a/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java b/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java index fc037104f64d..a14b7986b730 100644 --- a/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java +++ b/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java @@ -2609,19 +2609,7 @@ public List getVirtualFolderContent(VirtualFolderType type, boolean only } public void deleteAllFiles() { - String where = ProviderTableMeta.FILE_ACCOUNT_OWNER + "= ? AND " + - ProviderTableMeta.FILE_PATH + "= ?"; - String[] whereArgs = new String[]{user.getAccountName(), OCFile.ROOT_PATH}; - - if (getContentResolver() != null) { - getContentResolver().delete(ProviderTableMeta.CONTENT_URI_DIR, where, whereArgs); - } else { - try { - getContentProviderClient().delete(ProviderTableMeta.CONTENT_URI_DIR, where, whereArgs); - } catch (RemoteException e) { - Log_OC.e(TAG, "Exception in deleteAllFiles for account " + user.getAccountName() + ": " + e.getMessage(), e); - } - } + fileDao.deleteAllFiles(user.getAccountName()); } public String getFolderName(String path) { diff --git a/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java b/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java index 6061de24d1d2..fb87de46c992 100644 --- a/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java +++ b/app/src/main/java/com/owncloud/android/providers/FileContentProvider.java @@ -150,30 +150,31 @@ private int delete(SupportSQLiteDatabase db, Uri uri, String where, String... wh private int deleteDirectory(SupportSQLiteDatabase db, Uri uri, String where, String... whereArgs) { int count = 0; - Cursor children = query(uri, PROJECTION_CONTENT_TYPE, null, null, null); - if (children != null) { - if (children.moveToFirst()) { - long childId; - boolean isDir; - while (!children.isAfterLast()) { - childId = children.getLong(children.getColumnIndexOrThrow(ProviderTableMeta._ID)); - isDir = MimeType.DIRECTORY.equals(children.getString( - children.getColumnIndexOrThrow(ProviderTableMeta.FILE_CONTENT_TYPE) - )); - if (isDir) { - count += delete(db, ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_DIR, childId), - null, (String[]) null); - } else { - count += delete(db, ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, childId), - null, (String[]) null); - } - children.moveToNext(); + boolean isSingleDirectory = uri.getPathSegments().size() > MINIMUM_PATH_SEGMENTS_SIZE; + Cursor children = isSingleDirectory + ? query(db, uri, PROJECTION_CONTENT_TYPE, null, null, null) + : query(db, uri, PROJECTION_CONTENT_TYPE, where, whereArgs, null); + if (children.moveToFirst()) { + long childId; + boolean isDir; + while (!children.isAfterLast()) { + childId = children.getLong(children.getColumnIndexOrThrow(ProviderTableMeta._ID)); + isDir = MimeType.DIRECTORY.equals(children.getString( + children.getColumnIndexOrThrow(ProviderTableMeta.FILE_CONTENT_TYPE) + )); + if (isDir) { + count += delete(db, ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_DIR, childId), + null, (String[]) null); + } else { + count += delete(db, ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, childId), + null, (String[]) null); } + children.moveToNext(); } - children.close(); } + children.close(); - if (uri.getPathSegments().size() > MINIMUM_PATH_SEGMENTS_SIZE) { + if (isSingleDirectory) { count += deleteWithUri(db, uri, where, whereArgs); } @@ -489,7 +490,8 @@ private Cursor query(SupportSQLiteDatabase db, // add ID to arguments if Uri has more than one segment if (uriMatch != ROOT_DIRECTORY && uri.getPathSegments().size() > SINGLE_PATH_SEGMENT) { String idColumn = uriMatch == DIRECTORY ? ProviderTableMeta.FILE_PARENT : ProviderTableMeta._ID; - selection = idColumn + "=? AND " + selection; + String idSelection = idColumn + "=?"; + selection = TextUtils.isEmpty(selection) ? idSelection : idSelection + " AND " + selection; selectionArgs = VerificationUtils.prependUriFirstSegmentToSelectionArgs(selectionArgs, uri); } diff --git a/app/src/test/java/com/nextcloud/utils/extensions/FileUpsertTest.kt b/app/src/test/java/com/nextcloud/utils/extensions/FileStorageManagerTests.kt similarity index 97% rename from app/src/test/java/com/nextcloud/utils/extensions/FileUpsertTest.kt rename to app/src/test/java/com/nextcloud/utils/extensions/FileStorageManagerTests.kt index cf2f27cd9492..20a465923c24 100644 --- a/app/src/test/java/com/nextcloud/utils/extensions/FileUpsertTest.kt +++ b/app/src/test/java/com/nextcloud/utils/extensions/FileStorageManagerTests.kt @@ -43,7 +43,7 @@ import java.nio.file.Files import java.util.concurrent.Callable @Suppress("TooManyFunctions") -class FileUpsertTest { +class FileStorageManagerTests { companion object { private const val ACCOUNT_NAME = "user@nextcloud.example.com" @@ -282,6 +282,14 @@ class FileUpsertTest { } } + @Test + fun `deleteAllFiles deletes only rows of the current account`() { + storageManager.deleteAllFiles() + + verify(exactly = 1) { fileDao.deleteAllFiles(ACCOUNT_NAME) } + verify(exactly = 0) { contentResolver.delete(any(), any(), any()) } + } + @Test fun `upsertSingleFile updates existing row and never inserts when path already exists`() { val cursor = mockk(relaxed = true) From e5d38cf0bfc36686d37c46f867a7cee5ee1a48a2 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 3 Nov 2026 15:01:56 +0300 Subject: [PATCH 4/4] use dao functions Signed-off-by: alperozturk96 --- .../providers/FileContentProviderTests.kt | 87 +++++++++ .../nextcloud/client/database/dao/FileDao.kt | 26 +++ .../entity/model/FolderMetadataUpdate.kt | 82 ++++++++ .../FileDataStorageManagerExtensions.kt | 96 +++------- .../utils/extensions/OCFileExtensions.kt | 105 +++++++++++ .../datamodel/FileDataStorageManager.java | 16 +- .../extensions/FileStorageManagerTests.kt | 175 +++++++++++------- 7 files changed, 433 insertions(+), 154 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/client/database/entity/model/FolderMetadataUpdate.kt diff --git a/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt b/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt index 2b2db82eb0f2..97d0c4a64aa4 100644 --- a/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt +++ b/app/src/androidTest/java/com/owncloud/android/providers/FileContentProviderTests.kt @@ -17,7 +17,10 @@ import androidx.sqlite.db.SupportSQLiteDatabase import androidx.test.platform.app.InstrumentationRegistry import com.nextcloud.client.database.NextcloudDatabase import com.nextcloud.client.database.typeConverter.OfflineOperationTypeConverter +import com.nextcloud.utils.extensions.toFileEntity +import com.nextcloud.utils.extensions.toFolderMetadataUpdate import com.owncloud.android.R +import com.owncloud.android.datamodel.OCFile import com.owncloud.android.db.ProviderMeta import com.owncloud.android.db.ProviderMeta.ProviderTableMeta import com.owncloud.android.utils.MimeType @@ -227,4 +230,88 @@ class FileContentProviderTests { assertTrue(storedPaths().isEmpty()) assertEquals(listOf("/"), storedPaths(OTHER_OWNER)) } + + @Test + fun fileDaoUpsertInsertsNewRowAndUpdatesExistingRowInPlace() { + createProviderWithInMemoryDatabase() + val fileDao = requireNotNull(inMemoryDatabase).fileDao() + val file = OCFile("/a.txt").apply { etag = "first" } + + val insertedId = fileDao.upsert(file.toFileEntity(OWNER)) + val updateResult = fileDao.upsert(file.apply { etag = "second" }.toFileEntity(OWNER).copy(id = insertedId)) + + assertEquals(-1L, updateResult) + assertEquals(listOf("/a.txt"), storedPaths()) + assertEquals("second", fileDao.getFileById(insertedId)?.etag) + } + + @Test + fun fileDaoDeleteFolderRemovesFolderAndAllDescendants() { + val provider = createProviderWithInMemoryDatabase() + val rootId = provider.insertFile("/", NO_PARENT, isFolder = true) + val folderId = provider.insertFile("/folder/", rootId, isFolder = true) + val subFolderId = provider.insertFile("/folder/sub/", folderId, isFolder = true) + provider.insertFile("/folder/a.txt", folderId) + provider.insertFile("/folder/sub/b.txt", subFolderId) + + requireNotNull(inMemoryDatabase).fileDao().deleteFolder(folderId, OWNER, "/folder/") + + assertEquals(listOf("/"), storedPaths()) + } + + @Test + fun fileDaoDeleteFolderKeepsFolderRowWhenPathDoesNotMatch() { + val provider = createProviderWithInMemoryDatabase() + val rootId = provider.insertFile("/", NO_PARENT, isFolder = true) + val folderId = provider.insertFile("/folder/", rootId, isFolder = true) + provider.insertFile("/folder/a.txt", folderId) + + requireNotNull(inMemoryDatabase).fileDao().deleteFolder(folderId, OWNER, "/other/") + + assertEquals(listOf("/", "/folder/"), storedPaths().sortedBy { it }) + } + + @Test + fun fileDaoDeleteFileRemovesOnlyRowMatchingIdOwnerAndPath() { + val provider = createProviderWithInMemoryDatabase() + val rootId = provider.insertFile("/", NO_PARENT, isFolder = true) + val fileId = provider.insertFile("/a.txt", rootId) + provider.insertFile("/b.txt", rootId) + val fileDao = requireNotNull(inMemoryDatabase).fileDao() + + fileDao.deleteFile(fileId, OTHER_OWNER, "/a.txt") + fileDao.deleteFile(fileId, OWNER, "/b.txt") + assertEquals(listOf("/", "/a.txt", "/b.txt"), storedPaths().sortedBy { it }) + + fileDao.deleteFile(fileId, OWNER, "/a.txt") + assertEquals(listOf("/", "/b.txt"), storedPaths().sortedBy { it }) + } + + @Test + fun fileDaoUpdateFolderMetadataKeepsFileOnlyColumns() { + createProviderWithInMemoryDatabase() + val fileDao = requireNotNull(inMemoryDatabase).fileDao() + val storedFolder = OCFile("/folder/").apply { + setFolder() + etag = "old" + storagePath = "/storage/folder" + setE2eCounter(5L) + etagInConflict = "conflict" + } + val folderId = fileDao.upsert(storedFolder.toFileEntity(OWNER)) + val serverFolder = OCFile("/folder/").apply { + setFolder() + fileId = folderId + etag = "new" + } + + fileDao.updateFolderMetadata(serverFolder.toFolderMetadataUpdate(OWNER)) + + val updatedFolder = requireNotNull(fileDao.getFileById(folderId)) + assertEquals("new", updatedFolder.etag) + assertEquals(0L, updatedFolder.contentLength) + assertEquals(storedFolder.storagePath, updatedFolder.storagePath) + assertEquals(5L, updatedFolder.e2eCounter) + assertEquals("conflict", updatedFolder.etagInConflict) + } } diff --git a/app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt b/app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt index 86976a5c3088..37336d517b21 100644 --- a/app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt +++ b/app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt @@ -10,7 +10,9 @@ package com.nextcloud.client.database.dao import androidx.room.Dao import androidx.room.Query import androidx.room.Update +import androidx.room.Upsert import com.nextcloud.client.database.entity.FileEntity +import com.nextcloud.client.database.entity.model.FolderMetadataUpdate import com.owncloud.android.db.ProviderMeta.ProviderTableMeta import com.owncloud.android.utils.MimeType @@ -19,6 +21,30 @@ interface FileDao { @Update fun update(entity: FileEntity) + @Upsert + fun upsert(entity: FileEntity): Long + + @Update(entity = FileEntity::class) + fun updateFolderMetadata(metadata: FolderMetadataUpdate) + + @Query("DELETE FROM filelist WHERE _id = :id AND file_owner = :fileOwner AND path = :path") + fun deleteFile(id: Long, fileOwner: String, path: String): Int + + @Query( + """ + WITH RECURSIVE descendants(_id) AS ( + SELECT _id FROM filelist WHERE parent = :folderId + UNION + SELECT f._id FROM filelist f + INNER JOIN descendants d ON f.parent = d._id + ) + DELETE FROM filelist + WHERE _id IN (SELECT _id FROM descendants) + OR (_id = :folderId AND file_owner = :fileOwner AND path = :path) +""" + ) + fun deleteFolder(folderId: Long, fileOwner: String, path: String): Int + @Query("SELECT * FROM filelist WHERE _id = :id LIMIT 1") fun getFileById(id: Long): FileEntity? diff --git a/app/src/main/java/com/nextcloud/client/database/entity/model/FolderMetadataUpdate.kt b/app/src/main/java/com/nextcloud/client/database/entity/model/FolderMetadataUpdate.kt new file mode 100644 index 000000000000..a8b977b73073 --- /dev/null +++ b/app/src/main/java/com/nextcloud/client/database/entity/model/FolderMetadataUpdate.kt @@ -0,0 +1,82 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Alper Ozturk + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.database.entity.model + +import androidx.room.ColumnInfo +import com.owncloud.android.db.ProviderMeta.ProviderTableMeta + +data class FolderMetadataUpdate( + @ColumnInfo(name = ProviderTableMeta._ID) + val id: Long, + @ColumnInfo(name = ProviderTableMeta.FILE_MODIFIED) + val modified: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_MODIFIED_AT_LAST_SYNC_FOR_DATA) + val modifiedAtLastSyncForData: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_PARENT) + val parent: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_UPLOADED) + val uploaded: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_CREATION) + val creation: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_CONTENT_TYPE) + val contentType: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_CONTENT_LENGTH) + val contentLength: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_NAME) + val name: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_PATH) + val path: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_PATH_DECRYPTED) + val pathDecrypted: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_ACCOUNT_OWNER) + val accountOwner: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_IS_ENCRYPTED) + val isEncrypted: Int?, + @ColumnInfo(name = ProviderTableMeta.FILE_IS_READ_ONLY) + val isReadOnly: Int?, + @ColumnInfo(name = ProviderTableMeta.FILE_LAST_SYNC_DATE) + val lastSyncDate: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_LAST_SYNC_DATE_FOR_DATA) + val lastSyncDateForData: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_ETAG) + val etag: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_ETAG_ON_SERVER) + val etagOnServer: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_SHARED_VIA_LINK) + val sharedViaLink: Int?, + @ColumnInfo(name = ProviderTableMeta.FILE_SHARED_WITH_SHAREE) + val sharedWithSharee: Int?, + @ColumnInfo(name = ProviderTableMeta.FILE_PERMISSIONS) + val permissions: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_REMOTE_ID) + val remoteId: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_LOCAL_ID) + val localId: Long, + @ColumnInfo(name = ProviderTableMeta.FILE_FAVORITE) + val favorite: Int?, + @ColumnInfo(name = ProviderTableMeta.FILE_HIDDEN) + val hidden: Int?, + @ColumnInfo(name = ProviderTableMeta.FILE_UNREAD_COMMENTS_COUNT) + val unreadCommentsCount: Int?, + @ColumnInfo(name = ProviderTableMeta.FILE_OWNER_ID) + val ownerId: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_OWNER_DISPLAY_NAME) + val ownerDisplayName: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_NOTE) + val note: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_SHAREES) + val sharees: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_TAGS) + val tags: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_RICH_WORKSPACE) + val richWorkspace: String?, + @ColumnInfo(name = ProviderTableMeta.FILE_INTERNAL_TWO_WAY_SYNC_TIMESTAMP) + val internalTwoWaySync: Long?, + @ColumnInfo(name = ProviderTableMeta.FILE_INTERNAL_TWO_WAY_SYNC_RESULT) + val internalTwoWaySyncResult: String? +) diff --git a/app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt b/app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt index 26a260b34368..55e5923e2af1 100644 --- a/app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt +++ b/app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt @@ -7,12 +7,8 @@ package com.nextcloud.utils.extensions -import android.content.ContentProviderOperation -import android.content.ContentUris -import android.content.OperationApplicationException -import android.database.sqlite.SQLiteDatabase -import android.os.RemoteException import com.nextcloud.client.database.dao.FileDao +import com.nextcloud.client.database.entity.FileEntity import com.nextcloud.client.database.entity.model.ShareeKey import com.nextcloud.model.HTTPStatusCodes import com.owncloud.android.MainApp @@ -373,51 +369,41 @@ fun FileDataStorageManager.saveFolder(folder: OCFile, updatedFiles: List "${filesToRemove.size} files to remove" ) - upsertFolderContent(folder, updatedFiles) - val removedChildren = filesToRemove.filter { it.parentId == folder.fileId } removedChildren.forEach { deleteLocalCopy(it) } - val operations = removedChildren.map { deleteOperation(it) } + updateFolderOperation(folder) - applyFileOperations(ArrayList(operations)) + db.runInTransaction { + upsertFolderContent(folder, updatedFiles) + removedChildren.forEach { deleteFromDatabase(it) } + fileDao.updateFolderMetadata(folder.toFolderMetadataUpdate(user.accountName)) + } + + MainApp.getAppContext().contentResolver.notifyChange(ProviderTableMeta.CONTENT_URI, null) } private fun FileDataStorageManager.upsertFolderContent(folder: OCFile, files: List) { - db.runInTransaction { - val writableDatabase = db.openHelper.writableDatabase - files.forEach { file -> - val contentValues = createContentValuesForFile(file).apply { - put(ProviderTableMeta.FILE_PARENT, folder.fileId) - } - - val existingFileId = findExistingFileId(file) - if (existingFileId == null) { - file.fileId = writableDatabase.insert( - ProviderTableMeta.FILE_TABLE_NAME, - SQLiteDatabase.CONFLICT_NONE, - contentValues - ) - return@forEach - } - - writableDatabase.update( - ProviderTableMeta.FILE_TABLE_NAME, - SQLiteDatabase.CONFLICT_NONE, - contentValues, - "${ProviderTableMeta._ID} = ?", - arrayOf(existingFileId) - ) + files.forEach { file -> + val existingFile = findExistingFile(file) + val entity = file.toFileEntity(user.accountName).copy( + id = existingFile?.id, + parent = folder.fileId, + mountType = existingFile?.mountType + ) + + val insertedRowId = fileDao.upsert(entity) + if (existingFile == null) { + file.fileId = insertedRowId } } } -private fun FileDataStorageManager.findExistingFileId(file: OCFile): Long? { +private fun FileDataStorageManager.findExistingFile(file: OCFile): FileEntity? { val fileById = fileDao.getFileById(file.fileId) if (fileById != null) { - return fileById.id + return fileById } - return fileDao.getFileByEncryptedRemotePath(file.remotePath, user.accountName)?.id + return fileDao.getFileByEncryptedRemotePath(file.remotePath, user.accountName) } private fun FileDataStorageManager.deleteLocalCopy(file: OCFile) { @@ -439,39 +425,11 @@ private fun FileDataStorageManager.deleteLocalCopy(file: OCFile) { } } -private fun FileDataStorageManager.deleteOperation(file: OCFile): ContentProviderOperation { - val contentUri = if (file.isFolder) ProviderTableMeta.CONTENT_URI_DIR else ProviderTableMeta.CONTENT_URI_FILE - return ContentProviderOperation.newDelete(ContentUris.withAppendedId(contentUri, file.fileId)) - .withSelection( - "${ProviderTableMeta.FILE_ACCOUNT_OWNER} = ? AND ${ProviderTableMeta.FILE_PATH} = ?", - arrayOf(user.accountName, file.remotePath) - ) - .build() -} - -private fun FileDataStorageManager.updateFolderOperation(folder: OCFile): ContentProviderOperation = - ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI) - .withValues(createContentValuesForFolder(folder)) - .withSelection("${ProviderTableMeta._ID} = ?", arrayOf(folder.fileId.toString())) - .build() - -private fun FileDataStorageManager.applyFileOperations(operations: ArrayList) { - Log_OC.d(FileDataStorageManager.TAG, "Sending ${operations.size} operations to FileContentProvider") - - try { - val resolver = contentResolver - if (resolver != null) { - resolver.applyBatch(MainApp.getAuthority(), operations) - } else { - contentProviderClient.applyBatch(operations) - } - } catch (e: OperationApplicationException) { - logFileOperationsFailure(e) - } catch (e: RemoteException) { - logFileOperationsFailure(e) +private fun FileDataStorageManager.deleteFromDatabase(file: OCFile) { + if (file.isFolder) { + fileDao.deleteFolder(file.fileId, user.accountName, file.remotePath) + return } -} -private fun logFileOperationsFailure(e: Exception) { - Log_OC.e(FileDataStorageManager.TAG, "Exception in batch of operations ${e.message}", e) + fileDao.deleteFile(file.fileId, user.accountName, file.remotePath) } diff --git a/app/src/main/java/com/nextcloud/utils/extensions/OCFileExtensions.kt b/app/src/main/java/com/nextcloud/utils/extensions/OCFileExtensions.kt index acad982bb83b..0f6a8a0fb49b 100644 --- a/app/src/main/java/com/nextcloud/utils/extensions/OCFileExtensions.kt +++ b/app/src/main/java/com/nextcloud/utils/extensions/OCFileExtensions.kt @@ -7,6 +7,9 @@ package com.nextcloud.utils.extensions +import com.google.gson.Gson +import com.nextcloud.client.database.entity.FileEntity +import com.nextcloud.client.database.entity.model.FolderMetadataUpdate import com.owncloud.android.MainApp import com.owncloud.android.datamodel.GalleryItems import com.owncloud.android.datamodel.GalleryRow @@ -23,6 +26,10 @@ import java.io.File import java.util.Calendar private const val TAG = "OCFileExtensions" +private const val NO_LOCK_TYPE = -1 +private const val FOLDER_CONTENT_LENGTH = 0L + +private val gson = Gson() fun List.filterFilenames(): List = distinctBy { it.fileName } @@ -117,3 +124,101 @@ fun OCFile.isLocalETagOutdated(): Boolean { val serverETag = etagInConflict return !serverETag.isNullOrEmpty() && etag.eTagChanged(serverETag) } + +fun OCFile.toFileEntity(accountName: String): FileEntity = FileEntity( + id = fileId.takeIf { it > 0 }, + name = fileName, + encryptedName = encryptedFileName, + path = remotePath, + pathDecrypted = decryptedRemotePath, + parent = parentId, + creation = creationTimestamp, + modified = modificationTimestamp, + contentType = mimeType, + contentLength = fileLength, + storagePath = storagePath, + accountOwner = accountName, + lastSyncDate = lastSyncDateForProperties, + lastSyncDateForData = lastSyncDateForData, + modifiedAtLastSyncForData = modificationTimestampAtLastSyncForData, + etag = etag, + etagOnServer = etagOnServer, + sharedViaLink = isSharedViaLink.toInt(), + permissions = permissions, + remoteId = remoteId, + localId = localId, + updateThumbnail = isUpdateThumbnailNeeded.toInt(), + isDownloading = isDownloading.toInt(), + favorite = isFavorite.toInt(), + hidden = shouldHide().toInt(), + isEncrypted = isEncrypted.toInt(), + etagInConflict = etagInConflict, + sharedWithSharee = isSharedWithSharee.toInt(), + mountType = null, + hasPreview = isPreviewAvailable.toInt(), + unreadCommentsCount = unreadCommentsCount, + ownerId = ownerId, + ownerDisplayName = ownerDisplayName, + note = note, + sharees = gson.toJson(sharees), + richWorkspace = richWorkspace, + metadataSize = gson.toJson(imageDimension), + metadataLivePhoto = linkedFileIdForLivePhoto, + locked = isLocked.toInt(), + lockType = lockType?.value ?: NO_LOCK_TYPE, + lockOwner = lockOwnerId, + lockOwnerDisplayName = lockOwnerDisplayName, + lockOwnerEditor = lockOwnerEditor, + lockTimestamp = lockTimestamp, + lockTimeout = lockTimeout.toInt(), + lockToken = lockToken, + tags = gson.toJson(tags), + metadataGPS = gson.toJson(geoLocation), + e2eCounter = e2eCounter, + internalTwoWaySync = internalFolderSyncTimestamp, + internalTwoWaySyncResult = internalFolderSyncResult, + uploaded = uploadTimestamp, + isReadOnly = isReadOnly.toInt() +) + +fun OCFile.toFolderMetadataUpdate(accountName: String): FolderMetadataUpdate { + val entity = toFileEntity(accountName) + return FolderMetadataUpdate( + id = fileId, + modified = entity.modified, + modifiedAtLastSyncForData = entity.modifiedAtLastSyncForData, + parent = entity.parent, + uploaded = entity.uploaded, + creation = entity.creation, + contentType = entity.contentType, + contentLength = FOLDER_CONTENT_LENGTH, + name = entity.name, + path = entity.path, + pathDecrypted = entity.pathDecrypted, + accountOwner = entity.accountOwner, + isEncrypted = entity.isEncrypted, + isReadOnly = entity.isReadOnly, + lastSyncDate = entity.lastSyncDate, + lastSyncDateForData = entity.lastSyncDateForData, + etag = entity.etag, + etagOnServer = entity.etagOnServer, + sharedViaLink = entity.sharedViaLink, + sharedWithSharee = entity.sharedWithSharee, + permissions = entity.permissions, + remoteId = entity.remoteId, + localId = entity.localId, + favorite = entity.favorite, + hidden = entity.hidden, + unreadCommentsCount = entity.unreadCommentsCount, + ownerId = entity.ownerId, + ownerDisplayName = entity.ownerDisplayName, + note = entity.note, + sharees = entity.sharees, + tags = entity.tags, + richWorkspace = entity.richWorkspace, + internalTwoWaySync = entity.internalTwoWaySync, + internalTwoWaySyncResult = entity.internalTwoWaySyncResult + ) +} + +private fun Boolean.toInt(): Int = if (this) 1 else 0 diff --git a/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java b/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java index a14b7986b730..f9e6e0bda1d8 100644 --- a/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java +++ b/app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java @@ -739,7 +739,6 @@ public void saveNewFile(OCFile newFile) { * Returns a {@link ContentValues} filled with values that are common to both files and folders * * @see #createContentValuesForFile(OCFile) - * @see #createContentValuesForFolder(OCFile) */ @SuppressFBWarnings("CE") private ContentValues createContentValuesBase(OCFile fileOrFolder) { @@ -779,26 +778,13 @@ private ContentValues createContentValuesBase(OCFile fileOrFolder) { return cv; } - /** - * Returns a {@link ContentValues} filled with values for a folder - * - * @see #createContentValuesForFile(OCFile) - * @see #createContentValuesBase(OCFile) - */ - public ContentValues createContentValuesForFolder(OCFile folder) { - final ContentValues cv = createContentValuesBase(folder); - cv.put(ProviderTableMeta.FILE_CONTENT_LENGTH, 0); - return cv; - } - /** * Returns a {@link ContentValues} filled with values for a file * - * @see #createContentValuesForFolder(OCFile) * @see #createContentValuesBase(OCFile) */ @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE") - public ContentValues createContentValuesForFile(OCFile file) { + private ContentValues createContentValuesForFile(OCFile file) { final ContentValues cv = createContentValuesBase(file); cv.put(ProviderTableMeta.FILE_CONTENT_LENGTH, file.getFileLength()); cv.put(ProviderTableMeta.FILE_ENCRYPTED_NAME, file.getEncryptedFileName()); diff --git a/app/src/test/java/com/nextcloud/utils/extensions/FileStorageManagerTests.kt b/app/src/test/java/com/nextcloud/utils/extensions/FileStorageManagerTests.kt index 20a465923c24..6d92d1452bad 100644 --- a/app/src/test/java/com/nextcloud/utils/extensions/FileStorageManagerTests.kt +++ b/app/src/test/java/com/nextcloud/utils/extensions/FileStorageManagerTests.kt @@ -6,8 +6,8 @@ */ package com.nextcloud.utils.extensions -import android.content.ContentProviderOperation import android.content.ContentResolver +import android.content.Context import android.content.ContentUris import android.content.ContentValues import android.database.Cursor @@ -47,18 +47,22 @@ class FileStorageManagerTests { companion object { private const val ACCOUNT_NAME = "user@nextcloud.example.com" - private const val AUTHORITY = "org.nextcloud" private const val FOLDER_ID = 3L private const val OTHER_FOLDER_ID = 4L private const val FOLDER_PATH = "/folder/" private const val FILE_PATH = "/folder/file.txt" private const val REMOVED_FILE_PATH = "/folder/removed.txt" + private const val REMOVED_FOLDER_PATH = "/folder/removed/" private const val UNKNOWN_FILE_ID = -1L private const val EXISTING_ROW_ID = 7L private const val REMOVED_ROW_ID = 8L + private const val FOLDER_CONTENT_LENGTH = 0L private const val NEW_ROW_ID = 42L + private const val UPDATED_ROW_ID = -1L + private const val MOUNT_TYPE = 2 + private const val NO_LOCK_TYPE = -1 private const val TRANSACTION = "transaction" - private const val BATCH = "batch" + private const val NOTIFY = "notify" } private lateinit var fileDao: FileDao @@ -67,8 +71,6 @@ class FileStorageManagerTests { private lateinit var contentResolver: ContentResolver private lateinit var storageManager: FileDataStorageManager private val events = mutableListOf() - private val deleteBuilder = operationBuilder() - private val updateBuilder = operationBuilder() private val folder = OCFile(FOLDER_PATH).apply { fileId = FOLDER_ID } @Before @@ -76,6 +78,9 @@ class FileStorageManagerTests { fileDao = mockk(relaxed = true) every { fileDao.getFileById(any()) } returns null every { fileDao.getFileByEncryptedRemotePath(any(), any()) } returns null + every { fileDao.upsert(any()) } answers { + if (firstArg().id == null) NEW_ROW_ID else UPDATED_ROW_ID + } writableDatabase = mockk(relaxed = true) database = mockk(relaxed = true) @@ -84,26 +89,20 @@ class FileStorageManagerTests { runTransactionsImmediately() contentResolver = mockk(relaxed = true) - every { contentResolver.applyBatch(any(), any()) } answers { - events.add(BATCH) - emptyArray() - } + every { contentResolver.notifyChange(any(), any()) } answers { events.add(NOTIFY) } + val context = mockk(relaxed = true) + every { context.contentResolver } returns contentResolver mockkStatic(MainApp::class) - every { MainApp.getAppContext() } returns mockk(relaxed = true) - every { MainApp.getAuthority() } returns AUTHORITY + every { MainApp.getAppContext() } returns context mockkObject(NextcloudDatabase.Companion) every { NextcloudDatabase.getInstance(any()) } returns database every { NextcloudDatabase.instance() } returns database mockkConstructor(OfflineOperationsRepository::class) - mockkConstructor(ContentValues::class) mockkStatic(TextUtils::class) every { TextUtils.isEmpty(any()) } answers { arg(0).isNullOrEmpty() } mockkStatic(ContentUris::class) every { ContentUris.withAppendedId(any(), any()) } returns mockk() - mockkStatic(ContentProviderOperation::class) - every { ContentProviderOperation.newDelete(any()) } returns deleteBuilder - every { ContentProviderOperation.newUpdate(any()) } returns updateBuilder val user = mockk(relaxed = true) every { user.accountName } returns ACCOUNT_NAME @@ -115,12 +114,6 @@ class FileStorageManagerTests { unmockkAll() } - private fun operationBuilder(): ContentProviderOperation.Builder = mockk { - every { withSelection(any(), any()) } returns this - every { withValues(any()) } returns this - every { build() } returns mockk() - } - private fun runTransactionsImmediately() { every { database.runInTransaction(any()) } answers { events.add(TRANSACTION) @@ -137,9 +130,12 @@ class FileStorageManagerTests { every { database.runInTransaction(any>()) } returns null } - private fun existingRow(): FileEntity = mockk { every { id } returns EXISTING_ROW_ID } + private fun existingRow(): FileEntity = mockk { + every { id } returns EXISTING_ROW_ID + every { mountType } returns MOUNT_TYPE + } - private fun stubInsert() { + private fun stubProviderInsert() { every { writableDatabase.insert(ProviderTableMeta.FILE_TABLE_NAME, SQLiteDatabase.CONFLICT_NONE, any()) } returns NEW_ROW_ID @@ -150,26 +146,26 @@ class FileStorageManagerTests { } private fun verifyUpdatedRow(rowId: Long) { - verify(exactly = 1) { - writableDatabase.update( - ProviderTableMeta.FILE_TABLE_NAME, - SQLiteDatabase.CONFLICT_NONE, - any(), - "${ProviderTableMeta._ID} = ?", - match { it.contentEquals(arrayOf(rowId)) } - ) - } + verify(exactly = 1) { fileDao.upsert(match { it.id == rowId && it.mountType == MOUNT_TYPE }) } + } + + private fun removedFile(): OCFile = OCFile(REMOVED_FILE_PATH).apply { + fileId = REMOVED_ROW_ID + parentId = FOLDER_ID + } + + private fun verifyNoInsert() { + verify(exactly = 0) { fileDao.upsert(match { it.id == null }) } } @Test fun `saveFolder inserts file and assigns generated id when no row matches id or path`() { - stubInsert() val file = OCFile(FILE_PATH) saveFolder(updatedFiles = listOf(file)) assertEquals(NEW_ROW_ID, file.fileId) - verify(exactly = 0) { writableDatabase.update(any(), any(), any(), any(), any()) } + verify(exactly = 1) { fileDao.upsert(match { it.id == null && it.mountType == null }) } } @Test @@ -179,7 +175,7 @@ class FileStorageManagerTests { saveFolder(updatedFiles = listOf(OCFile(FILE_PATH).apply { fileId = EXISTING_ROW_ID })) verifyUpdatedRow(EXISTING_ROW_ID) - verify(exactly = 0) { writableDatabase.insert(any(), any(), any()) } + verifyNoInsert() } @Test @@ -189,70 +185,73 @@ class FileStorageManagerTests { saveFolder(updatedFiles = listOf(OCFile(FILE_PATH).apply { fileId = UNKNOWN_FILE_ID })) verifyUpdatedRow(EXISTING_ROW_ID) - verify(exactly = 0) { writableDatabase.insert(any(), any(), any()) } + verifyNoInsert() } @Test fun `saveFolder updates instead of inserting when the same folder is saved twice`() { - stubInsert() saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) every { fileDao.getFileByEncryptedRemotePath(FILE_PATH, ACCOUNT_NAME) } returns existingRow() saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) - verify(exactly = 1) { writableDatabase.insert(any(), any(), any()) } + verify(exactly = 1) { fileDao.upsert(match { it.id == null }) } verifyUpdatedRow(EXISTING_ROW_ID) } @Test - fun `saveFolder stores children under the saved folder`() { - stubInsert() + fun `saveFolder stores children under the saved folder for the current account`() { + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH).apply { parentId = OTHER_FOLDER_ID })) - saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) - - verify { anyConstructed().put(ProviderTableMeta.FILE_PARENT, FOLDER_ID) } + verify(exactly = 1) { + fileDao.upsert(match { it.parent == FOLDER_ID && it.accountOwner == ACCOUNT_NAME }) + } } @Test - fun `saveFolder reads and writes children only inside a transaction`() { + fun `saveFolder reads and writes the database only inside a transaction`() { skipTransactions() - saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) + saveFolder(updatedFiles = listOf(OCFile(FILE_PATH)), filesToRemove = listOf(removedFile())) verify(exactly = 0) { fileDao.getFileById(any()) } verify(exactly = 0) { fileDao.getFileByEncryptedRemotePath(any(), any()) } - verify(exactly = 0) { writableDatabase.insert(any(), any(), any()) } - verify(exactly = 0) { writableDatabase.update(any(), any(), any(), any(), any()) } + verify(exactly = 0) { fileDao.upsert(any()) } + verify(exactly = 0) { fileDao.deleteFile(any(), any(), any()) } + verify(exactly = 0) { fileDao.updateFolderMetadata(any()) } } @Test - fun `saveFolder commits children before applying removals and folder update`() { - stubInsert() - + fun `saveFolder notifies file observers after the transaction`() { saveFolder(updatedFiles = listOf(OCFile(FILE_PATH))) - assertEquals(listOf(TRANSACTION, BATCH), events) + assertEquals(listOf(TRANSACTION, NOTIFY), events) + verify(exactly = 1) { contentResolver.notifyChange(ProviderTableMeta.CONTENT_URI, null) } } @Test fun `saveFolder deletes only removed files whose parent is the saved folder`() { - val removedChild = OCFile(REMOVED_FILE_PATH).apply { + val fileOfOtherFolder = OCFile("/other/removed.txt").apply { parentId = OTHER_FOLDER_ID } + + saveFolder(filesToRemove = listOf(removedFile(), fileOfOtherFolder)) + + verify(exactly = 1) { fileDao.deleteFile(REMOVED_ROW_ID, ACCOUNT_NAME, REMOVED_FILE_PATH) } + verify(exactly = 1) { fileDao.deleteFile(any(), any(), any()) } + verify(exactly = 0) { fileDao.deleteFolder(any(), any(), any()) } + } + + @Test + fun `saveFolder deletes a removed folder together with its descendants`() { + val removedFolder = OCFile(REMOVED_FOLDER_PATH).apply { fileId = REMOVED_ROW_ID parentId = FOLDER_ID + setFolder() } - val fileOfOtherFolder = OCFile("/other/removed.txt").apply { parentId = OTHER_FOLDER_ID } - saveFolder(filesToRemove = listOf(removedChild, fileOfOtherFolder)) + saveFolder(filesToRemove = listOf(removedFolder)) - verify(exactly = 1) { ContentProviderOperation.newDelete(any()) } - verify(exactly = 1) { ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, REMOVED_ROW_ID) } - verify(exactly = 1) { - deleteBuilder.withSelection( - "${ProviderTableMeta.FILE_ACCOUNT_OWNER} = ? AND ${ProviderTableMeta.FILE_PATH} = ?", - match { it.contentEquals(arrayOf(ACCOUNT_NAME, REMOVED_FILE_PATH)) } - ) - } - verify(exactly = 1) { contentResolver.applyBatch(AUTHORITY, match { it.size == 2 }) } + verify(exactly = 1) { fileDao.deleteFolder(REMOVED_ROW_ID, ACCOUNT_NAME, REMOVED_FOLDER_PATH) } + verify(exactly = 0) { fileDao.deleteFile(any(), any(), any()) } } @Test @@ -270,14 +269,17 @@ class FileStorageManagerTests { } @Test - fun `saveFolder updates folder row by its id`() { + fun `saveFolder updates folder metadata by its id with empty content length`() { saveFolder() - verify(exactly = 1) { ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI) } verify(exactly = 1) { - updateBuilder.withSelection( - "${ProviderTableMeta._ID} = ?", - match { it.contentEquals(arrayOf(FOLDER_ID.toString())) } + fileDao.updateFolderMetadata( + match { + it.id == FOLDER_ID && + it.path == FOLDER_PATH && + it.accountOwner == ACCOUNT_NAME && + it.contentLength == FOLDER_CONTENT_LENGTH + } ) } } @@ -287,7 +289,40 @@ class FileStorageManagerTests { storageManager.deleteAllFiles() verify(exactly = 1) { fileDao.deleteAllFiles(ACCOUNT_NAME) } - verify(exactly = 0) { contentResolver.delete(any(), any(), any()) } + } + + @Test + fun `toFileEntity converts flags to integers and keeps missing lock type as -1`() { + val file = OCFile(FILE_PATH).apply { + fileId = EXISTING_ROW_ID + isFavorite = true + isEncrypted = false + } + + val entity = file.toFileEntity(ACCOUNT_NAME) + + assertEquals(EXISTING_ROW_ID, entity.id) + assertEquals(FILE_PATH, entity.path) + assertEquals(ACCOUNT_NAME, entity.accountOwner) + assertEquals(1, entity.favorite) + assertEquals(0, entity.isEncrypted) + assertEquals(NO_LOCK_TYPE, entity.lockType) + assertEquals(null, entity.mountType) + } + + @Test + fun `toFileEntity leaves id empty for files that are not stored yet`() { + val entity = OCFile(FILE_PATH).apply { fileId = UNKNOWN_FILE_ID }.toFileEntity(ACCOUNT_NAME) + + assertEquals(null, entity.id) + } + + @Test + fun `toFileEntity serializes metadata with gson`() { + val entity = OCFile(FILE_PATH).toFileEntity(ACCOUNT_NAME) + + assertEquals("null", entity.metadataSize) + assertEquals("null", entity.metadataGPS) } @Test @@ -311,7 +346,7 @@ class FileStorageManagerTests { val cursor = mockk(relaxed = true) every { cursor.moveToFirst() } returns false every { writableDatabase.query(any()) } returns cursor - stubInsert() + stubProviderInsert() val rowIds = captureAppendedRowIds() FileContentProvider().upsertSingleFile(