Portlet Form Controllers

[johnalewis]

We've been working on porting a fairly complex enterprise application from the Spring Servlet MVC framework to the Spring Portlet MVC framework. As Nick Lothian and Rainer Schmitz discovered, the Form Controller hierarchy is difficult to port over from the Servlet area to the Portlet area because of the two-phase nature of portlet requests. Since we were porting a large number of existing controllers descended from AbstractController, AbstractFormController, or SimpleFormController, it was critical to that all the former functionality of these parent be present and that our logic work the same as before.

Nick Lothian's version of SimplePortletFormController gave us a good starting point and helped us understand a lot of the issues with portlet development in general and with Spring portlets in particular. However, a lot of our controllers manage database content, so we needed proper separation between the action and render phase of the request, which this controller did not give us.

Rainer Schmitz's work went much further and provided the traditional hierarchy of controllers and further demonstrated the issues with separating out the two phases of the portlet request. However, these still did not give us the complete control that we needed. We especially needed a version of SimpleFormController that could handle the separation of the action logic from the render logic.

Over the past few months we have developed a new set of the AbstractController, BaseCommandController, AbstractFormController, and SimpleFormController classes that we think rigorously address all the issues with portlet development and that have allowed us to port over existing servlet controllers with relative ease.

We've included a portlet version of WebContentGenerator to give the controller classes their proper ancestry and provide control over things like content caching and access to the web application context and to centralized logging.

We've also create a new set of the binding classes (PortletRequestDataBinder, PortletRequestParameterPropertyValues and PortletRequestBindingException) that support special field markers, correct problems with form elements such as radio buttons and checkboxes, and handle redisplay of invalid submits of non-string parameters properly.

All of the classes are fully documented, including workflow descriptions in the controllers that cover the special nature of portlet development.

Our hope is that these classes can handle the needs of everyone working with form controllers in Spring portlets and that these can be merged into the sandbox source code.

The classes are posted in a zip file on the Spring Portlet Wiki page at:
http://opensource.atlassian.com/conf...isplay/JSR168/

The link to the file is (sorry for the long link):
http://opensource.atlassian.com/conf...ontrollers.zip

Please send me any questions or comments you may have about these classes. We are eager to support their adoption in the Spring Portlet community.

[swift]

John,

I had a look at your code, it looks really good!
I've written some more portlets since my last contribution and always disliked the way the action phase was handled (i. e. not handled). My indroduction of an ActionFormController cured some of the symptons (especially by enabling redirects etc), but your approach is much cleaner and avoids duplication of controller hierarchies.

However, I still dislike the enforced use of portlet session to propagate the form command from action to render phase. While it's unavoidable in some cases you can often get away without propagating an object to the render phase, i. e. data needed by the render phase can be propagated as request parameters.
(In this case render phase will just display the request parameters or executes some execution or data fetching by itself. In AbstractFormController you write:
Be especially aware that the action phase is called only once, but that the render phase will be called repeatedly by the portal -- it does this every time the page containing the portlet is updated, even if the activity is in some other portlet. That's not the whole truth: when portlet caching is turned on the render phase is only executed when the cache is expired. Thus it's possible to do some time consuming processing within the render phase without it being executed on ever page refresh.)

Anyway, your implementation opens up all options. I'm just looking for a way to avoid session use where it's not needed, or to at least clean up the session afterwards.
The latter probably can be achieved by removing FORM_RENDER_COMMAND_SESSION_ATTRIBUTE and FORM_RENDER_ERRORS_SESSION_ATTRIBUTE from the session in onSubmitRender() in a concrete controller implementation.
The former seems to be impossible. While I can imaging to provide a different version of AbstractFormController.setRenderCommand() to propagate command data to the render phase as request parameters instead of a session attribute this still leaves out the errors (BindException) object...

Cheers,
Rainer Schmitz

[johnalewis]

Rainer,

Thanks for the feedback -- sorry for the slow reply.

You've gone directly to my least favorite part of our solution -- putting the command object and the bind errors into the session. I agree that it is an unpleasant solution, but it is the only viable way we could come up with to do what we needed. We experimented with trying to pass everything forward via the render parameters, but we have some pretty complex command objects and that amounted to serializing and deserializing the objects -- very painful and slow. I'm hoping that a future version of the spec will allow us to pass forward objects to the render request and eliminate this need. I'm totally open to a different solution, but this seemed the only way to solve the general case using the current spec.

You are of course correct about the caching of the results of the render phase and that my description is an oversimplification of the real situation. My objective was to make sure users realize that the render code can get executed multiple times and to make sure it is idempotent. This seems to be a tough concept for some developers and so I wanted to make sure the warning is plain enough.

There is a good discussion going on in the Sping Developer mailing list about the future of the portlet framework and whether it should be part of the core Spring release or should be established as an official subproject. If you aren't already, you might want to start reading that list.

Let me know if you have any other feedback. I am very interested in making these controller classes as good as possible and hopefully making them part of the formal code for the framework.

We've also done some work with parameter-based controller mappings and I want to make sure that we reconcile our work with yours and get that into the formal code as well.

[swift]

Maybe we should add a flag to AbstractFormController to indicate wether the command object shall be stored in the session automatically or not. If the flag is set to false the implementing subclass can decide wether it uses sole response parameters or handles the session by itself.

I followed the discussion in the developer list and am looking forward to see some progress on this topic. Right now nobody seems in charge to incorporate the code we've provided.

Rainer

[johnalewis]

That sounds like a good idea. I'll have to look at what the ramifications are to the render-phase methods since the command and errors objects won't be available. I think we should keep the default as the current behavior so that it is as close to the servlet version as possible, but for those who know they don't need to command or errors object in the render phase they can eliminate the session usage.

I've almost got AbstractWizardFormController done now as well, so I'll be adding that one to the set. I've talked with Juergen directly about getting developer access to the sandbox so that I can start committing these things. Since no one seems to be driving the portlet framework out of the sandbox, I guess it's up to us!