Skip to content
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.

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

Unable to reference PHP DI dependencies in Symfony 3.1 #16

Open
pevdh opened this issue Jul 11, 2016 · 6 comments
Open

Unable to reference PHP DI dependencies in Symfony 3.1 #16

pevdh opened this issue Jul 11, 2016 · 6 comments

Comments

@pevdh
Copy link

pevdh commented Jul 11, 2016

First of all, thank you for this very useful bridge!

I am trying to use the Symfony bridge with Symfony 3.1, but I ran into an issue where I seem to be unable to reference to PHP DI dependencies from the Symfony services.yml configuration file.

This is my services.yml configuration file (as suggested in the docs):

services:
    app.page_layout_extension:
        class: AppBundle\Twig\PageLayoutExtension
        arguments:
            - 'AppBundle\Layout\PageLayoutFactory'
            - 'AppBundle\Service\ContentBlockService'
        tags:
            - {name: twig.extension}

The signature of the PageLayoutExtension constructor is public function __construct(PageLayoutFactory $pageLayoutFactory, ContentBlockService $contentBlockService)

This configuration yields the following error:

PHP Fatal error:  Uncaught Symfony\Component\Debug\Exception\FatalThrowableError: Type error: Argument 1 passed to AppBundle\Twig\PageLayoutExtension::__construct() must be an instance of AppBundle\Layout\PageLayoutFactory, string given, called in /var/cache/dev/appDevDebugProjectContainer.php on line 300 in /src/AppBundle/Twig/PageLayoutExtension.php:23
Stack trace:
#0 /var/cache/dev/appDevDebugProjectContainer.php(300): AppBundle\Twig\PageLayoutExtension->__construct('AppBundle\\Layou...', 'AppBundle\\Servi...')
#1 /vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Container.php(275): appDevDebugProjectContainer->getApp_PageLayoutExtensionService()
#2 /vendor/php-di/symfony-bridge/src/SymfonyContainerBridge.php(93): Symfony\Component\DependencyInjection\Container->get('app.page_layout...', 1)
#3 /vendor/php-di/symfony-bridge/src/SymfonyContainer in /src/AppBundle/Twig/PageLayoutExtension.php on line 23

Upon inspection, this error is caused by this line in appDevDebugContainer.php:

return $this->services['app.page_layout_extension'] = new \AppBundle\Twig\PageLayoutExtension('AppBundle\\Layout\\PageLayoutFactory', 'AppBundle\\Service\\ContentBlockService');

Which makes the error obvious. This line should read:

return $this->services['app.page_layout_extension'] = new \AppBundle\Twig\PageLayoutExtension($this->get('AppBundle\\Layout\\PageLayoutFactory'), $this->get('AppBundle\\Service\\ContentBlockService'));

Looking into the issue, I came across the following suggested modification of services.yml (forgot where I read it):

services:
    app.page_layout_extension:
        class: AppBundle\Twig\PageLayoutExtension
        arguments:
            - '@AppBundle\Layout\PageLayoutFactory'
            - '@AppBundle\Service\ContentBlockService'
        tags:
            - {name: twig.extension}

However, this makes that same appDevDebugContainer.php line translate to:

return $this->services['app.page_layout_extension'] = new \AppBundle\Twig\PageLayoutExtension($this->get('appbundle\layout\pagelayoutfactory'), $this->get('appbundle\service\contentblockservice'));

(Note the lowercase service names). Which is clearly wrong as well.

Am I missing something or is this new "functionality" of Symfony's container? Are there any workarounds?

@mnapoli
Copy link
Member

mnapoli commented Jul 11, 2016

First of all yes you have to use @ in front of service names, it's the Symfony convention for saying "this is a service". Else you are simply injecting a string. It's like using DI\get() in PHP-DI configuration, if you don't use that then you will simply be injecting a string, not the service, and that's not what you want.

So the second example sounds correct.

Now yes in Symfony all service IDs are turned to lower case. I don't remember that being a problem because in PHP class names are case insensitive, and it should be covered by the tests of this package.

So in the end did the last example work? If not, what error did you get?

@mnapoli
Copy link
Member

mnapoli commented Jul 11, 2016

See here: https://github.com/PHP-DI/Symfony-Bridge/blob/master/tests/FunctionalTest/Fixtures/config/class1.yml This test uses the same notation as you.

And I just realize the documentation is wrong so that's maybe what mislead you: http://php-di.org/doc/frameworks/symfony2.html#using-symfonys-services-in-php-di

You can also inject PHP-DI services in a Symfony service:

services:
    # app.user_service is a Symfony service
    app.user_service:
        class: 'AppBundle\\Service\\UserService'
        arguments:
            # UserRepository is created by PHP-DI
            - 'AppBundle\\Service\\UserRepository'

The @ is missing. I will add it!

mnapoli added a commit to PHP-DI/PHP-DI that referenced this issue Jul 11, 2016
The `@` was missing to mark the argument as a service reference. Reported in PHP-DI/Symfony-Bridge#16
@pevdh
Copy link
Author

pevdh commented Jul 11, 2016

Hmm, your suggestion about the case sensitivity is correct, I think. However, I still get You have requested a non-existent service "appbundle\service\contentblockservice", sometimes. It seems random. Might have something to do with the cache, but I do not have the time to investigate it :/

Thanks for the quick update of the documentation!

Stack trace:

ServiceNotFoundException in SymfonyContainerBridge.php line 90:
You have requested a non-existent service "appbundle\service\contentblockservice".

in SymfonyContainerBridge.php line 90
at SymfonyContainerBridge->get('appbundle\service\contentblockservice') in appDevDebugProjectContainer.php line 300
at appDevDebugProjectContainer->getApp_CmsExtensionService() in Container.php line 275
at Container->get('app.cms_extension', '1') in SymfonyContainerBridge.php line 72
at SymfonyContainerBridge->get('app.cms_extension') in appDevDebugProjectContainer.php line 3119
at appDevDebugProjectContainer->getTwigService() in Container.php line 275
at Container->get('twig', '1') in SymfonyContainerBridge.php line 72
at SymfonyContainerBridge->get('twig') in appDevDebugProjectContainer.php line 3376
at appDevDebugProjectContainer->getWebProfiler_Controller_ProfilerService() in Container.php line 275
at Container->get('web_profiler.controller.profiler', '1') in SymfonyContainerBridge.php line 72
at SymfonyContainerBridge->get('web_profiler.controller.profiler') in classes.php line 2974
at ControllerResolver->createController('web_profiler.controller.profiler:panelAction') in classes.php line 2424
at ControllerResolver->getController(object(Request)) in TraceableControllerResolver.php line 58
at TraceableControllerResolver->getController(object(Request)) in HttpKernel.php line 136
at HttpKernel->handleRaw(object(Request), '1') in HttpKernel.php line 68
at HttpKernel->handle(object(Request), '1', true) in Kernel.php line 177
at Kernel->handle(object(Request)) in app_dev.php line 30
at require('/web/app_dev.php') in router_dev.php line 40

@mnapoli
Copy link
Member

mnapoli commented Jul 12, 2016

Mmh if it's "sometimes" then it might be a cache issue indeed, or maybe related to a specific machine? Anyway if you can isolate a specific problem feel free to reopen an issue, the best thing is to have a reproducible test case or something similar.

@mnapoli mnapoli closed this as completed Jul 12, 2016
@Fedott
Copy link

Fedott commented Sep 9, 2016

Hi!

I've faced the same problem and have figured out why it is happening.

The current test gives us false-positive result. It happens because the class names in PHP are case-insensitive, so class_exists('camelcaseclass') returns true if CamelCaseClass exists. But if we use autoloader and the appropriate class has not been loaded before calling class_exists, than class_exists('camecaseclass') will return false because of unix filesystem case-sensivity (it will look for the file called "camelcaseclass.php", which is actually missing, instead of the correct "CameCaseClass.php").

Thereby, when we use the class name in the dependencies of the service defined in the Symfony container, the dependency id is strtolower'ed. That causes the autoloader to look for the incorrect file when the php-di tries to resolve the dependency.

There are two ways of making the test work properly. The first is moving the test itself (symfonyGetInPHPDI) to the very top of the TestCase file. The second is to use different classes in the tests.

@mnapoli
Copy link
Member

mnapoli commented Sep 9, 2016

Ohh that's a nasty bug, thanks @Fedott for the details. I'm reopening this, I won't have time to look at this this week-end but feel free to send a pull request, even if it's just to add a test that reproduces the problem it will be useful.

@mnapoli mnapoli reopened this Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants