From c91b52d38cdfe68d17485795be6811c2120280b9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 7 May 2015 13:37:38 +0200 Subject: [PATCH 1/6] move emitter implementation to a trait --- lib/private/hooks/basicemitter.php | 78 +--------------------- lib/private/hooks/emittertrait.php | 103 +++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 77 deletions(-) create mode 100644 lib/private/hooks/emittertrait.php diff --git a/lib/private/hooks/basicemitter.php b/lib/private/hooks/basicemitter.php index 03d0d1d74c..ae0f96fbc8 100644 --- a/lib/private/hooks/basicemitter.php +++ b/lib/private/hooks/basicemitter.php @@ -23,81 +23,5 @@ namespace OC\Hooks; abstract class BasicEmitter implements Emitter { - - /** - * @var (callable[])[] $listeners - */ - protected $listeners = array(); - - /** - * @param string $scope - * @param string $method - * @param callable $callback - */ - public function listen($scope, $method, $callback) { - $eventName = $scope . '::' . $method; - if (!isset($this->listeners[$eventName])) { - $this->listeners[$eventName] = array(); - } - if (array_search($callback, $this->listeners[$eventName]) === false) { - $this->listeners[$eventName][] = $callback; - } - } - - /** - * @param string $scope optional - * @param string $method optional - * @param callable $callback optional - */ - public function removeListener($scope = null, $method = null, $callback = null) { - $names = array(); - $allNames = array_keys($this->listeners); - if ($scope and $method) { - $name = $scope . '::' . $method; - if (isset($this->listeners[$name])) { - $names[] = $name; - } - } elseif ($scope) { - foreach ($allNames as $name) { - $parts = explode('::', $name, 2); - if ($parts[0] == $scope) { - $names[] = $name; - } - } - } elseif ($method) { - foreach ($allNames as $name) { - $parts = explode('::', $name, 2); - if ($parts[1] == $method) { - $names[] = $name; - } - } - } else { - $names = $allNames; - } - - foreach ($names as $name) { - if ($callback) { - $index = array_search($callback, $this->listeners[$name]); - if ($index !== false) { - unset($this->listeners[$name][$index]); - } - } else { - $this->listeners[$name] = array(); - } - } - } - - /** - * @param string $scope - * @param string $method - * @param array $arguments optional - */ - protected function emit($scope, $method, $arguments = array()) { - $eventName = $scope . '::' . $method; - if (isset($this->listeners[$eventName])) { - foreach ($this->listeners[$eventName] as $callback) { - call_user_func_array($callback, $arguments); - } - } - } + use EmitterTrait; } diff --git a/lib/private/hooks/emittertrait.php b/lib/private/hooks/emittertrait.php new file mode 100644 index 0000000000..a80d7d5b03 --- /dev/null +++ b/lib/private/hooks/emittertrait.php @@ -0,0 +1,103 @@ + + * @author Robin Appelman + * + * @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 OC\Hooks; + +trait EmitterTrait { + + /** + * @var (callable[])[] $listeners + */ + protected $listeners = array(); + + /** + * @param string $scope + * @param string $method + * @param callable $callback + */ + public function listen($scope, $method, $callback) { + $eventName = $scope . '::' . $method; + if (!isset($this->listeners[$eventName])) { + $this->listeners[$eventName] = array(); + } + if (array_search($callback, $this->listeners[$eventName]) === false) { + $this->listeners[$eventName][] = $callback; + } + } + + /** + * @param string $scope optional + * @param string $method optional + * @param callable $callback optional + */ + public function removeListener($scope = null, $method = null, $callback = null) { + $names = array(); + $allNames = array_keys($this->listeners); + if ($scope and $method) { + $name = $scope . '::' . $method; + if (isset($this->listeners[$name])) { + $names[] = $name; + } + } elseif ($scope) { + foreach ($allNames as $name) { + $parts = explode('::', $name, 2); + if ($parts[0] == $scope) { + $names[] = $name; + } + } + } elseif ($method) { + foreach ($allNames as $name) { + $parts = explode('::', $name, 2); + if ($parts[1] == $method) { + $names[] = $name; + } + } + } else { + $names = $allNames; + } + + foreach ($names as $name) { + if ($callback) { + $index = array_search($callback, $this->listeners[$name]); + if ($index !== false) { + unset($this->listeners[$name][$index]); + } + } else { + $this->listeners[$name] = array(); + } + } + } + + /** + * @param string $scope + * @param string $method + * @param array $arguments optional + */ + protected function emit($scope, $method, $arguments = array()) { + $eventName = $scope . '::' . $method; + if (isset($this->listeners[$eventName])) { + foreach ($this->listeners[$eventName] as $callback) { + call_user_func_array($callback, $arguments); + } + } + } +} From 24131586d74836a0f08f57511f5ec40a3d3ee156 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 7 May 2015 14:07:02 +0200 Subject: [PATCH 2/6] call mount providers that are registered after the filesystem is setup --- .../files/config/mountprovidercollection.php | 7 +++- lib/private/files/filesystem.php | 41 +++++++++++++++---- tests/lib/files/filesystem.php | 35 ++++++++++++++++ 3 files changed, 75 insertions(+), 8 deletions(-) diff --git a/lib/private/files/config/mountprovidercollection.php b/lib/private/files/config/mountprovidercollection.php index 326d72001d..a14a6ef796 100644 --- a/lib/private/files/config/mountprovidercollection.php +++ b/lib/private/files/config/mountprovidercollection.php @@ -22,12 +22,16 @@ namespace OC\Files\Config; +use OC\Hooks\Emitter; +use OC\Hooks\EmitterTrait; use OCP\Files\Config\IMountProviderCollection; use OCP\Files\Config\IMountProvider; use OCP\Files\Storage\IStorageFactory; use OCP\IUser; -class MountProviderCollection implements IMountProviderCollection { +class MountProviderCollection implements IMountProviderCollection, Emitter { + use EmitterTrait; + /** * @var \OCP\Files\Config\IMountProvider[] */ @@ -65,5 +69,6 @@ class MountProviderCollection implements IMountProviderCollection { */ public function registerProvider(IMountProvider $provider) { $this->providers[] = $provider; + $this->emit('\OC\Files\Config', 'registerMountProvider', [$provider]); } } diff --git a/lib/private/files/filesystem.php b/lib/private/files/filesystem.php index 10c64e1301..1893996ffc 100644 --- a/lib/private/files/filesystem.php +++ b/lib/private/files/filesystem.php @@ -59,7 +59,11 @@ namespace OC\Files; +use OC\Cache\File; +use OC\Files\Config\MountProviderCollection; use OC\Files\Storage\StorageFactory; +use OCP\Files\Config\IMountProvider; +use OCP\IUserManager; class Filesystem { @@ -78,6 +82,8 @@ class Filesystem { static private $normalizedPathCache = array(); + static private $listeningForProviders = false; + /** * classname which used for hooks handling * used as signalclass in OC_Hooks::emit() @@ -371,14 +377,15 @@ class Filesystem { $root = \OC_User::getHome($user); - $userObject = \OC_User::getManager()->get($user); + $userManager = \OC::$server->getUserManager(); + $userObject = $userManager->get($user); if (is_null($userObject)) { - \OCP\Util::writeLog('files', ' Backends provided no user object for '.$user, \OCP\Util::ERROR); + \OCP\Util::writeLog('files', ' Backends provided no user object for ' . $user, \OCP\Util::ERROR); throw new \OC\User\NoUserException(); } - $homeStorage = \OC_Config::getValue( 'objectstore' ); + $homeStorage = \OC_Config::getValue('objectstore'); if (!empty($homeStorage)) { // sanity checks if (empty($homeStorage['class'])) { @@ -412,16 +419,33 @@ class Filesystem { self::mountCacheDir($user); // Chance to mount for other storages - if($userObject) { - $mountConfigManager = \OC::$server->getMountProviderCollection(); + /** @var \OC\Files\Config\MountProviderCollection $mountConfigManager */ + $mountConfigManager = \OC::$server->getMountProviderCollection(); + if ($userObject) { $mounts = $mountConfigManager->getMountsForUser($userObject); array_walk($mounts, array(self::$mounts, 'addMount')); } + + self::listenForNewMountProviders($mountConfigManager, $userManager); \OC_Hook::emit('OC_Filesystem', 'post_initMountPoints', array('user' => $user, 'user_dir' => $root)); } + private static function listenForNewMountProviders(MountProviderCollection $mountConfigManager, IUserManager $userManager) { + if (!self::$listeningForProviders) { + self::$listeningForProviders = true; + $mountConfigManager->listen('\OC\Files\Config', 'registerMountProvider', function (IMountProvider $provider) use ($userManager) { + foreach (Filesystem::$usersSetup as $user => $setup) { + $userObject = $userManager->get($user); + $mounts = $provider->getMountsForUser($userObject, Filesystem::getLoader()); + array_walk($mounts, array(self::$mounts, 'addMount')); + } + }); + } + } + /** * Mounts the cache directory + * * @param string $user user name */ private static function mountCacheDir($user) { @@ -455,6 +479,7 @@ class Filesystem { /** * get the relative path of the root data directory for the current user + * * @return string * * Returns path like /admin/files @@ -537,7 +562,7 @@ class Filesystem { if (!$path || $path[0] !== '/') { $path = '/' . $path; } - if (strpos($path, '/../') !== FALSE || strrchr($path, '/') === '/..') { + if (strpos($path, '/../') !== false || strrchr($path, '/') === '/..') { return false; } return true; @@ -577,6 +602,7 @@ class Filesystem { /** * check if the directory should be ignored when scanning * NOTE: the special directories . and .. would cause never ending recursion + * * @param String $dir * @return boolean */ @@ -745,6 +771,7 @@ class Filesystem { /** * Fix common problems with a file path + * * @param string $path * @param bool $stripTrailingSlash * @param bool $isAbsolutePath @@ -761,7 +788,7 @@ class Filesystem { $cacheKey = json_encode([$path, $stripTrailingSlash, $isAbsolutePath]); - if(isset(self::$normalizedPathCache[$cacheKey])) { + if (isset(self::$normalizedPathCache[$cacheKey])) { return self::$normalizedPathCache[$cacheKey]; } diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php index 082d22781f..b5ee27fad5 100644 --- a/tests/lib/files/filesystem.php +++ b/tests/lib/files/filesystem.php @@ -22,11 +22,32 @@ namespace Test\Files; +use OC\Files\Mount\MountPoint; +use OC\Files\Storage\Temporary; use OC\User\NoUserException; +use OCP\Files\Config\IMountProvider; +use OCP\Files\Storage\IStorageFactory; +use OCP\IUser; + +class DummyMountProvider implements IMountProvider { + private $mounts = []; + + /** + * @param array $mounts + */ + public function __construct(array $mounts) { + $this->mounts = $mounts; + } + + public function getMountsForUser(IUser $user, IStorageFactory $loader) { + return isset($this->mounts[$user->getUID()]) ? $this->mounts[$user->getUID()] : []; + } +} class Filesystem extends \Test\TestCase { const TEST_FILESYSTEM_USER1 = "test-filesystem-user1"; + const TEST_FILESYSTEM_USER2 = "test-filesystem-user1"; /** * @var array tmpDirs @@ -44,6 +65,10 @@ class Filesystem extends \Test\TestCase { protected function setUp() { parent::setUp(); + $userBackend = new \OC_User_Dummy(); + $userBackend->createUser(self::TEST_FILESYSTEM_USER1, self::TEST_FILESYSTEM_USER1); + $userBackend->createUser(self::TEST_FILESYSTEM_USER2, self::TEST_FILESYSTEM_USER2); + \OC::$server->getUserManager()->registerBackend($userBackend); $this->loginAsUser(); } @@ -271,6 +296,7 @@ class Filesystem extends \Test\TestCase { /** * Tests that an exception is thrown when passed user does not exist. + * * @expectedException \OC\User\NoUserException */ public function testLocalMountWhenUserDoesNotExist() { @@ -380,4 +406,13 @@ class Filesystem extends \Test\TestCase { \OC_Config::setValue('cache_path', $oldCachePath); } + + public function testRegisterMountProviderAfterSetup() { + \OC\Files\Filesystem::initMountPoints(self::TEST_FILESYSTEM_USER2); + $this->assertEquals('/', \OC\Files\Filesystem::getMountPoint('/foo/bar')); + $mount = new MountPoint(new Temporary([]), '/foo/bar'); + $mountProvider = new DummyMountProvider([self::TEST_FILESYSTEM_USER2 => [$mount]]); + \OC::$server->getMountProviderCollection()->registerProvider($mountProvider); + $this->assertEquals('/foo/bar/', \OC\Files\Filesystem::getMountPoint('/foo/bar')); + } } From 6f47a547d8a86d1425b00ae0d11a692e772ec4ac Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 7 May 2015 14:40:44 +0200 Subject: [PATCH 3/6] only use mount provider if we have a valid user --- lib/private/files/filesystem.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/private/files/filesystem.php b/lib/private/files/filesystem.php index 1893996ffc..2391e84db5 100644 --- a/lib/private/files/filesystem.php +++ b/lib/private/files/filesystem.php @@ -436,8 +436,10 @@ class Filesystem { $mountConfigManager->listen('\OC\Files\Config', 'registerMountProvider', function (IMountProvider $provider) use ($userManager) { foreach (Filesystem::$usersSetup as $user => $setup) { $userObject = $userManager->get($user); - $mounts = $provider->getMountsForUser($userObject, Filesystem::getLoader()); - array_walk($mounts, array(self::$mounts, 'addMount')); + if ($userObject) { + $mounts = $provider->getMountsForUser($userObject, Filesystem::getLoader()); + array_walk($mounts, array(self::$mounts, 'addMount')); + } } }); } From 8926bca0c7f27774959ec831c830c883bd284100 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 8 May 2015 13:51:32 +0200 Subject: [PATCH 4/6] phpdoc, strict and type hints --- lib/private/files/filesystem.php | 6 ++++++ lib/private/hooks/emittertrait.php | 10 +++++----- tests/lib/files/filesystem.php | 7 +++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/private/files/filesystem.php b/lib/private/files/filesystem.php index 2391e84db5..212deb24b7 100644 --- a/lib/private/files/filesystem.php +++ b/lib/private/files/filesystem.php @@ -430,6 +430,12 @@ class Filesystem { \OC_Hook::emit('OC_Filesystem', 'post_initMountPoints', array('user' => $user, 'user_dir' => $root)); } + /** + * Get mounts from mount providers that are registered after setup + * + * @param MountProviderCollection $mountConfigManager + * @param IUserManager $userManager + */ private static function listenForNewMountProviders(MountProviderCollection $mountConfigManager, IUserManager $userManager) { if (!self::$listeningForProviders) { self::$listeningForProviders = true; diff --git a/lib/private/hooks/emittertrait.php b/lib/private/hooks/emittertrait.php index a80d7d5b03..5d471a3f55 100644 --- a/lib/private/hooks/emittertrait.php +++ b/lib/private/hooks/emittertrait.php @@ -34,12 +34,12 @@ trait EmitterTrait { * @param string $method * @param callable $callback */ - public function listen($scope, $method, $callback) { + public function listen($scope, $method, callable $callback) { $eventName = $scope . '::' . $method; if (!isset($this->listeners[$eventName])) { $this->listeners[$eventName] = array(); } - if (array_search($callback, $this->listeners[$eventName]) === false) { + if (array_search($callback, $this->listeners[$eventName], true) === false) { $this->listeners[$eventName][] = $callback; } } @@ -49,7 +49,7 @@ trait EmitterTrait { * @param string $method optional * @param callable $callback optional */ - public function removeListener($scope = null, $method = null, $callback = null) { + public function removeListener($scope = null, $method = null, callable $callback = null) { $names = array(); $allNames = array_keys($this->listeners); if ($scope and $method) { @@ -77,7 +77,7 @@ trait EmitterTrait { foreach ($names as $name) { if ($callback) { - $index = array_search($callback, $this->listeners[$name]); + $index = array_search($callback, $this->listeners[$name], true); if ($index !== false) { unset($this->listeners[$name][$index]); } @@ -92,7 +92,7 @@ trait EmitterTrait { * @param string $method * @param array $arguments optional */ - protected function emit($scope, $method, $arguments = array()) { + protected function emit($scope, $method, array $arguments = array()) { $eventName = $scope . '::' . $method; if (isset($this->listeners[$eventName])) { foreach ($this->listeners[$eventName] as $callback) { diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php index b5ee27fad5..b7061bd19a 100644 --- a/tests/lib/files/filesystem.php +++ b/tests/lib/files/filesystem.php @@ -39,6 +39,13 @@ class DummyMountProvider implements IMountProvider { $this->mounts = $mounts; } + /** + * Get the pre-registered mount points + * + * @param IUser $user + * @param IStorageFactory $loader + * @return \OCP\Files\Mount\IMountPoint[] + */ public function getMountsForUser(IUser $user, IStorageFactory $loader) { return isset($this->mounts[$user->getUID()]) ? $this->mounts[$user->getUID()] : []; } From 0497534a6e5c7c856faf2f4a1939ab34a7f59a8f Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 8 May 2015 14:27:22 +0200 Subject: [PATCH 5/6] more type hints --- lib/private/files/node/root.php | 4 ++-- lib/private/hooks/emitter.php | 4 ++-- lib/private/hooks/forwardingemitter.php | 4 ++-- lib/private/hooks/legacyemitter.php | 2 +- lib/private/hooks/publicemitter.php | 2 +- lib/private/user/session.php | 4 ++-- tests/lib/hooks/forwardingemitter.php | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/private/files/node/root.php b/lib/private/files/node/root.php index e47f98b0f2..7ffb3674a8 100644 --- a/lib/private/files/node/root.php +++ b/lib/private/files/node/root.php @@ -94,7 +94,7 @@ class Root extends Folder implements IRootFolder { * @param string $method * @param callable $callback */ - public function listen($scope, $method, $callback) { + public function listen($scope, $method, callable $callback) { $this->emitter->listen($scope, $method, $callback); } @@ -103,7 +103,7 @@ class Root extends Folder implements IRootFolder { * @param string $method optional * @param callable $callback optional */ - public function removeListener($scope = null, $method = null, $callback = null) { + public function removeListener($scope = null, $method = null, callable $callback = null) { $this->emitter->removeListener($scope, $method, $callback); } diff --git a/lib/private/hooks/emitter.php b/lib/private/hooks/emitter.php index 895bd2d9ca..bea3f289b8 100644 --- a/lib/private/hooks/emitter.php +++ b/lib/private/hooks/emitter.php @@ -37,7 +37,7 @@ interface Emitter { * @param callable $callback * @return void */ - public function listen($scope, $method, $callback); + public function listen($scope, $method, callable $callback); /** * @param string $scope optional @@ -45,5 +45,5 @@ interface Emitter { * @param callable $callback optional * @return void */ - public function removeListener($scope = null, $method = null, $callback = null); + public function removeListener($scope = null, $method = null, callable $callback = null); } diff --git a/lib/private/hooks/forwardingemitter.php b/lib/private/hooks/forwardingemitter.php index 853004310f..90c1970f48 100644 --- a/lib/private/hooks/forwardingemitter.php +++ b/lib/private/hooks/forwardingemitter.php @@ -40,7 +40,7 @@ abstract class ForwardingEmitter extends BasicEmitter { * @param string $method * @param callable $callback */ - public function listen($scope, $method, $callback) { + public function listen($scope, $method, callable $callback) { parent::listen($scope, $method, $callback); foreach ($this->forwardEmitters as $emitter) { $emitter->listen($scope, $method, $callback); @@ -50,7 +50,7 @@ abstract class ForwardingEmitter extends BasicEmitter { /** * @param \OC\Hooks\Emitter $emitter */ - protected function forward($emitter) { + protected function forward(Emitter $emitter) { $this->forwardEmitters[] = $emitter; //forward all previously connected hooks diff --git a/lib/private/hooks/legacyemitter.php b/lib/private/hooks/legacyemitter.php index b0912d740d..b28854f463 100644 --- a/lib/private/hooks/legacyemitter.php +++ b/lib/private/hooks/legacyemitter.php @@ -23,7 +23,7 @@ namespace OC\Hooks; abstract class LegacyEmitter extends BasicEmitter { - protected function emit($scope, $method, $arguments = array()) { + protected function emit($scope, $method, array $arguments = array()) { \OC_Hook::emit($scope, $method, $arguments); parent::emit($scope, $method, $arguments); } diff --git a/lib/private/hooks/publicemitter.php b/lib/private/hooks/publicemitter.php index 71641c9d5e..12de07b27c 100644 --- a/lib/private/hooks/publicemitter.php +++ b/lib/private/hooks/publicemitter.php @@ -28,7 +28,7 @@ class PublicEmitter extends BasicEmitter { * @param string $method * @param array $arguments optional */ - public function emit($scope, $method, $arguments = array()) { + public function emit($scope, $method, array $arguments = array()) { parent::emit($scope, $method, $arguments); } } diff --git a/lib/private/user/session.php b/lib/private/user/session.php index a66ae6bd8a..75a884fb45 100644 --- a/lib/private/user/session.php +++ b/lib/private/user/session.php @@ -81,7 +81,7 @@ class Session implements IUserSession, Emitter { * @param string $method * @param callable $callback */ - public function listen($scope, $method, $callback) { + public function listen($scope, $method, callable $callback) { $this->manager->listen($scope, $method, $callback); } @@ -90,7 +90,7 @@ class Session implements IUserSession, Emitter { * @param string $method optional * @param callable $callback optional */ - public function removeListener($scope = null, $method = null, $callback = null) { + public function removeListener($scope = null, $method = null, callable $callback = null) { $this->manager->removeListener($scope, $method, $callback); } diff --git a/tests/lib/hooks/forwardingemitter.php b/tests/lib/hooks/forwardingemitter.php index decf6bb354..5e8e252d3e 100644 --- a/tests/lib/hooks/forwardingemitter.php +++ b/tests/lib/hooks/forwardingemitter.php @@ -17,7 +17,7 @@ class DummyForwardingEmitter extends \OC\Hooks\ForwardingEmitter { /** * @param \OC\Hooks\Emitter $emitter */ - public function forward($emitter) { + public function forward(\OC\Hooks\Emitter $emitter) { parent::forward($emitter); } } From a9455be14a36013adead81de06e3799e6e7efb9b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 8 May 2015 15:05:14 +0200 Subject: [PATCH 6/6] more hints --- lib/private/repair.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/repair.php b/lib/private/repair.php index c690fe4a8c..43350995be 100644 --- a/lib/private/repair.php +++ b/lib/private/repair.php @@ -142,7 +142,7 @@ class Repair extends BasicEmitter { * * Re-declared as public to allow invocation from within the closure above in php 5.3 */ - public function emit($scope, $method, $arguments = array()) { + public function emit($scope, $method, array $arguments = array()) { parent::emit($scope, $method, $arguments); } }