From 064e4e77eaec2502c14db177bb6a2d6f58d46aab Mon Sep 17 00:00:00 2001 From: Gabriel Felipe Soares Date: Tue, 28 Sep 2021 09:47:26 +0200 Subject: [PATCH 1/2] fix: fix multiple service providers support --- core/DependencyInjection/ContainerBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/DependencyInjection/ContainerBuilder.php b/core/DependencyInjection/ContainerBuilder.php index 8624e61de..2d75e00c0 100644 --- a/core/DependencyInjection/ContainerBuilder.php +++ b/core/DependencyInjection/ContainerBuilder.php @@ -127,7 +127,7 @@ private function getTemporaryServiceFileContent(): string return function (ContainerConfigurator $configurator): void { - %s + ' . str_repeat('%s' . PHP_EOL, count($contents)) . ' };', $contents ); From d080e0b2ad88b96d1a00bfcfb2faf5ccff2da2e8 Mon Sep 17 00:00:00 2001 From: Gabriel Felipe Soares Date: Tue, 28 Sep 2021 16:39:12 +0200 Subject: [PATCH 2/2] fix: DI Container legacy classes loading --- core/DependencyInjection/ContainerBuilder.php | 71 ++++++-- core/DependencyInjection/ContainerCache.php | 2 +- core/DependencyInjection/ContainerStarter.php | 12 +- core/DependencyInjection/LegacyFileLoader.php | 171 ------------------ core/DependencyInjection/README.md | 19 +- .../ContainerBuilderTest.php | 1 - .../ContainerCacheTest.php | 11 -- .../LegacyFileLoaderTest.php | 91 ---------- 8 files changed, 68 insertions(+), 310 deletions(-) delete mode 100644 core/DependencyInjection/LegacyFileLoader.php delete mode 100755 test/unit/core/DependencyInjection/LegacyFileLoaderTest.php diff --git a/core/DependencyInjection/ContainerBuilder.php b/core/DependencyInjection/ContainerBuilder.php index 2d75e00c0..16c2d7e91 100644 --- a/core/DependencyInjection/ContainerBuilder.php +++ b/core/DependencyInjection/ContainerBuilder.php @@ -28,7 +28,12 @@ use Psr\Container\ContainerInterface; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder as SymfonyContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException as SymfonyServiceNotFoundException; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; +use oat\oatbox\service\ServiceNotFoundException; +use Symfony\Component\DependencyInjection\Reference; class ContainerBuilder extends SymfonyContainerBuilder { @@ -38,20 +43,15 @@ class ContainerBuilder extends SymfonyContainerBuilder /** @var bool|null */ private $cachePath; - /** @var string|null */ - private $configPath; - /** @var ContainerInterface */ private $legacyContainer; public function __construct( - string $configPath, string $cachePath, ContainerInterface $legacyContainer, bool $isDebugEnabled = null, ContainerCache $cache = null ) { - $this->configPath = $configPath; $this->cachePath = $cachePath; $this->legacyContainer = $legacyContainer; $this->cache = $cache ?? new ContainerCache( @@ -97,19 +97,60 @@ public function forceBuild(): ContainerInterface ); $phpLoader->load('services.php'); - $legacyLoader = new LegacyFileLoader( - $this, - new FileLocator( - [ - $this->configPath - ] - ) - ); - $legacyLoader->load('*/*.conf.php'); - return $this->cache->forceLoad(); } + /** + * @inheritDoc + */ + public function get(string $id, int $invalidBehavior = SymfonyContainerInterface::EXCEPTION_ON_INVALID_REFERENCE) + { + try { + return parent::get($id, $invalidBehavior); + } catch (SymfonyServiceNotFoundException $exception) { + } + + try { + return $this->legacyContainer->get($id); + } catch (ServiceNotFoundException $exception) { + throw new SymfonyServiceNotFoundException($id); + } + } + + /** + * @inheritDoc + */ + public function has(string $id) + { + if (parent::has($id)) { + return true; + } + + try { + $this->legacyContainer->get($id); + + return true; + } catch (ServiceNotFoundException $exception) { + return false; + } + } + + /** + * @inheritDoc + */ + public function findDefinition(string $id) + { + try { + return parent::findDefinition($id); + } catch (SymfonyServiceNotFoundException $exception) { + return (new Definition($id)) + ->setAutowired(true) + ->setPublic(true) + ->setFactory(new Reference(LegacyServiceGateway::class)) + ->setArguments([$id]); + } + } + private function getTemporaryServiceFileContent(): string { $contents = []; diff --git a/core/DependencyInjection/ContainerCache.php b/core/DependencyInjection/ContainerCache.php index fa27e5d0c..0a851bd9a 100644 --- a/core/DependencyInjection/ContainerCache.php +++ b/core/DependencyInjection/ContainerCache.php @@ -65,7 +65,7 @@ public function __construct( public function isFresh(): bool { - return !$this->isEnvVarTrue('DI_CONTAINER_FORCE_BUILD') && $this->configCache->isFresh(); + return $this->configCache->isFresh(); } public function load(): ContainerInterface diff --git a/core/DependencyInjection/ContainerStarter.php b/core/DependencyInjection/ContainerStarter.php index fb3cc444c..a7c15ff01 100644 --- a/core/DependencyInjection/ContainerStarter.php +++ b/core/DependencyInjection/ContainerStarter.php @@ -36,31 +36,22 @@ final class ContainerStarter /** @var ContainerInterface */ private $legacyContainer; - /** @var string|null */ - private $configPath; - /** @var string|null */ private $cachePath; public function __construct( ContainerInterface $legacyContainer, - string $configPath = null, string $cachePath = null ) { - if (!$configPath) { - $configPath = defined('CONFIG_PATH') ? CONFIG_PATH : null; - } - if (!$cachePath) { $cachePath = defined('GENERIS_CACHE_PATH') ? GENERIS_CACHE_PATH : null; } - if (!$configPath || !$cachePath) { + if (!$cachePath) { throw new LogicException('Required application constants were not initialized!'); } $this->legacyContainer = $legacyContainer; - $this->configPath = $configPath; $this->cachePath = $cachePath; } @@ -77,7 +68,6 @@ public function getContainerBuilder(): ContainerBuilder { if (!$this->containerBuilder) { $this->containerBuilder = new ContainerBuilder( - $this->configPath, $this->cachePath, $this->legacyContainer ); diff --git a/core/DependencyInjection/LegacyFileLoader.php b/core/DependencyInjection/LegacyFileLoader.php deleted file mode 100644 index 6d63fd6d9..000000000 --- a/core/DependencyInjection/LegacyFileLoader.php +++ /dev/null @@ -1,171 +0,0 @@ -createLoadClosure(); - - /** - * @var string $path - * @var SplFileInfo $info - */ - foreach ($this->glob($resource, false, $globResource) as $path => $info) { - try { - $class = $loadClassClosure($path); - - if ($class instanceof ServiceFactoryInterface) { - $this->registerInjectableClassDefinition($info); - } - - if ($this->isLegacyClass($class)) { - $this->registerClassDefinition($info, $class); - } - } finally { - $this->instanceof = []; - $this->registerAliasesForSinglyImplementedInterfaces(); - } - } - } - - private function registerInjectableClassDefinition(SplFileInfo $info): void - { - $alias = $this->createAlias($info); - - $definition = (new Definition(ServiceFactoryInterface::class)) - ->setAutowired(true) - ->setPublic(true) - ->setFactory(new Reference(LegacyServiceGateway::class)) - ->setArguments([$alias]); - - $this->container->setDefinition($alias, $definition); - } - - /** - * @param object $class - */ - private function registerClassDefinition(SplFileInfo $info, $class): void - { - $className = get_class($class); - $serviceName = $this->getServiceName($className); - $alias = $this->createAlias($info); - - $definition = new Definition($serviceName); - $definition->setAutowired(true) - ->setPublic(true) - ->setFactory(new Reference(LegacyServiceGateway::class)) - ->setArguments([$alias]); - - $this->container->setDefinition($serviceName, $definition); - - $this->container->setAlias($alias, $serviceName) - ->setPublic(true); - - if ($className !== $serviceName) { - $this->container->setAlias($className, $serviceName) - ->setPublic(true); - } - } - - private function createLoadClosure(): Closure - { - return Closure::bind( - function ($path) { - return include $path; - }, - $this, - ProtectedPhpFileLoader::class - ); - } - - private function getServiceName(string $className): string - { - $interfacesWithServiceId = array_filter( - (new ReflectionClass($className))->getInterfaces(), - function (ReflectionClass $interface) { - return array_key_exists('SERVICE_ID', $interface->getConstants()); - } - ); - - if ($interfacesWithServiceId) { - $interface = array_pop($interfacesWithServiceId); - - return $interface->getName(); - } - - return $className; - } - - private function createAlias(SplFileInfo $info): string - { - $pathInfo = explode('/', pathinfo($info->getRealPath(), PATHINFO_DIRNAME)); - $prefix = end($pathInfo); - - return $prefix . '/' . $info->getBasename('.conf.php'); - } - - /** - * @param object $subjectClass - */ - private function isLegacyClass($subjectClass): bool - { - $legacy = array_filter( - self::LEGACY_CONFIGURABLE_CLASS_LIST, - function ($class) use ($subjectClass) { - return is_a($subjectClass, $class); - } - ); - - return !empty($legacy); - } -} diff --git a/core/DependencyInjection/README.md b/core/DependencyInjection/README.md index 1ae405860..7213ba3b2 100644 --- a/core/DependencyInjection/README.md +++ b/core/DependencyInjection/README.md @@ -27,14 +27,16 @@ class MyContainerServiceProvider implements ContainerServiceProviderInterface $parameters->set('someParam', 'someValue'); - $services->set(MyService::class, MyService::class)->args( - [ - service(MyOtherService::class), - service(MyLegacyService::SERVICE_ID), - env('MY_ENV_VAR'), - param('someParam'), - ] - ); + $services->set(MyService::class, MyService::class) + ->public() + ->args( + [ + service(MyOtherService::class), + service(MyLegacyService::SERVICE_ID), + env('MY_ENV_VAR'), + param('someParam'), + ] + ); } } ``` @@ -167,7 +169,6 @@ To avoid container caching (useful on dev mode), please add the following variab ```shell DI_CONTAINER_DEBUG=true -DI_CONTAINER_FORCE_BUILD=true ``` ## Testing container inside Legacy Services (ConfigurableService) diff --git a/test/unit/core/DependencyInjection/ContainerBuilderTest.php b/test/unit/core/DependencyInjection/ContainerBuilderTest.php index fddb4f055..4fc3ee3df 100755 --- a/test/unit/core/DependencyInjection/ContainerBuilderTest.php +++ b/test/unit/core/DependencyInjection/ContainerBuilderTest.php @@ -57,7 +57,6 @@ public function setUp(): void $this->tempDir = sys_get_temp_dir(); $this->cache = $this->createMock(ContainerCache::class); $this->subject = new ContainerBuilder( - $this->tempDir, $this->tempDir, $legacyContainer, true, diff --git a/test/unit/core/DependencyInjection/ContainerCacheTest.php b/test/unit/core/DependencyInjection/ContainerCacheTest.php index 7362a4649..158f4d101 100755 --- a/test/unit/core/DependencyInjection/ContainerCacheTest.php +++ b/test/unit/core/DependencyInjection/ContainerCacheTest.php @@ -60,8 +60,6 @@ public function setUp(): void public function testLoadFromCache(): void { - $_ENV['DI_CONTAINER_FORCE_BUILD'] = false; - $this->configCache->expects($this->once()) ->method('isFresh') ->willReturn(true); @@ -107,19 +105,10 @@ public function testForceLoad(): void public function testIsFresh(): void { - $_ENV['DI_CONTAINER_FORCE_BUILD'] = false; - $this->configCache->expects($this->once()) ->method('isFresh') ->willReturn(true); $this->assertTrue($this->subject->isFresh()); } - - public function testIsNotFreshForced(): void - { - $_ENV['DI_CONTAINER_FORCE_BUILD'] = true; - - $this->assertFalse($this->subject->isFresh()); - } } diff --git a/test/unit/core/DependencyInjection/LegacyFileLoaderTest.php b/test/unit/core/DependencyInjection/LegacyFileLoaderTest.php deleted file mode 100755 index 319e2f218..000000000 --- a/test/unit/core/DependencyInjection/LegacyFileLoaderTest.php +++ /dev/null @@ -1,91 +0,0 @@ -containerBuilder = $this->createMock(ContainerBuilder::class); - $this->fileLocator = $this->createMock(FileLocatorInterface::class); - $this->subject = new LegacyFileLoader($this->containerBuilder, $this->fileLocator); - } - - public function testLoad(): void - { - $alias = $this->createMock(Alias::class); - - $this->containerBuilder - ->expects($this->exactly(1)) - ->method('setAlias') - ->willReturn($alias); - - $this->containerBuilder - ->expects($this->atLeast(2)) - ->method('setDefinition'); - - $injectableService = 'fileLocator - ->expects($this->exactly(1)) - ->method('locate') - ->willReturn($tmpDir); - - $this->assertNull($this->subject->load('*.conf.php')); - } - - public function testSupports(): void - { - $this->assertTrue($this->subject->supports('something.conf.php')); - $this->assertFalse($this->subject->supports('*.conf.php')); - } -}