Merge pull request #24 from wick-ed/master

Advices can reference several pointcuts now using pointcut(<Name1>, <Name2>) annotation now
appserver-io · Mar 17, 2015 · 167072d · 167072d
2 parents 81abd31 + ee0c327
commit 167072d
Show file tree

Hide file tree

Showing 12 changed files with 340 additions and 90 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,13 @@
+# Version 1.3.0
+
+## Bugfixes
+
+* Multiple before advices have not been stacked correctly
+
+## Features
+
+* Advices can reference several pointcuts now using pointcut(<Name1>, <Name2>) annotation now
+
 # Version 1.2.0
 
 ## Bugfixes

diff --git a/bootstrap.php b/bootstrap.php
@@ -42,3 +42,7 @@
 
     unlink($cacheDir . DIRECTORY_SEPARATOR . $cachedFile);
 }
+
+// make the aspects known
+new \AppserverIo\Doppelgaenger\Tests\Data\Aspects\PointcutReferencingTestAspect();
+new \AppserverIo\Doppelgaenger\Tests\Data\Aspects\MainAspectTestClass();
diff --git a/build.default.properties b/build.default.properties
@@ -8,7 +8,7 @@
 #--------------------------------------------------------------------------------
 
 # ---- Module Release Settings --------------------------------------------------
-release.version = 1.2.0
+release.version = 1.3.0
 
 # ---- PHPCPD Settings ----------------------------------------------------------
 # Directories

diff --git a/src/AspectRegister.php b/src/AspectRegister.php
@@ -59,7 +59,7 @@ public function __construct()
         parent::__construct();
 
         $this->itemType = '\AppserverIo\Doppelgaenger\Entities\Definitions\Aspect';
