On 2/2/13 2:16 PM, Mark Struberg wrote:
>>> 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.
It might not have a good TCK coverage...
>
> 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.
Nothing can be destroyed during the preDestroy callstack. In particular
fields declared in the superclass. All injected references should be
also there.
I'd rather be symmetrical with the requirement that all instances must
be created before the PostConstruct call chain is started, and say that
no instances are destroyed until the PreDestroy chain has finished.
-marina
> 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
>>>>>