Craig,
Thanks again for the feedback. Comments inline below....
On 8/17/2011 8:11 PM, Craig Ringer wrote:
> Linda,
>
> It looks really good to me. Thanks for running with this.
>
> PLUSES
> ======
>
> I particularly like the choice to enable CDI in entity listeners based on whether or not the containing archive is a
> bean archive, rather than having a special JPA-specific annotation. IMO that's _much_ better.
>
> The wording you've used gives containers the freedom to ignore or report listeners that depend on CDI but prevents it
> from invoking them. I think it's important that they not be invoked if they don't have CDI available, so that's a good
> thing.
>
> CHANGES?
> ========
>
> It says that "the persistence provider must not invoke entity
> listeners that depend upon CDI injection" where CDI isn't available. How is that to be determined? I didn't see any
> definition for this in the CDI spec, albeit at a fairly quick search and scan. Is a listener determiend to be dependent
> on CDI injection with the presence of any javax.inject or javax.enterprise.inject annotation? What about
> @javax.annotation.Resource or the presence of PreDestroy and PostConstruct methods, both of which are used by CDI but
> defined elsewhere and available for other uses?
>
Determining whether CDI is available is the easy part, as the BeanManager will be passed in to the createEMF call.
Determining whether the entity listener makes use of CDI is one of the reasons I had initially suggested use
of an annotation. I don't recall the final decision by the 299 expert group, but I believe that the @Inject
annotation is not *strictly* required by CDI. We should probably note that that persistence providers
should be able to rely on the use of @Inject in determining whether a listener relies on CDI, and that portable
applications must use it to specify injection points.
PostConstruct and PreDestroy are optional. It is an open issue as to whether if there were no injection points
the listener should be passed to CDI or not if @PostConstruct or @PreDestroy were used. I am inclined to flag
that as a portability item, but would like to hear from the group on this.
Use of @Resource for resource injection is not supported in entity listeners, as injection is being performed by
CDI, not the Java EE container. The application can however use producer fields/methods to declare resources
to be injected by CDI, or the entity listener can make use of JNDI lookup (as is supported in JPA today).
> Maybe it's nitpicking, but I'd potentially want to make it clearer that "PreDestroy and PostConstruct" methods refer to
> CDI, not methods named PreDestroy() and PostConstruct(). It _should_ be blindingly obvious that that's the intent, but
> given how people sometimes interpret specs perhaps "lifecycle callback methods annotated @PostConstruct and @PreDestroy
> (per JSR-250 2.5-2.6)" would be better?
>
Yes, that wording is better.
> Similary, I'd prefer to qualify the first use of "bean archive" as "bean archive (per JSR-299 12.1)" and even CDI as
> "CDI (JSR-299)" though that might be overboard.
>
> I'd also like to change:
>
> "The persistence provider is only required to support CDI injection into entity listeners in Java EE container
> environments."
>
> to read:
>
> "The persistence provider is only required to support CDI injection
> into entity listeners in Java EE container environments. The persistence provider /may/ support CDI injection into
> entity listeners in other environments where the BeanManager is available."
>
> ... or some other wording to the same effect. That's because embeddable CDI is on the way (see recent Weld work). It'd
> be good to make it clear that JPA implementers may use it in non-container contexts where it's available. The original
> wording doesn't exclude the possibility, but it'd be nice to make it explicitly permitted.
>
Yes, I think this is a good suggestion, as embeddable CDI is on the JSR 346 agenda.
>
>
> Finally, should persistence providers be permitted to grab the BeanManager directly from JNDI rather than being required
> to get it from the createConstainerEntityManagerFactory map arg? This would permit an updated JPA implementation in a
> container that hasn't yet been updated to be aware of JPA 2.1 to enable CDI in bean archives by fetching the BeanManager
> directly. CDI defines a standard location for the BeanManager in JNDI, so this will be safe.
>
Yes, this is not disallowed. Java EE specifies that the BeanManager be available under java:comp/env for components
that contain a bean archive. As suggested in the EG feedback, requiring the container to provide it in the
bootstrapping contract is a tidier solution.
> On 18/08/2011 3:59 AM, Linda DeMichiel wrote:
>> The following are the spec changes I propose to capture the semantics
>> of support for CDI injection into entity listeners.
>>
>> Please let me know as soon as possible if you have any corrections,
>> disagree with any of this, etc., etc.
>>
>> thanks,
>>
>> -Linda
>>
>> ------
>> The following is intended for insertion into section 3.5 (Entity
>> Listeners and Callback Methods):
>>
>> Entity listener classes in Java EE environments support dependency
>> injection through CDI when the containing archive is a bean archive.
>> An entity listener class that makes use of CDI injection may also
>> define PostConstruct and PreDestroy methods. These methods will be
>> invoked after injection has taken place and before the entity listener
>> instance is destroyed respectively.
>>
>> The persistence provider is responsible for using the CDI SPI to
>> create instances of the entity listener class; to perform injection
>> upon such instances; to invoke their PostConstruct and PreDestroy
>> methods, if any; and to dispose of the entity listener instances.
>>
>> The persistence provider is only required to support CDI injection
>> into entity listeners in Java EE container environments. If the
>> containing archive is not a bean archive, the BeanManager will not be
>> available, and the persistence provider must not invoke entity
>> listeners that depend upon CDI injection.
>>
>> An entity listener is a noncontextual object. In supporting injection
>> into entity listeners, the persistence provider must behave as if it
>> carries out the following steps involving the use of the CDI SPI.
>> (See [CDI spec] and [Weld reference]).
>>
>> 1. Obtain a BeanManager instance. (See section 9.1.)
>>
>> 2. Create an AnnotatedType instance for the entity listener class.
>>
>> 3. Create an InjectionTarget instance for the annotated type.
>>
>> 4. Create a CreationalContext.
>>
>> 5. Instantiate the listener by calling the InjectionTarget produce method.
>>
>> 6. Inject the listener instance by calling the InjectionTarget inject
>> method on the instance.
>>
>> 7. Invoke the PostConstruct callback, if any, by calling the
>> InjectionTarget postConstruct method on the instance.
>>
>> When the listener instance is to be destroyed, the persistence provider
>> must
>> behave as if it carries out the following steps.
>>
>> 1. Call the InjectionTarget preDestroy method on the instance.
>>
>> 2. Call the InjectionTarget dispose method on the instance
>>
>> 3. Call the CreationalContext release method.
>>
>> Persistence providers may optimize the steps above, e.g. by avoiding
>> calls to the actual CDI SPI and relying on container-specific
>> interfaces instead, as long as the outcome is the same.
>>
>> ---
>>
>> For obtaining the BeanManager instance, I propose the following
>> change to Section 9.1 (Bootstrapping in Java EE Environments), added
>> as a third bullet to the list on p.358:
>>
>> If the containing archive of the persistence unit is a bean archive, a
>> BeanManager instance must be made available by the container. The
>> container is responsible for passing this BeanManager instance via the
>> map that is passed as an argument to the
>> createConstainerEntityManagerFactory
>> call. The map key used must be the standard property name
>> javax.persistence.bean.manager. [Final property name TBD.]
>>
>>
>>
>
>
> 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/