From 357263a70bf56e351e0d292c6144f998dad7f079 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 30 May 2019 14:20:15 +0200 Subject: [PATCH 1/2] Do not try to autoload built in types This avoids calls to the autoloader (or chain of autoloaders) to see if for example 'principalPrefix' class can be found. While we already know it is a string. Signed-off-by: Roeland Jago Douma --- apps/dav/tests/unit/AppInfo/PluginManagerTest.php | 12 ++++++------ .../DependencyInjection/DIContainer.php | 9 ++------- .../AppFramework/Utility/SimpleContainer.php | 14 +++++--------- lib/private/ServerContainer.php | 9 ++------- lib/public/IContainer.php | 3 ++- .../ContactsMenu/ActionProviderStoreTest.php | 6 +++--- 6 files changed, 20 insertions(+), 33 deletions(-) diff --git a/apps/dav/tests/unit/AppInfo/PluginManagerTest.php b/apps/dav/tests/unit/AppInfo/PluginManagerTest.php index ed19bdd53c..0e4c0d20ea 100644 --- a/apps/dav/tests/unit/AppInfo/PluginManagerTest.php +++ b/apps/dav/tests/unit/AppInfo/PluginManagerTest.php @@ -80,12 +80,12 @@ class PluginManagerTest extends TestCase { $server->method('query') ->will($this->returnValueMap([ - ['\OCA\DAV\ADavApp\PluginOne', 'dummyplugin1'], - ['\OCA\DAV\ADavApp\PluginTwo', 'dummyplugin2'], - ['\OCA\DAV\ADavApp\CollectionOne', 'dummycollection1'], - ['\OCA\DAV\ADavApp\CollectionTwo', 'dummycollection2'], - ['\OCA\DAV\ADavApp2\PluginOne', 'dummy2plugin1'], - ['\OCA\DAV\ADavApp2\CollectionOne', 'dummy2collection1'], + ['\OCA\DAV\ADavApp\PluginOne', true, 'dummyplugin1'], + ['\OCA\DAV\ADavApp\PluginTwo', true, 'dummyplugin2'], + ['\OCA\DAV\ADavApp\CollectionOne', true, 'dummycollection1'], + ['\OCA\DAV\ADavApp\CollectionTwo', true, 'dummycollection2'], + ['\OCA\DAV\ADavApp2\PluginOne', true, 'dummy2plugin1'], + ['\OCA\DAV\ADavApp2\CollectionOne', true, 'dummy2collection1'], ])); $expectedPlugins = [ diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index a6a9b20574..6d337bb932 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -374,17 +374,12 @@ class DIContainer extends SimpleContainer implements IAppContainer { }); } - /** - * @param string $name - * @return mixed - * @throws QueryException if the query could not be resolved - */ - public function query($name) { + public function query(string $name, bool $autoload = true) { try { return $this->queryNoFallback($name); } catch (QueryException $firstException) { try { - return $this->getServer()->query($name); + return $this->getServer()->query($name, $autoload); } catch (QueryException $secondException) { if ($firstException->getCode() === 1) { throw $secondException; diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 6c2844e681..6f50523bbf 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -65,7 +65,8 @@ class SimpleContainer extends Container implements IContainer { } try { - $parameters[] = $this->query($resolveName); + $builtIn = $parameter->hasType() && $parameter->getType()->isBuiltin(); + $parameters[] = $this->query($resolveName, !$builtIn); } catch (QueryException $e) { // Service not found, use the default value when available if ($parameter->isDefaultValueAvailable()) { @@ -105,23 +106,18 @@ class SimpleContainer extends Container implements IContainer { } } - - /** - * @param string $name name of the service to query for - * @return mixed registered service for the given $name - * @throws QueryException if the query could not be resolved - */ - public function query($name) { + public function query(string $name, bool $autoload = true) { $name = $this->sanitizeName($name); if ($this->offsetExists($name)) { return $this->offsetGet($name); - } else { + } else if ($autoload) { $object = $this->resolve($name); $this->registerService($name, function () use ($object) { return $object; }); return $object; } + throw new QueryException('Could not resolve ' . $name . '!'); } /** diff --git a/lib/private/ServerContainer.php b/lib/private/ServerContainer.php index 8c2b49bb08..b67b4d1e70 100644 --- a/lib/private/ServerContainer.php +++ b/lib/private/ServerContainer.php @@ -113,12 +113,7 @@ class ServerContainer extends SimpleContainer { throw new QueryException(); } - /** - * @param string $name name of the service to query for - * @return mixed registered service for the given $name - * @throws QueryException if the query could not be resolved - */ - public function query($name) { + public function query(string $name, bool $autoload = true) { $name = $this->sanitizeName($name); if (isset($this[$name])) { @@ -147,6 +142,6 @@ class ServerContainer extends SimpleContainer { } } - return parent::query($name); + return parent::query($name, $autoload); } } diff --git a/lib/public/IContainer.php b/lib/public/IContainer.php index f7ca069767..558c72291c 100644 --- a/lib/public/IContainer.php +++ b/lib/public/IContainer.php @@ -61,11 +61,12 @@ interface IContainer { * Look up a service for a given name in the container. * * @param string $name + * @param bool $autoload Should we try to autoload the service. If we are trying to resolve built in types this makes no sense for example * @return mixed * @throws QueryException if the query could not be resolved * @since 6.0.0 */ - public function query($name); + public function query(string $name, bool $autoload = true); /** * A value is stored in the container with it's corresponding name diff --git a/tests/lib/Contacts/ContactsMenu/ActionProviderStoreTest.php b/tests/lib/Contacts/ContactsMenu/ActionProviderStoreTest.php index 8738e19b51..fb5e815447 100644 --- a/tests/lib/Contacts/ContactsMenu/ActionProviderStoreTest.php +++ b/tests/lib/Contacts/ContactsMenu/ActionProviderStoreTest.php @@ -81,8 +81,8 @@ class ActionProviderStoreTest extends TestCase { $this->serverContainer->expects($this->exactly(2)) ->method('query') ->will($this->returnValueMap([ - [EMailProvider::class, $provider1], - ['OCA\Contacts\Provider1', $provider2] + [EMailProvider::class, true, $provider1], + ['OCA\Contacts\Provider1', true, $provider2] ])); $providers = $this->actionProviderStore->getProviders($user); @@ -106,7 +106,7 @@ class ActionProviderStoreTest extends TestCase { $this->serverContainer->expects($this->once()) ->method('query') ->will($this->returnValueMap([ - [EMailProvider::class, $provider1], + [EMailProvider::class, true, $provider1], ])); $providers = $this->actionProviderStore->getProviders($user); From 49361c40e26d0235572c83f98f48f876027ae835 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 30 May 2019 14:37:26 +0200 Subject: [PATCH 2/2] Typehint builtin types in constructor to not initiate autoloading Signed-off-by: Roeland Jago Douma --- apps/dav/lib/CalDAV/CalDavBackend.php | 2 +- apps/dav/lib/Connector/Sabre/Principal.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index 14de848013..2d3dbd8dd1 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -192,7 +192,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription ISecureRandom $random, ILogger $logger, EventDispatcherInterface $dispatcher, - $legacyEndpoint = false) { + bool $legacyEndpoint = false) { $this->db = $db; $this->principalBackend = $principalBackend; $this->userManager = $userManager; diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index 812f9d5416..902c70bdaf 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -91,7 +91,7 @@ class Principal implements BackendInterface { IUserSession $userSession, IConfig $config, IAppManager $appManager, - $principalPrefix = 'principals/users/') { + string $principalPrefix = 'principals/users/') { $this->userManager = $userManager; $this->groupManager = $groupManager; $this->shareManager = $shareManager;