symfony-cmf / resource-bundle

Resource Location for CMF documents based on Puli
https://cmf.symfony.com
5 stars 7 forks source link

Support for enhancers #26

Closed dantleech closed 8 years ago

dantleech commented 8 years ago

This PR integrates https://github.com/symfony-cmf/resource/pull/19 (description enhancers).

Enhancers are added via. definition factories (i.e. the same way that we now add repositories - #18)

class CmfResourceBundle extends Bundle
{
    public function build(ContainerBuilder $container)
    {
        $extension = $container->getExtension('cmf_resource');
        // ...
        $extension->addDescriptionEnhancerFactory('sonata_admin', new SonataAdminFactory());
dantleech commented 8 years ago

/cc @dbu @WouterJ

wouterj commented 8 years ago

This PR doesn't need the complex factory logic: It doesn't have configuration options, just enabled or disabled. Let's use the normal tagged service and compiler pass approach for enhancers.

dantleech commented 8 years ago

@WouterJ the problem with the tagged service approach is that we can only enable services "blindly". The bundle has no knowledge of what enhancers it has until they are explicitly enabled.

With the factory approach we can make the bundle aware of the enhacners whilst not having to load their definitions (and trigger exceptions for unmet dependencies).

wouterj commented 8 years ago

@dantleech inside a compiler pass, you can execute $container->findTaggedServiceIds('cmf_resource.enhancer'). This means we don't need to load the definitions and the bundle is aware of all available enhancers.

Even more, when the enhancers are marked private (which should be done for internal services), they will be removed from the container as long as they aren't enabled.

dantleech commented 8 years ago

Hmm. Thinking about you're right. Was thinking of a different use case, while we are explicitly enabling there is no issue with tags.

dantleech commented 8 years ago

Applied fixes to #29 which replaces this PR.