From 06c062bebda22f6f3ae01c11bb7f88f0552454a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 29 Apr 2025 15:54:20 +0200 Subject: [PATCH 1/8] feat: Use Lazy ghosts for Dependency injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will only work with PHP 8.4, so we’ll need to put it behind a version check later. Signed-off-by: Côme Chilliet --- .../AppFramework/Utility/SimpleContainer.php | 73 ++++++++++--------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 9af65a37ab8..d245cc05042 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -23,8 +23,7 @@ use function class_exists; * SimpleContainer is a simple implementation of a container on basis of Pimple */ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { - /** @var Container */ - private $container; + private Container $container; public function __construct() { $this->container = new Container(); @@ -49,53 +48,55 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { /** * @param ReflectionClass $class the class to instantiate - * @return \stdClass the created class + * @return object the created class * @suppress PhanUndeclaredClassInstanceof */ - private function buildClass(ReflectionClass $class) { + private function buildClass(ReflectionClass $class): object { $constructor = $class->getConstructor(); if ($constructor === null) { + /* No constructor, return a instance directly */ return $class->newInstance(); } + return $class->newLazyGhost(function (object $object) use ($constructor): void { + $object->__construct(...array_map(function (ReflectionParameter $parameter) { + $parameterType = $parameter->getType(); - return $class->newInstanceArgs(array_map(function (ReflectionParameter $parameter) { - $parameterType = $parameter->getType(); + $resolveName = $parameter->getName(); - $resolveName = $parameter->getName(); - - // try to find out if it is a class or a simple parameter - if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { - $resolveName = $parameterType->getName(); - } - - try { - $builtIn = $parameter->hasType() && ($parameter->getType() instanceof ReflectionNamedType) - && $parameter->getType()->isBuiltin(); - return $this->query($resolveName, !$builtIn); - } catch (QueryException $e) { - // Service not found, use the default value when available - if ($parameter->isDefaultValueAvailable()) { - return $parameter->getDefaultValue(); + // try to find out if it is a class or a simple parameter + if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { + $resolveName = $parameterType->getName(); } - if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { - $resolveName = $parameter->getName(); - try { - return $this->query($resolveName); - } catch (QueryException $e2) { - // Pass null if typed and nullable - if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) { - return null; - } + try { + $builtIn = $parameter->hasType() && ($parameter->getType() instanceof ReflectionNamedType) + && $parameter->getType()->isBuiltin(); + return $this->query($resolveName, !$builtIn); + } catch (QueryException $e) { + // Service not found, use the default value when available + if ($parameter->isDefaultValueAvailable()) { + return $parameter->getDefaultValue(); + } - // don't lose the error we got while trying to query by type - throw new QueryException($e->getMessage(), (int)$e->getCode(), $e); + if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { + $resolveName = $parameter->getName(); + try { + return $this->query($resolveName); + } catch (QueryException $e2) { + // Pass null if typed and nullable + if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) { + return null; + } + + // don't lose the error we got while trying to query by type + throw new QueryException($e->getMessage(), (int)$e->getCode(), $e); + } } - } - throw $e; - } - }, $constructor->getParameters())); + throw $e; + } + }, $constructor->getParameters())); + }); } public function resolve($name) { From 1bf41550d0b0240809664ccead2661c5467d222f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 29 Apr 2025 16:11:02 +0200 Subject: [PATCH 2/8] chore: Suppress psalm error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/AppFramework/Utility/SimpleContainer.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index d245cc05042..119c5018443 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -58,6 +58,7 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { return $class->newInstance(); } return $class->newLazyGhost(function (object $object) use ($constructor): void { + /** @psalm-suppress DirectConstructorCall For lazy ghosts we have to call the constructor directly */ $object->__construct(...array_map(function (ReflectionParameter $parameter) { $parameterType = $parameter->getType(); @@ -69,8 +70,8 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { } try { - $builtIn = $parameter->hasType() && ($parameter->getType() instanceof ReflectionNamedType) - && $parameter->getType()->isBuiltin(); + $builtIn = $parameterType !== null && ($parameterType instanceof ReflectionNamedType) + && $parameterType->isBuiltin(); return $this->query($resolveName, !$builtIn); } catch (QueryException $e) { // Service not found, use the default value when available From 86ff5810d71d93c31a2509b090ba7b881725d700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 29 Apr 2025 16:47:32 +0200 Subject: [PATCH 3/8] fix: Only use Lazy objects if PHP is 8.4 or higher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../AppFramework/Utility/SimpleContainer.php | 76 ++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 119c5018443..9110f43f288 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -12,6 +12,7 @@ use Closure; use OCP\AppFramework\QueryException; use OCP\IContainer; use Pimple\Container; +use Psr\Container\ContainerExceptionInterface; use Psr\Container\ContainerInterface; use ReflectionClass; use ReflectionException; @@ -57,47 +58,54 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { /* No constructor, return a instance directly */ return $class->newInstance(); } - return $class->newLazyGhost(function (object $object) use ($constructor): void { - /** @psalm-suppress DirectConstructorCall For lazy ghosts we have to call the constructor directly */ - $object->__construct(...array_map(function (ReflectionParameter $parameter) { - $parameterType = $parameter->getType(); + if (PHP_VERSION_ID >= 80400) { + /* For PHP>=8.4, use a lazy ghost to delay constructor and dependency resolving */ + /** @psalm-suppress UndefinedMethod */ + return $class->newLazyGhost(function (object $object) use ($constructor): void { + /** @psalm-suppress DirectConstructorCall For lazy ghosts we have to call the constructor directly */ + $object->__construct(...$this->buildClassConstructorParameters($constructor)); + }); + } else { + return $class->newInstanceArgs($this->buildClassConstructorParameters($constructor)); + } + } - $resolveName = $parameter->getName(); + private function buildClassConstructorParameters(\ReflectionMethod $constructor): array { + return array_map(function (ReflectionParameter $parameter) { + $parameterType = $parameter->getType(); - // try to find out if it is a class or a simple parameter - if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { - $resolveName = $parameterType->getName(); - } + $resolveName = $parameter->getName(); - try { - $builtIn = $parameterType !== null && ($parameterType instanceof ReflectionNamedType) - && $parameterType->isBuiltin(); - return $this->query($resolveName, !$builtIn); - } catch (QueryException $e) { - // Service not found, use the default value when available - if ($parameter->isDefaultValueAvailable()) { - return $parameter->getDefaultValue(); - } + // try to find out if it is a class or a simple parameter + if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { + $resolveName = $parameterType->getName(); + } + + try { + $builtIn = $parameterType !== null && ($parameterType instanceof ReflectionNamedType) + && $parameterType->isBuiltin(); + return $this->query($resolveName, !$builtIn); + } catch (ContainerExceptionInterface $e) { + // Service not found, use the default value when available + if ($parameter->isDefaultValueAvailable()) { + return $parameter->getDefaultValue(); + } - if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { - $resolveName = $parameter->getName(); - try { - return $this->query($resolveName); - } catch (QueryException $e2) { - // Pass null if typed and nullable - if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) { - return null; - } - - // don't lose the error we got while trying to query by type - throw new QueryException($e->getMessage(), (int)$e->getCode(), $e); + if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { + $resolveName = $parameter->getName(); + try { + return $this->query($resolveName); + } catch (ContainerExceptionInterface $e2) { + // Pass null if typed and nullable + if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) { + return null; } } - - throw $e; } - }, $constructor->getParameters())); - }); + + throw $e; + } + }, $constructor->getParameters()); } public function resolve($name) { From 98b2cfc4162115e7295f683d2810e53df93010b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 29 Apr 2025 17:32:15 +0200 Subject: [PATCH 4/8] fix: Fix several side effects of lazy ghosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/AppFramework/Utility/SimpleContainer.php | 3 +++ lib/public/AppFramework/App.php | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 9110f43f288..200d5c078a8 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -100,6 +100,9 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) { return null; } + + // don't lose the error we got while trying to query by type + throw new QueryException($e->getMessage(), (int)$e->getCode(), $e); } } diff --git a/lib/public/AppFramework/App.php b/lib/public/AppFramework/App.php index 6860de7c324..eec5c6e83e9 100644 --- a/lib/public/AppFramework/App.php +++ b/lib/public/AppFramework/App.php @@ -69,6 +69,12 @@ class App { $step['args'][1] === $classNameParts[1]) { $setUpViaQuery = true; break; + } elseif (isset($step['class'], $step['function'], $step['args'][0]) && + $step['class'] === \ReflectionClass::class && + $step['function'] === 'initializeLazyObject' && + $step['args'][0] === $this) { + $setUpViaQuery = true; + break; } } From a42ef7d633703de8cccbec1460b7c88b48995572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 29 Apr 2025 17:35:30 +0200 Subject: [PATCH 5/8] fix: Fix psalm issue and update baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- build/psalm-baseline.xml | 44 ---------------------------------------- 1 file changed, 44 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 39eeb1e91d1..7b3bc4bb813 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3450,53 +3450,9 @@ - - newInstance()]]> - newInstanceArgs(array_map(function (ReflectionParameter $parameter) { - $parameterType = $parameter->getType(); - - $resolveName = $parameter->getName(); - - // try to find out if it is a class or a simple parameter - if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { - $resolveName = $parameterType->getName(); - } - - try { - $builtIn = $parameter->hasType() && ($parameter->getType() instanceof ReflectionNamedType) - && $parameter->getType()->isBuiltin(); - return $this->query($resolveName, !$builtIn); - } catch (QueryException $e) { - // Service not found, use the default value when available - if ($parameter->isDefaultValueAvailable()) { - return $parameter->getDefaultValue(); - } - - if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { - $resolveName = $parameter->getName(); - try { - return $this->query($resolveName); - } catch (QueryException $e2) { - // Pass null if typed and nullable - if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) { - return null; - } - - // don't lose the error we got while trying to query by type - throw new QueryException($e->getMessage(), (int)$e->getCode(), $e); - } - } - - throw $e; - } - }, $constructor->getParameters()))]]> - - - - getCode()]]> From a10182f6fb3eecd2b944f4842d8c51e07628049f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 5 May 2025 10:17:19 +0200 Subject: [PATCH 6/8] fix(tests): Force lazy ghost initialisation in container tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/AppFramework/Utility/SimpleContainerTest.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/lib/AppFramework/Utility/SimpleContainerTest.php b/tests/lib/AppFramework/Utility/SimpleContainerTest.php index 754da8e5fb3..a1cc1e76aeb 100644 --- a/tests/lib/AppFramework/Utility/SimpleContainerTest.php +++ b/tests/lib/AppFramework/Utility/SimpleContainerTest.php @@ -36,7 +36,9 @@ class ClassComplexConstructor { } class ClassNullableUntypedConstructorArg { + public $class; public function __construct($class) { + $this->class = $class; } } class ClassNullableTypedConstructorArg { @@ -217,6 +219,8 @@ class SimpleContainerTest extends \Test\TestCase { $object = $this->container->query( 'Test\AppFramework\Utility\ClassComplexConstructor' ); + /* Use the object to trigger DI on PHP >= 8.4 */ + get_object_vars($object); } public function testRegisterFactory(): void { @@ -243,7 +247,11 @@ class SimpleContainerTest extends \Test\TestCase { public function testQueryUntypedNullable(): void { $this->expectException(\OCP\AppFramework\QueryException::class); - $this->container->query(ClassNullableUntypedConstructorArg::class); + $object = $this->container->query( + ClassNullableUntypedConstructorArg::class + ); + /* Use the object to trigger DI on PHP >= 8.4 */ + get_object_vars($object); } public function testQueryTypedNullable(): void { From 2eed6d3a890958777faa93cf7e1cdf8f77762a88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 14 May 2025 18:42:42 +0200 Subject: [PATCH 7/8] feat: Add a configuration toggle for lazy objects in DI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- config/config.sample.php | 8 ++++++++ lib/base.php | 3 +++ lib/private/AppFramework/Utility/SimpleContainer.php | 4 +++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/config/config.sample.php b/config/config.sample.php index ac15d9f5aeb..da87edff575 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -2743,4 +2743,12 @@ $CONFIG = [ * Defaults to true. */ 'files.trash.delete' => true, + +/** + * Enable lazy objects feature from PHP 8.4 to be used in the Dependency Injection. + * Should improve performances by avoiding buiding unused objects. + * + * Defaults to false. + */ +'enable_lazy_objects' => false, ]; diff --git a/lib/base.php b/lib/base.php index 2b08137aff2..9fa6707654a 100644 --- a/lib/base.php +++ b/lib/base.php @@ -618,6 +618,9 @@ class OC { } $loaderEnd = microtime(true); + // Enable lazy loading if activated + \OC\AppFramework\Utility\SimpleContainer::$useLazyObjects = (bool)self::$config->getValue('enable_lazy_objects'); + // setup the basic server self::$server = new \OC\Server(\OC::$WEBROOT, self::$config); self::$server->boot(); diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 200d5c078a8..481c12cc708 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -24,6 +24,8 @@ use function class_exists; * SimpleContainer is a simple implementation of a container on basis of Pimple */ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { + public static bool $useLazyObjects = false; + private Container $container; public function __construct() { @@ -58,7 +60,7 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { /* No constructor, return a instance directly */ return $class->newInstance(); } - if (PHP_VERSION_ID >= 80400) { + if (PHP_VERSION_ID >= 80400 && self::$useLazyObjects) { /* For PHP>=8.4, use a lazy ghost to delay constructor and dependency resolving */ /** @psalm-suppress UndefinedMethod */ return $class->newLazyGhost(function (object $object) use ($constructor): void { From 78ff8e233fcfaf596a85ddc3bbacda7aca537fa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Fri, 16 May 2025 17:07:53 +0200 Subject: [PATCH 8/8] fix: Switch lazy object to enabled by default on PHP 8.4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- config/config.sample.php | 4 ++-- lib/base.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index da87edff575..1578398c72e 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -2748,7 +2748,7 @@ $CONFIG = [ * Enable lazy objects feature from PHP 8.4 to be used in the Dependency Injection. * Should improve performances by avoiding buiding unused objects. * - * Defaults to false. + * Defaults to true. */ -'enable_lazy_objects' => false, +'enable_lazy_objects' => true, ]; diff --git a/lib/base.php b/lib/base.php index 9fa6707654a..55856889489 100644 --- a/lib/base.php +++ b/lib/base.php @@ -619,7 +619,7 @@ class OC { $loaderEnd = microtime(true); // Enable lazy loading if activated - \OC\AppFramework\Utility\SimpleContainer::$useLazyObjects = (bool)self::$config->getValue('enable_lazy_objects'); + \OC\AppFramework\Utility\SimpleContainer::$useLazyObjects = (bool)self::$config->getValue('enable_lazy_objects', true); // setup the basic server self::$server = new \OC\Server(\OC::$WEBROOT, self::$config);