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:
- 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.
- 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.
- 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
- 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.
- 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<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 ...
}
// 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.
Nice first blog entry Blake!
ReplyDeleteI 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.
Nice blog entry, Blake. I like the metaphor :-).
ReplyDeleteI'm curious, though -- why didn't you use DI when you were referencing WeatherPreferences from WeatherController?
Because the app server this was being developed on didn't have CDDI integration. DI would be a great solution there, however.
ReplyDelete