i agree that we have an un-(/under-)specified area here. i've pushed the changes since it's currently the only approach which works with both implementations (owb and weld). (for now the handling of dependent scoped invocation-handlers isn't included.)
@repackaging: it was just an example, however, we can discuss the details once we agreed on an implementation. @john: you asked for supporting qualifiers in combination with @InvocationHandlerBinding. i'm not sure what you tried to get in. for me it sounded more like [1] (and i don't agree with that). regards, gerhard [1] http://s.apache.org/5nM 2012/12/27 Mark Struberg <[email protected]> > > > it works already for the invocation-handler, but >only< with owb. > > Yes, this is because OWB applies the interceptor and decorators _outside_ > of Producer<T>/InjectionTarget<T>. > Weld does this _inside_ thus it only works for Bean<?> which have a > Producer<T>/InjectionTarget<T>. > > Both ways are allowed by the spec, so we cannot rely on it. > > > Thus the only _portable_ way to implement interceptors on the > InvocationHandlers (which is imo a must) is to pick them up as CDI beans -> > option C.) > > > > @no core-dependencies: > > agreed - nobody said that we will keep it that way. e.g. we can repackage > > the proxy lib (with the shade-plugin for maven). > > Nope, that gonna be 2MB++. For my personal taste this is just too fat to > be shaded in! > > > LieGrue, > strub > > > > ----- Original Message ----- > > From: Gerhard Petracek <[email protected]> > > To: [email protected] > > Cc: > > Sent: Thursday, December 27, 2012 2:24 PM > > Subject: Re: [DISCUSS] [DELTASPIKE-113] Review and Discuss ServiceHandler > > > > @abstract classes: > > i agree with john (in view of more complex queries). that's actually the > > only reason why we need DELTASPIKE-113 (for DELTASPIKE-60). > > > > @interceptors: > > it works already for the invocation-handler, but >only< with owb. > > since DELTASPIKE-60 is just for doing the actual queries, it's a > > restriction which affects esp. simple constellations. > > once you call such daos from a transactional bean (/service), you only > have > > issues with fine grained interceptors for daos (e.g. > security-interceptors). > > -> it depends on your application, if you see the restriction at all. > > > > @no core-dependencies: > > agreed - nobody said that we will keep it that way. e.g. we can repackage > > the proxy lib (with the shade-plugin for maven). > > > > regards, > > gerhard > > > > > > > > 2012/12/27 Mark Struberg <[email protected]> > > > >> whoops, forgot to share the links ^^ > >> > >> https://svn.apache.org/repos/asf/commons/sandbox/privilizer/trunk/ > >> http://commons.apache.org/sandbox/privilizer/ > >> > >> Please note that our docs are not yet updated to reflect the generic > >> weaver. > >> > >> LieGrue, > >> strub > >> > >> > >> > >> > >> ----- Original Message ----- > >> > From: Mark Struberg <[email protected]> > >> > To: "[email protected]" < > >> [email protected]> > >> > Cc: > >> > Sent: Thursday, December 27, 2012 1:30 PM > >> > Subject: Re: [DISCUSS] [DELTASPIKE-113] Review and Discuss > > ServiceHandler > >> > > >> > compile time would be an option! > >> > > >> > It happens that Matt and I are currently working on commons-weaver > > [1]. > >> > It started as 'privilizer' (to generate doPrilived blocks fo > > @Privileged > >> > annotated methods) but we moved it to a more generic pattern > recently. > >> > It basically provides an ant task and a maven-plugin which trigger > the > >> > WeaveProcessor. This WeaveProcessor picks up all provided weaving > >> plugins. In > >> > the privilizer plugin we just change the bytecode of the existing > >> classes. > >> > We could easily create such a weaving plugin which would change the > >> abstract > >> > class into a full class and fill in the InvocationHandler calls. > >> > The resulting class files do not have any runtime dependencies that > > way. > >> > Javassist or whatever you choose to do the bytecode stuff is only > used > > at > >> > compile time. > >> > > >> > > >> > LieGrue, > >> > strub > >> > > >> > > >> > > >> >> ________________________________ > >> >> From: John D. Ament <[email protected]> > >> >> To: [email protected]; Mark Struberg > >> > <[email protected]> > >> >> Sent: Thursday, December 27, 2012 1:19 PM > >> >> Subject: Re: [DISCUSS] [DELTASPIKE-113] Review and Discuss > >> ServiceHandler > >> >> > >> >> > >> >> Agreed (separate module since it has a dependency on javassist) > >> >> > >> >> > >> >> I think abstract classes are a must. I can think of some DAO use > > cases > >> > where the handler's approach may not match how they want the > > search to > >> > operate, plus if they want to do a criteria query I can't think of > > a > >> generic > >> > way to do that (yet). > >> >> > >> >> > >> >> Can't we use a ProducerMethod to obscure the fact that we > > cannot simply > >> > install a bean up front? Is there a timing issue w/ when we can add > > the > >> producer > >> > methods vs beans? What about a compile-time option where we generate > >> the class > >> > up front via a compiler plugin or maven plugin? > >> >> > >> >> > >> >> John > >> >> > >> >> > >> >> > >> >> On Thu, Dec 27, 2012 at 7:10 AM, Mark Struberg > > <[email protected]> > >> > wrote: > >> >> > >> >> > >> >>> > >> >>> Indeed. If we need any byte code engineering library then it > > must not > >> be > >> > in core-impl but in a separate module. core-impl shall not have _any_ > >> 3rd party > >> > runtime dependencies. > >> >>> > >> >>> LieGrue, > >> >>> strub > >> >>> > >> >>> > >> >>> > >> >>> > >> >>>> ________________________________ > >> >>>> From: Romain Manni-Bucau <[email protected]> > >> >>>> To: Mark Struberg <[email protected]>; > >> > [email protected] > >> >>>> Sent: Thursday, December 27, 2012 12:41 PM > >> >>> > >> >>>> Subject: Re: [DISCUSS] [DELTASPIKE-113] Review and Discuss > >> > ServiceHandler > >> >>>> > >> >>>> > >> >>>> We proxy abstract classes? Is that mandatory? I would like > > to be > >> > able to skip javassist as forced dependency. > >> >>>> Le 27 déc. 2012 12:20, "Mark Struberg" > >> > <[email protected]> a écrit : > >> >>>> > >> >>>> As pointed out by Gerhard on IRC we have 2 different areas > > where we > >> > need interception > >> >>>>> > >> >>>>> 1.) on the InvocationHandler and > >> >>>>> > >> >>>>> 2.) on the abstract class. > >> >>>>> > >> >>>>> In hindsight of DELTASPIKE-60 I'm thinking about > >> > @Transactional and @Securec, etc. > >> >>>>> > >> >>>>> LieGrue, > >> >>>>> strub > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>> ----- Original Message ----- > >> >>>>>> From: Mark Struberg <[email protected]> > >> >>>>>> To: > > "[email protected]" > >> > <[email protected]> > >> >>>>>> Cc: > >> >>>>>> Sent: Thursday, December 27, 2012 11:24 AM > >> >>>>>> Subject: Re: [DISCUSS] [DELTASPIKE-113] Review > > and Discuss > >> > ServiceHandler > >> >>>>>> > >> >>>>>> You are right! And we need to take this C. route. > > But for > >> > other reasons than > >> >>>>>> having a different state lifecycle in the > > servicehandler > >> > than in the service. > >> >>>>>> > >> >>>>>> The reason is that the CDI spec doesn't > > define a > >> > portable way how to > >> >>>>>> intercept contextual instances generated via a > >> > Bean<T>. This is only > >> >>>>>> defined for Decorators and 'Managed > > Beans' > >> > (Bean<T> resulting from > >> >>>>>> a scanned class). > >> >>>>>> > >> >>>>>> In practice there would also be an option to > > generate a > >> > Proxy class and add an > >> >>>>>> AnnotatedType for it. I think this also works in > > all > >> > containers. The problem > >> >>>>>> here being that this doesn't work in CDI-1.0 > > as there > >> > are yet no > >> >>>>>> 'synthetic annotated types' plus we have > > the > >> > problem that we would need > >> >>>>>> to know this info in BeforeBeanDiscovery (for > >> > #addAnnotatedType). So we would > >> >>>>>> need to manually scan with some other tools than > >> > ProcessAnnoatedType. That would > >> >>>>>> btw something to consider in our CDI spec. a > > Method > >> >>>>>> > >> >>>>>> > > ProcessAnnotatedType#addSyntheticAnnotatedType(..); > >> >>>>>> > >> >>>>>> > >> >>>>>> Anyway. It doesn't work in CDI-1.0 thus we > > only have > >> > the way to pick up the > >> >>>>>> InvocationHandlers via CDI itself. Which means > > they also > >> > have their own scope! > >> >>>>>> Otherwise we would not be able to add an > > @Transactional to > >> > the servicehandler > >> >>>>>> InvocationHandler. > >> >>>>>> > >> >>>>>> LieGrue, > >> >>>>>> strub > >> >>>>>> > >> >>>>>> > >> >>>>>> > >> >>>>>> ----- Original Message ----- > >> >>>>>>> From: John D. Ament > > <[email protected]> > >> >>>>>>> To: [email protected]; > > Mark Struberg > >> >>>>>> <[email protected]> > >> >>>>>>> Cc: > >> >>>>>>> Sent: Thursday, December 27, 2012 12:57 AM > >> >>>>>>> Subject: Re: [DISCUSS] [DELTASPIKE-113] > > Review and > >> > Discuss ServiceHandler > >> >>>>>>> > >> >>>>>>> i think there's a C here as well that > > can be > >> > considered (which is what > >> >>>>>>> I've > >> >>>>>>> been driving to): > >> >>>>>>> > >> >>>>>>> Allow the scope of the InvocationHandler to > > drive the > >> > scope of the > >> >>>>>>> InvocationProxy (the interface/abstract > > class we just > >> > proxied), with an > >> >>>>>>> override to a narrower scope (if so chosen > > by the app > >> > developer). This > >> >>>>>>> approach closely mirrors the CDI approach of > > injecting > >> > a session scoped > >> >>>>>>> object in to a request scoped object, or > > another > >> > session scoped object (so > >> >>>>>>> it's relate-able). We don't veto > > the > >> > InvocationHandler and instead > >> >>>>>> > >> >>>>>>> allow > >> >>>>>>> it to retain its original scope (in fact, we > > don't > >> > have to do anything > >> >>>>>> with > >> >>>>>>> the invocation handler until runtime and > > validation). > >> > We just have to make > >> >>>>>>> sure we install the InvocationProxy with the > >> > appropriate scopes. > >> >>>>>>> > >> >>>>>>> > >> >>>>>>> On Wed, Dec 26, 2012 at 5:15 PM, Mark > > Struberg > >> > <[email protected]> > >> >>>>>> wrote: > >> >>>>>>> > >> >>>>>>>> I think we have to first point out all > > available > >> > options. > >> >>>>>>>> > >> >>>>>>>> Option A.) is to treat the > > InvocationHandler as > >> > CDI bean and create > >> >>>>>> all > >> >>>>>>>> the proxies as @Dependent beans and > > inject them > >> > directly. > >> >>>>>>>> So you would _not_ get a normalscoped > > CDI proxy > >> > (Contextual Reference) > >> >>>>>> but > >> >>>>>>>> our own proxy which is different for > > each > >> > injection point. And this > >> >>>>>> own > >> >>>>>>>> proxy resolves the InvocationHandler as > > CDI > >> > beans. > >> >>>>>>>> > >> >>>>>>>> Option B.) The InvocationHandler is > > _no_ CDI bean > >> > at all. It's > >> >>>>>> even > >> >>>>>>> vetoed > >> >>>>>>>> as bean! We take the scope and the > > qualifiers, > >> > etc from the > >> >>>>>>> 'serviced' > >> >>>>>>>> interface/abstract class and create a > >> > Bean<?> for each of it > >> >>>>>> which > >> >>>>>>> gets > >> >>>>>>>> those scopes and qualifiers. The > > registered Beans > >> > will create > >> >>>>>> Contextual > >> >>>>>>>> Instances which are _our_ > > servicehandler proxies. > >> > Those will be stored > >> >>>>>> in > >> >>>>>>>> the Contexts. During injection the CDI > > container > >> > will apply all > >> >>>>>>>> NormalScoped mechanism like the CDI > > proxy over > >> > our internal > >> >>>>>> servicehandler > >> >>>>>>>> proxy. > >> >>>>>>>> > >> >>>>>>>> Both ways will provide similar results, > > but they > >> > each have a different > >> >>>>>>>> impact on side effects, states and > > handling. > >> >>>>>>>> > >> >>>>>>>> I think B.) is what Gerhard > > implemented, right? > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> What option was used in Seam? > >> >>>>>>>> > >> >>>>>>>> LieGrue, > >> >>>>>>>> strub > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> ----- Original Message ----- > >> >>>>>>>> > From: Gerhard Petracek > >> > <[email protected]> > >> >>>>>>>> > To: > > [email protected] > >> >>>>>>>> > Cc: > >> >>>>>>>> > Sent: Wednesday, December 26, 2012 > > 9:59 PM > >> >>>>>>>> > Subject: Re: [DISCUSS] > > [DELTASPIKE-113] > >> > Review and Discuss > >> >>>>>>> ServiceHandler > >> >>>>>>>> > > >> >>>>>>>> > hi john, > >> >>>>>>>> > > >> >>>>>>>> > as mentioned before: > >> >>>>>>>> > > >> >>>>>>>> >> @ InvocationHandler as a > > separated bean > >> > (at runtime): > >> >>>>>>>> >> currently i can't see a > > benefit for > >> > DELTASPIKE-60. > >> >>>>>>>> > > >> >>>>>>>> > regards, > >> >>>>>>>> > gerhard > >> >>>>>>>> > > >> >>>>>>>> > > >> >>>>>>>> > > >> >>>>>>>> > 2012/12/26 John D. Ament > >> > <[email protected]> > >> >>>>>>>> > > >> >>>>>>>> >> Gerhard, > >> >>>>>>>> >> > >> >>>>>>>> >> Just so I'm clear, when I > > was > >> > referring to the current > >> >>>>>>> implementation, > >> >>>>>>>> > it > >> >>>>>>>> >> was the one shipped with > > Seam3/Solder: > >> >>>>>>>> >> > >> >>>>>>>> >> > >> >>>>>>>> > > >> >>>>>>>> > >> >>>>>>> > >> >>>>>> > >> > > >> > > > https://github.com/seam/solder/tree/develop/impl/src/main/java/org/jboss/solder/serviceHandler > >> >>>>>>>> >> > >> >>>>>>>> >> It does look like we're > > doing > >> > something very similar by > >> >>>>>>> veto'ing > >> >>>>>>>> > the > >> >>>>>>>> >> handler classes. > >> >>>>>>>> >> > >> >>>>>>>> >> else if > >> >>>>>>> > > (InvocationHandler.class.isAssignableFrom(beanClass)) > >> >>>>>>>> >> { > >> >>>>>>>> >> > >> > validateInvocationHandler(beanClass, > >> >>>>>>>> bindingAnnotationClass); > >> >>>>>>>> >> > >> >>>>>>>> >> > >> >>>>>> > > this.partialBeanHandlers.put(bindingAnnotationClass, > >> >>>>>>>> >> (Class<? extends > >> > InvocationHandler>) beanClass); > >> >>>>>>>> >> pat.veto(); > >> >>>>>>>> >> } > >> >>>>>>>> >> > >> >>>>>>>> >> I believe as a result, we > > have to do > >> > what you're doing > >> >>>>>> in > >> >>>>>>>> >> PartialBeanLifecycle.create > > (line 75) > >> > to manually create the > >> >>>>>> > >> >>>>>>> instance. > >> >>>>>>>> >> If we just let the scopes > > handle the > >> > scopes whether this is > >> >>>>>> a > >> >>>>>>> new > >> >>>>>>>> >> instance or an existing > > instance should > >> > resolve itself more > >> >>>>>>> naturally. > >> >>>>>>>> >> > >> >>>>>>>> >> > >> >>>>>>>> >> > >> >>>>>>>> >> On Wed, Dec 26, 2012 at 2:06 > > PM, John > >> > D. Ament > >> >>>>>>> <[email protected] > >> >>>>>>>> >> >wrote: > >> >>>>>>>> >> > >> >>>>>>>> >> > Gerhard, > >> >>>>>>>> >> > > >> >>>>>>>> >> > I apologize, I > > hadn't realized > >> > you implemented this > >> >>>>>> > >> >>>>>>> feature, > >> >>>>>>>> > considering > >> >>>>>>>> >> > it has been assigned to > > me. > >> >>>>>>>> >> > > >> >>>>>>>> >> > John > >> >>>>>>>> >> > > >> >>>>>>>> >> > > >> >>>>>>>> >> > On Wed, Dec 26, 2012 at > > 1:56 PM, > >> > Gerhard Petracek < > >> >>>>>>>> >> > > > [email protected]> > >> > wrote: > >> >>>>>>>> >> > > >> >>>>>>>> >> >> hi john, > >> >>>>>>>> >> >> > >> >>>>>>>> >> >> that can't be - > > the > >> > described example > >> >>>>>> (/excerpt) is > >> >>>>>>> a copy of > >> >>>>>>>> > a working > >> >>>>>>>> >> >> example (tested with > > owb and > >> > weld). > >> >>>>>>>> >> >> > >> >>>>>>>> >> >> the only use-case > > (we have so > >> > far) which can't > >> >>>>>> be > >> >>>>>>> implemented > >> >>>>>>>> > with std. > >> >>>>>>>> >> >> cdi > >> >>>>>>>> >> >> mechanisms (due to > > abstract > >> > classes) is > >> >>>>>> DELTASPIKE-60. > >> >>>>>>>> >> >> > >> >>>>>>>> >> >> @ InvocationHandler > > as a > >> > separated bean (at > >> >>>>>> runtime): > >> >>>>>>>> >> >> currently i > > can't see a > >> > benefit for > >> >>>>>> DELTASPIKE-60. > >> >>>>>>>> >> >> > >> >>>>>>>> >> >> regards, > >> >>>>>>>> >> >> gerhard > >> >>>>>>>> >> >> > >> >>>>>>>> >> >> > >> >>>>>>>> >> >> > >> >>>>>>>> >> >> 2012/12/26 John D. > > Ament > >> >>>>>> <[email protected]> > >> >>>>>>>> >> >> > >> >>>>>>>> >> >> > but the > >> >>>>>>>> >> >> > specific one > > annotated a > >> > certain way. The > >> >>>>>> cleanest > >> >>>>>>> way > >> >>>>>>>> > (conceptual > >> >>>>>>>> >> >> > > >> >>>>>>>> >> >> > >> >>>>>>>> >> > > >> >>>>>>>> >> > > >> >>>>>>>> >> > >> >>>>>>>> > > >> >>>>>>>> > >> >>>>>>> > >> >>>>>> > >> >>>>> > >> >>>> > >> >>>> > >> >>> > >> >> > >> >> > >> >> > >> > > >> > > >