-        $this->defaultOffset = 'name';
+        $this->defaultOffset = 'qualifiedName';
     }
 
     /**
@@ -71,6 +71,9 @@ public function __construct()
      */
     public function lookupAdvice($adviceExpression)
     {
+        // clean the expression
+        $adviceExpression = trim(ltrim(rtrim($adviceExpression, '()'), '\\'));
+
         // if there is an aspect name within the expression we have to filter our search range and cut the expression
         $container = $this->container;
         if (strpos($adviceExpression, '->')) {
@@ -110,6 +113,10 @@ public function lookupAspects($expression)
      */
     protected function lookupEntries($container, $expression)
     {
+
+        // clean the expression
+        $expression = trim(ltrim(rtrim($expression, '()'), '\\'));
+
         // if we got the complete name of the aspect we can return it alone
         if ($this->entryExists($expression)) {
             return array($this->get($expression));
@@ -118,7 +125,7 @@ protected function lookupEntries($container, $expression)
         // as it seems we got something else we have to get all regex about
         $matches = array();
         foreach ($container as $entry) {
-            if (fnmatch(ltrim(rtrim($expression, '()'), '\\'), $entry->getQualifiedName())) {
+            if (fnmatch($expression, $entry->getQualifiedName())) {
                 $matches[] = $entry;
             }
         }
@@ -135,6 +142,9 @@ protected function lookupEntries($container, $expression)
      */
     public function lookupPointcuts($pointcutExpression)
     {
+        // clean the expression
+        $pointcutExpression = trim(ltrim(rtrim($pointcutExpression, '()'), '\\'));
+
         // if there is an aspect name within the expression we have to filter our search range and cut the expression
         $container = $this->container;
         if (strpos($pointcutExpression, '->')) {
@@ -237,7 +247,15 @@ public function register(AspectDefinition $aspectDefinition)
                     $pointcut = $pointcutFactory->getInstance(array_pop($annotation->values));
                     if ($pointcut instanceof PointcutPointcut) {
                         // get the referenced pointcuts for the split parts of the expression
-                        $pointcut->setReferencedPointcuts($this->lookupPointcuts($pointcut->getExpression()));
+                        $expressionParts = explode(PointcutPointcut::EXPRESSION_CONNECTOR, $pointcut->getExpression());
+
+                        // lookup all the referenced pointcuts
+                        $referencedPointcuts = array();
+                        foreach ($expressionParts as $expressionPart) {
+                            $referencedPointcuts = array_merge($referencedPointcuts, $this->lookupPointcuts($expressionPart));
+                        }
+
+                        $pointcut->setReferencedPointcuts($referencedPointcuts);
                     }
 
                     $advice->getPointcuts()->add($pointcut);
@@ -247,6 +265,6 @@ public function register(AspectDefinition $aspectDefinition)
             }
         }
 
-        $this->set($aspect->getName(), $aspect);
+        $this->set($aspectDefinition->getQualifiedName(), $aspect);
     }
 }
diff --git a/src/Entities/Pointcuts/BlankPointcut.php b/src/Entities/Pointcuts/BlankPointcut.php
@@ -1,7 +1,7 @@
 <?php
 
 /**
- * \AppserverIo\Doppelgaenger\Entities\Pointcut\PointcutPointcut
+ * \AppserverIo\Doppelgaenger\Entities\Pointcut\BlankPointcut
  *
  * NOTICE OF LICENSE
  *
@@ -20,10 +20,8 @@
 
 namespace AppserverIo\Doppelgaenger\Entities\Pointcuts;
 
-use AppserverIo\Doppelgaenger\Entities\Definitions\FunctionDefinition;
-
 /**
- * Pointcut expression for specifying a collection of other pointcuts and annotations expressing them
+ * Dummy pointcut expression as a placeholder if an advice does not specify any explicitly e.g. @Around
  *
  * @author    Bernhard Wick <bw@appserver.io>
  * @copyright 2015 TechDivision GmbH - <info@appserver.io>
@@ -36,13 +34,6 @@
 class BlankPointcut extends AbstractPointcut
 {
 
-    /**
-     * Connector for referencing of several pointcuts at once
-     *
-     * @var string ADD_CONNECTOR
-     */
-    const ADD_CONNECTOR = '&&';
-
     /**
      * Whether or not the pointcut is considered static, meaning is has to be weaved and evaluated during runtime
      * anyway
@@ -88,16 +79,6 @@ public function getExecutionString($assignTo = null)
         return '';
     }
 
-    /**
-     * Getter for the $referencedPointcuts property
-     *
-     * @return array
-     */
-    public function getReferencedPointcuts()
-    {
-        return array();
-    }
-
     /**
      * Whether or not the pointcut matches a given candidate.
      * Weave pointcuts will always return true, as they do not pose any condition
@@ -110,16 +91,4 @@ public function matches($candidate)
     {
         return true;
     }
-
-    /**
-     * Setter for the $referencedPointcuts property
-     *
-     * @param array $referencedPointcuts Pointcuts referenced by this pointcut's expression
-     *
-     * @return null
-     */
-    public function setReferencedPointcuts(array $referencedPointcuts)
-    {
-        $this->referencedPointcuts = $referencedPointcuts;
-    }
 }
diff --git a/src/Entities/Pointcuts/PointcutFactory.php b/src/Entities/Pointcuts/PointcutFactory.php
@@ -143,12 +143,7 @@ public function getInstance($expression)
 
             // if we are already in a wrapping connector pointcut then we will cut it off as those are not distinguished
             // by type but rather by their connector
-            if (strpos($expression, AndPointcut::TYPE) === 0) {
-                $expression = str_replace(AndPointcut::TYPE, '', $expression);
-
-            } elseif (strpos($expression, OrPointcut::TYPE) === 0) {
-                $expression = str_replace(OrPointcut::TYPE, '', $expression);
-            }
+            $expression = $this->trimConnectorTypes($expression);
 
             // now lets have a look if we are wrapped in some outer brackets
             if (strlen($expression) === $this->getBracketSpan($expression)) {
@@ -164,7 +159,7 @@ public function getInstance($expression)
                 }
             }
 
-            // or-connection comes secondly
+            // or-connection comes second
             if (strpos($expression, OrPointcut::CONNECTOR) !== false) {
                 $class = '\AppserverIo\Doppelgaenger\Entities\Pointcuts\OrPointcut';
                 $tmp = $this->findConnectorPointcut($expression, $class);
@@ -196,6 +191,24 @@ public function getInstance($expression)
         $pointcut = new $class(substr(trim(str_replace($type, '', $expression), '( '), 0, -1), $isNegated);
 
         return $pointcut;
+    }
+
+    /**
+     * Will cut of any connector pointcut type as they are distinguished by connector rather than type
+     *
+     * @param string $expression The pointcut expression to trim
+     *
+     * @return string
+     */
+    protected function trimConnectorTypes($expression)
+    {
+        if (strpos($expression, AndPointcut::TYPE) === 0) {
+            $expression = str_replace(AndPointcut::TYPE, '', $expression);
+
+        } elseif (strpos($expression, OrPointcut::TYPE) === 0) {
+            $expression = str_replace(OrPointcut::TYPE, '', $expression);
+        }
 
+        return $expression;
     }
 }
diff --git a/src/Entities/Pointcuts/PointcutPointcut.php b/src/Entities/Pointcuts/PointcutPointcut.php
@@ -39,9 +39,9 @@ class PointcutPointcut extends AbstractPointcut
     /**
      * Connector for referencing of several pointcuts at once
      *
-     * @var string ADD_CONNECTOR
+     * @var string EXPRESSION_CONNECTOR
      */
-    const ADD_CONNECTOR = '&&';
+    const EXPRESSION_CONNECTOR = ',';
 
     /**
      * Whether or not the pointcut is considered static, meaning is has to be weaved and evaluated during runtime
@@ -61,7 +61,7 @@ class PointcutPointcut extends AbstractPointcut
     /**
      * Pointcuts referenced by this pointcut's expression
      *
-     * @var array $referencedPointcuts
+     * @var \AppserverIo\Doppelgaenger\Interfaces\PointcutInterface[] $referencedPointcuts
      */
     protected $referencedPointcuts;
 
