From 3431d547a9834dce70fab8a835862bb276c8575b Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 1 Oct 2014 15:13:10 +0200 Subject: [PATCH] fix performance issues --- apps/files_sharing/lib/share/folder.php | 47 +++++++++++ apps/files_sharing/tests/backend.php | 105 ++++++++++++++++++++++++ lib/private/share/share.php | 74 +++++++++++++---- 3 files changed, 208 insertions(+), 18 deletions(-) create mode 100644 apps/files_sharing/tests/backend.php diff --git a/apps/files_sharing/lib/share/folder.php b/apps/files_sharing/lib/share/folder.php index 4426beec636..2671f5738b7 100644 --- a/apps/files_sharing/lib/share/folder.php +++ b/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); diff --git a/apps/files_sharing/tests/backend.php b/apps/files_sharing/tests/backend.php new file mode 100644 index 00000000000..9653713a9f9 --- /dev/null +++ b/apps/files_sharing/tests/backend.php @@ -0,0 +1,105 @@ + + * + * 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 . + * + */ + +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']); + + } + +} diff --git a/lib/private/share/share.php b/lib/private/share/share.php index d861f0510e4..e580acc19c6 100644 --- a/lib/private/share/share.php +++ b/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'; + } - $where = ' `' . $column . '` = ? AND `item_type` = ? '; + $select = self::createSelectStatement(self::FORMAT_NONE, true); + + $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';