Skip to content

Private services stay public when required by more than one service #12588

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

Closed
bestform opened this issue Nov 27, 2014 · 6 comments
Closed

Private services stay public when required by more than one service #12588

bestform opened this issue Nov 27, 2014 · 6 comments

Comments

@bestform
Copy link
Contributor

The documentation says, that a service, that you declare private, will not be available via the get method of the compiled container (see: http://symfony.com/doc/current/components/dependency_injection/advanced.html)

This is not true. A private service will only be removed by the RemoveUnusedDefinitionsPass compiler pass, if it is only used by one other service. In this case, the container doesn't need to keep track of the instance, as it can just instantiate one instance of the private service in the getter of the service in need and forget about it.

But should the service be used by more than one service, the container keeps track of the lazily loaded instance by not removing the service.

This in itself isn't bad, but the problem is, that at this point, the container also provides the private service publicly to its users.

I've written two new test cases, which reproduce the problem. The first test case (testPrivateServiceIsNotAccessibleWhenRequiredByOneOtherService) is successful, the second one (testPrivateServiceIsNotAccessibleWhenRequiredByMoreThanOneOtherService) fails, as the private service hasn't been removed.

You can see the test cases here: https://github.com/bestform/symfony/compare/test-for-private-services

I would suggest that the container keeps track of the instances internally while forbidding the access from the outside to be consistent with the documentation and in general to be more intuitive. Private services should be private, no matter how the compilation pass of the container is able to optimize their usage.

@boekkooi
Copy link
Contributor

It looks like this problem is also occurs for set, has, initialized,getServiceIds.

@jakzal
Copy link
Contributor

jakzal commented Nov 28, 2014

Marking a service as private is simply giving a hint to the container builder that the service is not expected to be accessed directly, and therefore can be potentially inlined. See previous discussion on this in #2586.

I think the problem is in documentation, which should emphasise more the fact that a private service is a hint for the container. It says "Now that the service is private, you cannot call: $container->get('foo')", but few paragraphs before there's a note mentioning that a service is only inlined if it's referenced by a single service.

@jakzal
Copy link
Contributor

jakzal commented Nov 28, 2014

I created a doc issue to make this chapter clearer symfony/symfony-docs#4524.

@jakzal jakzal closed this as completed Nov 28, 2014
@timglabisch
Copy link

this is some kind of weird. the audience of the documentations are the developers, not the internals of the di container. so the question is why would someone like to give the container such a hint?

  ...giving a hint to the container builder that the service is not expected to be
  accessed directly, and therefore can be potentially inlined

potentially is bad and if this doesn't belongs to the developer, i ask myself - why to use and teach about this feature. from my point of view the container is lacking a way of just exposing bunch of services. right now most applications are exposing a huge mess of useless internal services.

@bestform
Copy link
Contributor Author

I second the post of @timglabisch. I guess it isn't far fetched to think, that developers get the impression, the private attribute would help to clean up the 'huge mess of useless internal services'.

But there is a bigger problem: You expose services, that might vanish some day without the developer using it (eg. by getting it inside a container aware controller) can do anything about it.

E.g.: The developer sees a private service provided by the container in his/her controller and uses it. Later the bundle, that provided the service, drops one usage leaving just one usage in the container. Then the service magically disappears from the container.

The developer of the bundle won't see a BC break, as he just changed the internal configuration and usage of a service he explicitly declared private. And the user won't recognize the break either until his/her application explodes at runtime when calling the action of the controller using the service.

I see your point, that this is a documentation issue at the moment, but in my opinion, we should think about whether it wouldn't be better to hide private services from the public interface of the container in general to avoid the described problem.

@bestform
Copy link
Contributor Author

bestform commented Sep 2, 2016

For anyone who might stumble upon this issue: It has been addressed in this PR: #19146
A good description of the solution can be found here: http://symfony.com/blog/new-in-symfony-3-2-improved-private-services

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

4 participants