users@ejb-spec.java.net

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

From: Mark Struberg <struberg_at_yahoo.de>
Date: Mon, 4 Feb 2013 15:05:57 +0000 (GMT)

I've now read through the responses  and to me personally the cons were 'there might be backward compat issues' without giving a single example.


I would be happy to have a single example of a code which would be broken. Until then I see no reason to not introduce that change.


LieGrue,
strub


>________________________________
> From: Marina Vatkina <marina.vatkina_at_oracle.com>
>To: users_at_ejb-spec.java.net
>Sent: Monday, February 4, 2013 2:12 AM
>Subject: [ejb-spec users] Re: [cdi-dev] Fwd: Interceptors spec 1.2 draft is available for review
>
>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
>>>>>>>>
>
>
>
>