wangscript / warp-persist

Automatically exported from code.google.com/p/warp-persist
0 stars 0 forks source link

Allow users to bind the TX interceptor multiple times #16

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
For example, you might want to achieve this if some types use a class level
@Transactional and some don't, so you still want to be able to exclude
methods individually:
bindInterceptor(annotatedWith(Transactional.class), any(), ...);
bindInterceptor(not(annotatedWith(Transactional.class)),
annotatedWith(Transactional.class), ...);

But you can't configure that with the current forAll API. A solution with
little impact would be to chain forAll calls:

Guice.createInjector(PersistenceService.usingHibernate()
    .across(UnitOfWork.REQUEST)
    .forAll(annotatedWith(Transactional.class), any())
    .forAll(not(annotatedWith(Transactional.class)),
annotatedWith(Transactional.class))
    .buildModule(); 

We should also consider making the above configuration the default.

Discussion here:
http://groups.google.com/group/warp-core/browse_thread/thread/98402cb8405ceab0

Original issue reported on code.google.com by robbie.v...@gmail.com on 22 Sep 2008 at 12:23

GoogleCodeExporter commented 9 years ago

Original comment by robbie.v...@gmail.com on 21 Dec 2008 at 5:40

GoogleCodeExporter commented 9 years ago
r253 has the forAll chaining. I think we should also change the default because 
the
above is what most people want.

Original comment by robbie.v...@gmail.com on 21 Dec 2008 at 8:41

GoogleCodeExporter commented 9 years ago
Changing the default will complicate things if people use a single forAll. We'll
stick with the current default.

Original comment by robbie.v...@gmail.com on 21 Dec 2008 at 8:53

GoogleCodeExporter commented 9 years ago
I don't think we need this--can't you just have really smart matchers in the 
first place?

Yea, also changing the default will force guice to proxy every single class it 
creates, which is an enormous 
overhead (like a certain *other* aop mechanism).

Original comment by dha...@gmail.com on 21 Dec 2008 at 10:51

GoogleCodeExporter commented 9 years ago
Sorry I'm lost, can you explain how that would cause Guice to proxy every single
class? Here is the code in Guice:
http://code.google.com/p/google-guice/source/browse/trunk/src/com/google/inject/
ProxyFactory.java

It seems to me that a class will only get proxied if the class matches and at 
least
one of the methods matches.

That said, I think that also means, like you said, that it should be possible to
achieve the above example by using a smart method matcher:
forAll(any(), myReallySmartMethodMatcher());

pseudo code:
return ((method.isAnnotationPresent(tx) &&
!method.getDeclaringClass().isAnnotationPresent(tx)) ||
(!method.isAnnotationPresent(tx) && 
method.getDeclaringClass().isAnnotationPresent(tx));

Anyway this feels like a hack, I would still prefer forAll chaining.

Original comment by robbie.v...@gmail.com on 22 Dec 2008 at 2:19

GoogleCodeExporter commented 9 years ago

Original comment by robbie.v...@gmail.com on 22 Dec 2008 at 2:19