JSF Anti-Patterns and Pitfalls

Java Development News:

JSF Anti-Patterns and Pitfalls

By Dennis Byrne

01 Feb 2008 | TheServerSide.com

This article covers anti-patterns and pitfalls of day to day JSF development. Most of these issues have kept the author up at night; some of these are the same old challenges with a new face, pun intended. These challenges include performance, tight coupling, thread safety, security, interoperability and just plain ugliness.

The Validating Setter

Constructors are a good place to put validation logic for a domain model. In a perfect world every XML marshalling framework, object-relational mapping library, and dependency injection framework would support constructor injection - our domain models would not need setters for each field. As it turns out we do not live in a perfect world.

The JSF specification defines a dependency injection mechanism for managed beans: setter injection was defined, constructor injection was not. I understand this. JSF is a standard for java MVC web development, not dependency injection. Unfortunately this limitation gave rise to a whole slew of workarounds for application developers who want to perform domain validation and bean initialization logic.

The Validating Setter anti-pattern occurs when code that would normally be in the managed bean constructor is moved to the setter that would be invoked last by JSF dependency injection. How does one know which setter will be called last? The specification states that a JSF implementation must inject the dependencies of a managed bean in the order in which they are configured.

<managed-bean>
<managed-bean-name>iteration</managed-bean-name>
  <managed-bean-class>net.dbyrne.agile.Iteration</managed-bean-class>
  <managed-bean-scope>request</managed-bean-scope>
<managed-property> <!-- setStart called first -->
  <property-name>start</property-name>
  <value>#{projectBean.currentStart}</value>
  </managed-property>
  <managed-property>
  <property-name>end</property-name>
<value>#{projectBean.currentEnd}</value>
  </managed-property>
  <managed-property><!-- setLast called last -->
  <property-name>last</property-name>
  <value>hack</value>
</managed-property>
</managed-bean>

Below is the Iteration class. The author of this class wants the start and end fields to be mandatory – an Iteration without these is invalid. Furthermore, start should never follow end.

public class Iteration {

private Calendar start, end; // injected

// sans setters and getters for start, end

public void setLast(String last) {

    if(start == null || end == null)
   throw new NullPointerException("incomplete range");

     if(start.after(end))
   throw new IllegalStateException("start cannot be after end");

}

}

Let's look at solutions. Applications that are using JSF 1.2 can take advantage of the PostConstruct annotation. Below, the PostConstruct annotation instructs the JSF implementation to invoke the initialize method after the managed bean has been created.

public class Iteration {

private Calendar start, end; // injected

// sans setters and getters for start, end

@javax.annotation.PostConstruct
public void initialize() {

    if(start == null || end == null)
     throw new NullPointerException("incomplete range");

     if(start.after(end))
     throw new IllegalStateException("start cannot be after end");

}
}

The PostConstruct approach is sexy in a time when much of the java community is revolting against XML in favor of annotations. But it doesn't quite solve the problem - the Iteration author wants every Iteration to have a start and end. The PostConstruct annotation insures that the JSF implementation will not create an invalid Iteration, but there is no way to make sure the JSF implementation has exclusive access to the no args constructor (and the setters).

The PostConstruct approach is not bad, but a better approach would be to ditch the dependency injection capabilities that come with any JSF implementation and to use a full blown dependency injection framework. Using Spring is as easy as placing the following lines of code in your JSF deployment descriptor.

<application>
<variable-resolver>
    org.springframework.web.jsf.DelegatingVariableResolver
</variable-resolver>
</application>

The latest release of Apache MyFaces provides a Guice VariableResolver as well.

The Map Trick

I'll go far enough to say there is never an excuse to use the Map Trick. Like the Validating Setter, the Map Trick begins with a small limitation of the spec. JSF EL and Unified EL do not support parameterized method invocation of managed bean methods. The closest feature available is static method invocation via JSP EL or Facelets. Tapestry developers, or anyone else familiar with the OGNL expression language, are often disappointed to learn this.

The Map interface is the only exception. JSP EL, JSF EL and Unified EL all support invocation of the get method, a parameterized method of the Map interface.

#{myManagedBean.silvert} // pulls 'silvert' from managed bean Map
#{param['foo']}          // pulls 'foo' request parameter

Some developers have implemented their own Map to take advantage of this.

public class MapTrick implements Map {