@@ -126,7 +126,7 @@ public function matches($candidate)
     /**
      * Setter for the $referencedPointcuts property
      *
-     * @param array $referencedPointcuts Pointcuts referenced by this pointcut's expression
+     * @param \AppserverIo\Doppelgaenger\Interfaces\PointcutInterface[] $referencedPointcuts Pointcuts referenced by this pointcut's expression
      *
      * @return null
      */

diff --git a/src/StreamFilters/AdviceFilter.php b/src/StreamFilters/AdviceFilter.php
@@ -173,35 +173,37 @@ protected function injectAdviceCode(& $bucketData, array $sortedPointcutExpressi
                     $bucketData
                 );
 
-            } elseif ($joinpoint === Before::ANNOTATION) {
-                // before advices have to be woven differently as we have to take changes of the call parameters into account
-
-                // we have to build up an assignment of the potentially altered parameters in our method invocation object
-                // to the original parameters of the method call.
-                // This way we can avoid e.g. broken references by func_get_args and other problems
-                $parameterNames = array();
-                foreach ($functionDefinition->getParameterDefinitions() as $parameterDefinition) {
-                    $parameterNames[] = $parameterDefinition->name;
-                }
-                $parameterAssignmentCode = 'list(' .
-                    implode(',', $parameterNames) .
-                    ') = array_values(' . ReservedKeywords::METHOD_INVOCATION_OBJECT . '->getParameters());';
-
-                $pointcutExpression = $pointcutExpressions[0];
-                $bucketData = str_replace(
-                    $placeholderHook,
-                    $placeholderHook . $pointcutExpression->toCode() . $parameterAssignmentCode,
-                    $bucketData
-                );
-
             } else {
                 // iterate all the others and inject the code
                 foreach ($pointcutExpressions as $pointcutExpression) {
-                    $bucketData = str_replace(
-                        $placeholderHook,
-                        $placeholderHook . $pointcutExpression->toCode(),
-                        $bucketData
-                    );
+                    if ($joinpoint === Before::ANNOTATION) {
+                        // before advices have to be woven differently as we have to take changes of the call parameters into account
+
+                        // we have to build up an assignment of the potentially altered parameters in our method invocation object
+                        // to the original parameters of the method call.
+                        // This way we can avoid e.g. broken references by func_get_args and other problems
+                        $parameterNames = array();
+                        foreach ($functionDefinition->getParameterDefinitions() as $parameterDefinition) {
+                            $parameterNames[] = $parameterDefinition->name;
+                        }
+                        $parameterAssignmentCode = 'list(' .
+                            implode(',', $parameterNames) .
+                            ') = array_values(' . ReservedKeywords::METHOD_INVOCATION_OBJECT . '->getParameters());';
+
+                        $bucketData = str_replace(
+                            $placeholderHook,
+                            $placeholderHook . $pointcutExpression->toCode() . $parameterAssignmentCode,
+                            $bucketData
+                        );
+
+                    } else {
+                        // all join-point NOT Before or Around can be woven very simply
+                        $bucketData = str_replace(
+                            $placeholderHook,
+                            $placeholderHook . $pointcutExpression->toCode(),
+                            $bucketData
+                        );
+                    }
                 }
             }
         }

diff --git a/tests/Tests/Data/Advised/PointcutReferencingTestClass.php b/tests/Tests/Data/Advised/PointcutReferencingTestClass.php
@@ -139,4 +139,62 @@ public function iHaveAnAroundAdvice()
     {
         return 'iHaveAnAroundAdvice';
     }
+
+    /**
+     * Used to test pointcut based weaving of several "Around" advices
+     *
+     * @return integer
+     */
+    public function iHaveTwoAroundAdvicesIncrementingMyResult()
+    {
+        return 1;
+    }
+
+    /**
+     * Used to test pointcut based weaving of several "Before" advices
+     *
+     * @param integer $param An integer param
+     *
+     * @return integer
+     */
+    public function iHaveTwoBeforeAdvices($param)
+    {
+        return $param;
+    }
+
+    /**
+     * Used to test pointcut based weaving of several "Before" advices
+     *
+     * @param integer $param An integer param
+     *
+     * @return integer
+     */
+    public function iHaveTwoBeforeAdvicesOfTheSameAspect($param)
+    {
+        return $param;
+    }
+
+    /**
+     * Used to test pointcut based weaving of one "Before" advice
+     *
+     * @param integer $param An integer param
+     *
+     * @return integer
+     */
+    public function iHaveASimpleBeforeAdvice1($param)
+    {
+        return $param;
+    }
+
+    /**
+     * Used to test pointcut based weaving of one "Before" advice
+     *
+     * @param integer $param An integer param
+     *
+     * @return integer
+     */
+    public function iHaveASimpleBeforeAdvice2($param)
+    {
+        return $param;
+    }
 }