When you have a page that needs some data to display (like a list of items), you usually populate the request in your struts action. But if that page is also a form.. things start to break down.
Here's what I mean:
<action path="/foo" type="FooAction" input="foo.jsp">
<forward name="viewSuccess" path="foo.jsp"/>
<forward name="updateSuccess" path="next.jsp"/>
<forward name="updateFailure" path="foo.jsp"/>
</action>
public class FooAction extends DispatchAction {
public view(...) {
request.setAttribute( "foos", businessLogic.getFoos() );
BarForm bar = (BarForm)form;
bar.populate( businessLogic.getBar() );
return viewSuccess;
}
public update(...) {
BarForm bar = (BarForm)form;
try {
businessLogic.doSomething( bar.getSomething() );
} catch( Exception e ) {
handleError( e );
return updateFailure;
}
return updateSuccess;
}
}
The problem? Well, if you have an error (with struts-validator or with the business logic), things will get forwarded to foo.jsp, but it will not get the data needed to properly render the page.
One solution is to make a common method for the loading the data that foo.jsp needs into the request. Although this works fine. I don't like it for two reasons:
-You need to keep calling this method for each method in your dispatch action.
-It breaks encapsulation.
If you change the action definition as follows, you get a better page, but it has a new problem, can you guess it?
<action path="/foo" type="FooAction" input="/foo.do&method=view">
<forward name="viewSuccess" path="foo.jsp"/>
<forward name="updateSuccess" path="next.jsp"/>
<forward name="updateFailure" path="/foo.do&method=view"/>
</action>
If you guessed that any changes the user made to the form are lost if an error occures, you're right! Since the view() method blindly overwrites the form's contents from the business logic, any changes made are blown away.
The solution I use is basically this:
public class FooAction extends DispatchAction {
public view(...) {
request.setAttribute( "foos", businessLogic.getFoos() );
if( !hasErrors( request ) ) {
BarForm bar = (BarForm)form;
bar.populate( businessLogic.getBar() );
}
return viewSuccess;
}
...
public boolean hasErrors( HttpServletRequest request ) {
ActionErrors errors = request.getAttribute( Globals.ERROR_KEY );
return errors != null && errors.size() > 0;
}
}
This seems to work fine for me. I think the best solution is to have a common abstract base class that can enforce this pattern for you:
public abstract class FormAction extends DispatchAction {
public ActionForward view(...) {
prepareRequest(...);
if( !hasErrors( request ) ) {
prepareForm(...);
}
return viewSuccess;
}
public ActionForward update() {
return updateForm(...);
}
public void prepareRequest(...) {}
public abstract prepareForm(...);
public abstract updateForm(...);
public boolean hasErrors( HttpServletRequest request ) {
ActionErrors errors = request.getAttribute( Globals.ERROR_KEY );
return errors != null && errors.size() > 0;
}
}
Update: Changed some of the method names in the abstract class based on Billy's comments.