 public Object get(Object key) {

  return new BusinessLogic().doSomething(key);

 }

 public void clear() { }
 public boolean containsKey(Object arg) { return false; }
 public boolean containsValue(Object arg) { return false; }
 public Set entrySet() {return null; }
 public boolean isEmpty() { return false; }
 public Set keySet() { return null; }
 public Object put(Object key, Object value) { return null; }
 public void putAll(Map arg) { }
 public Object remove(Object arg) { return null; }
 public int size() { return 0; }
 public Collection values() { return null; }

}

When the EL Resolver invokes the get method the parameter is then used by business logic. I once visited a project where an architect had created an entire mini framework around the Map Trick. Needless to say many of the developers were complaining of severe coupling between the view and model.

The Déjà vu PhaseListener

One of the most active mailing lists for the Apache Software Foundation is users@myfaces.apache.org . It is a wonderful place for exchanging new ideas, trading technical solutions, and flame wars. After all these years, MyFaces remains an open source project where many of my teammates on the development team still interact with application developers. A classic problem on the mailing list is the infamous "Deja Vu PhaseListener Effect". If you are at a computer right now, google "MyFaces PhaseListeners twice" - you'll see.

Let's look at how this happens. First, PhaseListeners are registered in a JSF configuration file.

<lifecycle>
  <phase-listener>net.dbyrne.PhaseListenerImpl</phase-listener>
</lifecycle>

Next, JSF configuration files are in turn specified via the javax.faces.CONFIG_FILES context parameter in the deployment descriptor.

<context-param>
    <description>comma separated list of JSF conf files</description>
    <param-name>javax.faces.CONFIG_FILES</param-name>
    <param-value>
  /WEB-INF/faces-config.xml,/WEB-INF/burns.xml
    </param-value>
</context-param>

Why are the PhaseListeners firing twice? Because all JSF implementations automatically parse /WEB-INF/faces-config.xml, if it exists, as well as all files specified in the comma separated list of the javax.faces.CONFIG_FILES context parameter. When /WEB-INF/faces-config.xml is specified in the javax.faces.CONFIG_FILES context parameter it can be parsed twice. The PhaseListeners configured in this file are consequently registered twice at startup and invoked twice at runtime.

XML Fetishism

Let's be real. JSF is all about hundreds of lines of XML, not thousands. I once visited a project with a configuration file similar to the following. The "contact us" page was accessible from every page on the site and a separate action rule was used for each page.

<navigation-rule>
 <from-view-id>/home.xhtml</from-view-id>
 <navigation-case>
  <from-outcome>contact_us</from-outcome>
  <to-view-id>/contact.xhtml</to-view-id>
 </navigation-case>
</navigation-rule>
<navigation-rule>
 <from-view-id>/site_map.xhtml</from-view-id>
 <navigation-case>
  <from-outcome>contact_us</from-outcome>
  <to-view-id>/contact.xhtml</to-view-id>
 </navigation-case>
</navigation-rule>
<navigation-rule>
 <from-view-id>/about_us.xhtml</from-view-id>
 <navigation-case>
  <from-outcome>contact_us</from-outcome>
  <to-view-id>/contact.xhtml</to-view-id>
 </navigation-case>
</navigation-rule>
<!-- continued ... -->

A global navigation rule was used to significantly reduce the size of the configuration file.

<navigation-rule>
 <from-view-id>*</from-view-id>
 <navigation-case>
  <from-outcome>contact_us</from-outcome>
  <to-view-id>/contact.xhtml</to-view-id>
 </navigation-case>
</navigation-rule>

XML Hell also provides a number of opportunities for runtime exceptions. Let's look at some examples.

    <managed-bean>
    <managed-bean-name>invalid</managed-bean-name>
      <!-- misspelled class name throws ClassNotFoundException -->
      <managed-bean-class>net.dbyrne.misspelled.Invalid</managed-bean-class>
     <managed-bean-scope>session</managed-bean-scope>
     <managed-property>
       <!-- one side of cyclical reference -->
      <property-name>incorrect</property-name>
      <value>#{incorrect}</value>
      </managed-property>
    </managed-bean>

    <managed-bean>
     <managed-bean-name>incorrect</managed-bean-name>
      <managed-bean-class>net.dbyrne.validate.Incorrect</managed-bean-class>
   <managed-bean-scope>session</managed-bean-scope>
      <managed-property>
       <!-- other side of cyclical reference -->
      <property-name>invalid</property-name>
      <value>#{invalid}</value>
      </managed-property>
    </managed-bean>

Notice the misspelled class name and the cyclical reference between the two managed beans found in this configuration file. A JSF implementation is not required to warn you about either of these problems at startup, but both will result in a runtime exception.

    <managed-bean>
<!-- first duplicate -->
     <managed-bean-name>duplicate</managed-bean-name>
      <managed-bean-class>
       net.dbyrne.validate.FirstDuplicate
      </managed-bean-class>
     <managed-bean-scope>session</managed-bean-scope>
    </managed-bean>

    <managed-bean> <!-- second duplicate -->
     <managed-bean-name>duplicate</managed-bean-name>
      <managed-bean-class>
       net.dbyrne.validate.SecondDuplicate
      </managed-bean-class>
     <managed-bean-scope>application</managed-bean-scope>
    </managed-bean>

This problem was fixed in JSF 1.2 but for people still using 1.1 this causes all sorts of headaches that are real hard to debug. Most 1.1 implementations will silently ignore the first managed bean and simply load the second.

    <managed-bean>
     <managed-bean-name>requestScopeManagedBean</managed-bean-name>
     <managed-bean-class>
      net.dbyrne.RequestScopeManagedBean
     </managed-bean-class>
     <managed-bean-scope>request</managed-bean-scope>
    </managed-bean>

    <managed-bean>
     <managed-bean-name>sessionScopeManagedBean</managed-bean-name>
     <managed-bean-class>
      net.dbyrne.SessionScopeManagedBean
     </managed-bean-class>
     <managed-bean-scope>session</managed-bean-scope>
     <managed-property>
       <!-- throws runtime exception -->
      <property-name>requestScopeManagedBean</property-name>
      <value>#{requestScopeManagedBean}</value>
     </managed-property>
    </managed-bean>

A JSF implementation must also throw an exception at runtime if it encounters a managed bean with a "subscoped" managed property. In the following example, the session scoped managed bean is not valid because it refers to a request scoped managed bean.

Many of these configuration pitfalls can be detected with a set of generic JUnit tests distributed as part of JBoss JSFUnit. You can also avoid some of these problems with annotations libraries found in Seam or Shale.

Thread Safety

You never have to worry about some parts of the application. Calls to FacesContext.getCurrentInstance() return a thread local data structure. Request and unscoped managed beans are of course safe as well.

Other parts of you application are accessed by more than one thread at a time. PhaseListeners and Renderers are prime examples. Each PhaseListener is global to the application and it subscribes to at least one phase event for every request. The relationship between component instances and Renderer instances is many to one: components of the same type share the same Renderer instance for the entire application. Session and application scoped managed beans are obviously accessed by more than one thread.

Custom tag handlers do not need to be thread safe, but the container can reuse the tag instance. Always reset the field values by implementing the release method.

You may or may not need to make a custom Converter thread safe. Registering a Converter and using a converter tag will never introduce a race condition. The component will get a new Converter instance each time it is needed.

 <converter>
  <converter-id>threadUnsafeConverter</converter-id>
         <converter-class>net.dbyrne.ThreadUnsafeConverter</converter-class>
     </converter>

 <h:inputText value="#{managedBean.value}" >
  <f:converter converterId="threadUnsafeConverter" >
 </h:inputText>

Using the converter attribute however could introduce a race condition because it is possible the same Converter instance will be used simultaneously by more than once request.

    <managed-bean>
     <managed-bean-name>threadUnsafeConverter</managed-bean-name>
     <managed-bean-class>net.dbyrne.ThreadUnsafeConverter</managed-bean-class>
     <managed-bean-scope>session</managed-bean-scope>
    </managed-bean>

    <h:inputText value="#{managedBean.value}" converter="#{threadUnsafeConverter}" />

Custom Validators have the same thread safety constraints as custom Converters.

Facelets Migration Challenge: Tags with Behavior

Java developers have long been able to create their own JSP tags. JSF developers inherited this because the default view technology for JSF is JSP. JSF hooks into JSP by invoking the setProperties method when the JSP tag is first rendered. This is the tag handler's one chance to pass the tag attribute values on to the UIComponent mapped to the tag.

public class WidgetTag extends UIComponentELTag{

 private String styleClass = "default_class";
 private String title;

