From 991a8d04b6e497661192a162b2495e6b25229711 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Sat, 22 Jun 2013 11:27:33 +0200 Subject: [PATCH 01/13] refactor publish workflow to use security, cleanup configuration --- CHANGELOG.md | 8 ++ DependencyInjection/CmfCoreExtension.php | 20 +++- DependencyInjection/Configuration.php | 8 +- EventListener/PublishWorkflowListener.php | 16 +-- .../PublishTimePeriodInterface.php | 30 ++++++ PublishWorkflow/PublishWorkflowChecker.php | 86 --------------- .../PublishWorkflowCheckerInterface.php | 18 ---- PublishWorkflow/PublishWorkflowInterface.php | 38 +------ PublishWorkflow/PublishableInterface.php | 24 +++++ Resources/config/publish_workflow.xml | 40 +++++++ Resources/config/services.xml | 19 +--- .../Voter/PublishTimePeriodVoter.php | 102 ++++++++++++++++++ .../Authorization/Voter/PublishableVoter.php | 80 ++++++++++++++ Twig/TwigExtension.php | 31 ++---- 14 files changed, 326 insertions(+), 194 deletions(-) create mode 100644 PublishWorkflow/PublishTimePeriodInterface.php delete mode 100644 PublishWorkflow/PublishWorkflowChecker.php delete mode 100644 PublishWorkflow/PublishWorkflowCheckerInterface.php create mode 100644 PublishWorkflow/PublishableInterface.php create mode 100644 Resources/config/publish_workflow.xml create mode 100644 Security/Authorization/Voter/PublishTimePeriodVoter.php create mode 100644 Security/Authorization/Voter/PublishableVoter.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 167483ab..9f06a473 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ Changelog ========= +* **2013-06-20**: [PublishWorkflow] Moved the access checks to security voter + and using isGranted 'VIEW' instead. + Removed twig function cmf_is_published, just use is_granted('VIEW', content) + instead. + Configuration was adjusted: The parameter for the role that may see unpublished + content moved from `role` to `publish_workflow.view_non_published_role`. The + publish_workflow_listener moved to `publish_workflow.request_listener`. + * **2013-05-16**: [PublishWorkFlowChecker] Removed Request argument from check method. Class now accepts a DateTime object to optionally "set" the current time. diff --git a/DependencyInjection/CmfCoreExtension.php b/DependencyInjection/CmfCoreExtension.php index 4755655b..5147b0db 100644 --- a/DependencyInjection/CmfCoreExtension.php +++ b/DependencyInjection/CmfCoreExtension.php @@ -17,13 +17,23 @@ public function load(array $configs, ContainerBuilder $container) $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); $loader->load('services.xml'); - $container->setParameter($this->getAlias().'.role', $config['role']); $container->setParameter($this->getAlias() . '.document_manager_name', $config['document_manager_name']); - if (!$config['publish_workflow_listener']) { - $container->removeDefinition($this->getAlias() . '.publish_workflow_listener'); - } elseif (!class_exists('Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter')) { - throw new InvalidConfigurationException("The 'publish_workflow_listener' may not be enabled unless 'Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter' is available."); + if (isset($config['publish_workflow'])) { + $this->loadPublishWorkflow($config['publish_workflow'], $loader, $container); + } + } + + public function loadPublishWorkflow($config, XmlFileLoader $loader, ContainerBuilder $container) { + $container->setParameter($this->getAlias().'.publish_workflow.view_non_published_role', $config['view_non_published_role']); + $loader->load('publish_workflow.xml'); + + if ($config['request_listener']) { + if (!class_exists('Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter')) { + throw new InvalidConfigurationException('The "publish_workflow.request_listener" may not be enabled unless "Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter" is available.'); + } + } else { + $container->removeDefinition($this->getAlias() . '.publish_workflow.request_listener'); } } diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 27c014f0..332d0e2f 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -15,8 +15,12 @@ public function getConfigTreeBuilder() $rootNode ->children() ->scalarNode('document_manager_name')->defaultValue('default')->end() - ->scalarNode('role')->defaultValue('IS_AUTHENTICATED_ANONYMOUSLY')->end() - ->booleanNode('publish_workflow_listener')->defaultFalse()->end() + ->arrayNode('publish_workflow') + ->children() + ->scalarNode('view_non_published_role')->defaultValue('CAN_VIEW_NON_PUBLISHED')->end() + ->booleanNode('request_listener')->defaultTrue()->end() + ->end() + ->end() ->end() ; diff --git a/EventListener/PublishWorkflowListener.php b/EventListener/PublishWorkflowListener.php index 8b91c60b..e59d1b6a 100644 --- a/EventListener/PublishWorkflowListener.php +++ b/EventListener/PublishWorkflowListener.php @@ -8,7 +8,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter; -use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowCheckerInterface; +use Symfony\Component\Security\Core\SecurityContext; /** * Makes sure only published routes and content can be accessed @@ -16,16 +16,16 @@ class PublishWorkflowListener implements EventSubscriberInterface { /** - * @var PublishWorkflowCheckerInterface + * @var SecurityContext */ - protected $publishWorkflowChecker; + protected $context; /** - * @param PublishWorkflowCheckerInterface $publishWorkflowChecker + * @param SecurityContext $context */ - public function __construct(PublishWorkflowCheckerInterface $publishWorkflowChecker) + public function __construct(SecurityContext $context) { - $this->publishWorkflowChecker = $publishWorkflowChecker; + $this->context = $context; } /** @@ -38,12 +38,12 @@ public function onKernelRequest(GetResponseEvent $event) $request = $event->getRequest(); $route = $request->attributes->get(DynamicRouter::ROUTE_KEY); - if ($route && !$this->publishWorkflowChecker->checkIsPublished($route, false, $request)) { + if ($route && !$this->context->isGranted('VIEW', $route)) { throw new NotFoundHttpException('Route not found at: ' . $request->getPathInfo()); } $content = $request->attributes->get(DynamicRouter::CONTENT_KEY); - if ($content && !$this->publishWorkflowChecker->checkIsPublished($content, false, $request)) { + if ($content && !$this->context->isGranted('VIEW', $content)) { throw new NotFoundHttpException('Content not found for: ' . $request->getPathInfo()); } } diff --git a/PublishWorkflow/PublishTimePeriodInterface.php b/PublishWorkflow/PublishTimePeriodInterface.php new file mode 100644 index 00000000..0ae42298 --- /dev/null +++ b/PublishWorkflow/PublishTimePeriodInterface.php @@ -0,0 +1,30 @@ +requiredRole = $requiredRole; - $this->securityContext = $securityContext; - $this->currentTime = new \DateTime(); - } - - /** - * Overwrite the current time - * - * @param \DateTime $currentTime - */ - public function setCurrentTime(\DateTime $currentTime) - { - $this->currentTime = $currentTime; - } - - /** - * {inheritDoc} - */ - public function checkIsPublished($document, $ignoreRole = false) - { - if (!$document instanceOf PublishWorkflowInterface) { - return true; - } - - if ($this->securityContext && $this->securityContext->isGranted($this->requiredRole)) { - if (!$ignoreRole) { - return true; - } - } - - $startDate = $document->getPublishStartDate(); - $endDate = $document->getPublishEndDate(); - $isPublishable = $document->isPublishable(); - - if (null === $startDate && null === $endDate) { - return $isPublishable !== false; - } - - if ((null === $startDate || $this->currentTime >= $startDate) && - (null === $endDate || $this->currentTime < $endDate) - ) { - return $isPublishable !== false; - } - - return false; - } -} diff --git a/PublishWorkflow/PublishWorkflowCheckerInterface.php b/PublishWorkflow/PublishWorkflowCheckerInterface.php deleted file mode 100644 index 23cb4c90..00000000 --- a/PublishWorkflow/PublishWorkflowCheckerInterface.php +++ /dev/null @@ -1,18 +0,0 @@ - + + + + + Symfony\Cmf\Bundle\CoreBundle\Twig\TwigExtension + Symfony\Cmf\Bundle\CoreBundle\EventListener\PublishWorkflowListener + Symfony\Cmf\Bundle\CoreBundle\Security\Authorization\Voter\PublishableVoter + Symfony\Cmf\Bundle\CoreBundle\Security\Authorization\Voter\PublishTimePeriodVoter + Symfony\Cmf\Bundle\CoreBundle\EventListener\RequestAwareListener + Symfony\Cmf\Bundle\CoreBundle\Admin\Extension\PublishWorkflowExtension + + + + + + + %cmf_core.publish_workflow.view_non_published_role% + + + + + + %cmf_core.publish_workflow.view_non_published_role% + + + + + + + + + + + + + + diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 48cad058..6e4e1b8a 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -6,38 +6,21 @@ Symfony\Cmf\Bundle\CoreBundle\Twig\TwigExtension - Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker - Symfony\Cmf\Bundle\CoreBundle\EventListener\PublishWorkflowListener Symfony\Cmf\Bundle\CoreBundle\EventListener\RequestAwareListener - Symfony\Cmf\Bundle\CoreBundle\Admin\Extension\PublishWorkflowExtension - + %cmf_core.document_manager_name% - - %cmf_core.role% - - - - - - - - - - - - diff --git a/Security/Authorization/Voter/PublishTimePeriodVoter.php b/Security/Authorization/Voter/PublishTimePeriodVoter.php new file mode 100644 index 00000000..706e84a8 --- /dev/null +++ b/Security/Authorization/Voter/PublishTimePeriodVoter.php @@ -0,0 +1,102 @@ +container = $container; + $this->bypassingRole = $bypassingRole; + // we create the timestamp on instantiation to avoid glitches due to + // the time passing during the request + $this->currentTime = new \DateTime(); + } + + /** + * Overwrite the current time. + * + * @param \DateTime $currentTime + */ + public function setCurrentTime(\DateTime $currentTime) + { + $this->currentTime = $currentTime; + } + + /** + * {@inheritdoc} + */ + public function supportsAttribute($attribute) + { + return $attribute === 'VIEW'; + } + + /** + * {@inheritdoc} + */ + public function supportsClass($class) + { + return is_subclass_of($class, 'Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishTimePeriodInterface'); + } + + /** + * {@inheritdoc} + * + * @param PublishTimePeriodInterface $object + */ + public function vote(TokenInterface $token, $object, array $attributes) + { + if (!$this->supportsClass(get_class($object)) + || !in_array('VIEW', $attributes) + ) { + return self::ACCESS_ABSTAIN; + } + + $context = $this->container->get('security.context'); + if ($this->bypassingRole && $context->isGranted($this->bypassingRole)) { + return self::ACCESS_ABSTAIN; + } + $startDate = $object->getPublishStartDate(); + $endDate = $object->getPublishEndDate(); + + if ((null === $startDate || $this->currentTime >= $startDate) && + (null === $endDate || $this->currentTime < $endDate) + ) { + return self::ACCESS_ABSTAIN; + } + + return self::ACCESS_DENIED; + } +} \ No newline at end of file diff --git a/Security/Authorization/Voter/PublishableVoter.php b/Security/Authorization/Voter/PublishableVoter.php new file mode 100644 index 00000000..f3930c78 --- /dev/null +++ b/Security/Authorization/Voter/PublishableVoter.php @@ -0,0 +1,80 @@ +container = $container; + $this->bypassingRole = $bypassingRole; + } + + /** + * {@inheritdoc} + */ + public function supportsAttribute($attribute) + { + return $attribute === 'VIEW'; + } + + /** + * {@inheritdoc} + */ + public function supportsClass($class) + { + return is_subclass_of($class, 'Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableInterface'); + } + + /** + * {@inheritdoc} + * + * @param PublishableInterface $object + */ + public function vote(TokenInterface $token, $object, array $attributes) + { + if (!$this->supportsClass(get_class($object)) + || !in_array('VIEW', $attributes) + ) { + return self::ACCESS_ABSTAIN; + } + + $context = $this->container->get('security.context'); + if ($this->bypassingRole && $context->isGranted($this->bypassingRole)) { + return self::ACCESS_ABSTAIN; + } + + if (! $object->isPublishable()) { + return self::ACCESS_DENIED; + } + + return self::ACCESS_ABSTAIN; + } +} \ No newline at end of file diff --git a/Twig/TwigExtension.php b/Twig/TwigExtension.php index 0e0e8ec7..253697f5 100644 --- a/Twig/TwigExtension.php +++ b/Twig/TwigExtension.php @@ -2,11 +2,11 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Twig; -use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowCheckerInterface; use Doctrine\Common\Persistence\ManagerRegistry; use Doctrine\ODM\PHPCR\Exception\MissingTranslationException; use Doctrine\ODM\PHPCR\DocumentManager; use PHPCR\Util\PathHelper; +use Symfony\Component\Security\Core\SecurityContext; class TwigExtension extends \Twig_Extension { @@ -16,20 +16,20 @@ class TwigExtension extends \Twig_Extension protected $dm; /** - * @var PublishWorkflowCheckerInterface + * @var SecurityContext */ - protected $publishWorkflowChecker; + protected $context; /** * Instantiate the content controller. * - * @param PublishWorkflowCheckerInterface $publishWorkflowChecker + * @param SecurityContext $context * @param ManagerRegistry $registry * @param string $objectManagerName */ - public function __construct(PublishWorkflowCheckerInterface $publishWorkflowChecker, $registry = null, $objectManagerName = null) + public function __construct(SecurityContext $context, $registry = null, $objectManagerName = null) { - $this->publishWorkflowChecker = $publishWorkflowChecker; + $this->context = $context; if ($registry && $registry instanceof ManagerRegistry) { $this->dm = $registry->getManager($objectManagerName); @@ -43,8 +43,6 @@ public function __construct(PublishWorkflowCheckerInterface $publishWorkflowChec */ public function getFunctions() { - $functions = array('cmf_is_published' => new \Twig_Function_Method($this, 'isPublished')); - if ($this->dm) { $functions['cmf_child'] = new \Twig_Function_Method($this, 'getChild'); $functions['cmf_children'] = new \Twig_Function_Method($this, 'getChildren'); @@ -136,7 +134,7 @@ private function getDocument($document, $ignoreRole = false, $class = null) } if (empty($document) - || (null !== $ignoreRole && !$this->publishWorkflowChecker->checkIsPublished($document, $ignoreRole)) + || (null !== $ignoreRole && !$this->context->isGranted('VIEW', $document)) || (null != $class && !($document instanceof $class)) ) { return null; @@ -178,21 +176,6 @@ public function findMany($paths = array(), $limit = false, $offset = false, $ign return $result; } - /** - * Check if a document is published - * - * @param $document - * @return Boolean - */ - public function isPublished($document) - { - if (empty($document)) { - return false; - } - - return $this->publishWorkflowChecker->checkIsPublished($document, true); - } - /** * Get the locales of the document * From 6c7aadad057cb5c79b88603c8a4559ec28a21d9d Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 24 Jun 2013 21:56:47 +0200 Subject: [PATCH 02/13] refactoring to have a separate check --- CHANGELOG.md | 15 +- CmfCoreBundle.php | 2 + DependencyInjection/CmfCoreExtension.php | 17 ++- .../Compiler/AddPublishedVotersPass.php | 39 +++++ DependencyInjection/Configuration.php | 2 + EventListener/PublishWorkflowListener.php | 21 +-- PublishWorkflow/PublishWorkflowChecker.php | 137 ++++++++++++++++++ .../Voter/PublishTimePeriodVoter.php | 50 +++---- PublishWorkflow/Voter/PublishableVoter.php | 59 ++++++++ Resources/config/publish_workflow.xml | 44 ++++-- Resources/config/services.xml | 4 +- .../Authorization/Voter/PublishableVoter.php | 80 ---------- .../Authorization/Voter/PublishedVoter.php | 72 +++++++++ Twig/TwigExtension.php | 50 +++++-- 14 files changed, 432 insertions(+), 160 deletions(-) create mode 100644 DependencyInjection/Compiler/AddPublishedVotersPass.php create mode 100644 PublishWorkflow/PublishWorkflowChecker.php rename {Security/Authorization => PublishWorkflow}/Voter/PublishTimePeriodVoter.php (58%) create mode 100644 PublishWorkflow/Voter/PublishableVoter.php delete mode 100644 Security/Authorization/Voter/PublishableVoter.php create mode 100644 Security/Authorization/Voter/PublishedVoter.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f06a473..9f6d2866 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,16 @@ Changelog ========= -* **2013-06-20**: [PublishWorkflow] Moved the access checks to security voter - and using isGranted 'VIEW' instead. - Removed twig function cmf_is_published, just use is_granted('VIEW', content) - instead. +* **2013-06-20**: [PublishWorkflow] The PublishWorkflowChecker now implements + SecurityContextInterface and the individual checks are moved to voters. + Use the service cmf_core.publish_workflow.checker and call + `isGranted('VIEW', $content)` - or `'VIEW_PUBLISHED'` if you don't want to + see unpublished content even if the current user is allowed to see it. Configuration was adjusted: The parameter for the role that may see unpublished - content moved from `role` to `publish_workflow.view_non_published_role`. The - publish_workflow_listener moved to `publish_workflow.request_listener`. + content moved from `role` to `publish_workflow.view_non_published_role`. + The security context is also triggered by a core security voter, so that + using the isGranted method of the standard security will check for + publication. * **2013-05-16**: [PublishWorkFlowChecker] Removed Request argument from check method. Class now accepts a DateTime object to diff --git a/CmfCoreBundle.php b/CmfCoreBundle.php index cffac093..92c2832a 100644 --- a/CmfCoreBundle.php +++ b/CmfCoreBundle.php @@ -5,6 +5,7 @@ use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Cmf\Bundle\CoreBundle\DependencyInjection\Compiler\RequestAwarePass; +use Symfony\Cmf\Bundle\CoreBundle\DependencyInjection\Compiler\AddPublishedVotersPass; class CmfCoreBundle extends Bundle { @@ -12,5 +13,6 @@ public function build(ContainerBuilder $container) { parent::build($container); $container->addCompilerPass(new RequestAwarePass()); + $container->addCompilerPass(new AddPublishedVotersPass()); } } diff --git a/DependencyInjection/CmfCoreExtension.php b/DependencyInjection/CmfCoreExtension.php index 5147b0db..94073a1a 100644 --- a/DependencyInjection/CmfCoreExtension.php +++ b/DependencyInjection/CmfCoreExtension.php @@ -19,21 +19,24 @@ public function load(array $configs, ContainerBuilder $container) $container->setParameter($this->getAlias() . '.document_manager_name', $config['document_manager_name']); - if (isset($config['publish_workflow'])) { + if ($config['publish_workflow']['enabled']) { $this->loadPublishWorkflow($config['publish_workflow'], $loader, $container); } } - public function loadPublishWorkflow($config, XmlFileLoader $loader, ContainerBuilder $container) { + public function loadPublishWorkflow($config, XmlFileLoader $loader, ContainerBuilder $container) + { $container->setParameter($this->getAlias().'.publish_workflow.view_non_published_role', $config['view_non_published_role']); $loader->load('publish_workflow.xml'); - if ($config['request_listener']) { - if (!class_exists('Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter')) { - throw new InvalidConfigurationException('The "publish_workflow.request_listener" may not be enabled unless "Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter" is available.'); - } - } else { + if (!$config['request_listener']) { $container->removeDefinition($this->getAlias() . '.publish_workflow.request_listener'); + } elseif (!class_exists('Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter')) { + throw new InvalidConfigurationException('The "publish_workflow.request_listener" may not be enabled unless "Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter" is available.'); + } + + if (!class_exists('Sonata\AdminBundle\Admin\AdminExtension')) { + $container->removeDefinition($this->getAlias() . '.cmf_core.admin_extension.publish_workflow'); } } diff --git a/DependencyInjection/Compiler/AddPublishedVotersPass.php b/DependencyInjection/Compiler/AddPublishedVotersPass.php new file mode 100644 index 00000000..943507e4 --- /dev/null +++ b/DependencyInjection/Compiler/AddPublishedVotersPass.php @@ -0,0 +1,39 @@ + + * @author Johannes M. Schmitt + */ +class AddPublishedVotersPass implements CompilerPassInterface +{ + /** + * {@inheritDoc} + */ + public function process(ContainerBuilder $container) + { + if (!$container->hasDefinition('cmf_core.publish_workflow.access_decision_manager')) { + return; + } + + $voters = new \SplPriorityQueue(); + foreach ($container->findTaggedServiceIds('cmf_published_voter') as $id => $attributes) { + $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; + $voters->insert(new Reference($id), $priority); + } + + $voters = iterator_to_array($voters); + ksort($voters); + + $container->getDefinition('cmf_core.publish_workflow.access_decision_manager')->replaceArgument(0, array_values($voters)); + } +} diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 332d0e2f..ae4019ca 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -16,7 +16,9 @@ public function getConfigTreeBuilder() ->children() ->scalarNode('document_manager_name')->defaultValue('default')->end() ->arrayNode('publish_workflow') + ->addDefaultsIfNotSet() ->children() + ->booleanNode('enabled')->defaultTrue()->end() ->scalarNode('view_non_published_role')->defaultValue('CAN_VIEW_NON_PUBLISHED')->end() ->booleanNode('request_listener')->defaultTrue()->end() ->end() diff --git a/EventListener/PublishWorkflowListener.php b/EventListener/PublishWorkflowListener.php index e59d1b6a..2fcd6763 100644 --- a/EventListener/PublishWorkflowListener.php +++ b/EventListener/PublishWorkflowListener.php @@ -7,25 +7,28 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker; + use Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter; -use Symfony\Component\Security\Core\SecurityContext; /** - * Makes sure only published routes and content can be accessed + * Makes sure only published routes and content can be accessed. + * + * @author David Buchmann */ class PublishWorkflowListener implements EventSubscriberInterface { /** - * @var SecurityContext + * @var PublishWorkflowChecker */ - protected $context; + protected $publishWorkflowChecker; /** - * @param SecurityContext $context + * @param PublishWorkflowChecker $publishWorkflowChecker */ - public function __construct(SecurityContext $context) + public function __construct(PublishWorkflowChecker $publishWorkflowChecker) { - $this->context = $context; + $this->publishWorkflowChecker = $publishWorkflowChecker; } /** @@ -38,12 +41,12 @@ public function onKernelRequest(GetResponseEvent $event) $request = $event->getRequest(); $route = $request->attributes->get(DynamicRouter::ROUTE_KEY); - if ($route && !$this->context->isGranted('VIEW', $route)) { + if ($route && !$this->publishWorkflowChecker->isGranted('VIEW', $route)) { throw new NotFoundHttpException('Route not found at: ' . $request->getPathInfo()); } $content = $request->attributes->get(DynamicRouter::CONTENT_KEY); - if ($content && !$this->context->isGranted('VIEW', $content)) { + if ($content && !$this->publishWorkflowChecker->isGranted('VIEW', $content)) { throw new NotFoundHttpException('Content not found for: ' . $request->getPathInfo()); } } diff --git a/PublishWorkflow/PublishWorkflowChecker.php b/PublishWorkflow/PublishWorkflowChecker.php new file mode 100644 index 00000000..361e6709 --- /dev/null +++ b/PublishWorkflow/PublishWorkflowChecker.php @@ -0,0 +1,137 @@ + +*/ +class PublishWorkflowChecker implements SecurityContextInterface +{ + /** + * This attribute means the user is allowed to see this content, either + * because it is published or because he is granted the bypassingRole. + */ + const VIEW_ATTRIBUTE = 'VIEW'; + + /** + * This attribute means the content is available for viewing by anonymous + * users. This can be used where the role based exception from the + * publication check is not wanted. + * + * The role exception is handled by the workflow checker, the individual + * voters should treat VIEW and VIEW_PUBLISHED the same. + */ + const VIEW_PUBLISHED_ATTRIBUTE = 'VIEW_PUBLISHED'; + + /** + * @var ContainerInterface + */ + private $container; + + /** + * @var bool|string Role allowed to bypass security check or false to never + * bypass + */ + private $bypassingRole; + + /** + * @var AccessDecisionManagerInterface + */ + private $accessDecisionManager; + + /** + * @var TokenInterface + */ + private $token; + + /** + * @param ContainerInterface $container to get the security context from. + * We cannot inject the security context directly as this would lead + * to a circular reference. + * @param AccessDecisionManagerInterface $accessDecisionManager + * @param string $bypassingRole A role that is allowed to bypass the + * publishable check. + */ + public function __construct(ContainerInterface $container, AccessDecisionManagerInterface $accessDecisionManager, $bypassingRole = false) + { + $this->container = $container; + $this->accessDecisionManager = $accessDecisionManager; + $this->bypassingRole = $bypassingRole; + } + + /** + * {@inheritDoc} + */ + public function getToken() + { + if (null === $this->token) { + $securityContext = $this->container->get('security.context'); + + return $securityContext->getToken(); + } + + return $this->token; + } + + /** + * {@inheritDoc} + */ + public function setToken(TokenInterface $token = null) + { + $this->token = $token; + } + + /** + * Checks if the access decision manager supports the given class. + * + * @param string $class A class name + * + * @return boolean true if this decision manager can process the class + */ + public function supportsClass($class) + { + return $this->accessDecisionManager->supportsClass($class); + } + + /** + * {@inheritDoc} + */ + public function isGranted($attributes, $object = null) + { + if (!is_array($attributes)) { + $attributes = array($attributes); + } + + $securityContext = $this->container->get('security.context'); + if (null !== $securityContext->getToken() + && (count($attributes) === 1) + && self::VIEW_ATTRIBUTE === reset($attributes) + && $securityContext->isGranted($this->bypassingRole) + ) { + return true; + } + + $token = $this->getToken(); + if (null === $token) { + // not logged in, surely we can not skip the check. + // create a dummy token to check for publication even if no + // firewall is present. + $token = new AnonymousToken('', ''); + } + + return $this->accessDecisionManager->decide($token, $attributes, $object); + } +} \ No newline at end of file diff --git a/Security/Authorization/Voter/PublishTimePeriodVoter.php b/PublishWorkflow/Voter/PublishTimePeriodVoter.php similarity index 58% rename from Security/Authorization/Voter/PublishTimePeriodVoter.php rename to PublishWorkflow/Voter/PublishTimePeriodVoter.php index 706e84a8..0800c5e0 100644 --- a/Security/Authorization/Voter/PublishTimePeriodVoter.php +++ b/PublishWorkflow/Voter/PublishTimePeriodVoter.php @@ -1,7 +1,8 @@ */ class PublishTimePeriodVoter implements VoterInterface { - /** - * @var ContainerInterface - */ - private $container; - - /** - * @var bool|string Role allowed to bypass security check or false to never - * bypass - */ - private $bypassingRole; - /** * @var \DateTime */ protected $currentTime; - /** - * @param ContainerInterface $container to get the security context from. - * We cannot inject the security context directly as this would lead - * to a circular reference. - * @param string $bypassingRole A role that is allowed to bypass the - * publishable check. - */ - public function __construct(ContainerInterface $container, $bypassingRole = false) + public function __construct() { - $this->container = $container; - $this->bypassingRole = $bypassingRole; // we create the timestamp on instantiation to avoid glitches due to // the time passing during the request $this->currentTime = new \DateTime(); @@ -60,7 +43,9 @@ public function setCurrentTime(\DateTime $currentTime) */ public function supportsAttribute($attribute) { - return $attribute === 'VIEW'; + return PublishWorkflowChecker::VIEW_ATTRIBUTE === $attribute + || PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE === $attribute + ; } /** @@ -78,25 +63,24 @@ public function supportsClass($class) */ public function vote(TokenInterface $token, $object, array $attributes) { - if (!$this->supportsClass(get_class($object)) - || !in_array('VIEW', $attributes) - ) { + if (!$this->supportsClass(get_class($object))) { return self::ACCESS_ABSTAIN; } - - $context = $this->container->get('security.context'); - if ($this->bypassingRole && $context->isGranted($this->bypassingRole)) { - return self::ACCESS_ABSTAIN; + foreach($attributes as $attribute) { + if (! $this->supportsAttribute($attribute)) { + return self::ACCESS_ABSTAIN; + } } + $startDate = $object->getPublishStartDate(); $endDate = $object->getPublishEndDate(); if ((null === $startDate || $this->currentTime >= $startDate) && (null === $endDate || $this->currentTime < $endDate) ) { - return self::ACCESS_ABSTAIN; + return self::ACCESS_GRANTED; } return self::ACCESS_DENIED; } -} \ No newline at end of file +} diff --git a/PublishWorkflow/Voter/PublishableVoter.php b/PublishWorkflow/Voter/PublishableVoter.php new file mode 100644 index 00000000..30a898c3 --- /dev/null +++ b/PublishWorkflow/Voter/PublishableVoter.php @@ -0,0 +1,59 @@ + + */ +class PublishableVoter implements VoterInterface +{ + /** + * {@inheritdoc} + */ + public function supportsAttribute($attribute) + { + return PublishWorkflowChecker::VIEW_ATTRIBUTE === $attribute + || PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE === $attribute + ; + } + + /** + * {@inheritdoc} + */ + public function supportsClass($class) + { + return is_subclass_of($class, 'Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableInterface'); + } + + /** + * {@inheritdoc} + * + * @param PublishableInterface $object + */ + public function vote(TokenInterface $token, $object, array $attributes) + { + if (!$this->supportsClass(get_class($object))) { + return self::ACCESS_ABSTAIN; + } + foreach($attributes as $attribute) { + if (! $this->supportsAttribute($attribute)) { + return self::ACCESS_ABSTAIN; + } + } + + if (! $object->isPublishable()) { + return self::ACCESS_DENIED; + } + + return self::ACCESS_GRANTED; + } +} \ No newline at end of file diff --git a/Resources/config/publish_workflow.xml b/Resources/config/publish_workflow.xml index 79fe64ee..d5fb00e5 100644 --- a/Resources/config/publish_workflow.xml +++ b/Resources/config/publish_workflow.xml @@ -5,33 +5,51 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - Symfony\Cmf\Bundle\CoreBundle\Twig\TwigExtension - Symfony\Cmf\Bundle\CoreBundle\EventListener\PublishWorkflowListener - Symfony\Cmf\Bundle\CoreBundle\Security\Authorization\Voter\PublishableVoter - Symfony\Cmf\Bundle\CoreBundle\Security\Authorization\Voter\PublishTimePeriodVoter - Symfony\Cmf\Bundle\CoreBundle\EventListener\RequestAwareListener + Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker + Symfony\Component\Security\Core\Authorization\AccessDecisionManager + Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\Voter\PublishableVoter + Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\Voter\PublishTimePeriodVoter + Symfony\Cmf\Bundle\CoreBundle\EventListener\PublishWorkflowListener + Symfony\Cmf\Bundle\CoreBundle\Security\Authorization\Voter\PublishedVoter Symfony\Cmf\Bundle\CoreBundle\Admin\Extension\PublishWorkflowExtension - - + + + unanimous + true + + + + + %cmf_core.publish_workflow.view_non_published_role% - + + + + - - %cmf_core.publish_workflow.view_non_published_role% - + - + + - + + + + + + + + + diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 6e4e1b8a..6fd90fcc 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -12,8 +12,8 @@ - - + + %cmf_core.document_manager_name% diff --git a/Security/Authorization/Voter/PublishableVoter.php b/Security/Authorization/Voter/PublishableVoter.php deleted file mode 100644 index f3930c78..00000000 --- a/Security/Authorization/Voter/PublishableVoter.php +++ /dev/null @@ -1,80 +0,0 @@ -container = $container; - $this->bypassingRole = $bypassingRole; - } - - /** - * {@inheritdoc} - */ - public function supportsAttribute($attribute) - { - return $attribute === 'VIEW'; - } - - /** - * {@inheritdoc} - */ - public function supportsClass($class) - { - return is_subclass_of($class, 'Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableInterface'); - } - - /** - * {@inheritdoc} - * - * @param PublishableInterface $object - */ - public function vote(TokenInterface $token, $object, array $attributes) - { - if (!$this->supportsClass(get_class($object)) - || !in_array('VIEW', $attributes) - ) { - return self::ACCESS_ABSTAIN; - } - - $context = $this->container->get('security.context'); - if ($this->bypassingRole && $context->isGranted($this->bypassingRole)) { - return self::ACCESS_ABSTAIN; - } - - if (! $object->isPublishable()) { - return self::ACCESS_DENIED; - } - - return self::ACCESS_ABSTAIN; - } -} \ No newline at end of file diff --git a/Security/Authorization/Voter/PublishedVoter.php b/Security/Authorization/Voter/PublishedVoter.php new file mode 100644 index 00000000..86ead327 --- /dev/null +++ b/Security/Authorization/Voter/PublishedVoter.php @@ -0,0 +1,72 @@ + + */ +class PublishedVoter implements VoterInterface +{ + /** + * @var PublishWorkflowChecker + */ + private $publishWorkflowChecker; + + /** + * @param PublishWorkflowChecker $publishWorkflowChecker + */ + public function __construct(PublishWorkflowChecker $publishWorkflowChecker) + { + $this->publishWorkflowChecker = $publishWorkflowChecker; + } + + /** + * {@inheritdoc} + */ + public function supportsAttribute($attribute) + { + return PublishWorkflowChecker::VIEW_ATTRIBUTE === $attribute + || PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE === $attribute + ; + } + + /** + * {@inheritdoc} + */ + public function supportsClass($class) + { + return $this->publishWorkflowChecker->supportsClass($class); + } + + /** + * {@inheritdoc} + * + * @param object $object + */ + public function vote(TokenInterface $token, $object, array $attributes) + { + if (!$this->supportsClass(get_class($object))) { + return self::ACCESS_ABSTAIN; + } + foreach($attributes as $attribute) { + if (! $this->supportsAttribute($attribute)) { + return self::ACCESS_ABSTAIN; + } + } + + if ($this->publishWorkflowChecker->isGranted($attributes, $object)) { + return self::ACCESS_GRANTED; + } + + return self::ACCESS_DENIED; + } +} \ No newline at end of file diff --git a/Twig/TwigExtension.php b/Twig/TwigExtension.php index 253697f5..87bc8f43 100644 --- a/Twig/TwigExtension.php +++ b/Twig/TwigExtension.php @@ -2,11 +2,13 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Twig; +use PHPCR\Util\PathHelper; use Doctrine\Common\Persistence\ManagerRegistry; use Doctrine\ODM\PHPCR\Exception\MissingTranslationException; use Doctrine\ODM\PHPCR\DocumentManager; -use PHPCR\Util\PathHelper; -use Symfony\Component\Security\Core\SecurityContext; +use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker; +use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; +use Symfony\Component\Security\Core\SecurityContextInterface; class TwigExtension extends \Twig_Extension { @@ -16,20 +18,20 @@ class TwigExtension extends \Twig_Extension protected $dm; /** - * @var SecurityContext + * @var SecurityContextInterface */ - protected $context; + protected $publishWorkflowChecker; /** * Instantiate the content controller. * - * @param SecurityContext $context + * @param SecurityContextInterface $publishWorkflowChecker * @param ManagerRegistry $registry * @param string $objectManagerName */ - public function __construct(SecurityContext $context, $registry = null, $objectManagerName = null) + public function __construct(SecurityContextInterface $publishWorkflowChecker = null, $registry = null, $objectManagerName = null) { - $this->context = $context; + $this->publishWorkflowChecker = $publishWorkflowChecker; if ($registry && $registry instanceof ManagerRegistry) { $this->dm = $registry->getManager($objectManagerName); @@ -43,6 +45,7 @@ public function __construct(SecurityContext $context, $registry = null, $objectM */ public function getFunctions() { + $functions = array('cmf_is_published' => new \Twig_Function_Method($this, 'isPublished')); if ($this->dm) { $functions['cmf_child'] = new \Twig_Function_Method($this, 'getChild'); $functions['cmf_children'] = new \Twig_Function_Method($this, 'getChildren'); @@ -121,8 +124,11 @@ public function find($path) /** * Get a document instance and validate if its eligible * - * @param string|object $document the id of a document or the document object itself - * @param Boolean|null $ignoreRole if the role should be ignored or null if publish workflow should be ignored + * @param string|object $document the id of a document or the document + * object itself + * @param boolean|null $ignoreRole whether the bypass role should be + * ignored (leading to only show published content regardless of the + * current user) or null to skip the published check completely. * @param null|string $class class name to filter on * * @return null|object @@ -132,9 +138,13 @@ private function getDocument($document, $ignoreRole = false, $class = null) if (is_string($document)) { $document = $this->dm->find(null, $document); } + if (null !== $ignoreRole && null === $this->publishWorkflowChecker) { + throw new InvalidConfigurationException('You can not fetch only published documents when the publishWorkflowChecker is not set. Either enable the publish workflow or pass "ignoreRole = null" to skip publication checks.'); + } if (empty($document) - || (null !== $ignoreRole && !$this->context->isGranted('VIEW', $document)) + || (false === $ignoreRole && !$this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $document)) + || (true === $ignoreRole && !$this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE, $document)) || (null != $class && !($document instanceof $class)) ) { return null; @@ -176,6 +186,26 @@ public function findMany($paths = array(), $limit = false, $offset = false, $ign return $result; } + /** + * Check if a document is published, regardless of the current users role. + * + * @param object $document + * + * @return boolean + */ + public function isPublished($document) + { + if (null === $this->publishWorkflowChecker) { + throw new InvalidConfigurationException('You can not check for publication as the publish workflow is not enabled.'); + } + + if (empty($document)) { + return false; + } + + return $this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE, true); + } + /** * Get the locales of the document * From 65ccec27876d95ef37fcef4d91d4180d15b0d095 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Thu, 27 Jun 2013 07:57:45 +0200 Subject: [PATCH 03/13] cleanup after feedback --- CHANGELOG.md | 2 +- EventListener/PublishWorkflowListener.php | 28 +++++++++++++++---- PublishWorkflow/PublishWorkflowChecker.php | 4 +-- .../Voter/PublishTimePeriodVoter.php | 28 +++++++++++-------- PublishWorkflow/Voter/PublishableVoter.php | 19 ++++++++----- Resources/config/publish_workflow.xml | 4 +-- Resources/config/schema/core-1.0.xsd | 10 +++++-- .../Authorization/Voter/PublishedVoter.php | 2 +- Twig/TwigExtension.php | 4 +-- 9 files changed, 67 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f6d2866..8a48ed5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ Changelog * **2013-06-20**: [PublishWorkflow] The PublishWorkflowChecker now implements SecurityContextInterface and the individual checks are moved to voters. Use the service cmf_core.publish_workflow.checker and call - `isGranted('VIEW', $content)` - or `'VIEW_PUBLISHED'` if you don't want to + `isGranted('VIEW', $content)` - or `'VIEW_ANONYMOUS'` if you don't want to see unpublished content even if the current user is allowed to see it. Configuration was adjusted: The parameter for the role that may see unpublished content moved from `role` to `publish_workflow.view_non_published_role`. diff --git a/EventListener/PublishWorkflowListener.php b/EventListener/PublishWorkflowListener.php index 2fcd6763..878f3c12 100644 --- a/EventListener/PublishWorkflowListener.php +++ b/EventListener/PublishWorkflowListener.php @@ -12,7 +12,8 @@ use Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter; /** - * Makes sure only published routes and content can be accessed. + * A request listener that makes sure only published routes and content can be + * accessed. * * @author David Buchmann */ @@ -23,12 +24,30 @@ class PublishWorkflowListener implements EventSubscriberInterface */ protected $publishWorkflowChecker; + /** + * The attribute to check with the workflow checker, typically VIEW or VIEW_ANONYMOUS + * + * @var string + */ + private $attribute; + /** * @param PublishWorkflowChecker $publishWorkflowChecker + * @param string $attribute the attribute name to check */ - public function __construct(PublishWorkflowChecker $publishWorkflowChecker) + public function __construct(PublishWorkflowChecker $publishWorkflowChecker, $attribute = 'VIEW') { $this->publishWorkflowChecker = $publishWorkflowChecker; + $this->attribute = $attribute; + } + + public function getAttribute() + { + return $this->attribute; + } + public function setAttribute($attribute) + { + $this->attribute = $attribute; } /** @@ -41,12 +60,12 @@ public function onKernelRequest(GetResponseEvent $event) $request = $event->getRequest(); $route = $request->attributes->get(DynamicRouter::ROUTE_KEY); - if ($route && !$this->publishWorkflowChecker->isGranted('VIEW', $route)) { + if ($route && !$this->publishWorkflowChecker->isGranted($this->getAttribute(), $route)) { throw new NotFoundHttpException('Route not found at: ' . $request->getPathInfo()); } $content = $request->attributes->get(DynamicRouter::CONTENT_KEY); - if ($content && !$this->publishWorkflowChecker->isGranted('VIEW', $content)) { + if ($content && !$this->publishWorkflowChecker->isGranted($this->getAttribute(), $content)) { throw new NotFoundHttpException('Content not found for: ' . $request->getPathInfo()); } } @@ -62,5 +81,4 @@ static public function getSubscribedEvents() KernelEvents::REQUEST => array(array('onKernelRequest', 1)), ); } - } diff --git a/PublishWorkflow/PublishWorkflowChecker.php b/PublishWorkflow/PublishWorkflowChecker.php index 361e6709..c5bd38e6 100644 --- a/PublishWorkflow/PublishWorkflowChecker.php +++ b/PublishWorkflow/PublishWorkflowChecker.php @@ -32,9 +32,9 @@ class PublishWorkflowChecker implements SecurityContextInterface * publication check is not wanted. * * The role exception is handled by the workflow checker, the individual - * voters should treat VIEW and VIEW_PUBLISHED the same. + * voters should treat VIEW and VIEW_ANONYMOUS the same. */ - const VIEW_PUBLISHED_ATTRIBUTE = 'VIEW_PUBLISHED'; + const VIEW_ANONYMOUS_ATTRIBUTE = 'VIEW_ANONYMOUS'; /** * @var ContainerInterface diff --git a/PublishWorkflow/Voter/PublishTimePeriodVoter.php b/PublishWorkflow/Voter/PublishTimePeriodVoter.php index 0800c5e0..f56cbd55 100644 --- a/PublishWorkflow/Voter/PublishTimePeriodVoter.php +++ b/PublishWorkflow/Voter/PublishTimePeriodVoter.php @@ -44,7 +44,7 @@ public function setCurrentTime(\DateTime $currentTime) public function supportsAttribute($attribute) { return PublishWorkflowChecker::VIEW_ATTRIBUTE === $attribute - || PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE === $attribute + || PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE === $attribute ; } @@ -66,21 +66,27 @@ public function vote(TokenInterface $token, $object, array $attributes) if (!$this->supportsClass(get_class($object))) { return self::ACCESS_ABSTAIN; } - foreach($attributes as $attribute) { - if (! $this->supportsAttribute($attribute)) { - return self::ACCESS_ABSTAIN; - } - } $startDate = $object->getPublishStartDate(); $endDate = $object->getPublishEndDate(); - if ((null === $startDate || $this->currentTime >= $startDate) && - (null === $endDate || $this->currentTime < $endDate) - ) { - return self::ACCESS_GRANTED; + $decision = self::ACCESS_GRANTED; + foreach($attributes as $attribute) { + if (! $this->supportsAttribute($attribute)) { + // there was an unsupported attribute in the request. + // now we only abstain or deny if we find a supported attribute + // and the content is not publishable + $decision = self::ACCESS_ABSTAIN; + continue; + } + + if ((null !== $startDate && $this->currentTime < $startDate) || + (null !== $endDate && $this->currentTime > $endDate) + ) { + return self::ACCESS_DENIED; + } } - return self::ACCESS_DENIED; + return $decision; } } diff --git a/PublishWorkflow/Voter/PublishableVoter.php b/PublishWorkflow/Voter/PublishableVoter.php index 30a898c3..44226d68 100644 --- a/PublishWorkflow/Voter/PublishableVoter.php +++ b/PublishWorkflow/Voter/PublishableVoter.php @@ -22,7 +22,7 @@ class PublishableVoter implements VoterInterface public function supportsAttribute($attribute) { return PublishWorkflowChecker::VIEW_ATTRIBUTE === $attribute - || PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE === $attribute + || PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE === $attribute ; } @@ -44,16 +44,21 @@ public function vote(TokenInterface $token, $object, array $attributes) if (!$this->supportsClass(get_class($object))) { return self::ACCESS_ABSTAIN; } + + $decision = self::ACCESS_GRANTED; foreach($attributes as $attribute) { if (! $this->supportsAttribute($attribute)) { - return self::ACCESS_ABSTAIN; + // there was an unsupported attribute in the request. + // now we only abstain or deny if we find a supported attribute + // and the content is not publishable + $decision = self::ACCESS_ABSTAIN; + continue; + } + if (! $object->isPublishable()) { + return self::ACCESS_DENIED; } } - if (! $object->isPublishable()) { - return self::ACCESS_DENIED; - } - - return self::ACCESS_GRANTED; + return $decision; } } \ No newline at end of file diff --git a/Resources/config/publish_workflow.xml b/Resources/config/publish_workflow.xml index d5fb00e5..9355e7c0 100644 --- a/Resources/config/publish_workflow.xml +++ b/Resources/config/publish_workflow.xml @@ -29,11 +29,11 @@ - + - + diff --git a/Resources/config/schema/core-1.0.xsd b/Resources/config/schema/core-1.0.xsd index 3d6c6d26..28e5aa1f 100644 --- a/Resources/config/schema/core-1.0.xsd +++ b/Resources/config/schema/core-1.0.xsd @@ -9,10 +9,14 @@ - - - + + + + + + + diff --git a/Security/Authorization/Voter/PublishedVoter.php b/Security/Authorization/Voter/PublishedVoter.php index 86ead327..facaf064 100644 --- a/Security/Authorization/Voter/PublishedVoter.php +++ b/Security/Authorization/Voter/PublishedVoter.php @@ -35,7 +35,7 @@ public function __construct(PublishWorkflowChecker $publishWorkflowChecker) public function supportsAttribute($attribute) { return PublishWorkflowChecker::VIEW_ATTRIBUTE === $attribute - || PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE === $attribute + || PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE === $attribute ; } diff --git a/Twig/TwigExtension.php b/Twig/TwigExtension.php index 87bc8f43..2177c360 100644 --- a/Twig/TwigExtension.php +++ b/Twig/TwigExtension.php @@ -144,7 +144,7 @@ private function getDocument($document, $ignoreRole = false, $class = null) if (empty($document) || (false === $ignoreRole && !$this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $document)) - || (true === $ignoreRole && !$this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE, $document)) + || (true === $ignoreRole && !$this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $document)) || (null != $class && !($document instanceof $class)) ) { return null; @@ -203,7 +203,7 @@ public function isPublished($document) return false; } - return $this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_PUBLISHED_ATTRIBUTE, true); + return $this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, true); } /** From e4e1275e80c2735ab13b238a81618c3e23ca8383 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Sun, 30 Jun 2013 22:43:35 +0200 Subject: [PATCH 04/13] fix tests and cleanup issues --- DependencyInjection/Configuration.php | 2 +- PublishWorkflow/PublishWorkflowChecker.php | 1 + .../PublishWorkflow/PublishWorkflowTest.php | 103 +++++++ .../Twig/TwigExtensionHierarchyTest.php | 18 +- .../PublishWorkflowCheckerTest.php | 281 ++++++++---------- .../Voter/PublishTimePeriodVoterTest.php | 131 ++++++++ .../Voter/PublishableVoterTest.php | 98 ++++++ Tests/Unit/Twig/TwigExtensionTest.php | 88 +++++- Twig/TwigExtension.php | 2 +- composer.json | 2 +- 10 files changed, 557 insertions(+), 169 deletions(-) create mode 100644 Tests/Functional/PublishWorkflow/PublishWorkflowTest.php create mode 100644 Tests/Unit/PublishWorkflow/Voter/PublishTimePeriodVoterTest.php create mode 100644 Tests/Unit/PublishWorkflow/Voter/PublishableVoterTest.php diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index ae4019ca..30e4d26d 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -19,7 +19,7 @@ public function getConfigTreeBuilder() ->addDefaultsIfNotSet() ->children() ->booleanNode('enabled')->defaultTrue()->end() - ->scalarNode('view_non_published_role')->defaultValue('CAN_VIEW_NON_PUBLISHED')->end() + ->scalarNode('view_non_published_role')->defaultValue('ROLE_CAN_VIEW_NON_PUBLISHED')->end() ->booleanNode('request_listener')->defaultTrue()->end() ->end() ->end() diff --git a/PublishWorkflow/PublishWorkflowChecker.php b/PublishWorkflow/PublishWorkflowChecker.php index c5bd38e6..1770095d 100644 --- a/PublishWorkflow/PublishWorkflowChecker.php +++ b/PublishWorkflow/PublishWorkflowChecker.php @@ -116,6 +116,7 @@ public function isGranted($attributes, $object = null) } $securityContext = $this->container->get('security.context'); + if (null !== $securityContext->getToken() && (count($attributes) === 1) && self::VIEW_ATTRIBUTE === reset($attributes) diff --git a/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php b/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php new file mode 100644 index 00000000..bfa749ba --- /dev/null +++ b/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php @@ -0,0 +1,103 @@ +container = $this->getContainer(); + $this->pwc = $this->container->get('cmf_core.publish_workflow.checker'); + } + + public function testPublishable() + { + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $doc->expects($this->any()) + ->method('isPublishable') + ->will($this->returnValue(true)) + ; + + $this->assertTrue($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $doc)); + $this->assertTrue($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $doc)); + } + + public function testPublishPeriod() + { + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $doc->expects($this->any()) + ->method('isPublishable') + ->will($this->returnValue(true)) + ; + $doc->expects($this->any()) + ->method('getPublishEndDate') + ->will($this->returnValue(new \DateTime('01/01/1980'))) + ; + + $this->assertFalse($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $doc)); + $this->assertFalse($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $doc)); + } + + public function testIgnoreRoleHas() + { + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $doc->expects($this->any()) + ->method('isPublishable') + ->will($this->returnValue(false)) + ; + $roles = array( + new Role('ROLE_CAN_VIEW_NON_PUBLISHED') + ); + $token = new UsernamePasswordToken('test', 'pass', 'testprovider', $roles); + $context = $this->container->get('security.context'); + $context->setToken($token); + + $this->assertTrue($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $doc)); + $this->assertFalse($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $doc)); + } + + public function testIgnoreRoleNotHas() + { + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $doc->expects($this->any()) + ->method('isPublishable') + ->will($this->returnValue(false)) + ; + $roles = array( + new Role('OTHER_ROLE') + ); + $token = new UsernamePasswordToken('test', 'pass', 'testprovider', $roles); + /** @var $context SecurityContext */ + $context = $this->container->get('security.context'); + $context->setToken($token); + + $this->assertFalse($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $doc)); + $this->assertFalse($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $doc)); + } +} diff --git a/Tests/Functional/Twig/TwigExtensionHierarchyTest.php b/Tests/Functional/Twig/TwigExtensionHierarchyTest.php index 737994dd..790d93da 100644 --- a/Tests/Functional/Twig/TwigExtensionHierarchyTest.php +++ b/Tests/Functional/Twig/TwigExtensionHierarchyTest.php @@ -2,18 +2,29 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Tests\Functional\Twig; +use Doctrine\Bundle\PHPCRBundle\ManagerRegistry; +use PHPCR\SessionInterface; use Symfony\Cmf\Bundle\CoreBundle\Twig\TwigExtension; use Symfony\Cmf\Component\Testing\Functional\BaseTestCase; +use Symfony\Component\Security\Core\SecurityContextInterface; class TwigExtensionHierarchyTest extends BaseTestCase { + /** + * @var SecurityContextInterface|\PHPUnit_Framework_MockObject_MockObject + */ private $pwc; + + /** + * @var TwigExtension + */ private $extension; public function setUp() { $container = $this->getContainer(); $managerRegistry = $container->get('doctrine_phpcr'); + /** @var $session SessionInterface */ $session = $managerRegistry->getConnection(); $root = $session->getRootNode(); if ($root->hasNode('a')) { @@ -31,10 +42,11 @@ public function setUp() $session->save(); - $this->pwc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowCheckerInterface'); + $this->pwc = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface'); $this->pwc->expects($this->any()) - ->method('checkIsPublished') - ->will($this->returnValue(true)); + ->method('isGranted') + ->will($this->returnValue(true)) + ; $this->extension = new TwigExtension($this->pwc, $managerRegistry, 'default'); } diff --git a/Tests/Unit/PublishWorkflow/PublishWorkflowCheckerTest.php b/Tests/Unit/PublishWorkflow/PublishWorkflowCheckerTest.php index 680a6b9f..b900dd25 100644 --- a/Tests/Unit/PublishWorkflow/PublishWorkflowCheckerTest.php +++ b/Tests/Unit/PublishWorkflow/PublishWorkflowCheckerTest.php @@ -3,187 +3,158 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Tests\Unit\PublishWorkflow; use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker; +use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\SecurityContextInterface; class PublishWorkflowCheckerTest extends \PHPUnit_Framework_Testcase { + /** + * @var PublishWorkflowChecker + */ + private $pwfc; + + /** + * @var string + */ + private $role; + + /** + * @var ContainerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $container; + + /** + * @var SecurityContextInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $sc; + + /** + * @var PublishWorkflowInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $doc; + + /** + * @var AccessDecisionManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $adm; + public function setUp() { $this->role = 'IS_FOOBAR'; + $this->container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); $this->sc = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface'); + $this->container + ->expects($this->any()) + ->method('get') + ->with('security.context') + ->will($this->returnValue($this->sc)) + ; $this->doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $this->adm = $this->getMock('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface'); $this->stdClass = new \stdClass; - $this->pwfc = new PublishWorkflowChecker($this->role, $this->sc); + $this->pwfc = new PublishWorkflowChecker($this->container, $this->adm, $this->role); } - public function testDocDoesntImplementInterface() + /** + * Calling + */ + public function testIsGranted() { - $res = $this->pwfc->checkIsPublished($this->stdClass); - $this->assertTrue($res); + $token = new AnonymousToken('', ''); + $this->sc->expects($this->any()) + ->method('getToken') + ->will($this->returnValue($token)) + ; + $this->sc->expects($this->never()) + ->method('isGranted') + ; + $this->adm->expects($this->once()) + ->method('decide') + ->with($token, array(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE), $this->doc) + ->will($this->returnValue(true)) + ; + + $this->assertTrue($this->pwfc->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $this->doc)); } - public function providePublishWorkflowChecker() + public function testNotHasBypassRole() { - return array( - array(array( - 'expected' => true, - 'granted_role' => 'IS_FOOBAR', - 'is_publishable' => false, - )), - array(array( - 'expected' => true, - 'is_publishable' => true, - )), - array(array( - 'expected' => true, - 'granted_role' => 'TEST-3', - 'start_date' => new \DateTime('2000-01-01'), - 'end_date' => new \DateTime('2030-01-01'), - 'is_publishable' => true, - )), - array(array( - 'expected' => false, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => new \DateTime('01/01/2000'), - 'end_date' => new \DateTime('01/01/2001'), - 'is_publishable' => true, - )), - array(array( - 'expected' => false, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => new \DateTime('01/01/2000'), - 'end_date' => new \DateTime('01/01/2030'), - 'is_publishable' => false, - )), - array(array( - 'expected' => true, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => new \DateTime('01/01/2000'), - 'end_date' => null, - 'is_publishable' => true, - )), - array(array( - 'expected' => false, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => null, - 'end_date' => new \DateTime('01/01/2000'), - 'is_publishable' => true, - )), - array(array( - 'expected' => true, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => null, - 'end_date' => new \DateTime('01/01/2030'), - 'is_publishable' => true, - )), - array(array( - 'expected' => true, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => null, - 'end_date' => null, - 'is_publishable' => null, - )), - array(array( - 'expected' => true, - 'granted_role' => 'TEST-3', - 'start_date' => new \DateTime('2000-01-01'), - 'end_date' => new \DateTime('2030-01-01'), - 'is_publishable' => null, - )), - array(array( - 'expected' => false, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => new \DateTime('01/01/2000'), - 'end_date' => new \DateTime('01/01/2001'), - 'is_publishable' => null, - )), - array(array( - 'expected' => false, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => new \DateTime('01/01/2000'), - 'end_date' => new \DateTime('01/01/2030'), - 'is_publishable' => false, - )), - array(array( - 'expected' => true, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => new \DateTime('01/01/2000'), - 'end_date' => null, - 'is_publishable' => null, - )), - array(array( - 'expected' => false, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => null, - 'end_date' => new \DateTime('01/01/2000'), - 'is_publishable' => null, - )), - array(array( - 'expected' => true, - 'granted_role' => 'UNAUTH_ROLE', - 'start_date' => null, - 'end_date' => new \DateTime('01/01/2030'), - 'is_publishable' => null, - )), - // Test overwrite current time - array(array( - 'expected' => false, - 'is_publishable' => true, - 'end_date' => new \DateTime('01/01/2000'), - 'current_time' => new \DateTime('01/01/2001'), - )), - array(array( - 'expected' => true, - 'is_publishable' => true, - 'end_date' => new \DateTime('01/01/2000'), - 'current_time' => new \DateTime('01/01/1980'), - )), - ); + $token = new AnonymousToken('', ''); + $this->sc->expects($this->any()) + ->method('getToken') + ->will($this->returnValue($token)) + ; + $this->sc->expects($this->once()) + ->method('isGranted') + ->with($this->role) + ->will($this->returnValue(false)) + ; + $this->adm->expects($this->once()) + ->method('decide') + ->with($token, array(PublishWorkflowChecker::VIEW_ATTRIBUTE), $this->doc) + ->will($this->returnValue(true)) + ; + + $this->assertTrue($this->pwfc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $this->doc)); } - /** - * @dataProvider providePublishWorkflowChecker - */ - public function testPublishWorkflowChecker($options) + public function testHasBypassRole() { - $options = array_merge(array( - 'expected' => false, - 'granted_role' => 'NONE', - 'start_date' => null, - 'end_date' => null, - 'is_publishable' => null, - 'current_time' => null, - ), $options); - + $token = new AnonymousToken('', ''); $this->sc->expects($this->any()) + ->method('getToken') + ->will($this->returnValue($token)) + ; + $this->sc->expects($this->once()) ->method('isGranted') - ->will($this->returnCallback(function ($given) use ($options) { - return $given === $options['granted_role']; - })); + ->with($this->role) + ->will($this->returnValue(true)) + ; + $this->adm->expects($this->never()) + ->method('decide') + ; - $this->doc->expects($this->any()) - ->method('getPublishStartDate') - ->will($this->returnValue($options['start_date'])); - - $this->doc->expects($this->any()) - ->method('getPublishEndDate') - ->will($this->returnValue($options['end_date'])); + $this->assertTrue($this->pwfc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $this->doc)); + } - $this->doc->expects($this->any()) - ->method('isPublishable') - ->will($this->returnValue($options['is_publishable'])); + public function testNoFirewall() + { + $token = new AnonymousToken('', ''); + $this->sc->expects($this->any()) + ->method('getToken') + ->will($this->returnValue(null)) + ; + $this->sc->expects($this->never()) + ->method('isGranted') + ; + $this->adm->expects($this->once()) + ->method('decide') + ->with($token, array(PublishWorkflowChecker::VIEW_ATTRIBUTE), $this->doc) + ->will($this->returnValue(true)) + ; - if ($options['current_time']) { - $this->pwfc->setCurrentTime($options['current_time']); - } + $this->assertTrue($this->pwfc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $this->doc)); + } - $res = $this->pwfc->checkIsPublished($this->doc); + public function testSetToken() + { + $token = new AnonymousToken('x', 'y'); + $this->pwfc->setToken($token); + $this->assertSame($token, $this->pwfc->getToken()); + } - if (true === $options['expected']) { - $this->assertTrue($res); - } else { - $this->assertFalse($res); - } + public function testSupportsClass() + { + $class = 'Test\Class'; + $this->adm->expects($this->once()) + ->method('supportsClass') + ->with($class) + ->will($this->returnValue(true)) + ; + $this->assertTrue($this->pwfc->supportsClass($class)); } } diff --git a/Tests/Unit/PublishWorkflow/Voter/PublishTimePeriodVoterTest.php b/Tests/Unit/PublishWorkflow/Voter/PublishTimePeriodVoterTest.php new file mode 100644 index 00000000..458aed21 --- /dev/null +++ b/Tests/Unit/PublishWorkflow/Voter/PublishTimePeriodVoterTest.php @@ -0,0 +1,131 @@ +voter = new PublishTimePeriodVoter(); + $this->token = new AnonymousToken('', ''); + } + + public function providePublishWorkflowChecker() + { + return array( + array( + 'expected' => VoterInterface::ACCESS_GRANTED, + 'startDate' => new \DateTime('01/01/2000'), + 'endDate' => new \DateTime('01/02/2030'), + ), + array( + 'expected' => VoterInterface::ACCESS_DENIED, + 'startDate' => new \DateTime('01/01/2000'), + 'endDate' => new \DateTime('01/01/2001'), + ), + array( + 'expected' => VoterInterface::ACCESS_GRANTED, + 'startDate' => new \DateTime('01/01/2000'), + 'endDate' => null, + ), + array( + 'expected' => VoterInterface::ACCESS_DENIED, + 'startDate' => new \DateTime('01/01/2030'), + 'endDate' => null, + ), + array( + 'expected' => VoterInterface::ACCESS_GRANTED, + 'startDate' => null, + 'endDate' => new \DateTime('01/01/2030'), + ), + array( + 'expected' => VoterInterface::ACCESS_DENIED, + 'startDate' => null, + 'endDate' => new \DateTime('01/01/2000'), + ), + array( + 'expected' => VoterInterface::ACCESS_GRANTED, + 'startDate' => null, + 'endDate' => null, + ), + // unsupported attribute + array( + 'expected' => VoterInterface::ACCESS_ABSTAIN, + 'startDate' => new \DateTime('01/01/2000'), + 'endDate' => new \DateTime('01/01/2030'), + 'attributes' => array(PublishWorkflowChecker::VIEW_ATTRIBUTE, 'other'), + ), + // Test overwrite current time + array( + 'expected' => VoterInterface::ACCESS_DENIED, + 'startDate' => null, + 'endDate' => new \DateTime('01/01/2030'), + 'attributes' => PublishWorkflowChecker::VIEW_ATTRIBUTE, + 'currentTime' => new \DateTime('02/02/2030'), + ), + array( + 'expected' => VoterInterface::ACCESS_GRANTED, + 'startDate' => null, + 'endDate' => new \DateTime('01/01/2000'), + 'attributes' => PublishWorkflowChecker::VIEW_ATTRIBUTE, + 'currentTime' => new \DateTime('01/01/1980'), + ), + ); + } + + /** + * @dataProvider providePublishWorkflowChecker + */ + public function testPublishWorkflowChecker($expected, $startDate, $endDate, $attributes = PublishWorkflowChecker::VIEW_ATTRIBUTE, $currentTime = false) + { + $attributes = (array) $attributes; + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishTimePeriodInterface'); + + $doc->expects($this->any()) + ->method('getPublishStartDate') + ->will($this->returnValue($startDate)) + ; + + $doc->expects($this->any()) + ->method('getPublishEndDate') + ->will($this->returnValue($endDate)) + ; + + if (false !== $currentTime) { + $this->voter->setCurrentTime($currentTime); + } + + $this->assertEquals($expected, $this->voter->vote($this->token, $doc, $attributes)); + + } + + public function testUnsupportedClass() + { + $result = $this->voter->vote( + $this->token, + $this, + array(PublishWorkflowChecker::VIEW_ATTRIBUTE) + ); + $this->assertEquals(VoterInterface::ACCESS_ABSTAIN, $result); + } +} diff --git a/Tests/Unit/PublishWorkflow/Voter/PublishableVoterTest.php b/Tests/Unit/PublishWorkflow/Voter/PublishableVoterTest.php new file mode 100644 index 00000000..03acea10 --- /dev/null +++ b/Tests/Unit/PublishWorkflow/Voter/PublishableVoterTest.php @@ -0,0 +1,98 @@ +voter = new PublishableVoter(); + $this->token = new AnonymousToken('', ''); + } + + public function providePublishWorkflowChecker() + { + return array( + array( + 'expected' => VoterInterface::ACCESS_GRANTED, + 'isPublishable' => true, + 'attributes' => PublishWorkflowChecker::VIEW_ATTRIBUTE, + ), + array( + 'expected' => VoterInterface::ACCESS_DENIED, + 'isPublishable' => false, + 'attributes' => PublishWorkflowChecker::VIEW_ATTRIBUTE, + ), + array( + 'expected' => VoterInterface::ACCESS_GRANTED, + 'isPublishable' => true, + 'attributes' => array( + PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, + PublishWorkflowChecker::VIEW_ATTRIBUTE, + ), + ), + array( + 'expected' => VoterInterface::ACCESS_DENIED, + 'isPublishable' => false, + 'attributes' => PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, + ), + array( + 'expected' => VoterInterface::ACCESS_ABSTAIN, + 'isPublishable' => true, + 'attributes' => 'other', + ), + array( + 'expected' => VoterInterface::ACCESS_ABSTAIN, + 'isPublishable' => true, + 'attributes' => array(PublishWorkflowChecker::VIEW_ATTRIBUTE, 'other'), + ), + ); + } + + /** + * @dataProvider providePublishWorkflowChecker + * + * use for voters! + */ + public function testPublishWorkflowChecker($expected, $isPublishable, $attributes) + { + $attributes = (array) $attributes; + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableInterface'); + $doc->expects($this->any()) + ->method('isPublishable') + ->will($this->returnValue($isPublishable)) + ; + + $this->assertEquals($expected, $this->voter->vote($this->token, $doc, $attributes)); + } + + public function testUnsupportedClass() + { + $result = $this->voter->vote( + $this->token, + $this, + array(PublishWorkflowChecker::VIEW_ATTRIBUTE) + ); + $this->assertEquals(VoterInterface::ACCESS_ABSTAIN, $result); + } +} diff --git a/Tests/Unit/Twig/TwigExtensionTest.php b/Tests/Unit/Twig/TwigExtensionTest.php index 7bfc8972..2956d3bd 100644 --- a/Tests/Unit/Twig/TwigExtensionTest.php +++ b/Tests/Unit/Twig/TwigExtensionTest.php @@ -2,6 +2,7 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Tests\Unit\Twig; +use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker; use Symfony\Cmf\Bundle\CoreBundle\Twig\TwigExtension; class TwigExtensionTest extends \PHPUnit_Framework_TestCase @@ -10,11 +11,14 @@ class TwigExtensionTest extends \PHPUnit_Framework_TestCase private $managerRegistry; private $manager; private $uow; + /** + * @var TwigExtension + */ private $extension; public function setUp() { - $this->pwc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowCheckerInterface'); + $this->pwc = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface'); $this->managerRegistry = $this->getMockBuilder('Doctrine\Bundle\PHPCRBundle\ManagerRegistry') ->disableOriginalConstructor() @@ -100,6 +104,19 @@ public function testGetPath() $this->assertEquals('/foo/bar', $this->extension->getPath($document)); } + public function testGetPathInvalid() + { + $document = new \stdClass(); + + $this->uow->expects($this->once()) + ->method('getDocumentId') + ->with($document) + ->will($this->throwException(new \Exception('test'))); + ; + + $this->assertFalse($this->extension->getPath($document)); + } + public function testFind() { $document = new \stdClass(); @@ -140,17 +157,32 @@ public function testFindManyIgnoreRole() $this->manager->expects($this->any()) ->method('find') - ->will($this->onConsecutiveCalls($documentA, null, $documentA, $documentB)) + ->will($this->onConsecutiveCalls($documentA, $documentB)) ; $this->pwc->expects($this->any()) - ->method('checkIsPublished') - ->with($documentA) + ->method('isGranted') ->will($this->onConsecutiveCalls(false, true)) ; - $this->assertEquals(array($documentA), $this->extension->findMany(array('/foo', 'bar'), false, false, null)); - $this->assertEquals(array($documentB), $this->extension->findMany(array('/foo', 'bar'), false, false, false)); + $this->assertEquals(array($documentB), $this->extension->findMany(array('/foo', '/bar'), false, false, true)); + } + + public function testFindManyIgnoreWorkflow() + { + $documentA = new \stdClass(); + $documentB = new \stdClass(); + + $this->manager->expects($this->any()) + ->method('find') + ->will($this->onConsecutiveCalls($documentA, $documentB)) + ; + + $this->pwc->expects($this->never()) + ->method('isGranted') + ; + + $this->assertEquals(array($documentA, $documentB), $this->extension->findMany(array('/foo', '/bar'), false, false, null)); } public function testFindManyLimitOffset() @@ -168,6 +200,24 @@ public function testFindManyLimitOffset() $this->assertEquals(array($documentB), $this->extension->findMany(array('/foo', 'bar'), 1, 1, null)); } + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + */ + public function testFindManyNoWorkflow() + { + $this->extension = new TwigExtension(null, $this->managerRegistry, 'foo'); + + $documentA = new \stdClass(); + + $this->manager->expects($this->any()) + ->method('find') + ->with(null, '/foo') + ->will($this->returnValue($documentA)) + ; + + $this->extension->findMany(array('/foo', '/bar'), false, false); + } + public function testIsPublished() { $this->assertFalse($this->extension->isPublished(null)); @@ -175,8 +225,8 @@ public function testIsPublished() $document = new \stdClass(); $this->pwc->expects($this->any()) - ->method('checkIsPublished') - ->with($document) + ->method('isGranted') + ->with(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $document) ->will($this->onConsecutiveCalls(false, true)) ; @@ -184,6 +234,15 @@ public function testIsPublished() $this->assertTrue($this->extension->isPublished($document)); } + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + */ + public function testIsPublishedNoWorkflow() + { + $this->extension = new TwigExtension(null, $this->managerRegistry, 'foo'); + $this->extension->isPublished(new \stdClass()); + } + public function testGetLocalesFor() { $this->assertEquals(array(), $this->extension->getLocalesFor(null)); @@ -235,6 +294,19 @@ public function testGetChild() $this->assertEquals($child, $this->extension->getChild($parent, 'bar')); } + public function testGetChildError() + { + $parent = new \stdClass(); + + $this->uow->expects($this->once()) + ->method('getDocumentId') + ->with($parent) + ->will($this->throwException(new \Exception('test'))) + ; + + $this->assertFalse($this->extension->getChild($parent, 'bar')); + } + public function testGetChildren() { $parent = new \stdClass(); diff --git a/Twig/TwigExtension.php b/Twig/TwigExtension.php index 2177c360..6c00c41a 100644 --- a/Twig/TwigExtension.php +++ b/Twig/TwigExtension.php @@ -203,7 +203,7 @@ public function isPublished($document) return false; } - return $this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, true); + return $this->publishWorkflowChecker->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $document); } /** diff --git a/composer.json b/composer.json index feab7f61..67002db4 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ "symfony/framework-bundle": "~2.1" }, "require-dev": { - "symfony-cmf/routing": "~1.1-dev", + "symfony-cmf/routing-bundle": "~1.1-dev", "symfony-cmf/testing": "~1.0-dev", "sonata-project/admin-bundle": "2.2.*" }, From bdf22d761969057a8b133ddc1702f3bf807a25ff Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Tue, 2 Jul 2013 22:07:31 +0200 Subject: [PATCH 05/13] adding sequence info to xml schema --- Resources/config/schema/core-1.0.xsd | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Resources/config/schema/core-1.0.xsd b/Resources/config/schema/core-1.0.xsd index 28e5aa1f..4ad47259 100644 --- a/Resources/config/schema/core-1.0.xsd +++ b/Resources/config/schema/core-1.0.xsd @@ -8,6 +8,9 @@ + + + From c7a67aee420330339eb641a91fb7bdc6b1c909df Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 5 Jul 2013 22:43:03 +0200 Subject: [PATCH 06/13] totally refactored the handling for depth in the CmfHelper --- .travis.yml | 4 +- Templating/Helper/CmfHelper.php | 330 ++++++++++++------ .../Helper/CmfHelperHierarchyTest.php | 212 +++++++++++ .../Extension/CmfExtensionHierarchyTest.php | 134 ------- Tests/Resources/Document/RouteAware.php | 24 ++ 5 files changed, 470 insertions(+), 234 deletions(-) create mode 100644 Tests/Functional/Templating/Helper/CmfHelperHierarchyTest.php delete mode 100644 Tests/Functional/Twig/Extension/CmfExtensionHierarchyTest.php create mode 100644 Tests/Resources/Document/RouteAware.php diff --git a/.travis.yml b/.travis.yml index e045108e..b21bab46 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,11 +3,11 @@ language: php php: - 5.3 - 5.4 + - 5.5 env: -# - SYMFONY_VERSION=2.1.* - SYMFONY_VERSION=2.2.* -# - SYMFONY_VERSION=2.3.* + - SYMFONY_VERSION=2.3.* # - SYMFONY_VERSION=dev-master before_script: diff --git a/Templating/Helper/CmfHelper.php b/Templating/Helper/CmfHelper.php index 7f80314e..b50b0d7a 100644 --- a/Templating/Helper/CmfHelper.php +++ b/Templating/Helper/CmfHelper.php @@ -101,18 +101,12 @@ public function find($path) /** * Gets a document instance and validate if its eligible. * -<<<<<<< HEAD:Twig/TwigExtension.php * @param string|object $document the id of a document or the document * object itself * @param boolean|null $ignoreRole whether the bypass role should be * ignored (leading to only show published content regardless of the * current user) or null to skip the published check completely. * @param null|string $class class name to filter on -======= - * @param string|object $document the id of a document or the document object itself - * @param Boolean|null $ignoreRole if the role should be ignored or null if publish workflow should be ignored - * @param null|string $class class name to filter on ->>>>>>> origin/master:Templating/Helper/CmfHelper.php * * @return null|object */ @@ -171,19 +165,11 @@ public function findMany($paths = array(), $limit = false, $offset = false, $ign } /** -<<<<<<< HEAD:Twig/TwigExtension.php * Check if a document is published, regardless of the current users role. * * @param object $document * * @return boolean -======= - * Checks if a document is published. - * - * @param string $document - * - * @return Boolean ->>>>>>> origin/master:Templating/Helper/CmfHelper.php */ public function isPublished($document) { @@ -377,38 +363,28 @@ public function getDescendants($parent, $depth = null) /** * Check children for a possible following document * - * @param \Traversable $childNames - * @param Boolean $reverse - * @param string $parentPath + * @param array $childNames + * @param string $path * @param Boolean $ignoreRole * @param null|string $class - * @param null|string $nodeName * * @return null|object */ - private function checkChildren($childNames, $reverse, $parentPath, $ignoreRole = false, $class = null, $nodeName = null) + private function checkChildren(array $childNames, $path, $ignoreRole = false, $class = null) { - if ($reverse) { - $childNames = array_reverse($childNames->getArrayCopy()); - } - - $check = empty($nodeName); foreach ($childNames as $name) { if (strpos($name, 'phpcr_locale:') === 0) { continue; } - if ($check) { - try { - $child = $this->getDocument("$parentPath/$name", $ignoreRole, $class); - if ($child) { - return $child; - } - } catch (MissingTranslationException $e) { - continue; - } - } elseif ($nodeName == $name) { - $check = true; + try { + $child = $this->getDocument(ltrim($path, '/')."/$name", $ignoreRole, $class); + } catch (MissingTranslationException $e) { + continue; + } + + if ($child) { + return $child; } } @@ -416,130 +392,288 @@ private function checkChildren($childNames, $reverse, $parentPath, $ignoreRole = } /** - * Search for a following document + * Traverse the depth to find previous documents * - * @param string|object $path document instance or path - * @param string|object $anchor document instance or path - * @param null|integer $depth - * @param Boolean $reverse - * @param Boolean $ignoreRole - * @param null|string $class + * @param null|integer $depth + * @param integer $anchorDepth + * @param array $childNames + * @param string $path + * @param Boolean $ignoreRole + * @param null|string $class * * @return null|object */ - private function search($path, $anchor = null, $depth = null, $reverse = false, $ignoreRole = false, $class = null) + private function traversePrevDepth($depth, $anchorDepth, array $childNames, $path, $ignoreRole, $class) { - if (empty($path)) { - return null; + foreach ($childNames as $childName) { + $childPath = "$path/$childName"; + $node = $this->dm->getPhpcrSession()->getNode($childPath); + if (null === $depth || PathHelper::getPathDepth($childPath) - $anchorDepth < $depth) { + $childNames = $node->getNodeNames()->getArrayCopy(); + if (!empty($childNames)) { + $childNames = array_reverse($childNames); + $result = $this->traversePrevDepth($depth, $anchorDepth, $childNames, $childPath, $ignoreRole, $class); + if ($result) { + return $result; + } + } + } + + $result = $this->checkChildren($childNames, $node->getPath(), $ignoreRole, $class); + if ($result) { + return $result; + } } + } + /** + * Search for a previous document + * + * @param string|object $path document instance or path from which to search + * @param string|object $anchor document instance or path which serves as an anchor from which to flatten the hierarchy + * @param null|integer $depth depth up to which to traverse down the tree when an anchor is provided + * @param Boolean $ignoreRole if to ignore the role + * @param null|string $class the class to filter by + * + * @return null|object + */ + private function searchDepthPrev($path, $anchor, $depth = null, $ignoreRole = false, $class = null) + { if (is_object($path)) { $path = $this->dm->getUnitOfWork()->getDocumentId($path); } + if (null === $path || '/' === $path) { + return null; + } + $node = $this->dm->getPhpcrSession()->getNode($path); - if ($anchor) { - if (is_object($anchor)) { - $anchor = $this->dm->getUnitOfWork()->getDocumentId($anchor); - } + if (is_object($anchor)) { + $anchor = $this->dm->getUnitOfWork()->getDocumentId($anchor); + } + + if (0 !== strpos($path, $anchor)) { + throw new \RuntimeException("The anchor path '$anchor' is not a parent of the current path '$path'."); + } + + if ($path === $anchor) { + return null; + } - if (strpos($path, $anchor) !== 0) { - throw new \RuntimeException("The anchor path '$anchor' is not a parent of the current path '$path'."); + $parent = $node->getParent(); + $parentPath = $parent->getPath(); + + $childNames = $parent->getNodeNames()->getArrayCopy(); + if (!empty($childNames)) { + $childNames = array_reverse($childNames); + $key = array_search($node->getName(), $childNames); + $childNames = array_slice($childNames, $key + 1); + } + + // traverse the previous siblings down the tree + $result = $this->traversePrevDepth($depth, PathHelper::getPathDepth($anchor), $childNames, $parentPath, $ignoreRole, $class); + if ($result) { + return $result; + } + + // check siblings + $result = $this->checkChildren($childNames, $parentPath, $ignoreRole, $class); + if ($result) { + return $result; + } + + // check parents + // TODO do we need to traverse towards the anchor? + if (0 === strpos($parentPath, $anchor)) { + $parent = $parent->getParent(); + $childNames = $parent->getNodeNames()->getArrayCopy(); + $key = array_search(PathHelper::getNodeName($parentPath), $childNames); + $childNames = array_slice($childNames, 0, $key + 1); + $childNames = array_reverse($childNames); + $result = $this->checkChildren($childNames, $parent->getPath(), $ignoreRole, $class); + if ($result) { + return $result; } + } - if (!$reverse - && (null === $depth || PathHelper::getPathDepth($path() - PathHelper::getPathDepth($anchor)) < $depth) - ) { - $childNames = $node->getNodeNames(); - if ($childNames->count()) { - $result = $this->checkChildren($childNames, $reverse, $path, $ignoreRole, $class); - if ($result) { - return $result; - } - } + return null; + } + + /** + * Search for a next document + * + * @param string|object $path document instance or path from which to search + * @param string|object $anchor document instance or path which serves as an anchor from which to flatten the hierarchy + * @param null|integer $depth depth up to which to traverse down the tree when an anchor is provided + * @param Boolean $ignoreRole if to ignore the role + * @param null|string $class the class to filter by + * + * @return null|object + */ + private function searchDepthNext($path, $anchor, $depth = null, $ignoreRole = false, $class = null) + { + if (is_object($path)) { + $path = $this->dm->getUnitOfWork()->getDocumentId($path); + } + + if (null === $path || '/' === $path) { + return null; + } + + $node = $this->dm->getPhpcrSession()->getNode($path); + + if (is_object($anchor)) { + $anchor = $this->dm->getUnitOfWork()->getDocumentId($anchor); + } + + if (0 !== strpos($path, $anchor)) { + throw new \RuntimeException("The anchor path '$anchor' is not a parent of the current path '$path'."); + } + + // take the first eligible child if there are any + // TODO do we need to traverse away from the anchor up to the depth here? + if (null === $depth || PathHelper::getPathDepth($path) - PathHelper::getPathDepth($anchor) < $depth) { + $childNames = $node->getNodeNames()->getArrayCopy(); + $result = $this->checkChildren($childNames, $path, $ignoreRole, $class); + if ($result) { + return $result; } } - $nodename = $node->getName(); + $parent = $node->getParent(); + $parentPath = PathHelper::getParentPath($path); - do { - $parentNode = $node->getParent(); - $childNames = $parentNode->getNodeNames(); - $result = $this->checkChildren($childNames, $reverse, $parentNode->getPath(), $ignoreRole, $class, $nodename); - if ($result || !$anchor) { + // take the first eligible sibling + if (0 === strpos($parentPath, $anchor)) { + $childNames = $parent->getNodeNames()->getArrayCopy(); + $key = array_search($node->getName(), $childNames); + $childNames = array_slice($childNames, $key + 1); + $result = $this->checkChildren($childNames, $parentPath, $ignoreRole, $class); + if ($result) { return $result; } + } + + // take the first eligible parent, traverse up + while ('/' !== $parentPath) { + $parent = $parent->getParent(); + if (false === strpos($parent->getPath(), $anchor)) { + return null; + } - $node = $parentNode; - if ($nodename) { - $reverse = !$reverse; - $nodename = null; + $childNames = $parent->getNodeNames()->getArrayCopy(); + $key = array_search(PathHelper::getNodeName($parentPath), $childNames); + $childNames = array_slice($childNames, $key + 1); + $parentPath = $parent->getPath(); + $result = $this->checkChildren($childNames, $parentPath, $ignoreRole, $class); + if ($result) { + return $result; } - } while (!$anchor || $anchor !== $node->getPath()); + } return null; } + /** + * Search for a following document + * + * @param string|object $path document instance or path from which to search + * @param Boolean $reverse if to traverse back + * @param Boolean $ignoreRole if to ignore the role + * @param null|string $class the class to filter by + * + * @return null|object + */ + private function search($path, $reverse = false, $ignoreRole = false, $class = null) + { + if (is_object($path)) { + $path = $this->dm->getUnitOfWork()->getDocumentId($path); + } + + if (null === $path || '/' === $path) { + return null; + } + + $node = $this->dm->getPhpcrSession()->getNode($path); + $parentNode = $node->getParent(); + $childNames = $parentNode->getNodeNames()->getArrayCopy(); + if ($reverse) { + $childNames = array_reverse($childNames); + } + + $key = array_search($node->getName(), $childNames); + $childNames = array_slice($childNames, $key + 1); + return $this->checkChildren($childNames, $parentNode->getPath(), $ignoreRole, $class); + } + /** * Gets the previous document. * - * @param string|object $current document instance or path - * @param string|object $parent document instance or path - * @param null|integer $depth - * @param Boolean $ignoreRole - * @param null|string $class + * @param string|object $path document instance or path from which to search + * @param null|string|object $anchor document instance or path which serves as an anchor from which to flatten the hierarchy + * @param null|integer $depth depth up to which to traverse down the tree when an anchor is provided + * @param Boolean $ignoreRole if to ignore the role + * @param null|string $class the class to filter by * * @return null|object */ - public function getPrev($current, $parent = null, $depth = null, $ignoreRole = false, $class = null) + public function getPrev($current, $anchor = null, $depth = null, $ignoreRole = false, $class = null) { - return $this->search($current, $parent, $depth, true, $ignoreRole, $class); + if ($anchor) { + return $this->searchDepthPrev($current, $anchor, $depth, true, $ignoreRole, $class); + } + + return $this->search($current, true, $ignoreRole, $class); } /** * Gets the next document. * - * @param string|object $current document instance or path - * @param string|object $parent document instance or path - * @param null|integer $depth - * @param Boolean $ignoreRole - * @param null|string $class + * @param string|object $path document instance or path from which to search + * @param null|string|object $anchor document instance or path which serves as an anchor from which to flatten the hierarchy + * @param null|integer $depth depth up to which to traverse down the tree when an anchor is provided + * @param Boolean $ignoreRole if to ignore the role + * @param null|string $class the class to filter by * * @return null|object */ - public function getNext($current, $parent = null, $depth = null, $ignoreRole = false, $class = null) + public function getNext($current, $anchor = null, $depth = null, $ignoreRole = false, $class = null) { - return $this->search($current, $parent, $depth, false, $ignoreRole, $class); + if ($anchor) { + return $this->searchDepthNext($current, $anchor, $depth, $ignoreRole, $class); + } + + return $this->search($current, false, $ignoreRole, $class); } /** * Gets the previous linkable document. * - * @param string|object $current document instance or path - * @param string|object $parent document instance or path - * @param null|integer $depth - * @param Boolean $ignoreRole + * @param string|object $path document instance or path from which to search + * @param null|string|object $anchor document instance or path which serves as an anchor from which to flatten the hierarchy + * @param null|integer $depth depth up to which to traverse down the tree when an anchor is provided + * @param Boolean $ignoreRole if to ignore the role * * @return null|object */ - public function getPrevLinkable($current, $parent = null, $depth = null, $ignoreRole = false) + public function getPrevLinkable($current, $anchor = null, $depth = null, $ignoreRole = false) { - return $this->search($current, $parent, $depth, true, $ignoreRole, 'Symfony\Cmf\Component\Routing\RouteAwareInterface'); + return $this->getPrev($current, $anchor, $depth, $ignoreRole, 'Symfony\Cmf\Component\Routing\RouteAwareInterface'); } /** * Gets the next linkable document. * - * @param string|object $current document instance or path - * @param string|object $parent document instance or path - * @param null|integer $depth - * @param Boolean $ignoreRole + * @param string|object $path document instance or path from which to search + * @param null|string|object $anchor document instance or path which serves as an anchor from which to flatten the hierarchy + * @param null|integer $depth depth up to which to traverse down the tree when an anchor is provided + * @param Boolean $ignoreRole if to ignore the role * * @return null|object */ - public function getNextLinkable($current, $parent = null, $depth = null, $ignoreRole = false) + public function getNextLinkable($current, $anchor = null, $depth = null, $ignoreRole = false) { - return $this->search($current, $parent, $depth, false, $ignoreRole, 'Symfony\Cmf\Component\Routing\RouteAwareInterface'); + return $this->getNext($current, $anchor, $depth, $ignoreRole, 'Symfony\Cmf\Component\Routing\RouteAwareInterface'); } } diff --git a/Tests/Functional/Templating/Helper/CmfHelperHierarchyTest.php b/Tests/Functional/Templating/Helper/CmfHelperHierarchyTest.php new file mode 100644 index 00000000..238e459f --- /dev/null +++ b/Tests/Functional/Templating/Helper/CmfHelperHierarchyTest.php @@ -0,0 +1,212 @@ +getContainer(); + $managerRegistry = $container->get('doctrine_phpcr'); + /** @var $session SessionInterface */ + $session = $managerRegistry->getConnection(); + $root = $session->getRootNode(); + if ($root->hasNode('a')) { + $session->removeItem('/a'); + } + + /* + * /a + * /a/b + * /a/b/c + * /a/b/d + * /a/b/e + * /a/f + * /a/f/g + * /a/f/g/h + * /a/i + */ + $a = $root->addNode('a'); + $b = $a->addNode('b'); + $c = $b->addNode('c'); + $c->addMixin('phpcr:managed'); + $c->setProperty('phpcr:class', 'Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware'); + $b->addNode('d'); + $e = $b->addNode('e'); + $e->addMixin('phpcr:managed'); + $e->setProperty('phpcr:class', 'Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware'); + $f = $a->addNode('f'); + $g = $f->addNode('g'); + $g->addNode('h'); + $a->addNode('i'); + + $session->save(); + + $this->pwc = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface'); + $this->pwc->expects($this->any()) + ->method('isGranted') + ->will($this->returnValue(true)) + ; + + $this->extension = new CmfHelper($this->pwc, $managerRegistry, 'default'); + } + + public function testGetDescendants() + { + $this->assertEquals(array(), $this->extension->getDescendants(null)); + + $expected = array('/a/b', '/a/b/c', '/a/b/d', '/a/b/e', '/a/f', '/a/f/g', '/a/f/g/h', '/a/i'); + $this->assertEquals($expected, $this->extension->getDescendants('/a')); + + $expected = array('/a/b', '/a/f', '/a/i'); + $this->assertEquals($expected, $this->extension->getDescendants('/a', 1)); + } + + /** + * @dataProvider getPrevData + */ + public function testGetPrev($expected, $path, $anchor = null, $depth = null, $class = 'Doctrine\ODM\PHPCR\Document\Generic') + { + $prev = $this->extension->getPrev($path, $anchor, $depth); + if (null === $expected) { + $this->assertNull($prev); + } else { + $this->assertInstanceOf($class, $prev); + $this->assertEquals($expected, $prev->getId()); + } + } + + public static function getPrevData() + { + return array( + array(null, null), + array(null, '/a'), + array(null, '/a/b'), + array(null, '/a/b/c'), + array('/a/b/c', '/a/b/d', null, null, 'Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware'), + array('/a/b/d', '/a/b/e'), + array('/a/b', '/a/f'), + array(null, '/a/f/g'), + array(null, '/a/f/g/h'), + array(null, '/a', '/a'), + array('/a', '/a/b', '/a'), + array('/a/b', '/a/b/c', '/a'), + array('/a/b/c', '/a/b/d', '/a', null, 'Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware'), + array('/a/b/d', '/a/b/e', '/a'), + array('/a/b/e', '/a/f', '/a', null, 'Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware'), + array('/a/f', '/a/f/g', '/a'), + array('/a/f/g', '/a/f/g/h', '/a'), + array('/a/f/g/h', '/a/i', '/a'), + array('/a/f/g', '/a/i', '/a', 2), + ); + } + + /** + * @dataProvider getNextData + */ + public function testGetNext($expected, $path, $anchor = null, $depth = null, $class = 'Doctrine\ODM\PHPCR\Document\Generic') + { + $next = $this->extension->getNext($path, $anchor, $depth); + if (null === $expected) { + $this->assertNull($next); + } else { + $this->assertInstanceOf($class, $next); + $this->assertEquals($expected, $next->getId()); + } + } + + public static function getNextData() + { + return array( + array(null, null), + array(null, '/a'), + array('/a/f', '/a/b'), + array('/a/b/d', '/a/b/c'), + array('/a/b/e', '/a/b/d', null, null, 'Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware'), + array(null, '/a/b/e'), + array('/a/i', '/a/f'), + array(null, '/a/f/g'), + array(null, '/a/f/g/h'), + array('/a/b', '/a', '/a'), + array('/a/b/c', '/a/b', '/a', null, 'Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware'), + array('/a/b/d', '/a/b/c', '/a'), + array('/a/b/e', '/a/b/d', '/a', null, 'Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware'), + array('/a/f', '/a/b/e', '/a'), + array('/a/f/g', '/a/f', '/a'), + array('/a/f/g/h', '/a/f/g', '/a'), + array('/a/i', '/a/f/g/h', '/a'), + array(null, '/a/i', '/a'), + array(null, '/a/b/e', '/a/b'), + array('/a/i', '/a/f/g', '/a', 2), + ); + } + + /** + * @dataProvider getPrevLinkableData + */ + public function testGetPrevLinkable($expected, $path, $anchor = null, $depth = null) + { + $prev = $this->extension->getPrevLinkable($path, $anchor, $depth); + if (null === $expected) { + $this->assertNull($prev); + } else { + $this->assertInstanceOf('Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware', $prev); + $this->assertEquals($expected, $prev->getId()); + } + } + + public static function getPrevLinkableData() + { + // TODO: expand test case + return array( + array(null, null), + array(null, '/a/b/c'), + array('/a/b/c', '/a/b/d'), + array('/a/b/c', '/a/b/e'), + ); + } + + /** + * @dataProvider getNextLinkableData + */ + public function testGetNextLinkable($expected, $path, $anchor = null, $depth = null) + { + $next = $this->extension->getNextLinkable($path, $anchor, $depth); + if (null === $expected) { + $this->assertNull($next); + } else { + $this->assertInstanceOf('Symfony\Cmf\Bundle\CoreBundle\Tests\Resources\Document\RouteAware', $next); + $this->assertEquals($expected, $next->getId()); + } + } + + public static function getNextLinkableData() + { + // TODO: expand test case + return array( + array(null, null), + array('/a/b/e', '/a/b/c'), + array('/a/b/e', '/a/b/d'), + array(null, '/a/b/e'), + ); + } +} \ No newline at end of file diff --git a/Tests/Functional/Twig/Extension/CmfExtensionHierarchyTest.php b/Tests/Functional/Twig/Extension/CmfExtensionHierarchyTest.php deleted file mode 100644 index 65bc7fc9..00000000 --- a/Tests/Functional/Twig/Extension/CmfExtensionHierarchyTest.php +++ /dev/null @@ -1,134 +0,0 @@ -getContainer(); - $managerRegistry = $container->get('doctrine_phpcr'); - /** @var $session SessionInterface */ - $session = $managerRegistry->getConnection(); - $root = $session->getRootNode(); - if ($root->hasNode('a')) { - $session->removeItem('/a'); - } - - $a = $root->addNode('a'); - $b = $a->addNode('b'); - $c = $b->addNode('c'); - $d = $b->addNode('d'); - $e = $b->addNode('e'); - $f = $a->addNode('f'); - $g = $f->addNode('g'); - $h = $g->addNode('h'); - - $session->save(); - - $this->pwc = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface'); - $this->pwc->expects($this->any()) - ->method('isGranted') - ->will($this->returnValue(true)) - ; - - $this->extension = new CmfExtension($this->pwc, $managerRegistry, 'default'); - } - - public function testGetDescendants() - { - $this->assertEquals(array(), $this->extension->getDescendants(null)); - - $this->assertEquals(array('/a/b', '/a/b/c', '/a/b/d', '/a/b/e', '/a/f', '/a/f/g', '/a/f/g/h'), $this->extension->getDescendants('/a')); - - $this->assertEquals(array('/a/b', '/a/f'), $this->extension->getDescendants('/a', 1)); - } - - /** - * @dataProvider getPrevData - */ - public function testGetPrev($expected, $path) - { - $prev = $this->extension->getPrev($path); - if (null === $expected) { - $this->assertNull($prev); - } else { - $this->assertInstanceOf('Doctrine\ODM\PHPCR\Document\Generic', $prev); - $this->assertEquals($expected, $prev->getId()); - } - } - - public static function getPrevData() - { - return array( - array(null, null), - array(null, '/a'), - array('/a', '/a/b'), - array('/a/b', '/a/b/c'), - array('/a/b/c', '/a/b/d'), - array('/a/b/d', '/a/b/e'), - array('/a/b/e', '/a/f'), - array('/a/f', '/a/f/g'), - array('/a/f/g', '/a/f/g/h'), - ); - } - - /** - * @dataProvider getNextData - */ - public function testGetNext($expected, $path) - { - $next = $this->extension->getNext($path); - if (null === $expected) { - $this->assertNull($next); - } else { - $this->assertInstanceOf('Doctrine\ODM\PHPCR\Document\Generic', $next); - $this->assertEquals($expected, $next->getId()); - } - } - - public static function getNextData() - { - return array( - array(null, null), - array('/a/b', '/a'), - array('/a/b/c', '/a/b'), - array('/a/b/d', '/a/b/c'), - array('/a/b/e', '/a/b/d'), - array('/a/f', '/a/b/e'), - array('/a/f/g', '/a/f'), - array('/a/f/g/h', '/a/f/g'), - array(null, '/a/f/g/h'), - ); - } - - public function testGetPrevLinkable() - { - $this->assertNull($this->extension->getPrevLinkable(null)); - - $this->markTestIncomplete('TODO: write test'); - } - - public function testGetNextLinkable() - { - $this->assertNull($this->extension->getNextLinkable(null)); - - $this->markTestIncomplete('TODO: write test'); - } -} diff --git a/Tests/Resources/Document/RouteAware.php b/Tests/Resources/Document/RouteAware.php new file mode 100644 index 00000000..08996528 --- /dev/null +++ b/Tests/Resources/Document/RouteAware.php @@ -0,0 +1,24 @@ +id; + } + + public function getRoutes() + { + } +} \ No newline at end of file From 87fd7f32806b5171d1053b7ba9af496a6d1549aa Mon Sep 17 00:00:00 2001 From: dantleech Date: Wed, 10 Jul 2013 18:41:36 +0100 Subject: [PATCH 07/13] Refactored to inject helper rather than extending it. --- Templating/Helper/CmfHelper.php | 51 ++++++++++------- .../Unit/Twig/Extension/CmfExtensionTest.php | 26 +++++++++ Twig/Extension/CmfExtension.php | 55 +++++++++++-------- 3 files changed, 89 insertions(+), 43 deletions(-) create mode 100644 Tests/Unit/Twig/Extension/CmfExtensionTest.php diff --git a/Templating/Helper/CmfHelper.php b/Templating/Helper/CmfHelper.php index b50b0d7a..9c6c35ab 100644 --- a/Templating/Helper/CmfHelper.php +++ b/Templating/Helper/CmfHelper.php @@ -46,6 +46,15 @@ public function __construct(SecurityContextInterface $publishWorkflowChecker = n } } + protected function getDm() + { + if (!$this->dm) { + throw new \RuntimeException('Document Manager has not been initialized yet.'); + } + + return $this->dm; + } + /** * Gets the helper name. * @@ -81,7 +90,7 @@ public function getParentPath($document) public function getPath($document) { try { - return $this->dm->getUnitOfWork()->getDocumentId($document); + return $this->getDm()->getUnitOfWork()->getDocumentId($document); } catch (\Exception $e) { return false; } @@ -95,7 +104,7 @@ public function getPath($document) */ public function find($path) { - return $this->dm->find(null, $path); + return $this->getDm()->find(null, $path); } /** @@ -113,7 +122,7 @@ public function find($path) private function getDocument($document, $ignoreRole = false, $class = null) { if (is_string($document)) { - $document = $this->dm->find(null, $document); + $document = $this->getDm()->find(null, $document); } if (null !== $ignoreRole && null === $this->publishWorkflowChecker) { throw new InvalidConfigurationException('You can not fetch only published documents when the publishWorkflowChecker is not set. Either enable the publish workflow or pass "ignoreRole = null" to skip publication checks.'); @@ -194,7 +203,7 @@ public function isPublished($document) public function getLocalesFor($document, $includeFallbacks = false) { if (is_string($document)) { - $document = $this->dm->find(null, $document); + $document = $this->getDm()->find(null, $document); } if (empty($document)) { @@ -202,7 +211,7 @@ public function getLocalesFor($document, $includeFallbacks = false) } try { - $locales = $this->dm->getLocalesFor($document, $includeFallbacks); + $locales = $this->getDm()->getLocalesFor($document, $includeFallbacks); } catch (MissingTranslationException $e) { $locales = array(); } @@ -220,13 +229,13 @@ public function getChild($parent, $name) { if (is_object($parent)) { try { - $parent = $this->dm->getUnitOfWork()->getDocumentId($parent); + $parent = $this->getDm()->getUnitOfWork()->getDocumentId($parent); } catch (\Exception $e) { return false; } } - return $this->dm->find(null, "$parent/$name"); + return $this->getDm()->find(null, "$parent/$name"); } /** @@ -249,9 +258,9 @@ public function getChildren($parent, $limit = false, $offset = false, $filter = if ($limit || $offset) { if (is_object($parent)) { - $parent = $this->dm->getUnitOfWork()->getDocumentId($parent); + $parent = $this->getDm()->getUnitOfWork()->getDocumentId($parent); } - $node = $this->dm->getPhpcrSession()->getNode($parent); + $node = $this->getDm()->getPhpcrSession()->getNode($parent); $children = (array) $node->getNodeNames(); foreach ($children as $key => $child) { // filter before fetching data already to save some traffic @@ -268,7 +277,7 @@ public function getChildren($parent, $limit = false, $offset = false, $filter = $children = array_slice($children, $key); } } else { - $children = $this->dm->getChildren($parent, $filter); + $children = $this->getDm()->getChildren($parent, $filter); } $result = array(); @@ -327,7 +336,7 @@ private function getChildrenPaths($path, array &$children, $depth) --$depth; - $node = $this->dm->getPhpcrSession()->getNode($path); + $node = $this->getDm()->getPhpcrSession()->getNode($path); $names = (array) $node->getNodeNames(); foreach ($names as $name) { if (strpos($name, 'phpcr_locale:') === 0) { @@ -353,7 +362,7 @@ public function getDescendants($parent, $depth = null) $children = array(); if (is_object($parent)) { - $parent = $this->dm->getUnitOfWork()->getDocumentId($parent); + $parent = $this->getDm()->getUnitOfWork()->getDocumentId($parent); } $this->getChildrenPaths($parent, $children, $depth); @@ -407,7 +416,7 @@ private function traversePrevDepth($depth, $anchorDepth, array $childNames, $pat { foreach ($childNames as $childName) { $childPath = "$path/$childName"; - $node = $this->dm->getPhpcrSession()->getNode($childPath); + $node = $this->getDm()->getPhpcrSession()->getNode($childPath); if (null === $depth || PathHelper::getPathDepth($childPath) - $anchorDepth < $depth) { $childNames = $node->getNodeNames()->getArrayCopy(); if (!empty($childNames)) { @@ -440,17 +449,17 @@ private function traversePrevDepth($depth, $anchorDepth, array $childNames, $pat private function searchDepthPrev($path, $anchor, $depth = null, $ignoreRole = false, $class = null) { if (is_object($path)) { - $path = $this->dm->getUnitOfWork()->getDocumentId($path); + $path = $this->getDm()->getUnitOfWork()->getDocumentId($path); } if (null === $path || '/' === $path) { return null; } - $node = $this->dm->getPhpcrSession()->getNode($path); + $node = $this->getDm()->getPhpcrSession()->getNode($path); if (is_object($anchor)) { - $anchor = $this->dm->getUnitOfWork()->getDocumentId($anchor); + $anchor = $this->getDm()->getUnitOfWork()->getDocumentId($anchor); } if (0 !== strpos($path, $anchor)) { @@ -514,17 +523,17 @@ private function searchDepthPrev($path, $anchor, $depth = null, $ignoreRole = fa private function searchDepthNext($path, $anchor, $depth = null, $ignoreRole = false, $class = null) { if (is_object($path)) { - $path = $this->dm->getUnitOfWork()->getDocumentId($path); + $path = $this->getDm()->getUnitOfWork()->getDocumentId($path); } if (null === $path || '/' === $path) { return null; } - $node = $this->dm->getPhpcrSession()->getNode($path); + $node = $this->getDm()->getPhpcrSession()->getNode($path); if (is_object($anchor)) { - $anchor = $this->dm->getUnitOfWork()->getDocumentId($anchor); + $anchor = $this->getDm()->getUnitOfWork()->getDocumentId($anchor); } if (0 !== strpos($path, $anchor)) { @@ -588,14 +597,14 @@ private function searchDepthNext($path, $anchor, $depth = null, $ignoreRole = fa private function search($path, $reverse = false, $ignoreRole = false, $class = null) { if (is_object($path)) { - $path = $this->dm->getUnitOfWork()->getDocumentId($path); + $path = $this->getDm()->getUnitOfWork()->getDocumentId($path); } if (null === $path || '/' === $path) { return null; } - $node = $this->dm->getPhpcrSession()->getNode($path); + $node = $this->getDm()->getPhpcrSession()->getNode($path); $parentNode = $node->getParent(); $childNames = $parentNode->getNodeNames()->getArrayCopy(); if ($reverse) { diff --git a/Tests/Unit/Twig/Extension/CmfExtensionTest.php b/Tests/Unit/Twig/Extension/CmfExtensionTest.php new file mode 100644 index 00000000..4876004c --- /dev/null +++ b/Tests/Unit/Twig/Extension/CmfExtensionTest.php @@ -0,0 +1,26 @@ +cmfHelper = $this->getMockBuilder( + 'Symfony\Cmf\Bundle\CoreBundle\Templating\Helper\CmfHelper' + )->disableOriginalConstructor()->getMock(); + + $this->cmfExtension = new CmfExtension($this->cmfHelper); + $this->env = new \Twig_Environment(); + $this->env->addExtension($this->cmfExtension); + } + + + public function testFunctions() + { + $functions = $this->cmfExtension->getFunctions(); + $this->assertCount(15, $functions); + } +} diff --git a/Twig/Extension/CmfExtension.php b/Twig/Extension/CmfExtension.php index 5a71a902..1b4b93e4 100644 --- a/Twig/Extension/CmfExtension.php +++ b/Twig/Extension/CmfExtension.php @@ -4,8 +4,15 @@ use Symfony\Cmf\Bundle\CoreBundle\Templating\Helper\CmfHelper; -class CmfExtension extends CmfHelper implements \Twig_ExtensionInterface +class CmfExtension implements \Twig_ExtensionInterface { + protected $cmfHelper; + + public function __construct(CmfHelper $cmfHelper) + { + $this->cmfHelper = $cmfHelper; + } + /** * Get list of available functions * @@ -13,33 +20,32 @@ class CmfExtension extends CmfHelper implements \Twig_ExtensionInterface */ public function getFunctions() { - $functions = array('cmf_is_published' => new \Twig_Function_Method($this, 'isPublished')); - - if ($this->dm) { - $functions['cmf_child'] = new \Twig_Function_Method($this, 'getChild'); - $functions['cmf_children'] = new \Twig_Function_Method($this, 'getChildren'); - $functions['cmf_prev'] = new \Twig_Function_Method($this, 'getPrev'); - $functions['cmf_next'] = new \Twig_Function_Method($this, 'getNext'); - $functions['cmf_find'] = new \Twig_Function_Method($this, 'find'); - $functions['cmf_find_many'] = new \Twig_Function_Method($this, 'findMany'); - $functions['cmf_descendants'] = new \Twig_Function_Method($this, 'getDescendants'); - $functions['cmf_nodename'] = new \Twig_Function_Method($this, 'getNodeName'); - $functions['cmf_parent_path'] = new \Twig_Function_Method($this, 'getParentPath'); - $functions['cmf_path'] = new \Twig_Function_Method($this, 'getPath'); - $functions['cmf_document_locales'] = new \Twig_Function_Method($this, 'getLocalesFor'); + $functions = array( + new \Twig_SimpleFunction('cmf_is_published', array($this->cmfHelper, 'isPublished')), + new \Twig_SimpleFunction('cmf_child', array($this->cmfHelper, 'getChild')), + new \Twig_SimpleFunction('cmf_children', array($this->cmfHelper, 'getChildren')), + new \Twig_SimpleFunction('cmf_prev', array($this->cmfHelper, 'getPrev')), + new \Twig_SimpleFunction('cmf_next', array($this->cmfHelper, 'getNext')), + new \Twig_SimpleFunction('cmf_find', array($this->cmfHelper, 'find')), + new \Twig_SimpleFunction('cmf_find_many', array($this->cmfHelper, 'findMany')), + new \Twig_SimpleFunction('cmf_descendants', array($this->cmfHelper, 'getDescendants')), + new \Twig_SimpleFunction('cmf_nodename', array($this->cmfHelper, 'getNodeName')), + new \Twig_SimpleFunction('cmf_parent_path', array($this->cmfHelper, 'getParentPath')), + new \Twig_SimpleFunction('cmf_path', array($this->cmfHelper, 'getPath')), + new \Twig_SimpleFunction('cmf_document_locales', array($this->cmfHelper, 'getLocalesFor')), + ); - if (interface_exists('Symfony\Cmf\Component\Routing\RouteAwareInterface')) { - $functions['cmf_prev_linkable'] = new \Twig_Function_Method($this, 'getPrevLinkable'); - $functions['cmf_next_linkable'] = new \Twig_Function_Method($this, 'getNextLinkable'); - $functions['cmf_linkable_children'] = new \Twig_Function_Method($this, 'getLinkableChildren'); - } + if (interface_exists('Symfony\Cmf\Component\Routing\RouteAwareInterface')) { + $functions = array_merge($functions, array( + new \Twig_SimpleFunction('cmf_prev_linkable', array($this->cmfHelper, 'getPrevLinkable')), + new \Twig_SimpleFunction('cmf_next_linkable', array($this->cmfHelper, 'getNextLinkable')), + new \Twig_SimpleFunction('cmf_linkable_children', array($this->cmfHelper, 'getLinkableChildren')), + )); } return $functions; } - // from \Twig_Extension - /** * Initializes the runtime environment. * @@ -110,4 +116,9 @@ public function getGlobals() { return array(); } + + public function getName() + { + return 'cmf'; + } } From 45f1fba393ef4dd16bd95fbf49214192c1f55bf9 Mon Sep 17 00:00:00 2001 From: dantleech Date: Wed, 10 Jul 2013 19:04:30 +0100 Subject: [PATCH 08/13] Fixed service.xml and added functional test --- Resources/config/services.xml | 9 +++++++-- Tests/Functional/Twig/ServiceTest.php | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 Tests/Functional/Twig/ServiceTest.php diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 6fd90fcc..78be50f3 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -5,17 +5,22 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - Symfony\Cmf\Bundle\CoreBundle\Twig\TwigExtension + Symfony\Cmf\Bundle\CoreBundle\Twig\Extension\CmfExtension + Symfony\Cmf\Bundle\CoreBundle\Templating\Helper\CmfHelper Symfony\Cmf\Bundle\CoreBundle\EventListener\RequestAwareListener + + + + + %cmf_core.document_manager_name% - diff --git a/Tests/Functional/Twig/ServiceTest.php b/Tests/Functional/Twig/ServiceTest.php new file mode 100644 index 00000000..09229eb3 --- /dev/null +++ b/Tests/Functional/Twig/ServiceTest.php @@ -0,0 +1,16 @@ +getContainer()->get('twig'); + $ext = $twig->getExtension('cmf'); + $this->assertNotEmpty($ext); + } +} + From 4587c0cb55e3530b7514d5a061211f00dbd87f60 Mon Sep 17 00:00:00 2001 From: dantleech Date: Thu, 11 Jul 2013 18:45:56 +0100 Subject: [PATCH 09/13] Removed extra methods by extending Twig_Extension --- Twig/Extension/CmfExtension.php | 73 +-------------------------------- 1 file changed, 1 insertion(+), 72 deletions(-) diff --git a/Twig/Extension/CmfExtension.php b/Twig/Extension/CmfExtension.php index 1b4b93e4..0c11623b 100644 --- a/Twig/Extension/CmfExtension.php +++ b/Twig/Extension/CmfExtension.php @@ -4,7 +4,7 @@ use Symfony\Cmf\Bundle\CoreBundle\Templating\Helper\CmfHelper; -class CmfExtension implements \Twig_ExtensionInterface +class CmfExtension extends \Twig_Extension { protected $cmfHelper; @@ -46,77 +46,6 @@ public function getFunctions() return $functions; } - /** - * Initializes the runtime environment. - * - * This is where you can load some file that contains filter functions for instance. - * - * @param Twig_Environment $environment The current Twig_Environment instance - */ - public function initRuntime(\Twig_Environment $environment) - { - } - - /** - * Returns the token parser instances to add to the existing list. - * - * @return array An array of Twig_TokenParserInterface or Twig_TokenParserBrokerInterface instances - */ - public function getTokenParsers() - { - return array(); - } - - /** - * Returns the node visitor instances to add to the existing list. - * - * @return array An array of Twig_NodeVisitorInterface instances - */ - public function getNodeVisitors() - { - return array(); - } - - /** - * Returns a list of filters to add to the existing list. - * - * @return array An array of filters - */ - public function getFilters() - { - return array(); - } - - /** - * Returns a list of tests to add to the existing list. - * - * @return array An array of tests - */ - public function getTests() - { - return array(); - } - - /** - * Returns a list of operators to add to the existing list. - * - * @return array An array of operators - */ - public function getOperators() - { - return array(); - } - - /** - * Returns a list of global variables to add to the existing list. - * - * @return array An array of global variables - */ - public function getGlobals() - { - return array(); - } - public function getName() { return 'cmf'; From 003d8cdbb471e6cb1a59d6c865078a1d9df9f4c9 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Fri, 12 Jul 2013 13:31:54 +0200 Subject: [PATCH 10/13] split workflow editing interfaces --- ...ion.php => PublishTimePeriodExtension.php} | 8 ++---- Admin/Extension/PublishableExtension.php | 26 +++++++++++++++++++ CHANGELOG.md | 6 +++++ DependencyInjection/CmfCoreExtension.php | 3 ++- ...hp => PublishTimePeriodWriteInterface.php} | 11 ++------ PublishWorkflow/PublishableWriteInterface.php | 16 ++++++++++++ Resources/config/publish_workflow.xml | 9 +++++-- .../PublishWorkflow/PublishWorkflowTest.php | 12 ++++++--- .../PublishWorkflowCheckerTest.php | 6 ++--- .../Voter/PublishableVoterTest.php | 1 - 10 files changed, 72 insertions(+), 26 deletions(-) rename Admin/Extension/{PublishWorkflowExtension.php => PublishTimePeriodExtension.php} (78%) create mode 100644 Admin/Extension/PublishableExtension.php rename PublishWorkflow/{PublishWorkflowInterface.php => PublishTimePeriodWriteInterface.php} (67%) create mode 100644 PublishWorkflow/PublishableWriteInterface.php diff --git a/Admin/Extension/PublishWorkflowExtension.php b/Admin/Extension/PublishTimePeriodExtension.php similarity index 78% rename from Admin/Extension/PublishWorkflowExtension.php rename to Admin/Extension/PublishTimePeriodExtension.php index 54fe59ed..50e0020b 100644 --- a/Admin/Extension/PublishWorkflowExtension.php +++ b/Admin/Extension/PublishTimePeriodExtension.php @@ -6,11 +6,11 @@ use Sonata\AdminBundle\Form\FormMapper; /** - * Admin extension to add publish workflow fields. + * Admin extension to add publish workflow time period fields. * * @author Daniel Leech */ -class PublishWorkflowExtension extends AdminExtension +class PublishTimePeriodExtension extends AdminExtension { public function configureFormFields(FormMapper $formMapper) { @@ -21,10 +21,6 @@ public function configureFormFields(FormMapper $formMapper) $formMapper->with('form.group_publish_workflow', array( 'translation_domain' => 'CmfCoreBundle' - )) - ->add('publishable', 'checkbox', array( - 'required' => false, - ), array( )) ->add('publish_start_date', 'date', $dateOptions, array( 'help' => 'form.help_publish_start_date', diff --git a/Admin/Extension/PublishableExtension.php b/Admin/Extension/PublishableExtension.php new file mode 100644 index 00000000..744ffd07 --- /dev/null +++ b/Admin/Extension/PublishableExtension.php @@ -0,0 +1,26 @@ + + */ +class PublishableExtension extends AdminExtension +{ + public function configureFormFields(FormMapper $formMapper) + { + $formMapper->with('form.group_publish_workflow', array( + 'translation_domain' => 'CmfCoreBundle' + )) + ->add('publishable', 'checkbox', array( + 'required' => false, + ), array( + )) + ->end(); + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a48ed5e..c97559be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ Changelog The security context is also triggered by a core security voter, so that using the isGranted method of the standard security will check for publication. + The PublishWorkflowInterface is split into the reading interfaces + PublishableInterface and PublishTimePeriodInterface as well as + PublishableWriteInterface and PublishableTimePeriodWriteInterface. The sonata + admin extension has been split accordingly and there are now + cmf_core.admin_extension.publish_workflow.time_period and + cmf_core.admin_extension.publish_workflow.publishable. * **2013-05-16**: [PublishWorkFlowChecker] Removed Request argument from check method. Class now accepts a DateTime object to diff --git a/DependencyInjection/CmfCoreExtension.php b/DependencyInjection/CmfCoreExtension.php index 94073a1a..29a52a6c 100644 --- a/DependencyInjection/CmfCoreExtension.php +++ b/DependencyInjection/CmfCoreExtension.php @@ -36,7 +36,8 @@ public function loadPublishWorkflow($config, XmlFileLoader $loader, ContainerBui } if (!class_exists('Sonata\AdminBundle\Admin\AdminExtension')) { - $container->removeDefinition($this->getAlias() . '.cmf_core.admin_extension.publish_workflow'); + $container->removeDefinition($this->getAlias() . '.cmf_core.admin_extension.publish_workflow.publishable'); + $container->removeDefinition($this->getAlias() . '.cmf_core.admin_extension.publish_workflow.time_period'); } } diff --git a/PublishWorkflow/PublishWorkflowInterface.php b/PublishWorkflow/PublishTimePeriodWriteInterface.php similarity index 67% rename from PublishWorkflow/PublishWorkflowInterface.php rename to PublishWorkflow/PublishTimePeriodWriteInterface.php index 77cd9812..c6f5716b 100644 --- a/PublishWorkflow/PublishWorkflowInterface.php +++ b/PublishWorkflow/PublishTimePeriodWriteInterface.php @@ -3,9 +3,9 @@ namespace Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow; /** - * Interface models can implement to expose editable publishing settings. + * Interface to expose editable time period publishing information. */ -interface PublishWorkflowInterface extends PublishableInterface, PublishTimePeriodInterface +interface PublishTimePeriodWriteInterface extends PublishTimePeriodInterface { /** * Set the date from which the content should @@ -28,11 +28,4 @@ public function setPublishStartDate(\DateTime $publishDate = null); * @param \DateTime|null $publishDate */ public function setPublishEndDate(\DateTime $publishDate = null); - - /** - * Set the boolean flag if this content should be published or not. - * - * @return boolean - */ - public function setPublishable($publishable); } diff --git a/PublishWorkflow/PublishableWriteInterface.php b/PublishWorkflow/PublishableWriteInterface.php new file mode 100644 index 00000000..c4fd01e9 --- /dev/null +++ b/PublishWorkflow/PublishableWriteInterface.php @@ -0,0 +1,16 @@ +Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\Voter\PublishTimePeriodVoter Symfony\Cmf\Bundle\CoreBundle\EventListener\PublishWorkflowListener Symfony\Cmf\Bundle\CoreBundle\Security\Authorization\Voter\PublishedVoter - Symfony\Cmf\Bundle\CoreBundle\Admin\Extension\PublishWorkflowExtension + Symfony\Cmf\Bundle\CoreBundle\Admin\Extension\PublishableExtension + Symfony\Cmf\Bundle\CoreBundle\Admin\Extension\PublishTimePeriodExtension @@ -50,7 +51,11 @@ - + + + + + diff --git a/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php b/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php index bfa749ba..9b4a539b 100644 --- a/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php +++ b/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php @@ -4,6 +4,8 @@ use PHPCR\SessionInterface; use Doctrine\Bundle\PHPCRBundle\ManagerRegistry; +use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableInterface; +use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishTimePeriodInterface; use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker; use Symfony\Cmf\Bundle\CoreBundle\Twig\TwigExtension; use Symfony\Cmf\Component\Testing\Functional\BaseTestCase; @@ -38,7 +40,7 @@ public function setUp() public function testPublishable() { - $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableInterface'); $doc->expects($this->any()) ->method('isPublishable') ->will($this->returnValue(true)) @@ -50,7 +52,7 @@ public function testPublishable() public function testPublishPeriod() { - $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\Tests\Functional\PublishWorkflow\PublishModel'); $doc->expects($this->any()) ->method('isPublishable') ->will($this->returnValue(true)) @@ -66,7 +68,7 @@ public function testPublishPeriod() public function testIgnoreRoleHas() { - $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\Tests\Functional\PublishWorkflow\PublishModel'); $doc->expects($this->any()) ->method('isPublishable') ->will($this->returnValue(false)) @@ -84,7 +86,7 @@ public function testIgnoreRoleHas() public function testIgnoreRoleNotHas() { - $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\Tests\Functional\PublishWorkflow\PublishModel'); $doc->expects($this->any()) ->method('isPublishable') ->will($this->returnValue(false)) @@ -101,3 +103,5 @@ public function testIgnoreRoleNotHas() $this->assertFalse($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ANONYMOUS_ATTRIBUTE, $doc)); } } + +abstract class PublishModel implements PublishableInterface, PublishTimePeriodInterface {} \ No newline at end of file diff --git a/Tests/Unit/PublishWorkflow/PublishWorkflowCheckerTest.php b/Tests/Unit/PublishWorkflow/PublishWorkflowCheckerTest.php index b900dd25..281d0343 100644 --- a/Tests/Unit/PublishWorkflow/PublishWorkflowCheckerTest.php +++ b/Tests/Unit/PublishWorkflow/PublishWorkflowCheckerTest.php @@ -2,8 +2,8 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Tests\Unit\PublishWorkflow; +use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableWriteInterface; use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker; -use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; @@ -32,7 +32,7 @@ class PublishWorkflowCheckerTest extends \PHPUnit_Framework_Testcase private $sc; /** - * @var PublishWorkflowInterface|\PHPUnit_Framework_MockObject_MockObject + * @var PublishableWriteInterface|\PHPUnit_Framework_MockObject_MockObject */ private $doc; @@ -52,7 +52,7 @@ public function setUp() ->with('security.context') ->will($this->returnValue($this->sc)) ; - $this->doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface'); + $this->doc = $this->getMock('Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableWriteInterface'); $this->adm = $this->getMock('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface'); $this->stdClass = new \stdClass; diff --git a/Tests/Unit/PublishWorkflow/Voter/PublishableVoterTest.php b/Tests/Unit/PublishWorkflow/Voter/PublishableVoterTest.php index 03acea10..5cd367ad 100644 --- a/Tests/Unit/PublishWorkflow/Voter/PublishableVoterTest.php +++ b/Tests/Unit/PublishWorkflow/Voter/PublishableVoterTest.php @@ -3,7 +3,6 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Tests\Unit\PublishWorkflow\Voter; use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker; -use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowInterface; use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\Voter\PublishableVoter; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken; From e0d41d804f8f9e13ad676facc5c41872a676775d Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Sat, 13 Jul 2013 11:01:13 +0200 Subject: [PATCH 11/13] adjust the extension test --- .../PublishTimePeriodExtensionTest.php | 29 +++++++++++++++++++ ...nTest.php => PublishableExtensionTest.php} | 9 +++--- 2 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 Tests/Unit/Admin/Extensions/PublishTimePeriodExtensionTest.php rename Tests/Unit/Admin/Extensions/{PublishWorkflowExtensionTest.php => PublishableExtensionTest.php} (69%) diff --git a/Tests/Unit/Admin/Extensions/PublishTimePeriodExtensionTest.php b/Tests/Unit/Admin/Extensions/PublishTimePeriodExtensionTest.php new file mode 100644 index 00000000..528e6f89 --- /dev/null +++ b/Tests/Unit/Admin/Extensions/PublishTimePeriodExtensionTest.php @@ -0,0 +1,29 @@ +formMapper = $this->getMockBuilder( + 'Sonata\AdminBundle\Form\FormMapper' + )->disableOriginalConstructor()->getMock(); + + $this->extension = new PublishTimePeriodExtension(); + } + + public function testFormMapper() + { + $this->formMapper->expects($this->once()) + ->method('with') + ->will($this->returnSelf()); + $this->formMapper->expects($this->exactly(2)) + ->method('add') + ->will($this->returnSelf()); + + $this->extension->configureFormFields($this->formMapper); + } +} diff --git a/Tests/Unit/Admin/Extensions/PublishWorkflowExtensionTest.php b/Tests/Unit/Admin/Extensions/PublishableExtensionTest.php similarity index 69% rename from Tests/Unit/Admin/Extensions/PublishWorkflowExtensionTest.php rename to Tests/Unit/Admin/Extensions/PublishableExtensionTest.php index 361f6315..d15f0282 100644 --- a/Tests/Unit/Admin/Extensions/PublishWorkflowExtensionTest.php +++ b/Tests/Unit/Admin/Extensions/PublishableExtensionTest.php @@ -2,9 +2,10 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Tests\Unit\Admin\Extension; -use Symfony\Cmf\Bundle\CoreBundle\Admin\Extension\PublishWorkflowExtension; -class PublishWorkflowExtensionTest extends \PHPUnit_Framework_Testcase +use Symfony\Cmf\Bundle\CoreBundle\Admin\Extension\PublishableExtension; + +class PublishableExtensionTest extends \PHPUnit_Framework_Testcase { public function setUp() { @@ -12,7 +13,7 @@ public function setUp() 'Sonata\AdminBundle\Form\FormMapper' )->disableOriginalConstructor()->getMock(); - $this->extension = new PublishWorkflowExtension; + $this->extension = new PublishableExtension(); } public function testFormMapper() @@ -20,7 +21,7 @@ public function testFormMapper() $this->formMapper->expects($this->once()) ->method('with') ->will($this->returnSelf()); - $this->formMapper->expects($this->exactly(3)) + $this->formMapper->expects($this->exactly(1)) ->method('add') ->will($this->returnSelf()); From b516cd7e5bbeaca77a14f6d5780ffc86d4e20d10 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Sat, 13 Jul 2013 11:40:19 +0200 Subject: [PATCH 12/13] doc cleanups --- DependencyInjection/CmfCoreExtension.php | 4 +-- .../Compiler/AddPublishedVotersPass.php | 3 ++- .../PublishTimePeriodInterface.php | 6 +++-- PublishWorkflow/PublishWorkflowChecker.php | 25 +++++++++++++------ PublishWorkflow/PublishableInterface.php | 8 +++--- PublishWorkflow/PublishableWriteInterface.php | 2 +- Resources/config/publish_workflow.xml | 2 +- Templating/Helper/CmfHelper.php | 3 +++ 8 files changed, 34 insertions(+), 19 deletions(-) diff --git a/DependencyInjection/CmfCoreExtension.php b/DependencyInjection/CmfCoreExtension.php index 29a52a6c..2f84d544 100644 --- a/DependencyInjection/CmfCoreExtension.php +++ b/DependencyInjection/CmfCoreExtension.php @@ -36,8 +36,8 @@ public function loadPublishWorkflow($config, XmlFileLoader $loader, ContainerBui } if (!class_exists('Sonata\AdminBundle\Admin\AdminExtension')) { - $container->removeDefinition($this->getAlias() . '.cmf_core.admin_extension.publish_workflow.publishable'); - $container->removeDefinition($this->getAlias() . '.cmf_core.admin_extension.publish_workflow.time_period'); + $container->removeDefinition($this->getAlias() . '.admin_extension.publish_workflow.publishable'); + $container->removeDefinition($this->getAlias() . '.admin_extension.publish_workflow.time_period'); } } diff --git a/DependencyInjection/Compiler/AddPublishedVotersPass.php b/DependencyInjection/Compiler/AddPublishedVotersPass.php index 943507e4..2fb90be5 100644 --- a/DependencyInjection/Compiler/AddPublishedVotersPass.php +++ b/DependencyInjection/Compiler/AddPublishedVotersPass.php @@ -7,7 +7,8 @@ use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; /** - * Adds all configured publish workflow voters to the access decision manager. + * Adds all configured publish voters to the access decision manager used for + * the publish workflow checker. * * This is about the same as Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass * diff --git a/PublishWorkflow/PublishTimePeriodInterface.php b/PublishWorkflow/PublishTimePeriodInterface.php index 0ae42298..76610e0c 100644 --- a/PublishWorkflow/PublishTimePeriodInterface.php +++ b/PublishWorkflow/PublishTimePeriodInterface.php @@ -3,8 +3,10 @@ namespace Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow; /** - * Interface models can implement if they want to support time based publish - * checking. + * Interface for time period based publish checking. + * + * Both start and end date are optional, with null being interpreted as always + * started resp. never ending. */ interface PublishTimePeriodInterface { diff --git a/PublishWorkflow/PublishWorkflowChecker.php b/PublishWorkflow/PublishWorkflowChecker.php index 1770095d..d3426ecb 100644 --- a/PublishWorkflow/PublishWorkflowChecker.php +++ b/PublishWorkflow/PublishWorkflowChecker.php @@ -11,10 +11,18 @@ use Symfony\Component\HttpFoundation\Request; /** - * Implementation of a publish workflow checker as a security context. + * The publish workflow decides if a content is allowed to be shown. Contrary + * to the symfony core security context, this is even possible without a + * firewall configured for the current route. * - * It gives "admins" full access, - * while for other users it runs all cmf_published_voter + * The access decision manager is configured to be unanimous by default, and + * provided with all published voters tagged with cmf_published_voter. + * + * If the VIEW attribute is used and there is a firewall in place, there is a + * check if the current user is granted the bypassing role and if so, he can + * see even unpublished content. + * + * If VIEW_ANONYMOUS is used, the publication check is never bypassed. * * @author David Buchmann */ @@ -31,7 +39,7 @@ class PublishWorkflowChecker implements SecurityContextInterface * users. This can be used where the role based exception from the * publication check is not wanted. * - * The role exception is handled by the workflow checker, the individual + * The bypass role is handled by the workflow checker, the individual * voters should treat VIEW and VIEW_ANONYMOUS the same. */ const VIEW_ANONYMOUS_ATTRIBUTE = 'VIEW_ANONYMOUS'; @@ -42,8 +50,8 @@ class PublishWorkflowChecker implements SecurityContextInterface private $container; /** - * @var bool|string Role allowed to bypass security check or false to never - * bypass + * @var bool|string Role allowed to bypass the published check if the + * VIEW attribute is used, or false to never bypass */ private $bypassingRole; @@ -63,7 +71,7 @@ class PublishWorkflowChecker implements SecurityContextInterface * to a circular reference. * @param AccessDecisionManagerInterface $accessDecisionManager * @param string $bypassingRole A role that is allowed to bypass the - * publishable check. + * published check if we ask for the VIEW attribute . */ public function __construct(ContainerInterface $container, AccessDecisionManagerInterface $accessDecisionManager, $bypassingRole = false) { @@ -74,6 +82,9 @@ public function __construct(ContainerInterface $container, AccessDecisionManager /** * {@inheritDoc} + * + * Defaults to the token from the default security context, but can be + * overwritten locally. */ public function getToken() { diff --git a/PublishWorkflow/PublishableInterface.php b/PublishWorkflow/PublishableInterface.php index 81331f7f..642e141a 100644 --- a/PublishWorkflow/PublishableInterface.php +++ b/PublishWorkflow/PublishableInterface.php @@ -3,12 +3,10 @@ namespace Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow; /** - * Interface models can implement if they want to support publish checking with - * a binary flag. + * Interface for a binary publishable flag. * - * Several publish interfaces can be combined. Publish voters will return DENY - * if the condition is not met and ABSTAIN if it is met, to allow other voters - * to influence the decision as well. + * If the flag is false, the content is not published, if it is true it is + * published if no other voter objects. */ interface PublishableInterface { diff --git a/PublishWorkflow/PublishableWriteInterface.php b/PublishWorkflow/PublishableWriteInterface.php index c4fd01e9..d6bcacdc 100644 --- a/PublishWorkflow/PublishableWriteInterface.php +++ b/PublishWorkflow/PublishableWriteInterface.php @@ -8,7 +8,7 @@ interface PublishableWriteInterface extends PublishableInterface { /** - * Set the boolean flag if this content should be published or not. + * Set the boolean flag whether this content is publishable or not. * * @return boolean */ diff --git a/Resources/config/publish_workflow.xml b/Resources/config/publish_workflow.xml index 25a68b27..66d945ff 100644 --- a/Resources/config/publish_workflow.xml +++ b/Resources/config/publish_workflow.xml @@ -49,7 +49,7 @@ - + diff --git a/Templating/Helper/CmfHelper.php b/Templating/Helper/CmfHelper.php index 9c6c35ab..d6ab599c 100644 --- a/Templating/Helper/CmfHelper.php +++ b/Templating/Helper/CmfHelper.php @@ -176,6 +176,9 @@ public function findMany($paths = array(), $limit = false, $offset = false, $ign /** * Check if a document is published, regardless of the current users role. * + * If you need the bypass role, you will have a firewall configured and can + * simply use {{ is_granted('VIEW', document) }} + * * @param object $document * * @return boolean From fc927c99579a66a40d201b4a42184eea53342dab Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Sun, 14 Jul 2013 16:24:02 +0200 Subject: [PATCH 13/13] adjust test to fixed testing component --- .../PublishWorkflow/PublishWorkflowTest.php | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php b/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php index 9b4a539b..e3801267 100644 --- a/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php +++ b/Tests/Functional/PublishWorkflow/PublishWorkflowTest.php @@ -3,18 +3,21 @@ namespace Symfony\Cmf\Bundle\CoreBundle\Tests\Functional\PublishWorkflow; use PHPCR\SessionInterface; + use Doctrine\Bundle\PHPCRBundle\ManagerRegistry; + +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishableInterface; use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishTimePeriodInterface; use Symfony\Cmf\Bundle\CoreBundle\PublishWorkflow\PublishWorkflowChecker; -use Symfony\Cmf\Bundle\CoreBundle\Twig\TwigExtension; -use Symfony\Cmf\Component\Testing\Functional\BaseTestCase; -use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Cmf\Bundle\CoreBundle\Twig\Extension\CmfExtension; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Role\Role; use Symfony\Component\Security\Core\SecurityContext; use Symfony\Component\Security\Core\SecurityContextInterface; +use Symfony\Cmf\Component\Testing\Functional\BaseTestCase; + class PublishWorkflowTest extends BaseTestCase { /** @@ -23,19 +26,13 @@ class PublishWorkflowTest extends BaseTestCase private $pwc; /** - * @var ContainerInterface - */ - private $container; - - /** - * @var TwigExtension + * @var CmfExtension */ private $extension; public function setUp() { - $this->container = $this->getContainer(); - $this->pwc = $this->container->get('cmf_core.publish_workflow.checker'); + $this->pwc = $this->getContainer()->get('cmf_core.publish_workflow.checker'); } public function testPublishable() @@ -77,7 +74,7 @@ public function testIgnoreRoleHas() new Role('ROLE_CAN_VIEW_NON_PUBLISHED') ); $token = new UsernamePasswordToken('test', 'pass', 'testprovider', $roles); - $context = $this->container->get('security.context'); + $context = $this->getContainer()->get('security.context'); $context->setToken($token); $this->assertTrue($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $doc)); @@ -96,7 +93,7 @@ public function testIgnoreRoleNotHas() ); $token = new UsernamePasswordToken('test', 'pass', 'testprovider', $roles); /** @var $context SecurityContext */ - $context = $this->container->get('security.context'); + $context = $this->getContainer()->get('security.context'); $context->setToken($token); $this->assertFalse($this->pwc->isGranted(PublishWorkflowChecker::VIEW_ATTRIBUTE, $doc));