Sometimes I try to do things but it just doesn't work out the way I want it to, and I get real frustrated and then like I try hard to do it, and I like, take my time but it just doesn't work out the way I want it to. Its like, I concentrate on it real hard, but it just doesn't work out. And everything I do and everything I try, it never turns out. Its like, I need time to figure these things out.
--Institutionalized by Suicidal Tendencies
This blog is for people who feel this way about JSF. Or who just want a Pepsi.

Friday, February 26, 2010

References to UIComponents in Session-scoped Beans

JSF developers love session-scoped managed beans. I think this is because, like the perfect significant other, a session-scoped bean is always there for you. Later they realize that their session-scoped bean is more like a stalker--a stalker with threading and serialization issues that loafs on the couch consuming memory. But, in the first blush of ardor, when the developer wants to do everything with their new session-scoped bean, they might write some code that looks like this:
import java.util.List;
import javax.faces.event.ActionEvent;
import javax.faces.model.SelectItem;
import org.apache.myfaces.trinidad.component.UIXSelectMany;

/**
 *  Maintains a list of weather forecasts for locations for this user
 */
public final class WeatherBean {
  /** Set the component binding for the selectMany component */
  public void setRemoveLocationsComponent(UIXSelectMany removeComp) {
    _removeLocationsComp = removeComp;
  }

  /** Get the selectMany component containing the locations to remove */
  public UIXSelectMany getRemoveLocationsComponent() {
    return _removeLocationsComp;
  }

  /** The selectManyCheckbox's value is bound to this.
   *  This sets what is checked in the checkbox. */
  public void setLocationsToRemove(List<String> value) {
    _locationsToRemove = value;
  }

  /** The selectManyCheckbox's value is bound to this.
   *  This gets what checked in the checkbox. */
  public List<String> getLocationsToRemove() {
    return _locationsToRemove;
  }

  public void removeLocationListener(ActionEvent actionEvent) {
    // get the list of selected items in the selectManyCheckbox.
    // Then, loop through the locations and pull these out
    // of the location list and refresh.
    List<String> locationsToRemove = getLocationsToRemove();

    for (String location : locationsToRemove) {
      _removeLocation(location);
    }

    // force refresh of component value
    UIXEditableValue component = getRemoveLocationsComponent();

    component.setSubmittedValue(null);
    component.setValue(null);
    component.setLocalValueSet(false);
  }

  /** Lists the current locations in a SelectItem List so
   *  that it can be displayed as a select many checkbox. */
  public List<SelectItem> getLocationSelectItems() {
    ... turn List of Locations into a List of SelectItems ...
  }

  private void  _removeLocation(String location) {
    ... remove the location from our list of Locations we have weather for ...
  }

  private UIXSelectMany _removeLocationsComponent;
  private List<String>  _locationsToRemove;

  // list of locations to display weather for
  private List<String>  _locations;
}
And use it in a page like this:
<!-- Display list of checkboxes of current locations -->
<tr:selectManyCheckbox label="Locations"
                       id="smc1"
                       valuePassThru="true"
                       binding="#{weatherBean.removeLocationsComponent}"
                       value="#{weatherBean.locationsToRemove}">
  <f:selectItems value="#{weatherBean.locationSelectItems}" id="si1"/>
</tr:selectManyCheckbox>

<!-- Button to delete the selection locations -->
<tr:commandButton id="deleteCB"
                  text="Remove Locations"
                  actionListener="#{weatherBean.removeLocationListener}"/>
The example is from an application that displays weather for various locations using the Apache Trinidad JSF Components. This fragment of the application displays checkboxes for the user's current registered locations, allowing the user to select the locations to remove from the list stored in the _locations field. When the tags for the page are executed, the component binding of the removeLocationsComponent sets the component instance on the session-scoped WeatherBean instance. Then when the user clicks the "Remove Locations" button, the set of location Strings to remove is set on the WeatherBean by the ValueExpression #{weatherBean.locationSelectItems} during the update model JSF phase and the WeatherBean's removeLocationListener(ActionEvent) ActionListener is called during the invoke application JSF phase to remove the list of locations saved earlier from the displayed list of locations. This code works well when tested on the developer's machine. However, the relationship soon sours. There are two separate sets of problems here--Serialization problems and thread safety problems.

