Show sharees via propfind by tobiasKaminsky · Pull Request #14429 · nextcloud/server

tobiasKaminsky · 2019-02-28T09:30:51Z

❗ TODO: once this is merged, change/check OCFileListAdapter: showShareAvatar boolean ❗

Steps to test

create a file "share"
share this file to userA and userB
do a propfind with nc:sharees/, e.g.:

curl --request PROPFIND \
--url http://localhost/nc/remote.php/webdav/share \
--header 'authorization: Basic dG9iaTplOGVrei1SbVpaVC1DMkdDTi01clk2My1BeTl5QQ==' \
--header 'content-type: application/xml' \
--header 'ocs-apirequest: true' \
--cookie 'ocjycgrudn78=vgovrq3bgraie6rs6ai2sl42ag; oc_sessionPassphrase=oyqxSU8b6n4N8Aj0yfObGeheppYvsdzgMhEkMgHw76nWBRLVRRM%252FZcjxqQ5QHtm3knScFxIKIjCzDasJYhLEYxRsvabseMjOqDSa%252BNSkI%252Fi%252FDrm6cKlWmA4QBr3NtcGI; nc_sameSiteCookielax=true; nc_sameSiteCookiestrict=true' \
--data '<?xml version="1.0" encoding="UTF-8"?>
<D:propfind xmlns:D="DAV:">
<D:prop>
<sharees xmlns="http://nextcloud.org/ns" />
</D:prop></D:propfind>
'

see:
- Federated sharee: <nc:sharees>tobi@http://localhost/nc15</nc:sharees>
- Multiple sharees on same server: <nc:sharees>admin, Tobi Kaminsky</nc:sharees>
- One sharee on same server: <nc:sharees>admin</nc:sharees>

Please note that this shows only the user you have created & shared a file to.
The other way around (receiving a share) is already exists via < owner-id> and < owner-display-name>.

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

blizzz

too ambiguous maybe

blizzz · 2019-02-28T10:41:36Z

+ Share::SHARE_TYPE_ROOM
+ ];
+
+ if ($this->getPath() === "/") {


or-connect it with the condition in line 303

blizzz · 2019-02-28T10:47:29Z

+ }
+ }
+ }
+ return implode(', ', $sharees);


ain't it better return the array and format it where the output is being handled (the callee)? This also only works as long as "," is prohibited as identifier. Further, you have all the types mixed in and identifiers are not unique across those types. → What do you want to achieve eventually?

If I simply return $sharees, I do get that as plain string, e.g. "adminuser1user2".
If you have a better way, please feel free to change it or give me a hint.
I do not "care" as long as the client has a list of userIds of all sharees.

I removed Circle and Rooms, so that now only user, remote user and groups are returned.

blizzz · 2019-02-28T16:38:33Z

as discussed on irc, we agreed about not returning grouped by type and and each user, group etc. is inside an id-tag within the appropriate element.

rullzer

So I knew this looked familiar.

Because the way this is implemented it won't scale. If you request this property for a user on a share with 200 entries. It will fire off 300 SQL requests.

We should bundle it with https://github.com/nextcloud/server/blob/master/apps/dav/lib/Connector/Sabre/SharesPlugin.php

rullzer · 2019-03-01T09:39:49Z

+ $path = $userFolder->get($path);
+ $this->lock($path);
+ } catch (\OCP\Files\NotFoundException $e) {
+ throw new OCSNotFoundException($this->l->t('Wrong path, file/folder doesn\'t exist'));


This should not be here

rullzer · 2019-03-01T09:39:59Z

+ } catch (\OCP\Files\NotFoundException $e) {
+ throw new OCSNotFoundException($this->l->t('Wrong path, file/folder doesn\'t exist'));
+ } catch (LockedException $e) {
+ throw new OCSNotFoundException($this->l->t('Could not lock path'));


Same. This is not an OCS endpoint

tobiasKaminsky · 2019-07-02T06:58:49Z

Bump, can anyone help me here? Or direct me in the right direction? :-)

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

tobiasKaminsky · 2019-07-23T08:35:22Z

I really would like to have this in.

