goaop / goaop-symfony-bundle

[Outdated!] Integration bridge for Go! AOP framework and Symfony
55 stars 16 forks source link

[Known Limitation] Compatibility with Symfony framework >= 3.4 #22

Open jijzult opened 6 years ago

jijzult commented 6 years ago

Am running symfony 4.0.5, and trying to get go-aop working. Although I see that my test aspect and advisor are loaded, tried several ways to fire up through @Before, @Around, the advise never fires up...

$ composer create-project symfony/skeleton:^4.0 sf4
$ cd sf4
$ composer require roave/security-advisories
$ composer require goaop/goaop-symfony-bundle

# moved GoAppBundle to the top in config/bundles.php
$ composer require logger

FILE CONTENTS src/Command/ShineCommand.php:

<?php
namespace App\Command;

use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class ShineCommand extends Command
{
    private $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger   = $logger;
        parent::__construct();      // you *must* call the parent constructor
    }

    protected function configure()
    {
        $this->setName('app:shine');
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $this->logger->info( $string = 'Waking up the sun' );
        // ...
    }
}

FILE CONTENTS Aspect/LoggingAspect.php:

<?php
namespace App\Aspect;

use Go\Aop\Aspect;
use Go\Aop\Intercept\MethodInvocation;
use Go\Lang\Annotation\Before;
use Psr\Log\LoggerInterface;

/**
 * Application logging aspect
 */
class LoggingAspect implements Aspect
{
    /**
     * @var LoggerInterface
     */
    private $logger;

    public function __construct(LoggerInterface $logger)
    {
print "ASPECT INIT\n";
        $this->logger = $logger;
    }

    /**
     * Writes a log info before method execution
     *
     * @param MethodInvocation $invocation
     * @Before("execution(public **->*(*))")
     */
    public function beforeMethod(MethodInvocation $invocation)
    {
print "ASPECT FIRED in ".get_class($this)."\n";
        $this->logger->info($invocation, $invocation->getArguments());
    }
}

FILE CONTENTS (Appended) config/services.yaml:

parameters:
    container.dumper.inline_class_loader: false

services:
    logging.aspect:
        class:      App\Aspect\LoggingAspect
        public:     true
        arguments:  ["@logger"]
        tags:
            - { name: goaop.aspect }

go_aop:
    options:
        debug:      true
        app_dir:    "%kernel.root_dir%/../src"
        cache_dir:  "%kernel.cache_dir%/aspect"
        features:                                   # framework/src/Aop/Features.php
        - INTERCEPT_FUNCTIONS
        - INTERCEPT_INITIALIZATIONS
        - INTERCEPT_INCLUDES

\$ console debug:container goaop

 Select one of the following services to display its information:
  [0 ] goaop.aspect.kernel
  [1 ] goaop.aspect.container
  [2 ] goaop.cache.path.manager
  [3 ] goaop.cache.warmer
  [4 ] goaop.bridge.doctrine.metadata_load_interceptor
  [5 ] goaop.command.warmup
  [6 ] goaop.command.debug_advisor
  [7 ] goaop.command.debug_aspect
  [8 ] console.command.public_alias.goaop.command.warmup
  [9 ] console.command.public_alias.goaop.command.debug_advisor
  [10] console.command.public_alias.goaop.command.debug_aspect

\$ bin/console debug:aspect

Aspect debug information
========================

 Go\Symfony\GoAopBundle\Kernel\AspectSymfonyKernel has following enabled aspects:

App\Aspect\LoggingAspect
------------------------

Defined in: /home/holzmann/shared/pkg/sf4/src/Aspect/LoggingAspect.php

 Application logging aspect

Pointcuts and advices
 --------- ---------------------------------------- 
  Type      Identifier                              
 --------- ---------------------------------------- 
  Advisor   App\Aspect\LoggingAspect->beforeMethod  
 --------- ---------------------------------------- 

\$ bin/console debug:advisor

Advisor debug information
=========================

List of registered advisors in the container
 ---------------------------------------- ---------------------------- 
  Id                                       Expression                  
 ---------------------------------------- ---------------------------- 
  App\Aspect\LoggingAspect->beforeMethod   execution(public **->*(*))  
 ---------------------------------------- ---------------------------- 

\$ console debug:container goaop.aspect.container

Information for Service "goaop.aspect.container"
  Option            Value                           
  Service ID        goaop.aspect.container          
  Class             Go\Core\GoAspectContainer       
  Tags              -                               
  Calls             registerAspect, registerAspect  
  Public            yes                             
  Synthetic         no                              
  Lazy              no                              
  Shared            yes                             
  Abstract          no                              
  Autowired         no                              
  Autoconfigured    no                              
  Factory Service   goaop.aspect.kernel             
  Factory Method    getContainer                    

$ console debug:container goaop.aspect.kernel

Information for Service "goaop.aspect.kernel"
  Option           Value                                              
  Service ID       goaop.aspect.kernel                                
  Class            Go\Symfony\GoAopBundle\Kernel\AspectSymfonyKernel  
  Tags             -                                                  
  Calls            init                                               
  Public           yes                                                
  Synthetic        no                                                 
  Lazy             no                                                 
  Shared           yes                                                
  Abstract         no                                                 
  Autowired        no                                                 
  Autoconfigured   no                                                 
  Factory Class    Go\Symfony\GoAopBundle\Kernel\AspectSymfonyKernel  
  Factory Method   getInstance                                        

