doctrine service is public
#712
Conversation
| @@ -56,14 +56,14 @@ | |||
|
|
|||
| <service id="doctrine.dbal.connection.configuration" class="%doctrine.dbal.configuration.class%" public="false" abstract="true" /> | |||
|
|
|||
| <service id="doctrine" class="%doctrine.class%"> | |||
| <service id="doctrine" class="%doctrine.class%" public="true"> | |||
There was a problem hiding this comment.
This file doesn't specify defaults, so the service will be public by default.
There was a problem hiding this comment.
Services are going to be private by default in Symfony 4.0.
There was a problem hiding this comment.
Good point, I didn't think of that.
Resources/config/dbal.xml
Outdated
| <argument type="service" id="service_container" /> | ||
| <argument>%doctrine.connections%</argument> | ||
| <argument>%doctrine.entity_managers%</argument> | ||
| <argument>%doctrine.default_connection%</argument> | ||
| <argument>%doctrine.default_entity_manager%</argument> | ||
| </service> | ||
| <service id="Doctrine\Common\Persistence\ManagerRegistry" alias="doctrine" public="false" /> | ||
| <service id="Doctrine\Common\Persistence\ManagerRegistry" alias="doctrine" public="true" /> |
There was a problem hiding this comment.
Why make this service public? As far as I've seen, the new service aliases in Symfony (e.g. TranslatorInterface, RouterInterface) are all private.
There was a problem hiding this comment.
I guess that without this the auto-wiring wouldn't work 🤔
There was a problem hiding this comment.
Autowiring doesn't rely on public services. Private services can't be fetched using Container::get, but they can be used as references within the service configuration if I'm not mistaken. Maybe @fabpot could clear this up?
|
Should be done in 1.6.x, right? |
sroze
force-pushed
the
doctrine-service-is-public
branch
from
81c68f9 to
c9adf6c
Compare
|
@fabpot good point; rebased and changed PR target 👍 |
|
a bunch of other services must stay public too, as they are the main services of the bundle:
DoctrineBundle is already quite good at marking services as private where needed, so most existing public services should stay public (the exception are the form and validator ones, which were public only by requirement of Symfony < 3.3, and so can become private when using Symfony 4.x) |
|
The |
|
well, for the abstract service, this is useless, as child services have their visibility set explicitly. The connection factory does not need to be public either (Symfony 3.4 and 4.0 support using private services in service factories, but old versions of Symfony were buggy in this case). You still need to ensure that the And tests are broken |
sroze
force-pushed
the
doctrine-service-is-public
branch
2 times, most recently
from
5bd8bd7 to
2f24d33
Compare
sroze
force-pushed
the
doctrine-service-is-public
branch
from
2f24d33 to
e0a9109
Compare
|
@stof updated, added tests and they are green (HHVM failing but not related) 👍 |
|
Should we merge this? /cc @fabpot |
As used by Symfony's
ControllerTrait, make thedoctrineservice public.