From c01587d8b68f3a2b3efb47dbf561c6a2f2cbb7c0 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Fri, 27 May 2016 17:50:52 +0200 Subject: [PATCH 1/9] Introduced the concept of loaders This is the start of supporting annotations and mapping files instead of PHP extractor methods and lots of interfaces. --- CHANGELOG.md | 20 +- DependencyInjection/CmfSeoExtension.php | 1 + .../Compiler/RegisterExtractorsPass.php | 4 +- Loader/ExtractorLoader.php | 146 +++++++++++ Loader/SeoMetadataFactory.php | 74 ++++++ Resources/config/loaders.xml | 23 ++ Resources/config/services.xml | 2 +- SeoPresentation.php | 122 ++------- .../Fixtures/loader_config/empty.yml | 0 .../Fixtures/loader_config/valid1.yml | 0 .../Compiler/RegisterExtractorsPassTest.php | 12 +- Tests/Unit/Loader/ExtractorLoaderTest.php | 195 +++++++++++++++ Tests/Unit/Loader/SeoMetadataFactoryTest.php | 30 +++ Tests/Unit/SeoPresentationTest.php | 231 ++---------------- 14 files changed, 519 insertions(+), 341 deletions(-) create mode 100644 Loader/ExtractorLoader.php create mode 100644 Loader/SeoMetadataFactory.php create mode 100644 Resources/config/loaders.xml create mode 100644 Tests/Resources/Fixtures/loader_config/empty.yml create mode 100644 Tests/Resources/Fixtures/loader_config/valid1.yml create mode 100644 Tests/Unit/Loader/ExtractorLoaderTest.php create mode 100644 Tests/Unit/Loader/SeoMetadataFactoryTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c7694f8e..16fdbede 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,13 @@ Changelog ========= * **2016-06-18**: [BC BREAK] Removed all `*.class` parameters. - * **2016-05-08**: [BC BREAK] Removed `showAtion` in favor of `listAction` in the `SuggestionProviderController`. - * **2016-05-02**: [BC BREAK] Dropped PHP <5.5 support. - * **2016-05-02**: [BC BREAK] Dropped Symfony <2.8 support. + * **2016-05-27**: [BC BREAK] Removed `SeoPresentation::addExtractor()`, use `ExtractorLoader::addExtractor()` instead. + * **2016-05-27**: [BC BREAK] Changed the fourth argument of the constructorof `SeoPresentation` from `CacheInterface` + to `LoaderInterface`. + * **2016-05-27**: Added loaders. + * **2016-05-08**: [BC BREAK] Removed `showAtion` in favor of `listAction` in the `SuggestionProviderController` + * **2016-05-02**: [BC BREAK] Dropped PHP <5.5 support + * **2016-05-02**: [BC BREAK] Dropped Symfony <2.8 support 1.3.0 ----- @@ -22,15 +26,15 @@ Changelog * **2015-08-20**: Added templates configuration and `exclusion_rules` (based on the request matcher) to the error handling configuration * **2015-08-12**: Added configuration for the default data class of the `seo_metadata` form type. -* **2015-07-20**: Cleaned up the sitemap generation. If you used the unreleased +* **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 + 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 +* **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 of +* **2015-02-14**: [BC BREAK] Changed method visibility of `SeoPresentation#getSeoMetadata()` from private to public. * **2014-10-04**: Custom exception controller for error handling. diff --git a/DependencyInjection/CmfSeoExtension.php b/DependencyInjection/CmfSeoExtension.php index 4b2e4fea..8e919ef5 100644 --- a/DependencyInjection/CmfSeoExtension.php +++ b/DependencyInjection/CmfSeoExtension.php @@ -54,6 +54,7 @@ public function load(array $configs, ContainerBuilder $container) $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); $loader->load('services.xml'); + $loader->load('loaders.xml'); $loader->load('extractors.xml'); $this->loadSeoParameters($config, $container); diff --git a/DependencyInjection/Compiler/RegisterExtractorsPass.php b/DependencyInjection/Compiler/RegisterExtractorsPass.php index c406e82f..05154382 100644 --- a/DependencyInjection/Compiler/RegisterExtractorsPass.php +++ b/DependencyInjection/Compiler/RegisterExtractorsPass.php @@ -31,11 +31,11 @@ class RegisterExtractorsPass implements CompilerPassInterface */ public function process(ContainerBuilder $container) { - if (!$container->hasDefinition('cmf_seo.presentation')) { + if (!$container->hasDefinition('cmf_seo.loader.extractor')) { return; } - $strategyDefinition = $container->getDefinition('cmf_seo.presentation'); + $strategyDefinition = $container->getDefinition('cmf_seo.loader.extractor'); $taggedServices = $container->findTaggedServiceIds('cmf_seo.extractor'); foreach ($taggedServices as $id => $attributes) { diff --git a/Loader/ExtractorLoader.php b/Loader/ExtractorLoader.php new file mode 100644 index 00000000..1692c0a6 --- /dev/null +++ b/Loader/ExtractorLoader.php @@ -0,0 +1,146 @@ + + */ +class ExtractorLoader extends Loader +{ + /** + * @var null|CacheInterface + */ + private $cache; + + /** + * @var ExtractorInterface[][] + */ + private $extractors = array(); + + /** + * @param CacheInterface $cache + */ + public function __construct(CacheInterface $cache = null) + { + $this->cache = $cache; + } + + /** + * Add an extractor for SEO metadata. + * + * @param ExtractorInterface $extractor + * @param int $priority + */ + public function addExtractor(ExtractorInterface $extractor, $priority = 0) + { + if (!isset($this->extractors[$priority])) { + $this->extractors[$priority] = array(); + } + $this->extractors[$priority][] = $extractor; + } + + /** + * {@inheritdoc} + */ + public function supports($resource, $type = null) + { + return is_object($resource) && ((!$type && $this->containsExtractors($resource)) || 'extractors' === $type); + } + + /** + * {@inheritdoc} + * + * @param object $content + */ + public function load($content, $type = null) + { + $seoMetadata = SeoMetadataFactory::initializeSeoMetadata($content); + + $extractors = $this->getExtractorsForContent($content); + + foreach ($extractors as $extractor) { + $extractor->updateMetadata($content, $seoMetadata); + } + + return $seoMetadata; + } + + /** + * Returns and caches the extractors for content. + * + * @param object $content + * + * @return ExtractorInterface[] + */ + private function getExtractorsForContent($content) + { + $cachingAvailable = (bool) $this->cache; + + if (!$cachingAvailable) { + return $this->findExtractorsForContent($content); + } + + $extractors = $this->cache->loadExtractorsFromCache(get_class($content)); + + if (null === $extractors || !$extractors->isFresh()) { + $extractors = $this->findExtractorsForContent($content); + $this->cache->putExtractorsInCache(get_class($content), $extractors); + } + + return $extractors; + } + + /** + * Returns the extractors that support the content. + * + * @param object $content + * + * @return ExtractorInterface[] + */ + private function findExtractorsForContent($content) + { + $extractors = array(); + ksort($this->extractors); + foreach ($this->extractors as $priority) { + $supportedExtractors = array_filter($priority, function (ExtractorInterface $extractor) use ($content) { + return $extractor->supports($content); + }); + + $extractors = array_merge($extractors, $supportedExtractors); + } + + return $extractors; + } + + /** + * Whether there are extractors supporting the content. + * + * @param object $content + * + * @return bool + */ + private function containsExtractors($content) + { + $cacheAvailable = (bool) $this->cache; + if ($cacheAvailable) { + $extractors = $this->cache->loadExtractorsFromCache(get_class($content)); + + if (null !== $extractors) { + return count($extractors) > 0; + } + } + + ksort($this->extractors); + foreach (array_map('array_merge', $this->extractors) as $extractor) { + if ($extractor->supports($content)) { + return true; + } + } + + return false; + } +} diff --git a/Loader/SeoMetadataFactory.php b/Loader/SeoMetadataFactory.php new file mode 100644 index 00000000..c54edd42 --- /dev/null +++ b/Loader/SeoMetadataFactory.php @@ -0,0 +1,74 @@ + + */ +class SeoMetadataFactory +{ + /** + * @param object $content + * + * @return SeoMetadataInterface + * + * @throws InvalidArgumentException + */ + public static function initializeSeoMetadata($content) + { + if (!$content instanceof SeoAwareInterface) { + return new SeoMetadata(); + } + + $contentSeoMetadata = $content->getSeoMetadata(); + + if ($contentSeoMetadata instanceof SeoMetadataInterface) { + return self::copyMetadata($contentSeoMetadata); + } + + if (null === $contentSeoMetadata) { + $seoMetadata = new SeoMetadata(); + $content->setSeoMetadata($seoMetadata); // make sure it has metadata the next time + + return $seoMetadata; + } + + throw new InvalidArgumentException(sprintf( + 'getSeoMetadata must return either an instance of SeoMetadataInterface or null, "%s" given', + is_object($contentSeoMetadata) ? get_class($contentSeoMetadata) : gettype($contentSeoMetadata) + )); + } + + /** + * Copy the metadata object to sanitize it and remove doctrine traces. + * + * @param SeoMetadataInterface $contentSeoMetadata + * + * @return SeoMetadata + */ + private static function copyMetadata(SeoMetadataInterface $contentSeoMetadata) + { + $metadata = new SeoMetadata(); + + return $metadata + ->setTitle($contentSeoMetadata->getTitle()) + ->setMetaKeywords($contentSeoMetadata->getMetaKeywords()) + ->setMetaDescription($contentSeoMetadata->getMetaDescription()) + ->setOriginalUrl($contentSeoMetadata->getOriginalUrl()) + ->setExtraProperties($contentSeoMetadata->getExtraProperties() ?: array()) + ->setExtraNames($contentSeoMetadata->getExtraNames() ?: array()) + ->setExtraHttp($contentSeoMetadata->getExtraHttp() ?: array()) + ; + } +} diff --git a/Resources/config/loaders.xml b/Resources/config/loaders.xml new file mode 100644 index 00000000..3348267a --- /dev/null +++ b/Resources/config/loaders.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/Resources/config/services.xml b/Resources/config/services.xml index e2f78c83..56332cbc 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -33,7 +33,7 @@ - + diff --git a/SeoPresentation.php b/SeoPresentation.php index 9b4bb6b3..a46d7f64 100644 --- a/SeoPresentation.php +++ b/SeoPresentation.php @@ -12,13 +12,10 @@ namespace Symfony\Cmf\Bundle\SeoBundle; use Sonata\SeoBundle\Seo\SeoPage; -use Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface; use Symfony\Cmf\Bundle\SeoBundle\DependencyInjection\ConfigValues; -use Symfony\Cmf\Bundle\SeoBundle\Exception\InvalidArgumentException; -use Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface; use Symfony\Cmf\Bundle\SeoBundle\Model\AlternateLocaleCollection; -use Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadata; use Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadataInterface; +use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\Translation\TranslatorInterface; @@ -64,11 +61,6 @@ class SeoPresentation implements SeoPresentationInterface */ private $redirectResponse = false; - /** - * @var ExtractorInterface[] - */ - private $extractors = array(); - /** * @var TranslatorInterface */ @@ -80,9 +72,14 @@ class SeoPresentation implements SeoPresentationInterface private $configValues; /** - * @var null|CacheInterface + * @var LoaderInterface + */ + private $loader; + + /** + * @var SeoMetadataInterface[] */ - private $cache; + private $seoMetadatas = []; /** * The constructor will set the injected SeoPage - the service of @@ -91,18 +88,18 @@ class SeoPresentation implements SeoPresentationInterface * @param SeoPage $sonataPage * @param TranslatorInterface $translator * @param ConfigValues $configValues - * @param CacheInterface $cache + * @param LoaderInterface $loader */ public function __construct( SeoPage $sonataPage, TranslatorInterface $translator, ConfigValues $configValues, - CacheInterface $cache = null + LoaderInterface $loader ) { $this->sonataPage = $sonataPage; $this->translator = $translator; $this->configValues = $configValues; - $this->cache = $cache; + $this->loader = $loader; } /** @@ -121,20 +118,6 @@ public function getRedirectResponse() return $this->redirectResponse; } - /** - * Add an extractor for SEO metadata. - * - * @param ExtractorInterface $extractor - * @param int $priority - */ - public function addExtractor(ExtractorInterface $extractor, $priority = 0) - { - if (!isset($this->extractors[$priority])) { - $this->extractors[$priority] = array(); - } - $this->extractors[$priority][] = $extractor; - } - /** * Extract the SEO metadata from this object. * @@ -144,65 +127,12 @@ public function addExtractor(ExtractorInterface $extractor, $priority = 0) */ public function getSeoMetadata($content) { - if ($content instanceof SeoAwareInterface) { - $contentSeoMetadata = $content->getSeoMetadata(); - - if ($contentSeoMetadata instanceof SeoMetadataInterface) { - $seoMetadata = $this->copyMetadata($contentSeoMetadata); - } elseif (null === $contentSeoMetadata) { - $seoMetadata = new SeoMetadata(); - $content->setSeoMetadata($seoMetadata); // make sure it has metadata the next time - } else { - throw new InvalidArgumentException( - sprintf( - 'getSeoMetadata must return either an instance of SeoMetadataInterface or null, "%s" given', - is_object($contentSeoMetadata) ? get_class($contentSeoMetadata) : gettype($contentSeoMetadata) - ) - ); - } - } else { - $seoMetadata = new SeoMetadata(); + $hash = spl_object_hash($content); + if (isset($this->seoMetadatas[$hash])) { + return $this->seoMetadatas[$hash]; } - $cachingAvailable = (bool) $this->cache; - if ($cachingAvailable) { - $extractors = $this->cache->loadExtractorsFromCache(get_class($content)); - - if (null === $extractors || !$extractors->isFresh()) { - $extractors = $this->getExtractorsForContent($content); - $this->cache->putExtractorsInCache(get_class($content), $extractors); - } - } else { - $extractors = $this->getExtractorsForContent($content); - } - - foreach ($extractors as $extractor) { - $extractor->updateMetadata($content, $seoMetadata); - } - - return $seoMetadata; - } - - /** - * Returns the extractors for content. - * - * @param object $content - * - * @return ExtractorInterface[] - */ - private function getExtractorsForContent($content) - { - $extractors = array(); - ksort($this->extractors); - foreach ($this->extractors as $priority) { - $supportedExtractors = array_filter($priority, function (ExtractorInterface $extractor) use ($content) { - return $extractor->supports($content); - }); - - $extractors = array_merge($extractors, $supportedExtractors); - } - - return $extractors; + return $this->seoMetadatas[$hash] = $this->loader->load($content); } /** @@ -300,28 +230,6 @@ private function createKeywords($contentKeywords) return ('' !== $sonataKeywords ? $sonataKeywords.', ' : '').$contentKeywords; } - /** - * Copy the metadata object to sanitize it and remove doctrine traces. - * - * @param SeoMetadataInterface $contentSeoMetadata - * - * @return SeoMetadata - */ - private function copyMetadata(SeoMetadataInterface $contentSeoMetadata) - { - $metadata = new SeoMetadata(); - - return $metadata - ->setTitle($contentSeoMetadata->getTitle()) - ->setMetaKeywords($contentSeoMetadata->getMetaKeywords()) - ->setMetaDescription($contentSeoMetadata->getMetaDescription()) - ->setOriginalUrl($contentSeoMetadata->getOriginalUrl()) - ->setExtraProperties($contentSeoMetadata->getExtraProperties() ?: array()) - ->setExtraNames($contentSeoMetadata->getExtraNames() ?: array()) - ->setExtraHttp($contentSeoMetadata->getExtraHttp() ?: array()) - ; - } - /** * {inheritDoc}. */ diff --git a/Tests/Resources/Fixtures/loader_config/empty.yml b/Tests/Resources/Fixtures/loader_config/empty.yml new file mode 100644 index 00000000..e69de29b diff --git a/Tests/Resources/Fixtures/loader_config/valid1.yml b/Tests/Resources/Fixtures/loader_config/valid1.yml new file mode 100644 index 00000000..e69de29b diff --git a/Tests/Unit/DependencyInjection/Compiler/RegisterExtractorsPassTest.php b/Tests/Unit/DependencyInjection/Compiler/RegisterExtractorsPassTest.php index 1edda3d1..5808ee7e 100644 --- a/Tests/Unit/DependencyInjection/Compiler/RegisterExtractorsPassTest.php +++ b/Tests/Unit/DependencyInjection/Compiler/RegisterExtractorsPassTest.php @@ -33,13 +33,13 @@ public function testRegistersServicesWithExtractorTagAndDefaultPriority() $extractorService->addTag('cmf_seo.extractor'); $this->setDefinition('extractor_service', $extractorService); - $presentationService = new Definition(); - $this->setDefinition('cmf_seo.presentation', $presentationService); + $extractorLoaderService = new Definition(); + $this->setDefinition('cmf_seo.loader.extractor', $extractorLoaderService); $this->compile(); $this->assertContainerBuilderHasServiceDefinitionWithMethodCall( - 'cmf_seo.presentation', + 'cmf_seo.loader.extractor', 'addExtractor', array(new Reference('extractor_service'), 0) ); @@ -54,13 +54,13 @@ public function testRegistersServicesWithExtractorTagAndPriority() $extractorService->addTag('cmf_seo.extractor', array('priority' => 1)); $this->setDefinition('extractor_service', $extractorService); - $presentationService = new Definition(); - $this->setDefinition('cmf_seo.presentation', $presentationService); + $extractorLoaderService = new Definition(); + $this->setDefinition('cmf_seo.loader.extractor', $extractorLoaderService); $this->compile(); $this->assertContainerBuilderHasServiceDefinitionWithMethodCall( - 'cmf_seo.presentation', + 'cmf_seo.loader.extractor', 'addExtractor', array(new Reference('extractor_service'), 1) ); diff --git a/Tests/Unit/Loader/ExtractorLoaderTest.php b/Tests/Unit/Loader/ExtractorLoaderTest.php new file mode 100644 index 00000000..00c62f19 --- /dev/null +++ b/Tests/Unit/Loader/ExtractorLoaderTest.php @@ -0,0 +1,195 @@ + + */ +class ExtractorLoaderTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var ExtractorLoader + */ + private $loader; + private $content; + + protected function setUp() + { + $this->loader = new ExtractorLoader(); + $this->content = new \stdClass; + } + + public function testExtractors() + { + // promises + $extractor = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); + $extractor + ->expects($this->any()) + ->method('supports') + ->with($this->content) + ->will($this->returnValue(true)) + ; + $this->loader->addExtractor($extractor); + + // predictions + $extractor + ->expects($this->once()) + ->method('updateMetadata') + ; + + // test + $this->loader->load($this->content); + } + + public function testTitleExtractorsWithPriority() + { + // promises + $extractorDefault = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); + $extractorDefault + ->expects($this->any()) + ->method('supports') + ->with($this->content) + ->will($this->returnValue(true)) + ; + $extractorOne = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); + $extractorOne + ->expects($this->any()) + ->method('supports') + ->with($this->content) + ->will($this->returnValue(true)) + ; + $this->loader->addExtractor($extractorDefault); + $this->loader->addExtractor($extractorOne, 1); + + // predictions + $extractorDefault + ->expects($this->once()) + ->method('updateMetadata') + ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { + $seoMetadata->setTitle('First Title'); + })) + ; + $extractorOne + ->expects($this->once()) + ->method('updateMetadata') + ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { + $seoMetadata->setTitle('Final Title'); + })) + ; + + // test + $seoMetadata = $this->loader->load($this->content); + $this->assertEquals('Final Title', $seoMetadata->getTitle()); + } + + public function testDescriptionExtractorsWithPriority() + { + // promises + $extractorDefault = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); + $extractorDefault + ->expects($this->any()) + ->method('supports') + ->with($this->content) + ->will($this->returnValue(true)) + ; + $extractorOne = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); + $extractorOne + ->expects($this->any()) + ->method('supports') + ->with($this->content) + ->will($this->returnValue(true)) + ; + $this->loader->addExtractor($extractorDefault); + $this->loader->addExtractor($extractorOne, 1); + + // predictions + $extractorDefault + ->expects($this->once()) + ->method('updateMetadata') + ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { + $seoMetadata->setMetaDescription('First Description'); + })) + ; + $extractorOne + ->expects($this->once()) + ->method('updateMetadata') + ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { + $seoMetadata->setMetaDescription('Final Description'); + })) + ; + + // test + $seoMetadata = $this->loader->load($this->content); + $this->assertEquals('Final Description', $seoMetadata->getMetaDescription()); + } + + public function testCaching() + { + // promises + $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\ExtractorCollection') + ->disableOriginalConstructor() + ->getMock(); + $extractors + ->expects($this->any()) + ->method('isFresh') + ->will($this->returnValue(true)) + ; + $extractors + ->expects($this->any()) + ->method('getIterator') + ->will($this->returnValue(new \ArrayIterator())) + ; + $cache = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface'); + $cache + ->expects($this->any()) + ->method('loadExtractorsFromCache') + ->will($this->onConsecutiveCalls(null, $extractors)) + ; + $loader = new ExtractorLoader($cache); + + // predictions + $cache + ->expects($this->once()) + ->method('putExtractorsInCache') + ; + + $loader->load($this->content); + $loader->load($this->content); + } + + public function testCacheRefresh() + { + // promises + $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\ExtractorCollection') + ->disableOriginalConstructor() + ->getMock(); + $extractors + ->expects($this->any()) + ->method('isFresh') + ->will($this->returnValue(false)) + ; + $extractors + ->expects($this->any()) + ->method('getIterator') + ->will($this->returnValue(new \ArrayIterator())) + ; + $cache = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface'); + $cache + ->expects($this->any()) + ->method('loadExtractorsFromCache') + ->will($this->returnValue($extractors)) + ; + $loader = new ExtractorLoader($cache); + + // predictions + $cache + ->expects($this->once()) + ->method('putExtractorsInCache') + ; + + $loader->load($this->content); + } +} diff --git a/Tests/Unit/Loader/SeoMetadataFactoryTest.php b/Tests/Unit/Loader/SeoMetadataFactoryTest.php new file mode 100644 index 00000000..00079a88 --- /dev/null +++ b/Tests/Unit/Loader/SeoMetadataFactoryTest.php @@ -0,0 +1,30 @@ + + */ +class SeoMetadataFactoryTest extends \PHPUnit_Framework_TestCase +{ + public function testSeoAwareWithoutCurrentMetadata() + { + $content = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Tests\Resources\Document\SeoAwareContent'); + $content + ->expects($this->any()) + ->method('getSeoMetadata') + ->will($this->returnValue(null)) + ; + + $content + ->expects($this->once()) + ->method('setSeoMetadata') + ->with($this->isInstanceOf(SeoMetadataInterface::class)) + ; + + SeoMetadataFactory::initializeSeoMetadata($content); + } +} diff --git a/Tests/Unit/SeoPresentationTest.php b/Tests/Unit/SeoPresentationTest.php index 16fc01f1..bff2a374 100644 --- a/Tests/Unit/SeoPresentationTest.php +++ b/Tests/Unit/SeoPresentationTest.php @@ -14,6 +14,7 @@ use Symfony\Cmf\Bundle\SeoBundle\DependencyInjection\ConfigValues; use Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadataInterface; use Symfony\Cmf\Bundle\SeoBundle\SeoPresentation; +use Symfony\Component\Config\Loader\LoaderInterface; /** * This test will cover the behavior of the SeoPresentation Model @@ -28,11 +29,21 @@ class SeoPresentationTest extends \PHPUnit_Framework_Testcase private $translator; private $content; private $configValues; + private $loader; public function setUp() { $this->pageService = $this->getMock('Sonata\SeoBundle\Seo\SeoPage'); $this->translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface'); + $this->seoMetadata = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadata'); + $this->content = new \stdClass; + + $this->loader = $this->getMock(LoaderInterface::class); + $this->loader + ->expects($this->any()) + ->method('load') + ->will($this->returnValue($this->seoMetadata)); + $this->configValues = new ConfigValues(); $this->configValues->setDescription('default_description'); $this->configValues->setTitle('default_title'); @@ -41,17 +52,9 @@ public function setUp() $this->seoPresentation = new SeoPresentation( $this->pageService, $this->translator, - $this->configValues + $this->configValues, + $this->loader ); - - $this->seoMetadata = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadata'); - - $this->content = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Tests\Resources\Document\SeoAwareContent'); - $this->content - ->expects($this->any()) - ->method('getSeoMetadata') - ->will($this->returnValue($this->seoMetadata)) - ; } public function testDefaultTitle() @@ -77,7 +80,7 @@ public function testDefaultTitle() ; // test - $this->seoPresentation->updateSeoPage($this->content); + $this->seoPresentation->updateSeoPage(new \stdClass); } public function testContentTitle() @@ -177,131 +180,6 @@ public function testSettingKeywordsToSeoPage() $this->seoPresentation->updateSeoPage($this->content); } - public function testExtractors() - { - // promises - $this->translator - ->expects($this->any()) - ->method('trans') - ->will($this->returnValue('translation strategy test')) - ; - $extractor = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractor - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $this->seoPresentation->addExtractor($extractor); - - // predictions - $extractor - ->expects($this->once()) - ->method('updateMetadata') - ; - - // test - $this->seoPresentation->updateSeoPage($this->content); - } - - public function testTitleExtractorsWithPriority() - { - // promises - $this->translator - ->expects($this->any()) - ->method('trans') - ->with($this->equalTo('default_title'), $this->equalTo(array('%content_title%' => 'Final Title')), $this->equalTo(null)) - ->will($this->returnValue('translation strategy test')) - ; - $extractorDefault = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractorDefault - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $extractorOne = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractorOne - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $this->seoPresentation->addExtractor($extractorDefault); - $this->seoPresentation->addExtractor($extractorOne, 1); - - // predictions - $extractorDefault - ->expects($this->once()) - ->method('updateMetadata') - ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { - $seoMetadata->setTitle('First Title'); - })) - ; - $extractorOne - ->expects($this->once()) - ->method('updateMetadata') - ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { - $seoMetadata->setTitle('Final Title'); - })) - ; - - // test - $this->seoPresentation->updateSeoPage($this->content); - } - - public function testDescriptionExtractorsWithPriority() - { - $this->translator - ->expects($this->any()) - ->method('trans') - ->with($this->equalTo('default_description'), $this->equalTo(array('%content_description%' => 'Final Description')), $this->equalTo(null)) - ->will($this->returnValue('translation strategy test')) - ; - - // promises - $extractorDefault = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractorDefault - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $extractorOne = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractorOne - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $this->seoPresentation->addExtractor($extractorDefault); - $this->seoPresentation->addExtractor($extractorOne, 1); - - // predictions - $extractorDefault - ->expects($this->once()) - ->method('updateMetadata') - ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { - $seoMetadata->setMetaDescription('First Description'); - })) - ; - $extractorOne - ->expects($this->once()) - ->method('updateMetadata') - ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { - $seoMetadata->setMetaDescription('Final Description'); - })) - ; - $this->pageService - ->expects($this->once()) - ->method('addMeta') - ->with('name', 'description', 'translation strategy test') - ; - - // test - $this->seoPresentation->updateSeoPage($this->content); - } - public function testRedirect() { // promises @@ -321,87 +199,6 @@ public function testRedirect() $this->assertEquals('/redirect/target', $redirect->getTargetUrl()); } - public function testCaching() - { - // promises - $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\ExtractorCollection') - ->disableOriginalConstructor() - ->getMock(); - $extractors - ->expects($this->any()) - ->method('isFresh') - ->will($this->returnValue(true)) - ; - $extractors - ->expects($this->any()) - ->method('getIterator') - ->will($this->returnValue(new \ArrayIterator())) - ; - $cache = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface'); - $cache - ->expects($this->any()) - ->method('loadExtractorsFromCache') - ->will($this->onConsecutiveCalls(null, $extractors)) - ; - $seoPresentation = new SeoPresentation( - $this->pageService, - $this->translator, - $this->configValues, - $cache - ); - - // predictions - $cache - ->expects($this->once()) - ->method('putExtractorsInCache') - ; - - $seoPresentation->updateSeoPage($this->content); - $seoPresentation->updateSeoPage($this->content); - - return array($seoPresentation, $cache, $extractors); - } - - public function testCacheRefresh() - { - // promises - $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\ExtractorCollection') - ->disableOriginalConstructor() - ->getMock(); - $extractors - ->expects($this->any()) - ->method('isFresh') - ->will($this->returnValue(false)) - ; - $extractors - ->expects($this->any()) - ->method('getIterator') - ->will($this->returnValue(new \ArrayIterator())) - ; - $cache = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface'); - $cache - ->expects($this->any()) - ->method('loadExtractorsFromCache') - ->will($this->returnValue($extractors)) - ; - $seoPresentation = new SeoPresentation( - $this->pageService, - $this->translator, - $this->configValues, - $cache - ); - - // predictions - $cache - ->expects($this->once()) - ->method('putExtractorsInCache') - ; - - $seoPresentation->updateSeoPage($this->content); - - return array($seoPresentation, $cache, $extractors); - } - public function testSeoAwareWithoutCurrentMetadata() { $content = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Tests\Resources\Document\SeoAwareContent'); From b5f683ff43b6a71c1cce9dddc0a783ffc64722b7 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Fri, 27 May 2016 22:23:37 +0200 Subject: [PATCH 2/9] Implemented annotations for SEO metadata information --- Loader/Annotation/Extras.php | 73 ++++++++++ Loader/Annotation/MetaDescription.php | 40 ++++++ Loader/Annotation/MetaKeywords.php | 30 ++++ Loader/Annotation/OriginalUrl.php | 26 ++++ Loader/Annotation/SeoMetadataAnnotation.php | 19 +++ Loader/Annotation/Title.php | 26 ++++ Loader/AnnotationLoader.php | 130 ++++++++++++++++++ Loader/ExtractorLoader.php | 26 +--- Resources/config/loaders.xml | 9 +- Tests/Functional/Loader/LoaderTest.php | 31 +++++ .../Document/MethodAnnotatedContent.php | 71 ++++++++++ .../Document/PropertyAnnotatedContent.php | 48 +++++++ .../Fixtures/loader_config/valid1.yml | 7 + .../Unit/Loader/BaseAnnotationLoaderTest.php | 95 +++++++++++++ .../Loader/Method_AnnotationLoaderTest.php | 15 ++ .../Loader/Property_AnnotationLoaderTest.php | 16 +++ composer.json | 3 +- 17 files changed, 643 insertions(+), 22 deletions(-) create mode 100644 Loader/Annotation/Extras.php create mode 100644 Loader/Annotation/MetaDescription.php create mode 100644 Loader/Annotation/MetaKeywords.php create mode 100644 Loader/Annotation/OriginalUrl.php create mode 100644 Loader/Annotation/SeoMetadataAnnotation.php create mode 100644 Loader/Annotation/Title.php create mode 100644 Loader/AnnotationLoader.php create mode 100644 Tests/Functional/Loader/LoaderTest.php create mode 100644 Tests/Resources/Document/MethodAnnotatedContent.php create mode 100644 Tests/Resources/Document/PropertyAnnotatedContent.php create mode 100644 Tests/Unit/Loader/BaseAnnotationLoaderTest.php create mode 100644 Tests/Unit/Loader/Method_AnnotationLoaderTest.php create mode 100644 Tests/Unit/Loader/Property_AnnotationLoaderTest.php diff --git a/Loader/Annotation/Extras.php b/Loader/Annotation/Extras.php new file mode 100644 index 00000000..cd7f65c2 --- /dev/null +++ b/Loader/Annotation/Extras.php @@ -0,0 +1,73 @@ + + * @Annotation + */ +class Extras implements SeoMetadataAnnotation +{ + public $key; + public $type; + + private static $allowedTypesMethodMapping = [ + 'property' => 'addExtraProperty', + 'name' => 'addExtraName', + 'http-equiv' => 'addExtraHttp', + ]; + + public function serialize() + { + return serialize($this->key); + } + + public function unserialize($serialized) + { + list($this->key) = unserialize($serialized); + } + + public function configureSeoMetadata(SeoMetadataInterface $seoMetadata, $value) + { + if (null === $this->key || null === $this->type) { + if (!is_array($value)) { + throw new InvalidArgumentException( + 'Either set the "type" and "key" options for the @Extras annotation or provide an array with extras.' + ); + } + + $this->configureAllExtras($seoMetadata, $value); + + return; + } + + $this->guardTypeAllowed($this->type); + + $seoMetadata->{self::$allowedTypesMethodMapping[$this->type]}($this->key, $value); + } + + private function configureAllExtras(SeoMetadataInterface $seoMetadata, $value) + { + foreach ($value as $type => $extras) { + $this->guardTypeAllowed($type); + + foreach ($extras as $key => $value) { + $seoMetadata->{self::$allowedTypesMethodMapping[$type]}($key, $value); + } + } + } + + private function guardTypeAllowed($type) + { + if (!isset(self::$allowedTypesMethodMapping[$type])) { + throw new InvalidArgumentException(sprintf( + 'Extras type "%s" not in the list of allowed ones: "%s".', + $type, + implode('", "', array_keys(self::$allowedTypesMethodMapping)) + )); + } + } +} diff --git a/Loader/Annotation/MetaDescription.php b/Loader/Annotation/MetaDescription.php new file mode 100644 index 00000000..069cea9d --- /dev/null +++ b/Loader/Annotation/MetaDescription.php @@ -0,0 +1,40 @@ + + * @Annotation + */ +class MetaDescription implements SeoMetadataAnnotation +{ + /** + * The description length to truncate the description. + * + * The default value 0 disables truncation. + * + * @var int + */ + public $truncate = 0; + + public function serialize() + { + return serialize([$this->truncate]); + } + + public function unserialize($serialized) + { + list($this->truncate) = unserialize($serialized); + } + + public function configureSeoMetadata(SeoMetadataInterface $seoMetadata, $value) + { + if ($this->truncate > 0 && strlen($value) > $this->truncate) { + $value = substr($value, 0, $this->truncate).'...'; + } + + $seoMetadata->setMetaDescription($value); + } +} diff --git a/Loader/Annotation/MetaKeywords.php b/Loader/Annotation/MetaKeywords.php new file mode 100644 index 00000000..3f0c46c0 --- /dev/null +++ b/Loader/Annotation/MetaKeywords.php @@ -0,0 +1,30 @@ + + * @Annotation + */ +class MetaKeywords implements SeoMetadataAnnotation +{ + public function serialize() + { + return ''; + } + + public function unserialize($serialized) + { + } + + public function configureSeoMetadata(SeoMetadataInterface $seoMetadata, $value) + { + if ($value instanceof \Traversable) { + $value = iterator_to_array($value); + } + + $seoMetadata->setMetaKeywords(implode(', ', (array) $value)); + } +} diff --git a/Loader/Annotation/OriginalUrl.php b/Loader/Annotation/OriginalUrl.php new file mode 100644 index 00000000..6b3bef8a --- /dev/null +++ b/Loader/Annotation/OriginalUrl.php @@ -0,0 +1,26 @@ + + * @Annotation + */ +class OriginalUrl implements SeoMetadataAnnotation +{ + public function serialize() + { + return ''; + } + + public function unserialize($serialized) + { + } + + public function configureSeoMetadata(SeoMetadataInterface $seoMetadata, $value) + { + $seoMetadata->setOriginalUrl($value); + } +} diff --git a/Loader/Annotation/SeoMetadataAnnotation.php b/Loader/Annotation/SeoMetadataAnnotation.php new file mode 100644 index 00000000..a7437ff1 --- /dev/null +++ b/Loader/Annotation/SeoMetadataAnnotation.php @@ -0,0 +1,19 @@ + + */ +interface SeoMetadataAnnotation extends \Serializable +{ + /** + * Configures the seo metadata based on the extract value. + * + * @param SeoMetadataInterface $seoMetadata + * @param mixed $value + */ + public function configureSeoMetadata(SeoMetadataInterface $seoMetadata, $value); +} diff --git a/Loader/Annotation/Title.php b/Loader/Annotation/Title.php new file mode 100644 index 00000000..35d195db --- /dev/null +++ b/Loader/Annotation/Title.php @@ -0,0 +1,26 @@ + + * @Annotation + */ +class Title implements SeoMetadataAnnotation +{ + public function serialize() + { + return ''; + } + + public function unserialize($serialized) + { + } + + public function configureSeoMetadata(SeoMetadataInterface $seoMetadata, $value) + { + $seoMetadata->setTitle($value); + } +} diff --git a/Loader/AnnotationLoader.php b/Loader/AnnotationLoader.php new file mode 100644 index 00000000..031a458a --- /dev/null +++ b/Loader/AnnotationLoader.php @@ -0,0 +1,130 @@ + + */ +class AnnotationLoader extends Loader +{ + /** + * @var Reader + */ + private $reader; + + public function __construct(Reader $reader) + { + $this->reader = $reader; + } + + public function supports($resource, $type = null) + { + return is_object($resource) && $this->isAnnotated($resource); + } + + public function load($content, $type = null) + { + $seoMetadata = SeoMetadataFactory::initializeSeoMetadata($content); + + $data = $this->getAnnotationsFromContent($content); + + // todo cache $data + + $classReflection = new \ReflectionClass($content); + foreach ($data['properties'] as $propertyName => $annotations) { + /** @var SeoMetadataAnnotation $annotation */ + foreach ($annotations as $annotation) { + $property = $classReflection->getProperty($propertyName); + $property->setAccessible(true); + + $annotation->configureSeoMetadata($seoMetadata, $property->getValue($content)); + } + } + + foreach ($data['methods'] as $methodName => $annotations) { + /** @var SeoMetadataAnnotation $annotation */ + foreach ($annotations as $annotation) { + $annotation->configureSeoMetadata($seoMetadata, $content->{$methodName}()); + } + } + + return $seoMetadata; + } + + private function getAnnotationsFromContent($content) + { + $classReflection = new \ReflectionClass($content); + + return [ + 'properties' => $this->readProperties($classReflection->getProperties()), + 'methods' => $this->readMethods($classReflection->getMethods()), + ]; + } + + /** + * @param \ReflectionProperty[] $properties + * + * @return SeoMetadataAnnotation[][] + */ + private function readProperties(array $properties) + { + $propertyAnnotations = []; + + foreach ($properties as $reflectionProperty) { + $annotations = $this->reader->getPropertyAnnotations($reflectionProperty); + $propertyName = $reflectionProperty->getName(); + + foreach ($annotations as $annotation) { + if ($annotation instanceof SeoMetadataAnnotation) { + if (!isset($propertyAnnotations[$propertyName])) { + $propertyAnnotations[$propertyName] = []; + } + + $propertyAnnotations[$propertyName][] = $annotation; + } + } + } + + return $propertyAnnotations; + } + + /** + * @param \ReflectionMethod[] $methods + * + * @return SeoMetadataAnnotation[][] + */ + private function readMethods(array $methods) + { + $methodAnnotations = []; + + foreach ($methods as $reflectionMethod) { + $annotations = $this->reader->getMethodAnnotations($reflectionMethod); + $methodName = $reflectionMethod->getName(); + + foreach ($annotations as $annotation) { + if ($annotation instanceof SeoMetadataAnnotation) { + if (!isset($methodAnnotations[$methodName])) { + $methodAnnotations[$methodName] = []; + } + + $methodAnnotations[$methodName][] = $annotation; + } + } + } + + return $methodAnnotations; + } + + private function isAnnotated($content) + { + return 0 !== count(call_user_func_array('array_merge', $this->getAnnotationsFromContent($content))); + } +} diff --git a/Loader/ExtractorLoader.php b/Loader/ExtractorLoader.php index 1692c0a6..48706d1f 100644 --- a/Loader/ExtractorLoader.php +++ b/Loader/ExtractorLoader.php @@ -3,6 +3,7 @@ namespace Symfony\Cmf\Bundle\SeoBundle\Loader; use Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface; +use Symfony\Cmf\Bundle\SeoBundle\Cache\ExtractorCollection; use Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface; use Symfony\Component\Config\Loader\Loader; @@ -48,7 +49,7 @@ public function addExtractor(ExtractorInterface $extractor, $priority = 0) */ public function supports($resource, $type = null) { - return is_object($resource) && ((!$type && $this->containsExtractors($resource)) || 'extractors' === $type); + return is_object($resource) && $this->containsExtractors($resource); } /** @@ -74,14 +75,14 @@ public function load($content, $type = null) * * @param object $content * - * @return ExtractorInterface[] + * @return ExtractorCollection */ private function getExtractorsForContent($content) { $cachingAvailable = (bool) $this->cache; if (!$cachingAvailable) { - return $this->findExtractorsForContent($content); + return new ExtractorCollection($this->findExtractorsForContent($content)); } $extractors = $this->cache->loadExtractorsFromCache(get_class($content)); @@ -89,6 +90,7 @@ private function getExtractorsForContent($content) if (null === $extractors || !$extractors->isFresh()) { $extractors = $this->findExtractorsForContent($content); $this->cache->putExtractorsInCache(get_class($content), $extractors); + $extractors = new ExtractorCollection($extractors); } return $extractors; @@ -125,22 +127,6 @@ private function findExtractorsForContent($content) */ private function containsExtractors($content) { - $cacheAvailable = (bool) $this->cache; - if ($cacheAvailable) { - $extractors = $this->cache->loadExtractorsFromCache(get_class($content)); - - if (null !== $extractors) { - return count($extractors) > 0; - } - } - - ksort($this->extractors); - foreach (array_map('array_merge', $this->extractors) as $extractor) { - if ($extractor->supports($content)) { - return true; - } - } - - return false; + return 0 !== count(iterator_to_array($this->getExtractorsForContent($content))); } } diff --git a/Resources/config/loaders.xml b/Resources/config/loaders.xml index 3348267a..e6610018 100644 --- a/Resources/config/loaders.xml +++ b/Resources/config/loaders.xml @@ -9,15 +9,22 @@ + - + + + + + + + diff --git a/Tests/Functional/Loader/LoaderTest.php b/Tests/Functional/Loader/LoaderTest.php new file mode 100644 index 00000000..44bc78e4 --- /dev/null +++ b/Tests/Functional/Loader/LoaderTest.php @@ -0,0 +1,31 @@ + + */ +class LoaderTest extends BaseTestCase +{ + /** + * @dataProvider getLoadContentData + */ + public function testLoadContent($content) + { + $seoMetadata = $this->getContainer()->get('cmf_seo.loader')->load($content); + + $this->assertEquals('Default name.', $seoMetadata->getTitle()); + } + + public function getLoadContentData() + { + return [ + [new MethodAnnotatedContent()], + [(new ContentWithExtractors())->setTitle('Default name.')], + ]; + } +} diff --git a/Tests/Resources/Document/MethodAnnotatedContent.php b/Tests/Resources/Document/MethodAnnotatedContent.php new file mode 100644 index 00000000..b7b7ecfd --- /dev/null +++ b/Tests/Resources/Document/MethodAnnotatedContent.php @@ -0,0 +1,71 @@ + + */ +class MethodAnnotatedContent +{ + private $name; + private $description; + private $keywords; + public $extras = []; + + public function __construct($name = 'Default name.', $description = 'Default description.', $keywords = ['keyword1', 'keyword2']) + { + $this->name = $name; + $this->description = $description; + $this->keywords = $keywords; + } + + /** + * @SEO\Title + */ + public function getName() + { + return $this->name; + } + + /** + * @SEO\MetaDescription(truncate=30) + */ + public function getDescription() + { + return $this->description; + } + + /** + * @SEO\MetaKeywords + */ + public function getKeywords() + { + return $this->keywords; + } + + /** + * @SEO\OriginalUrl + */ + public function getOriginalUrl() + { + return '/home'; + } + + /** + * @SEO\Extras(type="property", key="og:title") + */ + public function getOgTitle() + { + return 'Extra Title.'; + } + + /** + * @SEO\Extras + */ + public function getExtras() + { + return $this->extras; + } +} diff --git a/Tests/Resources/Document/PropertyAnnotatedContent.php b/Tests/Resources/Document/PropertyAnnotatedContent.php new file mode 100644 index 00000000..1b9fe3a0 --- /dev/null +++ b/Tests/Resources/Document/PropertyAnnotatedContent.php @@ -0,0 +1,48 @@ + + */ +class PropertyAnnotatedContent +{ + /** + * @SEO\Title + */ + private $name; + + /** + * @SEO\MetaDescription(truncate=30) + */ + private $description; + + /** + * @SEO\MetaKeywords + */ + private $keywords; + + /** + * @SEO\OriginalUrl + */ + private $originalUrl = '/home'; + + /** + * @SEO\Extras(type="property", key="og:title") + */ + protected $ogTitle = 'Extra Title.'; + + /** + * @SEO\Extras + */ + public $extras = []; + + public function __construct($name = 'Default name.', $description = 'Default description.', $keywords = ['keyword1', 'keyword2']) + { + $this->name = $name; + $this->description = $description; + $this->keywords = $keywords; + } +} diff --git a/Tests/Resources/Fixtures/loader_config/valid1.yml b/Tests/Resources/Fixtures/loader_config/valid1.yml index e69de29b..c9672020 100644 --- a/Tests/Resources/Fixtures/loader_config/valid1.yml +++ b/Tests/Resources/Fixtures/loader_config/valid1.yml @@ -0,0 +1,7 @@ +Symfony\Cmf\Bundle\SeoBundle\Tests\Resources\Document\ContentWithExtractors: + properties: + title: { type: title } + description: { type: description } + + methods: + get diff --git a/Tests/Unit/Loader/BaseAnnotationLoaderTest.php b/Tests/Unit/Loader/BaseAnnotationLoaderTest.php new file mode 100644 index 00000000..96dc5a45 --- /dev/null +++ b/Tests/Unit/Loader/BaseAnnotationLoaderTest.php @@ -0,0 +1,95 @@ + + */ +abstract class BaseAnnotationLoaderTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var AnnotationLoader + */ + private $loader; + + protected function setUp() + { + $this->loader = new AnnotationLoader(new AnnotationReader()); + } + + abstract protected function getContent($title = 'Default name.', $description = 'Default description.', $keywords = ['keyword1', 'keyword2']); + + public function testPropertyAnnotations() + { + $seoMetadata = $this->loader->load($this->getContent()); + + $this->assertEquals('Default description.', $seoMetadata->getMetaDescription()); + $this->assertEquals('Default name.', $seoMetadata->getTitle()); + $this->assertEquals('keyword1, keyword2', $seoMetadata->getMetaKeywords()); + $this->assertEquals('/home', $seoMetadata->getOriginalUrl()); + $this->assertEquals(['og:title' => 'Extra Title.'], $seoMetadata->getExtraProperties()); + } + + /** + * @dataProvider getKeywordAnnotationData + */ + public function testKeywordAnnotation($contentValue, $seoMetadataValue) + { + $seoMetadata = $this->loader->load($this->getContent('', '', $contentValue)); + + $this->assertEquals($seoMetadataValue, $seoMetadata->getMetaKeywords()); + } + + public function getKeywordAnnotationData() + { + return [ + ['keyword A; keyword B', 'keyword A; keyword B'], + [new \ArrayIterator(['1st keyword', '2nd keyword']), '1st keyword, 2nd keyword'], + ]; + } + + /** + * @dataProvider getDescriptionAnnotationData + */ + public function testDescriptionAnnotation($contentValue, $seoMetadataValue) + { + $seoMetadata = $this->loader->load($this->getContent('', $contentValue)); + + $this->assertEquals($seoMetadataValue, $seoMetadata->getMetaDescription()); + } + + public function getDescriptionAnnotationData() + { + return [ + ['A much longer default description', 'A much longer default descript...'], + ]; + } + + public function testExtrasAnnotationWithArray() + { + $content = $this->getContent(); + $content->extras = [ + 'http-equiv' => ['robots' => 'index, follow'], + ]; + + $seoMetadata = $this->loader->load($content); + + $this->assertEquals(['robots' => 'index, follow'], $seoMetadata->getExtraHttp()); + } + + /** + * @expectedException \Symfony\Cmf\Bundle\SeoBundle\Exception\InvalidArgumentException + * @expectedExceptionMessage Either set the "type" and "key" options for the @Extras annotation or provide an array with extras. + */ + public function testExtrasAnnotationWithScalar() + { + $content = $this->getContent(); + $content->extras = 'index, follow'; + + $this->loader->load($content); + } +} diff --git a/Tests/Unit/Loader/Method_AnnotationLoaderTest.php b/Tests/Unit/Loader/Method_AnnotationLoaderTest.php new file mode 100644 index 00000000..027b87ca --- /dev/null +++ b/Tests/Unit/Loader/Method_AnnotationLoaderTest.php @@ -0,0 +1,15 @@ + + */ +class Method_AnnotationLoaderTest extends BaseAnnotationLoaderTest +{ + protected function getContent($title = 'Default name.', $description = 'Default description.', $keywords = ['keyword1', 'keyword2']) { + return new MethodAnnotatedContent($title, $description, $keywords); + } +} diff --git a/Tests/Unit/Loader/Property_AnnotationLoaderTest.php b/Tests/Unit/Loader/Property_AnnotationLoaderTest.php new file mode 100644 index 00000000..525ef4ff --- /dev/null +++ b/Tests/Unit/Loader/Property_AnnotationLoaderTest.php @@ -0,0 +1,16 @@ + + */ +class Property_AnnotationLoaderTest extends BaseAnnotationLoaderTest +{ + protected function getContent($title = 'Default name.', $description = 'Default description.', $keywords = ['keyword1', 'keyword2']) + { + return new PropertyAnnotatedContent($title, $description, $keywords); + } +} diff --git a/composer.json b/composer.json index c97e96b3..52bed3ee 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,8 @@ "php": "^5.5.9|^7.0", "symfony/framework-bundle": "^2.8|^3.0", "sonata-project/seo-bundle": "^1.1.3|^2.0", - "doctrine/collections": "^1.0" + "doctrine/collections": "^1.0", + "doctrine/annotations": "^1.2" }, "require-dev": { "symfony-cmf/testing": "^2.0", From 41d6532a9a22d065bb1ea0b003f3bf8f3cd77d89 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Tue, 31 May 2016 14:33:33 +0200 Subject: [PATCH 3/9] Added PSR-6 cache integration --- CHANGELOG.md | 12 +- Cache/CacheInterface.php | 38 ----- ...torCollection.php => CachedCollection.php} | 48 +++++-- Cache/FileCache.php | 131 ------------------ Loader/AnnotationLoader.php | 41 ++++-- Loader/ExtractorLoader.php | 33 +++-- Tests/Unit/Cache/FileCacheTest.php | 49 ------- Tests/Unit/Loader/ExtractorLoaderTest.php | 29 ++-- composer.json | 1 + 9 files changed, 116 insertions(+), 266 deletions(-) delete mode 100644 Cache/CacheInterface.php rename Cache/{ExtractorCollection.php => CachedCollection.php} (58%) delete mode 100644 Cache/FileCache.php delete mode 100644 Tests/Unit/Cache/FileCacheTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 16fdbede..3148acd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,15 @@ Changelog ========= * **2016-06-18**: [BC BREAK] Removed all `*.class` parameters. - * **2016-05-27**: [BC BREAK] Removed `SeoPresentation::addExtractor()`, use `ExtractorLoader::addExtractor()` instead. - * **2016-05-27**: [BC BREAK] Changed the fourth argument of the constructorof `SeoPresentation` from `CacheInterface` - to `LoaderInterface`. + * **2016-05-31**: [BC BREAK] Removed `CacheInterface` and `FileCache`, use + PSR-6 cache instead. + * **2016-05-27**: [BC BREAK] Removed `SeoPresentation::addExtractor()`, use + `ExtractorLoader::addExtractor()` instead. + * **2016-05-27**: [BC BREAK] Changed the fourth argument of the constructor of + `SeoPresentation` from `CacheInterface` to `LoaderInterface`. * **2016-05-27**: Added loaders. - * **2016-05-08**: [BC BREAK] Removed `showAtion` in favor of `listAction` in the `SuggestionProviderController` + * **2016-05-08**: [BC BREAK] Removed `showAtion` in favor of `listAction` in + the `SuggestionProviderController` * **2016-05-02**: [BC BREAK] Dropped PHP <5.5 support * **2016-05-02**: [BC BREAK] Dropped Symfony <2.8 support diff --git a/Cache/CacheInterface.php b/Cache/CacheInterface.php deleted file mode 100644 index 2d3554fa..00000000 --- a/Cache/CacheInterface.php +++ /dev/null @@ -1,38 +0,0 @@ - - */ -interface CacheInterface -{ - /** - * Fetches extractors from the cache. - * - * @param string $class - * - * @return ExtractorCollection|null - */ - public function loadExtractorsFromCache($class); - - /** - * Saves extractors into the cache. - * - * @param string $class - * @param array $extractors - */ - public function putExtractorsInCache($class, array $extractors); -} diff --git a/Cache/ExtractorCollection.php b/Cache/CachedCollection.php similarity index 58% rename from Cache/ExtractorCollection.php rename to Cache/CachedCollection.php index 443532cc..e2bfe7b6 100644 --- a/Cache/ExtractorCollection.php +++ b/Cache/CachedCollection.php @@ -12,16 +12,16 @@ namespace Symfony\Cmf\Bundle\SeoBundle\Cache; /** - * Contains the extractors for one particular content object. + * Contains the cached data for one particular content object. * * @author Wouter J */ -class ExtractorCollection implements \IteratorAggregate, \Serializable +class CachedCollection implements \IteratorAggregate, \Serializable { /** * @var array */ - private $extractors; + private $data; /** * @var null|string @@ -34,23 +34,53 @@ class ExtractorCollection implements \IteratorAggregate, \Serializable private $createdAt; /** - * @param array $extractors + * @param array $data * @param null|string $resource The path to the file of the content object, this is * used to determine if the cache needs to be updated */ - public function __construct(array $extractors, $resource = null) + public function __construct(array $data, $resource = null) { - $this->extractors = $extractors; + $this->data = $data; $this->resource = $resource; $this->createdAt = time(); } + /** + * Creates a CachedCollection based on the object and data to cache. + * + * @param object|string $objectOrClass Object instance or FQCN + * @param array $data + * + * @return static + */ + public static function createFromObject($objectOrClass, array $data) + { + $class = is_object($objectOrClass) ? get_class($objectOrClass) : $objectOrClass; + + static $fileLocations = []; + if (!isset($fileLocations[$class])) { + $fileLocations[$class] = (new \ReflectionClass($objectOrClass))->getFileName(); + } + + return new static($data, $fileLocations[$class]); + } + + public static function generateCacheItemKey($type, $class) + { + return sprintf('cmf_seo.%s.%s', $type, str_replace('\\', '.', $class)); + } + /** * {@inheritdoc} */ public function getIterator() { - return new \ArrayIterator($this->extractors); + return new \ArrayIterator($this->data); + } + + public function getData() + { + return $this->data; } /** @@ -83,7 +113,7 @@ public function isFresh($timestamp = null) public function serialize() { return serialize(array( - $this->extractors, + $this->data, $this->resource, $this->createdAt, )); @@ -95,7 +125,7 @@ public function serialize() public function unserialize($data) { list( - $this->extractors, + $this->data, $this->resource, $this->createdAt ) = unserialize($data); diff --git a/Cache/FileCache.php b/Cache/FileCache.php deleted file mode 100644 index 6199fb57..00000000 --- a/Cache/FileCache.php +++ /dev/null @@ -1,131 +0,0 @@ - - */ -class FileCache implements CacheInterface, CacheWarmerInterface, CacheClearerInterface -{ - private $dir; - - private $umask; - - public function __construct($baseDir, $dir, $umask = 0002) - { - if (!is_dir($baseDir)) { - throw new \InvalidArgumentException(sprintf('The directory "%s" does not exist.', $baseDir)); - } - if (!is_writable($baseDir)) { - throw new \InvalidArgumentException(sprintf('The directory "%s" is not writable.', $baseDir)); - } - - $this->dir = $baseDir.DIRECTORY_SEPARATOR.rtrim($dir, '\\/'); - - $this->umask = $umask; - - if (!is_dir($this->dir)) { - mkdir($this->dir, 0777 & ~$this->umask, true); - } - } - - /** - * {@inheritdoc} - */ - public function loadExtractorsFromCache($class) - { - $path = $this->dir.'/'.strtr($class, '\\', '-').'.cache.php'; - if (!file_exists($path)) { - return; - } - - return include $path; - } - - /** - * {@inheritdoc} - */ - public function putExtractorsInCache($class, array $extractors) - { - $path = $this->dir.'/'.strtr($class, '\\', '-').'.cache.php'; - - $reflection = new \ReflectionClass($class); - $extractors = new ExtractorCollection($extractors, $reflection->getFileName()); - - $tmpFile = tempnam($this->dir, 'metadata-cache'); - file_put_contents($tmpFile, 'umask); - - $this->renameFile($tmpFile, $path); - } - - /** - * Renames a file with fallback for windows. - * - * @param string $source - * @param string $target - * - * @throws \RuntimeException When the renaming can't be completed succesfully - * - * @author Johannes M. Schmitt - */ - private function renameFile($source, $target) - { - if (false === @rename($source, $target)) { - if (defined('PHP_WINDOWS_VERSION_BUILD')) { - if (false === copy($source, $target)) { - throw new \RuntimeException(sprintf('(WIN) Could not write new cache file to %s.', $target)); - } - if (false === unlink($source)) { - throw new \RuntimeException(sprintf('(WIN) Could not delete temp cache file to %s.', $source)); - } - } else { - throw new \RuntimeException(sprintf('Could not write new cache file to %s.', $target)); - } - } - } - - /** - * {@inheritdoc} - */ - public function isOptional() - { - return false; - } - - /** - * {@inheritdoc} - */ - public function warmUp($cacheDir) - { - if (!is_dir($this->dir)) { - mkdir($this->dir, 0775, true); - } - } - - /** - * {@inheritdoc} - */ - public function clear($cacheDir) - { - $filesystem = new Filesystem(); - if (is_dir($this->dir)) { - $filesystem->remove($this->dir); - } - } -} diff --git a/Loader/AnnotationLoader.php b/Loader/AnnotationLoader.php index 031a458a..b495033e 100644 --- a/Loader/AnnotationLoader.php +++ b/Loader/AnnotationLoader.php @@ -3,11 +3,9 @@ namespace Symfony\Cmf\Bundle\SeoBundle\Loader; use Doctrine\Common\Annotations\Reader; -use Symfony\Cmf\Bundle\SeoBundle\Loader\Annotation\MetaDescription; -use Symfony\Cmf\Bundle\SeoBundle\Loader\Annotation\MetaKeywords; +use Psr\Cache\CacheItemPoolInterface; +use Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection; use Symfony\Cmf\Bundle\SeoBundle\Loader\Annotation\SeoMetadataAnnotation; -use Symfony\Cmf\Bundle\SeoBundle\Loader\Annotation\Title; -use Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadataInterface; use Symfony\Component\Config\Loader\Loader; /** @@ -20,9 +18,15 @@ class AnnotationLoader extends Loader */ private $reader; - public function __construct(Reader $reader) + /** + * @var null|CacheItemPoolInterface + */ + private $cache; + + public function __construct(Reader $reader, CacheItemPoolInterface $cache = null) { $this->reader = $reader; + $this->cache = $cache; } public function supports($resource, $type = null) @@ -34,9 +38,7 @@ public function load($content, $type = null) { $seoMetadata = SeoMetadataFactory::initializeSeoMetadata($content); - $data = $this->getAnnotationsFromContent($content); - - // todo cache $data + $data = $this->getAnnotationForContent($content); $classReflection = new \ReflectionClass($content); foreach ($data['properties'] as $propertyName => $annotations) { @@ -59,7 +61,26 @@ public function load($content, $type = null) return $seoMetadata; } - private function getAnnotationsFromContent($content) + private function getAnnotationForContent($content) + { + $cachingAvailable = (bool) $this->cache; + + if (!$cachingAvailable) { + return $this->readAnnotations($content); + } + + $annotationsItem = $this->cache->getItem(CachedCollection::generateCacheItemKey('annotations', get_class($content))); + + if (!$annotationsItem->isHit() || !$annotationsItem->get()->isFresh()) { + $annotationsItem->set(CachedCollection::createFromObject($content, $this->readAnnotations($content))); + + $this->cache->save($annotationsItem); + } + + return $annotationsItem->get()->getData(); + } + + private function readAnnotations($content) { $classReflection = new \ReflectionClass($content); @@ -125,6 +146,6 @@ private function readMethods(array $methods) private function isAnnotated($content) { - return 0 !== count(call_user_func_array('array_merge', $this->getAnnotationsFromContent($content))); + return 0 !== count(call_user_func_array('array_merge', $this->readAnnotationsFromContent($content))); } } diff --git a/Loader/ExtractorLoader.php b/Loader/ExtractorLoader.php index 48706d1f..39146ca1 100644 --- a/Loader/ExtractorLoader.php +++ b/Loader/ExtractorLoader.php @@ -2,8 +2,8 @@ namespace Symfony\Cmf\Bundle\SeoBundle\Loader; -use Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface; -use Symfony\Cmf\Bundle\SeoBundle\Cache\ExtractorCollection; +use Psr\Cache\CacheItemPoolInterface; +use Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection; use Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface; use Symfony\Component\Config\Loader\Loader; @@ -13,7 +13,7 @@ class ExtractorLoader extends Loader { /** - * @var null|CacheInterface + * @var null|CacheItemPoolInterface */ private $cache; @@ -23,9 +23,9 @@ class ExtractorLoader extends Loader private $extractors = array(); /** - * @param CacheInterface $cache + * @param CacheItemPoolInterface $cache */ - public function __construct(CacheInterface $cache = null) + public function __construct(CacheItemPoolInterface $cache = null) { $this->cache = $cache; } @@ -75,25 +75,28 @@ public function load($content, $type = null) * * @param object $content * - * @return ExtractorCollection + * @return CachedCollection */ private function getExtractorsForContent($content) { $cachingAvailable = (bool) $this->cache; if (!$cachingAvailable) { - return new ExtractorCollection($this->findExtractorsForContent($content)); + return $this->findExtractorsForContent($content); } - $extractors = $this->cache->loadExtractorsFromCache(get_class($content)); + $extractorsItem = $this->cache->getItem( + CachedCollection::generateCacheItemKey('extractors', get_class($content)) + ); - if (null === $extractors || !$extractors->isFresh()) { - $extractors = $this->findExtractorsForContent($content); - $this->cache->putExtractorsInCache(get_class($content), $extractors); - $extractors = new ExtractorCollection($extractors); + // regenerate cache if needed + if (!$extractorsItem->isHit() || !$extractorsItem->get()->isFresh()) { + $extractorsItem->set($this->findExtractorsForContent($content)); + + $this->cache->save($extractorsItem); } - return $extractors; + return $extractorsItem->get(); } /** @@ -101,7 +104,7 @@ private function getExtractorsForContent($content) * * @param object $content * - * @return ExtractorInterface[] + * @return CachedCollection */ private function findExtractorsForContent($content) { @@ -115,7 +118,7 @@ private function findExtractorsForContent($content) $extractors = array_merge($extractors, $supportedExtractors); } - return $extractors; + return CachedCollection::createFromObject($content, $extractors); } /** diff --git a/Tests/Unit/Cache/FileCacheTest.php b/Tests/Unit/Cache/FileCacheTest.php deleted file mode 100644 index da66d7dd..00000000 --- a/Tests/Unit/Cache/FileCacheTest.php +++ /dev/null @@ -1,49 +0,0 @@ - - */ -class FileCacheTest extends \PHPUnit_Framework_TestCase -{ - public function testThrowingExceptionForUnknownBaseDir() - { - $thrown = false; - $baseDir = __DIR__.'/unknown'; - $dir = 'test'; - - try { - new FileCache($baseDir, $dir); - } catch (\InvalidArgumentException $e) { - $message = $e->getMessage(); - $thrown = true; - } - - $this->assertTrue($thrown); - $this->assertEquals($message, sprintf('The directory "%s" does not exist.', $baseDir)); - } - - public function testDirectoryCreation() - { - $baseDir = __DIR__.'/base'; - $dir = 'test'; - mkdir($baseDir); - new FileCache($baseDir, $dir); - - $expectedDirectory = $baseDir.'/'.$dir; - $this->assertTrue(is_dir($expectedDirectory)); - - rmdir($expectedDirectory); - rmdir($baseDir); - } -} diff --git a/Tests/Unit/Loader/ExtractorLoaderTest.php b/Tests/Unit/Loader/ExtractorLoaderTest.php index 00c62f19..a7f69816 100644 --- a/Tests/Unit/Loader/ExtractorLoaderTest.php +++ b/Tests/Unit/Loader/ExtractorLoaderTest.php @@ -129,7 +129,7 @@ public function testDescriptionExtractorsWithPriority() public function testCaching() { // promises - $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\ExtractorCollection') + $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection') ->disableOriginalConstructor() ->getMock(); $extractors @@ -142,18 +142,24 @@ public function testCaching() ->method('getIterator') ->will($this->returnValue(new \ArrayIterator())) ; - $cache = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface'); + $cacheItemNoHit = $this->getMock('Psr\Cache\CacheItemInterface'); + $cacheItemNoHit->expects($this->any())->method('isHit')->will($this->returnValue(false)); + $cacheItemNoHit->expects($this->any())->method('get')->will($this->returnValue($extractors)); + $cacheItemHit = $this->getMock('Psr\Cache\CacheItemInterface'); + $cacheItemHit->expects($this->any())->method('isHit')->will($this->returnValue(true)); + $cacheItemHit->expects($this->any())->method('get')->will($this->returnValue($extractors)); + $cache = $this->getMock('Psr\Cache\CacheItemPoolInterface'); $cache ->expects($this->any()) - ->method('loadExtractorsFromCache') - ->will($this->onConsecutiveCalls(null, $extractors)) + ->method('getItem') + ->will($this->onConsecutiveCalls($cacheItemNoHit, $cacheItemHit)) ; $loader = new ExtractorLoader($cache); // predictions $cache ->expects($this->once()) - ->method('putExtractorsInCache') + ->method('save') ; $loader->load($this->content); @@ -163,7 +169,7 @@ public function testCaching() public function testCacheRefresh() { // promises - $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\ExtractorCollection') + $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection') ->disableOriginalConstructor() ->getMock(); $extractors @@ -176,18 +182,21 @@ public function testCacheRefresh() ->method('getIterator') ->will($this->returnValue(new \ArrayIterator())) ; - $cache = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Cache\CacheInterface'); + $cacheItem = $this->getMock('Psr\Cache\CacheItemInterface'); + $cacheItem->expects($this->any())->method('isHit')->will($this->returnValue(true)); + $cacheItem->expects($this->any())->method('get')->will($this->returnValue($extractors)); + $cache = $this->getMock('Psr\Cache\CacheItemPoolInterface'); $cache ->expects($this->any()) - ->method('loadExtractorsFromCache') - ->will($this->returnValue($extractors)) + ->method('getItem') + ->will($this->returnValue($cacheItem)) ; $loader = new ExtractorLoader($cache); // predictions $cache ->expects($this->once()) - ->method('putExtractorsInCache') + ->method('save') ; $loader->load($this->content); diff --git a/composer.json b/composer.json index 52bed3ee..0c108310 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,7 @@ "doctrine/annotations": "^1.2" }, "require-dev": { + "symfony/cache": "^3.1", "symfony-cmf/testing": "^2.0", "doctrine/phpcr-odm": "^2.0", "doctrine/orm": "^2.4", From 9b2298186bc23ba698eb4590530c85fb71b5ecd0 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Thu, 2 Jun 2016 09:56:10 +0200 Subject: [PATCH 4/9] Symfony 3.1 is now stable --- .travis.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 02e1ed84..7228ea33 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,18 +14,22 @@ cache: env: matrix: - - SYMFONY_VERSION=2.8.* + - SYMFONY_VERSION=3.1.* global: - SYMFONY_DEPRECATIONS_HELPER=weak matrix: include: - - php: 5.6 - env: DEPS=dev SYMFONY_VERSION=3.1.* + - php: 7.0 + env: DEPS=dev SYMFONY_VERSION=3.2.* - php: 5.5 env: COMPOSER_FLAGS="--prefer-lowest" - - php: 5.6 + - php: 7.0 + env: SYMFONY_VERSION=2.8.* + - php: 7.0 env: DEPS=dev COMPOSER_FLAGS="--prefer-stable" SYMFONY_VERSION=3.0.* + - php: 7.0 + env: DEPS=dev COMPOSER_FLAGS="--prefer-stable" SYMFONY_VERSION=3.1.* fast_finish: true before_install: From 36c95e25d783268acc2c4318944921d0f871d294 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Thu, 2 Jun 2016 10:01:34 +0200 Subject: [PATCH 5/9] Fix CS --- Cache/CachedCollection.php | 4 ++-- Loader/Annotation/Extras.php | 9 +++++++++ Loader/Annotation/MetaDescription.php | 9 +++++++++ Loader/Annotation/MetaKeywords.php | 9 +++++++++ Loader/Annotation/OriginalUrl.php | 9 +++++++++ Loader/Annotation/SeoMetadataAnnotation.php | 9 +++++++++ Loader/Annotation/Title.php | 9 +++++++++ Loader/AnnotationLoader.php | 11 ++++++++++- Loader/ExtractorLoader.php | 9 +++++++++ Loader/SeoMetadataFactory.php | 9 +++++++++ Tests/Functional/Loader/LoaderTest.php | 9 +++++++++ Tests/Resources/Document/MethodAnnotatedContent.php | 9 +++++++++ .../Resources/Document/PropertyAnnotatedContent.php | 9 +++++++++ Tests/Unit/Loader/BaseAnnotationLoaderTest.php | 10 +++++++++- Tests/Unit/Loader/ExtractorLoaderTest.php | 11 ++++++++++- Tests/Unit/Loader/Method_AnnotationLoaderTest.php | 12 +++++++++++- Tests/Unit/Loader/Property_AnnotationLoaderTest.php | 9 +++++++++ Tests/Unit/Loader/SeoMetadataFactoryTest.php | 9 +++++++++ Tests/Unit/SeoPresentationTest.php | 5 ++--- 19 files changed, 161 insertions(+), 9 deletions(-) diff --git a/Cache/CachedCollection.php b/Cache/CachedCollection.php index e2bfe7b6..1eb69394 100644 --- a/Cache/CachedCollection.php +++ b/Cache/CachedCollection.php @@ -35,8 +35,8 @@ class CachedCollection implements \IteratorAggregate, \Serializable /** * @param array $data - * @param null|string $resource The path to the file of the content object, this is - * used to determine if the cache needs to be updated + * @param null|string $resource The path to the file of the content object, this is + * used to determine if the cache needs to be updated */ public function __construct(array $data, $resource = null) { diff --git a/Loader/Annotation/Extras.php b/Loader/Annotation/Extras.php index cd7f65c2..6a5338d4 100644 --- a/Loader/Annotation/Extras.php +++ b/Loader/Annotation/Extras.php @@ -1,5 +1,14 @@ $this->readProperties($classReflection->getProperties()), - 'methods' => $this->readMethods($classReflection->getMethods()), + 'methods' => $this->readMethods($classReflection->getMethods()), ]; } diff --git a/Loader/ExtractorLoader.php b/Loader/ExtractorLoader.php index 39146ca1..9f7172fe 100644 --- a/Loader/ExtractorLoader.php +++ b/Loader/ExtractorLoader.php @@ -1,5 +1,14 @@ diff --git a/Tests/Unit/Loader/ExtractorLoaderTest.php b/Tests/Unit/Loader/ExtractorLoaderTest.php index a7f69816..f1d38b53 100644 --- a/Tests/Unit/Loader/ExtractorLoaderTest.php +++ b/Tests/Unit/Loader/ExtractorLoaderTest.php @@ -1,5 +1,14 @@ loader = new ExtractorLoader(); - $this->content = new \stdClass; + $this->content = new \stdClass(); } public function testExtractors() diff --git a/Tests/Unit/Loader/Method_AnnotationLoaderTest.php b/Tests/Unit/Loader/Method_AnnotationLoaderTest.php index 027b87ca..b7c86bff 100644 --- a/Tests/Unit/Loader/Method_AnnotationLoaderTest.php +++ b/Tests/Unit/Loader/Method_AnnotationLoaderTest.php @@ -1,5 +1,14 @@ pageService = $this->getMock('Sonata\SeoBundle\Seo\SeoPage'); $this->translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface'); $this->seoMetadata = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadata'); - $this->content = new \stdClass; + $this->content = new \stdClass(); $this->loader = $this->getMock(LoaderInterface::class); $this->loader @@ -80,7 +79,7 @@ public function testDefaultTitle() ; // test - $this->seoPresentation->updateSeoPage(new \stdClass); + $this->seoPresentation->updateSeoPage(new \stdClass()); } public function testContentTitle() From 8f4618d7e50c597acc237010aa7ae3aa8c1ffd65 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Sat, 18 Jun 2016 12:23:08 +0200 Subject: [PATCH 6/9] Added cache tests for AnnotationLoader --- .travis.yml | 8 +- Resources/config/loaders.xml | 3 +- Resources/config/services.xml | 6 +- .../Unit/Loader/BaseAnnotationLoaderTest.php | 76 +++++++++++++++++++ Tests/Unit/SeoPresentationTest.php | 20 ----- 5 files changed, 85 insertions(+), 28 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7228ea33..9b45484a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,22 +14,18 @@ cache: env: matrix: - - SYMFONY_VERSION=3.1.* + - SYMFONY_VERSION=2.8.* global: - SYMFONY_DEPRECATIONS_HELPER=weak matrix: include: - php: 7.0 - env: DEPS=dev SYMFONY_VERSION=3.2.* + env: DEPS=dev SYMFONY_VERSION=3.1.* - php: 5.5 env: COMPOSER_FLAGS="--prefer-lowest" - - php: 7.0 - env: SYMFONY_VERSION=2.8.* - php: 7.0 env: DEPS=dev COMPOSER_FLAGS="--prefer-stable" SYMFONY_VERSION=3.0.* - - php: 7.0 - env: DEPS=dev COMPOSER_FLAGS="--prefer-stable" SYMFONY_VERSION=3.1.* fast_finish: true before_install: diff --git a/Resources/config/loaders.xml b/Resources/config/loaders.xml index e6610018..2fa6d383 100644 --- a/Resources/config/loaders.xml +++ b/Resources/config/loaders.xml @@ -18,13 +18,14 @@ - + + diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 56332cbc..87c1b0a9 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -27,7 +27,11 @@ - + + cmf_seo + + + diff --git a/Tests/Unit/Loader/BaseAnnotationLoaderTest.php b/Tests/Unit/Loader/BaseAnnotationLoaderTest.php index 177571a7..ec8d4705 100644 --- a/Tests/Unit/Loader/BaseAnnotationLoaderTest.php +++ b/Tests/Unit/Loader/BaseAnnotationLoaderTest.php @@ -100,4 +100,80 @@ public function testExtrasAnnotationWithScalar() $this->loader->load($content); } + + public function testCaching() + { + // promises + $annotations = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection') + ->disableOriginalConstructor() + ->getMock(); + $annotations + ->expects($this->any()) + ->method('isFresh') + ->will($this->returnValue(true)) + ; + $annotations + ->expects($this->any()) + ->method('getData') + ->will($this->returnValue(['properties' => [], 'methods' => []])) + ; + $cacheItemNoHit = $this->getMock('Psr\Cache\CacheItemInterface'); + $cacheItemNoHit->expects($this->any())->method('isHit')->will($this->returnValue(false)); + $cacheItemNoHit->expects($this->any())->method('get')->will($this->returnValue($annotations)); + $cacheItemHit = $this->getMock('Psr\Cache\CacheItemInterface'); + $cacheItemHit->expects($this->any())->method('isHit')->will($this->returnValue(true)); + $cacheItemHit->expects($this->any())->method('get')->will($this->returnValue($annotations)); + $cache = $this->getMock('Psr\Cache\CacheItemPoolInterface'); + $cache + ->expects($this->any()) + ->method('getItem') + ->will($this->onConsecutiveCalls($cacheItemNoHit, $cacheItemHit)) + ; + $loader = new AnnotationLoader(new AnnotationReader(), $cache); + + // predictions + $cache + ->expects($this->once()) + ->method('save') + ; + + $loader->load($this->getContent()); + $loader->load($this->getContent()); + } + + public function testCacheRefresh() + { + // promises + $annotations = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection') + ->disableOriginalConstructor() + ->getMock(); + $annotations + ->expects($this->any()) + ->method('isFresh') + ->will($this->returnValue(false)) + ; + $annotations + ->expects($this->any()) + ->method('getData') + ->will($this->returnValue(['properties' => [], 'methods' => []])) + ; + $cacheItem = $this->getMock('Psr\Cache\CacheItemInterface'); + $cacheItem->expects($this->any())->method('isHit')->will($this->returnValue(true)); + $cacheItem->expects($this->any())->method('get')->will($this->returnValue($annotations)); + $cache = $this->getMock('Psr\Cache\CacheItemPoolInterface'); + $cache + ->expects($this->any()) + ->method('getItem') + ->will($this->returnValue($cacheItem)) + ; + $loader = new AnnotationLoader(new AnnotationReader(), $cache); + + // predictions + $cache + ->expects($this->once()) + ->method('save') + ; + + $loader->load($this->getContent()); + } } diff --git a/Tests/Unit/SeoPresentationTest.php b/Tests/Unit/SeoPresentationTest.php index a1d19512..5f03b3fe 100644 --- a/Tests/Unit/SeoPresentationTest.php +++ b/Tests/Unit/SeoPresentationTest.php @@ -197,24 +197,4 @@ public function testRedirect() $this->assertInstanceOf('Symfony\Component\HttpFoundation\RedirectResponse', $redirect); $this->assertEquals('/redirect/target', $redirect->getTargetUrl()); } - - public function testSeoAwareWithoutCurrentMetadata() - { - $content = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Tests\Resources\Document\SeoAwareContent'); - $content - ->expects($this->any()) - ->method('getSeoMetadata') - ->will($this->returnValue(null)) - ; - - $content - ->expects($this->once()) - ->method('setSeoMetadata') - ->with($this->callback(function ($c) { - return $c instanceof SeoMetadataInterface; - })) - ; - - $this->seoPresentation->updateSeoPage($content); - } } From 0f04987553a4cb82c9a9fb01ae01c86c7b5f730e Mon Sep 17 00:00:00 2001 From: WouterJ Date: Fri, 12 Aug 2016 00:10:50 +0200 Subject: [PATCH 7/9] Removed unused test fixtures --- Tests/Resources/Fixtures/loader_config/empty.yml | 0 Tests/Resources/Fixtures/loader_config/valid1.yml | 7 ------- 2 files changed, 7 deletions(-) delete mode 100644 Tests/Resources/Fixtures/loader_config/empty.yml delete mode 100644 Tests/Resources/Fixtures/loader_config/valid1.yml diff --git a/Tests/Resources/Fixtures/loader_config/empty.yml b/Tests/Resources/Fixtures/loader_config/empty.yml deleted file mode 100644 index e69de29b..00000000 diff --git a/Tests/Resources/Fixtures/loader_config/valid1.yml b/Tests/Resources/Fixtures/loader_config/valid1.yml deleted file mode 100644 index c9672020..00000000 --- a/Tests/Resources/Fixtures/loader_config/valid1.yml +++ /dev/null @@ -1,7 +0,0 @@ -Symfony\Cmf\Bundle\SeoBundle\Tests\Resources\Document\ContentWithExtractors: - properties: - title: { type: title } - description: { type: description } - - methods: - get From 624327b4e8ff34b41a93fb1265d8933bd584b201 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Fri, 12 Aug 2016 14:49:14 +0200 Subject: [PATCH 8/9] Use Prophecy in new tests --- .../Unit/Loader/BaseAnnotationLoaderTest.php | 93 ++++----- Tests/Unit/Loader/ExtractorLoaderTest.php | 194 +++++------------- Tests/Unit/Loader/SeoMetadataFactoryTest.php | 47 +++-- 3 files changed, 124 insertions(+), 210 deletions(-) diff --git a/Tests/Unit/Loader/BaseAnnotationLoaderTest.php b/Tests/Unit/Loader/BaseAnnotationLoaderTest.php index ec8d4705..2c441fb2 100644 --- a/Tests/Unit/Loader/BaseAnnotationLoaderTest.php +++ b/Tests/Unit/Loader/BaseAnnotationLoaderTest.php @@ -13,6 +13,10 @@ use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Cmf\Bundle\SeoBundle\Loader\AnnotationLoader; +use Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection; +use Prophecy\Argument; +use Psr\Cache\CacheItemInterface; +use Psr\Cache\CacheItemPoolInterface; /** * @author Wouter de Jong @@ -104,38 +108,25 @@ public function testExtrasAnnotationWithScalar() public function testCaching() { // promises - $annotations = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection') - ->disableOriginalConstructor() - ->getMock(); - $annotations - ->expects($this->any()) - ->method('isFresh') - ->will($this->returnValue(true)) - ; - $annotations - ->expects($this->any()) - ->method('getData') - ->will($this->returnValue(['properties' => [], 'methods' => []])) - ; - $cacheItemNoHit = $this->getMock('Psr\Cache\CacheItemInterface'); - $cacheItemNoHit->expects($this->any())->method('isHit')->will($this->returnValue(false)); - $cacheItemNoHit->expects($this->any())->method('get')->will($this->returnValue($annotations)); - $cacheItemHit = $this->getMock('Psr\Cache\CacheItemInterface'); - $cacheItemHit->expects($this->any())->method('isHit')->will($this->returnValue(true)); - $cacheItemHit->expects($this->any())->method('get')->will($this->returnValue($annotations)); - $cache = $this->getMock('Psr\Cache\CacheItemPoolInterface'); - $cache - ->expects($this->any()) - ->method('getItem') - ->will($this->onConsecutiveCalls($cacheItemNoHit, $cacheItemHit)) - ; - $loader = new AnnotationLoader(new AnnotationReader(), $cache); + $annotations = $this->prophesize(CachedCollection::class); + $annotations->isFresh()->willReturn(true); + $annotations->getData()->willReturn(['properties' => [], 'methods' => []]); + + $cacheItemProphet = $this->prophesize(CacheItemInterface::class); + $cacheItemProphet->isHit()->willReturn(false); + $cacheItemProphet->set(Argument::type(CachedCollection::class))->will(function () { + $this->isHit()->willReturn(true); + }); + $cacheItemProphet->get()->willReturn($annotations->reveal()); + $cacheItem = $cacheItemProphet->reveal(); + + $cache = $this->prophesize(CacheItemPoolInterface::class); + $cache->getItem('cmf_seo.annotations.'.str_replace('\\', '.', get_class($this->getContent())))->willReturn($cacheItem); + + $loader = new AnnotationLoader(new AnnotationReader(), $cache->reveal()); // predictions - $cache - ->expects($this->once()) - ->method('save') - ; + $cache->save($cacheItem)->shouldBeCalledTimes(1); $loader->load($this->getContent()); $loader->load($this->getContent()); @@ -144,35 +135,23 @@ public function testCaching() public function testCacheRefresh() { // promises - $annotations = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection') - ->disableOriginalConstructor() - ->getMock(); - $annotations - ->expects($this->any()) - ->method('isFresh') - ->will($this->returnValue(false)) - ; - $annotations - ->expects($this->any()) - ->method('getData') - ->will($this->returnValue(['properties' => [], 'methods' => []])) - ; - $cacheItem = $this->getMock('Psr\Cache\CacheItemInterface'); - $cacheItem->expects($this->any())->method('isHit')->will($this->returnValue(true)); - $cacheItem->expects($this->any())->method('get')->will($this->returnValue($annotations)); - $cache = $this->getMock('Psr\Cache\CacheItemPoolInterface'); - $cache - ->expects($this->any()) - ->method('getItem') - ->will($this->returnValue($cacheItem)) - ; - $loader = new AnnotationLoader(new AnnotationReader(), $cache); + $annotations = $this->prophesize(CachedCollection::class); + $annotations->isFresh()->willReturn(false); + $annotations->getData()->willReturn(['properties' => [], 'methods' => []]); + + $cacheItemProphet = $this->prophesize(CacheItemInterface::class); + $cacheItemProphet->isHit()->willReturn(true); + $cacheItemProphet->get()->willReturn($annotations->reveal()); + $cacheItem = $cacheItemProphet->reveal(); + + $cache = $this->prophesize(CacheItemPoolInterface::class); + $cache->getItem('cmf_seo.annotations.'.str_replace('\\', '.', get_class($this->getContent())))->willReturn($cacheItem); + + $loader = new AnnotationLoader(new AnnotationReader(), $cache->reveal()); // predictions - $cache - ->expects($this->once()) - ->method('save') - ; + $cacheItemProphet->set(Argument::type(CachedCollection::class))->shouldBeCalled(); + $cache->save($cacheItem)->shouldBeCalled(); $loader->load($this->getContent()); } diff --git a/Tests/Unit/Loader/ExtractorLoaderTest.php b/Tests/Unit/Loader/ExtractorLoaderTest.php index f1d38b53..49ee989b 100644 --- a/Tests/Unit/Loader/ExtractorLoaderTest.php +++ b/Tests/Unit/Loader/ExtractorLoaderTest.php @@ -11,8 +11,13 @@ namespace Symfony\Cmf\Bundle\SeoBundle\Tests\Unit\Loader; +use Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection; +use Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface; use Symfony\Cmf\Bundle\SeoBundle\Loader\ExtractorLoader; use Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadataInterface; +use Prophecy\Argument; +use Psr\Cache\CacheItemInterface; +use Psr\Cache\CacheItemPoolInterface; /** * @author Wouter de Jong @@ -34,142 +39,65 @@ protected function setUp() public function testExtractors() { // promises - $extractor = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractor - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $this->loader->addExtractor($extractor); + $extractor = $this->prophesize(ExtractorInterface::class); + $extractor->supports($this->content)->willReturn(true); + + $this->loader->addExtractor($extractor->reveal()); // predictions - $extractor - ->expects($this->once()) - ->method('updateMetadata') - ; + $extractor->updateMetadata($this->content, Argument::type(SeoMetadataInterface::class))->shouldBeCalled(); // test $this->loader->load($this->content); } - public function testTitleExtractorsWithPriority() + public function testExtractorsWithPriority() { // promises - $extractorDefault = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractorDefault - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $extractorOne = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractorOne - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $this->loader->addExtractor($extractorDefault); - $this->loader->addExtractor($extractorOne, 1); + $extractorDefault = $this->prophesize(ExtractorInterface::class); + $extractorDefault->supports($this->content)->willReturn(true); + + $extractorOne = $this->prophesize(ExtractorInterface::class); + $extractorOne->supports($this->content)->willReturn(true); + + $this->loader->addExtractor($extractorDefault->reveal()); + $this->loader->addExtractor($extractorOne->reveal(), 1); // predictions - $extractorDefault - ->expects($this->once()) - ->method('updateMetadata') - ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { - $seoMetadata->setTitle('First Title'); - })) - ; - $extractorOne - ->expects($this->once()) - ->method('updateMetadata') - ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { - $seoMetadata->setTitle('Final Title'); - })) - ; + $extractorDefault->updateMetadata(Argument::cetera())->will(function ($arguments) { + $arguments[1]->setTitle('First Title'); + }); + $extractorOne->updateMetadata(Argument::cetera())->will(function ($arguments) { + $arguments[1]->setTitle('Final Title'); + }); // test $seoMetadata = $this->loader->load($this->content); $this->assertEquals('Final Title', $seoMetadata->getTitle()); } - public function testDescriptionExtractorsWithPriority() + public function testCaching() { // promises - $extractorDefault = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractorDefault - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $extractorOne = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Extractor\ExtractorInterface'); - $extractorOne - ->expects($this->any()) - ->method('supports') - ->with($this->content) - ->will($this->returnValue(true)) - ; - $this->loader->addExtractor($extractorDefault); - $this->loader->addExtractor($extractorOne, 1); + $extractors = $this->prophesize(CachedCollection::class); + $extractors->isFresh()->willReturn(true); + $extractors->getIterator()->willReturn(new \ArrayIterator()); - // predictions - $extractorDefault - ->expects($this->once()) - ->method('updateMetadata') - ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { - $seoMetadata->setMetaDescription('First Description'); - })) - ; - $extractorOne - ->expects($this->once()) - ->method('updateMetadata') - ->will($this->returnCallback(function ($content, SeoMetadataInterface $seoMetadata) { - $seoMetadata->setMetaDescription('Final Description'); - })) - ; + $cacheItemProphet = $this->prophesize(CacheItemInterface::class); + $cacheItemProphet->isHit()->willReturn(false); + $cacheItemProphet->set(Argument::type(CachedCollection::class))->will(function () { + $this->isHit()->willReturn(true); + }); + $cacheItemProphet->get()->willReturn($extractors->reveal()); + $cacheItem = $cacheItemProphet->reveal(); - // test - $seoMetadata = $this->loader->load($this->content); - $this->assertEquals('Final Description', $seoMetadata->getMetaDescription()); - } + $cache = $this->prophesize(CacheItemPoolInterface::class); + $cache->getItem('cmf_seo.extractors.stdClass')->willReturn($cacheItem); - public function testCaching() - { - // promises - $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection') - ->disableOriginalConstructor() - ->getMock(); - $extractors - ->expects($this->any()) - ->method('isFresh') - ->will($this->returnValue(true)) - ; - $extractors - ->expects($this->any()) - ->method('getIterator') - ->will($this->returnValue(new \ArrayIterator())) - ; - $cacheItemNoHit = $this->getMock('Psr\Cache\CacheItemInterface'); - $cacheItemNoHit->expects($this->any())->method('isHit')->will($this->returnValue(false)); - $cacheItemNoHit->expects($this->any())->method('get')->will($this->returnValue($extractors)); - $cacheItemHit = $this->getMock('Psr\Cache\CacheItemInterface'); - $cacheItemHit->expects($this->any())->method('isHit')->will($this->returnValue(true)); - $cacheItemHit->expects($this->any())->method('get')->will($this->returnValue($extractors)); - $cache = $this->getMock('Psr\Cache\CacheItemPoolInterface'); - $cache - ->expects($this->any()) - ->method('getItem') - ->will($this->onConsecutiveCalls($cacheItemNoHit, $cacheItemHit)) - ; - $loader = new ExtractorLoader($cache); + $loader = new ExtractorLoader($cache->reveal()); // predictions - $cache - ->expects($this->once()) - ->method('save') - ; + $cache->save($cacheItem)->shouldBeCalledTimes(1); $loader->load($this->content); $loader->load($this->content); @@ -178,35 +106,23 @@ public function testCaching() public function testCacheRefresh() { // promises - $extractors = $this->getMockBuilder('Symfony\Cmf\Bundle\SeoBundle\Cache\CachedCollection') - ->disableOriginalConstructor() - ->getMock(); - $extractors - ->expects($this->any()) - ->method('isFresh') - ->will($this->returnValue(false)) - ; - $extractors - ->expects($this->any()) - ->method('getIterator') - ->will($this->returnValue(new \ArrayIterator())) - ; - $cacheItem = $this->getMock('Psr\Cache\CacheItemInterface'); - $cacheItem->expects($this->any())->method('isHit')->will($this->returnValue(true)); - $cacheItem->expects($this->any())->method('get')->will($this->returnValue($extractors)); - $cache = $this->getMock('Psr\Cache\CacheItemPoolInterface'); - $cache - ->expects($this->any()) - ->method('getItem') - ->will($this->returnValue($cacheItem)) - ; - $loader = new ExtractorLoader($cache); + $extractors = $this->prophesize(CachedCollection::class); + $extractors->isFresh()->willReturn(false); + $extractors->getIterator()->willReturn(new \ArrayIterator()); + + $cacheItemProphet = $this->prophesize(CacheItemInterface::class); + $cacheItemProphet->isHit()->willReturn(true); + $cacheItemProphet->get()->willReturn($extractors->reveal()); + $cacheItem = $cacheItemProphet->reveal(); + + $cache = $this->prophesize(CacheItemPoolInterface::class); + $cache->getItem('cmf_seo.extractors.stdClass')->willReturn($cacheItem); + + $loader = new ExtractorLoader($cache->reveal()); // predictions - $cache - ->expects($this->once()) - ->method('save') - ; + $cacheItemProphet->set(Argument::type(CachedCollection::class))->shouldBeCalled(); + $cache->save($cacheItem)->shouldBeCalled(); $loader->load($this->content); } diff --git a/Tests/Unit/Loader/SeoMetadataFactoryTest.php b/Tests/Unit/Loader/SeoMetadataFactoryTest.php index 143389eb..51f89dc6 100644 --- a/Tests/Unit/Loader/SeoMetadataFactoryTest.php +++ b/Tests/Unit/Loader/SeoMetadataFactoryTest.php @@ -13,6 +13,8 @@ use Symfony\Cmf\Bundle\SeoBundle\Loader\SeoMetadataFactory; use Symfony\Cmf\Bundle\SeoBundle\Model\SeoMetadataInterface; +use Symfony\Cmf\Bundle\SeoBundle\SeoAwareInterface; +use Prophecy\Argument; /** * @author Wouter de Jong @@ -21,19 +23,36 @@ class SeoMetadataFactoryTest extends \PHPUnit_Framework_TestCase { public function testSeoAwareWithoutCurrentMetadata() { - $content = $this->getMock('Symfony\Cmf\Bundle\SeoBundle\Tests\Resources\Document\SeoAwareContent'); - $content - ->expects($this->any()) - ->method('getSeoMetadata') - ->will($this->returnValue(null)) - ; - - $content - ->expects($this->once()) - ->method('setSeoMetadata') - ->with($this->isInstanceOf(SeoMetadataInterface::class)) - ; - - SeoMetadataFactory::initializeSeoMetadata($content); + $content = $this->prophesize(SeoAwareInterface::class); + $content->getSeoMetadata()->willReturn(null); + + $content->setSeoMetadata(Argument::type(SeoMetadataInterface::class))->shouldBeCalled(); + + SeoMetadataFactory::initializeSeoMetadata($content->reveal()); + } + + public function testSeoAwareWithCurrentMetadata() + { + $seoMetadata = $this->prophesize(SeoMetadataInterface::class); + $seoMetadata->getTitle()->willReturn('Test'); + $seoMetadata->getMetaKeywords()->willReturn('test1, test2'); + $seoMetadata->getMetaDescription()->willReturn('Some copy test'); + $seoMetadata->getOriginalUrl()->willReturn('http://example.org/test'); + $seoMetadata->getExtraProperties()->willReturn(['og:title' => 'Test Extra']); + $seoMetadata->getExtraNames()->willReturn(['robots' => 'index, follow']); + $seoMetadata->getExtraHttp()->willReturn(['Content-Type' => 'text/html']); + + $content = $this->prophesize(SeoAwareInterface::class); + $content->getSeoMetadata()->willReturn($seoMetadata->reveal()); + + $returnedSeoMetadata = SeoMetadataFactory::initializeSeoMetadata($content->reveal()); + + $this->assertEquals('Test', $returnedSeoMetadata->getTitle()); + $this->assertEquals('test1, test2', $returnedSeoMetadata->getMetaKeywords()); + $this->assertEquals('Some copy test', $returnedSeoMetadata->getMetaDescription()); + $this->assertEquals('http://example.org/test', $returnedSeoMetadata->getOriginalUrl()); + $this->assertEquals(['og:title' => 'Test Extra'], $returnedSeoMetadata->getExtraProperties()); + $this->assertEquals(['robots' => 'index, follow'], $returnedSeoMetadata->getExtraNames()); + $this->assertEquals(['Content-Type' => 'text/html'], $returnedSeoMetadata->getExtraHttp()); } } From 05edab4012d167bf83c15542469805106802590c Mon Sep 17 00:00:00 2001 From: WouterJ Date: Fri, 12 Aug 2016 14:49:50 +0200 Subject: [PATCH 9/9] Minor typo fix --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3148acd0..432e8c59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,8 @@ Changelog * **2016-05-27**: [BC BREAK] Changed the fourth argument of the constructor of `SeoPresentation` from `CacheInterface` to `LoaderInterface`. * **2016-05-27**: Added loaders. - * **2016-05-08**: [BC BREAK] Removed `showAtion` in favor of `listAction` in - the `SuggestionProviderController` + * **2016-05-08**: [BC BREAK] Removed `showAction()` in favor of `listAction()` + in the `SuggestionProviderController` * **2016-05-02**: [BC BREAK] Dropped PHP <5.5 support * **2016-05-02**: [BC BREAK] Dropped Symfony <2.8 support