Skip to content

[DependencyInjection] Fix fetching lazy non-shared services multiple times with as files true #50996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 19, 2023
Merged

[DependencyInjection] Fix fetching lazy non-shared services multiple times with as files true #50996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,12 +928,10 @@ protected static function {$methodName}(\$container$lazyInitialization)
if (!$definition->isShared()) {
$code .= sprintf(' %s ??= ', $factory);

if ($asFile) {
$code .= "self::do(...);\n\n";
} elseif ($definition->isPublic()) {
$code .= sprintf("fn () => self::%s(\$container);\n\n", $methodName);
if ($definition->isPublic()) {
$code .= sprintf("fn () => self::%s(\$container);\n\n", $asFile ? 'do' : $methodName);
} else {
$code .= sprintf("self::%s(...);\n\n", $methodName);
$code .= sprintf("self::%s(...);\n\n", $asFile ? 'do' : $methodName);
}
}
$lazyLoad = $asGhostObject ? '$proxy' : 'false';
Expand Down
62 changes: 43 additions & 19 deletions src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,24 +339,6 @@ public function testDumpAsFilesWithLazyFactoriesInlined()
$this->assertStringMatchesFormatFile(self::$fixturesPath.'/php/services9_lazy_inlined_factories.txt', $dump);
}

public function testNonSharedLazyDumpAsFiles()
{
$container = include self::$fixturesPath.'/containers/container_non_shared_lazy.php';
$container->register('non_shared_foo', \Bar\FooLazyClass::class)
->setFile(realpath(self::$fixturesPath.'/includes/foo_lazy.php'))
->setShared(false)
->setPublic(true)
->setLazy(true);
$container->compile();
$dumper = new PhpDumper($container);
$dump = print_r($dumper->dump(['as_files' => true, 'file' => __DIR__, 'inline_factories' => false, 'inline_class_loader' => false]), true);

if ('\\' === \DIRECTORY_SEPARATOR) {
$dump = str_replace("'.\\DIRECTORY_SEPARATOR.'", '/', $dump);
}
$this->assertStringMatchesFormatFile(self::$fixturesPath.'/php/services_non_shared_lazy_as_files.txt', $dump);
}

public function testServicesWithAnonymousFactories()
{
$container = include self::$fixturesPath.'/containers/container19.php';
Expand Down Expand Up @@ -772,7 +754,7 @@ public function testNonSharedLazy()
$container = new ContainerBuilder();

$container
->register('foo', Foo::class)
->register('foo', \Bar\FooLazyClass::class)
Comment on lines -775 to +757
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor mistake I made in my original PR, doesn't really change anything, but it makes the test a bit more consistent.

->setFile(realpath(self::$fixturesPath.'/includes/foo_lazy.php'))
->setShared(false)
->setLazy(true)
Expand Down Expand Up @@ -803,6 +785,48 @@ public function testNonSharedLazy()
$this->assertNotSame($foo1, $foo2);
}

public function testNonSharedLazyAsFiles()
{
$container = new ContainerBuilder();

$container
->register('non_shared_foo', \Bar\FooLazyClass::class)
->setFile(realpath(self::$fixturesPath.'/includes/foo_lazy.php'))
->setShared(false)
->setLazy(true)
->setPublic(true);

$container->compile();
$dumper = new PhpDumper($container);
$dumps = $dumper->dump([
'class' => 'Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File',
'as_files' => true,
'inline_factories' => false,
'inline_class_loader' => false,
]);

$stringDump = print_r($dumps, true);
$this->assertStringMatchesFormatFile(
self::$fixturesPath.'/php/services_non_shared_lazy_as_files.txt',
'\\' === \DIRECTORY_SEPARATOR ? str_replace("'.\\DIRECTORY_SEPARATOR.'", '/', $stringDump) : $stringDump
);

$lastDump = array_pop($dumps);
foreach (array_reverse($dumps) as $dump) {
eval('?>'.$dump);
}
Comment on lines +814 to +817
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to do this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, that's fine


$container = eval('?>'.$lastDump);

$foo1 = $container->get('non_shared_foo');
$this->assertTrue($foo1->resetLazyObject());

$foo2 = $container->get('non_shared_foo');
$this->assertTrue($foo2->resetLazyObject());

$this->assertNotSame($foo1, $foo2);
}

/**
* @testWith [false]
* [true]
Expand Down
5 changes: 0 additions & 5 deletions ...ony/Component/DependencyInjection/Tests/Fixtures/containers/container_non_shared_lazy.php
View file
Open in desktop

This file was deleted.

42 changes: 20 additions & 22 deletions ...ny/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy_as_files.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException;
/**
* @internal This class has been auto-generated by the Symfony Dependency Injection Component.
*/
class getNonSharedFooService extends ProjectServiceContainer
class getNonSharedFooService extends Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File
{
/**
* Gets the public 'non_shared_foo' service.
Expand All @@ -19,16 +19,16 @@ class getNonSharedFooService extends ProjectServiceContainer
*/
public static function do($container, $lazyLoad = true)
{
$container->factories['non_shared_foo'] ??= self::do(...);
$container->factories['non_shared_foo'] ??= fn () => self::do($container);

if (true === $lazyLoad) {
return $container->createProxy('FooLazyClassGhost0fc418d', static fn () => \FooLazyClassGhost0fc418d::createLazyGhost(static fn ($proxy) => self::do($container, $proxy)));
return $container->createProxy('FooLazyClassGhost%s', static fn () => \FooLazyClassGhost%s::createLazyGhost(static fn ($proxy) => self::do($container, $proxy)));
}

static $include = true;

if ($include) {
include_once $container->targetDir.''.'/Fixtures/includes/foo_lazy.php';
include_once '%sfoo_lazy.php';

$include = false;
}
Expand All @@ -37,11 +37,11 @@ class getNonSharedFooService extends ProjectServiceContainer
}
}

[Container%s/FooLazyClassGhost0fc418d.php] => <?php
[Container%s/FooLazyClassGhost%s.php] => <?php

namespace Container%s;

class FooLazyClassGhost0fc418d extends \Bar\FooLazyClass implements \Symfony\Component\VarExporter\LazyObjectInterface
class FooLazyClassGhost%s extends \Bar\FooLazyClass implements \Symfony\Component\VarExporter\LazyObjectInterface
{
use \Symfony\Component\VarExporter\LazyGhostTrait;

Expand All @@ -53,11 +53,11 @@ class_exists(\Symfony\Component\VarExporter\Internal\Hydrator::class);
class_exists(\Symfony\Component\VarExporter\Internal\LazyObjectRegistry::class);
class_exists(\Symfony\Component\VarExporter\Internal\LazyObjectState::class);

if (!\class_exists('FooLazyClassGhost0fc418d', false)) {
\class_alias(__NAMESPACE__.'\\FooLazyClassGhost0fc418d', 'FooLazyClassGhost0fc418d', false);
if (!\class_exists('FooLazyClassGhost%s', false)) {
\class_alias(__NAMESPACE__.'\\FooLazyClassGhost%s', 'FooLazyClassGhost%s', false);
}

[Container%s/ProjectServiceContainer.php] => <?php
[Container%s/Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File.php] => <?php

namespace Container%s;

Expand All @@ -73,14 +73,12 @@ use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
/**
* @internal This class has been auto-generated by the Symfony Dependency Injection Component.
*/
class ProjectServiceContainer extends Container
class Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File extends Container
{
protected $targetDir;
protected $parameters = [];

public function __construct(private array $buildParameters = [], protected string $containerDir = __DIR__)
{
$this->targetDir = \dirname($containerDir);
$this->services = $this->privates = [];
$this->fileMap = [
'non_shared_foo' => 'getNonSharedFooService',
Expand Down Expand Up @@ -124,7 +122,7 @@ class ProjectServiceContainer extends Container
}
}

[ProjectServiceContainer.preload.php] => <?php
[Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File.preload.php] => <?php

// This file has been auto-generated by the Symfony Dependency Injection Component
// You can reference it in the "opcache.preload" php.ini setting on PHP >= 7.4 when preloading is desired
Expand All @@ -135,9 +133,9 @@ if (in_array(PHP_SAPI, ['cli', 'phpdbg'], true)) {
return;
}

require dirname(__DIR__, %d).'%svendor/autoload.php';
(require __DIR__.'/ProjectServiceContainer.php')->set(\Container%s\ProjectServiceContainer::class, null);
require __DIR__.'/Container%s/FooLazyClassGhost0fc418d.php';
require '%svendor/autoload.php';
(require __DIR__.'/Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File.php')->set(\Container%s\Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File::class, null);
require __DIR__.'/Container%s/FooLazyClassGhost%s.php';
require __DIR__.'/Container%s/getNonSharedFooService.php';

$classes = [];
Expand All @@ -146,23 +144,23 @@ $classes[] = 'Symfony\Component\DependencyInjection\ContainerInterface';

$preloaded = Preloader::preload($classes);

[ProjectServiceContainer.php] => <?php
[Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File.php] => <?php

// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.

if (\class_exists(\Container%s\ProjectServiceContainer::class, false)) {
if (\class_exists(\Container%s\Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File::class, false)) {
// no-op
} elseif (!include __DIR__.'/Container%s/ProjectServiceContainer.php') {
} elseif (!include __DIR__.'/Container%s/Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File.php') {
touch(__DIR__.'/Container%s.legacy');

return;
}

if (!\class_exists(ProjectServiceContainer::class, false)) {
\class_alias(\Container%s\ProjectServiceContainer::class, ProjectServiceContainer::class, false);
if (!\class_exists(Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File::class, false)) {
\class_alias(\Container%s\Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File::class, Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File::class, false);
}

return new \Container%s\ProjectServiceContainer([
return new \Container%s\Symfony_DI_PhpDumper_Service_Non_Shared_Lazy_As_File([
'container.build_hash' => '%s',
'container.build_id' => '%s',
'container.build_time' => %d,
Expand Down
6 changes: 3 additions & 3 deletions ...fony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy_public.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ protected function createProxy($class, \Closure $factory)
/**
* Gets the public 'foo' service.
*
* @return \Symfony\Component\DependencyInjection\Tests\Compiler\Foo
* @return \Bar\FooLazyClass
*/
protected static function getFooService($container, $lazyLoad = true)
{
$container->factories['foo'] ??= fn () => self::getFooService($container);

if (true === $lazyLoad) {
return $container->createProxy('FooGhost80f7cfc', static fn () => \FooGhost80f7cfc::createLazyGhost(static fn ($proxy) => self::getFooService($container, $proxy)));
return $container->createProxy('FooLazyClassGhost2108fce', static fn () => \FooLazyClassGhost2108fce::createLazyGhost(static fn ($proxy) => self::getFooService($container, $proxy)));
}

static $include = true;
Expand All @@ -66,7 +66,7 @@ protected static function getFooService($container, $lazyLoad = true)
}
}

class FooGhost80f7cfc extends \Symfony\Component\DependencyInjection\Tests\Compiler\Foo implements \Symfony\Component\VarExporter\LazyObjectInterface
class FooLazyClassGhost2108fce extends \Bar\FooLazyClass implements \Symfony\Component\VarExporter\LazyObjectInterface
{
use \Symfony\Component\VarExporter\LazyGhostTrait;

Expand Down