#7815 Delayed synchronization

[BatPio]

Fixes #7815 Fixes #2323

• [ ] Tests written, or not not needed Some tests are written. I'm waiting for suggestions on what else can be tested.

Delay configuration:

[codecov[bot]]

Codecov Report

Merging #11278 (3d40c3a) into master (aeb1632) will increase coverage by 0.07%. The diff coverage is 37.20%.

:exclamation: Current head 3d40c3a differs from pull request most recent head 501d1fe. Consider uploading reports for the commit 501d1fe to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #11278      +/-   ##
============================================
+ Coverage     31.44%   31.52%   +0.07%     
- Complexity     3413     3427      +14     
============================================
  Files           575      577       +2     
  Lines         42018    42146     +128     
  Branches       5660     5664       +4     
============================================
+ Hits          13214    13286      +72     
- Misses        26861    26905      +44     
- Partials       1943     1955      +12     

Impacted Files	Coverage Δ
...com/nextcloud/client/database/NextcloudDatabase.kt	100.00% <ø> (ø)
...cloud/client/database/entity/SyncedFolderEntity.kt	0.00% <0.00%> (ø)
...java/com/nextcloud/client/di/ComponentsModule.java	0.00% <ø> (ø)
...in/java/com/nextcloud/client/jobs/FilesSyncWork.kt	21.95% <0.00%> (-0.18%)	:arrow_down:
...loud/android/datamodel/FilesystemDataProvider.java	4.39% <0.00%> (-0.10%)	:arrow_down:
...oud/android/datamodel/SyncedFolderDisplayItem.java	35.29% <ø> (ø)
...ncloud/android/datamodel/SyncedFolderProvider.java	10.93% <0.00%> (-0.18%)	:arrow_down:
...ain/java/com/owncloud/android/db/ProviderMeta.java	100.00% <ø> (ø)
...cloud/android/ui/activity/SyncedFoldersActivity.kt	20.43% <0.00%> (-0.55%)	:arrow_down:
.../dialog/SyncedFolderPreferencesDialogFragment.java	0.00% <0.00%> (ø)
... and 11 more

[BatPio]

@AlvaroBrey Thanks for hints! I made fixes

[nextcloud-android-bot]

master-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/8399-IT-master-22-20

[andreapx]

@BatPio is this still a work in progress or has it been abandoned?

[BatPio]

@JonasMayerDev @alperozturk96

Hi, sorry for delay, I updated my branch to master and now I have a problem with check: "TooManyFunctions - 15/15 - [Class 'SyncedFolderPreferencesDialogFragment' with '15' functions detected. "

Can you give me some suggestions on what can be moved out of this class?

[alperozturk96]

@JonasMayerDev @alperozturk96

Hi, sorry for delay, I updated my branch to master and now I have a problem with check: "TooManyFunctions - 15/15 - [Class 'SyncedFolderPreferencesDialogFragment' with '15' functions detected. "

Can you give me some suggestions on what can be moved out of this class?

You can use

@Suppress("TooManyFunctions")

this annotation to suppress it, or you can create a new class, e.g. DelaySyncDialogManager and move new functions inside it.

[BatPio]

@alperozturk96

Thanks for advice

I couldn't find any example of DialogManager in the project, so I used @Suppress("TooManyFunctions") . I think the presence of showUploadDelayDialog in SyncedFolderPreferencesDialogFragment is acceptable, because they are closely related.

I think MR is ready for CR

[uspecies]

i just wonder what the upper limit for the day value will be? as i guess i may choose everything between 1 to 12 months in days. So for example 31 days may not satisfy all users #just saying :p

i will start using NC after this PR is released :) cheers!

[PhilLab]

This functionality is really helpful, thanks for investing the time here @BatPio. I was in the middle of setting up my dev environment to implement exactly this functionality when I saw that you already did the work 🙏 .

@tobiasKaminsky @alperozturk96 what is missing to get this merged?

1. Merge is blocked because changes were requested by @AlvaroBrey two years ago, but they seem to have been addressed by now. Can this be considered done?
2. Tests have been added. Are there more required?
3. Due to this PR being open a long time, there were a lot of merges from master. Do we need a cleanup of the git history on this branch (though non-trivial, due to all the merges from master) or can we leave it like it is?

[BatPio]

@PhilLab Right, let's finish the job! :)

@tobiasKaminsky @alperozturk96 I've updated my branch to master and merged my mechanism with the latest changes in FilesSyncWork. My MR is ready for CR.

[BatPio]

@alperozturk96 Hi, thanks for the code review. I have made fixes.

I have rebased my branch to master. Next rebase should be easy, but there is a lot of commits in the history. I hope it is not a problem and they will disappear during merging with squash.

You can continue code review :)

[BatPio]

@alperozturk96

Unfortunately rebase still doesn't work on my branch. Can we update it by merge with commit and after review merge it with squash?

[alperozturk96]

@ZetaTom @tobiasKaminsky Could you review and test this PR?

[alperozturk96]

@BatPio Thank you so much for the update. Code looks ok to me, but I want to test as well.

How can I test the delay? I can test the auto upload and set the delay, but not sure how to exactly test the delay.

[BatPio]

@alperozturk96 Testing is time-consuming.

After setting the auto upload delay for a directory, you need to add a new file to that directory and wait. After the delay time has elapsed, the file should appear on the server. It is important to check that the file has not appeared on the server before the delay time has elapsed. Due to the Android's sync interval, the delay may increase by an additional 15 minutes. In case of a 30-minute delay, the file should appear on server no earlier than after 30 minutes and no later than after 45 minutes.

I tested this with screenshots. Unfortunately, I have no idea how to test this without waiting.

[BatPio]

@alperozturk96

I have adapted my code to the current solution with FilesystemDataProvider. MR is ready for testing

[alperozturk96]

@tobiasKaminsky @ZetaTom

I tested auto upload seems like okay. Could you please review, especially FilesystemDataProvider.java and FilesSyncWork.kt ?

[jancborchardt]

When reading through the linked issues https://github.com/nextcloud/android/issues/7815 and https://github.com/nextcloud/android/issues/2323 – it seems that if we implement this delay setting at all, then it should be only there when auto upload is set to "Delete after upload". Also, instead of a flexible time picker, we should do it like elsewhere in Nextcloud and have some reasonable presets. E.g.:

• Upload immediately (default)
• 15 minutes
• 30 minutes
• 1 hour
• 24 hours
• 1 week

These presets already seem to cover all the cases brought up in the issues, without detailed time configuration needed.

"Send later" in Nextcloud Mail	"Clear status after"-picker in online status

Do you see other solutions here, especially ones which don’t add another customizable setting but just do it automatically? @tobiasKaminsky @alperozturk96 @ZetaTom

[PhilLab]

Re @jancborchardt

When reading through the linked issues #7815 and #2323 – it seems that if we implement this delay setting at all, then it should be only there when auto upload is set to "Delete after upload".

I think this feature also makes a ton of sense without auto deletion. It allows you to clean up the taken photos before they hit the cloud. Think of taking 5 photos of the same scene, where you then immediately delete those where people have their eyes closed, etc...

So it should be available for all upload types

[jancborchardt]

@PhilLab ah, thanks for the note, good point.

Then it should just be changed to the suggested intervals as mentioned.

[BatPio]

@ZetaTom @jancborchardt Thank you for the code review, I made changes

@ZetaTom You are right, the phrase "Delay file upload" can be misleading. I changed it to "Minimum file age"

@jancborchardt I replaced DurationPicker with a list with fixed values

After combining both ideas:

Since the sync interval in Android is 15 minutes, I think this value is not useful. Instead, I would add values of 6 hours and 3 days

[BatPio]

@alperozturk96 @ZetaTom

I've reworked the UI as suggested by @jancborchardt . Could you please review the changes?

[jancborchardt]

@tobiasKaminsky @alperozturk96 @ZetaTom could you give this a review? :)

[tobiasKaminsky]

Will review tomorrow

[BatPio]

Will review tomorrow @tobiasKaminsky

Hi, I updated my branch to master and resolved the conflicts with the database version. Could you please do a review?