Sunday, August 10, 2008

c:forEach with JSF could ruin your day

Last week, a co-worker of mine at Oracle was attempting to use a forEach tag to produce components in a loop and developed issues. Having some experience trying to deal with for each I started looking into the problem and found a disturbing problem.

The JSTL <c:forEach/> tag was written to make JSP development easier, but was never intended to be used with Java Server Faces. In JSP 2.1 some enhancements were made to improve the usage of non-JSF JSP tags intermingled with JSF by introducing deferred expressions. The idea behind the deferred expressions was to be able to create expressions that would normally have to be evaluated at JSP tag execution and allow them to be executed with JSF components instead. The problem is that the implementation of deferred expressions is flawed for the <c:forEach/> loop except for basic usage only.

Use Case

For the illustration of the problem with the <c:forEach/> I will simplify the use case. The are two main problems with the tag both of which involve the changing of the list bound to the items attribute. For the first, consider the following requirements:

  • For each loop required, JSF stamped iteration components cannot be used (like JSP tags that include a page)
  • Ajax used to partially render the page
  • Id's of components must be tied to the item in the loop to allow actions

Problem 1 - static EL

A simple example can be used to show the problem:

<h:form>
  <c:forEach var="item" items="#{bean.items}">
    <h:commandButton
      id="button_${var.key}"
      value="Remove #{var.key}"
      actionListener="#{bean.remove}" />
  </c:forEach>
</h:form>

Take for example a list of "A", "B", "C" and "D". The buttons render as:

ID Text
button_A Remove A
button_B Remove B
button_C Remove C
button_D Remove D

Now should the user click "Remove A", the managed bean removes the value from the items list. The result is:

ID Text
button_B Remove C
button_C Remove D
button_D Remove

Notice the mismatch of ID and text? Notice that the last button's #{var.key} evaluated to null?

