Wednesday, March 25, 2009

Wicket Patterns and Pitfalls #3

This is the third article in a series of articles about common patterns and pitfalls when using Wicket (http://wicket.apache.org). Most of these patterns apply only when using Wicket in conjunction with Spring and Hibernate and these might be very specific, but others are more general. See also Wicket Pitfalls and Patterns #2.


OpenSessionInView-LoadableDetachableModel-Pattern and its Pitfalls



Environment: Wicket, Spring, Hibernate, OpenSessionInView

This article targets some pitfalls when using the OpenSessionInView (OSIV) pattern in combination with LoadableDetachableModels (LDM) and Hibernate. This combination is a very commonly used pattern, because it greatly simplifies handling of Hibernate entities (no more detached entities), and it relieves us from the pain of dealing with LazyInitializationExceptions (how many hours did we spend fixing these?). But although this pattern is very useful and it's highly recommended, there might be some tricky side effects, which have to be considered. These are described in this article.

The OSIV-LDM-Pattern
In order to explain the benefits of the OSIV-LDM-Pattern, let's take a look at a simple example. Assume we have a Hibernate entity Person with three properties firstName, lastName and username:

@Entity
public class Person {
@Id @GeneratedValue(strategy = GenerationType.AUTO)
private Long id;
private String lastName;
private String firstName;
private String username;
...
}

A simple page to edit a person could be implemented like this:

public PersonEditPage(final Long personId) {

super(new PersonModel(personId));

Form form = new Form("form",
new CompoundPropertyModel(getModel()));
add(form);
form.add(new TextField("firstName"));
form.add(new TextField("lastName"));
form.add(new TextField("username"));

form.add(new Button("saveButton",
new Model("Save")) {

public void onSubmit() {
Person person = (Person) PersonEditPage
.this.getModelObject();
// save person
}

});
}

The page simply consists of three text fields for the person's properties and a button to submit the changes. In this example, PersonModel is a LoadableDetachableModel for Person entities. This can be implemented easily, for example by inheriting from GenericLoadableDetachableModel introduced in the previous article:

public class PersonModel
extends GenericLoadableDetachableModel {
public PersonModel(Long id) {
super(Person.class, id);
}
}

Now, in this situation, the combination of the LDM with an OSIV-Filter (i.e. the OSIV-LDM-Pattern) is very elegant for serveral reasons.

  • The LDM keeps session size small, as the entities are not put into the HTTP session but are always fetched from the database on demand instead (if the haven't been fetched already).

  • As a result, entities are not detached (Hibernate detached) between requests.

  • The OSIV pattern opens a Hibernate session at the beginning of a request and keeps it open until all components have been rendered. So within a request, entities are not detached, as well.

  • Consequently, when OSIV is used with LDMs, entities will never be in detached state (neither within a request, nor between request).


In the example above, the PersonModel is attached to the page at page construction. During the render phase, the first time the model object is accessed it is fetched from the database. Until the end of the render phase, the OSIV keeps the Hibernate session open, so the entity is not detached. During event processing, as in Button.onSubmit(), Wicket first loads the model object, so it is fetched from the database (it is in persistent state), and then applies any changes to it. So, in Button.onSubmit() above, the person object is still attached to the Hibernate session and changes have been applied. Consequently, we never have to deal with detached entities or LazyInitializationExceptions at any point in a request cycle.

OSIV-LDM-Pitfall 1
But there are some things to be aware of. In the example above, when Button.onSubmit() is called, Wicket has pulled the entity from the model (from the database) before and has applied the converted and possibly validated input to it, for example the user could have changed the username of the person. At this moment, the entity is still attached to the Hibernate session. So, there is nothing more to do to save the changes to the database, because at the end of the request, the session will be closed by the OSIV-Filter and the changes will be flushed to the database ... But if we run the example above, we'll eventually recognize that the changes are not persisted to the database. So, let's try a bit harder and call session.saveOrUpdate(person):

public void onSubmit() {
Person person = (Person) PersonEditPage
.this.getModelObject();
sessionFactory.getCurrentSession()
.saveOrUpdate(person);
}

Nothing. The changes are still not persisted to the database.

The reason is, that the OSIV-Filter will by default set the flush mode of the session to FlushMode.MANUAL, which means that the session will not get flushed (synchronized to the db) on close or in between. To solve the problem we have to flush the session manually by calling session.flush():

public void onSubmit() {
Person person = (Person) PersonEditPage
.this.getModelObject();
sessionFactory.getCurrentSession()
.saveOrUpdate(person);
sessionFactory.getCurrentSession().flush();
}

Okay, this was an easy one. Let's tackle the next pitfall.

OSIV-LDM-Pitfall 2
The example above uses kind of a bad practice. Instead of handling persistent state in a Wicket component, it's recommended to move this code into the service layer, which takes care of transaction handling. [The OSIV-Filter does not do any transaction handling, everything you do in Wicket components is out of any transactional scope.] Let's improve our code by checking if the user changed the username of the person to a username that already exists in the database. If so, the application should reject the change. For this purpose the submit method calls a transactional person service:

public void onSubmit() {
Person person = (Person) PersonEditPage
.this.getModelObject();
try {
personService.updatePersonDetails(person);
// show success message
} catch (DuplicateUsernameException e) {
error("Username " + e.getUsername()
+ " already exists");
// show error message
}
}

The service method PersonService.updatePersonDetails(person) is called to update the person details. This method first queries all persons with the same username as the person provided as the argument from the databse. If there is no other person with this username, the person is updated with the new username by calling session.saveOrUpdate(). Otherwise, if there is a person with the same username, a DuplicateUsernameException is thrown:

@Service @Transactional
public class PersonService implements IPersonService {

@Autowired
private IPersonDao personDao;

public void updatePersonDetails(Person person)
throws DuplicateUsernameException {

// Check for duplicate username
Person checkPerson = personDao
.getPersonByUsername(person.getUsername());

if (checkPerson == null || checkPerson == person) {
personDao.saveOrUpdate(person);
} else {
throw new DuplicateUsernameException(
person.getUsername());
}
}
}

If we give it a try, and change the username of one person to the username of another person, a DuplicateUsernameException is thrown and it looks like everything works fine. But somehow the person's new username is persisted to the database, although this username already exists! (Assume for this example that there is no database unique constraint on username). The DuplicateUsernameException was thrown, so obviously personDao.saveOrUpdate(person) has never been called, so how did the changes get persisted?

The key is, that the Spring transaction manager has changed the session's flush mode from FlushMode.MANUAL (set by the OSIV-Filter) to FlushMode.AUTO at the beginning of the transaction. In this mode, Hibernate sometimes flushes the session before queries to ensure that the query does not return stale data. In the example, Hibernate flushes the session before the query executed by personDao.getPersonByUsername(person.getUsername()). The result of this flushing is, that person, which includes the new username, is also flushed to the database, unexpectedly. This is, because of the OSIV-LDM-Pattern the person has been attached to the Hibernate session from the beginning of the request. If it had been detached, it would not have been flushed.

Note: The same would also happen, if a query is called from onSubmit() and then a service is called.

The best advice, I can give, is to always be aware of this side effect of the OSIV-LDM-Pattern. When you have identified a problematic case, there are some ways to get around these problems. One is to validate the input before it gets applied to the model object. In the case above, we could implement a Wicket validator, that checks for duplicate usernames. Wicket validators are executed before the changes are applied to the model object, so if the username already exists, the username of the person would not be changed. Another, but not quite elegant way, is to use some kind of DTO like helper objects, which are used as model objects instead of the entities themselves. The changes have to be transferred from the DTO to the entity manually then.

Conclusion
The OSIV-LDM-Pattern is a very elegant way to handle Hibernate entities. Sometimes the problems above are quite hard to identify, but in real world situations they are quite rare and in most cases they can be tackled easily.

Finally, I'd like to mention, that the OSIV pattern has some further consequences, e.g. related to session handling after exceptions/transaction rollbacks, and because only one session is used for each request. But these topics are out of the scope of this article.


Comments are welcome.

5 comments:

Anonymous said...

I think this Problem is not special to wicket.

I think the better solution looks like:

@Transactional
... changeUserName(User user,String newUsername)
{
if (!userWithUsernameExists(newUsername))
{
user.setUsername(newUsername);
update(user);
}
else
{
// throw some error
}
}

.. Why do you call saveOrUpdate to modify a username? Maybe i did'nt get it all... :)

Nick Wiedenbrück said...

@michel: That's right, the problem really isn't special only to Wicket, but it also appears with other web frameworks, that support OSIV and have some kind of variation of the LDM.

Your suggestion implies, that your solution does not use a LDM, or any Model to whicht the username input field is bound. But that's really one of the strengths of wicket. But anyway, your example would be a solution, right. It's kind of like the DTO solution I described in the article.

Thanks for the comment, michel.

Garidan said...

I think the right thing (even if boring to manage) is to use a DTO, because the object edited in the form should NOT be an entity until validation occurs. More code to write but less problems, easier to understand and clean logic.

Another way, could be (but I never tried it) call session.clear() when validation detects errors, so to clear pending changes (javadoc:"Completely clear the session. Evict all loaded instances and cancel all pending saves, updates and deletions.").

Ryan said...

session.clear() will cause some problems as well... for one thing you will get LazyInit exceptions for anything called after the clear. One technique I've used, and I am not sure if it is the best way, is to make all my transactions read-only using Spring. Then you either have a SaveDAO or save methods in your daos that are marked as "REQUIRES_NEW" transaction and are not read only. These methods will take the object and merge it, thus saving it to the database. This has worked well for me so far. You will want save methods that can take collections as well so you can save multiple objects efficiently in one transaction.

I'm still not sure if this should be considered a good practice?

Unknown said...

@Transactional(rollbackFor=Exception.class) will solve the problem; the update will be rolled back when the DuplicateUsernameException is thrown.