performance: move local file#17082
Conversation
alperozturk96
added
3. to review
performance 🚀
There was a problem hiding this comment.
Pull request overview
Refactors FileDataStorageManager.moveLocalFile(...) to a Kotlin extension-based implementation intended to improve performance/clarity, and adds unit tests around DB updates, filesystem moves, and media scan behavior when moving files/folders.
Changes:
- Replaced the inlined Java
moveLocalFileimplementation with a delegation toFileDataStorageManagerExtensions.moveFiles. - Added a Room DAO bulk update method (
FileDao.updateAll) and moved descendant path/storagePath update logic into DAO-backed Kotlin code. - Added a new unit test suite covering guard conditions, entity updates, hierarchy updates, filesystem rename behavior, and media scan triggering.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java | Delegates moveLocalFile to the Kotlin extension and exposes TAG for shared logging. |
| app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt | Introduces moveFiles, filesystem move helper, and DB update logic for moved descendants + media scan handling. |
| app/src/main/java/com/nextcloud/client/database/dao/FileDao.kt | Adds updateAll(List<FileEntity>) for bulk updates used by the move logic. |
| app/src/test/java/com/owncloud/android/datamodel/MoveFilesTestBase.kt | Shared mocking/setup utilities for the new move-local-file test suite. |
| app/src/test/java/com/owncloud/android/datamodel/MoveFilesGuardTest.kt | Tests early-return guard behavior (null/nonexistent/root/invalid parent, etc.). |
| app/src/test/java/com/owncloud/android/datamodel/MoveFileEntitiesUpdateTest.kt | Tests DB entity field updates (path/pathDecrypted/storagePath/parent) for moves. |
| app/src/test/java/com/owncloud/android/datamodel/MoveFilesFilesystemAndMediaTest.kt | Tests filesystem rename and media scan behavior for moved media vs non-media files. |
| app/src/test/java/com/owncloud/android/datamodel/MoveFilesHierarchyTest.kt | Tests descendant updates (paths/parents/storagePath) when moving folders with hierarchies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
APK file: https://github.com/nextcloud/android/actions/runs/26279321751/artifacts/7157181307 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @Suppress("ReturnCount") | ||
| private fun moveLocalFiles(accountName: String, ocFile: OCFile, defaultSavePath: String, targetPath: String): Boolean { | ||
| val localFile = File(FileStorageUtils.getDefaultSavePathFor(accountName, ocFile)) | ||
| if (!localFile.exists()) { |
|
|
||
| val targetFile = File(defaultSavePath + targetPath) | ||
| val targetFolder = targetFile.getParentFile() | ||
| if (targetFolder != null && !targetFolder.exists() && !targetFolder.mkdirs()) { |
|
|
||
| val targetFile = File(defaultSavePath + targetPath) | ||
| val targetFolder = targetFile.getParentFile() | ||
| if (targetFolder != null && !targetFolder.exists() && !targetFolder.mkdirs()) { |
| ) | ||
| } | ||
|
|
||
| if (!localFile.renameTo(targetFile)) { |
| ) | ||
| } | ||
|
|
||
| if (!localFile.renameTo(targetFile)) { |
alperozturk96
force-pushed
the
performance/move-local-file
branch
from
18bcde5 to
c461a50
Compare
|
APK file: https://github.com/nextcloud/android/actions/runs/26497289452/artifacts/7234556505 |
|
blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
|
test-Unit test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/17082-Unit-test-07-34 |
Issue
Renaming folder too slow
Changes
How to test?
Demo
Screen.Recording.2026-05-22.at.12.11.42.mp4