Add a command to poll incoming federated shares for updates #34903

VicDeo · 2019-03-26T16:50:47Z

Description

Allow manually poll federated share updates and update the storage root
to help the sync client properly detect federated share updates

Related Issue

Fixes <issue_link>

Motivation and Context

How Has This Been Tested?

setup two OC instances oc1 and oc2
create a federated share from user@oc1 to user@oc2 with a deep nested folder structure
Accept the share by user@oc2
Setup a desktop client for user@oc2, let it sync the data
update the share by user@oc1 (e.g by uploading something into /really/deeply/nested/folder)
wait for the client - nothing is synced
run php occ incomingShares:sync
wait a bit - the changes is synced

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:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

Document
Backport (if applicable set "backport-request" label and remove when the backport was done)

VicDeo · 2019-03-26T16:58:32Z

@PVince81 Won't you mind a quick preliminary review?

codecov · 2019-03-26T17:28:11Z

Codecov Report

Merging #34903 into master will decrease coverage by <.01%.
The diff coverage is 62.22%.

@@             Coverage Diff              @@
##             master   #34903      +/-   ##
============================================
- Coverage     65.34%   65.34%   -0.01%     
- Complexity    18492    18505      +13     
============================================
  Files          1208     1209       +1     
  Lines         69983    70028      +45     
  Branches       1280     1280              
============================================
+ Hits          45732    45761      +29     
- Misses        23879    23895      +16     
  Partials        372      372

Flag	Coverage Δ	Complexity Δ
#javascript	`53.04% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`66.75% <62.22%> (-0.01%)`	`18505 <12> (+13)`

Impacted Files	Coverage Δ	Complexity Δ
...s/federatedfilesharing/lib/AppInfo/Application.php	`46.85% <25%> (-2.01%)`	`19 <2> (+3)`
...atedfilesharing/lib/Command/PollIncomingShares.php	`75.75% <75.75%> (ø)`	`10 <10> (?)`
...ederatedfilesharing/lib/FederatedShareProvider.php	`61.13% <0%> (ø)`	`85% <0%> (ø)`	⬇️
lib/private/Share20/DefaultShareProvider.php	`98.33% <0%> (ø)`	`114% <0%> (ø)`	⬇️
apps/files_trashbin/lib/Expiration.php	`98.27% <0%> (+1.72%)`	`29% <0%> (ø)`	⬇️

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 568f5b1...06b148e. Read the comment docs.

apps/federatedfilesharing/lib/Command/SyncIncomingShares.php

mmattel · 2019-03-26T17:32:32Z

This is documenation relevant because it is a new command !
Please edit the PR to reflect a doc change and add in the docs repro a issue for it.

PVince81

See comments

apps/federatedfilesharing/appinfo/info.xml

apps/federatedfilesharing/lib/Command/SyncIncomingShares.php

PVince81 · 2019-03-28T09:06:55Z

apps/federatedfilesharing/lib/Command/SyncIncomingShares.php

+			->from('share_external')
+			->where($qb->expr()->eq('accepted', $qb->expr()->literal('1')));
+
+		return $qb->execute();


would this cursor lock the table for any further concurrent updates ?

@tomneedham @butonic

this is just an ordinary SELECT DISTINCT query, so I don't think so

jvillafanez · 2019-03-28T13:16:00Z

apps/federatedfilesharing/lib/Command/PollIncomingShares.php

