Merge pull request #16810 from nextcloud/fix/shared-file-list-metadata

fix(share-list-fragment): overriding metadata with wrong values

Author: alperozturk96

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<OCShare>): List<OCFile> = shares
+    fun buildOCFilesFromShares(shares: List<OCShare>, storageManager: FileDataStorageManager): List<OCFile> = 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<OCFile>,
         data: List<Any>,
-        storageManager: FileDataStorageManager?,
+        storageManager: FileDataStorageManager,
         accountName: String
     ): List<OCFile> = 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,49 @@ 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<OCShare>): OCFile {
+    private fun buildOCFile(path: String, shares: List<OCShare>, storageManager: FileDataStorageManager): OCFile {
         require(shares.all { it.path == path })
-        // common attributes
+
         val firstShare = shares.first()
-        val file = OCFile(path).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
-                    )
-                }
-        }
+        val firstShareTimestamp = shares.minOf { it.sharedDate * MILLIS_PER_SECOND }
+
+        val linkShares = shares.filter { it.shareType in LINK_SHARE_TYPES }
+        val userShares = shares.filter { it.shareType !in LINK_SHARE_TYPES }
+
+        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 {
+        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
+    }
 }

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();
     }
 

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<List<Any>>,
     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<FileFragment.ContainerActivity> = WeakReference(containerActivity)
     private val fragmentReference: WeakReference<OCFileListFragment> = 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<OCFile> {
-        val storage = fileDataStorageManager ?: return emptyList()
-        return if (searchType == SearchRemoteOperation.SearchType.SHARED_FILTER) {
-            storage.fileDao
+    private suspend fun loadCachedDbFiles(searchType: SearchRemoteOperation.SearchType): List<OCFile> =
+        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<List<Any>>? {
@@ -202,7 +195,6 @@ class OCFileListSearchTask(
     @Suppress("DEPRECATION")
     private suspend fun parseAndSaveVirtuals(data: List<Any>, fragment: OCFileListFragment) =
         withContext(Dispatchers.IO) {
-            val fileDataStorageManager = fileDataStorageManager ?: return@withContext
             val activity = fragment.activity ?: return@withContext
             val now = System.currentTimeMillis()
 
@@ -219,16 +211,16 @@ 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(
                             ocFile,
                             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")

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)

app/src/test/java/com/owncloud/android/ui/adapter/OCShareToOCFileConverterTest.kt

@@ -7,13 +7,40 @@
  */
 package com.owncloud.android.ui.adapter
 
+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 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 +51,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 +83,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 +126,7 @@ class OCShareToOCFileConverterTest {
                 }
         )
 
-        val result = OCShareToOCFileConverter.buildOCFilesFromShares(shares)
+        val result = OCShareToOCFileConverter.buildOCFilesFromShares(shares, storageManager)
 
         assertEquals("Wrong file list size", 2, result.size)