Hi again Richard, folks, Comments inlined...
2009/6/24 Richard Hauswald <[email protected]>: > In my particular case: I use guice for DI. Guice supports scoping of > objects. This includes session scope. [...] Interceptors often access session > values. Stripes-Guice > also supports interceptor instantiation using guice. But the only > thing I can inject is the injector it self to use it later to retrieve > the required stateful information inside the intercept method. Ok, so stuff like : void intercept(...) { MyStatefulObject mso = // hold a ref to the session-scoped sful object mso.doStatefulThings() ... } (I don't have any experience with Guice but I guess your injector thing is stuff like Spring's bean factory) If that's what you do, then it's MyStatefulObject that should be thread safe :P And I don't see any reason why injecting the MyStatefulObject instance into the interceptor would change this, be it at construction time, of via method-based injection. And even with a new interceptor instance, it's just the same. Imagine that the interceptor(s) handles several requests concurrently and all invoke a session-scoped, but NOT thread-safe MyStatefulObject... Could lead to a very nasty bug IMO. Thread safety all boils down to your business classes. Unless I've missed something, I don't see any reason why the lifecycle of interceptors would allow you to create shared objects (through session or anything) that aren't thread safe... > If a > new interceptor would be instantiated each request I could inject the > required stateful values directly using the constructor. See above : don't think it's related. If your stateful object is shared, then it has to be thread safe... by definition, several threads (not instances) are gonna invoke the instance ! > This would > make programming and testing interceptors more easy, cause I can ask > for the things I need, not for things I need to get the things I need. > Less code, less chance of errors and less mocking effort for testing. What would this change to your tests ? What's the difference, when it comes to testing, between those two different implems : // needs DI system (Spring, Guice, whatever) and does the lookup class StatelessInterceptor ... { private MyDiSystem myDiSystem def StatelessInterceptor(MyDiSystem myDiSystem) { this.myDiSystem = myDiSystem } void intercept(...) { MySful sful = (MySful)myDiSystem.getBean('sfulBean') sful.doStuff() } } // needs MySful instance class InjectedInterceptor ... { def InjectedInterceptor(MySful sful) { this.sful = sful } void intercept(...) { sful.doStuff() } } ? Both versions need a ref to some collaborator in order to work. Ok, the second doesn't depend on the DI implem etc. But apart from that, what difference ? Just one line, for a perfectly immutable, thread safe object ? Worth the cost IMO :) Last, the interceptor should in any case delegate to helper classes, which should be unit tested first. Then, the only real way to make sure your interceptor actually works is via webtests IMHO. MockRoundtrip is already quite cool, but clearly, it doesn't compare to a real web testsuite. > If I do need a state in my interceptor I will have to create the > stateful objects anyway - regardless if a new interceptor is > instantiated or a new stateful object inside the intercept method. Not create them, apparently you just obtain a reference on them (from a DI system, from the HTTP session, whatever). Excepted for an interceptor that counts how many times some actions are used (and which would hold an integer field to store the count), I don't see why it would require any state... Note that servlets work like that as well (and servlet filters too) :P > IMHO garbage collecting of objects created inside a method is still > something that needs to be done so the gc has to work harder in any > way. Again, your sful instances have to exist prior to inject them anyway, so there's no object creation inside the interceptor method, only a "lookup". If we had closures or even function pointers in Java, then interceptors would probably be well implemented using them :P > Compared to the number of objects created when a request comes > in, a chain of 10 interceptors wouldn't really matter. I don't have figures, but I usually avoid load unless it's required... And it actually could be quite huge, depends on the number of interceptors, stages and most of all incoming requests... Can be quite a lot... > Anyway this > problem can be solved using a single annotation indicating if the > interceptor should be singleton or not. Sure it's no big deal in the idea, but as usual it raises questions... What e.g. for the lifecycle stages ? Do you need a new instance for each stage ? One for the whole request per interceptor ? ... It's not lazyness, really, and in the end if people are interested in this feature, then be it. Shouldn't be that hard to implement... But I'm curious, Id like to understand the problem with the current implementation before :P > The web flow I'm used to is that you don't care about thread safety(as > long as no information needs to flow between two concurrent requests) > since every request and its children are existing in their own thread > and environment which is realized by creating different instances of > the same classes for every request. ActionBeans are the best example > for this. IMHO Stripes interceptors are also part of the control layer > - the same one ActionBeans belong to. This smells like they should > share the scope they are existing in. Action beans are stateful by definition : they keep state between the different stages of the lifecycle. Several methods can be invoked (sequentially, not concurrently, that's why you don't need them to be thread safe) on the instance so that it can perform the handling of the HTTP request. IMHO interceptors fall into the category of the Stripes Dispatcher and Filter, more than into the category of ActionBeans... > I know one should not get into the business of trying to stop people > doing silly things but the singleton scope of interceptors invites > unexperienced programmers (like me) to use synchronized methods. Why ? > The > execution time of the gc scales with the number of cpu's. Thread > synchronization does not. IMHO this is not really getting into the > business here since many web frameworks are following the pattern to > isolate a request by creating new objects - including stripes. Even Tomcat does, for sure, but synchronization has nothing to deal with this as far as I understand. Again, I might have missed something, but I don't have any idea about why the interceptor lifecycle could be a problem... Last, those session things made me think of the ActionBeanContext... Stripes already provides some indirection for accessing session scoped things via the context, maybe it can solve your problem ? Can't you use the context (which is stateful for the time of a request) to provide refs to your sful objects to your interceptor ? Cheers Remi ------------------------------------------------------------------------------ _______________________________________________ Stripes-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/stripes-users
