From 56a5a644d879319e1e4fae1a62821282b6800554 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Wed, 5 Apr 2017 15:00:45 +0200 Subject: [PATCH] added route-behavior to article-page (#131) --- DependencyInjection/SuluArticleExtension.php | 17 ++ Document/ArticlePageDocument.php | 72 ++++++++- Document/Behavior/PageBehavior.php | 37 +++++ Document/Subscriber/ArticlePageSubscriber.php | 33 ---- Document/Subscriber/PageSubscriber.php | 135 ++++++++++++++++ Document/Subscriber/RoutableSubscriber.php | 41 ++++- Resources/config/services.xml | 15 ++ Routing/ArticlePageRouteGenerator.php | 76 +++++++++ .../Subscriber/ArticlePageSubscriberTest.php | 29 ---- .../Subscriber/PageSubscriberTest.php | 145 ++++++++++++++++++ .../Subscriber/RoutableSubscriberTest.php | 35 ++++- .../Routing/ArticlePageRouteGeneratorTest.php | 77 ++++++++++ 12 files changed, 644 insertions(+), 68 deletions(-) create mode 100644 Document/Behavior/PageBehavior.php create mode 100644 Document/Subscriber/PageSubscriber.php create mode 100644 Routing/ArticlePageRouteGenerator.php create mode 100644 Tests/Unit/Document/Subscriber/PageSubscriberTest.php create mode 100644 Tests/Unit/Routing/ArticlePageRouteGeneratorTest.php diff --git a/DependencyInjection/SuluArticleExtension.php b/DependencyInjection/SuluArticleExtension.php index b70e1a782..1b9d39246 100644 --- a/DependencyInjection/SuluArticleExtension.php +++ b/DependencyInjection/SuluArticleExtension.php @@ -94,6 +94,23 @@ public function prepend(ContainerBuilder $container) ); } + if ($container->hasExtension('sulu_route')) { + $container->prependExtensionConfig( + 'sulu_route', + [ + 'mappings' => [ + ArticlePageDocument::class => [ + 'generator' => 'article_page', + 'options' => [ + 'route_schema' => '/{translator.trans("page")}-{object.getPageNumber()}', + 'parent' => '{object.getParent().getRoutePath()}', + ], + ], + ], + ] + ); + } + if ($container->hasExtension('fos_rest')) { $container->prependExtensionConfig( 'fos_rest', diff --git a/Document/ArticlePageDocument.php b/Document/ArticlePageDocument.php index 8c1d7ea06..bbc1c22b3 100644 --- a/Document/ArticlePageDocument.php +++ b/Document/ArticlePageDocument.php @@ -11,6 +11,9 @@ namespace Sulu\Bundle\ArticleBundle\Document; +use Sulu\Bundle\ArticleBundle\Document\Behavior\PageBehavior; +use Sulu\Bundle\ArticleBundle\Document\Behavior\RoutableBehavior; +use Sulu\Bundle\RouteBundle\Model\RouteInterface; use Sulu\Component\Content\Document\Behavior\StructureBehavior; use Sulu\Component\Content\Document\Structure\Structure; use Sulu\Component\Content\Document\Structure\StructureInterface; @@ -30,6 +33,8 @@ class ArticlePageDocument implements AutoNameBehavior, PathBehavior, StructureBehavior, + RoutableBehavior, + PageBehavior, ArticleInterface { /** @@ -72,6 +77,16 @@ class ArticlePageDocument implements */ private $structure; + /** + * @var RouteInterface + */ + private $route; + + /** + * @var string + */ + private $routePath; + /** * @var int */ @@ -207,15 +222,66 @@ public function getStructure() } /** - * Returns page. - * - * @return int + * {@inheritdoc} + */ + public function getId() + { + return $this->uuid; + } + + /** + * {@inheritdoc} + */ + public function getRoute() + { + return $this->route; + } + + /** + * {@inheritdoc} + */ + public function setRoute(RouteInterface $route) + { + $this->route = $route; + $this->routePath = $route->getPath(); + + return $this; + } + + /** + * {@inheritdoc} + */ + public function getRoutePath() + { + return $this->routePath; + } + + /** + * {@inheritdoc} + */ + public function setRoutePath($routePath) + { + $this->routePath = $routePath; + } + + /** + * {@inheritdoc} */ public function getPageNumber() { return $this->pageNumber; } + /** + * {@inheritdoc} + */ + public function setPageNumber($pageNumber) + { + $this->pageNumber = $pageNumber; + + return $this; + } + /** * {@inheritdoc} */ diff --git a/Document/Behavior/PageBehavior.php b/Document/Behavior/PageBehavior.php new file mode 100644 index 000000000..7035c18f1 --- /dev/null +++ b/Document/Behavior/PageBehavior.php @@ -0,0 +1,37 @@ + [ ['setTitleOnPersist', 2048], - ['setPageNumberOnPersist', 0], ['setStructureTypeToParent', -2048], ['setWorkflowStageOnArticle', -2048], ], - Events::HYDRATE => [['setPageNumberOnHydrate', 0]], ]; } @@ -150,21 +147,6 @@ private function getPageTitleProperty(ArticlePageDocument $document) return null; } - /** - * Set page-number to document on persist. - * - * @param PersistEvent $event - */ - public function setPageNumberOnPersist(PersistEvent $event) - { - $document = $event->getDocument(); - if (!$document instanceof ArticlePageDocument) { - return; - } - - $event->getAccessor()->set('pageNumber', $event->getNode()->getIndex() + 1); - } - /** * Set structure-type to parent document. * @@ -183,19 +165,4 @@ public function setStructureTypeToParent(PersistEvent $event) $document->getParent()->setStructureType($document->getStructureType()); $this->documentManager->persist($document->getParent(), $event->getLocale()); } - - /** - * Set page-number to document on persist. - * - * @param HydrateEvent $event - */ - public function setPageNumberOnHydrate(HydrateEvent $event) - { - $document = $event->getDocument(); - if (!$document instanceof ArticlePageDocument) { - return; - } - - $event->getAccessor()->set('pageNumber', $event->getNode()->getIndex() + 1); - } } diff --git a/Document/Subscriber/PageSubscriber.php b/Document/Subscriber/PageSubscriber.php new file mode 100644 index 000000000..74e7232d0 --- /dev/null +++ b/Document/Subscriber/PageSubscriber.php @@ -0,0 +1,135 @@ +documentInspector = $documentInspector; + $this->propertyEncoder = $propertyEncoder; + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() + { + return [ + Events::PERSIST => [['handlePersist', -1024]], + Events::REMOVE => [['handleRemove', 5]], + Events::METADATA_LOAD => 'handleMetadataLoad', + ]; + } + + /** + * Add page-number to metadata. + * + * @param MetadataLoadEvent $event + */ + public function handleMetadataLoad(MetadataLoadEvent $event) + { + $metadata = $event->getMetadata(); + + if (false === $metadata->getReflectionClass()->isSubclassOf(PageBehavior::class)) { + return; + } + + $metadata->addFieldMapping( + 'pageNumber', + [ + 'encoding' => 'system', + 'property' => self::FIELD, + ] + ); + } + + /** + * Set the page-number to new pages. + * + * @param PersistEvent $event + */ + public function handlePersist(PersistEvent $event) + { + $document = $event->getDocument(); + if (!$document instanceof PageBehavior || $document->getPageNumber()) { + return; + } + + $parentDocument = $document->getParent(); + + $page = 1; + foreach ($parentDocument->getChildren() as $child) { + if (!$child instanceof PageBehavior) { + continue; + } + + ++$page; + } + + $childNode = $this->documentInspector->getNode($document); + $childNode->setProperty($this->propertyEncoder->systemName(static::FIELD), $page); + } + + /** + * Adjust the page-numbers of siblings when removing a page. + * + * @param RemoveEvent $event + */ + public function handleRemove(RemoveEvent $event) + { + $document = $event->getDocument(); + if (!$document instanceof PageBehavior) { + return; + } + + $parentDocument = $document->getParent(); + + $page = 1; + foreach ($parentDocument->getChildren() as $child) { + if (!$child instanceof PageBehavior || $child->getUuid() === $document->getUuid()) { + continue; + } + + $childNode = $this->documentInspector->getNode($child); + $childNode->setProperty($this->propertyEncoder->systemName(static::FIELD), $page++); + } + } +} diff --git a/Document/Subscriber/RoutableSubscriber.php b/Document/Subscriber/RoutableSubscriber.php index 837bfa126..b3a05553c 100644 --- a/Document/Subscriber/RoutableSubscriber.php +++ b/Document/Subscriber/RoutableSubscriber.php @@ -19,6 +19,7 @@ use Sulu\Bundle\RouteBundle\Entity\RouteRepositoryInterface; use Sulu\Bundle\RouteBundle\Manager\RouteManagerInterface; use Sulu\Component\DocumentManager\DocumentManagerInterface; +use Sulu\Component\DocumentManager\Behavior\Mapping\ChildrenBehavior; use Sulu\Component\DocumentManager\Event\AbstractMappingEvent; use Sulu\Component\DocumentManager\Event\ConfigureOptionsEvent; use Sulu\Component\DocumentManager\Event\CopyEvent; @@ -96,7 +97,10 @@ public static function getSubscribedEvents() return [ Events::HYDRATE => [['handleHydrate', -500]], Events::PERSIST => [['handleRouteUpdate', 1], ['handleRoute', 0]], - Events::REMOVE => [['handleRemove', -500]], + Events::REMOVE => [ + // high priority to ensure nodes are not deleted until we iterate over children + ['handleRemove', 1024], + ], Events::COPY => ['handleCopy', -2000], Events::METADATA_LOAD => 'handleMetadataLoad', Events::CONFIGURE_OPTIONS => 'configureOptions', @@ -181,6 +185,11 @@ public function handleRemove(RemoveEvent $event) } $this->entityManager->remove($route); + + if ($document instanceof ChildrenBehavior) { + $this->removeChildRoutes($document); + } + $this->entityManager->flush(); } @@ -214,6 +223,36 @@ public function handleCopy(CopyEvent $event) $this->entityManager->flush(); } + /** + * Iterate over children and remove routes. + * + * @param ChildrenBehavior $document + */ + private function removeChildRoutes(ChildrenBehavior $document) + { + foreach ($document->getChildren() as $child) { + if ($child instanceof RoutableBehavior) { + $this->removeChildRoute($child); + } + + if ($child instanceof ChildrenBehavior) { + $this->removeChildRoutes($child); + } + } + } + + /** + * Removes route if exists. + * + * @param RoutableBehavior $document + */ + private function removeChildRoute(RoutableBehavior $document) + { + $route = $this->routeRepository->findByPath($document->getRoutePath(), $document->getOriginalLocale()); + if ($route) { + $this->entityManager->remove($route); + } + } /** * Add route to metadata. diff --git a/Resources/config/services.xml b/Resources/config/services.xml index adcfc9de9..ce24933ec 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -117,6 +117,13 @@ + + + + + + @@ -125,6 +132,7 @@ + @@ -170,6 +178,13 @@ + + + + + + routeGenerator = $routeGenerator; + $this->tokenProvider = $tokenProvider; + } + + /** + * {@inheritdoc} + */ + public function generate($entity, array $options) + { + $parent = $options['parent']; + + $tokens = []; + preg_match_all('/{(.*?)}/', $parent, $matches); + $tokenNames = $matches[1]; + + foreach ($tokenNames as $name) { + $tokenName = '{' . $name . '}'; + $tokenValue = $this->tokenProvider->provide($entity, $name); + + $tokens[$tokenName] = $tokenValue; + } + + $parentPath = strtr($parent, $tokens); + + return $this->routeGenerator->generate($entity, ['route_schema' => $parentPath . $options['route_schema']]); + } + + /** + * {@inheritdoc} + */ + public function getOptionsResolver(array $options) + { + return (new OptionsResolver())->setRequired(['route_schema', 'parent']); + } +} diff --git a/Tests/Unit/Document/Subscriber/ArticlePageSubscriberTest.php b/Tests/Unit/Document/Subscriber/ArticlePageSubscriberTest.php index 5aa36af32..02b2fbe01 100644 --- a/Tests/Unit/Document/Subscriber/ArticlePageSubscriberTest.php +++ b/Tests/Unit/Document/Subscriber/ArticlePageSubscriberTest.php @@ -11,7 +11,6 @@ namespace Sulu\Bundle\ArticleBundle\Tests\Unit\Document\Subscriber; -use PHPCR\NodeInterface; use Prophecy\Argument; use Sulu\Bundle\ArticleBundle\Document\ArticleDocument; use Sulu\Bundle\ArticleBundle\Document\ArticlePageDocument; @@ -24,9 +23,7 @@ use Sulu\Component\Content\Metadata\Factory\StructureMetadataFactoryInterface; use Sulu\Component\Content\Metadata\PropertyMetadata; use Sulu\Component\Content\Metadata\StructureMetadata; -use Sulu\Component\DocumentManager\DocumentAccessor; use Sulu\Component\DocumentManager\DocumentManagerInterface; -use Sulu\Component\DocumentManager\Event\HydrateEvent; use Sulu\Component\DocumentManager\Event\PersistEvent; class ArticlePageSubscriberTest extends \PHPUnit_Framework_TestCase @@ -206,32 +203,6 @@ public function testSetTitleOnPersistWithoutTagAndProperty() $this->subscriber->setTitleOnPersist($event); } - public function testSetPageNumberOnPersist() - { - $node = $this->prophesize(NodeInterface::class); - $node->getIndex()->willReturn(1); - - $accessor = $this->prophesize(DocumentAccessor::class); - $accessor->set('pageNumber', 2)->shouldBeCalled(); - - $event = $this->createEvent(PersistEvent::class, $node->reveal(), $accessor->reveal()); - - $this->subscriber->setPageNumberOnPersist($event); - } - - public function testSetPageNumberOnHydrate() - { - $node = $this->prophesize(NodeInterface::class); - $node->getIndex()->willReturn(1); - - $accessor = $this->prophesize(DocumentAccessor::class); - $accessor->set('pageNumber', 2)->shouldBeCalled(); - - $event = $this->createEvent(HydrateEvent::class, $node->reveal(), $accessor->reveal()); - - $this->subscriber->setPageNumberOnHydrate($event); - } - public function testSetWorkflowStageOnArticle() { $this->document->getParent()->willReturn($this->parentDocument->reveal()); diff --git a/Tests/Unit/Document/Subscriber/PageSubscriberTest.php b/Tests/Unit/Document/Subscriber/PageSubscriberTest.php new file mode 100644 index 000000000..57855c11e --- /dev/null +++ b/Tests/Unit/Document/Subscriber/PageSubscriberTest.php @@ -0,0 +1,145 @@ +documentInspector = $this->prophesize(DocumentInspector::class); + $this->propertyEncoder = $this->prophesize(PropertyEncoder::class); + + $this->document = $this->prophesize(PageBehavior::class); + $this->node = $this->prophesize(NodeInterface::class); + + $this->pageSubscriber = new PageSubscriber( + $this->documentInspector->reveal(), $this->propertyEncoder->reveal() + ); + } + + public function testHandlePersist() + { + $child1 = $this->prophesize(\stdClass::class); + $child2 = $this->prophesize(PageBehavior::class); + $child2->getUuid()->willReturn('1-1-1'); + $child3 = $this->prophesize(PageBehavior::class); + $child3->getUuid()->willReturn('1-2-2'); + + $this->documentInspector->getNode($this->document->reveal())->willReturn($this->node->reveal()); + + $event = $this->prophesize(PersistEvent::class); + $event->getDocument()->willReturn($this->document->reveal()); + + $parentDocument = $this->prophesize(ChildrenBehavior::class); + $parentDocument->getChildren()->willReturn( + [ + $child1->reveal(), + $child2->reveal(), + $child3->reveal(), + ] + ); + $this->document->getParent()->willReturn($parentDocument->reveal()); + $this->document->getUuid()->willReturn('1-2-3'); + $this->document->getPageNumber()->willReturn(null); + + $this->propertyEncoder->systemName(PageSubscriber::FIELD)->willReturn('sulu:' . PageSubscriber::FIELD); + + $this->node->setProperty('sulu:' . PageSubscriber::FIELD, 3)->shouldBeCalled(); + + $this->pageSubscriber->handlePersist($event->reveal()); + } + + public function testHandlePersistAlreadySet() + { + $event = $this->prophesize(PersistEvent::class); + $event->getDocument()->willReturn($this->document->reveal()); + + $this->document->getPageNumber()->willReturn(2); + $this->document->setPageNumber(Argument::any())->shouldNotBeCalled(); + + $this->pageSubscriber->handlePersist($event->reveal()); + } + + public function testHandleRemove() + { + $child1 = $this->prophesize(\stdClass::class); + $child2 = $this->prophesize(PageBehavior::class); + $child2->getUuid()->willReturn('1-1-1'); + $child3 = $this->prophesize(PageBehavior::class); + $child3->getUuid()->willReturn('1-2-2'); + + $childNode2 = $this->prophesize(NodeInterface::class); + $this->documentInspector->getNode($child2->reveal())->willReturn($childNode2->reveal()); + $childNode3 = $this->prophesize(NodeInterface::class); + $this->documentInspector->getNode($child3->reveal())->willReturn($childNode3->reveal()); + + $event = $this->prophesize(RemoveEvent::class); + $event->getDocument()->willReturn($this->document->reveal()); + + $parentDocument = $this->prophesize(ChildrenBehavior::class); + $parentDocument->getChildren()->willReturn( + [ + $child1->reveal(), + $child2->reveal(), + $this->document->reveal(), + $child3->reveal(), + ] + ); + $this->document->getParent()->willReturn($parentDocument->reveal()); + $this->document->getUuid()->willReturn('1-2-3'); + + $this->propertyEncoder->systemName(PageSubscriber::FIELD)->willReturn('sulu:' . PageSubscriber::FIELD); + + $childNode2->setProperty('sulu:' . PageSubscriber::FIELD, 1)->shouldBeCalled(); + $childNode3->setProperty('sulu:' . PageSubscriber::FIELD, 2)->shouldBeCalled(); + + $this->pageSubscriber->handleRemove($event->reveal()); + } +} diff --git a/Tests/Unit/Document/Subscriber/RoutableSubscriberTest.php b/Tests/Unit/Document/Subscriber/RoutableSubscriberTest.php index ff078ef71..64fa59c32 100644 --- a/Tests/Unit/Document/Subscriber/RoutableSubscriberTest.php +++ b/Tests/Unit/Document/Subscriber/RoutableSubscriberTest.php @@ -22,6 +22,7 @@ use Sulu\Bundle\RouteBundle\Model\RouteInterface; use Sulu\Component\DocumentManager\DocumentManagerInterface; use Sulu\Component\DocumentManager\Event\CopyEvent; +use Sulu\Component\DocumentManager\Behavior\Mapping\ChildrenBehavior; use Sulu\Component\DocumentManager\Event\HydrateEvent; use Sulu\Component\DocumentManager\Event\PersistEvent; use Sulu\Component\DocumentManager\Event\RemoveEvent; @@ -197,11 +198,41 @@ public function testCopy() $node = $this->prophesize(NodeInterface::class); $event->getCopiedNode()->willReturn($node->reveal()); - $this->propertyEncoder->localizedSystemName(RoutableSubscriber::ROUTE_FIELD, 'de') - ->willReturn('i18n:de-routePath'); + $this->propertyEncoder->localizedSystemName(RoutableSubscriber::ROUTE_FIELD, 'de')->willReturn( + 'i18n:de-routePath' + ); $node->setProperty('i18n:de-routePath', '/test'); $this->routableSubscriber->handleCopy($event->reveal()); } + + public function testHandleRemoveWithChildren() + { + $this->document->willImplement(ChildrenBehavior::class); + + $event = $this->prophesize(RemoveEvent::class); + $event->getDocument()->willReturn($this->document->reveal()); + + $this->document->getRoutePath()->willReturn('/test'); + $this->document->getOriginalLocale()->willReturn('de'); + $route1 = $this->prophesize(RouteInterface::class); + $this->routeRepository->findByPath('/test', 'de')->willReturn($route1->reveal()); + + $child = $this->prophesize(RoutableBehavior::class); + $child->willImplement(ChildrenBehavior::class); + $child->getChildren()->willReturn([]); + $this->document->getChildren()->willReturn([$child->reveal()]); + + $child->getRoutePath()->willReturn('/test/test-2'); + $child->getOriginalLocale()->willReturn('de'); + $route2 = $this->prophesize(RouteInterface::class); + $this->routeRepository->findByPath('/test/test-2', 'de')->willReturn($route2->reveal()); + + $this->entityManager->remove($route1->reveal())->shouldBeCalled(); + $this->entityManager->remove($route2->reveal())->shouldBeCalled(); + $this->entityManager->flush()->shouldBeCalled(); + + $this->routableSubscriber->handleRemove($event->reveal()); + } } diff --git a/Tests/Unit/Routing/ArticlePageRouteGeneratorTest.php b/Tests/Unit/Routing/ArticlePageRouteGeneratorTest.php new file mode 100644 index 000000000..188d3b452 --- /dev/null +++ b/Tests/Unit/Routing/ArticlePageRouteGeneratorTest.php @@ -0,0 +1,77 @@ +routeGenerator = $this->prophesize(RouteGeneratorInterface::class); + $this->tokenProvider = $this->prophesize(TokenProviderInterface::class); + + $this->articleRouteGenerator = new ArticlePageRouteGenerator( + $this->routeGenerator->reveal(), + $this->tokenProvider->reveal() + ); + } + + public function testGenerate() + { + $entity = $this->prophesize(\stdClass::class); + + $parent = '/{object.getParentPath()}'; + $this->tokenProvider->provide($entity->reveal(), 'object.getParentPath()')->willReturn('parent'); + + $this->routeGenerator->generate($entity->reveal(), ['route_schema' => '/parent' . $this->routeSchema]) + ->shouldBeCalled() + ->willReturn('/parent/page-2'); + + $result = $this->articleRouteGenerator->generate( + $entity->reveal(), + ['parent' => $parent, 'route_schema' => $this->routeSchema] + ); + $this->assertEquals('/parent/page-2', $result); + } + + public function testGetOptionsResolver() + { + $this->assertInstanceOf(OptionsResolver::class, $this->articleRouteGenerator->getOptionsResolver([])); + } +}