Make synced folder init/enable date persistent

[ashpieboop]

As seen in #1150 :

When the app crashes, is not run for some reason, and that files are added in auto upload folders during this downtime, even if the folders were enabled before and successfully synced some files, there are some that get ignored because the sync start time of an auto upload folder is reset when the application restarts.

This pull request fixes this bug/undesirable behavior.

[nextcloud-android-bot]

Unit test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/11508

[codecov]

Codecov Report

Merging #4787 into master will increase coverage by 0.13%.
The diff coverage is 30.1%.

@@             Coverage Diff              @@
##             master    #4787      +/-   ##
============================================
+ Coverage     17.51%   17.65%   +0.13%     
  Complexity        3        3              
============================================
  Files           376      377       +1     
  Lines         32385    32406      +21     
  Branches       4569     4580      +11     
============================================
+ Hits           5673     5721      +48     
+ Misses        25797    25765      -32     
- Partials        915      920       +5

Impacted Files	Coverage Δ	Complexity Δ
...ain/java/com/owncloud/android/db/ProviderMeta.java	84.61% <ø> (ø)	0 <0> (ø)	⬇️
...java/com/nextcloud/client/di/ComponentsModule.java	0% <ø> (ø)	0 <0> (ø)	⬇️
...wncloud/android/files/BootupBroadcastReceiver.java	0% <ø> (ø)	0 <0> (ø)	⬇️
...n/java/com/owncloud/android/jobs/FilesSyncJob.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...a/com/owncloud/android/jobs/AccountRemovalJob.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...va/com/owncloud/android/utils/FilesSyncHelper.java	22.47% <0%> (+2.66%)	0 <0> (ø)	⬇️
...wncloud/android/jobs/MediaFoldersDetectionJob.java	10.09% <100%> (+0.83%)	0 <0> (ø)	⬇️
...oud/android/ui/activity/SyncedFoldersActivity.java	23.97% <22.22%> (+1.18%)	0 <0> (ø)	⬇️
...a/com/owncloud/android/datamodel/SyncedFolder.java	50% <36.84%> (+38.46%)	0 <0> (ø)	⬇️
...ncloud/android/datamodel/SyncedFolderProvider.java	11.86% <40%> (+0.65%)	0 <0> (ø)	⬇️
... and 20 more

[AndyScherzinger]

@ArisuOngaku Thanks for this PR 🙏

@tobiasKaminsky @mario fancy taking a look/review ❤️ Code-wise it seems fine to me, besides one minor question regarding data migration for existing db entries.

[ashpieboop]

There you go, disabled folders get enable_date = -1 and enabled get enable_date = System.currentTimeMillis()

This avoids existing enabled folders from uploading old files, which when actually desired, should be satisfied by #4788

[nextcloud-android-bot]

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/11528

[ezaquarii]

@tobiasKaminsky @AndyScherzinger

[ashpieboop]

So I discovered dagger on my last push; I tested my code of course, but still, how can you tell whether dagger is available in a class? I'm not 100% sure for in SyncedFolderProvider and FileContentProvider.

[ashpieboop]

Yeah sorry for that, please be sure I learnt from this situation.

[AndyScherzinger]

Yeah sorry for that, please be sure I learnt from this situation.

Nothing to be sorry for! Just happy about your ping 🥇

[ezaquarii]

I'll take another look evening.

[ezaquarii]

@tobiasKaminsky @AndyScherzinger @ArisuOngaku I have only 1 minor comment - not sure if this is important so feel free to ignore it. Rest looks LGTM.

[ezaquarii]

Ok, so I'd this is not likely to happen due to our bugs, we don't care about stacktrace too much although it won't hurt to have it. Feel free to close the review and carry on as I won't be available for the rest of the business day.

[nextcloud-android-bot]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/11626.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]

Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- src/main/java/com/owncloud/android/datamodel/SyncedFolder.java  1
- src/main/java/com/owncloud/android/providers/FileContentProvider.java  3
         

Complexity decreasing per file
==============================
+ src/main/java/com/owncloud/android/utils/FilesSyncHelper.java  -1
         

See the complete overview on Codacy

[ezaquarii]

LGTM

[nextcloud-android-bot]

Codacy

306

Lint

Type	master	PR
Warnings	59	59
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	70
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	44
Dodgy code Warnings	136
Total	422

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	69
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	44
Dodgy code Warnings	138
Total	423

[AndyScherzinger]

@tobiasKaminsky LGTM code-wise from @ezaquarii's and my perspective. Fancy taking a look? ❤️

[tobiasKaminsky]

I did following test:

• fresh install with this branch
• enabled camera folder
• took some images --> uploaded fine
• disabled camera folder
• took some images
• re-enabled camera folder
  --> nothing happened
• took a new image --> only this was uploaded

As I understand it, this PR should then also upload those images within the disabled timespan, or?

But how should this work, if we have "only" enabled timestamp?
E.g:
1000s (just to have short timestamps): enable folder
1500s disable folder
1600s take some images
2000s enable folder again: then we search for images that are enabled since 2000s (

android/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java

Line 174 in 65da616

if (cursor.getLong(column_index_date_modified) >= enabledTimestampMs / 1000.0) {

)

But instead we would need the 1500s (disabled timestamp), or?
So we would need an "enabled/disabled" state and a "changedTimestamp"?

[ashpieboop]

@tobiasKaminsky No, this pull request does not aim to upload files in a synced folder that were created while the folder was disabled. This pull request only aims to correctly upload all files that are created while a synced folder is enabled, even when the app is closed (aka not monitoring changes) and later opened again.

Before this PR, the app effectively resetted this timestamp at each launch, meaning that when it crashed or wasn't started, any file created during this period would never get uploaded even if the synced folder was enabled.

edit: Though that could actually be a design ambiguity of the meaning of what enabled/disabled for a synced folder means. I took it as "don't upload files that are created in this folder while it's down".
Note though that with #4788, this logic should be handled when that setting is enabled (most probably will end up on by default), and I thought that the behavior you're describing (this PR's current behavior) would fit perfectly for when this other PR's setting is disabled.

[tobiasKaminsky]

Ah, then I misunderstood the idea of it and now it makes sense and this PR fits totally fine.

Thanks for your great enhancement ❤️

[ashpieboop]

No problem, it's been a pleasure ! ❤️

[tobiasKaminsky]

As you are a member of nextcloud, you can open up a branch next time directly in this repo, so you do not have to your own.
This makes reviewing/checkout a bit more easier for us :-)

I am looking forward to "see" you next time :-)

[ashpieboop]

Alright, thanks for that ! I'm most likely going to work on #4788 now.