diff --git a/CHANGELOG.md b/CHANGELOG.md index 7baa3627776..8af80a0e9a5 100644 --- 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 new file mode 100644 index 00000000000..94c800e9c8f --- /dev/null +++ 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/owncloudApp/src/main/java/com/owncloud/android/presentation/releasenotes/ReleaseNotesViewModel.kt b/owncloudApp/src/main/java/com/owncloud/android/presentation/releasenotes/ReleaseNotesViewModel.kt index 6a551141671..128bff6b34f 100644 --- a/owncloudApp/src/main/java/com/owncloud/android/presentation/releasenotes/ReleaseNotesViewModel.kt +++ b/owncloudApp/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 index 5e3510cec49..8dcdd19d184 100644 --- a/owncloudApp/src/main/res/values/strings.xml +++ b/owncloudApp/src/main/res/values/strings.xml @@ -750,6 +750,8 @@ Passwords for public links can be now generated via a new generator in Infinite Scale accounts Resolved the bug where long display names were truncated incorrectly Display names are now properly truncated in the middle with ellipsis (…) to maintain readability + Improvements in available offline performance + Files contained in an available offline folder will be refreshed only if they have been modified Open in web diff --git a/owncloudData/src/main/java/com/owncloud/android/data/files/datasources/LocalFileDataSource.kt b/owncloudData/src/main/java/com/owncloud/android/data/files/datasources/LocalFileDataSource.kt index 4cc9cd764d9..c8a1e8e405c 100644 --- a/owncloudData/src/main/java/com/owncloud/android/data/files/datasources/LocalFileDataSource.kt +++ b/owncloudData/src/main/java/com/owncloud/android/data/files/datasources/LocalFileDataSource.kt @@ -49,7 +49,7 @@ interface LocalFileDataSource { fun getFilesWithLastUsageOlderThanGivenTime(milliseconds: Long): List 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, folder: OCFile): List + fun saveFilesInFolderAndReturnTheFilesThatChanged(listOfFiles: List, folder: OCFile): List fun saveFile(file: OCFile) fun saveConflict(fileId: Long, eTagInConflict: String) fun cleanConflict(fileId: Long) diff --git a/owncloudData/src/main/java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSource.kt b/owncloudData/src/main/java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSource.kt index 77eb83d269a..1ec4bfd2b22 100644 --- a/owncloudData/src/main/java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSource.kt +++ b/owncloudData/src/main/java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSource.kt @@ -153,9 +153,9 @@ class OCLocalFileDataSource( ) } - override fun saveFilesInFolderAndReturnThem(listOfFiles: List, folder: OCFile): List { + override fun saveFilesInFolderAndReturnTheFilesThatChanged(listOfFiles: List, folder: OCFile): List { // 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 index a1a0d0daef8..3bed19eb04f 100644 --- 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, ): List { @@ -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 index c0fd90a45c0..d32d42c7e1b 100644 --- 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/owncloudData/src/test/java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSourceTest.kt b/owncloudData/src/test/java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSourceTest.kt index d9b932c020c..55c6766443a 100644 --- a/owncloudData/src/test/java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSourceTest.kt +++ b/owncloudData/src/test/java/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(), result) - verify(exactly = 1) { fileDao.insertFilesInFolderAndReturnThem(OC_FOLDER_ENTITY, emptyList()) } + verify(exactly = 1) { fileDao.insertFilesInFolderAndReturnTheFilesThatChanged(OC_FOLDER_ENTITY, emptyList()) } } @Test diff --git a/owncloudData/src/test/java/com/owncloud/android/data/files/repository/OCFileRepositoryTest.kt b/owncloudData/src/test/java/com/owncloud/android/data/files/repository/OCFileRepositoryTest.kt index 4b67bbd5404..eb3fffd4343 100644 --- a/owncloudData/src/test/java/com/owncloud/android/data/files/repository/OCFileRepositoryTest.kt +++ b/owncloudData/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()) } }