 public String getComponentType() {
  return "net.dbyrne.widget";
 }

 public String getRendererType() {
  return "net.dbyrne.widget";
 }

 public void setStyleClass(String styleClass) {
  this.styleClass = styleClass;
 }

 public void setTitle(String title) {
  this.title = title;
 }

 public void release() {
  super.release();
  styleClass = null;
  title = null;
 }

 protected void setProperties(UIComponent component) {

  super.setProperties(component);

  Widget span = (Widget) component;

  span.setStyleClass(styleClass);
  span.setTitle(title == null ? "no title" : title);

  FacesContext ctx = FacesContext.getCurrentInstance();
  Map sessionMap = ctx.getExternalContext().getSessionMap();
  span.setStyle((String) sessionMap.get("style"));

 }

}

An experienced JSF developer will notice a few things wrong with this picture. The styleClass field defaults to "default_style", the value of the title field depends on ternary logic, and one of the component values is derived from a value expression. Unfortunately all three of these behaviors have been implemented in the tag handler.

Each rendering behavior is lost when the component is used with an alternative view technology such as Facelets. When we develop custom JSF components, it is important to place this kind of logic out of a tag handler. The Apache MyFaces code base is a good example of this. The source code for 25 components and 25 tag handlers of the JSF 1.2 implementation are generated from meta-data at build time, with no behavior.

Law of Demeter

Once upon a JavaOne conference, Mathias needed to borrow some money. "Craig", said Mathias, "I owe Martin a beer. Can I borrow five bucks"? Craig obliged, finding himself in an awkward moment as Mathias retrieved Craig's wallet and removed five dollars. "Mathias", said Craig, "if I began storing my money in my shoe, or a purse, you would have to change the way you borrow money from me". "You are right", said Mathias, "my money borrowing logic is coupled to your money hiding logic. Next time I'll ask you for the money, and let you be concerned about where it is stored". Craig knew Mathias was a smart guy and dismissed the incident as a harmless cultural misunderstanding. It wasn't; it was a classic demonstration of the Law of Demeter.

Everyone knows people should not reach into each other's pockets for money [Editor's note: erm… noted!], but many people think it's OK for objects to do it.

// highly sensitive to changes of the domain model
employee.getDepartment().getManager().getOffice().getAddress().getZip()

 <!-- highly sensitive to changes of the domain model -->
#{employee.department.manager.office.address.zip}

Few experienced developers need to be convinced that the above java method chain, or train wreck, does not observe a basic object oriented principle. But some do not recognize that the equally long EL expression is worse. The luxury of a compilation error is not available because EL is interpreted. MethodNotFoundErrors and NullPointerExceptions are common with view templates like this. Refactoring is a pain because EL is loosely typed. When EL expressions become five or six segments long it couples the view template to the model. EL expressions should take advantage of encapsulation, just like plain Java.

<!-- encapsulated, insensitive to changes -->
#{employee.departmentalManagerOfficeZip}

Code to Interface

JSF was designed to encourage an "embrace and extend" mindset. The JSF component model is consequently driven largely by interfaces. The ImplementationDependentManagedBean is an example of what happens when these interfaces are not observed; and yes, I have had to fix code like this.

import org.apache.myfaces.component.html.ext.HtmlInputHidden;
import org.apache.myfaces.component.html.ext.HtmlInputText;
import org.apache.myfaces.component.html.ext.HtmlOutputText;

public class ImplementationDependentManagedBean {

 private HtmlInputText input ;
 private HtmlInputHidden hidden ;
 private HtmlOutputText output ;

 /* getters & setters ommitted */

 public boolean recordTotal(ActionEvent event) {

  long total = ((Long)input.getValue()).longValue();
  total += ((Long)hidden.getValue()).longValue();
  total += ((Long)output.getValue()).longValue();

  return new JmsUtil().broadcastTotal(total);
 }

}

This managed bean has three component binding properties. Business logic is applied to each component in an action listener. Three classes from a MyFaces Tomahawk package have been imported. Notice the business logic only invokes the getValue method of each component. The getValue method is inherited from a superclass found in the core JSF API. By replacing these imports the class becomes interoperable.

import javax.faces.component.ValueHolder;

public class RefactoredManagedBean {

   private ValueHolder input ;
   private ValueHolder hidden ;
   private ValueHolder output ;

   /* getters & setters ommitted */

