provide configuration options to disable guessers/voters/loaders

symfony-cmf · Aug 1, 2015 · 023acde · 023acde
1 parent 755d8e2
commit 023acde
Show file tree

Hide file tree

Showing 7 changed files with 183 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,13 +1,17 @@
 Changelog
 =========
 
-* **2015-07-20**: Cleaned up the sitemap generation. If you used the unreleased version of 
-  sitemaps, you will need to adjust your code. See https://github.com/symfony-cmf/SeoBundle/pull/225
-* **2015-02-24**: Configuration for content_key moved to content_listener section, and its now possible to disable
-  The content listener by setting cmf_seo.content_listener.enabled: false
+* **2015-07-20**: Cleaned up the sitemap generation. If you used the unreleased 
+  version of sitemaps, you will need to adjust your code. See https://github.com/symfony-cmf/SeoBundle/pull/225
+  Options are available to keep all or no voters|guessers|loaders enabled or 
+  enable them one by one by their service id.
+* **2015-02-24**: Configuration for `content_key` moved to the `content_listener` 
+  section, and its now possible to disable the content listener by setting 
+  `cmf_seo.content_listener.enabled: false`
 * **2015-02-14**: Added sitemap generation
-* **2015-02-14**: [BC BREAK] Changed method visibility from private to public of SeoPresentation#getSeoMetadata()
-* **2014-10-04**: Custom exception controller for error handling
+* **2015-02-14**: [BC BREAK] Changed method visibility of 
+  `SeoPresentation#getSeoMetadata()` from private to public.
+* **2014-10-04**: Custom exception controller for error handling.
 
 1.1.0-RC3
 ---------

diff --git a/DependencyInjection/CmfSeoExtension.php b/DependencyInjection/CmfSeoExtension.php
@@ -11,7 +11,6 @@
 
 namespace Symfony\Cmf\Bundle\SeoBundle\DependencyInjection;
 
-use Symfony\Cmf\Bundle\SeoBundle\CmfSeoBundle;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -37,6 +36,12 @@ class CmfSeoExtension extends Extension
      */
     private $contentListenerEnabled = false;
 
+    private $sitemapHelperMap = array(
+        'loaders' => 'cmf_seo.sitemap.loader',
+        'guessers' => 'cmf_seo.sitemap.guesser',
+        'voters' => 'cmf_seo.sitemap.voter'
+    );
+
     /**
      * {@inheritDoc}
      */
