Feedback from Craig Ringer on the users list.
-------- Original Message --------
Subject: Re: [jpa-spec users] [jsr338-experts] injection into entity listeners
Date: Thu, 21 Jul 2011 08:51:45 +0800
From: Craig Ringer <craig_at_postnewspapers.com.au>
To: jsr338-experts_at_jpa-spec.java.net
CC: Linda DeMichiel <linda.demichiel_at_oracle.com>, gordon.yorke_at_oracle.com, Adam Bien <abien_at_adam-bien.com>
On 21/07/2011 3:51 AM, Linda DeMichiel wrote:
> * An entity listener class may be defined for CDI injection.
> A listener class that is intended to receive CDI injection must
> be annotated with the @BeanListener annotation (final annotation
> name TBD).
Is that a performance optimization out of concern that unnecessary injection or indirection of instantiation via CDI may
slow things down if it's not needed?
By and large, in Java EE 6 post-CDI, things are injectable by default with no class annotations or any other
configuration required to enable injection. The presence of CDI injection points like @Inject or @Resource is
sufficient. For consistency and usability it might (see below) be desirable to retain that for entity listeners if it's
posible to do so without harmful side effects. Adding a magic "enable CDI" annotation may cause headaches for people
chasing unexpected NPEs /when/ they forget it and breaks the usual pattern of CDI-enabled apps.
In this case I actually think requiring an enabler annotation /will /be justified. I don't think using the BeanManager
SPI to do injection at instantiation time will be enough (see below). If full CDI support is needed, I can see good
reasons to keep listeners that don't need CDI injection and scoping as simple and fast as possible.
> * An entity listener class that is defined as a @BeanListener
> may specify PostConstruct and PreDestroy methods. These methods
> will be invoked by the provider after injection has taken
> place and before the entity listener instance is destroyed respectively.
That should be taken care of by use of the CDI SPI to allocate and destroy entity listeners. The provider should not
need to invoke them directly, and may land up invoking them again if CDI has already done so.
Perhaps: "If any no-argument method annotated @PostConstruct is found on an entity listener annotated with
@BeanListener, this method must be invoked exactly once after the constructor and initializer have run and before any
other methods are called."
I don't think this section will matter anyway if CDI-enabled listeners become managed beans, as I suspect will need to
happen. See below.
> * The persistence provider is only required to support CDI
> injection into listeners in Java EE container environments
> where the BeanManager is available in JNDI as java:comp/BeanManager.
> When this is not the case, @BeanListener classes will be
> ignored.
Total agreement there.
Implementations might decide to log a warning, but the spec doesn't need to care about that.
> Finally, just to be very clear about this, an entity listener
> would thus be, in CDI terminology, an "unmanaged object".
> In other words, it would receive CDI injections, would not itself
> be available for injection into other CDI-managed beans.
At first reading I didn't see any reason to make entity listeners managed objects. After writing my comments on
lifecycle below, though, I realized that they probably should be because (IMO) they need to be able to be scoped and
because injection is nearly useless without context.
The JPA 2.0 spec states that:
"Entity listeners are stateless. The lifecycle of an entity listener is unspecified."
Is that necessarily going to be useful for a listener subject to injection?
Consider just one use case for injection: injecting out the current security principal to update the "last changed by
user" field of an entity. If no lifecycle is defined and no consideration of state is made, there is no obvious point at
which (re)injection should be performed to ensure that the injected principal matches the current state of the
application. Since providers are also currently free to re-use and recycle entity listeners, the java.security.Principal
of the user active at the time the listener was created will be injected, and /will not be replaced when the listener is
called again on entities changed by other users/. The listener will be getting the wrong answer.
The only way to work around this at the application level will be by always injecting an Instance<Principal> and looking
it up on each use within the entity listener ... or to grab an EJB from JNDI and use it, ignoring CDI completely. While
that works, it is a big break from the general CDI programming model and - IMO - if CDI is to be supported, it should be
supported properly.
Right now, if an EntityListener wants any kind of information related to the user session, it has to look things up in
JNDI. If injection is to substitute for this, it needs to offer at least the same match with current context.
I'd go so far as to say that supporting injection without definining re-injection points in a lifecycle for entity
listeners is going to be nearly useless. CDI - /Contexts /and dependency injection - is all about scope and lifetime.
Applications are going to want access to more than just @ApplicationScoped managed beans, they're going to want to have
access to application state that may be bound to the current user session or even user request. They can already do this
to some extent via JNDI lookups of @Stateful EJBs, so I'm not suggesting introducing a completely new idea, just a way
to fit it into the CDI model now standardized in EE 6.
I know that getting into state and lifecycle is a can of worms it may not be desirable to open. Nonetheless it should
probably be considered in more detail - if only to confirm that it's right to sidestep it if that turns out to be the
case. It'll be hard to change the behavior of injection-aware entity listeners down the track if JPA 2.1 includes them
with undefined life cycle, and it's my strong belief that in the long run such a change /will/ be needed.
If a @BeanListener annotation (perhaps better as @EntityListenerBean) *is* to be used, one advantage is that it'd allow
the JPA 2.1 spec to define a lifecycle for such listeners without risking messing with existing listeners and risking
introducing backward compatibility issues.
Here's a broad outline of what I'm thinking:
- Unless annotated with @EntityListenerBean, entity listeners are stateless, have unspecified lifecycle, are not subject
to injection or CDI lifecycle callbacks, and are instantiated directly by persistence providers.
- Persistence providers may choose to emit a warning if CDI annotations are detected on an entity listener that is not
annotated @EntityListenerBean .
- An entity listener annotated with @EntityListenerBean is a managed bean. Such an "entity listener bean" has a
lifecycle managed by CDI, is scoped by CDI and is subject to injection.
- Persistence providers must obtain references to entity listener beans from CDI. Providers must never directly
instantiate entity listener beans.
In other words, an entity listener bean, identified by @EntityListenerBean, is a regular CDI managed bean that is
referenced as an entity listener and defines one or more entity listener callback methods.
That isn't especially cheap, but it's a cost only paid by listeners that need those features. It doesn't impose any cost
on existing listeners.
I don't have the experience with the guts of EclipseLink and Hibernate required to have much of an opinion on
implementation difficulty. Given the way the BeanManager is focused on supporting this use case I suspect it won't be
much more difficult than what Linda proposed, but I don't know that for sure. I'm hoping it'll be as simple as
categorizing listeners into CDI- and non-CDI during annotation processing at startup, so instead of one list of
listeners for each entity there are two. The non-CDI listeners are unaffected. For the CDI listeners, the BeanManager is
asked for an instance of the class whenever it is needed and the returned instance is called.
I'd love to have the feedback from someone with experience with CDI integration on this. The Mojarra team integrated CDI
support so they may have some useful feedback, as may the Seam 3 Faces team (
http://seamframework.org/Seam3/FacesModule)
who extended and enhanced it.
--
Craig Ringer
POST Newspapers 276 Onslow Rd, Shenton Park Ph: 08 9381 3088 Fax: 08 9388 2258 ABN: 50 008 917 717
http://www.postnewspapers.com.au/