diff options
author | Morris Jobke <hey@morrisjobke.de> | 2018-09-27 18:09:36 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-09-27 18:09:36 +0300 |
commit | ca2f2c227d7ea45ff63d32638a648212d3b30723 (patch) | |
tree | 16e7abb3c17320dd5d572afd6b9381041be093e4 | |
parent | a6304378b0b6cb6fa32721bc3d61678716b651a4 (diff) | |
parent | a6e78a4db6a44c6376e6e27b0c4461cd4a97a5dc (diff) |
Merge pull request #10882 from nextcloud/large-share-count-performance-12
[12] Improve performance when dealing with large numbers of shares
-rw-r--r-- | apps/files_sharing/lib/MountProvider.php | 25 | ||||
-rw-r--r-- | apps/files_sharing/lib/SharedMount.php | 38 | ||||
-rw-r--r-- | apps/files_sharing/lib/SharedStorage.php | 10 | ||||
-rw-r--r-- | lib/private/Files/Config/UserMountCache.php | 51 | ||||
-rw-r--r-- | lib/private/Files/Mount/Manager.php | 61 | ||||
-rw-r--r-- | lib/private/Files/View.php | 23 | ||||
-rw-r--r-- | lib/private/Share20/Manager.php | 4 |
7 files changed, 138 insertions, 74 deletions
diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index a02d6350499..8208e5fc755 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -25,6 +25,8 @@ namespace OCA\Files_Sharing; +use OC\Cache\CappedMemoryCache; +use OC\Files\View; use OCP\Files\Config\IMountProvider; use OCP\Files\Storage\IStorageFactory; use OCP\IConfig; @@ -81,20 +83,35 @@ class MountProvider implements IMountProvider { $superShares = $this->buildSuperShares($shares, $user); $mounts = []; + $view = new View('/' . $user->getUID() . '/files'); + $ownerViews = []; + $sharingDisabledForUser = $this->shareManager->sharingDisabledForUser($user->getUID()); + $foldersExistCache = new CappedMemoryCache(); foreach ($superShares as $share) { try { - $mounts[] = new SharedMount( + /** @var \OCP\Share\IShare $parentShare */ + $parentShare = $share[0]; + $owner = $parentShare->getShareOwner(); + if (!isset($ownerViews[$owner])) { + $ownerViews[$owner] = new View('/' . $parentShare->getShareOwner() . '/files'); + } + $mount = new SharedMount( '\OCA\Files_Sharing\SharedStorage', $mounts, [ 'user' => $user->getUID(), // parent share - 'superShare' => $share[0], + 'superShare' => $parentShare, // children/component of the superShare 'groupedShares' => $share[1], + 'ownerView' => $ownerViews[$owner], + 'sharingDisabledForUser' => $sharingDisabledForUser ], - $storageFactory + $storageFactory, + $view, + $foldersExistCache ); + $mounts[$mount->getMountPoint()] = $mount; } catch (\Exception $e) { $this->logger->logException($e); $this->logger->error('Error while trying to create shared mount'); @@ -102,7 +119,7 @@ class MountProvider implements IMountProvider { } // array_filter removes the null values from the array - return array_filter($mounts); + return array_values(array_filter($mounts)); } /** diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php index b42682ab2a8..96e9bc000ab 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -27,10 +27,12 @@ namespace OCA\Files_Sharing; +use OC\Cache\CappedMemoryCache; use OC\Files\Filesystem; use OC\Files\Mount\MountPoint; use OC\Files\Mount\MoveableMount; use OC\Files\View; +use OCP\Files\Storage\IStorageFactory; /** * Shared mount points can be moved by the user @@ -60,19 +62,19 @@ class SharedMount extends MountPoint implements MoveableMount { /** * @param string $storage * @param SharedMount[] $mountpoints - * @param array|null $arguments - * @param \OCP\Files\Storage\IStorageFactory $loader + * @param array $arguments + * @param IStorageFactory $loader + * @param View $recipientView */ - public function __construct($storage, array $mountpoints, $arguments = null, $loader = null) { + public function __construct($storage, array $mountpoints, $arguments, IStorageFactory $loader, View $recipientView, CappedMemoryCache $folderExistCache) { $this->user = $arguments['user']; - $this->recipientView = new View('/' . $this->user . '/files'); + $this->recipientView = $recipientView; $this->superShare = $arguments['superShare']; $this->groupedShares = $arguments['groupedShares']; - $newMountPoint = $this->verifyMountPoint($this->superShare, $mountpoints); + $newMountPoint = $this->verifyMountPoint($this->superShare, $mountpoints, $folderExistCache); $absMountPoint = '/' . $this->user . '/files' . $newMountPoint; - $arguments['ownerView'] = new View('/' . $this->superShare->getShareOwner() . '/files'); parent::__construct($storage, $absMountPoint, $arguments, $loader); } @@ -83,12 +85,18 @@ class SharedMount extends MountPoint implements MoveableMount { * @param SharedMount[] $mountpoints * @return string */ - private function verifyMountPoint(\OCP\Share\IShare $share, array $mountpoints) { + private function verifyMountPoint(\OCP\Share\IShare $share, array $mountpoints, CappedMemoryCache $folderExistCache) { $mountPoint = basename($share->getTarget()); $parent = dirname($share->getTarget()); - if (!$this->recipientView->is_dir($parent)) { + if ($folderExistCache->hasKey($parent)) { + $parentExists = $folderExistCache->get($parent); + } else { + $parentExists = $this->recipientView->is_dir($parent); + $folderExistCache->set($parent, $parentExists); + } + if (!$parentExists) { $parent = Helper::getShareFolder($this->recipientView); } @@ -134,19 +142,11 @@ class SharedMount extends MountPoint implements MoveableMount { $name = $pathinfo['filename']; $dir = $pathinfo['dirname']; - // Helper function to find existing mount points - $mountpointExists = function ($path) use ($mountpoints) { - foreach ($mountpoints as $mountpoint) { - if ($mountpoint->getShare()->getTarget() === $path) { - return true; - } - } - return false; - }; - $i = 2; - while ($view->file_exists($path) || $mountpointExists($path)) { + $absolutePath = $this->recipientView->getAbsolutePath($path) . '/'; + while ($view->file_exists($path) || isset($mountpoints[$absolutePath])) { $path = Filesystem::normalizePath($dir . '/' . $name . ' (' . $i . ')' . $ext); + $absolutePath = $this->recipientView->getAbsolutePath($path) . '/'; $i++; } diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 32304afea01..4855b8cc61c 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -78,6 +78,9 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto private $options; + /** @var boolean */ + private $sharingDisabledForUser; + public function __construct($arguments) { $this->ownerView = $arguments['ownerView']; $this->logger = \OC::$server->getLogger(); @@ -86,6 +89,11 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto $this->groupedShares = $arguments['groupedShares']; $this->user = $arguments['user']; + if (isset($arguments['sharingDisabledForUser'])) { + $this->sharingDisabledForUser = $arguments['sharingDisabledForUser']; + } else { + $this->sharingDisabledForUser = false; + } parent::__construct([ 'storage' => null, @@ -192,7 +200,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto $permissions |= \OCP\Constants::PERMISSION_DELETE; } - if (\OCP\Util::isSharingDisabledForUser()) { + if ($this->sharingDisabledForUser) { $permissions &= ~\OCP\Constants::PERMISSION_SHARE; } diff --git a/lib/private/Files/Config/UserMountCache.php b/lib/private/Files/Config/UserMountCache.php index f33176b1e7b..5b18d9743da 100644 --- a/lib/private/Files/Config/UserMountCache.php +++ b/lib/private/Files/Config/UserMountCache.php @@ -100,17 +100,31 @@ class UserMountCache implements IUserMountCache { } }, $mounts); $newMounts = array_values(array_filter($newMounts)); + $newMountRootIds = array_map(function (ICachedMountInfo $mount) { + return $mount->getRootId(); + }, $newMounts); + $newMounts = array_combine($newMountRootIds, $newMounts); $cachedMounts = $this->getMountsForUser($user); - $mountDiff = function (ICachedMountInfo $mount1, ICachedMountInfo $mount2) { - // since we are only looking for mounts for a specific user comparing on root id is enough - return $mount1->getRootId() - $mount2->getRootId(); - }; + $cachedMountRootIds = array_map(function (ICachedMountInfo $mount) { + return $mount->getRootId(); + }, $cachedMounts); + $cachedMounts = array_combine($cachedMountRootIds, $cachedMounts); - /** @var ICachedMountInfo[] $addedMounts */ - $addedMounts = array_udiff($newMounts, $cachedMounts, $mountDiff); - /** @var ICachedMountInfo[] $removedMounts */ - $removedMounts = array_udiff($cachedMounts, $newMounts, $mountDiff); + $addedMounts = []; + $removedMounts = []; + + foreach ($newMounts as $rootId => $newMount) { + if (!isset($cachedMounts[$rootId])) { + $addedMounts[] = $newMount; + } + } + + foreach ($cachedMounts as $rootId => $cachedMount) { + if (!isset($newMounts[$rootId])) { + $removedMounts[] = $cachedMount; + } + } $changedMounts = $this->findChangedMounts($newMounts, $cachedMounts); @@ -134,16 +148,19 @@ class UserMountCache implements IUserMountCache { * @return ICachedMountInfo[] */ private function findChangedMounts(array $newMounts, array $cachedMounts) { + $new = []; + foreach ($newMounts as $mount) { + $new[$mount->getRootId()] = $mount; + } $changed = []; - foreach ($newMounts as $newMount) { - foreach ($cachedMounts as $cachedMount) { + foreach ($cachedMounts as $cachedMount) { + $rootId = $cachedMount->getRootId(); + if (isset($new[$rootId])) { + $newMount = $new[$rootId]; if ( - $newMount->getRootId() === $cachedMount->getRootId() && - ( - $newMount->getMountPoint() !== $cachedMount->getMountPoint() || - $newMount->getStorageId() !== $cachedMount->getStorageId() || - $newMount->getMountId() !== $cachedMount->getMountId() - ) + $newMount->getMountPoint() !== $cachedMount->getMountPoint() || + $newMount->getStorageId() !== $cachedMount->getStorageId() || + $newMount->getMountId() !== $cachedMount->getMountId() ) { $changed[] = $newMount; } @@ -196,7 +213,7 @@ class UserMountCache implements IUserMountCache { } $mount_id = $row['mount_id']; if (!is_null($mount_id)) { - $mount_id = (int) $mount_id; + $mount_id = (int)$mount_id; } return new CachedMountInfo($user, (int)$row['storage_id'], (int)$row['root_id'], $row['mount_point'], $mount_id, isset($row['path'])? $row['path']:''); } diff --git a/lib/private/Files/Mount/Manager.php b/lib/private/Files/Mount/Manager.php index 7bd888a6389..39891bf572e 100644 --- a/lib/private/Files/Mount/Manager.php +++ b/lib/private/Files/Mount/Manager.php @@ -25,21 +25,28 @@ namespace OC\Files\Mount; +use OC\Cache\CappedMemoryCache; use \OC\Files\Filesystem; use OCP\Files\Mount\IMountManager; use OCP\Files\Mount\IMountPoint; class Manager implements IMountManager { - /** - * @var MountPoint[] - */ - private $mounts = array(); + /** @var MountPoint[] */ + private $mounts = []; + + /** @var CappedMemoryCache */ + private $inPathCache; + + public function __construct() { + $this->inPathCache = new CappedMemoryCache(); + } /** * @param IMountPoint $mount */ public function addMount(IMountPoint $mount) { $this->mounts[$mount->getMountPoint()] = $mount; + $this->inPathCache->clear(); } /** @@ -51,15 +58,17 @@ class Manager implements IMountManager { $mountPoint .= '/'; } unset($this->mounts[$mountPoint]); + $this->inPathCache->clear(); } /** * @param string $mountPoint * @param string $target */ - public function moveMount($mountPoint, $target){ + public function moveMount($mountPoint, $target) { $this->mounts[$target] = $this->mounts[$mountPoint]; unset($this->mounts[$mountPoint]); + $this->inPathCache->clear(); } /** @@ -70,23 +79,23 @@ class Manager implements IMountManager { */ public function find($path) { \OC_Util::setupFS(); - $path = $this->formatPath($path); - if (isset($this->mounts[$path])) { - return $this->mounts[$path]; - } + $path = Filesystem::normalizePath($path); - \OC_Hook::emit('OC_Filesystem', 'get_mountpoint', array('path' => $path)); - $foundMountPoint = ''; - $mountPoints = array_keys($this->mounts); - foreach ($mountPoints as $mountpoint) { - if (strpos($path, $mountpoint) === 0 and strlen($mountpoint) > strlen($foundMountPoint)) { - $foundMountPoint = $mountpoint; + $current = $path; + while (true) { + $mountPoint = $current . '/'; + if (isset($this->mounts[$mountPoint])) { + return $this->mounts[$mountPoint]; + } + + if ($current === '') { + return null; + } + + $current = dirname($current); + if ($current === '.' || $current === '/') { + $current = ''; } - } - if (isset($this->mounts[$foundMountPoint])) { - return $this->mounts[$foundMountPoint]; - } else { - return null; } } @@ -99,7 +108,12 @@ class Manager implements IMountManager { public function findIn($path) { \OC_Util::setupFS(); $path = $this->formatPath($path); - $result = array(); + + if (isset($this->inPathCache[$path])) { + return $this->inPathCache[$path]; + } + + $result = []; $pathLength = strlen($path); $mountPoints = array_keys($this->mounts); foreach ($mountPoints as $mountPoint) { @@ -107,11 +121,14 @@ class Manager implements IMountManager { $result[] = $this->mounts[$mountPoint]; } } + + $this->inPathCache[$path] = $result; return $result; } public function clear() { - $this->mounts = array(); + $this->mounts = []; + $this->inPathCache->clear(); } /** diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 36dd7e5c773..283ac6734da 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1438,16 +1438,21 @@ class View { $contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter $sharingDisabled = \OCP\Util::isSharingDisabledForUser(); + + $fileNames = array_map(function(ICacheEntry $content) { + return $content->getName(); + }, $contents); /** - * @var \OC\Files\FileInfo[] $files + * @var \OC\Files\FileInfo[] $fileInfos */ - $files = array_map(function (ICacheEntry $content) use ($path, $storage, $mount, $sharingDisabled) { + $fileInfos = array_map(function (ICacheEntry $content) use ($path, $storage, $mount, $sharingDisabled) { if ($sharingDisabled) { $content['permissions'] = $content['permissions'] & ~\OCP\Constants::PERMISSION_SHARE; } $owner = $this->getUserObjectForOwner($storage->getOwner($content['path'])); return new FileInfo($path . '/' . $content['name'], $storage, $content['path'], $content, $mount, $owner); }, $contents); + $files = array_combine($fileNames, $fileInfos); //add a folder for any mountpoint in this directory and add the sizes of other mountpoints to the folders $mounts = Filesystem::getMountManager()->findIn($path); @@ -1502,13 +1507,6 @@ class View { $rootEntry['permissions'] = $permissions & (\OCP\Constants::PERMISSION_ALL - (\OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE)); } - //remove any existing entry with the same name - foreach ($files as $i => $file) { - if ($file['name'] === $rootEntry['name']) { - unset($files[$i]); - break; - } - } $rootEntry['path'] = substr(Filesystem::normalizePath($path . '/' . $rootEntry['name']), strlen($user) + 2); // full path without /$user/ // if sharing was disabled for the user we remove the share permissions @@ -1517,7 +1515,7 @@ class View { } $owner = $this->getUserObjectForOwner($subStorage->getOwner('')); - $files[] = new FileInfo($path . '/' . $rootEntry['name'], $subStorage, '', $rootEntry, $mount, $owner); + $files[$rootEntry->getName()] = new FileInfo($path . '/' . $rootEntry['name'], $subStorage, '', $rootEntry, $mount, $owner); } } } @@ -1533,7 +1531,7 @@ class View { }); } - return $files; + return array_values($files); } else { return []; } @@ -1721,6 +1719,9 @@ class View { */ if ($mount->getStorage()) { $cache = $mount->getStorage()->getCache(); + if (!$cache) { + throw new NotFoundException(sprintf('File with id "%s" has not been found.', $id)); + } $internalPath = $cache->getPathById($id); if (is_string($internalPath)) { $fullPath = $mount->getMountPoint() . $internalPath; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 46b47920ce1..2e65c127eb7 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1177,6 +1177,10 @@ class Manager implements IManager { * @throws ShareNotFound */ public function getShareByToken($token) { + // tokens can't be valid local user names + if ($this->userManager->userExists($token)) { + throw new ShareNotFound(); + } $share = null; try { if($this->shareApiAllowLinks()) { |