[Pkg-owncloud-commits] [owncloud] 244/258: fix performance issues
David Prévot
taffit at moszumanska.debian.org
Sat Oct 11 17:22:41 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch master
in repository owncloud.
commit 5a37703b3ff4ac42294e19917060187666fc917c
Author: Bjoern Schiessle <schiessle at owncloud.com>
Date: Wed Oct 1 15:13:10 2014 +0200
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(-)
diff --git a/apps/files_sharing/lib/share/folder.php b/apps/files_sharing/lib/share/folder.php
index 4426bee..2671f57 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 0000000..9653713
--- /dev/null
+++ b/apps/files_sharing/tests/backend.php
@@ -0,0 +1,105 @@
+<?php
+/**
+ * ownCloud
+ *
+ * @author Bjoern Schiessle
+ * @copyright 2014 Bjoern Schiessle <schiessle at 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']);
+
+ }
+
+}
diff --git a/lib/private/share/share.php b/lib/private/share/share.php
index cfdc8aa..a5010ec 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';
+ }
+
+ $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
@@ -1292,7 +1295,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';
@@ -1316,7 +1319,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);
@@ -1430,7 +1433,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, '/');
@@ -1478,7 +1481,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
@@ -1538,6 +1541,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)) {
@@ -1545,6 +1557,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();
@@ -1798,6 +1824,8 @@ class Share extends \OC\Share\Constants {
$backend = self::getBackend($itemType);
$l = \OC_L10N::get('lib');
+ $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
@@ -1820,12 +1848,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';
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list