Avoid this common JSF mistake
May 01, 2008
Don't access the database in a method that feeds a UIData component! (e.g. <h:dataTable>) I see this mistake being made all the time. It's bad advice and just plain bad practice. Don't do it!
What most people don't realize (perhaps because they are not watching the SQL log output in their ORM tool) is that value expressions are resolved more than once during the JSF life cycle--usually a lot more than once. Every time the value expression that feeds the UIData component is resolved in this scenario, your database takes a hit. On top of that, the result set could change depending on what you are retrieving and how you are doing it.
I will present a brief example and then show how to fix it using Seam.
Let's assume that you are using the class shown below to retrieve a list of User entities. The bean delegates the task of querying the database to a UserService instance.
package example;
public class UserListBean {
private UserService userService;
public List getUsers() {
return userService.findAll();
}
}
Now let's configure this class as a managed bean in the JSF configuration file. The UserService instance is injected using an EL value expression (perhaps using an EL resolver that pulls the bean from the Spring container). By the way, choosing the session scope for this bean would be another bad practice that I am not going to get into here. Take a look at Seam's conversion scope if you are tempted to use the session.
<manage-bean>
<managed-bean-name>userList</managed-bean-name>
<managed-bean-class>example.UserListBean</managed-bean-class>
<managed-bean-scope>request</managed-bean-scope>
<managed-property>
<property-name>userService</property-name>
<property-value>#{userService}</property-value>
</managed-property>
</manage-bean>
Feeding to getUsers() method from the UserListBean managed bean to a UIData component demonstrates the bad practice I am talking about.
<h:dataTable id="users" var="user" value="#{userList.users}">
<h:column>
<f:facet name="header">Name</f:facet>
#{user.name}
</h:column>
</h:dataTable>
In this case, the SQL log would show several queries during render and several additional queries on a JSF postback, assuming there is a UICommand component somewhere on the page.
The correct way to do this would be to bind the list to a context variable using a Seam factory component so that the list of users is populated exactly once per page, regardless of how many times the variable is resolved. Note that we are scrapping the managed bean definition and using a Seam component instead. The choice of page scope for the users variable ensures that the list is not refetched on a JSF postback either.
package example;
@Name("userList")
public class UserListBean {
@In
private UserService userService;
@Factory(value = "users", scope = ScopeType.PAGE)
public List getUsers() {
return userService.findAll();
}
}
Now we can use the context variable named users in the JSF view.
<h:dataTable id="users" var="user" value="#{users}">
<h:column>
<f:facet name="header">Name</f:facet>
#{user.name}
</h:column>
</h:dataTable>
Please don't fall into the aforementioned trap again! If you want to know how to inject Spring beans in your Seam components using bijection, check out my latest series, Spring into Seam.
9 Comments from the Peanut Gallery
1 | Posted by Leesy on May 01, 2008 at 05:39 AM EST
Great tip. Now if you'll excuse me I'm off to check all my code...
2 | Posted by Pierre Deruel on May 01, 2008 at 05:46 AM EST
Hi,
I think there's a mistake in the last block of code:
At the end of the first line when you use the value attribute. I think that it's value = "#userList.users"
3 | Posted by Dan Allen on May 01, 2008 at 10:49 AM EST
I'm quite sure that what I have in the <h:dataTable> is correct. A value expression is written as #{userList.users}.
4 | Posted by Roger Mori on May 01, 2008 at 12:25 PM EST
This article it is great clue. However, I'm scared because my application uses this approach, which was recomended on the book Jboss Seam (Michael Juan) as "...avoid excesive bijection" because it is slow. My application uses long running conversations, and nested conversations intensively, and have noted queries being performed several times. However, starting a use case with a long running conversation, and then keeping nested conversations has solved this problem.
Since I'm new to Seam, i'd like to have your opinion.
Thank you.
5 | Posted by Dan Allen on May 02, 2008 at 01:07 AM EST
@Roger, I'm pretty sure that Michael recommended the factory approach in his book JBoss Seam, based on what I see in the source code.
As @Roger pointed out, an alternative solution is to start a long running conversation and then store the data (in this case a list) in the conversation context. The main point here is that the retrieval of the data should be separated from the access of the data. You shouldn't care how many times JSF decides to ask for the data, as long as there isn't a direct channel to the database.
6 | Posted by cvl on May 05, 2008 at 01:12 PM EST
@Pierre Deruel imho your question is valid, as I assume you're talking about the following line <h:dataTable id="users" var="user" value="#{users}">
the expression #{users} could come from the value property of the @Factory annotation @Factory(value = "users" ..
Either way, great point by Dan, and another reason of considering seam.
7 | Posted by Dan Allen on May 05, 2008 at 05:59 PM EST
@cvl Yes, that is the proper explanation. Perhaps @Pierre didn't realize that the #{users} expression is being supplied by the @Factory annotation. Perfectly understandable if you aren't familiar with Seam.
If you want to know the ins and outs of Seam, I promise you that if you read Seam in Action, you won't be let down.
8 | Posted by Markus Kühle on July 15, 2008 at 07:40 AM EST
In your solution you say that you should use the factory component delivered by Seam. But this approach is framework dependent (here seam) and can't be used in every application.
Maybe the simplest solution to avoid multipe db queries is to save your list in your bean and only if the list is null perform a db query (works well for request scoped beans).
public class UserListBean { private UserService userService; prviate List users; public List getUsers() { if(users == null){ users = userService.findAll(); } return users; } }ok, ugly code.. With JSF 1.2 it is possible to use @PostConstruct annotation to initialise your list.
public class UserListBean { private UserService userService; prviate List users; @PostConstruct public void init() { users = userService.findAll(); } public List getUsers() { return users; } }I think the real problem lies in the understanding how JSF works and that getters will be called not only once per page. Thus you should not perform db actions or do anything else in your getter methods.
But i do agree that with the Seam factory component there exist a quite good answer for this problem. Without Seam you have to search for an other solution.
9 | Posted by Dan Allen on July 15, 2008 at 09:08 AM EST
@Markus, yes, I will grant you that the first solution does indeed work, but I purposely didn't mention it because I find it to be extremely ugly. I do, however, like the second solution. That's a good suggestion. It relies on JSF 1.2, but then again, so does Seam.
Personally, I think its crazy not to use Seam when using JSF, and even crazier to use JSF 1.1. That is my opinion, though.
Got Something to Say?