From 76a528e5112698afec67eb6fce43e2ba809972ac Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 5 Oct 2015 16:18:14 +0200 Subject: [PATCH 1/6] Make the external share manager a mount provider --- apps/files_sharing/lib/external/manager.php | 61 +++++++------------ apps/files_sharing/lib/helper.php | 1 - .../tests/external/managertest.php | 28 +++++++-- 3 files changed, 44 insertions(+), 46 deletions(-) diff --git a/apps/files_sharing/lib/external/manager.php b/apps/files_sharing/lib/external/manager.php index 2ba02f40d2..662e70d123 100644 --- a/apps/files_sharing/lib/external/manager.php +++ b/apps/files_sharing/lib/external/manager.php @@ -28,9 +28,12 @@ namespace OCA\Files_Sharing\External; use OC\Files\Filesystem; use OCP\Files; +use OCP\Files\Config\IMountProvider; +use OCP\Files\Storage\IStorageFactory; use OC\Notification\IManager; +use OCP\IUser; -class Manager { +class Manager implements IMountProvider { const STORAGE = '\OCA\Files_Sharing\External\Storage'; /** @@ -152,26 +155,20 @@ class Manager { return $this->mountShare($options); } - private function setupMounts() { - // don't setup server-to-server shares if the admin disabled it - if (\OCA\Files_Sharing\Helper::isIncomingServer2serverShareEnabled() === false) { - return false; - } - - if (!is_null($this->uid)) { - $query = $this->connection->prepare(' + public function getMountsForUser(IUser $user, IStorageFactory $loader) { + $query = $this->connection->prepare(' SELECT `remote`, `share_token`, `password`, `mountpoint`, `owner` FROM `*PREFIX*share_external` WHERE `user` = ? AND `accepted` = ? '); - $query->execute(array($this->uid, 1)); - - while ($row = $query->fetch()) { - $row['manager'] = $this; - $row['token'] = $row['share_token']; - $this->mountShare($row); - } + $query->execute([$user->getUID(), 1]); + $mounts = []; + while ($row = $query->fetch()) { + $row['manager'] = $this; + $row['token'] = $row['share_token']; + $mounts[] = $this->getMount($row, $loader); } + return $mounts; } /** @@ -275,24 +272,6 @@ class Manager { return ($result['success'] && $status['ocs']['meta']['statuscode'] === 100); } - /** - * setup the server-to-server mounts - * - * @param array $params - */ - public static function setup(array $params) { - $externalManager = new \OCA\Files_Sharing\External\Manager( - \OC::$server->getDatabaseConnection(), - \OC\Files\Filesystem::getMountManager(), - \OC\Files\Filesystem::getLoader(), - \OC::$server->getHTTPHelper(), - \OC::$server->getNotificationManager(), - $params['user'] - ); - - $externalManager->setupMounts(); - } - /** * remove '/user/files' from the path and trailing slashes * @@ -304,16 +283,20 @@ class Manager { return rtrim(substr($path, strlen($prefix)), '/'); } + protected function getMount($data, IStorageFactory $storageFactory) { + $data['manager'] = $this; + $mountPoint = '/' . $this->uid . '/files' . $data['mountpoint']; + $data['mountpoint'] = $mountPoint; + $data['certificateManager'] = \OC::$server->getCertificateManager($this->uid); + return new Mount(self::STORAGE, $mountPoint, $data, $this, $storageFactory); + } + /** * @param array $data * @return Mount */ protected function mountShare($data) { - $data['manager'] = $this; - $mountPoint = '/' . $this->uid . '/files' . $data['mountpoint']; - $data['mountpoint'] = $mountPoint; - $data['certificateManager'] = \OC::$server->getCertificateManager($this->uid); - $mount = new Mount(self::STORAGE, $mountPoint, $data, $this, $this->storageLoader); + $mount = $this->getMount($data, $this->storageLoader); $this->mountManager->addMount($mount); return $mount; } diff --git a/apps/files_sharing/lib/helper.php b/apps/files_sharing/lib/helper.php index 38cb73be29..2d92cac6c9 100644 --- a/apps/files_sharing/lib/helper.php +++ b/apps/files_sharing/lib/helper.php @@ -30,7 +30,6 @@ namespace OCA\Files_Sharing; class Helper { public static function registerHooks() { - \OCP\Util::connectHook('OC_Filesystem', 'setup', '\OCA\Files_Sharing\External\Manager', 'setup'); \OCP\Util::connectHook('OC_Filesystem', 'delete', '\OC\Files\Cache\Shared_Updater', 'deleteHook'); \OCP\Util::connectHook('OC_Filesystem', 'post_rename', '\OC\Files\Cache\Shared_Updater', 'renameHook'); \OCP\Util::connectHook('OC_Filesystem', 'post_delete', '\OCA\Files_Sharing\Hooks', 'unshareChildren'); diff --git a/apps/files_sharing/tests/external/managertest.php b/apps/files_sharing/tests/external/managertest.php index 8e03c67a9a..2d31b16306 100644 --- a/apps/files_sharing/tests/external/managertest.php +++ b/apps/files_sharing/tests/external/managertest.php @@ -24,8 +24,10 @@ namespace OCA\Files_Sharing\Tests\External; use OC\Files\Storage\StorageFactory; use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\Tests\TestCase; +use Test\Traits\UserTrait; class ManagerTest extends TestCase { + use UserTrait; /** @var Manager **/ private $manager; @@ -38,10 +40,17 @@ class ManagerTest extends TestCase { private $uid; + /** + * @var \OCP\IUser + */ + private $user; + protected function setUp() { parent::setUp(); $this->uid = $this->getUniqueID('user'); + $this->createUser($this->uid, ''); + $this->user = \OC::$server->getUserManager()->get($this->uid); $this->mountManager = new \OC\Files\Mount\Manager(); $this->httpHelper = $httpHelper = $this->getMockBuilder('\OC\HTTPHelper')->disableOriginalConstructor()->getMock(); /** @var \OC\HTTPHelper $httpHelper */ @@ -55,6 +64,13 @@ class ManagerTest extends TestCase { ); } + private function setupMounts() { + $mounts = $this->manager->getMountsForUser($this->user, new StorageFactory()); + foreach ($mounts as $mount) { + $this->mountManager->addMount($mount); + } + } + public function testAddShare() { $shareData1 = [ @@ -77,7 +93,7 @@ class ManagerTest extends TestCase { $this->assertCount(1, $openShares); $this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); - self::invokePrivate($this->manager, 'setupMounts'); + $this->setupMounts(); $this->assertNotMount('SharedFolder'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); @@ -89,7 +105,7 @@ class ManagerTest extends TestCase { // New share falls back to "-1" appendix, because the name is already taken $this->assertExternalShareEntry($shareData2, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1'); - self::invokePrivate($this->manager, 'setupMounts'); + $this->setupMounts(); $this->assertNotMount('SharedFolder'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); @@ -111,7 +127,7 @@ class ManagerTest extends TestCase { $this->assertCount(1, $openShares); $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1'); - self::invokePrivate($this->manager, 'setupMounts'); + $this->setupMounts(); $this->assertMount($shareData1['name']); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); @@ -124,7 +140,7 @@ class ManagerTest extends TestCase { // New share falls back to the original name (no "-\d", because the name is not taken) $this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}'); - self::invokePrivate($this->manager, 'setupMounts'); + $this->setupMounts(); $this->assertMount($shareData1['name']); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); @@ -136,7 +152,7 @@ class ManagerTest extends TestCase { // Decline the third share $this->manager->declineShare($openShares[1]['id']); - self::invokePrivate($this->manager, 'setupMounts'); + $this->setupMounts(); $this->assertMount($shareData1['name']); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); @@ -151,7 +167,7 @@ class ManagerTest extends TestCase { $this->assertCount(1, $openShares); $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1'); - self::invokePrivate($this->manager, 'setupMounts'); + $this->setupMounts(); $this->assertMount($shareData1['name']); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); From a0c9dc298680eaa4f9f077978d8baefdec102d93 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 5 Oct 2015 16:19:13 +0200 Subject: [PATCH 2/6] register the external share mount provider --- apps/files_sharing/appinfo/application.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/files_sharing/appinfo/application.php b/apps/files_sharing/appinfo/application.php index ad5d5d6323..c3e2a1d707 100644 --- a/apps/files_sharing/appinfo/application.php +++ b/apps/files_sharing/appinfo/application.php @@ -138,6 +138,7 @@ class Application extends App { $server = $this->getContainer()->query('ServerContainer'); $mountProviderCollection = $server->getMountProviderCollection(); $mountProviderCollection->registerProvider($this->getContainer()->query('MountProvider')); + $mountProviderCollection->registerProvider($this->getContainer()->query('ExternalManager')); } public function setupPropagation() { From e1c562f66b8547e0418a14f9f4fe8616c622caa8 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 5 Oct 2015 16:39:11 +0200 Subject: [PATCH 3/6] extract the mount provider from the manager --- apps/files_sharing/appinfo/application.php | 13 +++- apps/files_sharing/lib/external/manager.php | 27 +------ .../lib/external/mountprovider.php | 75 +++++++++++++++++++ 3 files changed, 91 insertions(+), 24 deletions(-) create mode 100644 apps/files_sharing/lib/external/mountprovider.php diff --git a/apps/files_sharing/appinfo/application.php b/apps/files_sharing/appinfo/application.php index c3e2a1d707..872229fa74 100644 --- a/apps/files_sharing/appinfo/application.php +++ b/apps/files_sharing/appinfo/application.php @@ -118,6 +118,17 @@ class Application extends App { ); }); + $container->registerService('ExternalMountProvider', function (IContainer $c) { + /** @var \OCP\IServerContainer $server */ + $server = $c->query('ServerContainer'); + return new \OCA\Files_Sharing\External\MountProvider( + $server->getDatabaseConnection(), + function() use ($c) { + return $c->query('ExternalManager'); + } + ); + }); + $container->registerService('PropagationManager', function (IContainer $c) { /** @var \OCP\IServerContainer $server */ $server = $c->query('ServerContainer'); @@ -138,7 +149,7 @@ class Application extends App { $server = $this->getContainer()->query('ServerContainer'); $mountProviderCollection = $server->getMountProviderCollection(); $mountProviderCollection->registerProvider($this->getContainer()->query('MountProvider')); - $mountProviderCollection->registerProvider($this->getContainer()->query('ExternalManager')); + $mountProviderCollection->registerProvider($this->getContainer()->query('ExternalMountProvider')); } public function setupPropagation() { diff --git a/apps/files_sharing/lib/external/manager.php b/apps/files_sharing/lib/external/manager.php index 662e70d123..a017a465b5 100644 --- a/apps/files_sharing/lib/external/manager.php +++ b/apps/files_sharing/lib/external/manager.php @@ -28,12 +28,9 @@ namespace OCA\Files_Sharing\External; use OC\Files\Filesystem; use OCP\Files; -use OCP\Files\Config\IMountProvider; -use OCP\Files\Storage\IStorageFactory; use OC\Notification\IManager; -use OCP\IUser; -class Manager implements IMountProvider { +class Manager { const STORAGE = '\OCA\Files_Sharing\External\Storage'; /** @@ -155,22 +152,6 @@ class Manager implements IMountProvider { return $this->mountShare($options); } - public function getMountsForUser(IUser $user, IStorageFactory $loader) { - $query = $this->connection->prepare(' - SELECT `remote`, `share_token`, `password`, `mountpoint`, `owner` - FROM `*PREFIX*share_external` - WHERE `user` = ? AND `accepted` = ? - '); - $query->execute([$user->getUID(), 1]); - $mounts = []; - while ($row = $query->fetch()) { - $row['manager'] = $this; - $row['token'] = $row['share_token']; - $mounts[] = $this->getMount($row, $loader); - } - return $mounts; - } - /** * get share * @@ -283,12 +264,12 @@ class Manager implements IMountProvider { return rtrim(substr($path, strlen($prefix)), '/'); } - protected function getMount($data, IStorageFactory $storageFactory) { + public function getMount($data) { $data['manager'] = $this; $mountPoint = '/' . $this->uid . '/files' . $data['mountpoint']; $data['mountpoint'] = $mountPoint; $data['certificateManager'] = \OC::$server->getCertificateManager($this->uid); - return new Mount(self::STORAGE, $mountPoint, $data, $this, $storageFactory); + return new Mount(self::STORAGE, $mountPoint, $data, $this, $this->storageLoader); } /** @@ -296,7 +277,7 @@ class Manager implements IMountProvider { * @return Mount */ protected function mountShare($data) { - $mount = $this->getMount($data, $this->storageLoader); + $mount = $this->getMount($data); $this->mountManager->addMount($mount); return $mount; } diff --git a/apps/files_sharing/lib/external/mountprovider.php b/apps/files_sharing/lib/external/mountprovider.php new file mode 100644 index 0000000000..59072c203f --- /dev/null +++ b/apps/files_sharing/lib/external/mountprovider.php @@ -0,0 +1,75 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program 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, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\Files_Sharing\External; + +use OCP\Files\Config\IMountProvider; +use OCP\Files\Storage\IStorageFactory; +use OCP\IDBConnection; +use OCP\IUser; + +class MountProvider implements IMountProvider { + const STORAGE = '\OCA\Files_Sharing\External\Storage'; + + /** + * @var \OCP\IDBConnection + */ + private $connection; + + /** + * @var callable + */ + private $managerProvider; + + /** + * @param \OCP\IDBConnection $connection + * @param callable $managerProvider due to setup order we need a callable that return the manager instead of the manager itself + */ + public function __construct(IDBConnection $connection, callable $managerProvider) { + $this->connection = $connection; + $this->managerProvider = $managerProvider; + } + + public function getMount(IUser $user, $data, IStorageFactory $storageFactory) { + $data['manager'] = $this; + $mountPoint = '/' . $user->getUID() . '/files' . $data['mountpoint']; + $data['mountpoint'] = $mountPoint; + $data['certificateManager'] = \OC::$server->getCertificateManager($user->getUID()); + $managerProvider = $this->managerProvider; + return new Mount(self::STORAGE, $mountPoint, $data, $managerProvider(), $storageFactory); + } + + public function getMountsForUser(IUser $user, IStorageFactory $loader) { + $query = $this->connection->prepare(' + SELECT `remote`, `share_token`, `password`, `mountpoint`, `owner` + FROM `*PREFIX*share_external` + WHERE `user` = ? AND `accepted` = ? + '); + $query->execute([$user->getUID(), 1]); + $mounts = []; + while ($row = $query->fetch()) { + $row['manager'] = $this; + $row['token'] = $row['share_token']; + $mounts[] = $this->getMount($user, $row, $loader); + } + return $mounts; + } +} From 52c8b488cb0dd0fc95078602eb1e14516c74ae2e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 5 Oct 2015 16:43:10 +0200 Subject: [PATCH 4/6] fix test --- apps/files_sharing/tests/external/managertest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/tests/external/managertest.php b/apps/files_sharing/tests/external/managertest.php index 2d31b16306..7242779d45 100644 --- a/apps/files_sharing/tests/external/managertest.php +++ b/apps/files_sharing/tests/external/managertest.php @@ -23,6 +23,7 @@ namespace OCA\Files_Sharing\Tests\External; use OC\Files\Storage\StorageFactory; use OCA\Files_Sharing\External\Manager; +use OCA\Files_Sharing\External\MountProvider; use OCA\Files_Sharing\Tests\TestCase; use Test\Traits\UserTrait; @@ -44,6 +45,7 @@ class ManagerTest extends TestCase { * @var \OCP\IUser */ private $user; + private $mountProvider; protected function setUp() { parent::setUp(); @@ -62,10 +64,13 @@ class ManagerTest extends TestCase { \OC::$server->getNotificationManager(), $this->uid ); + $this->mountProvider = new MountProvider(\OC::$server->getDatabaseConnection(), function() { + return $this->manager; + }); } private function setupMounts() { - $mounts = $this->manager->getMountsForUser($this->user, new StorageFactory()); + $mounts = $this->mountProvider->getMountsForUser($this->user, new StorageFactory()); foreach ($mounts as $mount) { $this->mountManager->addMount($mount); } From e468b38bd4349f871d15efa506c3f5e362faea39 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 5 Oct 2015 17:14:39 +0200 Subject: [PATCH 5/6] sanitize mountpoint --- apps/files_sharing/lib/external/mountprovider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/external/mountprovider.php b/apps/files_sharing/lib/external/mountprovider.php index 59072c203f..1739cec543 100644 --- a/apps/files_sharing/lib/external/mountprovider.php +++ b/apps/files_sharing/lib/external/mountprovider.php @@ -50,7 +50,7 @@ class MountProvider implements IMountProvider { public function getMount(IUser $user, $data, IStorageFactory $storageFactory) { $data['manager'] = $this; - $mountPoint = '/' . $user->getUID() . '/files' . $data['mountpoint']; + $mountPoint = '/' . $user->getUID() . '/files/' . ltrim($data['mountpoint'], '/'); $data['mountpoint'] = $mountPoint; $data['certificateManager'] = \OC::$server->getCertificateManager($user->getUID()); $managerProvider = $this->managerProvider; From d3c1fcf9615339786d3134b8b30d150f9a80f89d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 6 Oct 2015 16:33:37 +0200 Subject: [PATCH 6/6] Fix getEtag for roots of shared storages --- apps/files_sharing/lib/sharedstorage.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 27dd2f1e48..b0e56f5f05 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -583,9 +583,6 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { } public function getETag($path) { - if ($path == '') { - $path = $this->getMountPoint(); - } if ($source = $this->getSourcePath($path)) { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); return $storage->getETag($internalPath);