From 023acdef5d976847faddf7f31c53a3466628c2f1 Mon Sep 17 00:00:00 2001 From: ElectricMaxxx Date: Wed, 29 Jul 2015 00:04:49 +0200 Subject: [PATCH] provide configuration options to disable guessers/voters/loaders --- CHANGELOG.md | 16 ++-- DependencyInjection/CmfSeoExtension.php | 73 ++++++++++++++++--- DependencyInjection/Configuration.php | 54 ++++++++++++++ Resources/config/schema/seo-1.0.xsd | 4 + Tests/Resources/Fixtures/config/config2.xml | 3 + .../CmfSeoExtensionTest.php | 44 +++++++++-- .../DependencyInjection/ConfigurationTest.php | 10 +++ 7 files changed, 183 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dccbca1d..e2e74ca8 100644 --- 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 index e030aec6..210984b0 100644 --- 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 index aea9fca5..0b552315 100644 --- 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,12 +212,18 @@ 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') @@ -200,6 +231,9 @@ private function addSitemapSection(NodeBuilder $nodeBuilder) ->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 index 4d1e25c8..29ba5564 100644 --- a/Resources/config/schema/seo-1.0.xsd +++ b/Resources/config/schema/seo-1.0.xsd @@ -70,8 +70,12 @@ + + + + diff --git a/Tests/Resources/Fixtures/config/config2.xml b/Tests/Resources/Fixtures/config/config2.xml index 13f58e5b..52353ebf 100644 --- a/Tests/Resources/Fixtures/config/config2.xml +++ b/Tests/Resources/Fixtures/config/config2.xml @@ -14,6 +14,9 @@ + _none + _none + _none diff --git a/Tests/Unit/DependencyInjection/CmfSeoExtensionTest.php b/Tests/Unit/DependencyInjection/CmfSeoExtensionTest.php index 9ff52761..042728ab 100644 --- 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 index 34f6f901..64892ac3 100644 --- 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); } } +