Merge pull request #14429 from tobiasKaminsky/shareesOnDav

Show sharees via propfind
2019-08-13 20:22:47 +02:00 · 2019-08-13 20:22:47 +02:00 · 34868f0964
commit 34868f0964
parent 3a99ef30df a85392615d
5 changed files with 150 additions and 115 deletions
--- 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',
--- 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',
--- a/apps/dav/lib/Connector/Sabre/ShareeList.php
+++ b/apps/dav/lib/Connector/Sabre/ShareeList.php
@ -0,0 +1,61 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019, Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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->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();
+		}
+	}
+}
--- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php
@ -37,7 +37,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
@ -66,10 +68,8 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
 	 */
 	private $userFolder;

-	/**
-	 * @var IShare[]
-	 */
-	private $cachedShareTypes;
+	/** @var IShare[] */
+	private $cachedShares = [];

 	private $cachedFolders = [];

@ -89,7 +89,6 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
 		$this->shareManager = $shareManager;
 		$this->userFolder = $userFolder;
 		$this->userId = $userSession->getUser()->getUID();
-		$this->cachedShareTypes = [];
 	}

 	/**
@ -106,20 +105,14 @@ 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'));
 	}

-	/**
-	 * 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 = [];
+	private function getShare(\OCP\Files\Node $node): array {
+		$result = [];
 		$requestedShareTypes = [
 			\OCP\Share::SHARE_TYPE_USER,
 			\OCP\Share::SHARE_TYPE_GROUP,
@ -130,40 +123,47 @@ 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
 			);
-			if (!empty($shares)) {
-				$shareTypes[] = $requestedShareType;
+			foreach ($shares as $share) {
+				$result[] = $share;
 			}
 		}
-		return $shareTypes;
+		return $result;
 	}

-	private function getSharesTypesInFolder(\OCP\Files\Folder $node) {
-		$shares = $this->shareManager->getSharesInFolder(
+	private function getSharesFolder(\OCP\Files\Folder $node): array {
+		return $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;
+	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);
+				$this->cachedShares[$sabreNode->getId()] = $shares;
+			} else {
+				return [];
+			}
 		}

-		return $shareTypesByFileId;
+		return $shares;
 	}

 	/**
@ -183,36 +183,34 @@ 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());

-			$childShares = $this->getSharesTypesInFolder($folderNode);
 			$this->cachedFolders[] = $sabreNode->getPath();
-			$this->cachedShareTypes[$folderNode->getId()] = $this->getShareTypes($folderNode);
+			$childShares = $this->getSharesFolder($folderNode);
 			foreach ($childShares as $id => $shares) {
-				$this->cachedShareTypes[$id] = $shares;
+				$this->cachedShares[$id] = $shares;
 			}
 		}

 		$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) {
+			$shares = $this->getShares($sabreNode);
+
+			return new ShareeList($shares);
+		});
 	}
 }
--- 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,
@ -131,11 +120,14 @@ 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)) {
-					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();
@ -233,11 +203,15 @@ 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){
+			->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 [];