Sync downloaded

[tobiasKaminsky]

Enhancement discussed in #2718

• remove "keep available offline"
• sync job
  • only on wifi
  • syncs all downloaded files
    • if conflict, just do nothing
    • deletes local files that are not on server any longer
• when entering folder with outdated downloaded file, a red icon is shown on file

TODO

• check handling after upgrade

[mario]

Would this support folders too?

[tobiasKaminsky]

Currently not as we do not have (yet) any possibility in UI to set "available offline" for folders.
It just enhances the current way.

We should discuss this properly and then implement it in a separate PR.
(keep in sync for folders is really close to two sync, or?)

[AndyScherzinger]

should also log the exception, else you just know it failed but not for which reason

[AndyScherzinger]

👍 looks good code-wise and also worked for me for the browsing scenario. Didn't test the actual sync though.

I also agree that we should have the folder variant in a different PR and that would be close to a two way sync while for example deleting a file on the client asks you if you want to delete the file on the server too, so that is rather a one way sync server->client + syncing back changed files to the server not sure if we should handle created files since that would mean someone created a file within the clients directory structure.

[AndyScherzinger]

Imho this needs another DB version and a change routine to remove the fields from the database

[tobiasKaminsky]

change routine to remove the fields from the database

There is no way in sqlite to remove an existing column.
We can only ignore it and on new installation just do not create it…

[mario]

Looks decent, but why are there so many Glide changes in there?

[tobiasKaminsky]

This extends #2871.
I will update #2871 today.

[sehucke]

Well, if you have 200x1Mb changes, your data plan is also gone…

What about this:
When entering a folder

* show outdated files with red cross on mobile

* auto sync folder on wifi

Having all downloaded files up to date in background on wifi, should already help a lot. And if you are on mobile data, you only "sync" manually those files you are opening, but then you know the downside (to spend data on it).

Sorry for my late reply... Just saw your post @tobiasKaminsky in #3246 .

Great ideas so far. But it would be great if the meaning of the red cross is in some way explained to the user. Is it "broken file", "not accessible", "no download possible / server broken"... or "not up to date"?! I bet most users might assume one of the first explanations. Maybe you add a toast informing the user that there are outdated files that need to be synced manually while in a mobile network. And maybe a red/white clock would resemble "outdated" much better than a red cross. :-)

Currently, the whole sync/offline/download feature is everything else than easy to understand for users.

[tobiasKaminsky]

@sehucke this is a very good feedback, thank you.

However I vote for merging this right now, as the meaning of icons stay the same and open up a new discussion how to improve this.

[AndyScherzinger]

However I vote for merging this right now, as the meaning of icons stay the same and open up a new discussion how to improve this.

I second that!

[sehucke]

👍

[tobiasKaminsky]

Rebased, should now compile again

[mario]

@tobiasKaminsky Codacy issues at least + build failed and I can then take a look.

[tobiasKaminsky]

Strange, only this PR is failing on tests, while e.g. #3350 is building fine: https://drone.nextcloud.com/nextcloud/android/7129

So I suggest, we ignore it ;-)

[tobiasKaminsky]

Hm. Seems indeed to be a valid problem:
Job intervall is set too low --> changed

[AndyScherzinger]

👍 works fine for me, I had

1. master + downloaded file (no wifi)
2. updated file on server
3. installed branch
4. file is marked red (since server changed, but no wifi)
5. activated wifi and waited for ~20 minutes
6. checked file -> makred green, up-to-date, no fetching server version, client already had the right file version

Thus merging

[AndyScherzinger]

@mario what do you think? Works fine for me (at least what I tested of course)

[matsest]

Would this allow for downloading/exporting a file from NC that is not affected by the sync? E.g. if I want to download/export a file from NC to my local Download folder and not Android/Media/com.nextcloud.android? At least for me, I don't have a Save to Device in the Share menu for a file in NC (as I have in Dropbox).

[AndyScherzinger]

@matsest No but you probably don't need that. SImply hit the share icon and choose an installed app, which allows for that e.g. Amaze File Manager :) (didn't test if this works though)

[matsest]

@AndyScherzinger Yes, that works with Amaze - have tested. However, it does not work with my default Files app (it's a crappy default Huawei app in EMUI), i.e. which is what "Save to Device" is in a range of other apps. I don't know much work there is to enable support for sharing to all these different CMFiles, Huawei Files app and other vendor specific default file apps. But it's a nice feature to be able to share across apps..

That's what I like about the sharing menu in Android - I probably don't need to be able to share a SMS to my Snapchat app (a crude example) and 15 other apps in the pop-up, but Android share menu allows for these interfaces between almost every app.

Specifically for a file sync app like NC I think it's a good idea that manual export/save to device should be possible without installing a third-party app for users - in addition to the offline/sync feature in this PR. That being said, I love Amaze and prefer it over default Files in EMUI, so I won't shed tears if this isn't taken into development ;)

Sorry if this was off topic for the PR (:

[tobiasKaminsky]

Specifically for a file sync app like NC I think it's a good idea that manual export/save to device

That is the problem: if you save a file to a folder outside NC it will never get updated. Therefore I am not a big fan of doing this.
If you have audio/video/images downloaded you can see them in gallery app / media player…
For further discussion please open up a new issue.

[mario]

Please make sure to drop the columns that we don't use anymore in the newest DB migration/transaction. Otherwise good.

[tobiasKaminsky]

#2886 (comment)

Andy: Imho this needs another DB version and a change routine to remove the fields from the database

#2886 (comment)

There is no way in sqlite to remove an existing column.
We can only ignore it and on new installation just do not create it…

:-D :-D

[tobiasKaminsky]

As this was previously based on glide branch, I forgot to remove the "update thumbnail" stuff.
So this is now back in, as this is required by the old thumbnail system.

[nextcloud-android-bot]

Lint

Type	master	PR
Warnings	83	83
Errors	0	0

FindBugs (new)

Warning Type	Number
Bad practice Warnings	35
Correctness Warnings	159
Internationalization Warnings	15
Malicious code vulnerability Warnings	11
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	66
Dodgy code Warnings	134
Total	548

FindBugs (master)

Warning Type	Number
Bad practice Warnings	35
Correctness Warnings	159
Internationalization Warnings	15
Malicious code vulnerability Warnings	11
Multithreaded correctness Warnings	9
Performance Warnings	121
Security Warnings	66
Dodgy code Warnings	134
Total	550