Merge pull request #4354 from owncloud/feature/improve_av_offline_per…

…formance Improve av. offline performance
owncloud · Apr 25, 2024 · d8c04d7 · d8c04d7
2 parents 1be13d1 + 1baca2c
commit d8c04d7
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -41,6 +41,7 @@ ownCloud admins and users.
 * Enhancement - Improvements in Manage Accounts view: [#4148](https://github.com/owncloud/android/issues/4148)
 * Enhancement - New setting for manual removal of local storage: [#4174](https://github.com/owncloud/android/issues/4174)
 * Enhancement - New setting for automatic removal of local files: [#4175](https://github.com/owncloud/android/issues/4175)
+* Enhancement - Avoid unnecessary requests when an av. offline folder is refreshed: [#4197](https://github.com/owncloud/android/issues/4197)
 * Enhancement - Unit tests for repository classes - Part 1: [#4232](https://github.com/owncloud/android/issues/4232)
 * Enhancement - Add a warning in http connections: [#4284](https://github.com/owncloud/android/issues/4284)
 * Enhancement - Make dialog more Android-alike: [#4303](https://github.com/owncloud/android/issues/4303)
@@ -144,6 +145,15 @@ ownCloud admins and users.
    https://github.com/owncloud/android/issues/4175
    https://github.com/owncloud/android/pull/4320
 
+* Enhancement - Avoid unnecessary requests when an av. offline folder is refreshed: [#4197](https://github.com/owncloud/android/issues/4197)
+
+   The available offline folders will only be refreshed when their eTag from the
+   server and the corresponding one of the local database are different, avoiding
+   sending unnecessary request.
+
+   https://github.com/owncloud/android/issues/4197
+   https://github.com/owncloud/android/pull/4354
+
 * Enhancement - Unit tests for repository classes - Part 1: [#4232](https://github.com/owncloud/android/issues/4232)
 
    Unit tests for OCAppRegistryRepository, OCAuthenticationRepository and

diff --git a/changelog/unreleased/4354 b/changelog/unreleased/4354
@@ -0,0 +1,7 @@
+Enhancement: Avoid unnecessary requests when an av. offline folder is refreshed
+
+The available offline folders will only be refreshed when their eTag from the server and the corresponding one of the local database are different,
+avoiding sending unnecessary request.
+
+https://github.com/owncloud/android/issues/4197
+https://github.com/owncloud/android/pull/4354
diff --git a/...App/src/main/java/com/owncloud/android/presentation/releasenotes/ReleaseNotesViewModel.kt b/...App/src/main/java/com/owncloud/android/presentation/releasenotes/ReleaseNotesViewModel.kt
@@ -110,7 +110,11 @@ class ReleaseNotesViewModel(
                 subtitle = R.string.release_notes_4_3_0_subtitle_display_name_truncation,
                 type = ReleaseNoteType.BUGFIX
             ),
+            ReleaseNote(
+                title = R.string.release_notes_4_3_0_title_improve_available_offline_performance,
+                subtitle = R.string.release_notes_4_3_0_subtitle_improve_available_offline_performance,
+                type = ReleaseNoteType.ENHANCEMENT
+            ),
         )
     }
 }
-
diff --git a/owncloudApp/src/main/res/values/strings.xml b/owncloudApp/src/main/res/values/strings.xml
@@ -750,6 +750,8 @@
     <string name="release_notes_4_3_0_subtitle_password_generator">Passwords for public links can be now generated via a new generator in Infinite Scale accounts</string>
     <string name="release_notes_4_3_0_title_display_name_truncation">Resolved the bug where long display names were truncated incorrectly</string>
     <string name="release_notes_4_3_0_subtitle_display_name_truncation">Display names are now properly truncated in the middle with ellipsis (…) to maintain readability</string>
+    <string name="release_notes_4_3_0_title_improve_available_offline_performance">Improvements in available offline performance</string>
+    <string name="release_notes_4_3_0_subtitle_improve_available_offline_performance">Files contained in an available offline folder will be refreshed only if they have been modified</string>
 
     <!-- Open in web -->
     <string name="ic_action_open_in_web">Open in web</string>

diff --git a/...loudData/src/main/java/com/owncloud/android/data/files/datasources/LocalFileDataSource.kt b/...loudData/src/main/java/com/owncloud/android/data/files/datasources/LocalFileDataSource.kt
@@ -49,7 +49,7 @@ interface LocalFileDataSource {
     fun getFilesWithLastUsageOlderThanGivenTime(milliseconds: Long): List<OCFile>
     fun moveFile(sourceFile: OCFile, targetFolder: OCFile, finalRemotePath: String, finalStoragePath: String)
     fun copyFile(sourceFile: OCFile, targetFolder: OCFile, finalRemotePath: String, remoteId: String, replace: Boolean?)
-    fun saveFilesInFolderAndReturnThem(listOfFiles: List<OCFile>, folder: OCFile): List<OCFile>
+    fun saveFilesInFolderAndReturnTheFilesThatChanged(listOfFiles: List<OCFile>, folder: OCFile): List<OCFile>
     fun saveFile(file: OCFile)
     fun saveConflict(fileId: Long, eTagInConflict: String)
     fun cleanConflict(fileId: Long)

diff --git a/.../java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSource.kt b/.../java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSource.kt
@@ -153,9 +153,9 @@ class OCLocalFileDataSource(
         )
     }
 
-    override fun saveFilesInFolderAndReturnThem(listOfFiles: List<OCFile>, folder: OCFile): List<OCFile> {
+    override fun saveFilesInFolderAndReturnTheFilesThatChanged(listOfFiles: List<OCFile>, folder: OCFile): List<OCFile> {
         // TODO: If it is root, add 0 as parent Id
-        val folderContent = fileDao.insertFilesInFolderAndReturnThem(
+        val folderContent = fileDao.insertFilesInFolderAndReturnTheFilesThatChanged(
             folder = folder.toEntity(),
             folderContent = listOfFiles.map { it.toEntity() }
         )

diff --git a/owncloudData/src/main/java/com/owncloud/android/data/files/db/FileDao.kt b/owncloudData/src/main/java/com/owncloud/android/data/files/db/FileDao.kt
@@ -206,7 +206,7 @@ interface FileDao {
      * return folder content
      */
     @Transaction
-    fun insertFilesInFolderAndReturnThem(
+    fun insertFilesInFolderAndReturnTheFilesThatChanged(
         folder: OCFileEntity,
         folderContent: List<OCFileEntity>,
     ): List<OCFileEntity> {
@@ -223,7 +223,13 @@ interface FileDao {
                 availableOfflineStatus = getNewAvailableOfflineStatus(folder.availableOfflineStatus, fileToInsert.availableOfflineStatus)
             })
         }
-        return getFolderContent(folderId)
+        val folderContentLocal = getFolderContent(folderId)
+
+        return folderContentLocal.filter { localFile ->
+            folderContent.any { changedFile ->
+                localFile.remoteId == changedFile.remoteId
+            }
+        }
     }
 
     @Transaction

diff --git a/owncloudData/src/main/java/com/owncloud/android/data/files/repository/OCFileRepository.kt b/owncloudData/src/main/java/com/owncloud/android/data/files/repository/OCFileRepository.kt
@@ -65,7 +65,7 @@ class OCFileRepository(
             accountName = parentFolder.owner,
             spaceWebDavUrl = spaceWebDavUrl,
         ).also {
-            localFileDataSource.saveFilesInFolderAndReturnThem(
+            localFileDataSource.saveFilesInFolderAndReturnTheFilesThatChanged(
                 folder = parentFolder,
                 listOfFiles = listOf(
                     OCFile(
@@ -424,7 +424,7 @@ class OCFileRepository(
                                 if (remoteFolder.isAvailableOffline) AVAILABLE_OFFLINE_PARENT else NOT_AVAILABLE_OFFLINE
 
                         })
-                } else {
+                } else if (localChildToSync.etag != remoteChild.etag || localChildToSync.localModificationTimestamp > remoteChild.lastSyncDateForData!!) {
                     // File exists in the database, we need to check several stuff.
                     folderContentUpdated.add(
                         remoteChild.apply {
@@ -461,7 +461,7 @@ class OCFileRepository(
             remoteFolder.etagInConflict = null
         }
 
-        return localFileDataSource.saveFilesInFolderAndReturnThem(
+        return localFileDataSource.saveFilesInFolderAndReturnTheFilesThatChanged(
             folder = remoteFolder,
             listOfFiles = folderContentUpdated
         )
@@ -598,7 +598,7 @@ class OCFileRepository(
     private fun deleteLocalFile(ocFile: OCFile, onlyFromLocalStorage: Boolean) {
         localStorageProvider.deleteLocalFile(ocFile)
         if (onlyFromLocalStorage) {
-            localFileDataSource.saveFile(ocFile.copy(storagePath = null, etagInConflict = null, lastUsage = null))
+            localFileDataSource.saveFile(ocFile.copy(storagePath = null, etagInConflict = null, lastUsage = null, etag = null))
         } else {
             localFileDataSource.deleteFile(ocFile.id!!)
         }

diff --git a/...a/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSourceTest.kt b/...a/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSourceTest.kt
@@ -4,8 +4,9 @@
  * @author Abel García de Prada
  * @author Aitor Ballesteros Pavón
  * @author Juan Carlos Garrote Gascón
+ * @author Aitor Ballesteros Pavón
  *
- * Copyright (C) 2023 ownCloud GmbH.
+ * Copyright (C) 2024 ownCloud GmbH.
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2,
@@ -524,25 +525,25 @@ class OCLocalFileDataSourceTest {
     }
 
     @Test
-    fun `saveFilesInFolderAndReturnThem saves a list of OCFile and returns them`() {
-        every { fileDao.insertFilesInFolderAndReturnThem(OC_FOLDER_ENTITY, listOf(OC_FILE_ENTITY)) } returns listOf(OC_FILE_ENTITY)
+    fun `saveFilesInFolderAndReturnTheFilesThatChanged saves a list of OCFile and returns only the changed files`() {
+        every { fileDao.insertFilesInFolderAndReturnTheFilesThatChanged(OC_FOLDER_ENTITY, listOf(OC_FILE_ENTITY)) } returns listOf(OC_FILE_ENTITY)
 
-        val result = ocLocalFileDataSource.saveFilesInFolderAndReturnThem(listOf(OC_FILE), OC_FOLDER)
+        val result = ocLocalFileDataSource.saveFilesInFolderAndReturnTheFilesThatChanged(listOf(OC_FILE), OC_FOLDER)
 
         assertEquals(listOf(OC_FILE), result)
 
-        verify(exactly = 1) { fileDao.insertFilesInFolderAndReturnThem(OC_FOLDER_ENTITY, listOf(OC_FILE_ENTITY)) }
+        verify(exactly = 1) { fileDao.insertFilesInFolderAndReturnTheFilesThatChanged(OC_FOLDER_ENTITY, listOf(OC_FILE_ENTITY)) }
     }
 
     @Test
-    fun `saveFilesInFolderAndReturnThem returns an empty list when DAO returns an empty list`() {
-        every { fileDao.insertFilesInFolderAndReturnThem(OC_FOLDER_ENTITY, emptyList()) } returns emptyList()
+    fun `saveFilesInFolderAndReturnTheFilesThatChanged returns an empty list when DAO returns an empty list`() {
+        every { fileDao.insertFilesInFolderAndReturnTheFilesThatChanged(OC_FOLDER_ENTITY, emptyList()) } returns emptyList()
 
-        val result = ocLocalFileDataSource.saveFilesInFolderAndReturnThem(emptyList(), OC_FOLDER)
+        val result = ocLocalFileDataSource.saveFilesInFolderAndReturnTheFilesThatChanged(emptyList(), OC_FOLDER)
 
         assertEquals(emptyList<OCFile>(), result)
 
-        verify(exactly = 1) { fileDao.insertFilesInFolderAndReturnThem(OC_FOLDER_ENTITY, emptyList()) }
+        verify(exactly = 1) { fileDao.insertFilesInFolderAndReturnTheFilesThatChanged(OC_FOLDER_ENTITY, emptyList()) }
     }
 
     @Test

diff --git a/...loudData/src/test/java/com/owncloud/android/data/files/repository/OCFileRepositoryTest.kt b/...loudData/src/test/java/com/owncloud/android/data/files/repository/OCFileRepositoryTest.kt
@@ -77,7 +77,7 @@ class OCFileRepositoryTest {
 
         verify(exactly = 1) {
             remoteFileDataSource.createFolder(any(), false, false, OC_ACCOUNT_NAME, null)
-            localFileDataSource.saveFilesInFolderAndReturnThem(any(), OC_FOLDER)
+            localFileDataSource.saveFilesInFolderAndReturnTheFilesThatChanged(any(), OC_FOLDER)
         }
     }
 
@@ -93,7 +93,7 @@ class OCFileRepositoryTest {
             remoteFileDataSource.createFolder(any(), false, false, OC_ACCOUNT_NAME, null)
         }
         verify(exactly = 0) {
-            localFileDataSource.saveFilesInFolderAndReturnThem(any(), OC_FOLDER)
+            localFileDataSource.saveFilesInFolderAndReturnTheFilesThatChanged(any(), OC_FOLDER)
         }
     }
 
@@ -302,7 +302,7 @@ class OCFileRepositoryTest {
 
         verify(exactly = 1) {
             remoteFileDataSource.refreshFolder(folderToFetch.remotePath, OC_ACCOUNT_NAME)
-            localFileDataSource.saveFilesInFolderAndReturnThem(
+            localFileDataSource.saveFilesInFolderAndReturnTheFilesThatChanged(
                 listOfFiles = listOfFilesRetrieved.drop(1),
                 folder = listOfFilesRetrieved.first()
             )
@@ -321,7 +321,7 @@ class OCFileRepositoryTest {
             remoteFileDataSource.refreshFolder(OC_FOLDER.remotePath, OC_ACCOUNT_NAME)
         }
         verify(exactly = 0) {
-            localFileDataSource.saveFilesInFolderAndReturnThem(any(), any())
+            localFileDataSource.saveFilesInFolderAndReturnTheFilesThatChanged(any(), any())
         }
     }