Serialization and Session-scoped Beans

When deployed in a clustered environment, Session-scoped beans need to be Serializable so that any beans changed during a request can be distributed to other servers for fail-over. At the end of the first request in a clustered environment, the above code fails with a NotSerializableException. (Since testing in a cluster is often a pain, Trinidad developers often test their applications with the -Dorg.apache.myfaces.trinidad.CHECK_STATE_SERIALIZATION=session,tree System property set so that Trinidad aggressively checks the serializability of properties stored into both the session Map and as attributes on the UIComponents in the component tree) This problem is apparently easily fixed by making the WeatherBean Serializable:
public final class WeatherBean implements Serializable{

 ... existing content ...

  private static final long serialVersionUID = 0L;
}
Unfortunately, this doesn't work. The class still isn't Serializable, because UIComponents aren't Serializable, so the serialization fails on the _removeLocationsComp field of WeatherBean. We could try to patch things up by marking _removeLocationsComp transient, but we have only scratched the surface of our problems.

Thread Safety Problems in the Example

There are a whole raft of concurrency and memory leak issues here:
  1. If the user opens two windows on this page, only the last window rendered has the correct UIXSelectMany instance. If the remove locations button is pressed on the first window after rendering the second window, the wrong UIXSelectMany instance will be used.
  2. Since UIComponents aren't thread-safe, the above case could also result in bizarre behavior if requests from both windows were being processed by the application server at the same time.
  3. In the above case, there could be other issues because the _removeLocationsComponent field isn't safely published across the Threads reading and writing its value
  4. If the Trinidad view state token cache isn't used, or if the user navigates to this page using the back button, a new UIXSelectMany instance will be created to handle the request, which also won't match the instance we are holding onto.
  5. If we don't clear the UIXSelectMany instance when we navigate off of this page, we will continue to pin the page's UIComponent tree in memory for the lifetime of the Session.
Well that sucks. I guess it's time to break up.

Breaking Up isn't that Hard to Do

We're breaking our session-scoped bean into two pieces. A request-scoped piece and a session-scoped piece. We move our UIComponent reference and the list of locations to remove (which also had concurrency problems) into our new request-scoped bean:
/** Stores state used for removing Locations at request scoped */
public final class RemoveLocationState {
  /** Set the component binding for the selectMany component */
  public void setRemoveLocationsComponent(UIXSelectMany removeComp) {
    _removeLocationsComp = removeComp;
  }

  /** Get the selectMany component containing the locations to remove */
  public UIXSelectMany getRemoveLocationsComponent() {
    return _removeLocationsComp;
  }

  /** The selectManyCheckbox's value is bound to this.
   *  This sets what is checked in the checkbox. */
  public void setLocationsToRemove(List<String> value) {
    _locationsToRemove = value;
  }

  /** The selectManyCheckbox's value is bound to this.
   *  This gets what checked in the checkbox. */
  public List<String> getLocationsToRemove() {
    return _locationsToRemove;
  }

  /** Lists the current locations in a SelectItem List so
   *  that it can be displayed as a select many checkbox. */
  public List<SelectItem> getLocationSelectItems() {
    ... turn List of Locations into a List of SelectItems ...
  }

  private UIXSelectMany _removeLocationsComponent;
  private List<String>  _locationsToRemove;
}
We then change our Session-scoped bean to use the state stored in the request-scoped bean. For a request-scoped bean named "weatherRemovalState", the code would look like this:
public final class WeatherBean implements Serializable {
  public void removeLocationListener(ActionEvent actionEvent) {

    // get the request-scoped weather removal state
    ExernalContext extContext =
                       FacesContext.getCurrentInstance().getExternalContext();

    RemoveLocationState removeState = (RemoveLocationState)
                        extContext.getRequestMap().get("weatherRemovalState");

    // get the list of selected items in the selectManyCheckbox.
    // Then, loop through the locations and pull these out
    // of the location list and refresh.
    List<String> locationsToRemove = removeState.getLocationsToRemove();

    for (String location : locationsToRemove) {
      _removeLocation(location);
    }

    // force refresh of component value
    UIXEditableValue component = removeState.getRemoveLocationsComponent();

    component.setSubmittedValue(null);
    component.setValue(null);
    component.setLocalValueSet(false);
  }

