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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Simplify wiring of service locators #29203

Open
zmitic opened this Issue Nov 13, 2018 · 11 comments

Comments

Projects
None yet
6 participants
@zmitic

zmitic commented Nov 13, 2018

@nicolas-grekas on Twitter asked me to create RFC. And when Symfony core member makes a call, you answer that call 馃槃

Is it possible to have autowiring for Service Locator? Example:

interface CustomizedServiceNameInterface
{
    public static function getName(): string;
}

interface TagCollectionInterface
{
    public static function getTagName(): string;
}

Because components need custom name instead of FQCN, programmer has to implement interface (otherwise, it works as usual FQCN as name):

class HeaderComponent implements CustomizedServiceNameInterface, ComponentInterface
{
    public static function getName(): string
    {
        return 'header';
    }
}

so when I make class that implements TagCollectionInterface:

class MyCollectionOfTaggedServices extends ServiceLocator implements TagCollectionInterface
{
    public static function getTagName(): string
    {
        return ComponentInterface::class;
    }
}

$factories is populated as

array [
  HeaderComponent::getName() => new Reference(HeaderComponent::class),
 // just an example, I know these are callables
];

instead of

array [
  HeaderComponent::class => new Reference(HeaderComponent::class),
];

Right now I have to write a compiler pass:

image

It is pretty messy code, but the thing is that I make array of components where key is read from ComponentInterface::getName() static method. The rest (like $map variable) is irrelevant to Symfony.

Anyway, tell me what you think. This idea will be scraped, I have better idea now (using annotations on controller + kernel.view event), define route tree just like NG and it will be good to go. It will not change the speed but will clean the code.

@Tobion

This comment has been minimized.

Member

Tobion commented Nov 17, 2018

Sorry but your description is not understandable to me. What kind of feedback are you asking for in this RFC?

@zmitic

This comment has been minimized.

zmitic commented Nov 17, 2018

@Tobion You are right, I just re-read it and it does sound confusing. I wanted to explain how this site works and why it is so fast; I did it because @nicolas-grekas was interested in that and I don't have a blog.

So I am closing it and when I make this as a bundle, I might create new RFC with link only.

However I think the idea of autowired service locators (for tagged services) is still something very usable and would like to see in the core.

@zmitic zmitic closed this Nov 17, 2018

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Nov 17, 2018

You're creating service locators using a convention to generate its keys. It's pretty easy right now to get collections of services without writting any DI pass (eg with !tagged), but it doesn't work for locators. The deep reason for this current impossibility is that we don't know which keys we should give to the services in the collection (for !tagged, we don't need special keys - an increasing number is enough.)

So, here, you're seeking for a way to express how the keys should be generated. A custom tag + DI pass does the job, but you're wondering if some more generic mechanism could allow reducing the need for custom code.

At least that's what I understood so far :)

@zmitic

This comment has been minimized.

zmitic commented Nov 17, 2018

@nicolas Exactly, thanks, I didn't know how to ask correctly. If I used tagged services, all would be instantiated and I would loose speed. In real site, having 50+ components is likely expected.

Even if $factories can be autowired as $id => new Reference($id) , it would be enough. The best solution is probably using @param like this issue but I see it is closed.

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Nov 17, 2018

I removed the first part in your description because I think it's not needed to explain your point. Reopening.

@yceruto

This comment has been minimized.

Contributor

yceruto commented Nov 17, 2018

I recently asked @ro0NL about this possibility that !tagged returned the collection indexed by the id of the service and make ServiceLocatorTagPass to support TaggedIteratorArgument.

Basically:

[...] wondering if some more generic mechanism could allow reducing the need for custom code. [...]

I would like something like this:

services:
    task_worker_locator:
        class: Symfony\Component\DependencyInjection\ServiceLocator
        arguments: ['!tagged task_worker']
        tags: ['container.service_locator']

instead of actual:

class WorkerCompilerPass implements CompilerPassInterface
{
    use PriorityTaggedServiceTrait;

    public function process(ContainerBuilder $container)
    {
        $definition = $container->findDefinition('task_worker_locator');

        $services = [];
        foreach ($this->findAndSortTaggedServices('task_worker', $container) as $reference) {
            $services[(string) $reference] = $reference;
        }

        $definition->replaceArgument(0, $services);
    }
}

later we can do this $this->workerLocator->get(TaggedTaskWorkerSrv::class)

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Nov 17, 2018

Also: foo_locator: !service_locator !tagged foo_tag. BUT I'm wondering if indexing by service id is legitimate: it looks like another form of "by convention" binding - ie something that works just "by chance" because the configuration happens to be OK. How can a consumer then know about the keys that exist in such a locator? FQCN is the only sensible way right now. But service ids don't have to be FQCN, so that !tagged might gather non-FQCN-identified services.
We should figure out this before moving anything forward IMHO.

@ciaranmcnulty

This comment has been minimized.

Contributor

ciaranmcnulty commented Nov 29, 2018

This would be useful for me too, I'm doing something like:

  my.user_repositories:
    class: Symfony\Component\DependencyInjection\ServiceLocator
    arguments:
    -
      memory: '@My\InMemory\UserRepository'
      mysql: '@My\Doctrine\UserRepository'
    tags: ['container.service_locator']

(A factory uses this to select service at runtime using an env DSN)

So in my case I want to get a specific key for each service that isn't just the ID. Maybe there's a convention we can use like:

  My\InMemory\UserRepository:
    tags:
      - { name: 'my.user.repository', service-key: 'memory'}

  my.user_repositories:
    class: Symfony\Component\DependencyInjection\ServiceLocator
    arguments: ['!tagged-keys my.user.repository']
    tags: ['container.service_locator']

?

Then we could do something app-specific with Autoconfiguration to generate those keys

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Nov 29, 2018

@ciaranmcnulty that's almost what we're working on with @deguif, to be submitted at SFCon's hack day :)

@ciaranmcnulty

This comment has been minimized.

Contributor

ciaranmcnulty commented Nov 29, 2018

@nicolas-grekas Damn I don't think I'm at the hack day but let's talk about it :-)

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Dec 14, 2018

See #29598

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment