Hi all --
I'm working my way through the various JSPs, in an effort to make them
a little more Stripes-friendly. The side-effect of re-working the JSPs
is that I get to do a little code-reviewing while I'm at it.
I've been looking at PreferencesTab.jsp in particular. It's a
little... complex. When about 3/4 of a JSP is scriptlet code, that
tends to suggest an opportunity to refactor.
Taking a look at the code, I've got a few reactions and
recommendations -- in order of priority.
First, I'd like to see the various user preferences bits become part
of a proper Java class, rather than continue to get bigger and bigger
as scriptlet code in the JSPs. As happens, the preferences themselves
are just String key/value pairs.
It seems to me that the logical thing to do is to make it a Map that
is returned (or settable) via something like WikiSession. Thus, in
JSPs or other classes, all you'd need to do is grab the WikiSession
reference and know that you could also retrieve the user prefs easily.
Doing it this way would also eliminate the need to store the
preferences in the session directly. This would also mean we could
dismantle the Preferences class and package and move its functions
into UserManager and SessionMonitor.
Recommended priority: for 2.8.
Second, we should start using JSP expression language more for prefs,
and use 'injection' techniques via filters to make things like
preferences available as variables. In particular, methods that must
execute on "all pages," and only execute once, are candidates for
injection. Example: Preferences.setupPreferences is called ONLY ONCE
in a JSP that all pages load: commonheader. It would be better to
simply use WikiServletFilter to inject the preferences Map as "$
{wikiPreferences}" or something similar.
Recommended priority: for 2.8.
Third, a long (long!) time ago, Janne and I discussed adding some
capabilities to the UserManager/UserDatabase classes that would allow
persistent storage of preferences. To keep things flexible, I think
the best way to store these things is to simply create an additional
table (in the JDBC case) that would stash key/value pairs. In the XML
case, we could add 'pref' elements as children of each 'user' element.
Doing it this way would enable us to store an unlimited number of
preferences, for anything we could imagine. I suspect Dirk can imagine
quite a lot of things. :)
Recommended priority: for 3.0.
Lastly, I'd like to understand the JSON side of things a bit more.
What are we doing with it?
I'd like to get the dev team's thoughts on all this. Before I do ANY
of this, I've got to get the Selenium webtests working, so that's
priority 1.
Andrew
