Hi Marina, Pete, other EG fellows!
I quickly read through the spec and took some small notes. Some points maybe right, others might be wrong, hope the feedback helps nontheless ;)
Chapter 2.1:
"Around-invoke interceptor methods, around-timeout interceptor methods, and lifecycle callback interceptor methods may be defined on the same interceptor class."
This sounds like there could be multiple of each in the same class, e.g. 2 AroundInvoke. I know this is clarified later, but maybe we can find a wording which makes this more clear from the beginning. It's also often unclear if 'on the same interceptor class' includes parent classes or not. Maybe add a link to a later paragraph where this is defined much more in detail.
Chapter 2.2:
Maybe add that @PostConstruct lifecycle interceptors are invoked in parent-class-first order, whereas @PreDestroy interceptors are invoked in subclass-first order. Or is this defined elsewhere already? They should follow constructor invocation order respective finalize order. It might also be ok to clarify that overloaded interceptor methods in subclasses disable their respective information from the parent class. Even introduce an own 'Interceptor Method resolution' chapter or similar?
Chapter 2.5:
"An around-invoke method must not be declared as abstract, final or static."
Why so? I can see that abstract makes no sense, buy we don't need to further proxy them, don't we? So why not final nor static? Has this to do with some EJB limitations?
"Only one around-invoke method may be defined on a given class."
Including superclasses or not? Would it be ok to have 1 @AroundInvoke in the subclass and another in the parent class or max 1 for the whole class hierarchy? This is in general not always clear from the wording. I remember having asked this a few years ago and since then we support 1 per each class in the hierarchy in OWB.
Chapter 2.8:
"For example, an around-invoke interceptor method may be defined to apply only to a specific target class method invocation,.."
Why 'for example'? A lifecycle interceptor cannot be annotated to some method, only @AroundInvoke can, isn't? To be honest I didn't get the paragraph about method-level timeout interceptors. Maybe it's just me understanding the term the wrong way. Can you give me a sample of a timeout interceptor applied to a method?
just formatting:
page 14 is intentionally left blank?
page 15/ Chapter3 has a huge empty block as well.
Chapter 3.2:
"If an interceptor does not declare an Interceptor annotation, it could be bound to components using Interceptors annotation or a deployment descriptor file."
Does this also imply forbidding the reverse? That a class annotated with @Interceptor must not be used in @Interceptors(..) ?
This chapter should also note that the rules for activation such @Interceptor classes might be defined in other specs (e.g. for CDI only if enabled in a beans.xml). Or we define those rules here.
Otherwise an @Interceptor which is not enabled in beans.xml would _not_ be active for CDI beans but enabled for EJBs? And what happens if you have a @RequestScoped @Stateful class SomeBean?
Chapter 3.4:
This chapter should generally introduce the restriction on 'active interceptors'.
This chapter defines the behavior of @Nonbinding. Does this mean we need to move this annotation from the CDI spec packages (e.g. cdi-api.jar) to the interceptor spec API jar? Or better to commons-annotations?
Chapter 3.4.2:
@Priority: which package is this? I guess commons-annoations, but where got this specced? Btw, it would be good to have the magic number as float…
"If an array-valued or annotation-valued member of an interceptor binding type is not annotated with the Nonbinding annotation, non-portable behavior results."
Wasn't there a solution for the array issue over on our CDI EG list?
Chapter 4:
"Method-level target class method interceptors are invoked in addition to any default interceptors and interceptors defined for the target class (and its superclasses)."
Imo this is way too blurry. Not 'in addition' but we should instead define whether 'before' or 'after', isn't? We should also define the order of subclass/superclass interceptors. For superclass it also depends whether the @InterceptorBinding has @Inherited or not, don't it? Or just apply standard Java rules and let 'overriding' kick in?
Chapter 5.2:
"If a target class has superclasses, any interceptor methods defined on those superclasses are invoked, most general superclass first."
This is true for all but @PreDestroy and @PrePassivate interceptors. Those must not follow ct rules but finalize rules and imo should get invoked in subclass-first order.
Chapter 5.4:
a.) @Priority should get a float instead of an int. It's easier to squeeze something in that way.
b.) We must clarify the relation between a priority defined in beans.xml and @Priority, or should we do that in the CDI spec? What about EJBs then?
Well, I only did read through it once and I guess I got a few things wrong. But at least these are a few ideas floating around in my head. Hope it helps.
LieGrue,
strub
----- Original Message -----
> From: Pete Muir <pmuir_at_redhat.com>
> To: cdi-dev_at_lists.jboss.org
> Cc:
> Sent: Wednesday, January 30, 2013 8:39 AM
> Subject: [cdi-dev] Fwd: Interceptors spec 1.2 draft is available for review
>
> FYI - big improvements to the interceptors spec - much better for alignment with
> CDI.
>
> I'll update CDI, next, to take account of this.
>
> Begin forwarded message:
>
>> From: Marina Vatkina <marina.vatkina_at_oracle.com>
>> Subject: Interceptors spec 1.2 draft is available for review
>> Date: 29 January 2013 22:06:36 GMT
>> To: jsr345-experts_at_ejb-spec.java.net, Pete Muir <pmuir_at_redhat.com>
>>
>> After a lot of work by Linda, me, and Pete, we have the Interceptors spec
> 1.2 draft for review:
> http://java.net/projects/interceptors-spec/downloads/download/interceptor-1-2-dr1.pdf
>>
>> What's there:
>> Editorial cleanup and conversion to standard template.
>> Assigned chapter numbers to sections and rearranged various sections and
> examples for better flow.
>> Clarified statement regarding transaction context of lifecycle callback
> methods
>> Added a note on a timeout method that is also a business method and
> around-timeout and around-invoke interceptors
>> Added Chapter 1 (Overview)
>> Added Chapter 3, derived from Chapter 9 of the CDI specification.
>> Removed deployment descriptors definitions (general notes about possibility
> of DDs are there)
>> Added examples with interceptor bindings to common sections.
>> Added standard Priority ranges
>>
>> What's not there:
>> @AroundConstruct interceptor
>> Notes on "throws Exception" clauses in the around-xxx method
> signatures
>> Perfect CDI alignment - the text (and fonts) might be not fully aligned.
>>
>> Please review ASAP.
>>
>> Pete, please forward to the CDI EG.
>>
>> thanks,
>> -marina
>
>
> _______________________________________________
> cdi-dev mailing list
> cdi-dev_at_lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/cdi-dev
>