  /** Lists the current locations in a SelectItem List so
   *  that it can be displayed as a select many checkbox. */
  public List&lt;SelectItem&gt; getLocationSelectItems() {
    ... turn List of Locations into a List of SelectItems ...
  }

  private void  _removeLocation(String location) {
    ... remove the location from our list of Locations we have weather for ...
  }

  // list of locations to display weather for
  private List<String>  _locations;

  private static final long serialVersionUID = 0L;
}
And we change to component binding and ValueExpression to use our request-scoped "weatherRemovalState" bean so that our page looks like this:
<!-- Display list of checkboxes of current locations -->
<tr:selectManyCheckbox label="Locations"
                       id="smc1"
                       valuePassThru="true"
                       binding="#{weatherRemovalState.removeLocationsComponent}"
                       value="#{weatherRemovalState.locationsToRemove}">
  <f:selectItems value="#{weatherBean.locationSelectItems}" id="si1"/>
</tr:selectManyCheckbox>

<!-- Button to delete the selection locations -->
<tr:commandButton id="deleteCB"
                  text="Remove Locations"
                  actionListener="#{weatherBean.removeLocationListener}"/>
Now a new instance of RemoveLocationState named "weatherRemovalState" is created on each request when the "Remove Locations" button is pressed. Since this instance is only referenced by the session-scoped WeatherBean from the request thread, we don't have any synchronization issues here. Likewise, since the instance falls away at the end of the request, we don't have any serialization of memory leak issues. But why stop there?

There's Never Too Much of a Good Thing

The only state that we really need to maintain at the Session level is the set of locations to display. Plus, we probably want to maintain the list of locations plus any other preferences (like whether we prefer fahrenheit or celcius) across sessions. We'll make WeatherBean request-scoped and add a new WeatherPreferences bean holding and persisting our preferences that is session-scoped:
/**
 *  Request-scoped bean controlling the weather,
 *  though not as well as Storm
 */
public final class WeatherController {
  /** Set the component binding for the selectMany component */
  public void setRemoveLocationsComponent(UIXSelectMany removeComp) {
    _removeLocationsComp = removeComp;
  }

  /** Get the selectMany component containing the locations to remove */
  public UIXSelectMany getRemoveLocationsComponent() {
    return _removeLocationsComp;
  }

  /** The selectManyCheckbox's value is bound to this.
   *  This sets what is checked in the checkbox. */
  public void setLocationsToRemove(List<String> value) {
    _locationsToRemove = value;
  }

  /** The selectManyCheckbox's value is bound to this.
   *  This gets what checked in the checkbox. */
  public List<String> getLocationsToRemove() {
    return _locationsToRemove;
  }

  public void removeLocationListener(ActionEvent actionEvent) {
    // get the preferences for the current session
    WeatherPreferences preferences = 
               WeatherPreferences.getCurrentWeatherPreferences();

    // get the list of selected items in the selectManyCheckbox.
    // Then, pull these out of the location list and refresh.
    List<String> locationsToRemove = getLocationsToRemove();

    preferences.removeLocations(locationsToRemove);

    // force refresh of component value
    UIXEditableValue component = getRemoveLocationsComponent();

    component.setSubmittedValue(null);
    component.setValue(null);
    component.setLocalValueSet(false);
  }

  private UIXSelectMany _removeLocationsComponent;
  private List<String>  _locationsToRemove;
}


/**
 *  Maintains the Session-scoped user preferences for weather
 */
public final class WeatherPreferences implements Serializable {
  /**
   * Retrieves the WeatherPreferences for this Session, loading it
   * if necessary
   */
  public static WeatherPreferences getCurrentWeatherPreferences()
  {
    FacesContext context = FacesContext.getCurrentInstance();
  
    // get the weather preferences from the Session if it has already been instantiated
    Object preferences = context.getExternalContext().getSessionMap().get("weatherPreferences");

    // OK, not is Session, instantiate it using the ELResolver
    if (preferences == null)
      preferences = context.getApplication().getELResolver().getValue(
                        context.getELContext(), null, "weatherPreferences");

    return (WeatherPreferences)preferences;    
  }


  private WeatherPreferences()
  {
    List<String> locations = ... load preferences from repository ...;

    _locations = new CopyOnWriteArrayList<String>(locations);
  }