I changed it as good as I can and can help/rewrite if needed.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

This makes the XML parsing more sane ;) Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer · 2019-07-31T11:12:12Z

Let me fix the tests this evening

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

rullzer · 2019-07-31T20:39:44Z

Ok so the test require some more work as they were/are not the cleanest. I'll have a look tomorrow.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer · 2019-08-02T09:36:34Z

Test all look good. Time for reviews.

rullzer

All good from my part

rullzer · 2019-08-02T09:37:31Z

Small note. Once we have this we could also enhance the server UI.
We could show who a file is shared to more easily etc.

georgehrke · 2019-08-06T08:09:45Z

Besides those two remarks it looks good and works

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

tobiasKaminsky · 2019-08-06T08:34:01Z

Can you test this please with an admin account:

<nc:sharees>
<nc:sharee>
<nc:id>admin</nc:id>
<nc:display-name/>
<nc:type>1</nc:type>
</nc:sharee>
<nc:sharee>
<nc:id>user1</nc:id>
<nc:display-name>User One</nc:display-name>
<nc:type>0</nc:type>
</nc:sharee>
<nc:sharee>
<nc:id>user2</nc:id>
<nc:display-name>user2</nc:display-name>
<nc:type>0</nc:type>
</nc:sharee>
</nc:sharees>

For some reason the display name of an admin is missing, but it should be at least userId.
It is working for other users…
I just want to be sure if this is a problem of my local system…

georgehrke · 2019-08-06T08:41:37Z

+ foreach ($this->shares as $share) {
+ $writer->startElement('{' . self::NS_NEXTCLOUD . '}sharee');
+ $writer->writeElement('{' . self::NS_NEXTCLOUD . '}id', $share->getSharedWith());
+ $writer->writeElement('{' . self::NS_NEXTCLOUD . '}display-name', $share->getSharedWithDisplayName());


Suggested change
$writer->writeElement('{' . self::NS_NEXTCLOUD . '}display-name', $share->getSharedWithDisplayName());
$writer->writeElement('{' . self::NS_NEXTCLOUD . '}display-name', $share->getSharedWithDisplayName() ?? $share->getSharedWith());

I wrote a test for android library and there the problem also exists.
It seems that always one of the display names is set to userId.

tobiasKaminsky · 2019-08-06T12:04:58Z

BUG:

create folder "testFolder"
share it to user1, user2
query "/remote.php/webdav/", see

 <d:response>
<d:href>/remote.php/webdav/testFolder/</d:href>
<d:propstat>
<d:prop>
<nc:sharees>
<nc:sharee>
<nc:id>user1</nc:id>
<nc:display-name>User One</nc:display-name>
<nc:type>0</nc:type>
</nc:sharee>
<nc:sharee>
<nc:id>user2</nc:id>
<nc:display-name>user2</nc:display-name>
<nc:type>0</nc:type>
</nc:sharee>
</nc:sharees>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>

query "/remote.php/webdav/testFolder", see:

 <d:response>
<d:href>/remote.php/webdav/testFolder/</d:href>
<d:propstat>
<d:prop>
<nc:sharees>
<nc:sharee>
<nc:id>user1</nc:id>
<nc:display-name>User One</nc:display-name>
<nc:type>0</nc:type>
</nc:sharee>
</nc:sharees>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>

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

tobiasKaminsky · 2019-08-08T09:01:39Z

Bug fixed via latest commit.

tobiasKaminsky · 2019-08-09T06:32:54Z

From my side this is ready to get in

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer · 2019-08-11T19:35:01Z

Fixed the failing tests

georgehrke

Seems to do what its supposed do and webdav compliance wise I'm also happy with it 👍

tobiasKaminsky requested review from blizzz and rullzer February 28, 2019 09:31

tobiasKaminsky added enhancement 3. to review Waiting for reviews feature: sharing feature: dav labels Feb 28, 2019

tobiasKaminsky added this to the Nextcloud 16 milestone Feb 28, 2019

blizzz reviewed Feb 28, 2019
View reviewed changes

tobiasKaminsky force-pushed the shareesOnDav branch from 40f086b to 3ff5424Compare February 28, 2019 13:52

rullzer requested changes Mar 1, 2019
View reviewed changes

MorrisJobke mentioned this pull request Mar 4, 2019
16.0.0 Alpha 1 #14501
Merged
45 tasks

rullzer mentioned this pull request Mar 4, 2019
Add note to dav endpoint #12978
Merged

rullzer modified the milestones: Nextcloud 16, Nextcloud 17 Mar 4, 2019

tobiasKaminsky force-pushed the shareesOnDav branch from 3ff5424 to 221a8c6Compare March 18, 2019 07:57

tobiasKaminsky mentioned this pull request Mar 18, 2019
Show sharees nextcloud/android#3765
Merged
6 tasks

MorrisJobke mentioned this pull request Jul 17, 2019
17.0.0 Beta 1 #16408
Merged
28 tasks

tobiasKaminsky added 2 commits July 22, 2019 13:59

Show sharees via propfind
e3991fb
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

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

tobiasKaminsky force-pushed the shareesOnDav branch 2 times, most recently from b7c0bdb to b965c79Compare July 23, 2019 08:33

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

tobiasKaminsky force-pushed the shareesOnDav branch from b965c79 to efa0733Compare July 23, 2019 08:34

tobiasKaminsky requested review from MorrisJobke, blizzz and rullzer July 23, 2019 08:34

Use proper caching
06a9ade
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer added 2 commits July 30, 2019 22:51

Use proper ShareeList
2a45763
This makes the XML parsing more sane ;) Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

Unify share fetching
21477bc
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

add display name to sharee list
42f00e8
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

This was referenced Jul 31, 2019
Change sharees format nextcloud/android-library#325
Merged
Sharees will be in NC17 nextcloud/android#4284
Merged

Fix tests
85d5400
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer requested review from georgehrke and juliusknorr August 2, 2019 09:36

rullzer approved these changes Aug 2, 2019
View reviewed changes

georgehrke requested changes Aug 6, 2019
View reviewed changes

Comment threadapps/dav/lib/Connector/Sabre/ShareeList.php Outdated
Comment threadapps/dav/lib/Connector/Sabre/ShareeList.php Outdated

use namespace everywhere
82ae6fb
no camelCase Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

georgehrke reviewed Aug 6, 2019
View reviewed changes

get all shares, not only first one per type
eb564e8
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

tobiasKaminsky requested a review from georgehrke August 9, 2019 06:32

Fix tests
a853926
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

georgehrke approved these changes Aug 13, 2019
View reviewed changes

rullzer merged commit 34868f0 into nextcloud:masterAug 13, 2019

tobiasKaminsky deleted the shareesOnDav branch August 14, 2019 04:47

	$writer->writeElement('{' . self::NS_NEXTCLOUD . '}display-name', $share->getSharedWithDisplayName());
	$writer->writeElement('{' . self::NS_NEXTCLOUD . '}display-name', $share->getSharedWithDisplayName() ?? $share->getSharedWith());

Uh oh!

Uh oh!

Conversation

tobiasKaminsky commented Feb 28, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blizzz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blizzz commented Feb 28, 2019

Uh oh!

rullzer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky commented Jul 2, 2019

Uh oh!

tobiasKaminsky commented Jul 23, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rullzer commented Jul 31, 2019

Uh oh!

rullzer commented Jul 31, 2019

Uh oh!

rullzer commented Aug 2, 2019

Uh oh!

rullzer left a comment

Choose a reason for hiding this comment

Uh oh!

rullzer commented Aug 2, 2019

Uh oh!

Uh oh!

Uh oh!

georgehrke commented Aug 6, 2019

Uh oh!

tobiasKaminsky commented Aug 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky commented Aug 6, 2019

Uh oh!

tobiasKaminsky commented Aug 8, 2019

Uh oh!

tobiasKaminsky commented Aug 9, 2019

Uh oh!

rullzer commented Aug 11, 2019

Uh oh!

georgehrke left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tobiasKaminsky commented Feb 28, 2019•
edited
Loading

tobiasKaminsky commented Jul 23, 2019•
edited
Loading