Rewrote property interceptors to be by reference and use trait for ge…

…neral logic, resolves #54, #232

demos/Demo/Aspect/PropertyInterceptorAspect.php

@@ -32,10 +32,18 @@ class PropertyInterceptorAspect implements Aspect
      */
     public function aroundFieldAccess(FieldAccess $fieldAccess)
     {
-        $value = $fieldAccess->proceed();
-        echo "Calling Around Interceptor for ", $fieldAccess, ", value: ", json_encode($value), PHP_EOL;
+        $isRead = $fieldAccess->getAccessType() == FieldAccess::READ;
+        // proceed all internal advices
+        $fieldAccess->proceed();
 
-        // $value = 666; You can change the return value for read/write operations in advice!
-        return $value;
+        if ($isRead) {
+            // if you want to change original property value, then return it by reference
+            $value = /* & */$fieldAccess->getValue();
+        } else {
+            // if you want to change value to set, then return it by reference
+            $value = /* & */$fieldAccess->getValueToSet();
+        }
+
+        echo "Calling After Interceptor for ", $fieldAccess, ", value: ", json_encode($value), PHP_EOL;
     }
 }

demos/Demo/Example/PropertyDemo.php

@@ -19,6 +19,8 @@ class PropertyDemo
 
     protected $protectedProperty = 456;
 
+    protected $indirectModificationCheck = [4, 5, 6];
+
     public function showProtected()
     {
         echo $this->protectedProperty, PHP_EOL;
@@ -28,4 +30,12 @@ public function setProtected($newValue)
     {
         $this->protectedProperty = $newValue;
     }
+
+    public function __construct()
+    {
+        array_push($this->indirectModificationCheck, 7, 8, 9);
+        if (count($this->indirectModificationCheck) !== 6) {
+            throw new \RuntimeException("Indirect modification doesn't work!");
+        }
+    }
 }

src/Aop/Framework/ClassFieldAccess.php

@@ -10,6 +10,7 @@
 
 namespace Go\Aop\Framework;
 
+use Go\Aop\AspectException;
 use Go\Aop\Intercept\FieldAccess;
 use Go\Aop\Intercept\Interceptor;
 use ReflectionProperty;
@@ -93,14 +94,54 @@ public function getField()
         return $this->reflectionProperty;
     }
 
+    /**
+     * Gets the current value of property
+     *
+     * @return mixed
+     */
+    public function &getValue()
+    {
+        $value = &$this->value;
+
+        return $value;
+    }
+
     /**
      * Gets the value that must be set to the field.
      *
      * @return mixed
      */
-    public function getValueToSet()
+    public function &getValueToSet()
     {
-        return $this->newValue;
+        $newValue = &$this->newValue;
+
+        return $newValue;
+    }
+
+    /**
+     * Checks scope rules for accessing property
+     *
+     * @param int $stackLevel Stack level for check
+     *
+     * @return true if access is OK
+     */
+    public function ensureScopeRule($stackLevel = 2)
+    {
+        $property = $this->reflectionProperty;
+
+        if ($property->isProtected()) {
+            $backTrace     = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, $stackLevel+1);
+            $accessor      = isset($backTrace[$stackLevel]) ? $backTrace[$stackLevel] : [];
+            $propertyClass = $property->class;
+            if (isset($accessor['class'])) {
+                if ($accessor['class'] === $propertyClass || is_subclass_of($accessor['class'], $propertyClass)) {
+                    return true;
+                }
+            }
+            throw new AspectException("Cannot access protected property {$propertyClass}::{$property->name}");
+        }
+
+        return true;
     }
 
     /**
@@ -109,22 +150,16 @@ public function getValueToSet()
      * Typically this method is called inside previous closure, as instance of Joinpoint is passed to callback
      * Do not call this method directly, only inside callback closures.
      *
-     * @return mixed
+     * @return void For field interceptor there is no return values
      */
     final public function proceed()
     {
         if (isset($this->advices[$this->current])) {
             /** @var $currentInterceptor Interceptor */
             $currentInterceptor = $this->advices[$this->current++];
 
-            return $currentInterceptor->invoke($this);
-        }
-
-        if ($this->accessType === self::WRITE) {
-            return $this->getValueToSet();
+            $currentInterceptor->invoke($this);
         }
-
-        return $this->value;
     }
 
     /**
@@ -137,28 +172,34 @@ final public function proceed()
      *
      * @return mixed
      */
-    final public function __invoke($instance, $accessType, $originalValue, $newValue = NAN)
+    final public function &__invoke($instance, $accessType, &$originalValue, $newValue = NAN)
     {
         if ($this->level) {
-            array_push($this->stackFrames, array($this->instance, $this->accessType, $this->value, $this->newValue));
+            array_push($this->stackFrames, [$this->instance, $this->accessType, &$this->value, &$this->newValue]);
         }
 
         ++$this->level;
 
         $this->current    = 0;
         $this->instance   = $instance;
         $this->accessType = $accessType;
-        $this->value      = $originalValue;
+        $this->value      = &$originalValue;
         $this->newValue   = $newValue;
 
-        $result = $this->proceed();
+        $this->proceed();
 
         --$this->level;
 
         if ($this->level) {
             list($this->instance, $this->accessType, $this->value, $this->newValue) = array_pop($this->stackFrames);
         }
 
+        if ($accessType == self::READ) {
+            $result = &$this->value;
+        } else {
+            $result = &$this->newValue;
+        };
+
         return $result;
 
     }

src/Aop/Intercept/FieldAccess.php

@@ -41,14 +41,23 @@ interface FieldAccess extends Joinpoint
      */
     public function getField();
 
