Acl assertions enhancement #5628

Merged
merged 6 commits into from Mar 4, 2014

Projects

None yet

5 participants

@gauthier

AssertionAggregate allows aggregating multiple assertion to be associated with one ACL rule. This has been done without modifying a single file of existing code.

AssertionManager is a simple Plugin Manager allowing to declare Acl asseertions as services. It can be used by AssertionAggregate to add assertions by passing their service name. It could also by used by the Acl component itself, and should be declared as a default service, so that it could rely on Config service to get intialized, but those features would need to alter existing code and I didn't want to do that at this time.

Maks3w and others added some commits Dec 16, 2013
@Maks3w Maks3w Merge pull request #5574 in master 9ff4573
@gauthier gauthier added AssertionAggregate and AssertionManager
AssertionAggregate allows aggregating multiple assertion to be associated with one ACL rule. This has been done without modifying a single file of existing code.

AssertionManager is a simple Plugin Manager allowing to declare Acl asseertions as services. It can be used by AssertionAggregate to add assertions by passing their service name. It could also by used by the Acl component itself, and should be declared as a default service, so that it could rely on Config service to get intialized, but those features would need to alter existing code and I didn't want to do that at this time.
36a1e9d
@samsonasik samsonasik commented on an outdated diff Dec 18, 2013
...Zend/Permissions/Acl/Assertion/AssertionAggregate.php
+
+ /**
+ *
+ * @var $manager AssertionManager
+ */
+ protected $assertionManager;
+
+ protected $mode = self::MODE_ALL;
+
+ /**
+ * Stacks an assertion in aggregate
+ *
+ * @param AssertionInterface|string $assertion
+ * if string, must match a AssertionManager declared service (checked later)
+ *
+ * @return \Zend\Permissions\Acl\Assertion\AssertionAggregate
@samsonasik samsonasik commented on an outdated diff Dec 18, 2013
...Zend/Permissions/Acl/Assertion/AssertionAggregate.php
+ return $this;
+ }
+
+ public function addAssertions(array $assertions)
+ {
+ foreach ($assertions as $assertion) {
+ $this->addAssertion($assertion);
+ }
+
+ return $this;
+ }
+
+ /**
+ * Empties assertions stack
+ *
+ * @return \Zend\Permissions\Acl\Assertion\AssertionAggregate
@samsonasik samsonasik commented on an outdated diff Dec 18, 2013
...Zend/Permissions/Acl/Assertion/AssertionAggregate.php
+ * Empties assertions stack
+ *
+ * @return \Zend\Permissions\Acl\Assertion\AssertionAggregate
+ */
+ public function clearAssertions()
+ {
+ $this->assertions = array();
+
+ return $this;
+ }
+
+ /**
+ *
+ * @param AssertionManager $manager
+ *
+ * @return \Zend\Permissions\Acl\Assertion\AssertionAggregate
@samsonasik samsonasik commented on an outdated diff Dec 18, 2013
...Zend/Permissions/Acl/Assertion/AssertionAggregate.php
+ return $this->assertionManager;
+ }
+
+ /**
+ * Set assertion chain behavior
+ *
+ * AssertionAggregate should assert to true when:
+ *
+ * - all assertions are true with MODE_ALL
+ * - at least one assertion is true with MODE_AT_LEAST_ONE
+ *
+ * @param string $mode
+ * indicates how assertion chain result should interpreted (either 'all' or 'at_least_one')
+ * @throws Exception
+ *
+ * @return \Zend\Permissions\Acl\Assertion\AssertionAggregate
@samsonasik samsonasik and 2 others commented on an outdated diff Dec 18, 2013
...y/Zend/Permissions/Acl/Assertion/AssertionManager.php
+
+ protected $sharedByDefault = false;
+
+ /**
+ * Validate the plugin
+ *
+ * Checks that the element is an instance of AssertionInterface
+ *
+ * @param mixed $plugin
+ * @throws Zend\PErmissions\Acl\Exception\InvalidElementException
+ * @return void
+ */
+ public function validatePlugin($plugin)
+ {
+ if (! $plugin instanceof AssertionInterface) {
+ throw new InvalidArgumentException(sprintf('Plugin of type %s is invalid; must implement Zend\Permissions\Acl\Assertion\AssertionInterface', (is_object($plugin) ? get_class($plugin) : gettype($plugin))));
@samsonasik
samsonasik Dec 18, 2013

I think it should be multiline

@gauthier
gauthier Dec 18, 2013

the code has been reformated using php-cs-fixer with level psr2... since it's checked using this tool, I guess it's ok like this :)

by the way, the travis built passed

@danizord
danizord Dec 18, 2013

PSR 2:

The soft limit on line length MUST be 120 characters; automated style checkers MUST warn but MUST NOT error at the soft limit.
Lines SHOULD NOT be longer than 80 characters; lines longer than that SHOULD be split into multiple subsequent lines of no more than 80 characters each.

@samsonasik samsonasik commented on an outdated diff Dec 18, 2013
...y/Zend/Permissions/Acl/Assertion/AssertionManager.php
+use Zend\ServiceManager\AbstractPluginManager;
+use Zend\Permissions\Acl\Exception\InvalidArgumentException;
+
+class AssertionManager extends AbstractPluginManager
+{
+
+ protected $sharedByDefault = false;
+
+ /**
+ * Validate the plugin
+ *
+ * Checks that the element is an instance of AssertionInterface
+ *
+ * @param mixed $plugin
+ * @throws Zend\PErmissions\Acl\Exception\InvalidElementException
+ * @return void
@samsonasik samsonasik commented on an outdated diff Dec 18, 2013
...y/Zend/Permissions/Acl/Assertion/AssertionManager.php
+
+use Zend\ServiceManager\AbstractPluginManager;
+use Zend\Permissions\Acl\Exception\InvalidArgumentException;
+
+class AssertionManager extends AbstractPluginManager
+{
+
+ protected $sharedByDefault = false;
+
+ /**
+ * Validate the plugin
+ *
+ * Checks that the element is an instance of AssertionInterface
+ *
+ * @param mixed $plugin
+ * @throws Zend\PErmissions\Acl\Exception\InvalidElementException
@samsonasik
samsonasik Dec 18, 2013

@throws InvalidArgumentException , not InvalidElementException

@weierophinney weierophinney and 1 other commented on an outdated diff Mar 3, 2014
.../Permissions/Acl/Assertion/AssertionAggregateTest.php
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ */
+namespace ZendTest\Permissions\Acl\Assertion;
+
+use Zend\Permissions\Acl\Assertion\AssertionAggregate;
+use ZendTest\Permissions\Acl\TestAsset\MockAssertion;
+use Zend\Di\Exception\UndefinedReferenceException;
+
+class AssertionAggregateTest extends \PHPUnit_Framework_TestCase
+{
+
+ /**
+ *
+ * @var
+ *
+ */
@weierophinney
weierophinney Mar 3, 2014 Zend Framework member

Either remove the docblock, or make it useful. :)

@gauthier
gauthier Mar 3, 2014

humm... looks like this was a "temporary" comment to ease Code Insight or something like this - sorry :) I just removed it.

@weierophinney weierophinney added this to the 2.3.0 milestone Mar 3, 2014
@weierophinney
Member

Looks great!

@weierophinney weierophinney added a commit that referenced this pull request Mar 4, 2014
@weierophinney weierophinney [#5628] CS fixes
- trailing whitespace
68f3ac8
@weierophinney weierophinney added a commit that referenced this pull request Mar 4, 2014
@weierophinney weierophinney Merge branch 'feature/5628' into develop
Close #5628
1c46a43
@weierophinney weierophinney merged commit 17c3127 into zendframework:develop Mar 4, 2014

1 check failed

Details default The Travis CI build could not complete due to an error
@weierophinney weierophinney self-assigned this Mar 4, 2014
@arekkas arekkas referenced this pull request in ZF-Commons/zfc-rbac Apr 6, 2014
Open

AssertionAggregate? #217

@weierophinney weierophinney added a commit to zendframework/zend-permissions-acl that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#5628 from gauthier/acl…
…_assertions_enhancement

Acl assertions enhancement
8d65685
@weierophinney weierophinney added a commit to zendframework/zend-permissions-acl that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#5628] CS fixes
- trailing whitespace
04a79fa
@weierophinney weierophinney added a commit to zendframework/zend-permissions-acl that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'feature/5628' into develop a30a4f1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment