goaop / framework Goto Github PK

Method AopChildFactory::injectJoinpoints() can receive list of advices as a second argument. If list of advices are present, we can easily wrap it with joinpoints and inject directly to the class. So, overhead for matching methods per class load will be smaller.

List of advices should be serialized and stored with the same class. This will give automatic invalidation of advices for debug mode and maximum performance for production mode.

There is a parse error in https://github.com/yiisoft/yii/blob/9bceff8643fef419b2933498d4ecfa3c6d0484b3/framework/base/CErrorHandler.php#L338 when using AOP.
Need to implement more reliable solution for parsing complex includes.

Composer now prepends itself to the list of autoloaders instead of appending, this breaks the logic of loading classes for internal classes.
Reference to the commit in composer: composer/composer@c80cb76

Need to decide, how to fix the logic of autoloading.

There is a conflict when both methods and properties are intercepted.
This is occurs when a proxy has new auto-generated methods (getter, setter and constructor or anything else) and getAdvicesForClass() uses proxy class reflection to extract all methods and generates code that relies on parent methods which can be absent in the general case.

I tried to use Go! + ZF2 and received fatal error:

Cannot redeclare Go\Instrument\ClassLoading\ensureLibraryAutoloaderIsFirst() (previously declared in /home/denis/web/TulaCoFramework/tulaco-php-framework/vendor/lisachenko/go-aop-php/src/Go/Instrument/ClassLoading/composer.php:20) in /home/denis/web/TulaCoFramework/tulaco-php-framework/data/cache/aspect/vendor/lisachenko/go-aop-php/src/Go/Instrument/ClassLoading/composer.php on line 40

Maybe I configured something wrong?

Pointcut parsed should be implemented as service with designators.

Privileged aspect has access to private or protected resources of other types. So code in advices of that aspect will be able to use private and protected fields of original class.

For example:

/**
 * Test class
 */
class Test {
    private $value = 123;
    public function concrete() {}
}

...

/**
 * @Privileged
 */
class Aspect {
    /**
     * @Before("execution Test->concrete(*)")
     */
    function beforeConcrete(MethodInvocation $invocation)
    {
        $obj = $invocation->getThis();
        echo $obj->value; // We can use private fields here because aspect is privileged
    }
}

Theoretically it's possible to use $this pointer directly for advices:

/**
 * @Privileged
 */
class Aspect {
    /**
     * @Before("execution Test->concrete(*)")
     */
    function beforeConcrete()
    {
        // $this will be in the scope of current object (Test), not the aspect scope!
        echo $this->value; // Just use private field of Test object
    }
}

Pointcut matcher should use one more check on inverted bitmask to prevent matching on static properties.
Notice relates to this bug:

User Notice: Trying to access undeclared property levels in 
    ../app/var/cache/aspect/vendor/monolog/src/Monolog/Logger.php line 427

Traits are a mechanism for code reuse in single inheritance languages such as PHP. A Trait is intended to reduce some limitations of single inheritance by enabling a developer to reuse sets of methods freely in several independent classes living in different class hierarchies

New code can contain traits, so library should be able to intercept methods in traits. This can be done by generating proxy trait for original trait and renaming proxy trait to the original trait name to intercept methods.

This is an unique feature and probably an unique pattern of design that allows to create proxy even for trait! 🤘

JMSExtraSecurity bundle verifies that secure annotation is copied to overridden method from the parent method annotation and @SatisfiesParentSecurityPolicy annotation is added to the child class.

Boolean pointcuts (&&, ||, !) don't check the class filter for nested pointcuts and this produce wrong behavior for matcher.

To build correct chain there is a need to use ordering for advices, this information should be defined with the help of annotations.

Some services can be shared in the container, this will reduce the time needed for initialization, for example, all aspects, pointcut parser, grammar, etc...

Subject should be implemented for flexible control over joinpoints. It will be awesome to add an annotation before the method and intercept it with AOP:

Method in the class:

    /**
     * Test cacheable by annotation
     *
     * @Cacheable
     * @param float $timeToSleep Amount of time to sleep
     *
     * @return string
     */
    public function cacheMe($timeToSleep)
    {
        usleep($timeToSleep * 1e6);
        return 'Yeah';
    }

Advice definition:

    /**
     * Cacheable methods
     *
     * @param MethodInvocation $invocation Invocation
     *
     * @Around("@annotation(Annotation\Cacheable)")
     */
    public function aroundCacheable(MethodInvocation $invocation)
    {
        static $memoryCache = array();

        $obj   = $invocation->getThis();
        $class = is_object($obj) ? get_class($obj) : $obj;
        $key   = $class . ':' . $invocation->getMethod()->name;
        if (!isset($memoryCache[$key])) {
            $memoryCache[$key] = $invocation->proceed();
        }
        return $memoryCache[$key];
    }

Source code of framework should be clear from notices and warnings, so this should be fixed asap.
Reference: CachingTransformer:80

Проблема по своему характеру и проявлению полностью соответсвует #63

how fast is your framework ?

Go uses special autoloader to avoid processing of itself. DebugClassLoader unregisters all loaders and wraps them with custom code.
This breaks an autoloading process of Go classes as they shouldn't be processed with source transformers.

There are cases when TokenReflection couldn't parse the source correctly (for example, NativeSessionHandler.php). In that cases FileProcessingException is thrown.
WeaverTransfomer should handle this exception and just skip that source. Unfortunately, it's impossible to generate a notice or a warning, because this can lead to an ErrorException in a modern frameworks that convert notices and warnings into exception in the development mode.

Currently interception of method with parameters passed by reference is not working, however it can be easily implemented since all arguments for invocation are passed in array.

All interceptors should implement Serializable interface. Key problem here is that Closure can not be serialized, so need to decide how this can be implemented.

With serialization support overhead for production mode can be lowered by getting the list of advices for the class from the cache.

Support for introduction advice should be available for PHP5.4 with the help of traits. Introduction will combine two parts: interfaces and traits into one declare-parents advice that can be applied to the specific classes.

Currently, pointcut can be referenced from another pointcut only by special 'pointcut' property. There is no way to combine a referenced pointcut with another pointcut.

This can be done in the PointcutParser, syntax for pointcut references should be the following: AspectClassName->pointcutMethodName

There is a class with protected property $tasks which is an array:

class TaskManager {

    protected $tasks = array();
}

There is also an advice which intercepts an access to this property:

    /**
     * Access interceptor
     * @param FieldAccess $property Joinpoint
     *
     * @Around("access(protected TaskManager->tasks)")
     * @return mixed
     */
    public function tasksChangeWatcher(FieldAccess $property) {
        static $tasksCount = 0;
        $value = $property->proceed();
        $tasksCnt = count($value);
        if ($tasksCount != $tasksCnt) {
            $tasksCount = $tasksCnt;
            Logger::getInstance()->addInfo("Number of tasks in the queue: $tasksCount");
        }
        return $value;
    }

During changing the value of this property in the application source code, php raises a warning:

Notice: Indirect modification of overloaded property TaskManager::$tasks has no effect in ...TaskManager.class.php on line 80

So if an advice applied to the private/protected property which is an array, then main source code cannot change it indirectly.

According to the official documentation http://www.php.net/manual/en/language.namespaces.rules.php there is a name resolution rule:

Inside namespace (say A\B), calls to unqualified functions are resolved at run-time. Here is how a call to function foo() is resolved:

It looks for a function from the current namespace: A\B\foo().
It tries to find and call the global function foo().

This rule can be used by Go! library to dynamically intercept global function calls in the namespaces. For example, if we want to intercept a function file_get_contents() in the application, we can create this function in the current namespace, so it will be used instead of global function.

There is a bug in the pointcut grammar, that restricts use of "negate" for complex pointcuts:

$this('Pointcut')
    ->is('Pointcut', '||', 'Pointcut')
    ->call(function($first, $_, $second) {
        return new OrPointcut($first, $second);
    })
// ...
    ->is('!', 'SinglePointcut')
    ->call(function($_, $first) {
        return new NotPointcut($first);
    })

    ->is('SinglePointcut');

Expression is('!', 'SinglePointcut') should be replaced with is('!', 'Pointcut'). Parse table should be also regenerated.

CachingTransformer doesn't delegate the transformation of source to the nested source transformers, so aspects are not working when cache directory is not set.

This can give more control over pointcuts, as advices can reference the same pointcut in the container.

To improve performance in development mode, need to use cache file, but this will break debugging with XDebug. This can be solved by using caching transformer that will be able to take transformed source from the cache. FilterInjector should be fixed too to trigger cache creation.

I'm trying to do a integration in a framework I'm bulding (Im using symfony HttpFoundation)

here is what I got so far

<?php

include __DIR__ . '/../vendor/lisachenko/go-aop-php/src/Go/Core/AspectKernel.php';

use Go\Core\AspectKernel;
use Go\Core\AspectContainer;

/**
 * Application Aspect Kernel
 */
class ApplicationAspectKernel extends AspectKernel
{

    /**
     * Configure an AspectContainer with advisors, aspects and pointcuts
     *
     * @param AspectContainer $container
     *
     * @return void
     */
    protected function configureAop(AspectContainer $container)
    {
        $container->registerAspect(new Nucleus\Cache\Caching());
    }
}

ApplicationAspectKernel::getInstance()->init(array(
    'appLoader' => __DIR__ . '/../vendor/autoload.php',
    // Configuration for autoload namespaces
    'autoloadPaths' => array(
        'Go' => __DIR__ . '/../vendor/lisachenko/go-aop-php/src/',
        'TokenReflection' => __DIR__ . '/../vendor/andrewsville/php-token-reflection/',
        'Doctrine\\Common' => __DIR__ . '/../vendor/doctrine/common/lib/',
        'Dissect' => __DIR__ . '/../vendor/jakubledl/dissect/src/'
    ),
    'includePaths' => array(
        __DIR__ . '/../vendor',
        __DIR__ . '/../src'
    )
));

$request = Symfony\Component\HttpFoundation\Request::createFromGlobals();

And a Aspect for testing

<?php 

namespace Nucleus\Cache;

use Go\Aop\Aspect;
use Go\Aop\Intercept\FieldAccess;
use Go\Aop\Intercept\MethodInvocation;
use Go\Lang\Annotation\After;
use Go\Lang\Annotation\Before;
use Go\Lang\Annotation\Around;
use Go\Lang\Annotation\Pointcut;

/**
 * Monitor aspect
 */
class Caching implements Aspect
{

    /**
     * Method that will be called before real method
     *
     * @param MethodInvocation $invocation Invocation
     * @Before("execution(public *->*(*))")
     */
    public function beforeMethodExecution(MethodInvocation $invocation)
    {
        $obj = $invocation->getThis();
        echo 'Calling Before Interceptor for method: ',
             is_object($obj) ? get_class($obj) : $obj,
             $invocation->getMethod()->isStatic() ? '::' : '->',
             $invocation->getMethod()->getName(),
             '()',
             ' with arguments: ',
             json_encode($invocation->getArguments()),
             "<br>\n";
    }
}

nothing is happening...

I check that the filter is properly registered using the stream_get_filters function;

array (size=13)
  0 => string 'convert.iconv.*' (length=15)
  1 => string 'string.rot13' (length=12)
  2 => string 'string.toupper' (length=14)
  3 => string 'string.tolower' (length=14)
  4 => string 'string.strip_tags' (length=17)
  5 => string 'convert.*' (length=9)
  6 => string 'consumed' (length=8)
  7 => string 'dechunk' (length=7)
  8 => string 'zlib.*' (length=6)
  9 => string 'bzip2.*' (length=7)
  10 => string 'mcrypt.*' (length=8)
  11 => string 'mdecrypt.*' (length=10)
  12 => string 'go.source.transforming.loader' (length=29)

