>> Unfortunately @PreDestroy in RI (GlassFish) is called in the
> "defined" order, i.e. the same order as @PostConstruct. Bill brought
> this up at the beginning of EE7, but because it wasn't defined when
> introduced, it's too late to change. So all interceptors are invoked
> according to the same rule.
>>
>> Does CDI behave differently?
>
> I'm pretty sure CDI does it the way mark describes. But I woulld have to
> check.
Well, at least in OpenWebBeans we do it subclass first for @PreDestroy ever since if I remember correctly. At least since a long time now, and we never had issues with any TCK.
The point is: parent-class-first for any destroyal operation would limit the functionality heavily. You e.g. can only touch fields and methods on your current class-level but not of any parent class as they might have been destroyed already. Thus by defining this as subclass-first you only gain functionality but don't loose any, right? Or did I overlook anything? Today you would need to implement the @PreDestroy only in the uppermost parent and call a virtual method which does subclass first. But even here it would be no problem to switch to subclass first for @PreDestroy methods as they are not implemented in the subclasses anyway...
So in my opinion we should define this for EE7 as this looks a perfectly backward compatible change for me. Of course this needs a review and other brains who check that we didn't overlook something.
Replies to the other parts will follow tomorrow.
LieGrue,
strub
----- Original Message -----
> From: Pete Muir <pmuir_at_redhat.com>
> To: marina.vatkina_at_oracle.com
> Cc: Mark Struberg <struberg_at_yahoo.de>; ejb-users <users_at_ejb-spec.java.net>
> Sent: Saturday, February 2, 2013 11:45 AM
> Subject: Re: [ejb-spec users] [cdi-dev] Fwd: Interceptors spec 1.2 draft is available for review
>
>
> On 2 Feb 2013, at 03:29, Marina Vatkina wrote:
>
>> Hi Mark,
>>
>> Thank you for the review!
>>
>> On 1/31/13 1:41 PM, Mark Struberg wrote:
>>> 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.
>>
>> How about this:
>>
>> "An interceptor class may define an interceptor method for all
> interposed invocations: an around-invoke interceptor method, around-timeout
> interceptor method, and a lifecycle callback interceptor method for each
> lifecycle event. See section xxx on invocation order of interceptors with
> superclasses."
>>
>> Or this"
>>
>> "An around-invoke interceptor method, around-timeout interceptor
> method, and lifecycle callback interceptor methods for different lifecycle
> events may be defined on the same interceptor class. See section xxx on
> invocation order of interceptors with superclasses."
>>
>>>
>>>
>>> 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?
>>
>> Unfortunately @PreDestroy in RI (GlassFish) is called in the
> "defined" order, i.e. the same order as @PostConstruct. Bill brought
> this up at the beginning of EE7, but because it wasn't defined when
> introduced, it's too late to change. So all interceptors are invoked
> according to the same rule.
>>
>> Does CDI behave differently?
>
> I'm pretty sure CDI does it the way mark describes. But I woulld have to
> check.
>
>>
>>>
>>> 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?
>>
>> Yes, it was an EJB limitation when interceptors were added. Is it a problem
> for CDI?
>
> No, it's just uncessary. We could relax this.
>
>>
>>>
>>> "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.
>>
>> It's there in the ordering chapter, where I just created a special
> section from this bullet:
>>
>> "If an interceptor class itself has superclasses, the interceptor
> methods defined by the interceptor class’s superclasses are invoked before the
> interceptor method defined by the interceptor class, most general superclass
> first."
>>
>> Do you think I should add a ref to each type explicitly?
>
> I think this needs to be explict in the definitions as well.
>
>>>
>>> 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?
>>
>> There can be multiple timeout methods defined via @Schedule. Each one can
> have its own set of interceptors (strange, but possible). I can remove "For
> example," if you feel it's confusing.
>>
>>>
>>> just formatting:
>>> page 14 is intentionally left blank?
>>
>> Yes. It follows EJB spec format - each chapter starts on the right page of
> an open book.
>>
>>> page 15/ Chapter3 has a huge empty block as well.
>>
>> Do you mean at the top? Every chapter starts at the bottom of its page.
>>
>>>
>>> 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(..) ?
>>
>> I'm not sure we should this should be forbidden, though ch2 says
> "Interceptors classes may be associated with the target class using either
> interceptor binding (see Chapter 3) or the Interceptors annotation (see Chapter
> 4)."
>>
>
> I don't think it does imply the reverse.
>
>>> 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.
>>
>> May be ch5 should be called "Enabling and Ordering Interceptors"?
>
> Agree with Mark. Let's make sure this extensible.
>
>>> 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?
>>
>> If it's enabled via @Interceptors on the @Stateful SomeBean, it will be
> called as all other "EJB" interceptor.
>
> Right, EJB on it's own doesn't support interceptor bindings.
>
>>>
>>> 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?
>>
>> Pete though we would, but we can't move an annotation to a different
> package without breaking backward compatibility, and other members of that
> package are CDI-specific. The version that I'm working on refers to the CDI
> spec on @Nonbinding rules.
>
> We would need to move the whole package (javax.enterprise.util) to common
> annotations. The other members of the package is are NOT CDI specific, and I
> asked the Java EE spec leads to consider moving the package, but it was turned
> down (not sure why exactly). We do have other specs who would like this package
> moved (JAX-RS).
>
>>
>>>
>>> Chapter 3.4.2:
>>> @Priority: which package is this? I guess commons-annoations, but where
> got this specced?
>>
>> In the common annotation MR
>>
>>> Btw, it would be good to have the magic number as float…
>>
>> Float is too complicated ;)
>
> I think int is large enough.
>
>>>
>>> "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?
>>
>> Pete?
>
> We discussed it, but there are still problems with it IIRC. We can always add
> this later.
>
>>
>>>
>>> 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?
>>
>> It's in addition - see rules in 5.3 (this section is changing places
> with 5.4/5.5 as we speak)
>
> Right the order is elsewhere.
>
>>
>>> 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?
>>
>> Pete said that subclass/superclass order is the same for the CDI and EJB
> interceptors.
>
> AFAIK the order is undefined.
>
>>>
>>> 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.
>>
>> It's correct. See above ;(
>>>
>>> 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?
>>
>> In the CDI spec ;)
>
> Yes. I need to do this.
>
>>
>>> What about EJBs then?
>>
>> It said - does not apply.
>>
>>>
>>>
>>> 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.
>>
>> It does. As you can see, it's not an easy task to merge 2 specs... If
> you have any other comments in the next couple of days, please post them. The MR
> specs are due for the JCP submission on the 7th.
>>
>> Best,
>> -marina
>>
>>>
>>> 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
>>>>
>>
>