Merge pull request #2068 from nextcloud/sharetypesinfolder-no-getdirectorycontent

remove unneeded getDirectoryContent when getting share types for a folder

2 files changed, 26 insertions, 22 deletions

diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php
index 56d76e66184..33e79364758 100644
--- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php
@@ -67,6 +67,8 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
 	 */
 	private $cachedShareTypes;
 
+	private $cachedFolders = [];
+
 	/**
 	 * @param \Sabre\DAV\Tree $tree tree
 	 * @param IUserSession $userSession user session
@@ -143,24 +145,18 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
 			false
 		);
 
-		$children = $node->getDirectoryListing();
+		$shareTypesByFileId = [];
 
-		$values = array_map(function (\OCP\Files\Node $node) use ($shares) {
-			/** @var IShare[] $shares */
-			$shares = (isset($shares[$node->getId()])) ? $shares[$node->getId()] : [];
+		foreach($shares as $fileId => $sharesForFile) {
 			$types = array_map(function(IShare $share) {
 				return $share->getShareType();
-			}, $shares);
+			}, $sharesForFile);
 			$types = array_unique($types);
 			sort($types);
-			return $types;
-		}, $children);
-
-		$keys = array_map(function (\OCP\Files\Node $node) {
-			return $node->getId();
-		}, $children);
+			$shareTypesByFileId[$fileId] = $types;
+		}
 
-		return array_combine($keys, $values);
+		return $shareTypesByFileId;
 	}
 
 	/**
@@ -185,6 +181,7 @@ 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;
@@ -195,8 +192,17 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
 			if (isset($this->cachedShareTypes[$sabreNode->getId()])) {
 				$shareTypes = $this->cachedShareTypes[$sabreNode->getId()];
 			} else {
-				$node = $this->userFolder->get($sabreNode->getPath());
-				$shareTypes = $this->getShareTypes($node);
+				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 new ShareTypeList($shareTypes);
diff --git a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php
index 2e17c7d0b38..9d8a66f24b6 100644
--- a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php
@@ -97,7 +97,7 @@ class SharesPluginTest extends \Test\TestCase {
 		$sabreNode->expects($this->any())
 			->method('getId')
 			->will($this->returnValue(123));
-		$sabreNode->expects($this->once())
+		$sabreNode->expects($this->any())
 			->method('getPath')
 			->will($this->returnValue('/subdir'));
 
@@ -155,7 +155,7 @@ class SharesPluginTest extends \Test\TestCase {
 		$sabreNode1->expects($this->any())
 			->method('getId')
 			->will($this->returnValue(111));
-		$sabreNode1->expects($this->never())
+		$sabreNode1->expects($this->any())
 			->method('getPath');
 		$sabreNode2 = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\File')
 			->disableOriginalConstructor()
@@ -163,8 +163,9 @@ class SharesPluginTest extends \Test\TestCase {
 		$sabreNode2->expects($this->any())
 			->method('getId')
 			->will($this->returnValue(222));
-		$sabreNode2->expects($this->never())
-			->method('getPath');
+		$sabreNode2->expects($this->any())
+			->method('getPath')
+			->will($this->returnValue('/subdir/foo'));
 
 		$sabreNode = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\Directory')
 			->disableOriginalConstructor()
@@ -198,9 +199,6 @@ class SharesPluginTest extends \Test\TestCase {
 		$node2->expects($this->any())
 			->method('getId')
 			->will($this->returnValue(222));
-		$node->expects($this->once())
-			->method('getDirectoryListing')
-			->will($this->returnValue([$node1, $node2]));
 
 		$this->userFolder->expects($this->once())
 			->method('get')
@@ -208,7 +206,7 @@ class SharesPluginTest extends \Test\TestCase {
 			->will($this->returnValue($node));
 		
 		$dummyShares = array_map(function($type) {
-			$share = $this->getMock('\OCP\Share\IShare');
+			$share = $this->getMockBuilder('\OCP\Share\IShare')->getMock();
 			$share->expects($this->any())
 				->method('getShareType')
 				->will($this->returnValue($type));