Tuesday, January 27, 2009

JSF Component Binding Stinks

Whoever came up with JSF component binding should be taken out back. While a nice thought, it really is just a purely awful piece of design and was not thought through at all.

Too harsh? Well, it sounds harsh, but when you consider all the flaws and bugs it causes, it isn't.

1. The problem

I have been telling people on the MyFaces mailing list to always ensure that they use binding with request scope beans, but I have realized that this advice was now bad. My advice should have been to never use binding at all, or to at least only use a setter, but no getter.

The reason behind this, is that there is a fundemental flaw in how binding was designed. Binding is too completely separate pieces of functionality, jammed together in one implementation:

  1. Component factory
  2. Component access from code

The latter is what people normally use binding for. This is how the framework uses binding in a JSP component tag:

  1. Does the tag have the binding attribute set?
    1. If so, call the getter method
      1. If returns not null, use that value
      2. Else, create a new component
        1. Call the setter with the new component

What you now see is that the getter method acts as both a normal JavaBean property getter as well as a factory. The problem is that 90% of the time, the lifetime of this member variable is too long. Almost always, the component in the bean should only be ever stored as long as the view is being processed. Take this example:

Developer has a managed bean with a request scope with a bound component. The page navigates from view1 to view2.

Jspx Code:

  <?xml version='1.0' encoding='utf-8'?>
  <jsp:root
    version="2.1"
    xmlns:f="http://java.sun.com/jsf/core"
    xmlns:h="http://java.sun.com/jsf/html"
    xmlns:jsp="http://java.sun.com/JSP/Page">
    <jsp:directive.page contentType="text/html;charset=utf-8"/>
    <f:view>
      <html>
        <body>
          <h:form>
            <h:outputText binding="#{myBean.textComponent}" />
          </h:form>
        </body>
      </html>
    </f:view>
  </jsp:root>

Now the views would probably be different, but it is not unusual for someone to reuse #{myBean.textComponent} in the page. Now what happens is that view1 causes the #{myBean.textComponent} to be set since it would return null on the restore view. View 2 is navigated to, but the #{myBean.textComponent} returns the component from view 1, so it is not re-created. Now we have an illegal state, a component is shared between two views.

There could have been two ways of fixing this:

  1. There could have been a create or factory attribute on the JSP tag and only that attribute could ever create the component and the set method on the binding would be used, but never the getter method.
  2. The JSP tag component creation should be smart enough to always check that a component is never shared across views. Therefore, if a binding expression returns a component that already has a parent and by looking up the tree, the view root is not the current view root, a new component should have been created.

So what is the best work around for this problem?

Well the best solution is to never use binding under any circumstances, ever, no exceptions. This is the only safe way to write JSF pages. If your bean needs to access a component, use findComponent or invokeOnComponent to get the component.

The other alternative is to check the view root yourself in you bean when the getter is called. If the view has changed, return null. Example:

  package blog;
  
  import java.util.Map;
  
  import javax.faces.component.UIComponent;
  import javax.faces.component.UIViewRoot;
  import javax.faces.context.FacesContext;
  
  public class MyBean
  {
    public void setMyComponent(UIComponent myComponent)
    {
      this.myComponent = myComponent;
    }
  
    public UIComponent getMyComponent()
    {
      checkViewRoot();
      return this.myComponent;
    }
  
    private void checkViewRoot()
    {
      UIViewRoot current = FacesContext.getCurrentInstance().getViewRoot();
      if (this.viewRoot == null || (current != null && current != this.viewRoot))
      {
        this.viewRoot = current;
        // view is new or has changed, make sure the component is not re-used
        this.myComponent = null;
      }
    }
  
    private UIComponent myComponent;
    private UIViewRoot viewRoot;
  }

Ugly? Bad performance? Yes! Unfortunately this may be the best solution if you have to use component binding. From what I have seen from JSF 2.0 I think there may be factory support, so that component binding to be able to gain access to a component in a bean is separated from component creation. I hope so. Until then, I strongly recommend that you forget that the binding attribute exists at all.

3 comments:

Rafael Ponte said...

I really haven't seen developers sharing component-bindings between pages. At least I don't that.

The most cases I saw they just share simple properties [no components] from the managed bean during navigation between pages.

Well, I think that this problem doesn't worth we leave using component bindings.

I believe that if we use an approach like "State-Driven Navigation" we can avoid problems as this cited for you.

Great post!

Unknown said...

I agree that bindings are not perfect, but you have to bind them to get some nifty features to work. So I disagree with your never use at all advice, but I strongly agree about the no getter. However, since you need a getter with EL (EL doesn't support write only properties), we simply have a guideline saying that the getter must always return null when used for component bindings.

Sankar said...

Great explanation with appropriate example.


sankar
java training in chennai