From e3991fbde0e97ad31af44c9f2506a8a1208d8f53 Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Tue, 26 Feb 2019 08:15:32 +0100 Subject: [PATCH 01/11] Show sharees via propfind Signed-off-by: tobiasKaminsky --- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 6 ++- apps/dav/lib/Connector/Sabre/Node.php | 57 ++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 99317f2bc1..4a15edfdf4 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -71,6 +71,7 @@ class FilesPlugin extends ServerPlugin { const MOUNT_TYPE_PROPERTYNAME = '{http://nextcloud.org/ns}mount-type'; const IS_ENCRYPTED_PROPERTYNAME = '{http://nextcloud.org/ns}is-encrypted'; const SHARE_NOTE = '{http://nextcloud.org/ns}note'; + const SHAREES_PROPERTYNAME = '{http://nextcloud.org/ns}sharees'; /** * Reference to main server object @@ -163,6 +164,7 @@ class FilesPlugin extends ServerPlugin { $server->protectedProperties[] = self::MOUNT_TYPE_PROPERTYNAME; $server->protectedProperties[] = self::IS_ENCRYPTED_PROPERTYNAME; $server->protectedProperties[] = self::SHARE_NOTE; + $server->protectedProperties[] = self::SHAREES_PROPERTYNAME; // normally these cannot be changed (RFC4918), but we want them modifiable through PROPPATCH $allowedProperties = ['{DAV:}getetag']; @@ -361,12 +363,14 @@ class FilesPlugin extends ServerPlugin { $propFind->handle(self::MOUNT_TYPE_PROPERTYNAME, function () use ($node) { return $node->getFileInfo()->getMountPoint()->getMountType(); }); - $propFind->handle(self::SHARE_NOTE, function() use ($node, $httpRequest) { return $node->getNoteFromShare( $httpRequest->getRawServerValue('PHP_AUTH_USER') ); }); + $propFind->handle(self::SHAREES_PROPERTYNAME, function() use ($node, $httpRequest) { + return $node->getShareeFromShare($httpRequest->getRawServerValue('PHP_AUTH_USER')); + }); } if ($node instanceof \OCA\DAV\Connector\Sabre\Node) { diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index f0917fe11b..4cf6835b89 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -43,6 +43,7 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share; use OCP\Share\IShare; +use OCP\Lock\ILockingProvider; abstract class Node implements \Sabre\DAV\INode { @@ -323,6 +324,62 @@ abstract class Node implements \Sabre\DAV\INode { return ''; } + /** + * @param string $user + * @return string + */ + public function getShareeFromShare($user) { + $sharees = []; + + if ($user == null) { + return $sharees; + } + $types = [ + Share::SHARE_TYPE_USER, + Share::SHARE_TYPE_REMOTE, + Share::SHARE_TYPE_GROUP, + ]; + + if ($this->getPath() === "/") { + return $sharees; + } + + $path = $this->getPath(); + + if ($path !== null) { + $userFolder = \OC::$server->getRootFolder()->getUserFolder($user); + try { + $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')); + } catch (LockedException $e) { + throw new OCSNotFoundException($this->l->t('Could not lock path')); + } + } + + foreach ($types as $shareType) { + $shares = $this->shareManager->getSharesBy($user, $shareType, $path, false, -1, 0); + foreach ($shares as $share) { + if ($share->getSharedBy() === $user) { + $sharees[] = $share->getSharedWith(); + } + } + } + return implode(', ', $sharees); + } + + /** + * Lock a Node + * + * @param \OCP\Files\Node $node + * @throws LockedException + */ + private function lock(\OCP\Files\Node $node) { + $node->lock(ILockingProvider::LOCK_SHARED); + $this->lockedNode = $node; + } + /** * @return string */ From b81fb182b3d793882a1942c852e8c3d2262b3de9 Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Mon, 22 Jul 2019 14:47:17 +0200 Subject: [PATCH 02/11] wip Signed-off-by: tobiasKaminsky --- .htaccess | 4 ++ apps/dav/lib/Connector/Sabre/FilesPlugin.php | 6 +- apps/dav/lib/Connector/Sabre/Node.php | 57 --------------- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 71 +++++++++++++++++++ 4 files changed, 76 insertions(+), 62 deletions(-) diff --git a/.htaccess b/.htaccess index cc2e0c95eb..9aa16c7cb9 100644 --- a/.htaccess +++ b/.htaccess @@ -65,3 +65,7 @@ Options -Indexes ModPagespeed Off +#### DO NOT CHANGE ANYTHING ABOVE THIS LINE #### + +ErrorDocument 403 // +ErrorDocument 404 // diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 4a15edfdf4..99317f2bc1 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -71,7 +71,6 @@ class FilesPlugin extends ServerPlugin { const MOUNT_TYPE_PROPERTYNAME = '{http://nextcloud.org/ns}mount-type'; const IS_ENCRYPTED_PROPERTYNAME = '{http://nextcloud.org/ns}is-encrypted'; const SHARE_NOTE = '{http://nextcloud.org/ns}note'; - const SHAREES_PROPERTYNAME = '{http://nextcloud.org/ns}sharees'; /** * Reference to main server object @@ -164,7 +163,6 @@ class FilesPlugin extends ServerPlugin { $server->protectedProperties[] = self::MOUNT_TYPE_PROPERTYNAME; $server->protectedProperties[] = self::IS_ENCRYPTED_PROPERTYNAME; $server->protectedProperties[] = self::SHARE_NOTE; - $server->protectedProperties[] = self::SHAREES_PROPERTYNAME; // normally these cannot be changed (RFC4918), but we want them modifiable through PROPPATCH $allowedProperties = ['{DAV:}getetag']; @@ -363,14 +361,12 @@ class FilesPlugin extends ServerPlugin { $propFind->handle(self::MOUNT_TYPE_PROPERTYNAME, function () use ($node) { return $node->getFileInfo()->getMountPoint()->getMountType(); }); + $propFind->handle(self::SHARE_NOTE, function() use ($node, $httpRequest) { return $node->getNoteFromShare( $httpRequest->getRawServerValue('PHP_AUTH_USER') ); }); - $propFind->handle(self::SHAREES_PROPERTYNAME, function() use ($node, $httpRequest) { - return $node->getShareeFromShare($httpRequest->getRawServerValue('PHP_AUTH_USER')); - }); } if ($node instanceof \OCA\DAV\Connector\Sabre\Node) { diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index 4cf6835b89..f0917fe11b 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -43,7 +43,6 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share; use OCP\Share\IShare; -use OCP\Lock\ILockingProvider; abstract class Node implements \Sabre\DAV\INode { @@ -324,62 +323,6 @@ abstract class Node implements \Sabre\DAV\INode { return ''; } - /** - * @param string $user - * @return string - */ - public function getShareeFromShare($user) { - $sharees = []; - - if ($user == null) { - return $sharees; - } - $types = [ - Share::SHARE_TYPE_USER, - Share::SHARE_TYPE_REMOTE, - Share::SHARE_TYPE_GROUP, - ]; - - if ($this->getPath() === "/") { - return $sharees; - } - - $path = $this->getPath(); - - if ($path !== null) { - $userFolder = \OC::$server->getRootFolder()->getUserFolder($user); - try { - $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')); - } catch (LockedException $e) { - throw new OCSNotFoundException($this->l->t('Could not lock path')); - } - } - - foreach ($types as $shareType) { - $shares = $this->shareManager->getSharesBy($user, $shareType, $path, false, -1, 0); - foreach ($shares as $share) { - if ($share->getSharedBy() === $user) { - $sharees[] = $share->getSharedWith(); - } - } - } - return implode(', ', $sharees); - } - - /** - * Lock a Node - * - * @param \OCP\Files\Node $node - * @throws LockedException - */ - private function lock(\OCP\Files\Node $node) { - $node->lock(ILockingProvider::LOCK_SHARED); - $this->lockedNode = $node; - } - /** * @return string */ diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index 46174c8118..e395bb7313 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -27,9 +27,13 @@ */ namespace OCA\DAV\Connector\Sabre; +use OCA\DAV\Connector\Sabre\Exception\FileLocked; use \Sabre\DAV\PropFind; use OCP\IUserSession; use OCP\Share\IShare; +use OCP\Files\NotFoundException; +use OCP\Lock\LockedException; +use \OCA\DAV\Connector\Sabre\Node; /** * Sabre Plugin to provide share-related properties @@ -37,7 +41,9 @@ use OCP\Share\IShare; class SharesPlugin extends \Sabre\DAV\ServerPlugin { const NS_OWNCLOUD = 'http://owncloud.org/ns'; + const NS_NEXTCLOUD = 'http://nextcloud.org/ns'; const SHARETYPES_PROPERTYNAME = '{http://owncloud.org/ns}share-types'; + const SHAREES_PROPERTYNAME = '{http://nextcloud.org/ns}sharees'; /** * Reference to main server object @@ -106,6 +112,7 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { $server->xml->namespacesMap[self::NS_OWNCLOUD] = 'oc'; $server->xml->elementMap[self::SHARETYPES_PROPERTYNAME] = ShareTypeList::class; $server->protectedProperties[] = self::SHARETYPES_PROPERTYNAME; + $server->protectedProperties[] = self::SHAREES_PROPERTYNAME; $this->server = $server; $this->server->on('propFind', array($this, 'handleGetProperties')); @@ -214,5 +221,69 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { return new ShareTypeList($shareTypes); }); + + $propFind->handle(self::SHAREES_PROPERTYNAME, function() use ($sabreNode) { + $test = $this->server->httpRequest->getRawServerValue('PHP_AUTH_USER'); + return $this->getShareeFromShare($sabreNode, $test); + }); + } + + /** + * @param \Sabre\DAV\INode $sabreNode + * @param string $user + * @return string + * @throws FileLocked + * @throws NotFoundException + */ + public function getShareeFromShare(\Sabre\DAV\INode $sabreNode, $user) { + $sharees = []; + + if ($user == null) { + return $sharees; + } + $types = [ + Share::SHARE_TYPE_USER, + Share::SHARE_TYPE_REMOTE, + Share::SHARE_TYPE_GROUP, + ]; + + if ($sabreNode->getPath() === "/") { + return $sharees; + } + + $path = $this->getPath(); + + if ($path !== null) { + $userFolder = \OC::$server->getRootFolder()->getUserFolder($user); + try { + $path = $userFolder->get($path); + $this->lock($path); + } catch (\OCP\Files\NotFoundException $e) { + throw new NotFoundException($this->l->t('Wrong path, file/folder doesn\'t exist')); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); + } + } + + foreach ($types as $shareType) { + $shares = $this->shareManager->getSharesBy($user, $shareType, $path, false, -1, 0); + foreach ($shares as $share) { + if ($share->getSharedBy() === $user) { + $sharees[] = $share->getSharedWith(); + } + } + } + return implode(', ', $sharees); + } + + /** + * Lock a Node + * + * @param \OCP\Files\Node $node + * @throws LockedException + */ + private function lock(\OCP\Files\Node $node) { + $node->lock(ILockingProvider::LOCK_SHARED); + $this->lockedNode = $node; } } From efa0733b040b4674cd2e9cf2ee54ee48164291d1 Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Tue, 23 Jul 2019 10:31:13 +0200 Subject: [PATCH 03/11] wip Signed-off-by: tobiasKaminsky --- .htaccess | 4 - apps/dav/lib/Connector/Sabre/SharesPlugin.php | 94 +++++++------------ 2 files changed, 34 insertions(+), 64 deletions(-) diff --git a/.htaccess b/.htaccess index 9aa16c7cb9..cc2e0c95eb 100644 --- a/.htaccess +++ b/.htaccess @@ -65,7 +65,3 @@ Options -Indexes ModPagespeed Off -#### DO NOT CHANGE ANYTHING ABOVE THIS LINE #### - -ErrorDocument 403 // -ErrorDocument 404 // diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index e395bb7313..be76171c33 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -27,13 +27,9 @@ */ namespace OCA\DAV\Connector\Sabre; -use OCA\DAV\Connector\Sabre\Exception\FileLocked; use \Sabre\DAV\PropFind; use OCP\IUserSession; use OCP\Share\IShare; -use OCP\Files\NotFoundException; -use OCP\Lock\LockedException; -use \OCA\DAV\Connector\Sabre\Node; /** * Sabre Plugin to provide share-related properties @@ -223,67 +219,45 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { }); $propFind->handle(self::SHAREES_PROPERTYNAME, function() use ($sabreNode) { - $test = $this->server->httpRequest->getRawServerValue('PHP_AUTH_USER'); - return $this->getShareeFromShare($sabreNode, $test); - }); - } + $user = $this->server->httpRequest->getRawServerValue('PHP_AUTH_USER'); - /** - * @param \Sabre\DAV\INode $sabreNode - * @param string $user - * @return string - * @throws FileLocked - * @throws NotFoundException - */ - public function getShareeFromShare(\Sabre\DAV\INode $sabreNode, $user) { - $sharees = []; - - if ($user == null) { - return $sharees; - } - $types = [ - Share::SHARE_TYPE_USER, - Share::SHARE_TYPE_REMOTE, - Share::SHARE_TYPE_GROUP, - ]; - - if ($sabreNode->getPath() === "/") { - return $sharees; - } - - $path = $this->getPath(); - - if ($path !== null) { - $userFolder = \OC::$server->getRootFolder()->getUserFolder($user); - try { - $path = $userFolder->get($path); - $this->lock($path); - } catch (\OCP\Files\NotFoundException $e) { - throw new NotFoundException($this->l->t('Wrong path, file/folder doesn\'t exist')); - } catch (LockedException $e) { - throw new FileLocked($e->getMessage(), $e->getCode(), $e); + if ($user == null) { + return []; } - } - foreach ($types as $shareType) { - $shares = $this->shareManager->getSharesBy($user, $shareType, $path, false, -1, 0); - foreach ($shares as $share) { - if ($share->getSharedBy() === $user) { - $sharees[] = $share->getSharedWith(); + if ($sabreNode->getPath() === "/") { + return []; + } + + $userFolder = \OC::$server->getRootFolder()->getUserFolder($user); + $path = $userFolder->get($sabreNode->getPath()); + + if (isset($this->cachedShareTypes[$sabreNode->getId()])) { + $shareTypes = $this->cachedShareTypes[$sabreNode->getId()]; + } else { + list($parentPath,) = \Sabre\Uri\split($sabreNode->getPath()); + if ($parentPath === '') { + $parentPath = '/'; + } + // if we already cached the folder this file is in we know there are no shares for this file + if (array_search($parentPath, $this->cachedFolders) === false) { + $node = $this->userFolder->get($sabreNode->getPath()); + $shareTypes = $this->getShareTypes($node); + } else { + return []; } } - } - return implode(', ', $sharees); - } - /** - * Lock a Node - * - * @param \OCP\Files\Node $node - * @throws LockedException - */ - private function lock(\OCP\Files\Node $node) { - $node->lock(ILockingProvider::LOCK_SHARED); - $this->lockedNode = $node; + foreach ($shareTypes as $shareType) { + $shares = $this->shareManager->getSharesBy($user, $shareType, $path, false, -1, 0); + + foreach ($shares as $share) { + if ($share->getSharedBy() === $user) { + $sharees[] = $share->getSharedWith(); + } + } + } + return implode(', ', $sharees); + }); } } From 06a9ade582c7230dbf1f34344ec33755dd929dcb Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 30 Jul 2019 22:35:44 +0200 Subject: [PATCH 04/11] Use proper caching Signed-off-by: Roeland Jago Douma --- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 79 +++++++++++++------ 1 file changed, 54 insertions(+), 25 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index be76171c33..e11d75301f 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -68,6 +68,9 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { */ private $userFolder; + /** @var IShare[] */ + private $cachedShares; + /** * @var IShare[] */ @@ -169,6 +172,41 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { return $shareTypesByFileId; } + private function getShare(\OCP\Files\Node $node): array { + $result = []; + $requestedShareTypes = [ + \OCP\Share::SHARE_TYPE_USER, + \OCP\Share::SHARE_TYPE_GROUP, + \OCP\Share::SHARE_TYPE_LINK, + \OCP\Share::SHARE_TYPE_REMOTE, + \OCP\Share::SHARE_TYPE_EMAIL, + \OCP\Share::SHARE_TYPE_ROOM, + \OCP\Share::SHARE_TYPE_CIRCLE, + ]; + foreach ($requestedShareTypes as $requestedShareType) { + // one of each type is enough to find out about the types + $shares = $this->shareManager->getSharesBy( + $this->userId, + $requestedShareType, + $node, + false, + 1 + ); + foreach ($shares as $share) { + $result[] = $share; + } + } + return $result; + } + + private function getSharesFolder(\OCP\Files\Folder $node): array { + return $this->shareManager->getSharesInFolder( + $this->userId, + $node, + true + ); + } + /** * Adds shares to propfind response * @@ -186,7 +224,10 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { // need prefetch ? if ($sabreNode instanceof \OCA\DAV\Connector\Sabre\Directory && $propFind->getDepth() !== 0 - && !is_null($propFind->getStatus(self::SHARETYPES_PROPERTYNAME)) + && ( + !is_null($propFind->getStatus(self::SHARETYPES_PROPERTYNAME)) || + !is_null($propFind->getStatus(self::SHAREES_PROPERTYNAME)) + ) ) { $folderNode = $this->userFolder->get($sabreNode->getPath()); @@ -196,6 +237,11 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { foreach ($childShares as $id => $shares) { $this->cachedShareTypes[$id] = $shares; } + + $childShares = $this->getSharesFolder($folderNode); + foreach ($childShares as $id => $shares) { + $this->cachedShares[$id] = $shares; + } } $propFind->handle(self::SHARETYPES_PROPERTYNAME, function () use ($sabreNode) { @@ -219,21 +265,8 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { }); $propFind->handle(self::SHAREES_PROPERTYNAME, function() use ($sabreNode) { - $user = $this->server->httpRequest->getRawServerValue('PHP_AUTH_USER'); - - if ($user == null) { - return []; - } - - if ($sabreNode->getPath() === "/") { - return []; - } - - $userFolder = \OC::$server->getRootFolder()->getUserFolder($user); - $path = $userFolder->get($sabreNode->getPath()); - - if (isset($this->cachedShareTypes[$sabreNode->getId()])) { - $shareTypes = $this->cachedShareTypes[$sabreNode->getId()]; + if (isset($this->cachedShares[$sabreNode->getId()])) { + $shares = $this->cachedShares[$sabreNode->getId()]; } else { list($parentPath,) = \Sabre\Uri\split($sabreNode->getPath()); if ($parentPath === '') { @@ -242,21 +275,17 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { // if we already cached the folder this file is in we know there are no shares for this file if (array_search($parentPath, $this->cachedFolders) === false) { $node = $this->userFolder->get($sabreNode->getPath()); - $shareTypes = $this->getShareTypes($node); + $shares = $this->getShare($node); } else { return []; } } - foreach ($shareTypes as $shareType) { - $shares = $this->shareManager->getSharesBy($user, $shareType, $path, false, -1, 0); - - foreach ($shares as $share) { - if ($share->getSharedBy() === $user) { - $sharees[] = $share->getSharedWith(); - } - } + $sharees = []; + foreach ($shares as $share) { + $sharees[] = $share->getSharedWith(); } + return implode(', ', $sharees); }); } From 2a4576344086adcf4c63150fc1288a850dd61ae9 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 30 Jul 2019 22:51:08 +0200 Subject: [PATCH 05/11] Use proper ShareeList This makes the XML parsing more sane ;) Signed-off-by: Roeland Jago Douma --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/lib/Connector/Sabre/ShareeList.php | 60 +++++++++++++++++++ apps/dav/lib/Connector/Sabre/SharesPlugin.php | 7 +-- 4 files changed, 63 insertions(+), 6 deletions(-) create mode 100644 apps/dav/lib/Connector/Sabre/ShareeList.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index b550e37a31..b14a5a1ab6 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -126,6 +126,7 @@ return array( 'OCA\\DAV\\Connector\\Sabre\\Server' => $baseDir . '/../lib/Connector/Sabre/Server.php', 'OCA\\DAV\\Connector\\Sabre\\ServerFactory' => $baseDir . '/../lib/Connector/Sabre/ServerFactory.php', 'OCA\\DAV\\Connector\\Sabre\\ShareTypeList' => $baseDir . '/../lib/Connector/Sabre/ShareTypeList.php', + 'OCA\\DAV\\Connector\\Sabre\\ShareeList' => $baseDir . '/../lib/Connector/Sabre/ShareeList.php', 'OCA\\DAV\\Connector\\Sabre\\SharesPlugin' => $baseDir . '/../lib/Connector/Sabre/SharesPlugin.php', 'OCA\\DAV\\Connector\\Sabre\\TagList' => $baseDir . '/../lib/Connector/Sabre/TagList.php', 'OCA\\DAV\\Connector\\Sabre\\TagsPlugin' => $baseDir . '/../lib/Connector/Sabre/TagsPlugin.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 736b77aa11..226f2838f5 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -141,6 +141,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Connector\\Sabre\\Server' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Server.php', 'OCA\\DAV\\Connector\\Sabre\\ServerFactory' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ServerFactory.php', 'OCA\\DAV\\Connector\\Sabre\\ShareTypeList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ShareTypeList.php', + 'OCA\\DAV\\Connector\\Sabre\\ShareeList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ShareeList.php', 'OCA\\DAV\\Connector\\Sabre\\SharesPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/SharesPlugin.php', 'OCA\\DAV\\Connector\\Sabre\\TagList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/TagList.php', 'OCA\\DAV\\Connector\\Sabre\\TagsPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/TagsPlugin.php', diff --git a/apps/dav/lib/Connector/Sabre/ShareeList.php b/apps/dav/lib/Connector/Sabre/ShareeList.php new file mode 100644 index 0000000000..8bfd992468 --- /dev/null +++ b/apps/dav/lib/Connector/Sabre/ShareeList.php @@ -0,0 +1,60 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\DAV\Connector\Sabre; + +use OCP\Share\IShare; +use Sabre\Xml\Element; +use Sabre\Xml\Reader; +use Sabre\Xml\Writer; +use Sabre\Xml\XmlSerializable; + +/** + * This property contains multiple "sharee" elements, each containing a share sharee + */ +class ShareeList implements XmlSerializable { + const NS_NEXTCLOUD = 'http://nextcloud.org/ns'; + + /** @var IShare[] */ + private $shares; + + public function __construct(array $shares) { + $this->shares = $shares; + } + + /** + * The xmlSerialize metod is called during xml writing. + * + * @param Writer $writer + * @return void + */ + function xmlSerialize(Writer $writer) { + foreach ($this->shares as $share) { + $writer->startElement('{' . self::NS_NEXTCLOUD . '}sharee'); + $writer->writeAttribute('type', $share->getShareType()); + $writer->write($share->getSharedWith()); + $writer->endElement(); + } + } +} diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index e11d75301f..2b486562b4 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -281,12 +281,7 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { } } - $sharees = []; - foreach ($shares as $share) { - $sharees[] = $share->getSharedWith(); - } - - return implode(', ', $sharees); + return new ShareeList($shares); }); } } From 21477bca775987d898735485eeb4fb6424eb04e6 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 30 Jul 2019 22:59:22 +0200 Subject: [PATCH 06/11] Unify share fetching Signed-off-by: Roeland Jago Douma --- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 126 ++++-------------- 1 file changed, 27 insertions(+), 99 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index 2b486562b4..eba9122995 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -69,12 +69,7 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { private $userFolder; /** @var IShare[] */ - private $cachedShares; - - /** - * @var IShare[] - */ - private $cachedShareTypes; + private $cachedShares = []; private $cachedFolders = []; @@ -94,7 +89,6 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { $this->shareManager = $shareManager; $this->userFolder = $userFolder; $this->userId = $userSession->getUser()->getUID(); - $this->cachedShareTypes = []; } /** @@ -117,61 +111,6 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { $this->server->on('propFind', array($this, 'handleGetProperties')); } - /** - * Return a list of share types for outgoing shares - * - * @param \OCP\Files\Node $node file node - * - * @return int[] array of share types - */ - private function getShareTypes(\OCP\Files\Node $node) { - $shareTypes = []; - $requestedShareTypes = [ - \OCP\Share::SHARE_TYPE_USER, - \OCP\Share::SHARE_TYPE_GROUP, - \OCP\Share::SHARE_TYPE_LINK, - \OCP\Share::SHARE_TYPE_REMOTE, - \OCP\Share::SHARE_TYPE_EMAIL, - \OCP\Share::SHARE_TYPE_ROOM, - \OCP\Share::SHARE_TYPE_CIRCLE, - ]; - foreach ($requestedShareTypes as $requestedShareType) { - // one of each type is enough to find out about the types - $shares = $this->shareManager->getSharesBy( - $this->userId, - $requestedShareType, - $node, - false, - 1 - ); - if (!empty($shares)) { - $shareTypes[] = $requestedShareType; - } - } - return $shareTypes; - } - - private function getSharesTypesInFolder(\OCP\Files\Folder $node) { - $shares = $this->shareManager->getSharesInFolder( - $this->userId, - $node, - true - ); - - $shareTypesByFileId = []; - - foreach($shares as $fileId => $sharesForFile) { - $types = array_map(function(IShare $share) { - return $share->getShareType(); - }, $sharesForFile); - $types = array_unique($types); - sort($types); - $shareTypesByFileId[$fileId] = $types; - } - - return $shareTypesByFileId; - } - private function getShare(\OCP\Files\Node $node): array { $result = []; $requestedShareTypes = [ @@ -207,6 +146,26 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { ); } + private function getShares(\Sabre\DAV\INode $sabreNode): array { + if (isset($this->cachedShares[$sabreNode->getId()])) { + $shares = $this->cachedShares[$sabreNode->getId()]; + } else { + list($parentPath,) = \Sabre\Uri\split($sabreNode->getPath()); + if ($parentPath === '') { + $parentPath = '/'; + } + // if we already cached the folder this file is in we know there are no shares for this file + if (array_search($parentPath, $this->cachedFolders) === false) { + $node = $this->userFolder->get($sabreNode->getPath()); + $shares = $this->getShare($node); + } else { + return []; + } + } + + return $shares; + } + /** * Adds shares to propfind response * @@ -231,13 +190,6 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { ) { $folderNode = $this->userFolder->get($sabreNode->getPath()); - $childShares = $this->getSharesTypesInFolder($folderNode); - $this->cachedFolders[] = $sabreNode->getPath(); - $this->cachedShareTypes[$folderNode->getId()] = $this->getShareTypes($folderNode); - foreach ($childShares as $id => $shares) { - $this->cachedShareTypes[$id] = $shares; - } - $childShares = $this->getSharesFolder($folderNode); foreach ($childShares as $id => $shares) { $this->cachedShares[$id] = $shares; @@ -245,41 +197,17 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { } $propFind->handle(self::SHARETYPES_PROPERTYNAME, function () use ($sabreNode) { - if (isset($this->cachedShareTypes[$sabreNode->getId()])) { - $shareTypes = $this->cachedShareTypes[$sabreNode->getId()]; - } else { - list($parentPath,) = \Sabre\Uri\split($sabreNode->getPath()); - if ($parentPath === '') { - $parentPath = '/'; - } - // if we already cached the folder this file is in we know there are no shares for this file - if (array_search($parentPath, $this->cachedFolders) === false) { - $node = $this->userFolder->get($sabreNode->getPath()); - $shareTypes = $this->getShareTypes($node); - } else { - return []; - } - } + $shares = $this->getShares($sabreNode); + + $shareTypes = array_unique(array_map(function(IShare $share) { + return $share->getShareType(); + }, $shares)); return new ShareTypeList($shareTypes); }); $propFind->handle(self::SHAREES_PROPERTYNAME, function() use ($sabreNode) { - if (isset($this->cachedShares[$sabreNode->getId()])) { - $shares = $this->cachedShares[$sabreNode->getId()]; - } else { - list($parentPath,) = \Sabre\Uri\split($sabreNode->getPath()); - if ($parentPath === '') { - $parentPath = '/'; - } - // if we already cached the folder this file is in we know there are no shares for this file - if (array_search($parentPath, $this->cachedFolders) === false) { - $node = $this->userFolder->get($sabreNode->getPath()); - $shares = $this->getShare($node); - } else { - return []; - } - } + $shares = $this->getShares($sabreNode); return new ShareeList($shares); }); From 42f00e80c7175ab199cb6d2bfae358c0cbb0517b Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Wed, 31 Jul 2019 13:42:25 +0200 Subject: [PATCH 07/11] add display name to sharee list Signed-off-by: tobiasKaminsky --- apps/dav/lib/Connector/Sabre/ShareeList.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/ShareeList.php b/apps/dav/lib/Connector/Sabre/ShareeList.php index 8bfd992468..642b57b0c7 100644 --- a/apps/dav/lib/Connector/Sabre/ShareeList.php +++ b/apps/dav/lib/Connector/Sabre/ShareeList.php @@ -52,8 +52,9 @@ class ShareeList implements XmlSerializable { function xmlSerialize(Writer $writer) { foreach ($this->shares as $share) { $writer->startElement('{' . self::NS_NEXTCLOUD . '}sharee'); - $writer->writeAttribute('type', $share->getShareType()); - $writer->write($share->getSharedWith()); + $writer->writeElement("id", $share->getSharedWith()); + $writer->writeElement("displayName", $share->getSharedWithDisplayName()); + $writer->writeElement('type', $share->getShareType()); $writer->endElement(); } } From 85d5400314d3b01563b4af4ab1c30c3cf2d79722 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 1 Aug 2019 10:02:25 +0200 Subject: [PATCH 08/11] Fix tests Signed-off-by: Roeland Jago Douma --- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 2 + .../unit/Connector/Sabre/SharesPluginTest.php | 100 +++++++----------- 2 files changed, 39 insertions(+), 63 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index eba9122995..ddc9312771 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -158,6 +158,7 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { if (array_search($parentPath, $this->cachedFolders) === false) { $node = $this->userFolder->get($sabreNode->getPath()); $shares = $this->getShare($node); + $this->cachedShares[$sabreNode->getId()] = $shares; } else { return []; } @@ -190,6 +191,7 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { ) { $folderNode = $this->userFolder->get($sabreNode->getPath()); + $this->cachedFolders[] = $sabreNode->getPath(); $childShares = $this->getSharesFolder($folderNode); foreach ($childShares as $id => $shares) { $this->cachedShares[$id] = $shares; diff --git a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php index d4dd3b49f3..4adc182692 100644 --- a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php @@ -68,28 +68,17 @@ class SharesPluginTest extends \Test\TestCase { public function setUp() { parent::setUp(); $this->server = new \Sabre\DAV\Server(); - $this->tree = $this->getMockBuilder(Tree::class) - ->disableOriginalConstructor() - ->getMock(); - $this->shareManager = $this->getMockBuilder(IManager::class) - ->disableOriginalConstructor() - ->getMock(); - $user = $this->getMockBuilder(IUser::class) - ->disableOriginalConstructor() - ->getMock(); + $this->tree = $this->createMock(Tree::class); + $this->shareManager = $this->createMock(IManager::class); + $user = $this->createMock(IUser::class); $user->expects($this->once()) ->method('getUID') ->will($this->returnValue('user1')); - $userSession = $this->getMockBuilder(IUserSession::class) - ->disableOriginalConstructor() - ->getMock(); + $userSession = $this->createMock(IUserSession::class); $userSession->expects($this->once()) ->method('getUser') ->will($this->returnValue($user)); - - $this->userFolder = $this->getMockBuilder(Folder::class) - ->disableOriginalConstructor() - ->getMock(); + $this->userFolder = $this->createMock(Folder::class); $this->plugin = new \OCA\DAV\Connector\Sabre\SharesPlugin( $this->tree, @@ -135,7 +124,10 @@ class SharesPluginTest extends \Test\TestCase { ) ->will($this->returnCallback(function($userId, $requestedShareType, $node, $flag, $limit) use ($shareTypes){ if (in_array($requestedShareType, $shareTypes)) { - return ['dummyshare']; + $share = $this->createMock(IShare::class); + $share->method('getShareType') + ->willReturn($requestedShareType); + return [$share]; } return []; })); @@ -162,30 +154,18 @@ class SharesPluginTest extends \Test\TestCase { * @dataProvider sharesGetPropertiesDataProvider */ public function testPreloadThenGetProperties($shareTypes) { - $sabreNode1 = $this->getMockBuilder(File::class) - ->disableOriginalConstructor() - ->getMock(); - $sabreNode1->expects($this->any()) - ->method('getId') - ->will($this->returnValue(111)); - $sabreNode1->expects($this->any()) - ->method('getPath'); - $sabreNode2 = $this->getMockBuilder(File::class) - ->disableOriginalConstructor() - ->getMock(); - $sabreNode2->expects($this->any()) - ->method('getId') - ->will($this->returnValue(222)); - $sabreNode2->expects($this->any()) - ->method('getPath') - ->will($this->returnValue('/subdir/foo')); + $sabreNode1 = $this->createMock(File::class); + $sabreNode1->method('getId') + ->willReturn(111); + $sabreNode2 = $this->createMock(File::class); + $sabreNode2->method('getId') + ->willReturn(222); + $sabreNode2->method('getPath') + ->willReturn('/subdir/foo'); - $sabreNode = $this->getMockBuilder(Directory::class) - ->disableOriginalConstructor() - ->getMock(); - $sabreNode->expects($this->any()) - ->method('getId') - ->will($this->returnValue(123)); + $sabreNode = $this->createMock(Directory::class); + $sabreNode->method('getId') + ->willReturn(123); // never, because we use getDirectoryListing from the Node API instead $sabreNode->expects($this->never()) ->method('getChildren'); @@ -194,29 +174,19 @@ class SharesPluginTest extends \Test\TestCase { ->will($this->returnValue('/subdir')); // node API nodes - $node = $this->getMockBuilder(Folder::class) - ->disableOriginalConstructor() - ->getMock(); - $node->expects($this->any()) - ->method('getId') - ->will($this->returnValue(123)); - $node1 = $this->getMockBuilder(File::class) - ->disableOriginalConstructor() - ->getMock(); - $node1->expects($this->any()) - ->method('getId') - ->will($this->returnValue(111)); - $node2 = $this->getMockBuilder(File::class) - ->disableOriginalConstructor() - ->getMock(); - $node2->expects($this->any()) - ->method('getId') - ->will($this->returnValue(222)); + $node = $this->createMock(Folder::class); + $node->method('getId') + ->willReturn(123); + $node1 = $this->createMock(File::class); + $node1->method('getId') + ->willReturn(111); + $node2 = $this->createMock(File::class); + $node2->method('getId') + ->willReturn(222); - $this->userFolder->expects($this->once()) - ->method('get') + $this->userFolder->method('get') ->with('/subdir') - ->will($this->returnValue($node)); + ->willReturn($node); $dummyShares = array_map(function($type) { $share = $this->getMockBuilder(IShare::class)->getMock(); @@ -235,9 +205,13 @@ class SharesPluginTest extends \Test\TestCase { $this->equalTo(false), $this->equalTo(1) ) - ->will($this->returnCallback(function($userId, $requestedShareType, $node, $flag, $limit) use ($shareTypes){ + ->will($this->returnCallback(function($userId, $requestedShareType, $node, $flag, $limit) use ($shareTypes, $dummyShares){ if ($node->getId() === 111 && in_array($requestedShareType, $shareTypes)) { - return ['dummyshare']; + foreach ($dummyShares as $dummyShare) { + if ($dummyShare->getShareType() === $requestedShareType) { + return [$dummyShare]; + } + } } return []; From 82ae6fb6d31274a84040b1984a4f35c79c8a400a Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Tue, 6 Aug 2019 10:32:46 +0200 Subject: [PATCH 09/11] use namespace everywhere no camelCase Signed-off-by: tobiasKaminsky --- apps/dav/lib/Connector/Sabre/ShareeList.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/ShareeList.php b/apps/dav/lib/Connector/Sabre/ShareeList.php index 642b57b0c7..a9e9dfc280 100644 --- a/apps/dav/lib/Connector/Sabre/ShareeList.php +++ b/apps/dav/lib/Connector/Sabre/ShareeList.php @@ -52,9 +52,9 @@ class ShareeList implements XmlSerializable { function xmlSerialize(Writer $writer) { foreach ($this->shares as $share) { $writer->startElement('{' . self::NS_NEXTCLOUD . '}sharee'); - $writer->writeElement("id", $share->getSharedWith()); - $writer->writeElement("displayName", $share->getSharedWithDisplayName()); - $writer->writeElement('type', $share->getShareType()); + $writer->writeElement('{' . self::NS_NEXTCLOUD . '}id', $share->getSharedWith()); + $writer->writeElement('{' . self::NS_NEXTCLOUD . '}display-name', $share->getSharedWithDisplayName()); + $writer->writeElement('{' . self::NS_NEXTCLOUD . '}type', $share->getShareType()); $writer->endElement(); } } From eb564e82de449128007ccb9748d698ab65a45591 Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Thu, 8 Aug 2019 11:01:23 +0200 Subject: [PATCH 10/11] get all shares, not only first one per type Signed-off-by: tobiasKaminsky --- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index ddc9312771..5edf28e6e5 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -123,13 +123,12 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { \OCP\Share::SHARE_TYPE_CIRCLE, ]; foreach ($requestedShareTypes as $requestedShareType) { - // one of each type is enough to find out about the types $shares = $this->shareManager->getSharesBy( $this->userId, $requestedShareType, $node, false, - 1 + -1 ); foreach ($shares as $share) { $result[] = $share; From a85392615d74f08a8ec62eca5d9ccc94294d89ec Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 11 Aug 2019 21:34:28 +0200 Subject: [PATCH 11/11] Fix tests Signed-off-by: Roeland Jago Douma --- apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php index 4adc182692..af8208791e 100644 --- a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php @@ -120,7 +120,7 @@ class SharesPluginTest extends \Test\TestCase { $this->anything(), $this->anything(), $this->equalTo(false), - $this->equalTo(1) + $this->equalTo(-1) ) ->will($this->returnCallback(function($userId, $requestedShareType, $node, $flag, $limit) use ($shareTypes){ if (in_array($requestedShareType, $shareTypes)) { @@ -203,7 +203,7 @@ class SharesPluginTest extends \Test\TestCase { $this->anything(), $this->anything(), $this->equalTo(false), - $this->equalTo(1) + $this->equalTo(-1) ) ->will($this->returnCallback(function($userId, $requestedShareType, $node, $flag, $limit) use ($shareTypes, $dummyShares){ if ($node->getId() === 111 && in_array($requestedShareType, $shareTypes)) {