[seam-dev] Fwd: [seam-commits] Seam SVN: r12493 - in modules/faces/trunk/impl/src/main: resources/META-INF/services and 1 other directory.
Lincoln Baxter, III
lincolnbaxter at gmail.com
Wed Apr 21 16:22:27 EDT 2010
I'm in agreement with this train of logic.
BeanManagerLocator sounds fine to me. BeanManagerAware may then go away
since this would be providing a utility class (not a superclass.)
--Lincoln
On Wed, Apr 21, 2010 at 4:07 PM, Dan Allen <dan.j.allen at gmail.com> wrote:
> On Wed, Apr 21, 2010 at 3:51 PM, Dan Allen <dan.j.allen at gmail.com> wrote:
>
>>
>>> On Thu, Apr 15, 2010 at 6:14 AM, Pete Muir <pmuir at redhat.com> wrote:
>>>
>>>> Guys,
>>>>
>>>> There still seems to be some confusion about why this is an issue that
>>>> we *must* address. Taking a step back, we have some general requirements for
>>>> Seam 3:
>>>>
>>>> * that we want to use the Java SE platform
>>>> * that we also want to use "app servers"
>>>> * that we want these extensions to be portable not just across Java EE
>>>> containers but across Servlet containers too
>>>> * that we want this to work in more esoteric deployment scenarios than a
>>>> single-war-per-server-instance OR bundling the extension as a library of the
>>>> war OR using special tricks like deployers in JBoss AS
>>>>
>>>> Then there is *no* general solution to this problem that I know of (I'm
>>>> not just not telling people what the solution because I'm some sort of
>>>> sadist ;-). Instead, we have a number of partial solutions, which, when
>>>> combined should get us the whole way there.
>>>>
>>>> I see that I was wrong to not put this in Weld Extensions from the
>>>> beginning. We can take Nik's basic design, add service providers into the
>>>> mix (to make this extensible and remove the hard dep on the servlet
>>>> container) which should at least mean we don't reinvent a square wheel many
>>>> times ;-).
>>>>
>>>> However, it is still *extremely* important that each time you can't just
>>>> inject the BeanManager you take a step back, and consider how you need to
>>>> access the BeanManager. For example, it is infinitely better to push the
>>>> BeanManager into some sort of locally propagated context than to resort to
>>>> lookup techniques. There is no guarantee that lookup techniques will work
>>>> everywhere, whilst a actively propagating the BeanManager will.
>>>>
>>>> So, before you extend BeanManagerAware, please please consider whether
>>>> there is a better option. It is a measure of last resort *only*. Often
>>>> people on the dev list will have a good idea about how to get it, so please
>>>> ask.
>>>>
>>>> I will add these notes to the lookup class. Before we do release
>>>> candidates of a module, you can expect the BeanManagerAware police to visit
>>>> and possibly demand changes too ;-)
>>>>
>>>> On 14 Apr 2010, at 20:53, Nicklas Karlsson wrote:
>>>>
>>>> > I think the real issue was the singleton extension...
>>>> >
>>>> > On Wed, Apr 14, 2010 at 10:39 PM, Lincoln Baxter, III <
>>>> lincolnbaxter at gmail.com> wrote:
>>>> > The statics are only used instead of new BeanManagerProvider{} ...
>>>> they are not a problem and can go away. Is there another complaint?
>>>> >
>>>> >
>>>> > On Wed, Apr 14, 2010 at 2:59 PM, Nicklas Karlsson <nickarls at gmail.com>
>>>> wrote:
>>>> > Reverting & repenting
>>>> >
>>>> > It's for the JSF -> CDI event bridge. I need a BeanManager to
>>>> propagate the events to but the problem is that the first JSF global system
>>>> event is a PostApplicationConstructed which is pretty soon.
>>>> >
>>>> > First I try to look for it in a servlet context attribute (which the
>>>> seam-servlet puts there but it's not there yet in this case)
>>>> > Then I try to look in JNDI (sure, it's there but you recommended we
>>>> should try to avoid this to make this available in as many places as
>>>> possible)
>>>> > Then I tried the static one as a last effort.
>>>> >
>>>> >
>>>> > On Wed, Apr 14, 2010 at 5:54 PM, Pete Muir <pmuir at redhat.com> wrote:
>>>> > Guys,
>>>> >
>>>> > We cannot rely on statics in Seam 3. Please don't add code like this.
>>>> If you have a problem accessing the BeanManager from your code, please ask
>>>> here about the best solution.
>>>> >
>>>> > Nik, can you revert this please. And then explain what situation you
>>>> are trying to address.
>>>> >
>>>> > Thanks.
>>>> >
>>>> > Begin forwarded message:
>>>> >
>>>> > > From: seam-commits at lists.jboss.org
>>>> > > Date: 14 April 2010 13:50:21 GMT+01:00
>>>> > > To: seam-commits at lists.jboss.org
>>>> > > Subject: [seam-commits] Seam SVN: r12493 - in
>>>> modules/faces/trunk/impl/src/main: resources/META-INF/services and 1 other
>>>> directory.
>>>> > > Reply-To: seam-commits at lists.jboss.org
>>>> > >
>>>> > > Author: nickarls
>>>> > > Date: 2010-04-14 08:50:21 -0400 (Wed, 14 Apr 2010)
>>>> > > New Revision: 12493
>>>> > >
>>>> > > Added:
>>>> > >
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/BeanManagerPickupExtension.java
>>>> > >
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/SingletonBeanManagerProvider.java
>>>> > > Modified:
>>>> > >
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/BeanManagerAware.java
>>>> > >
>>>> modules/faces/trunk/impl/src/main/resources/META-INF/services/javax.enterprise.inject.spi.Extension
>>>> > > Log:
>>>> > > Last attempt singletonish approach when others fail. Yes, I'm
>>>> ashamed.
>>>> > >
>>>> > > Modified:
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/BeanManagerAware.java
>>>> > > ===================================================================
>>>> > > ---
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/BeanManagerAware.java
>>>> 2010-04-14 12:21:26 UTC (rev 12492)
>>>> > > +++
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/BeanManagerAware.java
>>>> 2010-04-14 12:50:21 UTC (rev 12493)
>>>> > > @@ -45,6 +45,7 @@
>>>> > >
>>>> beanManagerProviders.add(ServletContextBeanManagerProvider.DEFAULT);
>>>> > > beanManagerProviders.add(JndiBeanManagerProvider.DEFAULT);
>>>> > > beanManagerProviders.add(JndiBeanManagerProvider.JBOSS_HACK);
>>>> > > +
>>>> beanManagerProviders.add(SingletonBeanManagerProvider.DEFAULT);
>>>> > > }
>>>> > >
>>>> > > protected BeanManager getBeanManager()
>>>> > >
>>>> > > Added:
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/BeanManagerPickupExtension.java
>>>> > > ===================================================================
>>>> > > ---
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/BeanManagerPickupExtension.java
>>>> (rev 0)
>>>> > > +++
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/BeanManagerPickupExtension.java
>>>> 2010-04-14 12:50:21 UTC (rev 12493)
>>>> > > @@ -0,0 +1,56 @@
>>>> > > +/*
>>>> > > + * JBoss, Home of Professional Open Source
>>>> > > + * Copyright 2010, Red Hat, Inc., and individual contributors
>>>> > > + * by the @authors tag. See the copyright.txt in the distribution
>>>> for a
>>>> > > + * full listing of individual contributors.
>>>> > > + *
>>>> > > + * This is free software; you can redistribute it and/or modify it
>>>> > > + * under the terms of the GNU Lesser General Public License as
>>>> > > + * published by the Free Software Foundation; either version 2.1 of
>>>> > > + * the License, or (at your option) any later version.
>>>> > > + *
>>>> > > + * This software is distributed in the hope that it will be useful,
>>>> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>>> > > + * Lesser General Public License for more details.
>>>> > > + *
>>>> > > + * You should have received a copy of the GNU Lesser General Public
>>>> > > + * License along with this software; if not, write to the Free
>>>> > > + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
>>>> MA
>>>> > > + * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
>>>> > > + */
>>>> > > +package org.jboss.seam.faces.cdi;
>>>> > > +
>>>> > > +import javax.enterprise.event.Observes;
>>>> > > +import javax.enterprise.inject.spi.AfterBeanDiscovery;
>>>> > > +import javax.enterprise.inject.spi.BeanManager;
>>>> > > +import javax.enterprise.inject.spi.Extension;
>>>> > > +
>>>> > > +/**
>>>> > > + * Singleton(ish) extension that observes the AfterBeanDiscovery
>>>> event and stores the BeanManager for access
>>>> > > + * in places where injection is not available and JNDI or
>>>> ServletContext access is not preferable.
>>>> > > + *
>>>> > > + * @author Nicklas Karlsson
>>>> > > + *
>>>> > > + */
>>>> > > +public class BeanManagerPickupExtension implements Extension
>>>> > > +{
>>>> > > + private static BeanManagerPickupExtension instance;
>>>> > > + private volatile BeanManager beanManager;
>>>> > > +
>>>> > > + public BeanManager getBeanManager()
>>>> > > + {
>>>> > > + return beanManager;
>>>> > > + }
>>>> > > +
>>>> > > + public static BeanManagerPickupExtension getInstance()
>>>> > > + {
>>>> > > + return instance;
>>>> > > + }
>>>> > > +
>>>> > > + public void pickupBeanManager(@Observes AfterBeanDiscovery e,
>>>> BeanManager beanManager)
>>>> > > + {
>>>> > > + this.beanManager = beanManager;
>>>> > > + BeanManagerPickupExtension.instance = this;
>>>> > > + }
>>>> > > +}
>>>> > >
>>>> > > Added:
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/SingletonBeanManagerProvider.java
>>>> > > ===================================================================
>>>> > > ---
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/SingletonBeanManagerProvider.java
>>>> (rev 0)
>>>> > > +++
>>>> modules/faces/trunk/impl/src/main/java/org/jboss/seam/faces/cdi/SingletonBeanManagerProvider.java
>>>> 2010-04-14 12:50:21 UTC (rev 12493)
>>>> > > @@ -0,0 +1,43 @@
>>>> > > +/*
>>>> > > + * JBoss, Home of Professional Open Source
>>>> > > + * Copyright 2010, Red Hat, Inc., and individual contributors
>>>> > > + * by the @authors tag. See the copyright.txt in the distribution
>>>> for a
>>>> > > + * full listing of individual contributors.
>>>> > > + *
>>>> > > + * This is free software; you can redistribute it and/or modify it
>>>> > > + * under the terms of the GNU Lesser General Public License as
>>>> > > + * published by the Free Software Foundation; either version 2.1 of
>>>> > > + * the License, or (at your option) any later version.
>>>> > > + *
>>>> > > + * This software is distributed in the hope that it will be useful,
>>>> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>>> > > + * Lesser General Public License for more details.
>>>> > > + *
>>>> > > + * You should have received a copy of the GNU Lesser General Public
>>>> > > + * License along with this software; if not, write to the Free
>>>> > > + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
>>>> MA
>>>> > > + * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
>>>> > > + */
>>>> > > +package org.jboss.seam.faces.cdi;
>>>> > > +
>>>> > > +import javax.enterprise.inject.spi.BeanManager;
>>>> > > +
>>>> > > +/**
>>>> > > + * A BeanManager provider for an extension provided singleton
>>>> > > + *
>>>> > > + * @author Nicklas Karlsson
>>>> > > + *
>>>> > > + */
>>>> > > +public class SingletonBeanManagerProvider implements
>>>> BeanManagerProvider
>>>> > > +{
>>>> > > +
>>>> > > + public static final BeanManagerProvider DEFAULT = new
>>>> SingletonBeanManagerProvider();
>>>> > > +
>>>> > > + @Override
>>>> > > + public BeanManager getBeanManager()
>>>> > > + {
>>>> > > + return
>>>> BeanManagerPickupExtension.getInstance().getBeanManager();
>>>> > > + }
>>>> > > +
>>>> > > +}
>>>> > >
>>>> > > Modified:
>>>> modules/faces/trunk/impl/src/main/resources/META-INF/services/javax.enterprise.inject.spi.Extension
>>>> > > ===================================================================
>>>> > > ---
>>>> modules/faces/trunk/impl/src/main/resources/META-INF/services/javax.enterprise.inject.spi.Extension
>>>> 2010-04-14 12:21:26 UTC (rev 12492)
>>>> > > +++
>>>> modules/faces/trunk/impl/src/main/resources/META-INF/services/javax.enterprise.inject.spi.Extension
>>>> 2010-04-14 12:50:21 UTC (rev 12493)
>>>> > > @@ -1,3 +1,4 @@
>>>> > > org.jboss.seam.faces.context.ViewScopedExtension
>>>> > > org.jboss.seam.faces.context.FlashScopedExtension
>>>> > > -org.jboss.seam.faces.context.FacesAnnotationsAdapterExtension
>>>> > > \ No newline at end of file
>>>> > > +org.jboss.seam.faces.context.FacesAnnotationsAdapterExtension
>>>> > > +org.jboss.seam.faces.cdi.BeanManagerPickupExtension
>>>> > >
>>>>
>>>
>> On Thu, Apr 15, 2010 at 10:51 AM, Lincoln Baxter, III <
>> lincolnbaxter at gmail.com> wrote:
>>
>> I like what Nik has done so far (in terms of the chain of responsibility
>>> pattern for various BM lookups.) So keeping that, let's talk about what else
>>> we need to provide the missing lookup functionality (that we can identify.)
>>>
>>> What's there now is good for a start.
>>>
>>
>> Funny that I implemented almost the same approach a year ago. Only, it had
>> two major shortcomings:
>>
>> a) I was lazy and just used the Java 6 service loader API
>> b) I used bad names for things (I called it a bridge API and that gave
>> Pete chills :))
>>
>>
>>> I agree -- BeanManagerAware is ONLY for places where you absolutely
>>> cannot access the beanmanager in a proper fashion, or cannot achieve
>>> injection in the proper fashion. This should be very clearly stated -- I'll
>>> update the docs.
>>
>>
>> I'm in tune with Pete and Lincoln that the locator should be used when
>> there is no other option. Yet, when it is used, it needs to avoid assuming
>> anything about the deployment environment (i.e., be pluggable).
>>
>> With that said, I think we should spend time getting the naming right.
>> BeanManagerAware just doesn't sound right to me. First of all, it sounds
>> like something in Spring. And second, it's not making the bean aware of
>> anything, it's a lookup mechanism. I'll throw in some proposals, but feel
>> free to refine:
>>
>> BeanManagerLookup#getBeanManager()
>> BeanManagerContext#getBeanManager()
>> BeanManagerContext#lookupBeanManager()
>> BeanManagerLocator#getBeanManager()
>>
>> We should avoid the use of statics to hold values too, because that can
>> cause real problems in certain deployment scenarios. I would think using a
>> ThreadLocal would be better.
>>
>> I didn't realize the BeanManagerAware was an abstract super class. In that
> context, the name makes sense. But I don't think it should be doing to
> resolution. The resolution itself should be handled in a separate class that
> BeanManagerAware uses. That way, we don't mix the two concerns.
>
> -Dan
>
> --
> Dan Allen
> Senior Software Engineer, Red Hat | Author of Seam in Action
> Registered Linux User #231597
>
> http://mojavelinux.com
> http://mojavelinux.com/seaminaction
> http://www.google.com/profiles/dan.j.allen
>
--
Lincoln Baxter, III
http://ocpsoft.com
http://scrumshark.com
"Keep it Simple"
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/seam-dev/attachments/20100421/907fa475/attachment-0001.html
More information about the seam-dev
mailing list