Re: Fwd: AOP extension for PHP

2012/6/20 Laupretre François <francois.laupretre@francetv.fr> > Hi, > Hi > > I just read your document about the AOP extension you are proposing. I > didn't know much to AOP before, but I like this approach. Among others, I > especially love the idea of reusing an existing library and modify its > behavior without touching its code. > Always good to know the documentation was enought to understand the purpose of the extension. > Here are some questions/comments that come to my mind: > > - in the 2.3.3 chapter, shouldn't you call $object->process() at > the end of adviceUpdatingArguments() ? If not, I don't see how the original > doStuff() code is executed. The same in 2.3.4 : should call process() > before calling getReturnedValue(). > Right, the documentation was outdated, fixed and uploaded the new one. > > - Chapter 2.4.4: I think that getReturnedValue() is also populated > in an 'around' advice after 'process()' was called. > Right, have to fix it in the documentation. > > - What happens if process() is called more than once in a single > advice (or in different advices) ? Is it authorized (as a way to execute > the function several times, possibly with different arguments) or forbidden > (protected by an error/exception, or just ignored) ? > It is authorized, it will call the method several times. > - Instead of executing the advices in the registration order, could > they be registered with an additional flag, allowing them to be registered > in first position in the current list ? > We discussed this but decided it won't be in our first release. > - I don't imagine how several 'around' advices are executed on a > joinpoint, as each one will probably call process() or completely replace > the original behavior. Wouldn't it make sense to restrict 'around' advices > to only one per joinpoint ? > They will be chained. The first advice will call process.... and that process will trigger the second advice... and so on. I think it have to be possible, even if* *I'd like to add an option on add_around to tell AOP to refuse others to add another advice on the same joinpoint (Julien does not agree and I can understand why). Now remain the tricky multiple call of process in the same advice. Not sure if Julien did handle this. Have to add this to the tests. > - As 'before' and 'after' advices just provide part of > 'around' > advice features, are they provided just as more readable shortcuts, easier > to implement than 'around' advices ? Of course, the question exists only if > we enable several 'around' advices per joinpoint. > It's just a quick and convenient way to write simple advice that does not have to deal with the execution of the triggered joinpoint. > > - can you provide some information on other joinpoint types, > especially exception. Will it be split in before/after/around ? Can the > advice access and/or change the exception's properties ? What if the advice > throws another exception ? does it replace the original exception (if > called after) ? does it disable throwing the original exception (if called > before) ? > We wanted exceptions to be part of the first release but we changed our mind as we are not completely decided on how it should work. I have to remove that part from the documentation. > - Would it be possible to consider other join points, such as > reading/writing to variables or class/object properties ? This would > provide an elegant way of implementing 'active' properties, or to implement > a finer-grained protection around properties (like a simpler way to define > public read-only properties). > This will be added very soon, some doctrine contributors already asked for the feature (https://github.com/Juliens/AOP/issues/3) > - Can an advice disable the execution of subsequent advices (sort > of 'break') ? > I did not want to include this feature as I think this will add extra (too much) complexity. If the community really want this to be featured, we may includes it thought. > - IMHO, the advice code would benefit from an easy way to identify > where it was called from (not the joinpoint, the code calling the > joinpoint). The calling function/method can already be computed using a > pair of PHP functions, but it is still complex to retrieve and slow. > Filtering the calling object through a pointcut-like syntax would make it > easy to implement a policy like 'only this given set of classes can call > this class or this method'. Like for properties, it would allow a much > finer-grained access protection to methods/functions. > I'm not sure I got this right.. Aren't AopTriggeredJoinpoint->getTriggeringObject / getTriggeringMethod what you're looking for ? > - What does the PHP execution stack look like when an exception is > thrown from an advice ? Does it list the advice function, or the original > function, which arguments does it display (if an advice modified them > before the exception was thrown) ? > We overlooked that part. We'll check this, but I guess it will list the adviced function. (to be confirmed) > > - Can you give more details on the poincut syntax ? Would it be > extensible by some user code (defining functions in a way similar to > XPath) ? Is it possible to restrict a pointcut to a single class instance ? > The updated documentation added some examples. We're still working on a more complete syntax to enable logic in selectors (eg (**/*admin* && (notIncluded/** || alsoNotIncluded/**))). Not sure we'll include this in the first release. It could be possible to add some user code to validate the pointcut, but I'm kind of afraid of this, performance wise, and for the sake of sanity. Again, if users really want this to be part of the extension, we can discuss this. Restricting the pointcut to a single class instance is not possible as is. If think if developpers wants to do this, it have to be explicit and is beyond the scope of pure AOP. It is more in the scope of dynamic proxies... but I may be wrong (I cast a quick glance at Spring AOP / AspectJ, it seems it is possible in their implementation). The question now is, if we want to enable this "does this has to be a specific option, or can it be adressed by the possibility of adding user code to validate a pointcut ?". I'll have to think about it twice before giving an opinion. > That's your homework for today :) > Thank you, I love to have homework (now that I don't have actual homework anymore ^^) > Anyway, great work, guys. Hope to see it in PHP soon. > thx, we hope so Gérald