[seam-dev] Fwd: [seam-commits] Seam SVN: r12493 - in modules/faces/trunk/impl/src/main: resources/META-INF/services and 1 other directory.
Dan Allen
dan.j.allen at gmail.com
Wed Apr 21 16:07:25 EDT 2010
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/seam-dev/attachments/20100421/3227273b/attachment-0001.html
More information about the seam-dev
mailing list