Make the name collision policy for auto uploads selectable #5810

fodinabor · 2020-04-05T17:46:50Z

Resolves #5827
Fix #5755
Fix #5599
Fix #4260
Fix #5846

Since #4788 synching WhatsApp Backups or similar files that don't change their name to Nextcloud is horrible, as one is asked how the files should be handled for every single file every day (well - as often the files change..).

This PR adds the option for synced folders how name collisions with updated files should be handled.
The options are (as proposed in #5599):

Ask me every time
Rename new version
Overwrite remote version

Disclaimer: probably did not run all tests cleanly and did not update the screenshots, as that seemed nearly impossible on Windows..

nextcloud-android-bot · 2020-04-05T18:06:18Z

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

nextcloud-android-bot · 2020-04-05T18:06:20Z

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

src/test/java/com/owncloud/android/ui/activity/SyncedFoldersActivityTest.java

nextcloud-android-bot · 2020-04-06T08:49:27Z

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

src/main/java/com/owncloud/android/ui/dialog/parcel/SyncedFolderParcelable.java

tobiasKaminsky · 2020-04-07T13:22:20Z

Welcome on board 🎉

Some very very minor issues.
Other than that is perfect ❤️

ezaquarii

I don't like how we treat serialization/deserialization here - it's brittle and the logic is spraed into multiple points of interest.

Also, naming is of a concern here.

src/main/java/com/owncloud/android/ui/dialog/SyncedFolderPreferencesDialogFragment.java

ezaquarii · 2020-04-07T13:57:32Z

src/main/java/com/owncloud/android/ui/dialog/parcel/SyncedFolderParcelable.java

+        return 0;
+    }
+
+    public void setNameCollisionPolicy(String nameCollisionPolicy) {


Did you mean setCollisionPolicyByName(String collisionPolicyName)? This function name does not make sense - it says that someobdy chooses a name for a collision policy.

I think it refers to the unfornately named NameCollisionPolicy type.

@tobiasKaminsky what do you think? can we fix it?

src/main/java/com/owncloud/android/ui/dialog/parcel/SyncedFolderParcelable.java

src/main/java/com/owncloud/android/datamodel/SyncedFolderDisplayItem.java

src/main/java/com/owncloud/android/ui/dialog/parcel/SyncedFolderParcelable.java

ezaquarii · 2020-04-07T14:12:49Z

src/main/java/com/owncloud/android/ui/dialog/parcel/SyncedFolderParcelable.java

+    }
+
+    public void setNameCollisionPolicy(String nameCollisionPolicy) {
+        switch (nameCollisionPolicy) {


This conversion could be folded into enum NameCollisionPolicy:

public static NameCollisionPolicy fromName(String policyName) { ... }

@tobiasKaminsky I think the name choosen for this concept is invalid. While here, we could rename it into CollisionPolicyName, so it respects english grammar rules. As it is now - it's just confusing.

src/main/res/values/attrs.xml

tobiasKaminsky · 2020-04-08T04:32:45Z

I don't like how we treat serialization/deserialization here - it's brittle and the logic is spraed into multiple points of interest.

As this is mainly c&p from the way I 🙈 handled this on other parts, I am fine to have this the same way for now, and refactor later.

fodinabor · 2020-04-08T11:45:17Z

I tried to make the NameCollisionPolicy / dialog index a bit cleaner and moved the code for this to the fragment, as the selection index to 'NameCollisionPolicy` mapping is only required in this specific dialog.

nextcloud-android-bot · 2020-04-08T12:05:51Z

Unit test failed, but no output was generated. Maybe a preliminary stage failed.

nextcloud-android-bot · 2020-04-08T12:08:50Z

Screenshot test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/13467-Screenshot

nextcloud-android-bot · 2020-04-08T12:16:04Z

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

tobiasKaminsky · 2020-04-09T07:12:04Z

/backport to stable-3.11

tobiasKaminsky · 2020-04-09T07:31:03Z

@fodinabor I took the freedom to reformat the code a bit.
We try to have each parameter in a new line, if it fits not in one line (>120 chars).
This is the next time then way more better to read.

nextcloud-android-bot · 2020-04-09T07:45:32Z

Unit test failed, but no output was generated. Maybe a preliminary stage failed.

tobiasKaminsky · 2020-04-14T15:21:59Z

Rebased and let us merge this tomorrow :-)

nextcloud-android-bot · 2020-04-14T18:00:14Z

Unit test failed, but no output was generated. Maybe a preliminary stage failed.

AndyScherzinger · 2020-04-15T09:31:09Z

@tobiasKaminsky can you take care of the spotbug counter? ❤️

Signed-off-by: Joachim Meyer <joachim@joameyer.de>

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

nextcloud-android-bot · 2020-04-15T17:15:32Z

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

Issues
======
+ Solved 8
           

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

See the complete overview on Codacy

nextcloud-android-bot · 2020-04-15T18:50:37Z

Unit test failed, but no output was generated. Maybe a preliminary stage failed.

nextcloud-android-bot · 2020-04-15T19:07:51Z

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

nextcloud-android-bot · 2020-04-15T22:48:53Z

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/13536.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 · 2020-04-15T22:51:33Z

Unit test failed, but no output was generated. Maybe a preliminary stage failed.

nextcloud-android-bot · 2020-04-15T22:58:57Z

Codacy

384

Lint

Type	master	PR
Warnings	94	94
Errors	1	1

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	70
Internationalization Warnings	11
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	77
Security Warnings	44
Dodgy code Warnings	142
Total	384

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	70
Internationalization Warnings	11
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	77
Security Warnings	44
Dodgy code Warnings	141
Total	383

nextcloud-android-bot · 2020-04-15T23:11:06Z

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

tobiasKaminsky · 2020-04-16T06:03:41Z

Thanks @fodinabor for this awesome contribution!

backportbot-nextcloud · 2020-04-16T06:04:07Z

backport to stable-3.11 in #5883

piknew · 2020-04-22T19:41:01Z

Will it fix the issue from #5755 if remote file does not exist yet? It is a case for 100% new file, eg. photo just taken, definitely not stored on remote (server) side yer. However, sync conflict is still reported by client?

piknew · 2020-04-24T22:30:16Z

v3.11.1 - not fixed for non-existing files. Strange as it seems to query... directory (not file). Following is apache access log when I click "conflicted file". App queries "getting remote version from server" (again, a file does not exist on server) and apache is reporting:

Apr 25 00:26:46 PKSERVER apache[25696]: *******:443 192.168.10.1 - "PROPFIND /remote.php/webdav/Photos/Camera/ HTTP/1.1" 207 RX:1809 TX:2068 "-" "Mozilla/5.0 (Android) Nextcloud-android/3.11.1"
Apr 25 00:26:46 PKSERVER apache[25708]: *******:443 192.168.10.1 - "PROPFIND /remote.php/webdav/Photos/Camera/ HTTP/1.1" 207 RX:1471 TX:13433 "-" "Mozilla/5.0 (Android) Nextcloud-android/3.11.1"

AndyScherzinger added the 3. to review label Apr 5, 2020

AndyScherzinger requested a review from tobiasKaminsky April 5, 2020 20:57

ashpieboop reviewed Apr 6, 2020

View reviewed changes

src/test/java/com/owncloud/android/ui/activity/SyncedFoldersActivityTest.java Outdated Show resolved Hide resolved

AndyScherzinger added the design label Apr 6, 2020

ashpieboop mentioned this pull request Apr 6, 2020

Sync conflict #5755

Closed

tobiasKaminsky reviewed Apr 7, 2020

View reviewed changes

src/main/java/com/owncloud/android/ui/dialog/parcel/SyncedFolderParcelable.java Outdated Show resolved Hide resolved

tobiasKaminsky reviewed Apr 7, 2020

View reviewed changes

src/main/java/com/owncloud/android/ui/dialog/parcel/SyncedFolderParcelable.java Outdated Show resolved Hide resolved

ezaquarii requested changes Apr 7, 2020

View reviewed changes

AndyScherzinger mentioned this pull request Apr 8, 2020

Upload of WhatsApp Backup failed #5827

Closed

nextcloud deleted a comment from fodinabor Apr 8, 2020

backportbot-nextcloud bot added the backport-request label Apr 9, 2020

This was referenced Apr 14, 2020

auto-upload and WhatsApp backups #4260

Closed

NextCloud #5846

Closed

ezaquarii approved these changes Apr 14, 2020

View reviewed changes

AndyScherzinger mentioned this pull request Apr 15, 2020

Ask if existing file should be replaced #431

Closed

AndyScherzinger approved these changes Apr 15, 2020

View reviewed changes

fodinabor and others added 3 commits April 15, 2020 19:12

Make the name collision policy for auto uploads selectable, fixes #5599.

32da47d

Signed-off-by: Joachim Meyer <joachim@joameyer.de>

code formatting

db3bd02

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

increase spotbugs

464aade

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

tobiasKaminsky merged commit af0b39e into nextcloud:master Apr 16, 2020

backportbot-nextcloud bot mentioned this pull request Apr 16, 2020

[stable-3.11] Make the name collision policy for auto uploads selectable #5883

Merged

AndyScherzinger added this to the Nextcloud App 3.12.0 milestone Apr 16, 2020

tobiasKaminsky removed the backport-request label Apr 16, 2020

dassio mentioned this pull request Sep 8, 2020

Add cancel upload in case of name collision in autoupload #6911

Closed

Uh oh!

Make the name collision policy for auto uploads selectable #5810

Make the name collision policy for auto uploads selectable #5810

Uh oh!

Conversation

fodinabor commented Apr 5, 2020 • edited by tobiasKaminsky Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nextcloud-android-bot commented Apr 5, 2020

Uh oh!

nextcloud-android-bot commented Apr 5, 2020

Uh oh!

Uh oh!

nextcloud-android-bot commented Apr 6, 2020

Uh oh!

Uh oh!

Uh oh!

tobiasKaminsky commented Apr 7, 2020

Uh oh!

ezaquarii left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ezaquarii Apr 7, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ezaquarii Apr 7, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tobiasKaminsky commented Apr 8, 2020

Uh oh!

fodinabor commented Apr 8, 2020

Uh oh!

nextcloud-android-bot commented Apr 8, 2020

Uh oh!

nextcloud-android-bot commented Apr 8, 2020

Uh oh!

nextcloud-android-bot commented Apr 8, 2020

Uh oh!

tobiasKaminsky commented Apr 9, 2020

Uh oh!

tobiasKaminsky commented Apr 9, 2020

Uh oh!

nextcloud-android-bot commented Apr 9, 2020

Uh oh!

tobiasKaminsky commented Apr 14, 2020

Uh oh!

nextcloud-android-bot commented Apr 14, 2020

Uh oh!

AndyScherzinger commented Apr 15, 2020

Uh oh!

nextcloud-android-bot commented Apr 15, 2020

Uh oh!

nextcloud-android-bot commented Apr 15, 2020

Uh oh!

nextcloud-android-bot commented Apr 15, 2020

Uh oh!

nextcloud-android-bot commented Apr 15, 2020

Uh oh!

nextcloud-android-bot commented Apr 15, 2020

Uh oh!

nextcloud-android-bot commented Apr 15, 2020

Codacy

Lint

SpotBugs (new)

SpotBugs (master)

Uh oh!

nextcloud-android-bot commented Apr 15, 2020

Uh oh!

tobiasKaminsky commented Apr 16, 2020

Uh oh!

backportbot-nextcloud bot commented Apr 16, 2020

Uh oh!

piknew commented Apr 22, 2020

Uh oh!

fodinabor commented Apr 5, 2020 •

edited by tobiasKaminsky

Loading

piknew commented Apr 24, 2020 •

edited

Loading