Add webdav property for share info in PROPFIND response #22789

PVince81 · 2016-03-02T17:29:15Z

To test

Put this in "propfind.txt":

<?xml version="1.0"?>
<a:propfind xmlns:a="DAV:" xmlns:oc="http://owncloud.org/ns">
        <a:prop>
        <oc:id/>
        <oc:fileid/>
        <oc:owner-id/>
        <oc:owner-display-name/>
        <oc:shares/>
        </a:prop>
</a:propfind>

Then: curl -D - -X PROPFIND -H "Content-Type: application/xml" --data-binary "@propfind.txt" http://admin:admin@localhost/owncloud/remote.php/files

Make sure to have something shares, then check the "oc:shares" response.

Tasks

TODO: decide what info we want to return: for the files app we actually only need a list of share types to be able to show icons, so we might want to rename it to oc:shareTypes or something
TODO: optionally we could also return a full list of shares + recipients
TODO: check performance
TODO: check boundaries, whether $paths are resolved properly, etc
TODO: unit tests
TODO: make the web UI use it (can be done separately)

@rullzer @DeepDiver1957 @blizzz

rullzer · 2016-03-02T17:31:15Z

apps/dav/lib/connector/sabre/sharesplugin.php

+	 */
+	private function getShares($node) {
+		$shares = [];
+		$shareTypes = [0, 1, 3]; // FIXME: use constants


Constants are in \OCP\Share:: for now... I still need to think where to move those properly.

PVince81 · 2016-03-02T17:34:53Z

So far we have to call getShares() 3 times for every file. So for a folder with 100 files it would be called 300 times.

To improve this, we could at some point add a specialized API call for this in the share manager:

    $shareTypes = $this->shareManager->getOutgoingShareTypes($userId, $nodes);

where $nodes is an array of multiple nodes.
This would only produce a single select:

   SELECT DISTINCT shareType FROM *PREFIX*share WHERE item_source IN (...)

PVince81 · 2016-03-02T17:35:12Z

and of course might need to chunk the IN parameters thanks to SQLite and others...

PVince81 · 2016-03-03T08:11:49Z

TODO: make sure this also works with reshares

PVince81 · 2016-03-15T14:11:27Z

Web UI was changed to make use of the new property.
Now the following lists properly display the sharing status:

"All files"
"Tags"
"Shared with others"
"Shared with link"

However the "Favorite" list doesn't use webdav yet, so that one is still missing the share status...

Basically the only benefits of this PR are:

no more delay in the web UI until the share status appears
legacy JS sharing code can be removed (for 9.1), OC.Share.loadIcons, OC.Share.updateIcons, etc
the "Tags" list is able to display the share status thanks to the Webdav property

How about I rename "oc:share" to "oc:shareTypes" ? Is it ok to have a comma separated list or do we want more fancy <oc:shareTypes><oc:shareType>1</oc:shareType><oc:shareType>3</oc:shareType></oc:shareTypes> ? Or like the permissions <oc:shareTypes>UGL</oc:shareTypes> ? (User, Group, Link)

rullzer · 2016-03-15T14:28:50Z

I prefer the fancy way:

<oc:shareTypes>
  <oc:shareType>1</oc:shareType>
  <oc:shareType>3</oc:shareType>
</oc:shareTypes>

PVince81 · 2016-03-16T10:07:47Z

Added 0045d7b to also fix the favorite file list. While it doesn't use the new webdav property, it still needs the JS changes from this PR to work. That's why I put them together inside the same PR.

PVince81 · 2016-03-16T10:40:35Z

@rullzer ok, I settled on this format:

<oc:share-types>
    <oc:share-type>1</oc:share-type>
    <oc:share-type>3</oc:share-type>
</oc:share-types>

using dashes instead of camel case because other properies are alike.

I haven't pushed yet, will add some unit tests first.

PVince81 · 2016-03-16T11:57:43Z

Ok, I'm done.

To test:

Create a folder "sub"
Create a bunch of subfolders "sub/user", "sub/group", "sub/link", "sub/user_link", "sub/nonshared", etc
Mark all subfolders as favorite
Tag all subfolders with "tag1"
For each subfolder, share it with a "user", "group" or "link" or any combination of those
Go to "sub" inside "All files" => share statuses must be displayed
Go to "Favorites" => share statuses must be displayed
Go to "Tags" and filter by "tag1" => share statuses must be displayed
Go to "Shared with others" => share statuses must be displayed
Go to "Shared with link" => share statuses must be displayed (only link shares)

Before this fix: share statuses were missing in the "Tags" and "Favorite" section
After this fix: share statuses displayed

Note: the other sections in the steps are only for regression testing.

Please review @rullzer @nickvergessen @icewind1991 @MorrisJobke @schiesbn @SergioBertolinSG

MorrisJobke · 2016-03-16T12:22:39Z

PHP Fatal error:  Uncaught TypeError: Argument 3 passed to OCA\Files\Controller\ApiController::__construct() must implement interface OCP\IUserSession, instance of Mock_TagService_864a049d given, called in /ssd/jenkins/workspace/server-master-linux-php7-ci/database/sqlite/label/SLAVE/apps/files/tests/controller/apicontrollertest.php on line 70 and defined in /ssd/jenkins/workspace/server-master-linux-php7-ci/database/sqlite/label/SLAVE/apps/files/controller/apicontroller.php:63
Stack trace:
#0 /ssd/jenkins/workspace/server-master-linux-php7-ci/database/sqlite/label/SLAVE/apps/files/tests/controller/apicontrollertest.php(70): OCA\Files\Controller\ApiController->__construct('files', Object(Mock_IRequest_e343c747), Object(Mock_TagService_864a049d), Object(Mock_IPreview_700bdc2c))
#1 phar:///usr/local/bin/phpunit/phpunit/Framework/TestCase.php(739): OCA\Files\Controller\ApiControllerTest->setUp()
#2 phar:///usr/local/bin/phpunit/php-invoker/Invoker.php(93): PHPUnit_Framework_TestCase->runBare()
#3 phar:///usr/local/bin/phpunit/php in /ssd/jenkins/workspace/server-master-linux-php7-ci/database/sqlite/label/SLAVE/apps/files/controller/apicontroller.php on line 63

PVince81 · 2016-03-16T12:54:59Z

Weeeird, when I tested it locally it worked. Maybe I messed up my rebase.

I'll have a look.

PVince81 · 2016-03-16T12:55:17Z

Ah yes of course, the unit test of the controller needs adjusting... forgot that

PVince81 · 2016-03-16T13:30:37Z

Fixed ApiController tests

MorrisJobke · 2016-03-17T17:00:31Z

This happened after switching to master (to compare) and switch back. (I dunno if this was also broken before the switch)

MorrisJobke · 2016-03-17T17:00:43Z

Beside the above this works :)

rullzer · 2016-03-17T18:28:17Z

Ok fixed... we need to deserializer for the repots stuff...

@MorrisJobke please have another look.

rullzer · 2016-03-17T18:28:33Z

Seems we should also have intergration tests for the report methods..

MorrisJobke · 2016-03-18T08:59:43Z

Retested and works now 👍

PVince81 · 2016-03-18T10:35:55Z

@karlitschek backport to 9.0.1 ?

PVince81 · 2016-03-18T10:39:51Z

stable9: #23384

karlitschek · 2016-03-18T11:29:31Z

please backport 👍

DeepDiver1975 · 2016-03-21T08:53:27Z

Where is the necessity of the backport coming from?

DeepDiver1975 · 2016-03-21T08:56:14Z

apps/dav/lib/connector/sabre/serverfactory.php

@@ -137,6 +137,12 @@ public function createServer($baseUri,

 			if($this->userSession->isLoggedIn()) {
 				$server->addPlugin(new \OCA\DAV\Connector\Sabre\TagsPlugin($objectTree, $this->tagManager));
+				$server->addPlugin(new \OCA\DAV\Connector\Sabre\SharesPlugin(


this plugin has to be added to the v2 endpoint as well

@rullzer can I ask you to duplicate all webdav tests so that the new endpoint is tested as well? THX

Yes added to my TODO... will do

PVince81 · 2016-03-21T09:12:28Z

Where is the necessity of the backport coming from?

This is to fix #22708 which is a regression.

PVince81 · 2016-03-21T09:14:38Z

@DeepDiver1975 you already merged the backport. Can you also merge this one ?

Then we'll need to make another PR + backport to fix your comments.

Add webdav property for share info in PROPFIND response

lock · 2019-08-06T22:01:36Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

PVince81 added feature:sharing app:dav labels Mar 2, 2016

PVince81 added this to the 9.1-next milestone Mar 2, 2016

DeepDiver1975 added the in progress label Mar 2, 2016

PVince81 mentioned this pull request Mar 2, 2016

Favorite / filter by tag file lists do not have share status icon #22708

Closed

rullzer reviewed Mar 2, 2016
View reviewed changes

blizzz added the 2 - Developing label Mar 4, 2016

PVince81 force-pushed the dav-sharesproperty branch from f641b43 to 2a7f3ea Compare March 15, 2016 14:07

PVince81 force-pushed the dav-sharesproperty branch from 0045d7b to 13ddefc Compare March 16, 2016 11:54

PVince81 added 3 - To Review and removed 2 - Developing labels Mar 16, 2016

PVince81 added sev2-high regression labels Mar 16, 2016

MorrisJobke added 2 - Developing and removed 3 - To Review labels Mar 16, 2016

PVince81 force-pushed the dav-sharesproperty branch from 13ddefc to 8f5f2b9 Compare March 16, 2016 13:29

PVince81 added 3 - To Review and removed 2 - Developing labels Mar 17, 2016

Remove duplicated copyright

6e6e002

rullzer force-pushed the dav-sharesproperty branch from 6066a7d to 6e6e002 Compare March 17, 2016 18:27

MorrisJobke added 4 - To release and removed 3 - To Review labels Mar 18, 2016

PVince81 mentioned this pull request Mar 18, 2016

[stable9] Add webdav share-types property to fix favorites/tags share status icon #23384

Merged

1 task

MorrisJobke added the 9.0.1-current label Mar 21, 2016

DeepDiver1975 reviewed Mar 21, 2016
View reviewed changes

PVince81 added 2 - Developing and removed 4 - To release labels Mar 21, 2016

PVince81 added 4 - To release and removed 2 - Developing labels Mar 21, 2016

DeepDiver1975 added a commit that referenced this pull request Mar 21, 2016

Merge pull request #22789 from owncloud/dav-sharesproperty

8852fda

Add webdav property for share info in PROPFIND response

DeepDiver1975 merged commit 8852fda into master Mar 21, 2016

DeepDiver1975 deleted the dav-sharesproperty branch March 21, 2016 10:15

DeepDiver1975 removed the in progress label Mar 21, 2016

rullzer mentioned this pull request Mar 24, 2016

Desktop Sharing / Desktop Overlay Icons - show shared by me files owncloud/client#3043

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add webdav property for share info in PROPFIND response #22789

Add webdav property for share info in PROPFIND response #22789

PVince81 commented Mar 2, 2016

rullzer Mar 2, 2016

PVince81 commented Mar 2, 2016

PVince81 commented Mar 2, 2016

PVince81 commented Mar 3, 2016

PVince81 commented Mar 15, 2016

rullzer commented Mar 15, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 16, 2016

MorrisJobke commented Mar 16, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 16, 2016

MorrisJobke commented Mar 17, 2016

MorrisJobke commented Mar 17, 2016

rullzer commented Mar 17, 2016

rullzer commented Mar 17, 2016

MorrisJobke commented Mar 18, 2016

PVince81 commented Mar 18, 2016

PVince81 commented Mar 18, 2016

karlitschek commented Mar 18, 2016

DeepDiver1975 commented Mar 21, 2016

DeepDiver1975 Mar 21, 2016

rullzer Mar 21, 2016

PVince81 commented Mar 21, 2016

PVince81 commented Mar 21, 2016

lock bot commented Aug 6, 2019

Add webdav property for share info in PROPFIND response #22789

Add webdav property for share info in PROPFIND response #22789

Conversation

PVince81 commented Mar 2, 2016

To test

Tasks

rullzer Mar 2, 2016

Choose a reason for hiding this comment

PVince81 commented Mar 2, 2016

PVince81 commented Mar 2, 2016

PVince81 commented Mar 3, 2016

PVince81 commented Mar 15, 2016

rullzer commented Mar 15, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 16, 2016

MorrisJobke commented Mar 16, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 16, 2016

MorrisJobke commented Mar 17, 2016

MorrisJobke commented Mar 17, 2016

rullzer commented Mar 17, 2016

rullzer commented Mar 17, 2016

MorrisJobke commented Mar 18, 2016

PVince81 commented Mar 18, 2016

PVince81 commented Mar 18, 2016

karlitschek commented Mar 18, 2016

DeepDiver1975 commented Mar 21, 2016

DeepDiver1975 Mar 21, 2016

Choose a reason for hiding this comment

rullzer Mar 21, 2016

Choose a reason for hiding this comment

PVince81 commented Mar 21, 2016

PVince81 commented Mar 21, 2016

lock bot commented Aug 6, 2019