HHH-16069 - Defer handling of custom types until after deployment time #5994
Conversation
sebersole
force-pushed
the
cdi-usertype2
branch
from
3f0f9fc to
3aacc4e
Compare
| * @implNote Defaulted for backwards compatibility | ||
| */ | ||
| <T> ManagedBean<T> getBean(String beanName, Class<T> beanContract); | ||
| default <T> ManagedBean<T> getBean(Class<T> beanClass, boolean cdiRequiredIfAvailable) { |
There was a problem hiding this comment.
You might want to use an enum instead of a boolean, both for more explicit code on call sites and to be free to add other behavior later if need be?
| if ( usableBeanManager == null ) { | ||
| BeanInstanceProducer fallbackProducer, | ||
| boolean cdiRequiredIfAvailable) { | ||
| if ( !cdiRequiredIfAvailable ) { |
There was a problem hiding this comment.
Shouldn't the cdiRequiredIfAvailable flag be ignored if usableBeanManager is non-null (and thus CDI is already available)?
E.g.:
| if ( !cdiRequiredIfAvailable ) { | |
| if ( !cdiRequiredIfAvailable && usableBeanManager == null ) { |
That way if the caller is lucky and CDI was already initialized, they can get a CDI bean.
| Class<B> beanType, | ||
| LifecycleOptions lifecycleOptions, | ||
| BeanInstanceProducer fallbackProducer) { | ||
| return getBean( beanType, lifecycleOptions, fallbackProducer, false ); |
There was a problem hiding this comment.
This changes the default behavior, though... Including for Hibernate Search in the "delayed" case. I'm not sure of the implications.
Since you're (likely) always going to call getBean() with the explicit cdiRequiredIfAvailable flag yourself, can you please preserve the legacy behavior for this method? Just for backwards compatibility of integrations?
| Class<B> beanType, | ||
| LifecycleOptions lifecycleOptions, | ||
| BeanInstanceProducer fallbackProducer, | ||
| boolean cdiRequiredIfAvailable) { |
There was a problem hiding this comment.
Maybe another name would be better, since BeanContainer can be used for non-CDI implementations? E.g. beanContainerRequiredIfAvailable or something like that.
| fallbackBeanInstanceProducer, | ||
| true |
There was a problem hiding this comment.
I can say for sure that existing integrators using this API (i.e. Hibernate Search) do not, at the moment, use true.
Since these tests are specifically about checking compatibility with Hibernate Search, it's probably better to keep calling the old method without the cdiRequiredIfAvailable flag?
This form circumvents CDI hosting of UserType impls when extended or delayed strategies are used.
Alternative to #5992