From e4ed03920c4ce0814ecaf7164421fbac1c5770e7 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 7 Apr 2026 14:41:40 +0200 Subject: [PATCH 1/3] fix(share-list-fragment): overriding metadata with default values Signed-off-by: alperozturk96 --- .../ui/adapter/OCShareToOCFileConverter.kt | 79 +++++++++++-------- .../ui/fragment/OCFileListFragment.java | 12 ++- .../ui/fragment/OCFileListSearchTask.kt | 34 +++----- .../android/ui/fragment/SharedListFragment.kt | 6 +- .../adapter/OCShareToOCFileConverterTest.kt | 36 ++++++++- 5 files changed, 104 insertions(+), 63 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverter.kt b/app/src/main/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverter.kt index fd17de1605f6..ae425785cf8e 100644 --- a/app/src/main/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverter.kt +++ b/app/src/main/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverter.kt @@ -21,6 +21,7 @@ import java.io.File object OCShareToOCFileConverter { private const val MILLIS_PER_SECOND = 1000 + private val LINK_SHARE_TYPES = setOf(ShareType.PUBLIC_LINK, ShareType.EMAIL) /** * Generates a list of incomplete [OCFile] from a list of [OCShare]. Retrieving OCFile directly by path may fail @@ -33,21 +34,21 @@ object OCShareToOCFileConverter { * * Note: This works only for files shared *by* the user, not files shared *with* the user. */ - fun buildOCFilesFromShares(shares: List): List = shares + fun buildOCFilesFromShares(shares: List, storageManager: FileDataStorageManager): List = shares .filter { !it.path.isNullOrEmpty() } .groupBy { it.path!! } .filterKeys { path -> path.isNotEmpty() && path.startsWith(OCFile.PATH_SEPARATOR) } .map { (path, sharesForPath) -> - buildOcFile(path, sharesForPath) + buildOcFile(path, sharesForPath, storageManager) } .sortedByDescending { it.firstShareTimestamp } suspend fun parseAndSaveShares( cachedFiles: List, data: List, - storageManager: FileDataStorageManager?, + storageManager: FileDataStorageManager, accountName: String ): List = withContext(Dispatchers.IO) { if (data.isEmpty()) { @@ -67,7 +68,7 @@ object OCShareToOCFileConverter { return@withContext cachedFiles } - val files = buildOCFilesFromShares(newShares) + val files = buildOCFilesFromShares(newShares, storageManager) val baseSavePath = FileStorageUtils.getSavePath(accountName) val newFiles = files.map { file -> @@ -79,49 +80,57 @@ object OCShareToOCFileConverter { file.lastSyncDateForData = candidate.lastModified() } } - storageManager?.saveFile(file) + storageManager.saveFile(file) file } - storageManager?.saveShares(newShares, accountName) + storageManager.saveShares(newShares, accountName) (cachedFiles + newFiles).distinctBy { it.remotePath } } - private fun buildOcFile(path: String, shares: List): OCFile { + private fun buildOcFile(path: String, shares: List, storageManager: FileDataStorageManager): OCFile { require(shares.all { it.path == path }) - // common attributes + val firstShare = shares.first() - val file = OCFile(path).apply { + val firstShareTimestamp = shares.minOf { it.sharedDate * MILLIS_PER_SECOND } + + val existingFile = ( + storageManager.getFileByDecryptedRemotePath(path) + ?.applyShareData(firstShare, firstShareTimestamp) + ) + val fileCreatedFromShare = OCFile(path).applyShareData(firstShare, firstShareTimestamp) + val file = existingFile ?: fileCreatedFromShare.apply { decryptedRemotePath = path - ownerId = firstShare.userId - ownerDisplayName = firstShare.ownerDisplayName - isPreviewAvailable = firstShare.isHasPreview - mimeType = firstShare.mimetype - note = firstShare.note - fileId = firstShare.fileSource - remoteId = firstShare.remoteId.toString() - // use first share timestamp as timestamp - firstShareTimestamp = shares.minOf { it.sharedDate * MILLIS_PER_SECOND } - // don't have file length or mod timestamp fileLength = -1 modificationTimestamp = -1 - isFavorite = firstShare.isFavorite } - if (shares.any { it.shareType in listOf(ShareType.PUBLIC_LINK, ShareType.EMAIL) }) { - file.isSharedViaLink = true - } - if (shares.any { it.shareType !in listOf(ShareType.PUBLIC_LINK, ShareType.EMAIL) }) { - file.isSharedWithSharee = true - file.sharees = shares - .filter { it.shareType != ShareType.PUBLIC_LINK && it.shareType != ShareType.EMAIL } - .map { - ShareeUser( - userId = it.userId, - displayName = it.sharedWithDisplayName, - shareType = it.shareType - ) - } + + return file.apply { + isSharedViaLink = shares.any { it.shareType in LINK_SHARE_TYPES } + isSharedWithSharee = shares.any { it.shareType !in LINK_SHARE_TYPES } + if (isSharedWithSharee) { + sharees = shares + .filter { it.shareType !in LINK_SHARE_TYPES } + .map { + ShareeUser( + userId = it.userId, + displayName = it.sharedWithDisplayName, + shareType = it.shareType + ) + } + } } - return file + } + + private fun OCFile.applyShareData(firstShare: OCShare, firstShareTimestamp: Long) = apply { + ownerId = firstShare.userId + ownerDisplayName = firstShare.ownerDisplayName + isPreviewAvailable = firstShare.isHasPreview + mimeType = firstShare.mimetype + note = firstShare.note + fileId = firstShare.fileSource + remoteId = firstShare.remoteId.toString() + this.firstShareTimestamp = firstShareTimestamp + isFavorite = firstShare.isFavorite } } diff --git a/app/src/main/java/com/owncloud/android/ui/fragment/OCFileListFragment.java b/app/src/main/java/com/owncloud/android/ui/fragment/OCFileListFragment.java index 4c08db6fe380..ae434dbebba3 100644 --- a/app/src/main/java/com/owncloud/android/ui/fragment/OCFileListFragment.java +++ b/app/src/main/java/com/owncloud/android/ui/fragment/OCFileListFragment.java @@ -1970,7 +1970,17 @@ protected void handleSearchEvent(SearchEvent event) { remoteOperation = getSearchRemoteOperation(currentUser, event); } - searchTask = new OCFileListSearchTask(mContainerActivity, this, remoteOperation, currentUser, event, SharedListFragment.TASK_TIMEOUT, preferences); + var storageManager = mContainerActivity.getStorageManager(); + if (storageManager == null) { + storageManager = new FileDataStorageManager(currentUser, requireContext().getContentResolver()); + } + + searchTask = new OCFileListSearchTask(this, + remoteOperation, + currentUser, event, + SharedListFragment.TASK_TIMEOUT, + preferences, + storageManager); searchTask.execute(); } diff --git a/app/src/main/java/com/owncloud/android/ui/fragment/OCFileListSearchTask.kt b/app/src/main/java/com/owncloud/android/ui/fragment/OCFileListSearchTask.kt index 67ada35c7978..26a8943b311d 100644 --- a/app/src/main/java/com/owncloud/android/ui/fragment/OCFileListSearchTask.kt +++ b/app/src/main/java/com/owncloud/android/ui/fragment/OCFileListSearchTask.kt @@ -44,24 +44,19 @@ import java.lang.ref.WeakReference @Suppress("LongParameterList", "ReturnCount", "TooGenericExceptionCaught") @SuppressLint("NotifyDataSetChanged") class OCFileListSearchTask( - containerActivity: FileFragment.ContainerActivity, fragment: OCFileListFragment, private val remoteOperation: RemoteOperation>, private val currentUser: User, private val event: SearchEvent, private val taskTimeout: Long, - private val preferences: AppPreferences + private val preferences: AppPreferences, + private val storageManager: FileDataStorageManager ) { companion object { private const val TAG = "OCFileListSearchTask" } - private val activityReference: WeakReference = WeakReference(containerActivity) private val fragmentReference: WeakReference = WeakReference(fragment) - - private val fileDataStorageManager: FileDataStorageManager? - get() = activityReference.get()?.storageManager - private var job: Job? = null @Suppress("TooGenericExceptionCaught", "DEPRECATION", "ReturnCount") @@ -91,13 +86,13 @@ class OCFileListSearchTask( return@launch } - fragment.adapter.prepareForSearchData(fileDataStorageManager, fragment.currentSearchType) + fragment.adapter.prepareForSearchData(storageManager, fragment.currentSearchType) val newList = if (searchType == SearchType.SHARED_FILTER) { OCShareToOCFileConverter.parseAndSaveShares( sortedFilesInDb, result.resultData ?: listOf(), - fileDataStorageManager, + storageManager, currentUser.accountName ) } else { @@ -126,16 +121,14 @@ class OCFileListSearchTask( fun isFinished(): Boolean = job?.isCompleted == true - private suspend fun loadCachedDbFiles(searchType: SearchRemoteOperation.SearchType): List { - val storage = fileDataStorageManager ?: return emptyList() - return if (searchType == SearchRemoteOperation.SearchType.SHARED_FILTER) { - storage.fileDao + private suspend fun loadCachedDbFiles(searchType: SearchRemoteOperation.SearchType): List = + if (searchType == SearchRemoteOperation.SearchType.SHARED_FILTER) { + storageManager.fileDao .getSharedFiles(currentUser.accountName) } else { - storage.fileDao + storageManager.fileDao .getFavoriteFiles(currentUser.accountName) - }.mapNotNull { storage.createFileInstance(it) } - } + }.mapNotNull { storageManager.createFileInstance(it) } @Suppress("DEPRECATION") private suspend fun fetchRemoteResults(): RemoteOperationResult>? { @@ -202,7 +195,6 @@ class OCFileListSearchTask( @Suppress("DEPRECATION") private suspend fun parseAndSaveVirtuals(data: List, fragment: OCFileListFragment) = withContext(Dispatchers.IO) { - val fileDataStorageManager = fileDataStorageManager ?: return@withContext val activity = fragment.activity ?: return@withContext val now = System.currentTimeMillis() @@ -219,8 +211,8 @@ class OCFileListSearchTask( val remoteFile = obj as? RemoteFile ?: continue var ocFile = FileStorageUtils.fillOCFile(remoteFile) FileStorageUtils.searchForLocalFileInDefaultPath(ocFile, currentUser.accountName) - ocFile = fileDataStorageManager.saveFileWithParent(ocFile, activity) - ocFile = handleEncryptionIfNeeded(ocFile, fileDataStorageManager, activity) + ocFile = storageManager.saveFileWithParent(ocFile, activity) + ocFile = handleEncryptionIfNeeded(ocFile, storageManager, activity) if (fragment.currentSearchType != SearchType.GALLERY_SEARCH && ocFile.isFolder) { RefreshFolderOperation( @@ -228,7 +220,7 @@ class OCFileListSearchTask( now, true, false, - fileDataStorageManager, + storageManager, currentUser, activity ).execute(currentUser, activity) @@ -253,7 +245,7 @@ class OCFileListSearchTask( // Save timestamp + virtual entries preferences.setPhotoSearchTimestamp(System.currentTimeMillis()) - fileDataStorageManager.saveVirtuals(contentValuesList) + storageManager.saveVirtuals(contentValuesList) } @Suppress("DEPRECATION") diff --git a/app/src/main/java/com/owncloud/android/ui/fragment/SharedListFragment.kt b/app/src/main/java/com/owncloud/android/ui/fragment/SharedListFragment.kt index 08e307aeae1e..81b6d422bb46 100644 --- a/app/src/main/java/com/owncloud/android/ui/fragment/SharedListFragment.kt +++ b/app/src/main/java/com/owncloud/android/ui/fragment/SharedListFragment.kt @@ -75,12 +75,12 @@ class SharedListFragment : val fetchResult = ReadFileRemoteOperation(partialFile.remotePath).execute(user, context) if (fetchResult.isSuccess) { val remoteFile = (fetchResult.data[0] as RemoteFile).apply { - val prevETag = mContainerActivity.storageManager.getFileByDecryptedRemotePath(remotePath) + val existingFile = mContainerActivity.storageManager.getFileByDecryptedRemotePath(remotePath) // Use previous eTag if exists to prevent break checkForChanges logic in RefreshFolderOperation. // Otherwise RefreshFolderOperation will show empty list - prevETag?.etag?.let { - etag = prevETag.etag + existingFile?.etag?.let { + etag = it } } val file = FileStorageUtils.fillOCFile(remoteFile) diff --git a/app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt b/app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt index 43c5eee4a39f..c7cdd415a8c1 100644 --- a/app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt +++ b/app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt @@ -7,13 +7,43 @@ */ package com.owncloud.android.ui.adapter +import android.text.TextUtils +import com.owncloud.android.datamodel.FileDataStorageManager import com.owncloud.android.lib.resources.shares.OCShare import com.owncloud.android.lib.resources.shares.ShareType +import com.owncloud.android.ui.activity.ComponentsGetter +import io.mockk.every +import io.mockk.mockkStatic +import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Before import org.junit.Test +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.whenever class OCShareToOCFileConverterTest { + @Mock + lateinit var storageManager: FileDataStorageManager + + @Mock + lateinit var transferServiceGetter: ComponentsGetter + + private lateinit var mocks: AutoCloseable + + @Before + fun setUpMocks() { + mocks = MockitoAnnotations.openMocks(this) + whenever(transferServiceGetter.storageManager) doReturn storageManager + } + + @After + fun tearDownMocks() { + mocks.close() + } + @Test fun testSingleOCShare() { val shares = listOf( @@ -24,7 +54,7 @@ class OCShareToOCFileConverterTest { } ) - val result = OCShareToOCFileConverter.buildOCFilesFromShares(shares) + val result = OCShareToOCFileConverter.buildOCFilesFromShares(shares, storageManager) assertEquals("Wrong file list size", 1, result.size) val ocFile = result[0] @@ -56,7 +86,7 @@ class OCShareToOCFileConverterTest { } ) - val result = OCShareToOCFileConverter.buildOCFilesFromShares(shares) + val result = OCShareToOCFileConverter.buildOCFilesFromShares(shares, storageManager) assertEquals("Wrong file list size", 1, result.size) val ocFile = result[0] @@ -99,7 +129,7 @@ class OCShareToOCFileConverterTest { } ) - val result = OCShareToOCFileConverter.buildOCFilesFromShares(shares) + val result = OCShareToOCFileConverter.buildOCFilesFromShares(shares, storageManager) assertEquals("Wrong file list size", 2, result.size) From 8394fb4e8c623f1a9c18dab6779c4889c8ede2f0 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 7 Apr 2026 15:19:26 +0200 Subject: [PATCH 2/3] fix(share-list-fragment): simplify logic Signed-off-by: alperozturk96 --- .../ui/adapter/OCShareToOCFileConverter.kt | 46 ++++++++----------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverter.kt b/app/src/main/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverter.kt index ae425785cf8e..cd80aa84a0b7 100644 --- a/app/src/main/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverter.kt +++ b/app/src/main/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverter.kt @@ -41,7 +41,7 @@ object OCShareToOCFileConverter { path.isNotEmpty() && path.startsWith(OCFile.PATH_SEPARATOR) } .map { (path, sharesForPath) -> - buildOcFile(path, sharesForPath, storageManager) + buildOCFile(path, sharesForPath, storageManager) } .sortedByDescending { it.firstShareTimestamp } @@ -88,38 +88,30 @@ object OCShareToOCFileConverter { (cachedFiles + newFiles).distinctBy { it.remotePath } } - private fun buildOcFile(path: String, shares: List, storageManager: FileDataStorageManager): OCFile { + private fun buildOCFile(path: String, shares: List, storageManager: FileDataStorageManager): OCFile { require(shares.all { it.path == path }) val firstShare = shares.first() val firstShareTimestamp = shares.minOf { it.sharedDate * MILLIS_PER_SECOND } - val existingFile = ( - storageManager.getFileByDecryptedRemotePath(path) - ?.applyShareData(firstShare, firstShareTimestamp) - ) - val fileCreatedFromShare = OCFile(path).applyShareData(firstShare, firstShareTimestamp) - val file = existingFile ?: fileCreatedFromShare.apply { - decryptedRemotePath = path - fileLength = -1 - modificationTimestamp = -1 - } + val linkShares = shares.filter { it.shareType in LINK_SHARE_TYPES } + val userShares = shares.filter { it.shareType !in LINK_SHARE_TYPES } - return file.apply { - isSharedViaLink = shares.any { it.shareType in LINK_SHARE_TYPES } - isSharedWithSharee = shares.any { it.shareType !in LINK_SHARE_TYPES } - if (isSharedWithSharee) { - sharees = shares - .filter { it.shareType !in LINK_SHARE_TYPES } - .map { - ShareeUser( - userId = it.userId, - displayName = it.sharedWithDisplayName, - shareType = it.shareType - ) - } - } - } + val file = (storageManager.getFileByDecryptedRemotePath(path) ?: newOCFile(path)) + + file.applyShareData(firstShare, firstShareTimestamp) + + file.isSharedViaLink = linkShares.isNotEmpty() + file.isSharedWithSharee = userShares.isNotEmpty() + file.sharees = userShares.map { ShareeUser(it.userId, it.sharedWithDisplayName, it.shareType) } + + return file + } + + private fun newOCFile(path: String) = OCFile(path).also { + it.decryptedRemotePath = path + it.fileLength = -1 + it.modificationTimestamp = -1 } private fun OCFile.applyShareData(firstShare: OCShare, firstShareTimestamp: Long) = apply { From b2bd4e18bb5eed3c67aa7ed048b3d4b5d75c7dd6 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 7 Apr 2026 15:23:34 +0200 Subject: [PATCH 3/3] fix(share-list-fragment): simplify logic Signed-off-by: alperozturk96 --- .../android/ui/adapter/OCShareToOCFileConverterTest.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt b/app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt index c7cdd415a8c1..c27062962451 100644 --- a/app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt +++ b/app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt @@ -7,13 +7,10 @@ */ package com.owncloud.android.ui.adapter -import android.text.TextUtils import com.owncloud.android.datamodel.FileDataStorageManager import com.owncloud.android.lib.resources.shares.OCShare import com.owncloud.android.lib.resources.shares.ShareType import com.owncloud.android.ui.activity.ComponentsGetter -import io.mockk.every -import io.mockk.mockkStatic import org.junit.After import org.junit.Assert.assertEquals import org.junit.Before