To understand the problem, it is best to describe a step by step process of how the code works:

  1. The for each tag is executed and the first iteration (count 0) is begun
  2. The tag creates an IndexedValueExpression. This sub-class of ValueExpression.
    • The expression to get the items is stored (#{bean.items})
    • The index of the for each iteration is stored (0)
  3. This IndexedValueExpression is then added to the VariableMapper as the var - item.
  4. The body of the tag is executed
  5. The command button tag is instructed to create or find a component with ID button_A.
  6. The command button component is created with all its attributes.
    • When the component is created, the ValueExpression instances are created
    • Each ValueExpression is created with a reference to the FunctionMapper and VariableMapper
  7. The for each tag proceeds with its execution and processes indexes 1-3

When the action executes and removes A, the following occurs during the render phase. First, the for each tag looks for the button for B and finds it. Then the loop repeats. At the end of processing the tags, the code realizes that button_A was never referenced, and is therefore removed from its parent, the form.

The problem is that once the component is created, the ValueExpression instances are created, and are never re-created. This is because the expressions with their mappers are serialized and restored as part of the component state. This means that button_B which was found for the first item after item A was removed still has its original ValueExpression for the value attribute. This expression has the variable mapper with the indexed value expression for the var which has index 1.

This means that if components are used with IDs that tie to the items, it is possible to have the value expressions pointing to the wrong index in the for each loop once changes are made to the items list.

Problem 2 - Component State

One workaround to problem one is to use dynamically created IDs. This will violate our above requirement of needing to match the component to the item for advanced PPR (AJAX) replacement, but it is a consideration.

Lets use the above use case with the IDs removed:

<h:form>
  <c:forEach var="item" items="#{bean.items}">
    <h:commandButton
      value="Remove #{var.key}"
      actionListener="#{bean.remove}" />
  </c:forEach>
</h:form>

This renders as:

ID Text
j_id_id2 Remove A
j_id_id2j_id_1 Remove B
j_id_id2j_id_2 Remove C
j_id_id2j_id_3 Remove D

When A is removed, since the item key is not used in the ID, the above problem is not witnessed:

ID Text
j_id_id2 Remove B
j_id_id2j_id_1 Remove C
j_id_id2j_id_2 Remove D

What you will notice is that item B which used component with ID j_id_id2j_id_1 is now rendered by component with ID j_id_id2. Unlike above, this component has the correct index stored for it, but its state may break the view. Now command buttons are not a great example of this, but many components have state that is more evident, like tree node expansion. What this will mean to the user is that component state that belongs with item B is now associated with item C.

How could this be addressed?

The fundamental problem is that the implementation of the for each loop tag assumes that the location of the components that are created are independent of the items used to produce them. The ValueExpression instances are created for components with hard coded indexes and there is no way that I know of to update the index to the correct value later.

One fix is to allow the for each loop to use keys for the item instead of indexes. The obvious problem with this idea is that it would involve a significant API enhancement to be able to specify a key for an object and the use of a Map instead of a List or array to be able to lookup the value for the key. Something like:

<h:form>
  <c:forEach var="item" keys="#{items.keys} items="#{bean.items}">
    <h:commandButton
      value="Remove #{var.key}"
      actionListener="#{bean.remove}" />
  </c:forEach>
</h:form>

Where keys would be a List or array of serializable objects for keys in the items which would be represented as a Map.

Summary

As I have always recommended on the Apache MyFaces and Facelets mailing lists the best solution is to never use JSTL tags for JSF development. Unfortunately there are some pieces of functionality which is hard to replicate using components. Perhaps JSF 2.0 will allow things like component controlled sub-page inclusion.

Update 2012-10-03

Partial fix to be available as part of Trinidad. My blog on the enhancement to the Trinidad for each tag.

15 comments:

renzo.tomaselli said...

Hi, unfortunately AFAIK there is at least one case where looping over a dynamic list of components has no workaround: e.g. when things are included through ui:include, then c:forEach is the only solution I know of.
Besides that - the simple workaround I use to overcome the statical behavior of c:forEach is to force the re-creation of all component list whenever it changes. This can be achieved by:
- catching a parent component by binding;
- cleaning its children list.
This way Facelets recreates the component list (along the "apply" iteration), asking the bean for the current list and all up-to-date ids.
Hope it helps.

Yao said...

Could you elaborate the workaround a little bit more? Any code examples would be helpful. Thanks!

Andrew said...

I have not coded the work-around, that is why I did not post it. It would be a bit involved as it would involve new expression objects, a new custom forEach JSP tag and a bunch of testing. I don't have the time at the moment for this research as I have a bunch of other things going on. This blog is more of a notice to people than it is a fix. My hope is that the JSF team can address the issue, that is why I logged a bug against the JSTL for this (it is not approved yet so has no public bug # to post here).

Matt said...

Glad I've found someone with the same problem.

I'm experiencing this now, but with adding items to a List.
My forEach simply creates an OutputText component for each iteration within an "li" element.

The weird behaviour when an item is added to the list after the initial render is that the loop iterates the correct number of times, but this happens:
1. Iterations prior to the last render nothing.
2. All additional items are rendered in the final iteration, but before the old "last" item.

So, if item C was added to a list the output would be:
[li][span>A[/span][/li]
[li] [/li]
[li][span]C[/span][span]B[/span][/li]

Pretty annoying!
I'm using JSP2.1, JSF 1.2(Sun RI with glassfish 2) and JSTL 1.2 (comes with glassfish 2).

Matt said...

...one thing I don't get is why this component caching would be occuring when the items List is provided by a request scoped bean. Surely it should be expected that with each request the bean list property may be different (it is set on each request). I guess there is no rationale behind bugs :)

Andrew said...

@Matt: The big problem is that it isn't really a bug as much as a design oversight. It just goes to show you that JSTL and JSF really are not made to be used together. As for you question on caching, it is not caching the items, but rather caching indexes on the ValueExpression instances inside of the component. Really ends up in a messy state. Best workaround is to not use JSTL.

Matt said...

I have noticed that while the initial rendering in my case results in the wrong values rendered within the wrong iteration HTML output, subsequent renderings are correct. Based on this, I found that setting the context view root to a new one is a workaround for this.

Andrew said...

Creating a new view root, or just clearing the children of the c:forEach "parent" will indeed "work" but you get the negative side effect of loosing all component state.

Jay said...

I am so happy after got this articles so thanks allot for it.
Outsourced Product Development

Aaron Newell said...

This blogpost was written in 2008. In JSF 2.0 you can use ui:repeat instead.

http://docs.oracle.com/cd/E17802_01/j2ee/javaee/javaserverfaces/2.0/docs/pdldocs/facelets/index.html

The advice about not mixing JSTL and JSF should be heeded.

Andrew Robinson said...

Actually ui:repeat does not help you at all over c:forEach. ui:repeat, like Trinidad's tr:iterator, is a stamping component. This means that it only has one component sub-tree. c:forEach, tr:forEach and other JSTL tags actually create one component sub-tree per iteration. This is necessary for creating components in a loop. For example, if you want to dynamically include different pages with jsp:include or if you are using Oracle ADF, the af:region, af:pageTemplate or af:declarativeComponent, you need to be able to change the view/page being included during tag execution.

So yes, you should always use stamping when you can, but it is not a replacement to the forEach tags.

Andrew Robinson said...

More on ui:repeat vs c:forEach may be understood with another blog of mine:
build time vs render time

Andrew Robinson said...

The Trinidad forEach tag has been upgraded to address these issues. I have just commit an enhancement as part of TRINIDAD-1940 to support map based components that allow the collection to be changed.

This will be part of the MyFaces Trinidad 2.1 release.

EAmez said...

Hi,

I'm facing this problem with c:forEach not being as dynamic as we expected. We must render dynamic columns in an ace:dataTable (IF3 with JSF2), because columns depends on previously selected values. We haven't been able to component tree in order to make it to generate again with correct indexes.

c:forEach is needed, and we can't use Trinidad. Any suggestion on how to recreate component tree?

Thanks in advance.

Andrew Robinson said...

The problem is simply that JSP is a poor technology for use with JSF when using JSP tags that are not JSF component tags. Without trying to use Trinidad, the best option for you may be to simply create all the components in a given page via Java code and not JSP/Facelets. This would give you the full control to create the components and create your own ValueExpressions as needed. Yes it stinks. The other is to see if you can use an iterating component (like tr:iterator) to stamp the columns (loop over data with the same component structure) if that is possible.

Sorry, but there really is no elegant solution at this point. I hope that changes can be introduced into a future JSF spec to address the shortcomings, but so far I am seeing no discussion on the topic in the EG (although I don't keep a close eye on it).