  /** Lists the current locations in a SelectItem List so
   *  that it can be displayed as a select many checkbox. */
  public List<SelectItem> getLocationSelectItems() {
    ... turn List of Locations into a List of SelectItems ...
  }

  /**
   * Remove the locations from the list of locations weather
   * is displayed for */
  public void  removeLocations(Collection<String> removeLocations){
    _locations.removeAll(removeLocations);
  }

  private static final long serialVersionUID = 0L;


  // list of locations to display weather for
  private final List<String>  _locations;
}
And the markup changed to use the request-scoped "weatherController" and session-scoped "weatherPreferences" beans.
<!-- Display list of checkboxes of current locations -->
<tr:selectManyCheckbox label="Locations"
                       id="smc1"
                       valuePassThru="true"
                       binding="#{weatherController.removeLocationsComponent}"
                       value="#{weatherController.locationsToRemove}">
  <f:selectItems value="#{weatherPreferences.locationSelectItems}" id="si1"/>
</tr:selectManyCheckbox>

<!-- Button to delete the selection locations -->
<tr:commandButton id="deleteCB"
                  text="Remove Locations"
                  actionListener="#{weatherController.removeLocationListener}"/>
Now everything is request-scoped except for the code that handles the persistence and threading issues of maintaining the Locations for this user session. While, in this case, we have discussed Session-scoped beans. Similar problems can occur with all scopes longer than request--application scope. The PageFlowScope of Apache Trinidad and Oracle ADF has these problems to a lesser extent. The ViewScope of JSF 2, Trinidad and ADF even less so. In general, bean refactoring is the best solution to the problem, but here are other slightly hacky ways of getting away with one bean.

But All I Need is One Bean to Make Me Happy

The trick to geting away with a single bean is removing the request scoped state from our session-scoped bean. In our original case, these two fields:
  private UIXSelectMany _removeLocationsComponent;
  private List<String>  _locationsToRemove;
The first step is realizing that we can get rid of _locationsToRemove by retrieving the value directly from the component instance rather than binding it. We then lose _removeLocationsComponent by using UIComponent.findComponent to retrieve our instance:
/**
 *  Maintains a list of weather forecasts for locations for this user
 */
public final class WeatherBean implements Serializable{
  public void removeLocationListener(ActionEvent actionEvent) {
    // get the list of selected items in the selectManyCheckbox.
    // and remove them
    List<String> locationsToRemove = _getLocationsToRemove();
    _removeLocations(locationsToRemove);

    // force refresh of component value
    UIXEditableValue component = getRemoveLocationsComponent();

    component.setSubmittedValue(null);
    component.setValue(null);
    component.setLocalValueSet(false);
  }

  /** Lists the current locations in a SelectItem List so
   *  that it can be displayed as a select many checkbox. */
  public List<SelectItem> getLocationSelectItems() {
    ... turn List of Locations into a List of SelectItems ...
  }

  /** Get the selectMany component containing the locations to remove */
  private UIXSelectMany _getRemoveLocationsComponent() {
    // find our UIXSelectMany by its id.  If the NamingContainer
    // hierarchy of the page changes, we're toast
    UIViewRoot viewRoot = FacesContext.getCurrentInstance().getViewRoot();
    return (UIXSelectMany)viewRoot.findComponent("smc1");
  }

  /** The selectManyCheckbox's value is bound to this.
   *  This gets what checked in the checkbox. */
  private List<String> getLocationsToRemove() {
    return (List<String>)_getRemoveLocationsComponent().getValue();
  }

  private void  _removeLocations(Collection<String> locations) {
    _locations.removeAll(locations);
  }

  private static final long serialVersionUID = 0L;

  // list of locations to display weather for
  private List<String>  _locations;
}
And the markup:
<!-- Display list of checkboxes of current locations -->
<tr:selectManyCheckbox label="Locations"
                       id="smc1"
                       valuePassThru="true">
  <f:selectItems value="#{weatherBean.locationSelectItems}" id="si1"/>
</tr:selectManyCheckbox>

<!-- Button to delete the selection locations -->
<tr:commandButton id="deleteCB"
                  text="Remove Locations"
                  actionListener="#{weatherBean.removeLocationListener}"/>

