goaop / goaop-symfony-bundle

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

Detection of circular references which breaks weaving #15

Open TheCelavi opened 7 years ago

TheCelavi commented 7 years ago

I have spent 2 days trying to figure out an issue of Go Aop not being able to weave around some classes in project. It happened that I had a circular reference issue that involved aspects, so that broke everything, and issue was not so obvious as it is with, per example, Symfony service container.

We had a external service providers (4 of them) with OAuth protected external api. In order to handle them, for each, a Provider class is created, and each Provider is injected into Manager.

Each provider could expire a token. So, we have RefreshTokenAspect, which will, in general, try to re-issue a token on some method call if that method fails (per example, Provider->fetchData()).

This weaving was possible.

However, there was a third Entity, which has method getData(). For that method, aspect is used on which Manager is injected to lazy-load data from some of the Providers.

And that failed Aspect Container without any obvious error.

As I have figure out, first, a Symfony service container is created. Then AOP kernel, then aspects, and then other services. However, if aspect requires some service, then that service will be initialized prior to aspect. And if that service is weaved, or its some sub-dependency, then you get a circular reference.

This complex situation can be simply explained as a rule: "There can not be an aspect defined which uses service that is weaved, or depends on any other service that is weaved".

It would be very convenient to build some kind of introspection that would detect this situation and warn about circular reference.

Possible workarounds:

TheCelavi commented 7 years ago

Lazy loaded services FTW.

TheCelavi commented 7 years ago

This looks promising: https://github.com/symfony/symfony/issues/20656#issuecomment-314110173

If implemented, it will ease up management with circular references.

Only thing which should be extremely useful is to track down dependencies and throw exception when circular dependency is detected.

lisachenko commented 7 years ago

Only thing which should be extremely useful is to track down dependencies and throw exception when circular dependency is detected.

Yes, indeed, this will be nice guard.