[seam-dev] Fwd: [seam-commits] Seam SVN: r12493 - in modules/faces/trunk/impl/src/main: resources/META-INF/services and 1 other directory.
Nicklas Karlsson
nickarls at gmail.com
Thu Apr 22 07:16:50 EDT 2010
I just extracted it from the two JSF event bridges that happened to need it.
And a week ago you said "I like the super class approach, it makes the
ugliness of it apparent nicely :-)" when I suggested the static helper
On Thu, Apr 22, 2010 at 1:45 PM, Pete Muir <pmuir at redhat.com> wrote:
> Nik, why did you do it as a superclass in the first place?
>
> On 21 Apr 2010, at 21:38, Dan Allen wrote:
>
> > On Wed, Apr 21, 2010 at 4:22 PM, Lincoln Baxter, III <
> lincolnbaxter at gmail.com> wrote:
> > 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.)
> >
> > You could still have BeanManagerAware I suppose...it would just be
> another way to get the BeanManager. You can either do the static call, or
> you can inherit and have the static call done for you.
> >
> > public class NonCDIBean extends BeanManagerAware {
> >
> > public void doSomething() {
> > BeanManager beanManager = getBeanManager();
> > ...
> > }
> > }
> >
> > vs
> >
> > public class NonCDIBean {
> >
> > public void doSomething() {
> > BeanManager beanManager = BeanManagerLocator.getBeanManager();
> > }
> > }
> >
> > Keep in mind, these non-CDI beans are not in the end developers code,
> this is internal extension stuff, for example the SystemEventListener and
> PhaseListener implementations for CDI event bridging.
> >
> > As for the locator, we are currently refining the strategy and will
> submit a formal proposal for review in a separate thread.
> >
> > -Dan
> >
> >
> > 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"
> >
> >
> >
> > --
> > 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
>
>
> _______________________________________________
> seam-dev mailing list
> seam-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/seam-dev
>
--
---
Nik
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/seam-dev/attachments/20100422/0bb19c04/attachment-0001.html
More information about the seam-dev
mailing list