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 <yo...@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
> yo...@hibernate.org
>
> On 14 December 2017 at 18:42, Steve Ebersole <st...@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 <st...@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 <yo...@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
>>>> yo...@hibernate.org
>>>>
>>>> On 14 December 2017 at 14:46, Steve Ebersole <st...@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 <st...@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 <yo...@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
>>>>>>> yo...@hibernate.org
>>>>>>>
>>>>>>> On 14 December 2017 at 14:38, Steve Ebersole <st...@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 <st...@hibernate.org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Its easier to cleanup
>>>>>>>>>
>>>>>>>>> On Thu, Dec 14, 2017 at 6:52 AM Steve Ebersole <
>>>>>>>>> st...@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 <
>>>>>>>>>> st...@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 <
>>>>>>>>>>> st...@hibernate.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Ah, nm.  They change the artifact name.  Boo!
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, Dec 14, 2017 at 6:39 AM Steve Ebersole <
>>>>>>>>>>>> st...@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 <
>>>>>>>>>>>>> st...@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 <
>>>>>>>>>>>>>> yo...@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
>>>>>>>>>>>>>>> yo...@hibernate.org
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 14 December 2017 at 08:01, Christian Beikov <
>>>>>>>>>>>>>>> christian.bei...@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@lists.jboss.org
>>>>>>>>>>>>>>> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>>>>>>>>>> >
>>>>>>>>>>>>>>> > _______________________________________________
>>>>>>>>>>>>>>> > hibernate-dev mailing list
>>>>>>>>>>>>>>> > hibernate-dev@lists.jboss.org
>>>>>>>>>>>>>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>>>>>>>>>> >
>>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>>> hibernate-dev mailing list
>>>>>>>>>>>>>>> hibernate-dev@lists.jboss.org
>>>>>>>>>>>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>
>>>>
>
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Reply via email to