[hibernate-dev] ManagedBeanRegistry - dependency injection support

Thu Dec 21 11:00:21 EST 2017

Yep, I saw it.  Just not sure I agree that this is not enough.

On Thu, Dec 21, 2017 at 9:42 AM Yoann Rodiere <yoann at hibernate.org> wrote:

> > However, I do think that there is still a need to expand the proposal
> Scott and I want to make to the CDI spec wrt something like our
> ExtendedBeanManager to also account for the shutdown phase
>
> +1, I sent an email about just that on the mailing list. There are some
> drawbacks to this approach though, and Sanne suggested deeper integration
> into CDI would be a more future-proof path.
> The subject of this thread was "CDI integration in Hibernate ORM and the
> Application scope".
>
> Yoann Rodière
> Hibernate NoORM Team
> yoann at hibernate.org
>
> On 21 December 2017 at 16:36, Steve Ebersole <steve at hibernate.org> wrote:
>
>> Awesome!  Glad it worked out.
>>
>> However, I do think that there is still a need to expand the proposal
>> Scott and I want to make to the CDI spec wrt something like our
>> ExtendedBeanManager to also account for the shutdown phase.  In addition to
>> knowing when the BeanManager is ready to use, it would be nice to know when
>> the BeanManager is ready to shutdown (a before shutdown hook).  At that
>> point we could begin cleaning up our CreationalContext, etc refs.
>>
>> Scott, do you already have enough insight into that in WildFly for us to
>> go ahead and do that in our integration today?
>>
>> On Thu, Dec 21, 2017, 1:53 AM Yoann Rodiere <yoann at hibernate.org> wrote:
>>
>>> 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
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>
>