\$ console app:shine -vv [2018-02-16 14:19:48] app.INFO: Waking up the sun [] []

lisachenko commented 6 years ago

Hi, sorry for the delay, wasn't available for OSS. From what I can see, all configuration is correct, will try to reproduce this case locally to see what's wrong with your example.

lisachenko commented 6 years ago

Ok, I can confirm that it will not work anymore. Symfony now uses it's own class loading/requiring mechanism, so AOP engine can't load transformed class from a cache.

Here is an example of how command service is defined in container right now:

<?php

use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;

// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
// Returns the public 'console.command.public_alias.App\Command\ShineCommand' shared autowired service.

include_once $this->targetDirs[3].'\\vendor\\symfony\\console\\Command\\Command.php';
include_once $this->targetDirs[3].'\\src\\Command\\ShineCommand.php';

return $this->services['console.command.public_alias.App\Command\ShineCommand'] = new \App\Command\ShineCommand(($this->privates['logger'] ?? $this->privates['logger'] = new \Symfony\Component\HttpKernel\Log\Logger()));

@nicolas-grekas suggested to turn off container.dumper.inline_class_loader parameter to the false in your config:

# ./config/services.yaml
parameters:
    container.dumper.inline_class_loader: false

After that everything should work.

nicolas-grekas commented 6 years ago

you should put that in a Flex recipe!

lisachenko commented 6 years ago

@nicolas-grekas interesting, how could I do this, please show me an example of recipe )

jijzult commented 6 years ago

No problem, already very happy you respond :)Thanks!

I just installed your seminar 2017 example, without symfony, so direct under apache.. example works (at least the first logger)..

I ported those pieces to symfony (so only framework, not the bridge.. just to see that part working first).. get the same symptoms as full (framework+symfony bridge) install.. so it initializes the aspects, but doesn't fire them..

Interesting comparison: the cache dir of your seminar example shows more than the symfony cache dir.. example cache: BusinessService.php _annotations/ _aspect/ _proxies/

The symfony cache/aspect is empty

Thanks for all the effort !

jijzult commented 6 years ago

Ha! I see lot of actionwhilst I was writing my reaction.. i'll test it as soon I get back.. thanks for the effort again

nicolas-grekas commented 6 years ago

@lisachenko see eg https://github.com/symfony/recipes-contrib/pull/296/files and https://github.com/symfony/recipes/blob/master/README.rst

angelov commented 6 years ago

@lisachenko I think i'll have some time this weekend so I may try to make the recipe

jijzult commented 6 years ago

Aspect gets fired, with

@Before("execution(public App\Command\ShineCommand->*(*))")

good :)

lisachenko commented 6 years ago

@jijzult yes, I can confirm that it's working after disabling class loader inlining.

But be aware, that by default for SF4 default logging changed significantly, this means that only warnings, errors and criticals are logged into file. You are using info level in your aspect and command, so, just add -vvv when running your console command.

@nicolas-grekas I think that it isn't good idea to always look at verbosity level, implemented in https://github.com/symfony/symfony/pull/24425. For prod environment it's definitely cool, but for the debug mode in CLI this should be always debug level. Otherwise developers should spend some time by looking at logs and discover that info level during development is not logging anymore.

lisachenko commented 6 years ago

Looks like I found next issue: with autowiring SF4 scans all resources for changes, thus aspect works only once. When cache is generated on second call, Kernel checks if the cache is fresh during Kernel->initializeContainer() call. At this moment bundles aren't booted yet, even GoAopBundle which is first. So composer loader isn't hooked, thus original class is loaded instead of transformed one. Look at ReflectionClassResource line 100, where class loading is triggered before container is ready. This is our trouble, because classes loaded via traditional class. Need to find a way, how to hook into Symfony initialization process and hook Composer's autoloader before initailizeContainer() method call.

jijzult commented 6 years ago

I had changed '*' to 'execute' to single out 1 Command method...

@Before("execution(public App\Command\ShineCommand->execute(*))")

which does not fire... but this might be related to your last/previous remark

With a new class outside the sf Command, eg. class Test with method execute(), that works fine!

ASPECT FIRED in App\Aspect\LoggingAspect: $invocation->proceed() shall fire ($invocation class + name):App\Test__AopProxied->execute
srosato commented 5 years ago

Is there a lot required to make it work with sf4? How can we help?

ARoddis commented 5 years ago

The project is obviously dead...1,5 years no Symfony 4 support...no Response...even to people offering to help...sad.

lisachenko commented 5 years ago

Hello everyone! I can confirm that Symfony3.4 have changed a lot in the mechanism of class loading, thus incompatible since that with SF3.4 and 4.0. Only next version of Go! AOP framework can help me to make it working again even for Symfony.

lisachenko commented 5 years ago

I've pinned this issue and add explicit notification in the README file

sidux commented 4 years ago

Hi @lisachenko i see that the new version of GoAop is in RC and i want to know which change in this version can fix this bundle compatibility. thx

ralusnom commented 4 years ago

Also for me it would be quite interesting if we could bring this package back to (working) life. Are there any news in this regard or can we provide support? @lisachenko, I suppose you do not have the time to contribute at the moment, right? :)