Skip to content
This repository has been archived by the owner on Apr 14, 2022. It is now read-only.

Proposal of basic AOP DSL #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Proposal of basic AOP DSL #7

wants to merge 2 commits into from

Conversation

hekonsek
Copy link
Contributor

@hekonsek hekonsek commented Feb 2, 2013

Hi,

Spring comes with a pretty support for AOP configuration via XML DSL. Since Scala provides amazing DSL capabilities, it will be a true shame not to create Scala DSL for AOP as well.

I created simple AOP DSL enclosed within the AopSupport trait. Under the hood it creates ProxyFactoryBean configured with the DefaultPointcutAdvisors.

class TestConfiguration extends FunctionalConfiguration with AopSupport {

  // Define some pointcuts and advices.
  val pointcut = (m: Method, c: Class[_]) => m.getName == "toString"
  val beforeAdvice = (m: Method, args: Array[AnyRef], target: Any) => { println("Before!") }

  // You can wrap existing bean into AOP proxy. 
  bean("myService")(new MyService)
  advice targetRef "myService" on pointcut using beforeAdvice

  // Or create proxy over embedded POJO.
  advice target (new MyService) on pointcut using beforeAdvice

  // You can name the proxy and apply multiple advises to it as well.
  advice(beanName = "myService").target(new MyService).
    on(pointcat).using(beforeAdvice).
    on(someOtherPointcut).using(afterAdvice)

}

Actually many interesting things can be done in the area of Scala DSL for Spring AOP. This is the minimal usable part of it.

Of course if you like this pull request, I can create Scaladoc for it as well.

Best regards.

@poutsma
Copy link

poutsma commented Feb 15, 2013

This looks pretty cool. I will try and merge it next week.

@poutsma
Copy link

poutsma commented Feb 19, 2013

I looked at it a bit more, and it definitely looks like a great addition to Spring Scala. However, there are some things I would like to improve before I merge it. I can either describe these changes here, as comments, or I can make them myself and put them up on my personal fork for you to inspect. Which do you prefer?

Thanks again

@hekonsek
Copy link
Contributor Author

Let's try to go with the comments :) .

@poutsma
Copy link

poutsma commented Feb 21, 2013

First of all I'd like to thank you for all you recent additions to Spring Scala. When I started the project, I hoped people would contribute, and it is good to see people like yourself making this happen.

So here are my comments:

Things I like:

  • I like really the overall idea of this DSL. Having AOP support really makes a nice addition to Spring Scala.
  • I particularly like the AopSupport name. Writing configurations like class TestConfiguration extends FunctionalConfiguration with AopSupport really has a nice flow. I think I will use the Support suffix for other sub-configuration traits as well (eg. WebSupport).

Things I am not sure about:

  • I wonder if the PointcutConversions could be expanded. Currently, it seems that only static pointcuts are supported, and I think we should support dynamic ones as well. But of course, we can always do so at a later point in time.
  • I am not sure if the 'grammar' of the DSL is correct. The overall sentence structure 'advice target [target] on [pointcut] using [advice]' just doesn't "feel" right to me, though I am not sure what would be better. I will think about this one a bit more.

Things I don't like:

  • I am not a big fan of the union-like data structure in the target TargetDefinition class. The way I understand it is that either the targetName or the target is set in that class, but not both. I thing this is better solved through having a abstract base class with concrete implementations (AbstractTargetDefinition, subclass TargetDefinition, other subclass TargetNameDefinition), if you know what I mean.
  • I don't think AdviceDefinition and AdvicePointcutDefinition need to be public classes. I think I would prefer them as traits (with anonymous implementations inside AopSupport). I also think that they should be top-level traits, and not defined within the scope in AopSupport, because they don't seem to need a reference to the enclosing class/object. See BeanLookupFunction/FunctionalConfiguration for a comparison.

Bear in mind that I have little experience in writing DSLs in Scala (though I did write a couple in Java), so everything I said should be seen in that context. If there are valid reasons for doing things the way you did, then please teach me :).

Once again, thanks for your help!

@hekonsek
Copy link
Contributor Author

Hello Arjen,

Actually this pull request was a kind of proof of concept to see how you like the idea. Since your feedback is positive, I'd happy to polish the commit a little bit :) .

Thank you for all the valuable comments. I'll address them and apply related improvements at the beginning of the next week.

Best regards.

@hekonsek
Copy link
Contributor Author

As I would like to take a look at the JdbcTemplate issue #9 (comment) in the first place, I return to this pull request at the beginning of the next week. Stay tuned :) .

@poutsma
Copy link

poutsma commented Feb 28, 2013

Well, I think we can postpone this DSL until after Milestone 2 anyway, right?

@hekonsek
Copy link
Contributor Author

Definitely. This is nice-to-have, not urgent. I'll return to this subject after M2 release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants