From f2152ecda9536659b0149fab4811f366b4cd7c37 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 16 Aug 2018 19:16:24 +0200 Subject: more efficient unique share target generation Signed-off-by: Robin Appelman --- apps/files_sharing/lib/MountProvider.php | 5 +++-- apps/files_sharing/lib/SharedMount.php | 18 +++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) (limited to 'apps') diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index fd4c537210f..e279b4303a8 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -85,7 +85,7 @@ class MountProvider implements IMountProvider { $mounts = []; foreach ($superShares as $share) { try { - $mounts[] = new SharedMount( + $mount = new SharedMount( '\OCA\Files_Sharing\SharedStorage', $mounts, [ @@ -97,6 +97,7 @@ class MountProvider implements IMountProvider { ], $storageFactory ); + $mounts[$mount->getMountPoint()] = $mount; } catch (\Exception $e) { $this->logger->logException($e); $this->logger->error('Error while trying to create shared mount'); @@ -104,7 +105,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 4f0dc89e997..ba3efaf3f87 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -135,20 +135,16 @@ 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) . '/'; + var_dump($absolutePath); $i++; + if ($i > 10) { + return $path; + } } return $path; -- cgit v1.2.3 From c897e2af8e43f37dda08d99a1a3460b6aa90da67 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 16 Aug 2018 19:56:49 +0200 Subject: more efficient way to detect added and removed mounts Signed-off-by: Robin Appelman --- apps/files_sharing/lib/SharedMount.php | 4 ---- lib/private/Files/Config/UserMountCache.php | 32 +++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) (limited to 'apps') diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php index ba3efaf3f87..bf274f2c2f6 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -140,11 +140,7 @@ class SharedMount extends MountPoint implements MoveableMount { while ($view->file_exists($path) || isset($mountpoints[$absolutePath])) { $path = Filesystem::normalizePath($dir . '/' . $name . ' (' . $i . ')' . $ext); $absolutePath = $this->recipientView->getAbsolutePath($path) . '/'; - var_dump($absolutePath); $i++; - if ($i > 10) { - return $path; - } } return $path; diff --git a/lib/private/Files/Config/UserMountCache.php b/lib/private/Files/Config/UserMountCache.php index 095a0df7e0c..07c6e1f2d62 100644 --- a/lib/private/Files/Config/UserMountCache.php +++ b/lib/private/Files/Config/UserMountCache.php @@ -102,17 +102,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(); - }; - - /** @var ICachedMountInfo[] $addedMounts */ - $addedMounts = array_udiff($newMounts, $cachedMounts, $mountDiff); - /** @var ICachedMountInfo[] $removedMounts */ - $removedMounts = array_udiff($cachedMounts, $newMounts, $mountDiff); + $cachedMountRootIds = array_map(function (ICachedMountInfo $mount) { + return $mount->getRootId(); + }, $cachedMounts); + $cachedMounts = array_combine($cachedMountRootIds, $cachedMounts); + + $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); -- cgit v1.2.3 From a396c376937397fb75bbd55c1180cba6e3dc414b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 16 Aug 2018 20:10:45 +0200 Subject: re-use view instances for shared storages Signed-off-by: Robin Appelman --- apps/files_sharing/lib/MountProvider.php | 15 +++++++++++++-- apps/files_sharing/lib/SharedMount.php | 8 ++++---- 2 files changed, 17 insertions(+), 6 deletions(-) (limited to 'apps') diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index e279b4303a8..a3db685c6af 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -27,6 +27,7 @@ namespace OCA\Files_Sharing; +use OC\Files\View; use OCP\Files\Config\IMountProvider; use OCP\Files\Storage\IStorageFactory; use OCP\IConfig; @@ -83,19 +84,29 @@ class MountProvider implements IMountProvider { $superShares = $this->buildSuperShares($shares, $user); $mounts = []; + $view = new View('/' . $user->getUID() . '/files'); + $ownerViews = []; foreach ($superShares as $share) { try { + /** @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] ], - $storageFactory + $storageFactory, + $view ); $mounts[$mount->getMountPoint()] = $mount; } catch (\Exception $e) { diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php index bf274f2c2f6..9a801311a41 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -61,19 +61,19 @@ class SharedMount extends MountPoint implements MoveableMount { /** * @param string $storage * @param SharedMount[] $mountpoints - * @param array|null $arguments + * @param array $arguments * @param \OCP\Files\Storage\IStorageFactory $loader + * @param View $recipientView */ - public function __construct($storage, array $mountpoints, $arguments = null, $loader = null) { + public function __construct($storage, array $mountpoints, $arguments, $loader, $recipientView) { $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); $absMountPoint = '/' . $this->user . '/files' . $newMountPoint; - $arguments['ownerView'] = new View('/' . $this->superShare->getShareOwner() . '/files'); parent::__construct($storage, $absMountPoint, $arguments, $loader); } -- cgit v1.2.3 From 237e4b75219e91b5b684b9907f19c7ddc73543cb Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 16 Aug 2018 20:18:13 +0200 Subject: don't check if target dir exists when using the default share target directory Signed-off-by: Robin Appelman --- apps/files_sharing/lib/Helper.php | 4 ++++ apps/files_sharing/lib/SharedMount.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'apps') diff --git a/apps/files_sharing/lib/Helper.php b/apps/files_sharing/lib/Helper.php index c8f46fa8132..465a55a0052 100644 --- a/apps/files_sharing/lib/Helper.php +++ b/apps/files_sharing/lib/Helper.php @@ -237,6 +237,10 @@ class Helper { return $path; } + public static function isUsingShareFolder() { + return \OC::$server->getConfig()->getSystemValue('share_folder', '/') !== '/'; + } + /** * get default share folder * diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php index 9a801311a41..dacc78edac3 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -89,7 +89,7 @@ class SharedMount extends MountPoint implements MoveableMount { $mountPoint = basename($share->getTarget()); $parent = dirname($share->getTarget()); - if (!$this->recipientView->is_dir($parent)) { + if (Helper::isUsingShareFolder() && !$this->recipientView->is_dir($parent)) { $parent = Helper::getShareFolder($this->recipientView); } -- cgit v1.2.3 From 110650ff588513df559c5d96b0451d88fc866fc5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 16 Aug 2018 20:24:02 +0200 Subject: only determine is sharing is disabled for a user once Signed-off-by: Robin Appelman --- apps/files_sharing/lib/MountProvider.php | 4 +++- apps/files_sharing/lib/SharedStorage.php | 10 +++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'apps') diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index a3db685c6af..c4fa6edc876 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -86,6 +86,7 @@ class MountProvider implements IMountProvider { $mounts = []; $view = new View('/' . $user->getUID() . '/files'); $ownerViews = []; + $sharingDisabledForUser = $this->shareManager->sharingDisabledForUser($user->getUID()); foreach ($superShares as $share) { try { /** @var \OCP\Share\IShare $parentShare */ @@ -103,7 +104,8 @@ class MountProvider implements IMountProvider { 'superShare' => $parentShare, // children/component of the superShare 'groupedShares' => $share[1], - 'ownerView' => $ownerViews[$owner] + 'ownerView' => $ownerViews[$owner], + 'sharingDisabledForUser' => $sharingDisabledForUser ], $storageFactory, $view diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index f681854cd2b..2d2f14fc554 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -79,6 +79,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(); @@ -87,6 +90,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, @@ -193,7 +201,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; } -- cgit v1.2.3 From 6dae2b92508696c6b47216099abdd3f3fcd8497e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 16 Aug 2018 21:05:01 +0200 Subject: cache parent exists status during share setup Signed-off-by: Robin Appelman --- apps/files_sharing/lib/Helper.php | 4 ---- apps/files_sharing/lib/MountProvider.php | 5 ++++- apps/files_sharing/lib/SharedMount.php | 18 +++++++++++++----- 3 files changed, 17 insertions(+), 10 deletions(-) (limited to 'apps') diff --git a/apps/files_sharing/lib/Helper.php b/apps/files_sharing/lib/Helper.php index 465a55a0052..c8f46fa8132 100644 --- a/apps/files_sharing/lib/Helper.php +++ b/apps/files_sharing/lib/Helper.php @@ -237,10 +237,6 @@ class Helper { return $path; } - public static function isUsingShareFolder() { - return \OC::$server->getConfig()->getSystemValue('share_folder', '/') !== '/'; - } - /** * get default share folder * diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index c4fa6edc876..5623f18d662 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -27,6 +27,7 @@ namespace OCA\Files_Sharing; +use OC\Cache\CappedMemoryCache; use OC\Files\View; use OCP\Files\Config\IMountProvider; use OCP\Files\Storage\IStorageFactory; @@ -87,6 +88,7 @@ class MountProvider implements IMountProvider { $view = new View('/' . $user->getUID() . '/files'); $ownerViews = []; $sharingDisabledForUser = $this->shareManager->sharingDisabledForUser($user->getUID()); + $foldersExistCache = new CappedMemoryCache(); foreach ($superShares as $share) { try { /** @var \OCP\Share\IShare $parentShare */ @@ -108,7 +110,8 @@ class MountProvider implements IMountProvider { 'sharingDisabledForUser' => $sharingDisabledForUser ], $storageFactory, - $view + $view, + $foldersExistCache ); $mounts[$mount->getMountPoint()] = $mount; } catch (\Exception $e) { diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php index dacc78edac3..4b38698f81d 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -28,10 +28,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 @@ -62,17 +64,17 @@ class SharedMount extends MountPoint implements MoveableMount { * @param string $storage * @param SharedMount[] $mountpoints * @param array $arguments - * @param \OCP\Files\Storage\IStorageFactory $loader + * @param IStorageFactory $loader * @param View $recipientView */ - public function __construct($storage, array $mountpoints, $arguments, $loader, $recipientView) { + public function __construct($storage, array $mountpoints, $arguments, IStorageFactory $loader, View $recipientView, CappedMemoryCache $folderExistCache) { $this->user = $arguments['user']; $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; parent::__construct($storage, $absMountPoint, $arguments, $loader); } @@ -84,12 +86,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 (Helper::isUsingShareFolder() && !$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); } -- cgit v1.2.3