users@ejb-spec.java.net

[ejb-spec users] Re: [cdi-dev] Fwd: Interceptors spec 1.2 draft is available for review

From: Marina Vatkina <marina.vatkina_at_oracle.com>
Date: Sun, 03 Feb 2013 17:12:25 -0800

Guys,

When I asked this questions on the EJB EG (see
http://java.net/projects/ejb-spec/lists/jsr345-experts/archive/2011-09/thread/1)
the answers split almost equally between preserving the order and making
the change.

I feel very uncomfortable making a last-minute change that can break an
existing behavior.

-marina

On 2/3/13 5:48 AM, Mark Struberg wrote:
> Btw, @PrePassivate might be a different beast. Calling this in a different order might result in different serialisation for some cases? If so then we would need to be careful to not introduce compat issues when a <=EE6 container talks with a >= EE7 container.
>
> LieGrue,
> strub
>
>
>
>
> ----- Original Message -----
>> From: Pete Muir <pmuir_at_redhat.com>
>> To: Mark Struberg <struberg_at_yahoo.de>
>> Cc: "marina.vatkina_at_oracle.com" <marina.vatkina_at_oracle.com>; ejb-users <users_at_ejb-spec.java.net>
>> Sent: Sunday, February 3, 2013 1:56 PM
>> Subject: Re: [ejb-spec users] [cdi-dev] Fwd: Interceptors spec 1.2 draft is available for review
>>
>> +1
>>
>> On 2 Feb 2013, at 17:16, 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.
>>>
>>> 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
>>>>>>>