Mailing-List: contact jspwiki-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: jspwiki-dev@incubator.apache.org
Received-SPF: pass (athena.apache.org: domain of andrew.jaquith@mac.com
 designates 17.148.16.72 as permitted sender)
MIME-version: 1.0
Content-transfer-encoding: 7BIT
Content-type: text/plain; charset=US-ASCII; format=flowed; delsp=yes
Sender: andrew.jaquith@mac.com
Message-id: <4D2C088D-4208-4D49-A85D-543E83FBE4D2@mac.com>
From: Andrew Jaquith <andrew.jaquith@mac.com>
To: jspwiki-dev@incubator.apache.org
In-reply-to: <4114C84B-6A48-4873-9A0E-CF3A19A5E7A3@ecyrd.com>
Subject: Re: url rewriting supported?
Date: Mon, 07 Jul 2008 15:21:30 -0500
References: <485919CA.9000902@ops.co.at>
 <02C1EF06-C327-4A18-B967-1C410894D3B5@mac.com> <48593648.7010409@ops.co.at>
 <485967B4.8010401@abstrakt.de> <485A0DB3.1040703@ops.co.at>
 <2B145127-837F-4C98-85D5-CBA35EB6FC14@mac.com>
 <AC60A9BA-8825-4507-90FF-DF01A40F2F8D@ecyrd.com>
 <CFEB2EB8-A746-4DC6-8BC5-FCBF3A17468F@mac.com>
 <4114C84B-6A48-4873-9A0E-CF3A19A5E7A3@ecyrd.com>

> Two problems: regexps are insanely slow.  When doing URL rewrites on  
> something this link-intensive, you want to start caching the HTML.   
> Which in turn will play havoc with the plugin code generation.   
> Currently one of the biggest timeusers (if not the biggest) is the  
> DefaultURLConstructor.

Regexp's are slow, no question. But remember that you probably won't  
need them unless you are actually transforming URLs into something  
that is NOT the default case. The defaults are (and I suspect will  
continue to be) URLs that look like this:

/[webapp]/Wiki.jsp?page=Foo&version=2

The key is making the default scheme nice and fast. If you wanted to  
use [a different URL constructor|a filter with regexps], you could do  
that separately and pay the requisite penalty.
>
> The other problem is that I think some of our template/skin  
> generation stuff might break if the context is lost...

By "context" I assume you mean "wiki context", as in WikiContext.EDIT?  
As much as I'd like to do away with those, and move towards strong  
typing (prefer UserPreferencesActionBean.class to WikiContext.PREFS),  
the legacy contexts will be with us for a while.

At present, I am working on making my earlier 3.0/Stripes integration  
as backwards-compatible as possible. Example: I have a custom  
annotation called @WikiRequestContext that is class-level, and permits  
class authors to annotate the wiki request context string ("edit",  
"prefs") etc it has. It should work quite nicely -- all we do is  
slightly retrofit WikiContext so that WikiContext.PREFS gets its value  
from an annotation lookup rather than a text string  
(UserPreferencesActionBean 
.getAnnotation(WikiRequestContext.class).value()). It pushes  
responsibility for declaring stuff like this to the class itself. It  
also makes it easier for authors who want to write their own  
ActionBeans (no more hacking the Command classes...).

> However, Java5 varargs I think give us a nice exit.  How about
>
> WikiContext.generateURL( String content, String param1, String  
> value1, ... )
>
> with
>
> PARAM_PAGE = "page"
> PARAM_VERSION = "version"
>
> and the JSP param could be "Wiki.jsp", or "rss.jsp" or "attach" or  
> "scripts/jspwiki.js" - direct servlet/JSP/resource names.  E.g.
>
> ...
> wikiContext.generateURL( "Wiki.jsp", PARAM_PAGE, newPage,  
> PARAM_VERSION, 1, "skin", "raw" );
> ...
>
> We could then have a SimpleURLConstructor (which just attaches the  
> params one after the other), a RegexpURLConstructor (which extends  
> the SimpleURLConstructor and just passes them through a regexp  
> list), and so on.  I like the fact that code can be optimized in  
> these cases quite a lot, but rules cannot.

Indeed. You might want to take a look at the Stripes URLBuilder class.  
You instantiate it with a locale first, and either a path or an  
ActionBean class:

    builder = new URLBuilder(locale, "/Wiki.jsp" );

or

    builder = new URLBuilder(locale, ViewActionBean.class );

Then, you add parameters either one-by-one or as an array:

    builder.addParameter( "page", "Foo" );

Note that the second argument is a vararg, because parameters can be  
multi-valued.

After that, the method build() returns the correctly formatted and  
encoded String.

Now, I am not trying to persuade you to use the Stripes URLBuilder.  
However, I mention it in detail to show you that it shares one  
property with what you outlined. Namely, the principle of passing  
parameters as separate arguments rather than the way we do now, as a  
monolithic blob. This is a nice departure from what we have done, but  
it's going to require us to blow up the URLConstructor class (as we  
know it) and substitute something else. Either SimpleURLConstructor or  
the Stripes alternative. I happen to like the Stripes URLBuilder  
because it has taken localization and multi-valued properties into  
account, and does not rely on "magic parameters" where authors would  
have to remember to supply an even number of varargs (the first arg is  
the param, the second is the value). Anyway, you might want to take a  
look at the source code for URLBuilder. :)

One thing I think we should look carefully at is whether we should  
keep the getURL() functions onto the WikiContext (aka the ActionBean)  
or move it elsewhere. My feeling is that we should deprecate  
WikiContext-level getURL() type methods and move them to the  
ActionBeanContext, which is associated with each ActionBean. That's  
where all of the HTTP-related stuff is, and I would rather keep the  
ActionBeans focused on functionality (bean properties and events)  
rather than the HTTP-related aspects of how they are presented.  
WikiContext.getURL() will be around for a while (for backwards  
compatibility reasons) but it should delegate to  
WikiActionBeanContext.getURL(). Same for WikiContext.getHttpRequest(),  
which should delegate to ActionBean.getRequest().

(In case you were wondering: the WikiActionBeanFactory, and the  
StripesFilter, both guarantee that every ActionBean [WikiContext] has  
an associated ActionBeanContext...)

> I don't think we really need to carry the notion of the request  
> context that much further,  as it's far clearer to point directly to  
> the JSP or resource instead of an abstract name. Unfortunately it  
> means that things like the ContentTag will break.  But then again,  
> the stripes-based UI is probably going to break it anyway.

I agree, mostly. The act of moving functions that are grouped together  
into ActionBeans means that we are better off simply specifying the  
bean itself, or a class reference to it, instead of the String (the  
request context). Using strong typing (the class) rather than strings  
means we can do other things too. For example, the ActionBean could  
have an annotation that names the content template JSP. Or (my  
preference) it could be calculated as a default value, and you use  
page markup or annotations for those cases where the defaults don't  
work.

Example: in my local builds each ActionBean has an annotation called  
@URLBinding that says how Stripes should map the 'bean to URLs. For  
example ViewActionBean's URLBinding is "/Wiki.action". The top-level  
JSP is easily calculated by chopping off .action and attaching ".jsp".  
Thus: Wiki.jsp. The content page is *generally* the top-level JSP name  
plus "Content". So -- that's the rule. Look up the URLBinding, chop  
off .action, and attach the suffix. "/WikiContent.jsp" would be the  
default by convention. (Except that Wiki.jsp's content page is  
PageContent.jsp... so for ContentTag you tell it to use  
PageContent.jsp.)

Pulling back from the detail, though, it is clear that certain things  
are going to break. But maybe less than we thought. My first attempt  
at Stripes integration made no attempt to be backwards compatible, and  
indeed, I made it deliberately incompatible with those features I did  
not like (e.g. URLConstructors, Commands). That was clearly too  
ambitious. My approach now is to keep as much compatibility as  
possible but aggressively deprecate methods or fields where the  
alternatives are better. WikiContext request contexts are one example.

In my copious free time, I have been taking another whack at 3.0. The  
primary incompatibilities are the removal of the WikiContext  
contructor (use a factory instead) and the related setRequestContext()  
method. In fact, if it is not too late, I would not mind removing the  
WikiContext constructor NOW, in 2.8, and forcing the use of  
WikiEngine.createContext(). That's only a few cases (15-20?), and  
there is little/no exposure in the JSPs.

The other major difficulty is the historical "overloading" of  
WikiContext to include non-page-related activities like groups and  
user maintenance. This is a little harder to tackle, but it should  
only affect about 30-40 places in the code.

Again, in the short term the goal is backwards compatibility, even if  
we carry forward some things that we will deprecate. My previous  
attempt modified about 300 classes; I think I can get that down to  
about 50. JSPs are the wildcard here, but here too there are some  
steps we can take to (hopefully) limit the changes to about 20 JSPs or  
so, and keep them relatively minor also.

Janne, sorry for the ramble... I have had lots of "plane time" lately,  
and have been trying to get my 3.0 builds re-synced with the 2.8  
branch. Seemed like a good time to bring you up to date.

Andrew