+	/**
+	 * @return \Doctrine\DBAL\Driver\Statement
+	 */
+	protected function getCursor() {


Move this to the FederatedShareProvider? It should be fine to inject the provider directly in order to use a public method that isn't in the interface.

My taste tells me that it should stay here.
Could be moved to the provider but it needs to be renamed into getUniqueRecipients and return them as an array of uids (and this array may be huuuge)

If you don't want to leak DB stuff, you may return a generator instead of an array. As long as it's clear that the function returns a generator of uids I think it's fine.

I think the current code is good enough

apps/federatedfilesharing/lib/Command/PollIncomingShares.php

tomneedham · 2019-03-28T17:11:23Z

apps/federatedfilesharing/lib/Command/PollIncomingShares.php

+					$storage = $mount->getStorage();
+					$this->refreshStorageRoot($storage);
+				} catch (\Exception $e) {
+					$output->writeln($e->getMessage());


Problem here, is exceptions never get logged and since this command is designed probably to run in cron, will go unnoticed. Would suggest to log as well.

@tomneedham at least some of them are logged by the dav app:

deo@reborn:~/public_html/oc-master> rm data/owncloud.log deo@reborn:~/public_html/oc-master> php occ incoming-shares:poll Sabre\HTTP\ClientHttpException: Unauthorized deo@reborn:~/public_html/oc-master> cat data/owncloud.log {"reqId":"0jcYYTjvA7asrGLBtk0R","level":3,"time":"2019-03-29T10:07:45+00:00","remoteAddr":"","user":"--","app":"no app in context","method":"--","url":"--","message":"Exception: {\"Exception\":\"Sabre\\\\HTTP\\\\ClientHttpException\",\"Message\":\"Unauthorized\",\"Code\":401,\"Trace\":\"#0 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Client.php(232): Sabre\\\\HTTP\\\\Client->send(Object(Sabre\\\\HTTP\\\\Request))\\n#1 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(259): Sabre\\\\DAV\\\\Client->propFind('http:\\\/\\\/localhos...', Array)\\n#2 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(579): OC\\\\Files\\\\Storage\\\\DAV->propfind('')\\n#3 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Common.php(175): OC\\\\Files\\\\Storage\\\\DAV->stat('')\\n#4 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Common->filemtime('')\\n#5 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Availability.php(252): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#6 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Availability->filemtime('')\\n#7 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#8 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/apps\\\/federatedfilesharing\\\/lib\\\/Command\\\/PollIncomingShares.php(110): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#9 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/apps\\\/federatedfilesharing\\\/lib\\\/Command\\\/PollIncomingShares.php(92): OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares->refreshStorageRoot(Object(OCA\\\\Files_Trashbin\\\\Storage))\\n#10 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Command\\\/Command.php(255): OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares->execute(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#11 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(960): Symfony\\\\Component\\\\Console\\\\Command\\\\Command->run(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#12 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(255): Symfony\\\\Component\\\\Console\\\\Application->doRunCommand(Object(OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares), Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#13 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(148): Symfony\\\\Component\\\\Console\\\\Application->doRun(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#14 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Console\\\/Application.php(164): Symfony\\\\Component\\\\Console\\\\Application->run(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#15 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/console.php(106): OC\\\\Console\\\\Application->run()\\n#16 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/occ(11): require_once('\\\/home\\\/deo\\\/publi...')\\n#17 {main}\",\"File\":\"\\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/sabre\\\/http\\\/lib\\\/Client.php\",\"Line\":155}"} {"reqId":"0jcYYTjvA7asrGLBtk0R","level":3,"time":"2019-03-29T10:07:45+00:00","remoteAddr":"","user":"--","app":"files_external","method":"--","url":"--","message":"Unauthorized"} {"reqId":"0jcYYTjvA7asrGLBtk0R","level":3,"time":"2019-03-29T10:07:45+00:00","remoteAddr":"","user":"--","app":"no app in context","method":"--","url":"--","message":"Exception: {\"Exception\":\"OCP\\\\Files\\\\StorageInvalidException\",\"Message\":\"Sabre\\\\HTTP\\\\ClientHttpException: Unauthorized\",\"Code\":0,\"Trace\":\"#0 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(830): OC\\\\Files\\\\Storage\\\\DAV->throwByStatusCode(401, Object(Sabre\\\\HTTP\\\\ClientHttpException), '')\\n#1 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(275): OC\\\\Files\\\\Storage\\\\DAV->convertException(Object(Sabre\\\\HTTP\\\\ClientHttpException), '')\\n#2 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(579): OC\\\\Files\\\\Storage\\\\DAV->propfind('')\\n#3 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Common.php(175): OC\\\\Files\\\\Storage\\\\DAV->stat('')\\n#4 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Common->filemtime('')\\n#5 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Availability.php(252): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#6 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Availability->filemtime('')\\n#7 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#8 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/apps\\\/federatedfilesharing\\\/lib\\\/Command\\\/PollIncomingShares.php(110): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#9 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/apps\\\/federatedfilesharing\\\/lib\\\/Command\\\/PollIncomingShares.php(92): OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares->refreshStorageRoot(Object(OCA\\\\Files_Trashbin\\\\Storage))\\n#10 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Command\\\/Command.php(255): OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares->execute(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#11 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(960): Symfony\\\\Component\\\\Console\\\\Command\\\\Command->run(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#12 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(255): Symfony\\\\Component\\\\Console\\\\Application->doRunCommand(Object(OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares), Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#13 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(148): Symfony\\\\Component\\\\Console\\\\Application->doRun(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#14 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Console\\\/Application.php(164): Symfony\\\\Component\\\\Console\\\\Application->run(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#15 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/console.php(106): OC\\\\Console\\\\Application->run()\\n#16 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/occ(11): require_once('\\\/home\\\/deo\\\/publi...')\\n#17 {main}\",\"File\":\"\\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php\",\"Line\":865}"} {"reqId":"0jcYYTjvA7asrGLBtk0R","level":3,"time":"2019-03-29T10:07:45+00:00","remoteAddr":"","user":"--","app":"files_external","method":"--","url":"--","message":"Sabre\\HTTP\\ClientHttpException: Unauthorized"}

it's likely logged on a higher level already ?

jvillafanez · 2019-03-29T10:16:20Z

We have to check what happens if the sharing app is disabled. We might need to workaround something.

VicDeo · 2019-03-29T11:24:01Z

@jvillafanez good point. Added:

deo@reborn:~/public_html/oc-master> php occ a:d files_sharing
files_sharing disabled
deo@reborn:~/public_html/oc-master> php occ incoming-shares:poll
Polling is not possible when files_sharing app is disabled. Please enable it with 'occ app:enable files_sharing'

jvillafanez · 2019-03-29T13:43:57Z

Do we want to go a bit further and check if the sharing app is present, not just disabled? or do we assume that the sharing app will always be present?
We might need to include a comment somewhere so we make clear what happen with the mount provider if the sharing app is disabled / removed.

VicDeo · 2019-03-29T13:48:50Z

@jvillafanez I can't go further too much as there are estimates.
It seems to me that if sharing app is disabled/missing/stolen/shipped_to_Mars then federated sharing is also pointless

PVince81 · 2019-03-29T13:52:35Z

codecov fail likely due to Application.php, skipping

PVince81 · 2019-03-29T13:52:46Z

@VicDeo please backport

VicDeo · 2019-03-29T14:13:26Z

stable10: #34933

VicDeo added 2 - Developing backport-request labels Mar 26, 2019

VicDeo added this to the development milestone Mar 26, 2019

VicDeo self-assigned this Mar 26, 2019

VicDeo force-pushed the cmd-to-sync-fedshares branch from da39051 to a3b3bd8 Compare March 26, 2019 16:51

mmattel reviewed Mar 26, 2019

View reviewed changes

apps/federatedfilesharing/lib/Command/SyncIncomingShares.php Outdated Show resolved Hide resolved

VicDeo force-pushed the cmd-to-sync-fedshares branch from a3b3bd8 to 3ba7264 Compare March 26, 2019 17:32

VicDeo force-pushed the cmd-to-sync-fedshares branch 2 times, most recently from 486c85e to b13e469 Compare March 27, 2019 13:24

PVince81 suggested changes Mar 28, 2019

View reviewed changes

Add a command to check federated share updates

53b43c9

VicDeo force-pushed the cmd-to-sync-fedshares branch from b13e469 to 53b43c9 Compare March 28, 2019 11:53

VicDeo changed the title ~~Add a command to check federated share updates~~ Add a command to poll incoming federated share updates Mar 28, 2019

VicDeo changed the title ~~Add a command to poll incoming federated share updates~~ Add a command to poll incoming federated shares for updates Mar 28, 2019

PVince81 requested a review from jvillafanez March 28, 2019 12:42

VicDeo added 3 - To Review and removed 2 - Developing labels Mar 28, 2019

jvillafanez reviewed Mar 28, 2019

View reviewed changes

Init command in appinfo

8c1e701

tomneedham reviewed Mar 28, 2019

View reviewed changes

Check files_sharing app presence

06b148e

PVince81 approved these changes Mar 29, 2019

View reviewed changes

PVince81 merged commit 6b9557c into master Mar 29, 2019

PVince81 deleted the cmd-to-sync-fedshares branch March 29, 2019 13:52

mmattel mentioned this pull request Mar 29, 2019

Document occ incoming-shares:poll for update detection with federated shares owncloud/docs#856

Closed

12 tasks

VicDeo mentioned this pull request Mar 29, 2019

[stable10] Add a command to poll incoming federated shares for updates #34933

Merged

10 tasks

VicDeo removed the backport-request label Mar 29, 2019

This was referenced Apr 3, 2019

Update federatedfilesharing version to 0.4 #34958

Merged

[stable10] Update federatedfilesharing version to 0.4 #34959

Merged

individual-it mentioned this pull request May 20, 2019

acceptance tests for "occ incoming-shares:poll" #35264

Closed

lock bot locked as resolved and limited conversation to collaborators Apr 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a command to poll incoming federated shares for updates #34903

Add a command to poll incoming federated shares for updates #34903

VicDeo commented Mar 26, 2019 •

edited

Loading

VicDeo commented Mar 26, 2019

codecov bot commented Mar 26, 2019 •

edited

Loading

mmattel commented Mar 26, 2019 •

edited

Loading

PVince81 left a comment

PVince81 Mar 28, 2019

VicDeo Mar 28, 2019

jvillafanez Mar 28, 2019

VicDeo Mar 29, 2019

jvillafanez Mar 29, 2019

PVince81 Mar 29, 2019

tomneedham Mar 28, 2019

VicDeo Mar 29, 2019

PVince81 Mar 29, 2019

jvillafanez commented Mar 29, 2019

VicDeo commented Mar 29, 2019

jvillafanez commented Mar 29, 2019

VicDeo commented Mar 29, 2019

PVince81 commented Mar 29, 2019

PVince81 commented Mar 29, 2019

VicDeo commented Mar 29, 2019

Add a command to poll incoming federated shares for updates #34903

Add a command to poll incoming federated shares for updates #34903

Conversation

VicDeo commented Mar 26, 2019 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Open tasks:

VicDeo commented Mar 26, 2019

codecov bot commented Mar 26, 2019 • edited Loading

Codecov Report

mmattel commented Mar 26, 2019 • edited Loading

PVince81 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jvillafanez commented Mar 29, 2019

VicDeo commented Mar 29, 2019

jvillafanez commented Mar 29, 2019

VicDeo commented Mar 29, 2019

PVince81 commented Mar 29, 2019

PVince81 commented Mar 29, 2019

VicDeo commented Mar 29, 2019

VicDeo commented Mar 26, 2019 •

edited

Loading

codecov bot commented Mar 26, 2019 •

edited

Loading

mmattel commented Mar 26, 2019 •

edited

Loading