This works, but the use of findComponent leaves something to be desired. Hard-coding the NamingContainer path to the component is a little fragile. In addition, findComponent can be a little slow. These issues can be avoided in the future by using the Trinidad ComponentReference instead.

You're the One That I Want

ComponentReference acts as a fast, safe pointer to a UIComponent. Rewriting the above code with ComponentReference gives us:
/**
 *  Maintains a list of weather forecasts for locations for this user
 */
public final class WeatherBean implements Serializable{
  /** Set the component binding for the selectMany component */
  public void setRemoveLocationsComponent(UIXSelectMany removeComp) {
    // we only need to create a new reference the first time since
    // all other references for this page would be identical
    if (_selectManyReference == null)
      _selectManyReference = UIComponentReference.newUIComponentReference(removeComp);
  }

  /** Get the selectMany component containing the locations to remove */
  public UIXSelectMany getRemoveLocationsComponent() {
    // get the UIXSelectMany from the reference
    return _selectManyReference.getComponent();
  }

  public void removeLocationListener(ActionEvent actionEvent) {
    // get the list of selected items in the selectManyCheckbox.
    // and remove them
    List<String> locationsToRemove = _getLocationsToRemove();
    _removeLocations(locationsToRemove);

    // force refresh of component value
    UIXEditableValue component = getRemoveLocationsComponent();

    component.setSubmittedValue(null);
    component.setValue(null);
    component.setLocalValueSet(false);
  }

  /** Lists the current locations in a SelectItem List so
   *  that it can be displayed as a select many checkbox. */
  public List<SelectItem> getLocationSelectItems() {
    ... turn List of Locations into a List of SelectItems ...
  }

  /** The selectManyCheckbox's value is bound to this.
   *  This gets what checked in the checkbox. */
  private List<String> getLocationsToRemove() {
    return (List<String>)getRemoveLocationsComponent().getValue();
  }

  private void  _removeLocations(Collection<String> locations) {
    _locations.removeAll(locations);
  }

  private static final long serialVersionUID = 0L;

  // list of locations to display weather for
  private List<String>  _locations;
  private volatile ComponentReference<UIXSelectMany> _selectManyReference;
}

<!-- Display list of checkboxes of current locations -->
<tr:selectManyCheckbox label="Locations"
                       id="smc1"
                       valuePassThru="true"
                       binding="#{weatherBean.removeLocationsComponent}">
  <f:selectItems value="#{weatherBean.locationSelectItems}" id="si1"/>
</tr:selectManyCheckbox>

<!-- Button to delete the selection locations -->
<tr:commandButton id="deleteCB"
                  text="Remove Locations"
                  actionListener="#{weatherBean.removeLocationListener}"/>
Here we are back to using the component binding again. When setRemoveLocationsComponent is called the first time, we create a ComponentReference to this component and then use that reference whenever we need to retrieve the correct component instance. We continue to fetch the List of Strings to remove all of the component instance directly, as before. So, what's not to like? Well, it doesn't work. The current version of ComponentReference isn't as smart as it could be and chokes when the component binding is called. However that will be fixed soon. The bigger problem is that this solution is still kind of lame--if you wanted to have remove controls on more than one page, you would have to use separate ComponentReference instances for each different hierarchy. ComponentReference is pretty cool, but it isn't really the right solution here--refactoring is. Sometimes the grass isn't greener. I hoped this helped. In future blogs I will be returning to the topics of scopes, ComponentReferences and other JSF topics.

3 comments:

  1. Nice first blog entry Blake!

    I learned so much. Like, I never knew that ST made a video for Institutionalized. And now I do! w00t!

    Okay, just kidding. There is a ton of good info here. Who knew session state matters could make for such a fun read? Looking forward to reading more!

    BTW, you probably need to check out:

    http://www.amazon.com/Institutionalized/dp/B000QYTQ3W

    Sadly, no video for BVF.

    ReplyDelete
  2. Nice blog entry, Blake. I like the metaphor :-).

    I'm curious, though -- why didn't you use DI when you were referencing WeatherPreferences from WeatherController?

    ReplyDelete
  3. Because the app server this was being developed on didn't have CDDI integration. DI would be a great solution there, however.

    ReplyDelete