Merge pull request #11380 from owncloud/fix_performance_issue_sharing

[sharing] fix performance issues

apps/files_sharing/lib/share/file.php

@@ -49,6 +49,11 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent {
 			$path = $this->path;
 			$this->path = null;
 			return $path;
+		} else {
+			$path = \OC\Files\Filesystem::getPath($itemSource);
+			if ($path) {
+				return $path;
+			}
 		}
 		return false;
 	}

apps/files_sharing/lib/share/folder.php

@@ -21,6 +21,53 @@
 
 class OC_Share_Backend_Folder extends OC_Share_Backend_File implements OCP\Share_Backend_Collection {
 
+	/**
+	 * get shared parents
+	 *
+	 * @param int $itemSource item source ID
+	 * @param string $shareWith with whom should the item be shared
+	 * @return array with shares
+	 */
+	public function getParents($itemSource, $shareWith = null) {
+		$result = array();
+		$parent = $this->getParentId($itemSource);
+		while ($parent) {
+			$shares = \OCP\Share::getItemSharedWithUser('folder', $parent, $shareWith);
+			if ($shares) {
+				foreach ($shares as $share) {
+					$name = substr($share['path'], strrpos($share['path'], '/') + 1);
+					$share['collection']['path'] = $name;
+					$share['collection']['item_type'] = 'folder';
+					$share['file_path'] = $name;
+					$displayNameOwner = \OCP\User::getDisplayName($share['uid_owner']);
+					$displayNameShareWith = \OCP\User::getDisplayName($share['share_with']);
+					$share['displayname_owner'] = ($displayNameOwner) ? $displayNameOwner : $share['uid_owner'];
+					$share['share_with_displayname'] = ($displayNameShareWith) ? $displayNameShareWith : $share['uid_owner'];
+
+					$result[] = $share;
+				}
+			}
+			$parent = $this->getParentId($parent);
+		}
+
+		return $result;
+	}
+
+	/**
+	 * get file cache ID of parent
+	 *
+	 * @param int $child file cache ID of child
+	 * @return mixed parent ID or null
+	 */
+	private function getParentId($child) {
+		$query = \OC_DB::prepare('SELECT `parent` FROM `*PREFIX*filecache` WHERE `fileid` = ?');
+		$result = $query->execute(array($child));
+		$row = $result->fetchRow();
+		$parent = ($row) ? $row['parent'] : null;
+
+		return $parent;
+	}
+
 	public function getChildren($itemSource) {
 		$children = array();
 		$parents = array($itemSource);

apps/files_sharing/tests/backend.php

@@ -0,0 +1,105 @@
+<?php
+/**
+ * ownCloud
+ *
+ * @author Bjoern Schiessle
+ * @copyright 2014 Bjoern Schiessle <schiessle@owncloud.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or any later version.
+ *
+ * This library 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 library.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+require_once __DIR__ . '/base.php';
+
+use OCA\Files\Share;
+
+/**
+ * Class Test_Files_Sharing
+ */
+class Test_Files_Sharing_Backend extends Test_Files_Sharing_Base {
+
+	const TEST_FOLDER_NAME = '/folder_share_api_test';
+
+	public $folder;
+	public $subfolder;
+	public $subsubfolder;
+
+	function setUp() {
+		parent::setUp();
+
+		$this->folder = self::TEST_FOLDER_NAME;
+		$this->subfolder  = '/subfolder_share_backend_test';
+		$this->subsubfolder = '/subsubfolder_share_backend_test';
+
+		$this->filename = '/share-backend-test.txt';
+
+		// save file with content
+		$this->view->file_put_contents($this->filename, $this->data);
+		$this->view->mkdir($this->folder);
+		$this->view->mkdir($this->folder . $this->subfolder);
+		$this->view->mkdir($this->folder . $this->subfolder . $this->subsubfolder);
+		$this->view->file_put_contents($this->folder.$this->filename, $this->data);
+		$this->view->file_put_contents($this->folder . $this->subfolder . $this->filename, $this->data);
+		$this->view->file_put_contents($this->folder . $this->subfolder . $this->subsubfolder . $this->filename, $this->data);
+	}
+
+	function tearDown() {
+		$this->view->unlink($this->filename);
+		$this->view->deleteAll($this->folder);
+
+		parent::tearDown();
+	}
+
+	function testGetParents() {
+
+		$fileinfo1 = $this->view->getFileInfo($this->folder);
+		$fileinfo2 = $this->view->getFileInfo($this->folder . $this->subfolder . $this->subsubfolder);
+		$fileinfo3 = $this->view->getFileInfo($this->folder . $this->subfolder . $this->subsubfolder . $this->filename);
+
+		$this->assertTrue(\OCP\Share::shareItem('folder', $fileinfo1['fileid'], \OCP\Share::SHARE_TYPE_USER,
+				self::TEST_FILES_SHARING_API_USER2, 31));
+		$this->assertTrue(\OCP\Share::shareItem('folder', $fileinfo2['fileid'], \OCP\Share::SHARE_TYPE_USER,
+				self::TEST_FILES_SHARING_API_USER3, 31));
+
+		$backend = new \OC_Share_Backend_Folder();
+
+		$result = $backend->getParents($fileinfo3['fileid']);
+		$this->assertSame(2, count($result));
+
+		$count1 = 0;
+		$count2 = 0;
+		foreach($result as $r) {
+			if ($r['path'] === 'files' . $this->folder) {
+				$this->assertSame(ltrim($this->folder, '/'), $r['collection']['path']);
+				$count1++;
+			} elseif ($r['path'] === 'files' . $this->folder . $this->subfolder . $this->subsubfolder) {
+				$this->assertSame(ltrim($this->subsubfolder, '/'), $r['collection']['path']);
+				$count2++;
+			} else {
+				$this->assertTrue(false, 'unexpected result');
+			}
+		}
+
+		$this->assertSame(1, $count1);
+		$this->assertSame(1, $count2);
+
+		$result1 = $backend->getParents($fileinfo3['fileid'], self::TEST_FILES_SHARING_API_USER3);
+		$this->assertSame(1, count($result1));
+		$elemet = reset($result1);
+		$this->assertSame('files' . $this->folder . $this->subfolder . $this->subsubfolder ,$elemet['path']);
+		$this->assertSame(ltrim($this->subsubfolder, '/') ,$elemet['collection']['path']);
+
+	}
+
+}

lib/private/share/share.php

@@ -295,8 +295,17 @@ class Share extends \OC\Share\Constants {
 		$shares = array();
 
 		$column = ($itemType === 'file' || $itemType === 'folder') ? 'file_source' : 'item_source';
+		if ($itemType === 'file' || $itemType === 'folder') {
+			$column = 'file_source';
+			$where = 'INNER JOIN `*PREFIX*filecache` ON `file_source` = `*PREFIX*filecache`.`fileid` WHERE';
+		} else {
+			$column = 'item_source';
+			$where = 'WHERE';
+		}
+
+		$select = self::createSelectStatement(self::FORMAT_NONE, true);
 
-		$where = ' `' . $column . '` = ? AND `item_type` = ? ';
+		$where .= ' `' . $column . '` = ? AND `item_type` = ? ';
 		$arguments = array($itemSource, $itemType);
 		// for link shares $user === null
 		if ($user !== null) {
@@ -304,13 +313,7 @@ class Share extends \OC\Share\Constants {
 			$arguments[] = $user;
 		}
 
-		// first check if there is a db entry for the specific user
-		$query = \OC_DB::prepare(
-				'SELECT *
-					FROM
-					`*PREFIX*share`
-					WHERE' . $where
-				);
+		$query = \OC_DB::prepare('SELECT ' . $select . ' FROM `*PREFIX*share` '. $where);
 
 		$result = \OC_DB::executeAudited($query, $arguments);
 
@@ -323,7 +326,7 @@ class Share extends \OC\Share\Constants {
 			$groups = \OC_Group::getUserGroups($user);
 
 			$query = \OC_DB::prepare(
-					'SELECT `file_target`, `permissions`, `expiration`
+					'SELECT *
 						FROM
 						`*PREFIX*share`
 						WHERE
@@ -1296,7 +1299,7 @@ class Share extends \OC\Share\Constants {
 		}
 		if (isset($item)) {
 			$collectionTypes = self::getCollectionItemTypes($itemType);
-			if ($includeCollections && $collectionTypes) {
+			if ($includeCollections && $collectionTypes && !in_array('folder', $collectionTypes)) {
 				$where .= ' AND (';
 			} else {
 				$where .= ' AND';
@@ -1320,7 +1323,7 @@ class Share extends \OC\Share\Constants {
 				}
 			}
 			$queryArgs[] = $item;
-			if ($includeCollections && $collectionTypes) {
+			if ($includeCollections && $collectionTypes && !in_array('folder', $collectionTypes)) {
 				$placeholders = join(',', array_fill(0, count($collectionTypes), '?'));
 				$where .= ' OR `item_type` IN ('.$placeholders.'))';
 				$queryArgs = array_merge($queryArgs, $collectionTypes);
@@ -1434,7 +1437,7 @@ class Share extends \OC\Share\Constants {
 							$mounts[$row['storage']] = current($mountPoints);
 						}
 					}
-					if ($mounts[$row['storage']]) {
+					if (!empty($mounts[$row['storage']])) {
 						$path = $mounts[$row['storage']]->getMountPoint().$row['path'];
 						$relPath = substr($path, $root); // path relative to data/user
 						$row['path'] = rtrim($relPath, '/');
@@ -1482,7 +1485,7 @@ class Share extends \OC\Share\Constants {
 					}
 				}
 				// Check if this is a collection of the requested item type
-				if ($includeCollections && $collectionTypes && in_array($row['item_type'], $collectionTypes)) {
+				if ($includeCollections && $collectionTypes && $row['item_type'] !== 'folder' && in_array($row['item_type'], $collectionTypes)) {
 					if (($collectionBackend = self::getBackend($row['item_type']))
 						&& $collectionBackend instanceof \OCP\Share_Backend_Collection) {
 						// Collections can be inside collections, check if the item is a collection
@@ -1542,6 +1545,15 @@ class Share extends \OC\Share\Constants {
 						$toRemove = $switchedItems[$toRemove];
 					}
 					unset($items[$toRemove]);
+				} elseif ($includeCollections && $collectionTypes && in_array($row['item_type'], $collectionTypes)) {
+					// FIXME: Thats a dirty hack to improve file sharing performance,
+					// see github issue #10588 for more details
+					// Need to find a solution which works for all back-ends
+					$collectionBackend = self::getBackend($row['item_type']);
+					$sharedParents = $collectionBackend->getParents($row['item_source']);
+					foreach ($sharedParents as $parent) {
+						$collectionItems[] = $parent;
+					}
 				}
 			}
 			if (!empty($collectionItems)) {
@@ -1549,6 +1561,20 @@ class Share extends \OC\Share\Constants {
 			}
 
 			return self::formatResult($items, $column, $backend, $format, $parameters);
+		} elseif ($includeCollections && $collectionTypes && in_array('folder', $collectionTypes)) {
+			// FIXME: Thats a dirty hack to improve file sharing performance,
+			// see github issue #10588 for more details
+			// Need to find a solution which works for all back-ends
+			$collectionItems = array();
+			$collectionBackend = self::getBackend('folder');
+			$sharedParents = $collectionBackend->getParents($item, $shareWith);
+			foreach ($sharedParents as $parent) {
+				$collectionItems[] = $parent;
+			}
+			if ($limit === 1) {
+				return reset($collectionItems);
+			}
+			return self::formatResult($collectionItems, $column, $backend, $format, $parameters);
 		}
 
 		return array();
@@ -1804,6 +1830,8 @@ class Share extends \OC\Share\Constants {
 		$l = \OC::$server->getL10N('lib');
 		$result = array();
 
+		$column = ($itemType === 'file' || $itemType === 'folder') ? 'file_source' : 'item_source';
+
 		$checkReshare = self::getItemSharedWithBySource($itemType, $itemSource, self::FORMAT_NONE, null, true);
 		if ($checkReshare) {
 			// Check if attempting to share back to owner
@@ -1826,12 +1854,22 @@ class Share extends \OC\Share\Constants {
 				} else {
 					// TODO Don't check if inside folder
 					$result['parent'] = $checkReshare['id'];
-					$result['itemSource'] = $checkReshare['item_source'];
-					$result['fileSource'] = $checkReshare['file_source'];
-					$result['suggestedItemTarget'] = $checkReshare['item_target'];
-					$result['suggestedFileTarget'] = $checkReshare['file_target'];
-					$result['filePath'] = $checkReshare['file_target'];
 					$result['expirationDate'] = min($expirationDate, $checkReshare['expiration']);
+					// only suggest the same name as new target if it is a reshare of the
+					// same file/folder and not the reshare of a child
+					if ($checkReshare[$column] === $itemSource) {
+						$result['filePath'] = $checkReshare['file_target'];
+						$result['itemSource'] = $checkReshare['item_source'];
+						$result['fileSource'] = $checkReshare['file_source'];
+						$result['suggestedItemTarget'] = $checkReshare['item_target'];
+						$result['suggestedFileTarget'] = $checkReshare['file_target'];
+					} else {
+						$result['filePath'] = ($backend instanceof \OCP\Share_Backend_File_Dependent) ? $backend->getFilePath($itemSource, $uidOwner) : null;
+						$result['suggestedItemTarget'] = null;
+						$result['suggestedFileTarget'] = null;
+						$result['itemSource'] = $itemSource;
+						$result['fileSource'] = ($backend instanceof \OCP\Share_Backend_File_Dependent) ? $itemSource : null;
+					}
 				}
 			} else {
 				$message = 'Sharing %s failed, because resharing is not allowed';