Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve av. offline performance #4197

Closed
10 tasks done
jesmrec opened this issue Oct 20, 2023 · 1 comment · Fixed by #4354
Closed
10 tasks done

Improve av. offline performance #4197

jesmrec opened this issue Oct 20, 2023 · 1 comment · Fixed by #4354

Comments

@jesmrec
Copy link
Collaborator

jesmrec commented Oct 20, 2023

Checking issue #4192, i notice av. offline behaves in a non-performant way:

  1. Set a folder with files inside as av. offline -> everything marked as av. offline
  2. Kill the app
  3. Open the app

Current: one PROPFIND request sent for every item inside the folder recursively. Even if no changes happened in the tree structure. In case a deep structure or populated folders, many requests are being wasted making the app non-performant.

Expected: Checking folder etag is enough to know whether the av. offline folder has changed or not. Only if etag changed, sync operation is triggered.

TASKS

  • Research (if needed)
  • Create branch feature/improve_available_offline_performance
  • Development tasks
    • Etag condition in refreshFolder()
    • Release notes
    • Calens
  • Code review and apply changes requested
  • Design test plan
  • QA
  • Merge branch feature/feature_name into master
@Aitorbp
Copy link
Collaborator

Aitorbp commented Apr 4, 2024

For this issue I have decided to make a change in the refreshFolder method inside the FileRepository. Currently within the refresh method all files are being returned, both those that have been modified from the server and those that have not.
Theoretically, we are only interested in refreshing those files that have been modified. Therefore, the etag we have on the server will be compared with the etag of the local database to extract only those files that have been modified, thus avoiding unnecessary requests.
Also, this change implies a change in the behaviour of the method, which is used by a number of classes in the application. At the generic level, it is used in the SynchronizeFolderUseCase and in the getFileFromRemoteId (which is only used in the ManageDeepLinkUseCase).
To facilitate the QA work @jesmrec, here are the uses of SynchronizeFolderUseCase in the application:

  • When we try to choose the destination path where to make an auto-upload
  • When we copy a file, we choose the space, it makes a sync of the content.
  • When we do a move operation
  • When we open the application
  • When we click on set available offline on a folder/file in the operations menu
  • When we click on unset available offline on a folder/file in the operations menu
  • When switching accounts
  • When clicking on synchronize in the operations list
  • When entering a folder
  • When exiting the folder, by clicking on back
  • when cancelling a copy/move file operation
  • When triggering the AccountDiscoveryWorker worker
  • When the AvailableOfflinePeriodicWorker is activated, the files that are available offline are synchronised.
  • When we receive an external file and we choose the destination place.

Places where it is not clear to me what it does:
DocumentsStorageProvider: I think it syncs the Childs documents of a parents. I don't know how to emulate the concrete case.
FIleSyncAdapter: I have the theory that a sync is made when a synchronisation is cancelled. Not emulated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants