Skip to content

Services are not private if included through a Service Subscriber/Locator #219

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
Pixelshaped opened this issue Dec 13, 2021 · 2 comments · Fixed by #220
Closed

Services are not private if included through a Service Subscriber/Locator #219

Pixelshaped opened this issue Dec 13, 2021 · 2 comments · Fixed by #220

Comments

@Pixelshaped
Copy link

Pixelshaped commented Dec 13, 2021

Since #211 there is no way to get Services on the container by default. This a nice idea but it overlooks legitimate use-cases: it is indeed possible to get services on the container without getting a deprecation warning if your service implements the ServiceSubscriberInterface for example (see docs: https://symfony.com/doc/current/service_container/service_subscribers_locators.html#defining-a-service-subscriber)

It's the preferred way to make loading of services lazy: "Sometimes, a service needs access to several other services without being sure that all of them will actually be used. In those cases, you may want the instantiation of the services to be lazy". If you want an official example, go look at AbstractController

Now this raises errors:

 ------ --------------------------------------------------------------------- 
  Line   src/Twig/CategoryExtension.php                                       
 ------ --------------------------------------------------------------------- 
  33     Service "App\Service\ProductCategoryHelper" is private.              
  33     Service "Symfony\Component\HttpFoundation\RequestStack" is private.  
 ------ --------------------------------------------------------------------- 
class CategoryExtension extends AbstractExtension implements ServiceSubscriberInterface
{
    public function __construct(
        private ContainerInterface $container
    ) {}

    public function getFunctions(): array
    {
        return [
            ... whole bunch of other functions ...
            new TwigFunction('get_category_tree', [$this, 'getCategoryTree']),
        ];
    }

    /**
     * @return Collection<int, Category>
     */
    public function getCategoryTree(): Collection
    {
        return $this->container->get(ProductCategoryHelper::class)->getCategoryTree($this->container->get(RequestStack::class)->getCurrentRequest()->getLocale());
    }

    /**
     * @return array<string>
     */
    public static function getSubscribedServices(): array
    {
        return [
            ProductCategoryHelper::class,
            RequestStack::class
        ];
    }
}

But really, it shouldn't.

@jeroennoten

Current Symfony version: 5.4.1

@jeroennoten
Copy link
Contributor

Hi @Pixelshaped, I think that we should add a check for classes implementing ServiceSubscriberInterface in the ContainerInterfacePrivateServiceRule.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants