8000 [DependencyInjection] BC break after PR 13519 · Issue #14171 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DependencyInjection] BC break after PR 13519 #14171

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

Jump to bottom
Closed
boekkooi opened this issue Apr 2, 2015 · 3 comments
Closed

[DependencyInjection] BC break after PR 13519 #14171

boekkooi opened this issue Apr 2, 2015 · 3 comments

Comments

@boekkooi
Copy link
Contributor
boekkooi commented Apr 2, 2015

So when updating from SF 2.6.1 to 2.6.6 i found my self stuck with a error on cache:clear.
After some research I found out that in one of our internal components we where doing the following:

$container
    # ...
     ->setFactoryService(new Reference($entityManagerService));

This used to work but due to a change made in #13519 if stopped working.
The code dumped after #13519 was merged looks something like (see the double $this->get):

return $this->services['service_name'] = new \SomeLoader($this->get($this->get('object_manager'))->getRepository('EntityClass'));

I'm not sure if this a BC break everyone is ok with but it confused the ..... out of me.
A simple fix would be to modify the code in src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php around line 1334.

-$service = $this->dumpValue($value->getFactoryService());
-
-return sprintf('%s->%s(%s)', 0 === strpos($service, '$') ? sprintf('$this->get(%s)', $service) : $this->getServiceCall($value->getFactoryService()), $value->getFactoryMethod(), implode(', ', $arguments));
+$service = null;
+if (!$value->getFactoryService() instanceof Reference) {
+    $service = $this->dumpValue($value->getFactoryService());
+    $service = (0 === strpos($service, '$') ? sprintf('$this->get(%s)', $service) : null);
+}
+$service = $service ?: $this->getServiceCall($value->getFactoryService());
+
+return sprintf('%s->%s(%s)', $service, $value->getFactoryMethod(), implode(', ', $arguments));

Is there any interest in a PR for this?
@stof
Copy link
Member
stof commented Apr 2, 2015

This is an invalid usage of Symfony. The setFactoryService method is documented as accepting a string argument only.
We never guarantee BC for argument types which are different from the supported ones

@boekkooi
Copy link
Contributor Author

@stof I'm not disagreeing there but still it worked for years and now it broke and the error message is not a nice understandable one.

aka the error i got was:

app/console ca:cl      
Clearing the cache for the dev environment with debug true
#0  Symfony\Component\DependencyInjection\Container->get() called at [/vagrant/app/cache/de_/ap_DevDebugProjectContainer.php:1637]
#1  ap_DevDebugProjectContainer->getBoekkooi_TwigJack_Loaders_EmailService() called at [/vagrant/app/bootstrap.php.cache:2104]
#2  Symfony\Component\DependencyInjection\Container->get() called at [/vagrant/app/cache/de_/ap_DevDebugProjectContainer.php:6725]
#3  ap_DevDebugProjectContainer->getTwig_LoaderService() called at [/vagrant/app/bootstrap.php.cache:2104]
#4  Symfony\Component\DependencyInjection\Container->get() called at [/vagrant/app/cache/de_/ap_DevDebugProjectContainer.php:6595]
#5  ap_DevDebugProjectContainer->getTwigService() called at [/vagrant/app/bootstrap.php.cache:2104]
#6  Symfony\Component\DependencyInjection\Container->get() called at [/vagrant/app/cache/de_/ap_DevDebugProjectContainer.php:1286]
#7  ap_DevDebugProjectContainer->getAssetic_AssetManagerService() called at [/vagrant/app/bootstrap.php.cache:2104]
#8  Symfony\Component\DependencyInjection\Container->get() called at [/vagrant/vendor/symfony/assetic-bundle/CacheWarmer/AssetManagerCacheWarmer.php:33]
#9  Symfony\Bundle\AsseticBundle\CacheWarmer\AssetManagerCacheWarmer->warmUp() called at [/vagrant/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/CacheWarmer/CacheWarmerAggregate.php:48]
#10 Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerAggregate->warmUp() called at [/vagrant/vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Command/CacheClearCommand.php:138]
#11 Symfony\Bundle\FrameworkBundle\Command\CacheClearCommand->warmup() called at [/vagrant/vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Command/CacheClearCommand.php:90]
#12 Symfony\Bundle\FrameworkBundle\Command\CacheClearCommand->execute() called at [/vagrant/vendor/symfony/symfony/src/Symfony/Component/Console/Command/Command.php:257]
#13 Symfony\Component\Console\Command\Command->run() called at [/vagrant/vendor/symfony/symfony/src/Symfony/Component/Console/Application.php:882]
#14 Symfony\Component\Console\Application->doRunCommand() called at [/vagrant/vendor/symfony/symfony/src/Symfony/Component/Console/Application.php:195]
#15 Symfony\Component\Console\Application->doRun() called at [/vagrant/vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:96]
#16 Symfony\Bundle\FrameworkBundle\Console\Application->doRun() called at [/vagrant/vendor/symfony/symfony/src/Symfony/Component/Console/Application.php:126]
#17 Symfony\Component\Console\Application->run() called at [/vagrant/app/console:24]

@Tobion
Copy link
Contributor
Tobion commented Apr 2, 2015

Closing as the behavior for wrong parameters is undefined of course.

@Tobion Tobion closed this as completed Apr 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@boekkooi @stof @Tobion
0