   public boolean recordTotal(ActionEvent event) {

    long total = 0;

    for(ValueHolder valued : new ValueHolder[] {input, hidden, output})
         total += ((Long)valued.getValue()).longValue();

    return new JmsUtil().broadcastTotal(total);

   }
}

ValueHolder is an interface implemented by a superclass of HtmlInputText, HtmlInputHidden and HtmlOutputText. By refactoring the class to use the ValueHolder interface, the managed bean can be used with the reference implementation as well as different ValueHolder component bindings (eg. HtmlOutputText, HtmlInputText, HtmlInputHidden, etc.). Polymorphism is used to clean up the business logic.

Remember, one of the beautiful things JSF provides is the freedom to express logic in POJOs. Application developers don't have to implement an interface or extend a class provided by the container. This doesn't mean it is alright to drop the principles of object oriented programming. When you see an opportunity to program to a standardized interface, do it.

Did you notice anything peculiar about the recordTotal action listener method? The JSF specification defines the return type of action listener methods to be void but the action listener has a return type of boolean. MyFaces allows this and ignores the method return type. Try to avoid this - it is acceptable for the reference implementation to throw an exception.

View State Encryption

A common misconception is that SSL excuses the need for view state encryption. Likewise, many developers assume using view state encryption means they have no need of SSL. Let the author be very clear: SSL and view state encryption have no common ground. They solve completely separate problems at different layers of the network protocol stack.

Consider the classic man in the middle attack. Manfred is a bank, Sean is an online customer, and Mike is an intermediary. Using plain HTTP, Mike can intercept the request from Sean to Manfred, record Sean's password, and forward the request to Manfred, unbeknownst to either party at each end.

By working at the transport layer of the network protocol stack, Manfred and Sean can use SSL to prevent Mike from using the intercepted information.

SSL provides point to point security, keeping Sean and Manfred safe from Mike; it does not keep Manfred safe from Sean if the application is using client side state saving. JSF builds a data structure to represent the component tree during the render response phase when using client side state saving. This data structure is called the view state. When using client side state saving, the view state is serialized, encoded, and slipped into the response via a hidden HTML field in the form.

When the HTML form is submitted it carries the view state value back to the server in the form of an HTTP parameter. JSF uses the value of this parameter to reconstruct the view during the restore view phase. The view is restored by reversing the process used to obtain the view state: it is decoded and deserialized. This poses a major security challenge to any JSF implementation because Sean has the freedom to change the view state. He can toggle the rendered attribute of UI controls that are not supposed to be available to him. He can point a commandButton to a method on any managed bean in the application. He can circumvent an action listener.

I strongly recommend view state encryption for public facing JSF applications using client side state saving in production because it prevents web clients from being able to tamper with the view state. I recommend disabling view state encryption for development and functional testing. Review the documentation for your JSF implementation in order to learn how you can enable view state encryption.

Portlet Issues

I feel sorry for Portlet developers. I really do. These people are always on the mailing lists and forums with problem after problem and it's never their fault. If any group of people has been bent over by the standards bodies, it is Portlet application developers.

Some parts of the JSF API behave different when inside a Portlet application. If your code is to run within a Portlet application and a regular Servlet container there are a few assumptions that need to be avoided. Here is some code assuming it will always run in a Servlet container.

FacesContext ctx = FacesContext.getCurrentInstance();
ExternalContext externalContext = ctx.getExternalContext();
ServletRequest request = (ServletRequest) externalContext.getRequest();
String id = request.getParameter("id");

If your code will run in a Portlet application you must make not make explicit casts to javax.servlet.ServletRequest or javax.servlet.ServletResponse. If this code were used in a Portlet application, ExternalContext.getRequest would return a javax.portlet.PortletRequest, causing a ClassCastException. An interoperable way to obtain a value of a request attribute is to do the following.

FacesContext ctx = FacesContext.getCurrentInstance();
ExternalContext externalContext = ctx.getExternalContext();
externalContext.getRequestParameterMap().get("id");

About the Author

Dennis Byrne currently works in the bay area for ThoughtWorks, a global consultancy with a focus on end-to-end agile software development of mission critical systems. Dennis is a committer and PMC member for Apache Myfaces. He is also a committer for JBoss JSFUnit. You can see Dennis' case study on JSF Anti-patterns this March at the 2008 TheServerSide Java Symposium in Las Vegas.