Include option to allow scanner to reuse etags only for files #37218

jvillafanez · 2020-04-07T09:12:10Z

This option will be used by the watcher, in case it detects a change in
a folder, so the etag of the folder will always change (not just
depending on mtime) but the contents might retain its current etag.

Description

Changes in gdrive weren't being propagated properly because the mtime of the folder doesn't change when a file is added / removed / modified there.
The etag of the folder didn't change, so the desktop client loses track of where to look for changes once it reaches the modified folder. This means that the new files inside the folder don't appear in the desktop (files aren't removed neither)

Note that the mtime of the folder in ownCloud still be the same as the one reported by gdrive

Related Issue

owncloud/client#7837 (comment)

Motivation and Context

gdrive integration don't behave like other external storages from a desktop client's perspective otherwise.

How Has This Been Tested?

Manually checked:

Connect ownCloud with a gdrive account
Let desktop client sync normally the first time
Add a new file directly in a folder inside the gdrive account
Make a propfind to the target folder (you can navigate the ownCloud's web UI to the target folder instead) to detect the change
Sync client should pick up the change

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

This option will be used by the watcher, in case it detects a change in a folder, so the etag of the folder will always change (not just depending on mtime) but the contents might retain its current etag.

update-docs · 2020-04-07T09:12:14Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

codecov · 2020-04-07T10:15:20Z

Codecov Report

Merging #37218 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master   #37218   +/-   ##
=========================================
  Coverage     64.87%   64.88%           
- Complexity    19146    19151    +5     
=========================================
  Files          1269     1269           
  Lines         74947    74950    +3     
  Branches       1331     1331           
=========================================
+ Hits          48625    48628    +3     
  Misses        25930    25930           
  Partials        392      392

Flag	Coverage Δ	Complexity Δ
#javascript	`54.14% <ø> (ø)`	`0.00 <ø> (ø)`
#phpunit	`66.07% <100.00%> (+<0.01%)`	`19151.00 <0.00> (+5.00)`

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Files/Cache/Scanner.php	`87.55% <100.00%> (+0.12%)`	`118.00 <0.00> (+4.00)`
lib/private/Files/Cache/Watcher.php	`100.00% <100.00%> (ø)`	`17.00 <0.00> (ø)`
...AppFramework/Utility/ControllerMethodReflector.php	`100.00% <0.00%> (ø)`	`10.00% <0.00%> (ø%)`
...les_sharing/lib/Controller/RemoteOcsController.php	`95.31% <0.00%> (+0.07%)`	`17.00% <0.00%> (+1.00%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8ae99c...564b5f1. Read the comment docs.

jvillafanez · 2020-04-07T10:51:18Z

@pmaier1 I'll need confirmation whether this PR will be in 10.4.1 or 10.5.0.
If this will be available in 10.4.1, we'll need to close this PR and open a new one targeting 10.4.1

Changelog entry to be added after confirmation.

mmattel · 2020-04-07T13:18:49Z

This could be related: #37170
(Browser reload does not always triggers a sync if new file is found created via external write)

@micbar @pmaier1 highly appreciate if this could go into 10.4.1

jvillafanez · 2020-04-07T14:45:43Z

#37170 should be fixed with this PR. The target use case is the same.

micbar · 2020-04-07T20:03:49Z

@jvillafanez @mmattel I have doubts for 10.4.1.

It touches a central part of core (etag propagation)
One could argument, that this change requires a minor version.

I would like to avoid the testing effort now. 10.4.1 should be out 1-2 days. This would delay other important fixes.

IMO waiting for a fix of Gdrive external storage compared to a very urgent bugfix of password_policy is an easy decision.

@pmaier1 @HanaGemela open for your input.

HanaGemela · 2020-04-08T07:47:44Z

I wouldn't delay 10.4.1

pmaier1 · 2020-04-08T12:20:48Z

10.4.1 is closed and should not be delayed by this. THX!

mrow4a · 2020-04-20T19:19:29Z

@jvillafanez I think we might have similar issue with fed shares. I noticed that browsing in UI also mixes etag propagation.

Try to verify this scenario:

pause sync client
add the files in gdrive in test1/test1.1/test1.1.1/test1.1.1.1
navigate in UI to test1, and to test1.1
unpause to sync in sync client -> it might happen changes won't be detected

mmattel · 2020-04-23T10:53:30Z

@mrow4a it would be a good idea to create a new issue so we can track it.
This comment gets definitely lost...

mrow4a · 2020-04-23T10:59:29Z

@mmattel ignore it for now, external storages are working a bit differently than shared external storages. if we get similar issue, maybe this would be good hint for someone.

The scanner's update method will reuse the size if possible. REUSE_ONLY_FOR_FILES flag will only be applicable to etag, not size Links to #37218

Include option to allow scanner to reuse etags only for files

78f3f67

This option will be used by the watcher, in case it detects a change in a folder, so the etag of the folder will always change (not just depending on mtime) but the contents might retain its current etag.

jvillafanez added the 3 - To Review label Apr 7, 2020

jvillafanez self-assigned this Apr 7, 2020

Add changelog entry

564b5f1

jvillafanez requested a review from DeepDiver1975 April 7, 2020 15:45

DeepDiver1975 approved these changes Apr 20, 2020

View reviewed changes

DeepDiver1975 merged commit c319878 into master Apr 20, 2020

delete-merged-branch bot deleted the gdrive_folder_fix branch April 20, 2020 08:31

davitol mentioned this pull request Jun 25, 2020

Tracking test results for ownCloud 10.5.0 #37593

Closed

65 tasks

jvillafanez added a commit that referenced this pull request Sep 29, 2020

Adjust scanner flags

78fb854

The scanner's update method will reuse the size if possible. REUSE_ONLY_FOR_FILES flag will only be applicable to etag, not size Links to #37218

jvillafanez mentioned this pull request Sep 29, 2020

Adjust scanner flags #37960

Merged

11 tasks

jvillafanez added a commit that referenced this pull request Oct 9, 2020

Adjust scanner flags

6554f5a

The scanner's update method will reuse the size if possible. REUSE_ONLY_FOR_FILES flag will only be applicable to etag, not size Links to #37218

JammingBen pushed a commit that referenced this pull request Nov 23, 2020

Adjust scanner flags

f787caa

The scanner's update method will reuse the size if possible. REUSE_ONLY_FOR_FILES flag will only be applicable to etag, not size Links to #37218

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include option to allow scanner to reuse etags only for files #37218

Include option to allow scanner to reuse etags only for files #37218

jvillafanez commented Apr 7, 2020

update-docs bot commented Apr 7, 2020

codecov bot commented Apr 7, 2020 •

edited

Loading

jvillafanez commented Apr 7, 2020 •

edited

Loading

mmattel commented Apr 7, 2020 •

edited

Loading

jvillafanez commented Apr 7, 2020

micbar commented Apr 7, 2020

HanaGemela commented Apr 8, 2020

pmaier1 commented Apr 8, 2020

mrow4a commented Apr 20, 2020 •

edited

Loading

mmattel commented Apr 23, 2020

mrow4a commented Apr 23, 2020

Include option to allow scanner to reuse etags only for files #37218

Include option to allow scanner to reuse etags only for files #37218

Conversation

jvillafanez commented Apr 7, 2020

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

update-docs bot commented Apr 7, 2020

codecov bot commented Apr 7, 2020 • edited Loading

Codecov Report

jvillafanez commented Apr 7, 2020 • edited Loading

mmattel commented Apr 7, 2020 • edited Loading

jvillafanez commented Apr 7, 2020

micbar commented Apr 7, 2020

HanaGemela commented Apr 8, 2020

pmaier1 commented Apr 8, 2020

mrow4a commented Apr 20, 2020 • edited Loading

mmattel commented Apr 23, 2020

mrow4a commented Apr 23, 2020

codecov bot commented Apr 7, 2020 •

edited

Loading

jvillafanez commented Apr 7, 2020 •

edited

Loading

mmattel commented Apr 7, 2020 •

edited

Loading

mrow4a commented Apr 20, 2020 •

edited

Loading