+    /**
+     * Gets the current value of property by reference
+     *
+     * @api
+     *
+     * @return mixed
+     */
+    public function &getValue();
+
     /**
      * Gets the value that must be set to the field, applicable only for WRITE access type
      *
      * @api
      *
      * @return mixed
      */
-    public function getValueToSet();
+    public function &getValueToSet();
 
     /**
      * Returns the access type.

src/Proxy/ClassProxy.php

@@ -495,15 +495,12 @@ public function __toString()
      */
     protected function addFieldInterceptorsCode(ParsedMethod $constructor = null)
     {
-        $byReference = false;
-        $this->setProperty(Property::IS_PRIVATE, '__properties', 'array()');
-        $this->setMethod(Method::IS_PUBLIC, '__get', $byReference, $this->getMagicGetterBody(), '$name');
-        $this->setMethod(Method::IS_PUBLIC, '__set', $byReference, $this->getMagicSetterBody(), '$name, $value');
+        $this->addTrait(PropertyInterceptionTrait::class);
         $this->isFieldsIntercepted = true;
         if ($constructor) {
             $this->override('__construct', $this->getConstructorBody($constructor, true));
         } else {
-            $this->setMethod(Method::IS_PUBLIC, '__construct', $byReference, $this->getConstructorBody(), '');
+            $this->setMethod(Method::IS_PUBLIC, '__construct', false, $this->getConstructorBody(), '');
         }
     }
 
@@ -534,53 +531,6 @@ protected function getOverriddenMethod(ParsedMethod $method, $body)
         return $code;
     }
 
-    /**
-     * Returns a code for magic getter to perform interception
-     *
-     * @return string
-     */
-    private function getMagicGetterBody()
-    {
-        return <<<'GETTER'
-if (\array_key_exists($name, $this->__properties)) {
-    return self::$__joinPoints["prop:$name"]->__invoke(
-        $this,
-        \Go\Aop\Intercept\FieldAccess::READ,
-        $this->__properties[$name]
-    );
-} elseif (\method_exists(\get_parent_class(), __FUNCTION__)) {
-    return parent::__get($name);
-} else {
-    trigger_error("Trying to access undeclared property {$name}");
-
-    return null;
-}
-GETTER;
-    }
-
-    /**
-     * Returns a code for magic setter to perform interception
-     *
-     * @return string
-     */
-    private function getMagicSetterBody()
-    {
-        return <<<'SETTER'
-if (\array_key_exists($name, $this->__properties)) {
-    $this->__properties[$name] = self::$__joinPoints["prop:$name"]->__invoke(
-        $this,
-        \Go\Aop\Intercept\FieldAccess::WRITE,
-        $this->__properties[$name],
-        $value
-    );
-} elseif (\method_exists(\get_parent_class(), __FUNCTION__)) {
-    parent::__set($name, $value);
-} else {
-    $this->$name = $value;
-}
-SETTER;
-    }
-
     /**
      * Returns constructor code
      *
@@ -594,7 +544,7 @@ private function getConstructorBody(ParsedMethod $constructor = null, $isCallPar
         $assocProperties = [];
         $listProperties  = [];
         foreach ($this->interceptedProperties as $propertyName) {
-            $assocProperties[] = "'$propertyName' => \$this->$propertyName";
+            $assocProperties[] = "'$propertyName' => &\$this->$propertyName";
             $listProperties[]  = "\$this->$propertyName";
         }
         $assocProperties = $this->indent(join(',' . PHP_EOL, $assocProperties));

src/Proxy/PropertyInterceptionTrait.php

@@ -0,0 +1,88 @@
+<?php
+/*
+ * Go! AOP framework
+ *
+ * @copyright Copyright 2016, Lisachenko Alexander <[email protected]>
+ *
+ * This source file is subject to the license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Go\Proxy;
+
+use Go\Aop\Framework\ClassFieldAccess;
+use Go\Aop\Intercept\FieldAccess;
+
+/**
+ * Trait that holds all general logic for working with intercepted properties
+ */
+trait PropertyInterceptionTrait
+{
+    /**
+     * Holds a collection of current values for intercepted properties
+     *
+     * @var array
+     */
+    private $__properties = [];
+
+    /**
+     * @inheritDoc
+     */
+    public function &__get($name)
+    {
+        if (\array_key_exists($name, $this->__properties)) {
+            /** @var ClassFieldAccess $fieldAccess */
+            $fieldAccess = self::$__joinPoints["prop:$name"];
+            $fieldAccess->ensureScopeRule();
+
+            $value = &$fieldAccess->__invoke($this,FieldAccess::READ, $this->__properties[$name]);
+        } elseif (\method_exists(\get_parent_class(), __FUNCTION__)) {
+            $value = parent::__get($name);
+        } else {
+            trigger_error("Trying to access undeclared property {$name}");
+
+            $value = null;
+        }
+
+        return $value;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function __set($name, $value)
+    {
+        if (\array_key_exists($name, $this->__properties)) {
+            /** @var ClassFieldAccess $fieldAccess */
+            $fieldAccess = self::$__joinPoints["prop:$name"];
+            $fieldAccess->ensureScopeRule();
+
+            $this->__properties[$name] = $fieldAccess->__invoke(
+                $this,
+                FieldAccess::WRITE,
+                $this->__properties[$name],
+                $value
+            );
+        } elseif (\method_exists(\get_parent_class(), __FUNCTION__)) {
+            parent::__set($name, $value);
+        } else {
+            $this->$name = $value;
+        }
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function __isset($name)
+    {
+        return isset($this->__properties[$name]);
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function __unset($name)
+    {
+        $this->__properties[$name] = null;
+    }
+}