DocumentsStorageProvider: prevent opening stale file contents

[jmue]

Replace SynchronizeFileOperation by ReadFileRemoteOperation to prevent opening stale file contents
fix #6883

Signed-off-by: Jens Mueller tschenser@gmx.de

[codecov]

Codecov Report

Merging #6917 into master will increase coverage by 1.33%.
The diff coverage is 64.28%.

@@             Coverage Diff              @@
##             master    #6917      +/-   ##
============================================
+ Coverage     27.08%   28.42%   +1.33%     
+ Complexity     2313        5    -2308     
============================================
  Files           425      428       +3     
  Lines         34178    34215      +37     
  Branches       4729     4723       -6     
============================================
+ Hits           9258     9726     +468     
+ Misses        23467    23024     -443     
- Partials       1453     1465      +12     

Impacted Files	Coverage Δ	Complexity Δ
...ud/android/providers/DocumentsStorageProvider.java	52.83% <64.28%> (+2.69%)	0.00 <0.00> (-29.00)	⬆️
.../owncloud/android/datamodel/MediaFoldersModel.java	0.00% <0.00%> (-33.34%)	0.00% <0.00%> (-1.00%)
...d/android/operations/SynchronizeFileOperation.java	0.00% <0.00%> (-24.22%)	0.00% <0.00%> (-3.00%)
...ud/android/operations/GetUserProfileOperation.java	88.88% <0.00%> (-11.12%)	0.00% <0.00%> (-3.00%)
...nextcloud/client/jobs/MediaFoldersDetectionWork.kt	6.87% <0.00%> (-8.40%)	0.00% <0.00%> (-2.00%)
...com/owncloud/android/ui/activity/BaseActivity.java	73.58% <0.00%> (-7.55%)	0.00% <0.00%> (-17.00%)
.../com/nextcloud/client/files/downloader/Registry.kt	91.11% <0.00%> (-4.24%)	0.00% <0.00%> (-25.00%)
...oud/android/operations/RefreshFolderOperation.java	73.28% <0.00%> (-3.82%)	0.00% <0.00%> (-49.00%)
.../nextcloud/client/jobs/BackgroundJobManagerImpl.kt	74.40% <0.00%> (-2.76%)	0.00% <0.00%> (-18.00%)
...in/java/com/owncloud/android/utils/ThemeUtils.java	52.86% <0.00%> (-2.23%)	0.00% <0.00%> (-61.00%)
... and 65 more

[tobiasKaminsky]

Maybe I am wrong, but

• download a file
• change it on server -> outdated on device
• open this file via DSP
• with current system: file gets synced, user sees new content
• with this approach: still content is there, as ReadFileRemoteOperation only updates metadata, but not content?

[jmue]

* with this approach: still content is there, as ReadFileRemoteOperation only updates metadata, but not content?

ReadFileRemoteOperation is only used for checking if a file download is still needed, if so this codepath is taken.

[grote]

Thanks @jmue for picking this up! I left some questions, but am not an expert on the Nextcloud bits.

Did you run the integration tests I had added? (The CI is flaky unfortunately)
Would maybe be nice to add another test to them that shows the improved behavior, aka not serving stale files anymore?

[nextcloud-android-bot]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/15841.apk



To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

[nextcloud-android-bot]

Codacy

722

Lint

Type	master	PR
Warnings	81	81
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	27
Correctness Warnings	54
Internationalization Warnings	9
Multithreaded correctness Warnings	9
Performance Warnings	72
Security Warnings	44
Dodgy code Warnings	105
Total	320

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	27
Correctness Warnings	54
Internationalization Warnings	9
Multithreaded correctness Warnings	9
Performance Warnings	72
Security Warnings	44
Dodgy code Warnings	105
Total	320

[grote]

I have tested this branch (rebased on current master HEAD) with seedvault and production data. Not only did I not find any new issues (with this particular usage pattern), but also get less connection errors.

[jmue]

@tobiasKaminsky Are there any open demands for code changes left? I don't know how I should further proceed here. From my perspective the code behavior at the reviewed sections is the same as on the master branch. Please guide me what is needed from my side here.

[tobiasKaminsky]

I just overlooked it.
Thanks for bringing this up again.
@grote thanks for testing this with seedvault 👍