And the path return by FilterInjectorTransformer::rewrite

php://filter/read=go.source.transforming.loader/resource=D:\nucleus\nucleus\web/../vendor/autoload.php

The filter function of SourceTransformingLoader is never call, probably whe it doesn't work...

Is there something I don't do correctly ? Or maybe a comflict with other filter and it never reach th filter that is registered ?

Need to refresh the cache if aspect was changed or configuration of kernel was updated.

For development mode this check should be made for each class on each request, for production mode we shouldn't use filemtime() function to determine freshness of cache.

Speed of regular expressions for source transformers can be increased by using studying flag for PCRE. This will result in up to 10 times more faster matching and replacing for source transformers.

Currently, proxy uses interface and trait name for class as is, without adding leading '' to always use FQCN. For classes in namespaces this won't work.

There is a scope error: Cannot access property ComposerAutoloaderInitXXX::$loader when trying to intercept static method that uses access to the private fields. This happens due to scope rebinding for LSB support, however, private fields will be in another (parent) scope, so there is an error.

Seems, it's better to use case-sensitive matching, because sometimes case-insensitive matching can give unpredictable result.
For example, *Name() pattern will match setName() and setSurname() methods.

hi,

Go! look nice, but i have not found the unit tests. Where are the unit tests ?
Framework is stable ?

Thx.

https://github.com/nikic/PHP-Parser has a real AST, so it can be really useful for building specific joinpoints in the source code.

Currently, only on-fly weaving is performing. This reduce overall performance as static reflection will take the place on each request. Additionally, op-code accelerators can not cache dynamic includes and just simply ignore that classes.

However, it's not easy to put transformed classes into another directory, as this will break an existing application, because DIR, FILE constants will be inside cache directory instead of original application one. ReflectionClass->getFileName() is also become incorrect.

Constants can be easily resolved during the source transforming process.

see https://scrutinizer-ci.com/g/lisachenko/go-aop-php/inspections/2a471369-5602-4f25-9add-d510129abab0?fixedFilesPerPage=10&fixedFilesPage=2#inspectioncomment-2126575

MagicConstantTransformer should replace only magic constants. It shouldn't replace __DIR__ and __FILE__ strings in another strings, phpDocs, etc..

Currently meta information for classes and methods are not preserved, need to copy it because many frameworks looks into annotations.

Go! requires "doctrine/common": "~2.2", but doctrine use branch aliasing "branch-alias": {"dev-master": "2.4.x-dev"}, so the last updates from master will be installed instead of stable version:

Loading composer repositories with package information
Updating dependencies
  - Installing doctrine/lexer (v1.0)
    Loading from cache

  - Installing doctrine/annotations (v1.0)
    Loading from cache

  - Installing doctrine/collections (v1.0)
    Loading from cache

  - Installing doctrine/cache (v1.0)
    Loading from cache

  - Installing doctrine/inflector (v1.0)
    Loading from cache

  - Installing doctrine/common (dev-master 185e6d7)
    Cloning 185e6d76765fe37ddb1a94aad8f51d97c92b0649

Additionally, doctrine introduces subpackages for 2.4.x. So autoloading for library is not working now, because composer can not be used for internal libraries of Go!

Composer supports providing an array of autoloading paths for a single namespace. In WeavingTransformer though, array_map is called for "realpath". That assumes all values are strings. It should be "array_map_recursive" type thing that at least goes one more array deep.

С письменным английским тяжело, посему отпишу по-русски.

При создании модульных тестов для приложения наткнулся на забавный момент:
в setUp() всё отлично инициализируется, но только в первой итерации тестов. Как только внутри PHPUnit вызывается setUp для следующего теста, ловим исключение о зарегистрированном потоке.
Очевидно, что в tearDown() поток не освобождается.

Трейс исключения: http://www.everfall.com/paste/id.php?ui2dua77pkop

И ещё 1 момент: в Go\Core\AspectKernel неплохо было бы добавить метод resetInstance всё для того же тестирования с помощью PHPUnit (привет синглтонам)