From 7efccb4c26518b351a8b58071cc3dbf4e79a4cfc Mon Sep 17 00:00:00 2001 From: Lisachenko Alexander Date: Sun, 3 Apr 2016 18:27:29 +0300 Subject: [PATCH 1/6] Merge classes into aspects to prevent scattering --- src/Aspect/InvariantCheckerAspect.php | 45 ++++++++++-- src/Aspect/PostconditionCheckerAspect.php | 57 +++++++++++++-- src/Aspect/PreconditionCheckerAspect.php | 50 +++++++++++-- src/Contract/InvariantContract.php | 86 ----------------------- src/Contract/PostconditionContract.php | 83 ---------------------- src/Contract/PreconditionContract.php | 76 -------------------- 6 files changed, 134 insertions(+), 263 deletions(-) delete mode 100644 src/Contract/InvariantContract.php delete mode 100644 src/Contract/PostconditionContract.php delete mode 100644 src/Contract/PreconditionContract.php diff --git a/src/Aspect/InvariantCheckerAspect.php b/src/Aspect/InvariantCheckerAspect.php index b78d44e..8be8978 100644 --- a/src/Aspect/InvariantCheckerAspect.php +++ b/src/Aspect/InvariantCheckerAspect.php @@ -13,20 +13,24 @@ use Doctrine\Common\Annotations\Reader; use Go\Aop\Aspect; use Go\Aop\Intercept\MethodInvocation; -use PhpDeal\Contract\InvariantContract; +use PhpDeal\Annotation\Invariant; +use PhpDeal\Contract\Contract; +use PhpDeal\Contract\Fetcher\ParentClass\InvariantFetcher; use PhpDeal\Exception\ContractViolation; use Go\Lang\Annotation\Around; +use ReflectionClass; -class InvariantCheckerAspect implements Aspect +class InvariantCheckerAspect extends Contract implements Aspect { /** - * @var InvariantContract + * @var InvariantFetcher */ - private $contractChecker; + private $invariantFetcher; public function __construct(Reader $reader) { - $this->contractChecker = new InvariantContract($reader); + parent::__construct($reader); + $this->invariantFetcher = new InvariantFetcher(Invariant::class); } /** @@ -40,6 +44,35 @@ public function __construct(Reader $reader) */ public function invariantContract(MethodInvocation $invocation) { - return $this->contractChecker->check($invocation); + $object = $invocation->getThis(); + $args = $this->getMethodArguments($invocation); + $class = $invocation->getMethod()->getDeclaringClass(); + if ($class->isCloneable()) { + $args['__old'] = clone $object; + } + + $result = $invocation->proceed(); + $args['__result'] = $result; + + $allContracts = $this->makeContractsUnique($this->fetchAllContracts($class)); + $this->fulfillContracts($allContracts, $object, $class->name, $args, $invocation); + + return $result; + } + + /** + * @param ReflectionClass $class + * @return array + */ + private function fetchAllContracts(ReflectionClass $class) + { + $allContracts = $this->invariantFetcher->getConditions($class, $this->reader); + foreach ($this->reader->getClassAnnotations($class) as $annotation) { + if ($annotation instanceof Invariant) { + $allContracts[] = $annotation; + } + } + + return $allContracts; } } diff --git a/src/Aspect/PostconditionCheckerAspect.php b/src/Aspect/PostconditionCheckerAspect.php index b125f92..24b9f29 100644 --- a/src/Aspect/PostconditionCheckerAspect.php +++ b/src/Aspect/PostconditionCheckerAspect.php @@ -13,20 +13,23 @@ use Doctrine\Common\Annotations\Reader; use Go\Aop\Aspect; use Go\Aop\Intercept\MethodInvocation; -use PhpDeal\Contract\PostconditionContract; +use PhpDeal\Annotation\Ensure; +use PhpDeal\Contract\Contract; +use PhpDeal\Contract\Fetcher\ParentClass\MethodConditionFetcher; use PhpDeal\Exception\ContractViolation; use Go\Lang\Annotation\Around; -class PostconditionCheckerAspect implements Aspect +class PostconditionCheckerAspect extends Contract implements Aspect { /** - * @var PostconditionContract + * @var MethodConditionFetcher */ - private $contractChecker; + private $methodConditionFetcher; public function __construct(Reader $reader) { - $this->contractChecker = new PostconditionContract($reader); + parent::__construct($reader); + $this->methodConditionFetcher = new MethodConditionFetcher(Ensure::class); } /** @@ -40,6 +43,48 @@ public function __construct(Reader $reader) */ public function postConditionContract(MethodInvocation $invocation) { - return $this->contractChecker->check($invocation); + $object = $invocation->getThis(); + $args = $this->getMethodArguments($invocation); + $class = $invocation->getMethod()->getDeclaringClass(); + if ($class->isCloneable()) { + $args['__old'] = clone $object; + } + + $result = $invocation->proceed(); + $args['__result'] = $result; + $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); + + $this->fulfillContracts($allContracts, $object, $class->name, $args, $invocation); + + return $result; + } + + /** + * @param MethodInvocation $invocation + * @return array + */ + private function fetchAllContracts(MethodInvocation $invocation) + { + $allContracts = $this->fetchParentsContracts($invocation); + foreach ($invocation->getMethod()->getAnnotations() as $annotation) { + if ($annotation instanceof Ensure) { + $allContracts[] = $annotation; + } + } + + return $allContracts; + } + + /** + * @param MethodInvocation $invocation + * @return array + */ + private function fetchParentsContracts(MethodInvocation $invocation) + { + return $this->methodConditionFetcher->getConditions( + $invocation->getMethod()->getDeclaringClass(), + $this->reader, + $invocation->getMethod()->name + ); } } diff --git a/src/Aspect/PreconditionCheckerAspect.php b/src/Aspect/PreconditionCheckerAspect.php index 2bc7aec..c63f86b 100644 --- a/src/Aspect/PreconditionCheckerAspect.php +++ b/src/Aspect/PreconditionCheckerAspect.php @@ -13,20 +13,23 @@ use Doctrine\Common\Annotations\Reader; use Go\Aop\Aspect; use Go\Aop\Intercept\MethodInvocation; -use PhpDeal\Contract\PreconditionContract; +use PhpDeal\Annotation\Verify; +use PhpDeal\Contract\Contract; +use PhpDeal\Contract\Fetcher\ParentClass\MethodConditionWithInheritDocFetcher; use PhpDeal\Exception\ContractViolation; use Go\Lang\Annotation\Before; -class PreconditionCheckerAspect implements Aspect +class PreconditionCheckerAspect extends Contract implements Aspect { /** - * @var PreconditionContract + * @var MethodConditionWithInheritDocFetcher */ - private $contractChecker; + private $methodConditionFetcher; public function __construct(Reader $reader) { - $this->contractChecker = new PreconditionContract($reader); + parent::__construct($reader); + $this->methodConditionFetcher = new MethodConditionWithInheritDocFetcher(Verify::class); } /** @@ -39,6 +42,41 @@ public function __construct(Reader $reader) */ public function preConditionContract(MethodInvocation $invocation) { - $this->contractChecker->check($invocation); + $object = $invocation->getThis(); + $args = $this->getMethodArguments($invocation); + $scope = $invocation->getMethod()->getDeclaringClass()->name; + + $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); + $this->fulfillContracts($allContracts, $object, $scope, $args, $invocation); + } + + /** + * @param MethodInvocation $invocation + * @return array + */ + private function fetchAllContracts(MethodInvocation $invocation) + { + $allContracts = $this->fetchParentsContracts($invocation); + + foreach ($invocation->getMethod()->getAnnotations() as $annotation) { + if ($annotation instanceof Verify) { + $allContracts[] = $annotation; + } + } + + return $allContracts; + } + + /** + * @param MethodInvocation $invocation + * @return array + */ + private function fetchParentsContracts(MethodInvocation $invocation) + { + return $this->methodConditionFetcher->getConditions( + $invocation->getMethod()->getDeclaringClass(), + $this->reader, + $invocation->getMethod()->name + ); } } diff --git a/src/Contract/InvariantContract.php b/src/Contract/InvariantContract.php deleted file mode 100644 index 2e179d8..0000000 --- a/src/Contract/InvariantContract.php +++ /dev/null @@ -1,86 +0,0 @@ - - * - * This source file is subject to the license that is bundled - * with this source code in the file LICENSE. - */ - -namespace PhpDeal\Contract; - -use Doctrine\Common\Annotations\Reader; -use Go\Aop\Intercept\MethodInvocation; -use PhpDeal\Contract\Fetcher\ParentClass\InvariantFetcher; -use PhpDeal\Exception\ContractViolation; -use PhpDeal\Annotation\Invariant; -use ReflectionClass; - -class InvariantContract extends Contract -{ - /** - * @var InvariantFetcher - */ - private $invariantFetcher; - - public function __construct(Reader $reader) - { - parent::__construct($reader); - $this->invariantFetcher = new InvariantFetcher(Invariant::class); - } - - /** - * Verifies invariants for contract class - * - * @Around("@within(PhpDeal\Annotation\Invariant) && execution(public **->*(*))") - * @param MethodInvocation $invocation - * @throws ContractViolation - * @return mixed - */ - public function check(MethodInvocation $invocation) - { - $object = $invocation->getThis(); - $args = $this->getMethodArguments($invocation); - $class = $invocation->getMethod()->getDeclaringClass(); - if ($class->isCloneable()) { - $args['__old'] = clone $object; - } - - $result = $invocation->proceed(); - $args['__result'] = $result; - - $allContracts = $this->makeContractsUnique($this->fetchAllContracts($class)); - $this->fulfillContracts($allContracts, $object, $class->name, $args, $invocation); - - return $result; - } - - /** - * @param ReflectionClass $class - * @return array - */ - private function fetchAllContracts(ReflectionClass $class) - { - $allContracts = $this->fetchParentsContracts($class); - foreach ($this->reader->getClassAnnotations($class) as $annotation) { - if ($annotation instanceof Invariant) { - $allContracts[] = $annotation; - } - } - - return $allContracts; - } - - /** - * @param ReflectionClass $class - * @return array - */ - private function fetchParentsContracts(ReflectionClass $class) - { - return $this->invariantFetcher->getConditions( - $class, - $this->reader - ); - } -} diff --git a/src/Contract/PostconditionContract.php b/src/Contract/PostconditionContract.php deleted file mode 100644 index cf93e5e..0000000 --- a/src/Contract/PostconditionContract.php +++ /dev/null @@ -1,83 +0,0 @@ - - * - * This source file is subject to the license that is bundled - * with this source code in the file LICENSE. - */ - -namespace PhpDeal\Contract; - -use Doctrine\Common\Annotations\Reader; -use Go\Aop\Intercept\MethodInvocation; -use PhpDeal\Contract\Fetcher\ParentClass\MethodConditionFetcher; -use PhpDeal\Exception\ContractViolation; -use PhpDeal\Annotation\Ensure; - -class PostconditionContract extends Contract -{ - /** - * @var MethodConditionFetcher - */ - private $methodConditionFetcher; - - public function __construct(Reader $reader) - { - parent::__construct($reader); - $this->methodConditionFetcher = new MethodConditionFetcher(Ensure::class); - } - - /** - * @param MethodInvocation $invocation - * @throws ContractViolation - * @return mixed - */ - public function check(MethodInvocation $invocation) - { - $object = $invocation->getThis(); - $args = $this->getMethodArguments($invocation); - $class = $invocation->getMethod()->getDeclaringClass(); - if ($class->isCloneable()) { - $args['__old'] = clone $object; - } - - $result = $invocation->proceed(); - $args['__result'] = $result; - $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); - - $this->fulfillContracts($allContracts, $object, $class->name, $args, $invocation); - - return $result; - } - - /** - * @param MethodInvocation $invocation - * @return array - */ - private function fetchAllContracts(MethodInvocation $invocation) - { - $allContracts = $this->fetchParentsContracts($invocation); - foreach ($invocation->getMethod()->getAnnotations() as $annotation) { - if ($annotation instanceof Ensure) { - $allContracts[] = $annotation; - } - } - - return $allContracts; - } - - /** - * @param MethodInvocation $invocation - * @return array - */ - private function fetchParentsContracts(MethodInvocation $invocation) - { - return $this->methodConditionFetcher->getConditions( - $invocation->getMethod()->getDeclaringClass(), - $this->reader, - $invocation->getMethod()->name - ); - } -} diff --git a/src/Contract/PreconditionContract.php b/src/Contract/PreconditionContract.php deleted file mode 100644 index c2ad95c..0000000 --- a/src/Contract/PreconditionContract.php +++ /dev/null @@ -1,76 +0,0 @@ - - * - * This source file is subject to the license that is bundled - * with this source code in the file LICENSE. - */ - -namespace PhpDeal\Contract; - -use Doctrine\Common\Annotations\Reader; -use Go\Aop\Intercept\MethodInvocation; -use PhpDeal\Contract\Fetcher\ParentClass\MethodConditionWithInheritDocFetcher; -use PhpDeal\Exception\ContractViolation; -use PhpDeal\Annotation\Verify; - -class PreconditionContract extends Contract -{ - /** - * @var MethodConditionWithInheritDocFetcher - */ - private $methodConditionFetcher; - - public function __construct(Reader $reader) - { - parent::__construct($reader); - $this->methodConditionFetcher = new MethodConditionWithInheritDocFetcher(Verify::class); - } - - /** - * @param MethodInvocation $invocation - * @Before("@execution(PhpDeal\Annotation\Verify)") - * @throws ContractViolation - */ - public function check(MethodInvocation $invocation) - { - $object = $invocation->getThis(); - $args = $this->getMethodArguments($invocation); - $scope = $invocation->getMethod()->getDeclaringClass()->name; - - $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); - $this->fulfillContracts($allContracts, $object, $scope, $args, $invocation); - } - - /** - * @param MethodInvocation $invocation - * @return array - */ - private function fetchAllContracts(MethodInvocation $invocation) - { - $allContracts = $this->fetchParentsContracts($invocation); - - foreach ($invocation->getMethod()->getAnnotations() as $annotation) { - if ($annotation instanceof Verify) { - $allContracts[] = $annotation; - } - } - - return $allContracts; - } - - /** - * @param MethodInvocation $invocation - * @return array - */ - private function fetchParentsContracts(MethodInvocation $invocation) - { - return $this->methodConditionFetcher->getConditions( - $invocation->getMethod()->getDeclaringClass(), - $this->reader, - $invocation->getMethod()->name - ); - } -} From e7e05a5b04e271696aeb5f5bf2414186cdcab8a7 Mon Sep 17 00:00:00 2001 From: Lisachenko Alexander Date: Sun, 3 Apr 2016 18:38:54 +0300 Subject: [PATCH 2/6] Removed Fetcher/MethodArgument class because it doesn't contain any separate logic --- src/Aspect/InvariantCheckerAspect.php | 2 +- src/Aspect/PostconditionCheckerAspect.php | 2 +- src/Aspect/PreconditionCheckerAspect.php | 2 +- src/Contract/Contract.php | 13 ++++++--- src/Contract/Fetcher/MethodArgument.php | 33 ----------------------- 5 files changed, 13 insertions(+), 39 deletions(-) delete mode 100644 src/Contract/Fetcher/MethodArgument.php diff --git a/src/Aspect/InvariantCheckerAspect.php b/src/Aspect/InvariantCheckerAspect.php index 8be8978..d7922b3 100644 --- a/src/Aspect/InvariantCheckerAspect.php +++ b/src/Aspect/InvariantCheckerAspect.php @@ -45,7 +45,7 @@ public function __construct(Reader $reader) public function invariantContract(MethodInvocation $invocation) { $object = $invocation->getThis(); - $args = $this->getMethodArguments($invocation); + $args = $this->fetchMethodArguments($invocation); $class = $invocation->getMethod()->getDeclaringClass(); if ($class->isCloneable()) { $args['__old'] = clone $object; diff --git a/src/Aspect/PostconditionCheckerAspect.php b/src/Aspect/PostconditionCheckerAspect.php index 24b9f29..3c017e9 100644 --- a/src/Aspect/PostconditionCheckerAspect.php +++ b/src/Aspect/PostconditionCheckerAspect.php @@ -44,7 +44,7 @@ public function __construct(Reader $reader) public function postConditionContract(MethodInvocation $invocation) { $object = $invocation->getThis(); - $args = $this->getMethodArguments($invocation); + $args = $this->fetchMethodArguments($invocation); $class = $invocation->getMethod()->getDeclaringClass(); if ($class->isCloneable()) { $args['__old'] = clone $object; diff --git a/src/Aspect/PreconditionCheckerAspect.php b/src/Aspect/PreconditionCheckerAspect.php index c63f86b..e975ae0 100644 --- a/src/Aspect/PreconditionCheckerAspect.php +++ b/src/Aspect/PreconditionCheckerAspect.php @@ -43,7 +43,7 @@ public function __construct(Reader $reader) public function preConditionContract(MethodInvocation $invocation) { $object = $invocation->getThis(); - $args = $this->getMethodArguments($invocation); + $args = $this->fetchMethodArguments($invocation); $scope = $invocation->getMethod()->getDeclaringClass()->name; $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); diff --git a/src/Contract/Contract.php b/src/Contract/Contract.php index 04bba17..2a942cc 100644 --- a/src/Contract/Contract.php +++ b/src/Contract/Contract.php @@ -14,7 +14,6 @@ use Doctrine\Common\Annotations\Reader; use DomainException; use Go\Aop\Intercept\MethodInvocation; -use PhpDeal\Contract\Fetcher\MethodArgument; use PhpDeal\Exception\ContractViolation; abstract class Contract @@ -33,12 +32,20 @@ public function __construct(Reader $reader) } /** + * Returns an associative list of arguments for the method invocation + * * @param MethodInvocation $invocation * @return array */ - protected function getMethodArguments(MethodInvocation $invocation) + protected function fetchMethodArguments(MethodInvocation $invocation) { - return (new MethodArgument())->fetch($invocation); + $parameters = $invocation->getMethod()->getParameters(); + $argumentNames = array_map(function (\ReflectionParameter $parameter) { + return $parameter->name; + }, $parameters); + $parameters = array_combine($argumentNames, $invocation->getArguments()); + + return $parameters; } /** diff --git a/src/Contract/Fetcher/MethodArgument.php b/src/Contract/Fetcher/MethodArgument.php deleted file mode 100644 index f1329d5..0000000 --- a/src/Contract/Fetcher/MethodArgument.php +++ /dev/null @@ -1,33 +0,0 @@ - - * - * This source file is subject to the license that is bundled - * with this source code in the file LICENSE. - */ - -namespace PhpDeal\Contract\Fetcher; - -use Go\Aop\Intercept\MethodInvocation; - -class MethodArgument -{ - /** - * Returns an associative list of arguments for the method invocation - * - * @param MethodInvocation $invocation - * @return array - */ - public function fetch(MethodInvocation $invocation) - { - $parameters = $invocation->getMethod()->getParameters(); - $argumentNames = array_map(function (\ReflectionParameter $parameter) { - return $parameter->name; - }, $parameters); - $parameters = array_combine($argumentNames, $invocation->getArguments()); - - return $parameters; - } -} From 416c35cc9c9bc669b7c252517a578a6ed6a896a3 Mon Sep 17 00:00:00 2001 From: Lisachenko Alexander Date: Sun, 3 Apr 2016 19:09:10 +0300 Subject: [PATCH 3/6] Simplify execution of contracts, make code more local for faster performance --- src/Aspect/InvariantCheckerAspect.php | 2 +- src/Aspect/PostconditionCheckerAspect.php | 2 +- src/Aspect/PreconditionCheckerAspect.php | 2 +- src/Contract/Contract.php | 54 +++++++++++------------ 4 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/Aspect/InvariantCheckerAspect.php b/src/Aspect/InvariantCheckerAspect.php index d7922b3..2576367 100644 --- a/src/Aspect/InvariantCheckerAspect.php +++ b/src/Aspect/InvariantCheckerAspect.php @@ -55,7 +55,7 @@ public function invariantContract(MethodInvocation $invocation) $args['__result'] = $result; $allContracts = $this->makeContractsUnique($this->fetchAllContracts($class)); - $this->fulfillContracts($allContracts, $object, $class->name, $args, $invocation); + $this->ensureContracts($invocation, $allContracts, $object, $class->name, $args); return $result; } diff --git a/src/Aspect/PostconditionCheckerAspect.php b/src/Aspect/PostconditionCheckerAspect.php index 3c017e9..7a42bef 100644 --- a/src/Aspect/PostconditionCheckerAspect.php +++ b/src/Aspect/PostconditionCheckerAspect.php @@ -54,7 +54,7 @@ public function postConditionContract(MethodInvocation $invocation) $args['__result'] = $result; $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); - $this->fulfillContracts($allContracts, $object, $class->name, $args, $invocation); + $this->ensureContracts($invocation, $allContracts, $object, $class->name, $args); return $result; } diff --git a/src/Aspect/PreconditionCheckerAspect.php b/src/Aspect/PreconditionCheckerAspect.php index e975ae0..ecc094a 100644 --- a/src/Aspect/PreconditionCheckerAspect.php +++ b/src/Aspect/PreconditionCheckerAspect.php @@ -47,7 +47,7 @@ public function preConditionContract(MethodInvocation $invocation) $scope = $invocation->getMethod()->getDeclaringClass()->name; $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); - $this->fulfillContracts($allContracts, $object, $scope, $args, $invocation); + $this->ensureContracts($invocation, $allContracts, $object, $scope, $args); } /** diff --git a/src/Contract/Contract.php b/src/Contract/Contract.php index 2a942cc..162cb40 100644 --- a/src/Contract/Contract.php +++ b/src/Contract/Contract.php @@ -58,33 +58,17 @@ protected function makeContractsUnique(array $allContracts) } /** - * @param array $allContracts - * @param object|string $instance - * @param string $scope - * @param array $args - * @param MethodInvocation $invocation - */ - protected function fulfillContracts($allContracts, $instance, $scope, array $args, MethodInvocation $invocation) - { - foreach ($allContracts as $contract) { - try { - $this->ensureContractSatisfied($instance, $scope, $args, $contract); - } catch (\Exception $e) { - throw new ContractViolation($invocation, $contract->value, $e); - } - } - } - - /** - * Returns a result of contract verification + * Performs verification of contracts for given invocation * + * @param MethodInvocation $invocation Current invocation + * @param array|Annotation[] $contracts Contract annotation * @param object|string $instance Invocation instance or string for static class * @param string $scope Scope of method * @param array $args List of arguments for the method - * @param Annotation $annotation Contract annotation + * * @throws DomainException */ - public function ensureContractSatisfied($instance, $scope, array $args, $annotation) + protected function ensureContracts(MethodInvocation $invocation, array $contracts, $instance, $scope, array $args) { static $invoker = null; if (!$invoker) { @@ -95,14 +79,28 @@ public function ensureContractSatisfied($instance, $scope, array $args, $annotat }; } - $instance = is_object($instance) ? $instance : null; - $invocationResult = $invoker->bindTo($instance, $scope)->__invoke($args, $annotation->value); + $instance = is_object($instance) ? $instance : null; + $boundInvoker = $invoker->bindTo($instance, $scope); - // we accept as a result only true or null - // null may be a result of assertions from beberlei/assert which passed - if ($invocationResult !== null && $invocationResult !== true) { - $errorMessage = 'Invalid return value received from the assertion body, only boolean or void accepted'; - throw new DomainException($errorMessage); + foreach ($contracts as $contract) { + $contractExpression = $contract->value; + try { + $invocationResult = $boundInvoker->__invoke($args, $contractExpression); + + // we accept as a result only true or null + // null may be a result of assertions from beberlei/assert which passed + if ($invocationResult !== null && $invocationResult !== true) { + $errorMessage = 'Invalid return value received from the assertion body,' + . ' only boolean or void can be returned'; + throw new DomainException($errorMessage); + } + + } catch (\Error $internalError) { + // PHP-7 friendly interceptor for fatal errors + throw new ContractViolation($invocation, $contractExpression, $internalError); + } catch (\Exception $internalException) { + throw new ContractViolation($invocation, $contractExpression, $internalException); + } } } } From 9995f2240c40d9ab44be0992040d0af783a91b61 Mon Sep 17 00:00:00 2001 From: Lisachenko Alexander Date: Sun, 3 Apr 2016 19:13:30 +0300 Subject: [PATCH 4/6] Removed makeContractsUnique method to prevent extra method call on each check --- src/Aspect/InvariantCheckerAspect.php | 4 ++-- src/Aspect/PostconditionCheckerAspect.php | 4 ++-- src/Aspect/PreconditionCheckerAspect.php | 4 ++-- src/Contract/Contract.php | 9 --------- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/Aspect/InvariantCheckerAspect.php b/src/Aspect/InvariantCheckerAspect.php index 2576367..a2d3663 100644 --- a/src/Aspect/InvariantCheckerAspect.php +++ b/src/Aspect/InvariantCheckerAspect.php @@ -54,7 +54,7 @@ public function invariantContract(MethodInvocation $invocation) $result = $invocation->proceed(); $args['__result'] = $result; - $allContracts = $this->makeContractsUnique($this->fetchAllContracts($class)); + $allContracts = $this->fetchAllContracts($class); $this->ensureContracts($invocation, $allContracts, $object, $class->name, $args); return $result; @@ -73,6 +73,6 @@ private function fetchAllContracts(ReflectionClass $class) } } - return $allContracts; + return array_unique($allContracts); } } diff --git a/src/Aspect/PostconditionCheckerAspect.php b/src/Aspect/PostconditionCheckerAspect.php index 7a42bef..8737b09 100644 --- a/src/Aspect/PostconditionCheckerAspect.php +++ b/src/Aspect/PostconditionCheckerAspect.php @@ -52,7 +52,7 @@ public function postConditionContract(MethodInvocation $invocation) $result = $invocation->proceed(); $args['__result'] = $result; - $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); + $allContracts = $this->fetchAllContracts($invocation); $this->ensureContracts($invocation, $allContracts, $object, $class->name, $args); @@ -72,7 +72,7 @@ private function fetchAllContracts(MethodInvocation $invocation) } } - return $allContracts; + return array_unique($allContracts); } /** diff --git a/src/Aspect/PreconditionCheckerAspect.php b/src/Aspect/PreconditionCheckerAspect.php index ecc094a..a840403 100644 --- a/src/Aspect/PreconditionCheckerAspect.php +++ b/src/Aspect/PreconditionCheckerAspect.php @@ -46,7 +46,7 @@ public function preConditionContract(MethodInvocation $invocation) $args = $this->fetchMethodArguments($invocation); $scope = $invocation->getMethod()->getDeclaringClass()->name; - $allContracts = $this->makeContractsUnique($this->fetchAllContracts($invocation)); + $allContracts = $this->fetchAllContracts($invocation); $this->ensureContracts($invocation, $allContracts, $object, $scope, $args); } @@ -64,7 +64,7 @@ private function fetchAllContracts(MethodInvocation $invocation) } } - return $allContracts; + return array_unique($allContracts); } /** diff --git a/src/Contract/Contract.php b/src/Contract/Contract.php index 162cb40..8ea18a8 100644 --- a/src/Contract/Contract.php +++ b/src/Contract/Contract.php @@ -48,15 +48,6 @@ protected function fetchMethodArguments(MethodInvocation $invocation) return $parameters; } - /** - * @param array $allContracts - * @return array - */ - protected function makeContractsUnique(array $allContracts) - { - return array_unique($allContracts); - } - /** * Performs verification of contracts for given invocation * From 9be4cdd8960f6fba1f376e0ae0eb1179492410e5 Mon Sep 17 00:00:00 2001 From: Lisachenko Alexander Date: Sun, 3 Apr 2016 19:21:01 +0300 Subject: [PATCH 5/6] Moved the Contract class as AbstractContractAspect --- .../Contract.php => Aspect/AbstractContractAspect.php} | 4 ++-- src/Aspect/InvariantCheckerAspect.php | 3 +-- src/Aspect/PostconditionCheckerAspect.php | 3 +-- src/Aspect/PreconditionCheckerAspect.php | 3 +-- 4 files changed, 5 insertions(+), 8 deletions(-) rename src/{Contract/Contract.php => Aspect/AbstractContractAspect.php} (98%) diff --git a/src/Contract/Contract.php b/src/Aspect/AbstractContractAspect.php similarity index 98% rename from src/Contract/Contract.php rename to src/Aspect/AbstractContractAspect.php index 8ea18a8..14e70e4 100644 --- a/src/Contract/Contract.php +++ b/src/Aspect/AbstractContractAspect.php @@ -8,7 +8,7 @@ * with this source code in the file LICENSE. */ -namespace PhpDeal\Contract; +namespace PhpDeal\Aspect; use Doctrine\Common\Annotations\Annotation; use Doctrine\Common\Annotations\Reader; @@ -16,7 +16,7 @@ use Go\Aop\Intercept\MethodInvocation; use PhpDeal\Exception\ContractViolation; -abstract class Contract +abstract class AbstractContractAspect { /** * @var Reader diff --git a/src/Aspect/InvariantCheckerAspect.php b/src/Aspect/InvariantCheckerAspect.php index a2d3663..71a9280 100644 --- a/src/Aspect/InvariantCheckerAspect.php +++ b/src/Aspect/InvariantCheckerAspect.php @@ -14,13 +14,12 @@ use Go\Aop\Aspect; use Go\Aop\Intercept\MethodInvocation; use PhpDeal\Annotation\Invariant; -use PhpDeal\Contract\Contract; use PhpDeal\Contract\Fetcher\ParentClass\InvariantFetcher; use PhpDeal\Exception\ContractViolation; use Go\Lang\Annotation\Around; use ReflectionClass; -class InvariantCheckerAspect extends Contract implements Aspect +class InvariantCheckerAspect extends AbstractContractAspect implements Aspect { /** * @var InvariantFetcher diff --git a/src/Aspect/PostconditionCheckerAspect.php b/src/Aspect/PostconditionCheckerAspect.php index 8737b09..f0cc26f 100644 --- a/src/Aspect/PostconditionCheckerAspect.php +++ b/src/Aspect/PostconditionCheckerAspect.php @@ -14,12 +14,11 @@ use Go\Aop\Aspect; use Go\Aop\Intercept\MethodInvocation; use PhpDeal\Annotation\Ensure; -use PhpDeal\Contract\Contract; use PhpDeal\Contract\Fetcher\ParentClass\MethodConditionFetcher; use PhpDeal\Exception\ContractViolation; use Go\Lang\Annotation\Around; -class PostconditionCheckerAspect extends Contract implements Aspect +class PostconditionCheckerAspect extends AbstractContractAspect implements Aspect { /** * @var MethodConditionFetcher diff --git a/src/Aspect/PreconditionCheckerAspect.php b/src/Aspect/PreconditionCheckerAspect.php index a840403..a74f8c8 100644 --- a/src/Aspect/PreconditionCheckerAspect.php +++ b/src/Aspect/PreconditionCheckerAspect.php @@ -14,12 +14,11 @@ use Go\Aop\Aspect; use Go\Aop\Intercept\MethodInvocation; use PhpDeal\Annotation\Verify; -use PhpDeal\Contract\Contract; use PhpDeal\Contract\Fetcher\ParentClass\MethodConditionWithInheritDocFetcher; use PhpDeal\Exception\ContractViolation; use Go\Lang\Annotation\Before; -class PreconditionCheckerAspect extends Contract implements Aspect +class PreconditionCheckerAspect extends AbstractContractAspect implements Aspect { /** * @var MethodConditionWithInheritDocFetcher From e1abbaedd4126ff10ef266d78fc11a04aeb54875 Mon Sep 17 00:00:00 2001 From: Lisachenko Alexander Date: Sun, 3 Apr 2016 20:47:51 +0300 Subject: [PATCH 6/6] Replace recursion with parent iterations in fetchers --- src/Aspect/InvariantCheckerAspect.php | 4 +- src/Aspect/PostconditionCheckerAspect.php | 3 +- src/Aspect/PreconditionCheckerAspect.php | 3 +- .../{Fetcher.php => AbstractFetcher.php} | 19 +++++-- .../Fetcher/ParentClass/InvariantFetcher.php | 27 ++++----- .../ParentClass/MethodConditionFetcher.php | 28 ++++----- .../MethodConditionWithInheritDocFetcher.php | 57 ++++++------------- 7 files changed, 63 insertions(+), 78 deletions(-) rename src/Contract/Fetcher/ParentClass/{Fetcher.php => AbstractFetcher.php} (60%) diff --git a/src/Aspect/InvariantCheckerAspect.php b/src/Aspect/InvariantCheckerAspect.php index 71a9280..e10bb8a 100644 --- a/src/Aspect/InvariantCheckerAspect.php +++ b/src/Aspect/InvariantCheckerAspect.php @@ -29,7 +29,7 @@ class InvariantCheckerAspect extends AbstractContractAspect implements Aspect public function __construct(Reader $reader) { parent::__construct($reader); - $this->invariantFetcher = new InvariantFetcher(Invariant::class); + $this->invariantFetcher = new InvariantFetcher(Invariant::class, $reader); } /** @@ -65,7 +65,7 @@ public function invariantContract(MethodInvocation $invocation) */ private function fetchAllContracts(ReflectionClass $class) { - $allContracts = $this->invariantFetcher->getConditions($class, $this->reader); + $allContracts = $this->invariantFetcher->getConditions($class); foreach ($this->reader->getClassAnnotations($class) as $annotation) { if ($annotation instanceof Invariant) { $allContracts[] = $annotation; diff --git a/src/Aspect/PostconditionCheckerAspect.php b/src/Aspect/PostconditionCheckerAspect.php index f0cc26f..a24c5c3 100644 --- a/src/Aspect/PostconditionCheckerAspect.php +++ b/src/Aspect/PostconditionCheckerAspect.php @@ -28,7 +28,7 @@ class PostconditionCheckerAspect extends AbstractContractAspect implements Aspec public function __construct(Reader $reader) { parent::__construct($reader); - $this->methodConditionFetcher = new MethodConditionFetcher(Ensure::class); + $this->methodConditionFetcher = new MethodConditionFetcher(Ensure::class, $reader); } /** @@ -82,7 +82,6 @@ private function fetchParentsContracts(MethodInvocation $invocation) { return $this->methodConditionFetcher->getConditions( $invocation->getMethod()->getDeclaringClass(), - $this->reader, $invocation->getMethod()->name ); } diff --git a/src/Aspect/PreconditionCheckerAspect.php b/src/Aspect/PreconditionCheckerAspect.php index a74f8c8..4346d07 100644 --- a/src/Aspect/PreconditionCheckerAspect.php +++ b/src/Aspect/PreconditionCheckerAspect.php @@ -28,7 +28,7 @@ class PreconditionCheckerAspect extends AbstractContractAspect implements Aspect public function __construct(Reader $reader) { parent::__construct($reader); - $this->methodConditionFetcher = new MethodConditionWithInheritDocFetcher(Verify::class); + $this->methodConditionFetcher = new MethodConditionWithInheritDocFetcher(Verify::class, $reader); } /** @@ -74,7 +74,6 @@ private function fetchParentsContracts(MethodInvocation $invocation) { return $this->methodConditionFetcher->getConditions( $invocation->getMethod()->getDeclaringClass(), - $this->reader, $invocation->getMethod()->name ); } diff --git a/src/Contract/Fetcher/ParentClass/Fetcher.php b/src/Contract/Fetcher/ParentClass/AbstractFetcher.php similarity index 60% rename from src/Contract/Fetcher/ParentClass/Fetcher.php rename to src/Contract/Fetcher/ParentClass/AbstractFetcher.php index b7e31fb..1727bee 100644 --- a/src/Contract/Fetcher/ParentClass/Fetcher.php +++ b/src/Contract/Fetcher/ParentClass/AbstractFetcher.php @@ -10,31 +10,42 @@ namespace PhpDeal\Contract\Fetcher\ParentClass; -class Fetcher +use Doctrine\Common\Annotations\Reader; + +abstract class AbstractFetcher { /** * @var string */ protected $expectedAnnotationType; + /** + * @var Reader + */ + protected $annotationReader; + /** * @param string $expectedAnnotationType + * @param Reader $reader */ - public function __construct($expectedAnnotationType) + public function __construct($expectedAnnotationType, Reader $reader) { $this->expectedAnnotationType = $expectedAnnotationType; + $this->annotationReader = $reader; } /** + * Performs filtering of annotations by the requested class name + * * @param array $annotations * @return array */ - protected function getContractAnnotations(array $annotations) + protected function filterContractAnnotation(array $annotations) { $contractAnnotations = []; foreach ($annotations as $annotation) { - if (is_a($annotation, $this->expectedAnnotationType)) { + if ($annotation instanceof $this->expectedAnnotationType) { $contractAnnotations[] = $annotation; } } diff --git a/src/Contract/Fetcher/ParentClass/InvariantFetcher.php b/src/Contract/Fetcher/ParentClass/InvariantFetcher.php index bee48d4..7638452 100644 --- a/src/Contract/Fetcher/ParentClass/InvariantFetcher.php +++ b/src/Contract/Fetcher/ParentClass/InvariantFetcher.php @@ -10,29 +10,30 @@ namespace PhpDeal\Contract\Fetcher\ParentClass; -use Doctrine\Common\Annotations\Reader; use ReflectionClass; -class InvariantFetcher extends Fetcher +class InvariantFetcher extends AbstractFetcher { /** + * Fetches conditions from all parent classes recursively + * * @param ReflectionClass $class - * @param Reader $reader - * @param array $contracts + * * @return array */ - public function getConditions(ReflectionClass $class, Reader $reader, array $contracts = []) + public function getConditions(ReflectionClass $class) { - $parentClass = $class->getParentClass(); - - if (!$parentClass) { - return $contracts; + $annotations = []; + $parentClasses = []; + while ($class = $class->getParentClass()) { + $parentClasses[] = $class; } - $annotations = $reader->getClassAnnotations($parentClass); - $contractAnnotations = $this->getContractAnnotations($annotations); - $contracts = array_merge($contracts, $contractAnnotations); + foreach ($parentClasses as $parentClass) { + $annotations = array_merge($annotations, $this->annotationReader->getClassAnnotations($parentClass)); + } + $contracts = $this->filterContractAnnotation($annotations); - return $this->getConditions($parentClass, $reader, $contracts); + return $contracts; } } diff --git a/src/Contract/Fetcher/ParentClass/MethodConditionFetcher.php b/src/Contract/Fetcher/ParentClass/MethodConditionFetcher.php index c654fe9..87d6069 100644 --- a/src/Contract/Fetcher/ParentClass/MethodConditionFetcher.php +++ b/src/Contract/Fetcher/ParentClass/MethodConditionFetcher.php @@ -10,31 +10,31 @@ namespace PhpDeal\Contract\Fetcher\ParentClass; -use Doctrine\Common\Annotations\Reader; use ReflectionClass; -class MethodConditionFetcher extends Fetcher +class MethodConditionFetcher extends AbstractFetcher { /** + * Fetches conditions from all parent method prototypes recursively + * * @param ReflectionClass $class - * @param Reader $reader * @param string $methodName - * @param array $contracts + * * @return array */ - public function getConditions(ReflectionClass $class, Reader $reader, $methodName, array $contracts = []) + public function getConditions(ReflectionClass $class, $methodName) { - $parentClass = $class->getParentClass(); - - if (!$parentClass) { - return $contracts; + $annotations = []; + $parentMethods = []; + while (($class = $class->getParentClass()) && $class->hasMethod($methodName)) { + $parentMethods[] = $class->getMethod($methodName); } - $parentMethod = $parentClass->getMethod($methodName); - $annotations = $reader->getMethodAnnotations($parentMethod); - $contractAnnotations = $this->getContractAnnotations($annotations); - $contracts = array_merge($contracts, $contractAnnotations); + foreach ($parentMethods as $parentMethod) { + $annotations = array_merge($annotations, $this->annotationReader->getMethodAnnotations($parentMethod)); + } + $contracts = $this->filterContractAnnotation($annotations); - return $this->getConditions($parentClass, $reader, $methodName, $contracts); + return $contracts; } } diff --git a/src/Contract/Fetcher/ParentClass/MethodConditionWithInheritDocFetcher.php b/src/Contract/Fetcher/ParentClass/MethodConditionWithInheritDocFetcher.php index 8803bfb..7d70388 100644 --- a/src/Contract/Fetcher/ParentClass/MethodConditionWithInheritDocFetcher.php +++ b/src/Contract/Fetcher/ParentClass/MethodConditionWithInheritDocFetcher.php @@ -10,60 +10,35 @@ namespace PhpDeal\Contract\Fetcher\ParentClass; -use Doctrine\Common\Annotations\Reader; use ReflectionClass; -use ReflectionMethod; -class MethodConditionWithInheritDocFetcher extends Fetcher +class MethodConditionWithInheritDocFetcher extends AbstractFetcher { /** + * Fetches conditions from all parent method prototypes recursively + * * @param ReflectionClass $class - * @param Reader $reader * @param string $methodName - * @param array $contracts + * * @return array */ - public function getConditions(ReflectionClass $class, Reader $reader, $methodName, array $contracts = []) + public function getConditions(ReflectionClass $class, $methodName) { - if ($this->hasInheritDoc($class->getMethod($methodName))) { - return $this->getConditionsWithInheritDoc($class, $reader, $methodName, $contracts); + $annotations = []; + $parentMethods = []; + while ( + preg_match('/\@inheritdoc/i', $class->getMethod($methodName)->getDocComment()) + && ($class = $class->getParentClass()) + && $class->hasMethod($methodName) + ) { + $parentMethods[] = $class->getMethod($methodName); } - return $contracts; - } - - /** - * @param ReflectionClass $class - * @param Reader $reader - * @param string $methodName - * @param array $contracts - * @return array - */ - private function getConditionsWithInheritDoc(ReflectionClass $class, Reader $reader, $methodName, array $contracts) - { - $parentClass = $class->getParentClass(); - if (!$parentClass) { - return $contracts; - } - - $parentMethod = $parentClass->getMethod($methodName); - $annotations = $reader->getMethodAnnotations($parentMethod); - $contractAnnotations = $this->getContractAnnotations($annotations); - $contracts = array_merge($contracts, $contractAnnotations); - - if ($this->hasInheritDoc($parentMethod)) { - return $this->getConditionsWithInheritDoc($parentClass, $reader, $methodName, $contracts); + foreach ($parentMethods as $parentMethod) { + $annotations = array_merge($annotations, $this->annotationReader->getMethodAnnotations($parentMethod)); } + $contracts = $this->filterContractAnnotation($annotations); return $contracts; } - - /** - * @param ReflectionMethod $method - * @return bool - */ - private function hasInheritDoc(ReflectionMethod $method) - { - return preg_match('/\@inheritdoc/i', $method->getDocComment()) > 0; - } }