[hibernate-dev] ManagedBeanRegistry - dependency injection support

Thu Dec 21 03:53:07 EST 2017

Hi,

Following our conversations on HipChat and the various changes you
implemented (thanks!), I tested the current implementation. There were a
few issues, but I managed to fix them and make all the tests in Hibernate
Search pass.
Here is a PR with the fixes:
https://github.com/hibernate/hibernate-orm/pull/2092

Yoann Rodière
Hibernate NoORM Team
yoann at hibernate.org

On 14 December 2017 at 18:42, Steve Ebersole <steve at hibernate.org> wrote:

> Here is the commit with initial support for named CDI beans and support
> for bypassing registry caching of ManagedBeans : https://github.com/
> hibernate/hibernate-orm/commit/ddc1f03abc675a27ed025b8c00495d39bca7fb60
>
> There is still a question of whether named beans support needs to do
> the javax.enterprise.inject.spi.InjectionTarget stuff
>
> Christian, I ended up going to "beans" as the package name because this
> supports non-CDI environments (direct instantiation) too.  Not overly a fan
> of "beans" (overloaded term) but it was a lesser of evils.
>
> On Thu, Dec 14, 2017 at 10:57 AM Steve Ebersole <steve at hibernate.org>
> wrote:
>
>> Yoann, does this approach still need to do the injections
>> (javax.enterprise.inject.spi.InjectionTarget)?
>>
>>
>> On Thu, Dec 14, 2017 at 8:01 AM Yoann Rodiere <yoann at hibernate.org>
>> wrote:
>>
>>> Here is how it should work from what I understand (adapted from an
>>> implementation in Search, which has slightly different requirements):
>>>
>>> static <T> T getBeanInstance(BeanManager beanManager, String beanName,
>>> Class<T> contract) {
>>> Set<Bean<?>> beans = beanManager.getBeans(contract, new NamedQualifier(
>>> beanName));
>>> if ( beans.isEmpty() || beans.size() > 1 ) {
>>> // TODO proper error messages
>>> throw new IllegalArgumentException( "No matching beans or multiple
>>> matching beans" );
>>> }
>>> Bean<?> bean = beans.iterator().next();
>>> CreationalContext<T> creationalContext = beanManager.createCreationalContext(
>>> bean );
>>> return contract.cast( beanManager.getReference( bean, contract,
>>> creationalContext ) );
>>> }
>>>
>>> With NamedQualifier being the implementation I gave before.
>>>
>>> Sure, let's talk about it later on HipChat. Especially the caching
>>> thing, it's really a blocker for Search.
>>>
>>> I'll be online to travel in about 3 hours, though.
>>>
>>>
>>> Yoann Rodière
>>> Hibernate NoORM Team
>>> yoann at hibernate.org
>>>
>>> On 14 December 2017 at 14:46, Steve Ebersole <steve at hibernate.org>
>>> wrote:
>>>
>>>> I'll be on HipChat later after I get back from taking my son and
>>>> daughter to school.  Maybe it is easier to discuss there.
>>>>
>>>> On Thu, Dec 14, 2017 at 7:44 AM Steve Ebersole <steve at hibernate.org>
>>>> wrote:
>>>>
>>>>> But your answer above does not answer my question ;)
>>>>>
>>>>> I still have no idea how to go from name+Class -> bean.
>>>>>
>>>>>
>>>>> On Thu, Dec 14, 2017 at 7:41 AM Yoann Rodiere <yoann at hibernate.org>
>>>>> wrote:
>>>>>
>>>>>> Yeah, it was 4AM in France when you asked :) I answered later on
>>>>>> HipChat, the answer is basically the one I gave in my email.
>>>>>>
>>>>>> Yoann Rodière
>>>>>> Hibernate NoORM Team
>>>>>> yoann at hibernate.org
>>>>>>
>>>>>> On 14 December 2017 at 14:38, Steve Ebersole <steve at hibernate.org>
>>>>>> wrote:
>>>>>>
>>>>>>> WRT to named beans, I asked Guillaume on HipChat what that is
>>>>>>> supposed to look like.  IIRC he mentioned producers in Paris, but I found
>>>>>>> no straight-forward way to get from name+class to a bean.
>>>>>>>
>>>>>>> He may have answered, I just have not been on HipChat yet today...
>>>>>>>
>>>>>>> On Thu, Dec 14, 2017 at 7:36 AM Steve Ebersole <steve at hibernate.org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Its easier to cleanup
>>>>>>>>
>>>>>>>> On Thu, Dec 14, 2017 at 6:52 AM Steve Ebersole <steve at hibernate.org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> There are a lot of changes to digest here, but if anyone wanted to
>>>>>>>>> take a look at this so far...
>>>>>>>>>
>>>>>>>>> https://github.com/hibernate/hibernate-orm/commit/
>>>>>>>>> 564ec55ca10c0d5d2afd73243dc0aa31759e8f5b
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Thu, Dec 14, 2017 at 6:47 AM Steve Ebersole <
>>>>>>>>> steve at hibernate.org> wrote:
>>>>>>>>>
>>>>>>>>>> Actually my fault.  Apparently renaming the package was way too
>>>>>>>>>> aggressive and renamed the artifact
>>>>>>>>>>
>>>>>>>>>> On Thu, Dec 14, 2017 at 6:40 AM Steve Ebersole <
>>>>>>>>>> steve at hibernate.org> wrote:
>>>>>>>>>>
>>>>>>>>>>> Ah, nm.  They change the artifact name.  Boo!
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Dec 14, 2017 at 6:39 AM Steve Ebersole <
>>>>>>>>>>> steve at hibernate.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Anyone know what happened to the 2.0 CDI artifact on Maven
>>>>>>>>>>>> Central?  It was there last week, but is no longer there...
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, Dec 14, 2017 at 5:54 AM Steve Ebersole <
>>>>>>>>>>>> steve at hibernate.org> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for the replies.  So unless we hear otherwise from
>>>>>>>>>>>>> anyone else, I will plan on supporting just one DI container.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, Dec 14, 2017 at 2:54 AM Yoann Rodiere <
>>>>>>>>>>>>> yoann at hibernate.org> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Same here, compositions don't seem to be a reasonable use
>>>>>>>>>>>>>> case. And even if
>>>>>>>>>>>>>> users provide a custom bean registry, they could just
>>>>>>>>>>>>>> implement their
>>>>>>>>>>>>>> specific behavior for a few specific case, then retrieve
>>>>>>>>>>>>>> another
>>>>>>>>>>>>>> implementations on their own and delegate to it however they
>>>>>>>>>>>>>> want.
>>>>>>>>>>>>>> Overriding the service initiator looks like a very reasonable
>>>>>>>>>>>>>> way to do
>>>>>>>>>>>>>> that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Regarding the package, "org.hibernate.resource.beans" seems
>>>>>>>>>>>>>> more
>>>>>>>>>>>>>> appropriate to me, since CDI is not the only implementation
>>>>>>>>>>>>>> we will get and
>>>>>>>>>>>>>> we know it. Also, if I wanted to nitpick, injection is not
>>>>>>>>>>>>>> really something
>>>>>>>>>>>>>> the bean registry must provide. We could imagine a bean
>>>>>>>>>>>>>> registry without
>>>>>>>>>>>>>> any support for injection, after all, just providing
>>>>>>>>>>>>>> "monolithic beans". It
>>>>>>>>>>>>>> would still make sense with respect to your
>>>>>>>>>>>>>> ManagedBeanRegistry API.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yoann Rodière
>>>>>>>>>>>>>> Hibernate NoORM Team
>>>>>>>>>>>>>> yoann at hibernate.org
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 14 December 2017 at 08:01, Christian Beikov <
>>>>>>>>>>>>>> christian.beikov at gmail.com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> > I don't think someone is actually going to use more than a
>>>>>>>>>>>>>> single DI
>>>>>>>>>>>>>> > framework and even if they do, they will probably bridge
>>>>>>>>>>>>>> one way or
>>>>>>>>>>>>>> > another between the DI frameworks to be able to access
>>>>>>>>>>>>>> beans from one in
>>>>>>>>>>>>>> > the other.
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > So I don't think we should do "compositions" since it's not
>>>>>>>>>>>>>> a big deal
>>>>>>>>>>>>>> > to integrate different DIs and is also IMO an edge case.
>>>>>>>>>>>>>> I'd prefer the
>>>>>>>>>>>>>> > package name `org.hibernate.resource.di` since CDI seems to
>>>>>>>>>>>>>> be just one
>>>>>>>>>>>>>> > of the possible "integrations".
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > Mit freundlichen Grüßen,
>>>>>>>>>>>>>> > ------------------------------
>>>>>>>>>>>>>> ------------------------------------------
>>>>>>>>>>>>>> > *Christian Beikov*
>>>>>>>>>>>>>> > Am 13.12.2017 um 21:04 schrieb Steve Ebersole:
>>>>>>>>>>>>>> > > https://hibernate.atlassian.net/browse/HHH-11259 and
>>>>>>>>>>>>>> friends are mainly
>>>>>>>>>>>>>> > > about back porting the work I did on 6.0 for the
>>>>>>>>>>>>>> ManagedBeanRegistry
>>>>>>>>>>>>>> > > abstraction over dependency injection containers.  We
>>>>>>>>>>>>>> will ship support
>>>>>>>>>>>>>> > for
>>>>>>>>>>>>>> > > CDI as well as non-managed beans (things we directly
>>>>>>>>>>>>>> instantiate).  Of
>>>>>>>>>>>>>> > > course we'd ideally make it easy to plug in other DI
>>>>>>>>>>>>>> containers such as
>>>>>>>>>>>>>> > > Spring.  So I wanted to discuss the configuration of this
>>>>>>>>>>>>>> support.
>>>>>>>>>>>>>> > >
>>>>>>>>>>>>>> > > The first thing to consider is whether we want to support
>>>>>>>>>>>>>> using multiple
>>>>>>>>>>>>>> > DI
>>>>>>>>>>>>>> > > containers simultaneously.  E.g. is it conceivable that
>>>>>>>>>>>>>> an application
>>>>>>>>>>>>>> > > might want to use both CDI and Spring simultaneously?  I
>>>>>>>>>>>>>> started building
>>>>>>>>>>>>>> > > in support for that via a CompositeManagedBeanRegistry
>>>>>>>>>>>>>> implementation,
>>>>>>>>>>>>>> > but
>>>>>>>>>>>>>> > > stepping back I want to gauge whether that is
>>>>>>>>>>>>>> "reasonable" before
>>>>>>>>>>>>>> > > continuing down that path
>>>>>>>>>>>>>> > >
>>>>>>>>>>>>>> > > Assuming that we do want to support such "compositions"
>>>>>>>>>>>>>> the next question
>>>>>>>>>>>>>> > > is how we see this being configured.  Clearly any time a
>>>>>>>>>>>>>> CDI BeanManager
>>>>>>>>>>>>>> > is
>>>>>>>>>>>>>> > > present during bootstrap we want to enable CDI
>>>>>>>>>>>>>> ManagedBeanRegistry
>>>>>>>>>>>>>> > > support.  How would users indicate additional
>>>>>>>>>>>>>> ManagedBeanRegistry impls
>>>>>>>>>>>>>> > be
>>>>>>>>>>>>>> > > added to the CompositeManagedBeanRegistry?  I have
>>>>>>>>>>>>>> opinions about this,
>>>>>>>>>>>>>> > but
>>>>>>>>>>>>>> > > I'd like to hear other's thoughts...
>>>>>>>>>>>>>> > >
>>>>>>>>>>>>>> > > Note that ManagedBeanRegistry is a service and is
>>>>>>>>>>>>>> initiated
>>>>>>>>>>>>>> > > via org.hibernate.resource.cdi.spi.ManagedBeanRegistryInitiator.
>>>>>>>>>>>>>> So it
>>>>>>>>>>>>>> > > would be possible to completely redefine
>>>>>>>>>>>>>> ManagedBeanRegistry support
>>>>>>>>>>>>>> > simply
>>>>>>>>>>>>>> > > by replacing that initiator.
>>>>>>>>>>>>>> > >
>>>>>>>>>>>>>> > > A minor point... notice that the package name here is
>>>>>>>>>>>>>> > > `org.hibernate.resource.cdi`, even though one of the
>>>>>>>>>>>>>> goals here is to
>>>>>>>>>>>>>> > > support non-CDI ManagedBeanRegistry impls.  Do we want to
>>>>>>>>>>>>>> use a different
>>>>>>>>>>>>>> > > package name?  Maybe `org.hibernate.resource.beans`?
>>>>>>>>>>>>>> > >   ``org.hibernate.resource.di`?
>>>>>>>>>>>>>> ``org.hibernate.resource.injection`?
>>>>>>>>>>>>>> > > Other suggestions?  I'm actually ok with
>>>>>>>>>>>>>> `org.hibernate.resource.cdi` -
>>>>>>>>>>>>>> > imo
>>>>>>>>>>>>>> > > "cdi" conveys the proper intent.  But if others feel
>>>>>>>>>>>>>> strongly it should
>>>>>>>>>>>>>> > be
>>>>>>>>>>>>>> > > something else, I am open to hearing what and why.
>>>>>>>>>>>>>> > > _______________________________________________
>>>>>>>>>>>>>> > > hibernate-dev mailing list
>>>>>>>>>>>>>> > > hibernate-dev at lists.jboss.org
>>>>>>>>>>>>>> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > _______________________________________________
>>>>>>>>>>>>>> > hibernate-dev mailing list
>>>>>>>>>>>>>> > hibernate-dev at lists.jboss.org
>>>>>>>>>>>>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>> hibernate-dev mailing list
>>>>>>>>>>>>>> hibernate-dev at lists.jboss.org
>>>>>>>>>>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>
>>>