@@ -253,30 +258,52 @@ private function loadSitemapHandling($config, XmlFileLoader $loader, ContainerBu
         $loader->load('sitemap.xml');
 
         $configurations = $config['configurations'];
-        // if there are no explicit configurations, enable the default sitemap
-        if (!count($configurations)) {
-            $configurations['sitemap'] = array();
+
+        $helperStatus = array();
+        foreach ($this->sitemapHelperMap as $helper => $tag) {
+            $helperStatus[$helper] = array();
+            $serviceDefinitionIds = $container->findTaggedServiceIds($tag);
+            foreach ($serviceDefinitionIds as $id => $attributes) {
+                if (0 === strncmp($this->getAlias(), $id, strlen($this->getAlias()))) {
+                    // avoid interfering with services that are not part of this bundle
+                    $helperStatus[$helper][$id] = array();
+                }
+            }
         }
 
-        foreach ($configurations as $key => $configuration) {
+        foreach ($configurations as $sitemapName => $configuration) {
             if (isset($configuration['default_change_frequency'])) {
                 $definition = new Definition('%cmf_seo.sitemap.guesser.default_change_frequency.class%', array(
                     $configuration['default_change_frequency']
                 ));
                 $definition->addTag('cmf_seo.sitemap.guesser', array(
-                    'sitemap' => $key,
+                    'sitemap' => $sitemapName,
                     'priority' => -1,
                 ));
-                $container->setDefinition($this->getAlias().'.sitemap.guesser.'.$key.'.default_change_frequency', $definition);
+                $container->setDefinition($this->getAlias().'.sitemap.guesser.'.$sitemapName.'.default_change_frequency', $definition);
             }
-            unset($configurations[$key]['default_change_frequency']);
+            unset($configurations[$sitemapName]['default_change_frequency']);
 
             // copy default configuration into this sitemap configuration to keep controller simple
             foreach ($config['defaults']['templates'] as $format => $name) {
-                if (!isset($configurations[$key]['templates'][$format])) {
-                    $configurations[$key]['templates'][$format] = $name;
+                if (!isset($configurations[$sitemapName]['templates'][$format])) {
+                    $configurations[$sitemapName]['templates'][$format] = $name;
                 }
             }
+            foreach ($helperStatus as $helper => $map) {
+                $status = count($configuration[$helper]) ? $configuration[$helper] : $config['defaults'][$helper];
+
+                foreach ($status as $s) {
+                    if ('_all' === $s) {
+                        foreach ($helperStatus[$helper] as $id => $sitemaps) {
+                            $helperStatus[$helper][$id][] = $sitemapName;
+                        }
+                    } elseif ('_none' !== $s) {
+                        $helperStatus[$helper][$s][] = $sitemapName;
+                    }
+                }
+                unset($configurations[$sitemapName][$helper]);
+            }
         }
 
         $container->setParameter($this->getAlias().'.sitemap.configurations', $configurations);
@@ -286,8 +313,34 @@ private function loadSitemapHandling($config, XmlFileLoader $loader, ContainerBu
             $config['defaults']['default_change_frequency']
         );
 
+        $this->handleSitemapHelper($helperStatus, $container);
+
         if (!$alternateLocale) {
             $container->removeDefinition($this->getAlias().'.sitemap.guesser.alternate_locales');
         }
     }
+
+    /**
+     * Each helper type out of the guessers, loaders and voters hav its on configuration to enable/disable them
+     *
+     * @param array $helperStatus Map of type => id => list of sitemaps
+     * @param ContainerBuilder $container
+     */
+    private function handleSitemapHelper($helperStatus, ContainerBuilder $container)
+    {
+        foreach ($helperStatus as $type => $status) {
+            foreach ($status as $id => $sitemaps) {
+                if (count($sitemaps)) {
+                    $definition = $container->getDefinition($id);
+                    $tags = $definition->getTag($this->sitemapHelperMap[$type]);
+                    $tag = reset($tags);
+                    $tag['sitemap'] = implode(',', $sitemaps);
+                    $definition->clearTag($this->sitemapHelperMap[$type]);
+                    $definition->addTag($this->sitemapHelperMap[$type], $tag);
+                } else {
+                    $container->removeDefinition($id);
+                }
+            }
+        }
+    }
 }
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -13,6 +13,7 @@
 
 use Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter;
 use Symfony\Cmf\Bundle\SeoBundle\SeoPresentation;
+use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
 use Symfony\Component\Config\Definition\Builder\NodeBuilder;
 use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 use Symfony\Component\Config\Definition\ConfigurationInterface;
@@ -33,6 +34,30 @@ public function getConfigTreeBuilder()
 
         $nodeBuilder = $treeBuilder->root('cmf_seo')
             ->addDefaultsIfNotSet()
+            ->beforeNormalization()
+                ->ifTrue(function ($config) {
+                    return isset($config['sitemap'])
+                        && (!isset($config['sitemap']['configurations'])
+                            || 0 == count($config['sitemap']['configurations'])
+                        )
+                        && !isset($config['sitemap']['configuration']) // xml configuration
+                    ;
+                })
+                ->then(function ($config) {
+                    if (true === $config['sitemap']) {
+                        $config['sitemap'] = array(
+                            'enabled' => true,
+                            'configurations' => array(
+                                'sitemap' => array()
+                            ),
+                        );
+                    } elseif (is_array($config['sitemap'])) {
+                        $config['sitemap']['configurations'] = array('sitemap' => array());
+                    }
+
+                    return $config;
+                })
+            ->end()
             ->beforeNormalization()
                 ->ifTrue(function ($config) {
                     return isset($config['content_key']) && !isset($config['content_listener']['content_key']);
@@ -187,19 +212,28 @@ private function addSitemapSection(NodeBuilder $nodeBuilder)
                                 ))
                                 ->prototype('scalar')->end()
                             ->end()
+                            ->append($this->getSitemapHelperNode('loaders', array('_all')))
+                            ->append($this->getSitemapHelperNode('guessers', array('_all')))
+                            ->append($this->getSitemapHelperNode('voters', array('_all')))
                         ->end()
                     ->end()
                     ->arrayNode('configurations')
                         ->useAttributeAsKey('name')
                         ->prototype('array')
                             ->fixXmlConfig('template')
+                            ->fixXmlConfig('loader')
+                            ->fixXmlConfig('guesser')
+                            ->fixXmlConfig('voter')
                             ->children()
                                 ->scalarNode('default_change_frequency')->defaultNull()->end()
                                 ->arrayNode('templates')
                                     ->useAttributeAsKey('format')
                                     ->requiresAtLeastOneElement()
                                     ->prototype('scalar')->end()
                                 ->end()
+                                ->append($this->getSitemapHelperNode('loaders', array()))
+                                ->append($this->getSitemapHelperNode('guessers', array()))
+                                ->append($this->getSitemapHelperNode('voters', array()))
                             ->end()
                         ->end()
                     ->end()
@@ -208,6 +242,26 @@ private function addSitemapSection(NodeBuilder $nodeBuilder)
         ;
     }
 
+    private function getSitemapHelperNode($type, $default)
+    {
+        $node = new ArrayNodeDefinition($type);
+        $node
+            ->beforeNormalization()
+                ->ifTrue(function ($config) {
+                    return is_string($config);
+                })
+                ->then(function ($config) {
+                    return array($config);
+                })
+            ->end()
+            ->defaultValue($default)
+            ->prototype('scalar')->end()
+            ->end()
+        ;
+
+        return $node;
+    }
+
     /**
      * Attach the content listener node to the tree.
      *

diff --git a/Resources/config/schema/seo-1.0.xsd b/Resources/config/schema/seo-1.0.xsd
@@ -70,8 +70,12 @@
     <xsd:complexType name="sitemap_configuration">
         <xsd:sequence>
             <xsd:element name="template" type="sitemap_template" minOccurs="1" maxOccurs="unbounded" />
+            <xsd:element name="loader" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
+            <xsd:element name="voter" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
+            <xsd:element name="guesser" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
         </xsd:sequence>
         <xsd:attribute name="default-change-frequency" type="change_frequency" default="always" />
+
     </xsd:complexType>
 
     <xsd:complexType name="named_sitemap_configuration">

diff --git a/Tests/Resources/Fixtures/config/config2.xml b/Tests/Resources/Fixtures/config/config2.xml
@@ -14,6 +14,9 @@
         <configuration default-change-frequency="never" name="default">
             <template format="xml">test.xml</template>
             <template format="html">test.html</template>
+            <loader>_none</loader>
+            <voter>_none</voter>
+            <guesser>_none</guesser>
         </configuration>
     </sitemap>
 

diff --git a/Tests/Unit/DependencyInjection/CmfSeoExtensionTest.php b/Tests/Unit/DependencyInjection/CmfSeoExtensionTest.php
@@ -3,7 +3,6 @@
 namespace Symfony\Cmf\SeoBundle\Tests\Unit\DependencyInjection;
 
 use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase;
-use Symfony\Cmf\Bundle\SeoBundle\CmfSeoBundle;
 use Symfony\Cmf\Bundle\SeoBundle\DependencyInjection\CmfSeoExtension;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
@@ -290,10 +289,11 @@ public function testSitemapConfiguration()
             'cmf_seo.sitemap.phpcr_loader',
             'Symfony\Cmf\Bundle\SeoBundle\Doctrine\Phpcr\SitemapDocumentProvider'
         );
+
         $this->assertContainerBuilderHasServiceDefinitionWithTag(
             'cmf_seo.sitemap.phpcr_loader',
             'cmf_seo.sitemap.loader',
-            array('priority' => -2)
+            array('priority' => -2, 'sitemap' => 'default,some_other')
         );
         $this->assertContainerBuilderHasService(
             'cmf_seo.sitemap.voter_chain',
@@ -302,7 +302,7 @@ public function testSitemapConfiguration()
         $this->assertContainerBuilderHasServiceDefinitionWithTag(
             'cmf_seo.sitemap.publish_workflow_voter',
             'cmf_seo.sitemap.voter',
-            array('priority' => -2)
+            array('priority' => -2, 'sitemap' => 'default,some_other')
         );
         $this->assertContainerBuilderHasService(
             'cmf_seo.sitemap.provider',
@@ -319,7 +319,7 @@ public function testSitemapConfiguration()
             $this->assertContainerBuilderHasServiceDefinitionWithTag(
                 $guesser,
                 'cmf_seo.sitemap.guesser',
-                array('priority' => -2)
+                array('priority' => -2, 'sitemap' => 'default,some_other')
             );
         }
         $this->assertContainerBuilderHasServiceDefinitionWithArgument(
@@ -362,7 +362,6 @@ public function testDefaultTemplatesSet()
             )
             ));
 
-
         $this->assertContainerBuilderHasParameter(
             'cmf_seo.sitemap.configurations',
             array(
@@ -376,6 +375,41 @@ public function testDefaultTemplatesSet()
         );
     }
 
+    public function testDisablingSitemapHelpers()
+    {
+        $this->container->setParameter(
+            'kernel.bundles',
+            array(
+                'DoctrinePHPCRBundle' => true,
+                'CmfRoutingBundle' => true,
+            )
+        );
+        $this->load(array(
+            'persistence' => array(
+                'phpcr' => true,
+            ),
+            'alternate_locale' => array(
+                'enabled' => true
+            ),
+            'sitemap' => array(
+                'defaults' => array(
+                    'default_change_frequency' => 'global-frequency',
+                    'loaders' => '_all',
+                    'guessers' => 'cmf_seo.sitemap.guesser.default_change_frequency',
+                    'voters' => '_none',
+                ),
+            )
+        ));
+
+        $this->assertContainerBuilderHasService('cmf_seo.sitemap.phpcr_loader');
+        $this->assertContainerBuilderHasService('cmf_seo.sitemap.guesser.default_change_frequency');
+        $this->assertContainerBuilderNotHasService('cmf_seo.sitemap.guesser.location');
+        $this->assertContainerBuilderNotHasService('cmf_seo.sitemap.guesser.location');
+        $this->assertContainerBuilderNotHasService('cmf_seo.sitemap.guesser.alternate_locales');
+        $this->assertContainerBuilderNotHasService('cmf_seo.sitemap.guesser.seo_metadata_title');
+        $this->assertContainerBuilderNotHasService('cmf_seo.sitemap.publish_workflow_voter');
+    }
+
     public function testDisableSeoContentListener()
     {
         $this->container->setParameter(

diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php
@@ -69,6 +69,9 @@ public function testDefaultsForAllConfigFormats()
                         'html' => 'CmfSeoBundle:Sitemap:index.html.twig',
                         'xml' => 'CmfSeoBundle:Sitemap:index.xml.twig',
                     ),
+                    'loaders' => array('_all'),
+                    'guessers' => array('_all'),
+                    'voters' => array('_all'),
                 ),
             ),
             'content_listener' => array(
@@ -100,6 +103,9 @@ public function testAdvancedXmlConfigurations()
                             'xml'  => 'test.xml',
                             'html' => 'test.html',
                         ),
+                        'loaders' => array(),
+                        'guessers' => array(),
+                        'voters' => array(),
                     ),
                 ),
                 'defaults' => array(
@@ -108,6 +114,9 @@ public function testAdvancedXmlConfigurations()
                         'html' => 'foo.html.twig',
                         'xml' => 'foo.xml.twig',
                     ),
+                    'loaders' => array('_all'),
+                    'guessers' => array('_all'),
+                    'voters' => array('_all'),
                 ),
             ),
             'persistence' => array(
@@ -145,3 +154,4 @@ public function testAdvancedXmlConfigurations()
         $this->assertProcessedConfigurationEquals($expectedConfiguration, $sources);
     }
 }
+