From 67909cf87bf17e6369d7e244aa956060aa40d25d Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 21 Mar 2017 20:53:37 +0100 Subject: [PATCH 1/6] Make DI work for all apps As stated in https://github.com/nextcloud/server/pull/3901#issuecomment-288135309 appid's don't have to match the namespace. Work around this Signed-off-by: Roeland Jago Douma --- .../DependencyInjection/DIContainer.php | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 06825d2dd5..9d27aff486 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -373,21 +373,28 @@ class DIContainer extends SimpleContainer implements IAppContainer { }); } + private function getFallbackNamespace($name) { + $segments = explode('\\', $name); + if (count($segments) >= 2) { + return $segments[0] . '\\' . ucfirst(strtolower($segments[1])); + } else { + return null; + } + } + public function query($name) { $name = $this->sanitizeName($name); if ($this->offsetExists($name)) { return parent::query($name); } else { - if (strpos($name, 'OCA\\') === 0 && substr_count($name, '\\') >= 2) { - $segments = explode('\\', $name); - if (strtolower($segments[1]) === strtolower($this['AppName'])) { - return parent::query($name); - } - } else if ($this['AppName'] === 'settings' && strpos($name, 'OC\\Settings\\') === 0) { + if ($this['AppName'] === 'settings' && strpos($name, 'OC\\Settings\\') === 0) { return parent::query($name); } else if ($this['AppName'] === 'core' && strpos($name, 'OC\\Core\\') === 0) { return parent::query($name); + } else if (strpos($name, \OC\AppFramework\App::buildAppNamespace($this['AppName'])) === 0 || + $this->getFallbackNamespace($name) === \OC\AppFramework\App::buildAppNamespace($this['AppName'])) { + return parent::query($name); } } From 9208f6379cf74731e2cd2acb77cac0403bacd00e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 22 Mar 2017 10:13:14 +0100 Subject: [PATCH 2/6] buildAppNamespace already has the fallback Signed-off-by: Joas Schilling --- .../AppFramework/DependencyInjection/DIContainer.php | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 9d27aff486..9078baf1d2 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -373,15 +373,6 @@ class DIContainer extends SimpleContainer implements IAppContainer { }); } - private function getFallbackNamespace($name) { - $segments = explode('\\', $name); - if (count($segments) >= 2) { - return $segments[0] . '\\' . ucfirst(strtolower($segments[1])); - } else { - return null; - } - } - public function query($name) { $name = $this->sanitizeName($name); @@ -392,8 +383,7 @@ class DIContainer extends SimpleContainer implements IAppContainer { return parent::query($name); } else if ($this['AppName'] === 'core' && strpos($name, 'OC\\Core\\') === 0) { return parent::query($name); - } else if (strpos($name, \OC\AppFramework\App::buildAppNamespace($this['AppName'])) === 0 || - $this->getFallbackNamespace($name) === \OC\AppFramework\App::buildAppNamespace($this['AppName'])) { + } else if (strpos($name, \OC\AppFramework\App::buildAppNamespace($this['AppName']) . '\\') === 0) { return parent::query($name); } } From 5695a4ec9287775912b9df1b0038964543e257d1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 22 Mar 2017 10:33:30 +0100 Subject: [PATCH 3/6] Don't do a recursive search Signed-off-by: Joas Schilling --- .../DependencyInjection/DIContainer.php | 22 ++++++++++++++++++- lib/private/ServerContainer.php | 2 +- lib/public/IContainer.php | 1 + .../TwoFactorAuth/ManagerTest.php | 2 +- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 9078baf1d2..4fb13b09ae 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -49,6 +49,7 @@ use OC\ServerContainer; use OCP\AppFramework\Http\IOutput; use OCP\AppFramework\IApi; use OCP\AppFramework\IAppContainer; +use OCP\AppFramework\QueryException; use OCP\Files\Folder; use OCP\Files\IAppData; use OCP\IL10N; @@ -373,7 +374,25 @@ class DIContainer extends SimpleContainer implements IAppContainer { }); } + /** + * @param string $name + * @return mixed + * @throws QueryException if the query could not be resolved + */ public function query($name) { + try { + return $this->queryNoFallback($name); + } catch (QueryException $e) { + return $this->getServer()->query($name); + } + } + + /** + * @param string $name + * @return mixed + * @throws QueryException if the query could not be resolved + */ + public function queryNoFallback($name) { $name = $this->sanitizeName($name); if ($this->offsetExists($name)) { @@ -388,6 +407,7 @@ class DIContainer extends SimpleContainer implements IAppContainer { } } - return $this->getServer()->query($name); + throw new QueryException('Could not resolve ' . $name . '!' . + ' Class can not be instantiated'); } } diff --git a/lib/private/ServerContainer.php b/lib/private/ServerContainer.php index df0293addf..fe868867b5 100644 --- a/lib/private/ServerContainer.php +++ b/lib/private/ServerContainer.php @@ -79,7 +79,7 @@ class ServerContainer extends SimpleContainer { $segments = explode('\\', $name); $appContainer = $this->getAppContainer(strtolower($segments[1])); try { - return $appContainer->query($name); + return $appContainer->queryNoFallback($name); } catch (QueryException $e) { // Didn't find the service in the respective app container, // ignore it and fall back to the core container. diff --git a/lib/public/IContainer.php b/lib/public/IContainer.php index 9dc7f9f941..f7ca069767 100644 --- a/lib/public/IContainer.php +++ b/lib/public/IContainer.php @@ -62,6 +62,7 @@ interface IContainer { * * @param string $name * @return mixed + * @throws QueryException if the query could not be resolved * @since 6.0.0 */ public function query($name); diff --git a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php index 1ea17f5d30..c031c39b5f 100644 --- a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php +++ b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php @@ -68,7 +68,7 @@ class ManagerTest extends TestCase { parent::setUp(); $this->user = $this->createMock(IUser::class); - $this->appManager = $this->createMock('\OC\App\AppManager'); + $this->appManager = $this->createMock(AppManager::class); $this->session = $this->createMock(ISession::class); $this->config = $this->createMock(IConfig::class); $this->activityManager = $this->createMock(IManager::class); From 8b94fbe0140918e6c330d7111f42d86f73f76744 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 22 Mar 2017 11:24:16 +0100 Subject: [PATCH 4/6] Add the AppContainer Signed-off-by: Roeland Jago Douma --- tests/lib/InfoXmlTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/lib/InfoXmlTest.php b/tests/lib/InfoXmlTest.php index 4e75ca7820..18391a20c0 100644 --- a/tests/lib/InfoXmlTest.php +++ b/tests/lib/InfoXmlTest.php @@ -63,6 +63,14 @@ class InfoXmlTest extends TestCase { $appPath = \OC_App::getAppPath($app); \OC_App::registerAutoloading($app, $appPath); + //Add the appcontainer + $applicationClassName = \OCP\AppFramework\App::buildAppNamespace($app) . '\\AppInfo\\Application'; + if (class_exists($applicationClassName)) { + $application = new $applicationClassName(); + } else { + $application = new \OCP\AppFramework\App($app); + } + if (isset($appInfo['background-jobs'])) { foreach ($appInfo['background-jobs'] as $job) { $this->assertTrue(class_exists($job), 'Asserting background job "' . $job . '" exists'); From 3f86f1276f69fb97f0c2c2a6e133152e695e2d9b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 22 Mar 2017 11:50:31 +0100 Subject: [PATCH 5/6] Also cache the namespace from appinfo Signed-off-by: Joas Schilling --- lib/private/AppFramework/App.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/private/AppFramework/App.php b/lib/private/AppFramework/App.php index e15e4a797e..5a7bc8fad4 100644 --- a/lib/private/AppFramework/App.php +++ b/lib/private/AppFramework/App.php @@ -61,11 +61,12 @@ class App { $appInfo = \OC_App::getAppInfo($appId); if (isset($appInfo['namespace'])) { - return $topNamespace . trim($appInfo['namespace']); + self::$nameSpaceCache[$appId] = trim($appInfo['namespace']); + } else { + // if the tag is not found, fall back to uppercasing the first letter + self::$nameSpaceCache[$appId] = ucfirst($appId); } - // if the tag is not found, fall back to uppercasing the first letter - self::$nameSpaceCache[$appId] = ucfirst($appId); return $topNamespace . self::$nameSpaceCache[$appId]; } From 9667ac2b8e24122cce7e446ff9dc70f6d162389f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 22 Mar 2017 11:50:58 +0100 Subject: [PATCH 6/6] Register the namespace with the autoloading to allow reverse recovery Signed-off-by: Joas Schilling --- lib/private/ServerContainer.php | 32 +++++++++++++++++++++++++------- lib/private/legacy/app.php | 1 + 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/private/ServerContainer.php b/lib/private/ServerContainer.php index fe868867b5..e7b1ed2dad 100644 --- a/lib/private/ServerContainer.php +++ b/lib/private/ServerContainer.php @@ -37,12 +37,26 @@ class ServerContainer extends SimpleContainer { /** @var DIContainer[] */ protected $appContainers; + /** @var string[] */ + protected $namespaces; + /** * ServerContainer constructor. */ public function __construct() { parent::__construct(); $this->appContainers = []; + $this->namespaces = []; + } + + /** + * @param string $appName + * @param string $appNamespace + */ + public function registerNamespace($appName, $appNamespace) { + // Cut of OCA\ and lowercase + $appNamespace = strtolower(substr($appNamespace, strrpos($appNamespace, '\\') + 1)); + $this->namespaces[$appNamespace] = $appName; } /** @@ -54,15 +68,19 @@ class ServerContainer extends SimpleContainer { } /** - * @param string $appName + * @param string $namespace * @return DIContainer + * @throws QueryException */ - public function getAppContainer($appName) { - if (isset($this->appContainers[$appName])) { - return $this->appContainers[$appName]; + protected function getAppContainer($namespace) { + if (isset($this->appContainers[$namespace])) { + return $this->appContainers[$namespace]; } - return new DIContainer($appName); + if (isset($this->namespaces[$namespace])) { + return new DIContainer($this->namespaces[$namespace]); + } + throw new QueryException(); } /** @@ -77,11 +95,11 @@ class ServerContainer extends SimpleContainer { // the apps container first. if (strpos($name, 'OCA\\') === 0 && substr_count($name, '\\') >= 2) { $segments = explode('\\', $name); - $appContainer = $this->getAppContainer(strtolower($segments[1])); try { + $appContainer = $this->getAppContainer(strtolower($segments[1])); return $appContainer->queryNoFallback($name); } catch (QueryException $e) { - // Didn't find the service in the respective app container, + // Didn't find the service or the respective app container, // ignore it and fall back to the core container. } } diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index c82d620882..5343e7ad17 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -189,6 +189,7 @@ class OC_App { self::$alreadyRegistered[$key] = true; // Register on PSR-4 composer autoloader $appNamespace = \OC\AppFramework\App::buildAppNamespace($app); + \OC::$server->registerNamespace($app, $appNamespace); \OC::$composerAutoloader->addPsr4($appNamespace . '\\', $path . '/lib/', true); if (defined('PHPUNIT_RUN') || defined('CLI_TEST_RUN')) { \OC::$composerAutoloader->addPsr4($appNamespace . '\\Tests\\', $path . '/tests/', true);