MyFaces Core
  1. MyFaces Core
  2. MYFACES-1999

In GuiceResolver managed-bean-scope is ignored, scope defaults to none

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.2.4
    • Fix Version/s: None
    • Component/s: JSR-252
    • Labels:
      None
    • Environment:
      Tomcat 6

      Description

      The problem with the current org.apache.myfaces.el.unified.resolver.GuiceResolver is that it doesn't take into account the managed-bean-scope from the ManagedBean configuration. Typically you end up with multiple instances of the managed bean (a new one every time the resolver is called) – ie, same as a managed-bean-scope of "none".

      Note, there is some code in the resolver like this:
      if (ectx == null ||
      ectx.getRequestMap().containsKey(property) ||
      ectx.getSessionMap().containsKey(property) ||
      ectx.getApplicationMap().containsKey(property) )
      return null;
      This works if there is already a request, session or application scope attribute with the given name, but if not, the managed bean is not added to the scope. You can see that the overriden "getValue" method from the ManagedBeanResolver was actually responsible for putting any new object into the scope via the "putInScope" method.

      The simplest way to fix this in the code seems to be to make the ManagedBeanResolver.putInScope method "protected" (rather than private), and then call it from the GuiceResolver.getValue method whenever a new value is obtained from the injector.

      Meanwhile, a work-around is to leverage the guice-servlet module and then to specify the scope in the guice module, something like this:
      bind(HelloWorldBacking.class).in(ServletScopes.REQUEST);
      However, for that you have to list all your managed beans in two places (faces-config.xml as well as your guice module), and also have to add the com.google.inject.servlet.GuiceFilter filter class to your web app.

        Activity

        Hide
        Leonardo Uribe added a comment -

        The actual implementation of GuiceResolver theorically is correct. Guice has a module for servlets (inside guice-1.0.zip distribution there is a jar called guice-servlet.jar), so anyone can use it and integrate with jsf web applications. From this point of view, myfaces GuiceResolver just gives a start point.

        This issue could be seen as an enhancement, but after a lot of time thinking about it I'm not very sure about modify its semantic due to backwards compatibility.

        One way to solve this is create a GuiceScopedResolver that handle this case (I'm not sure if this is really necessary). For do that, it is necessary to create some other classes used for infrastructure, and use the strategy 3 (the last on the previous comment, define beans on faces-config.xml).

        Show
        Leonardo Uribe added a comment - The actual implementation of GuiceResolver theorically is correct. Guice has a module for servlets (inside guice-1.0.zip distribution there is a jar called guice-servlet.jar), so anyone can use it and integrate with jsf web applications. From this point of view, myfaces GuiceResolver just gives a start point. This issue could be seen as an enhancement, but after a lot of time thinking about it I'm not very sure about modify its semantic due to backwards compatibility. One way to solve this is create a GuiceScopedResolver that handle this case (I'm not sure if this is really necessary). For do that, it is necessary to create some other classes used for infrastructure, and use the strategy 3 (the last on the previous comment, define beans on faces-config.xml).
        Leonardo Uribe made changes -
        Field Original Value New Value
        Issue Type Bug [ 1 ] Improvement [ 4 ]
        Hide
        Tony Robertson added a comment -

        An alternative (or complimentary) approach is to resolve the property name based on the available guice bindings that have been annotated with the same name. For example:

        Object value = null;
        for (Key<?> key : injector.getBindings().keySet()) {
        Annotation ann = key.getAnnotation();
        if (ann instanceof Named
        && managedBeanName.equals(((Named) ann).value()))

        { value = injector.getInstance(key); log.info("Instantiated " + managedBeanName + " using binding " + key + ", new instance is (" + value.getClass().getName() + ") " + value); break; }

        }

        (this could be cached to make it more efficient for repeated lookup).
        This has the advantage that you don't even have to define your managed beans in faces-config.xml for them to be available to your EL resolver. Just use the "annotatedWith" when defining the guice module.

        If you do still want to define them in faces-config.xml (perhaps for migration support, or for tool support), a neat way to combine would be to define a guice module as shown below. That way, any beans defined in the faces-confg don't need to be manually added to a guice module, and any guice managed class can also have faces beans injected by using @Inject @Named("myfacebean") ...
        Here is a first cut for the guice module:
        ---------------------------------
        package au.com.saltgroup.web.support;

        import javax.servlet.ServletContext;

        import org.apache.myfaces.config.RuntimeConfig;
        import org.apache.myfaces.config.element.ManagedBean;

        import com.google.inject.AbstractModule;
        import com.google.inject.Scopes;
        import com.google.inject.TypeLiteral;
        import com.google.inject.binder.ScopedBindingBuilder;
        import com.google.inject.name.Names;
        import com.google.inject.servlet.ServletScopes;

        public class FacesManagedBeansGuiceModule extends AbstractModule {

        private RuntimeConfig runtimeConfig;

        public FacesManagedBeansGuiceModule(ServletContext context)

        { runtimeConfig = (RuntimeConfig) context .getAttribute(RuntimeConfig.class.getName()); }

        @Override
        protected void configure() {
        for (ManagedBean mb : runtimeConfig.getManagedBeans().values()) {
        @SuppressWarnings("unchecked")
        ScopedBindingBuilder sbb = bind(mb.getManagedBeanClass())
        .annotatedWith(Names.named(mb.getManagedBeanName())).to(
        TypeLiteral.get(mb.getManagedBeanClass()));
        String s = mb.getManagedBeanScope();
        if ("application".equals(s))

        { sbb.in(Scopes.SINGLETON); }

        else if ("session".equals(s))

        { sbb.in(ServletScopes.SESSION); }

        else if ("request".equals(s))

        { sbb.in(ServletScopes.REQUEST); }

        }
        }
        }

        Show
        Tony Robertson added a comment - An alternative (or complimentary) approach is to resolve the property name based on the available guice bindings that have been annotated with the same name. For example: Object value = null; for (Key<?> key : injector.getBindings().keySet()) { Annotation ann = key.getAnnotation(); if (ann instanceof Named && managedBeanName.equals(((Named) ann).value())) { value = injector.getInstance(key); log.info("Instantiated " + managedBeanName + " using binding " + key + ", new instance is (" + value.getClass().getName() + ") " + value); break; } } (this could be cached to make it more efficient for repeated lookup). This has the advantage that you don't even have to define your managed beans in faces-config.xml for them to be available to your EL resolver. Just use the "annotatedWith" when defining the guice module. If you do still want to define them in faces-config.xml (perhaps for migration support, or for tool support), a neat way to combine would be to define a guice module as shown below. That way, any beans defined in the faces-confg don't need to be manually added to a guice module, and any guice managed class can also have faces beans injected by using @Inject @Named("myfacebean") ... Here is a first cut for the guice module: --------------------------------- package au.com.saltgroup.web.support; import javax.servlet.ServletContext; import org.apache.myfaces.config.RuntimeConfig; import org.apache.myfaces.config.element.ManagedBean; import com.google.inject.AbstractModule; import com.google.inject.Scopes; import com.google.inject.TypeLiteral; import com.google.inject.binder.ScopedBindingBuilder; import com.google.inject.name.Names; import com.google.inject.servlet.ServletScopes; public class FacesManagedBeansGuiceModule extends AbstractModule { private RuntimeConfig runtimeConfig; public FacesManagedBeansGuiceModule(ServletContext context) { runtimeConfig = (RuntimeConfig) context .getAttribute(RuntimeConfig.class.getName()); } @Override protected void configure() { for (ManagedBean mb : runtimeConfig.getManagedBeans().values()) { @SuppressWarnings("unchecked") ScopedBindingBuilder sbb = bind(mb.getManagedBeanClass()) .annotatedWith(Names.named(mb.getManagedBeanName())).to( TypeLiteral.get(mb.getManagedBeanClass())); String s = mb.getManagedBeanScope(); if ("application".equals(s)) { sbb.in(Scopes.SINGLETON); } else if ("session".equals(s)) { sbb.in(ServletScopes.SESSION); } else if ("request".equals(s)) { sbb.in(ServletScopes.REQUEST); } } } }
        Tony Robertson created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Tony Robertson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development