incubator-jspwiki-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Jaquith <andrew.jaqu...@mac.com>
Subject Re: Shed light on how PageManager saves pages?
Date Wed, 25 Jun 2008 04:16:42 GMT
> Yup, we've got a problem.  Most things in the WikiContext are such  
> that they do not survive serialization - WikiEngine,  
> HttpServletRequest, even WikiPage (due to interactions with the  
> CachingManager).
>
> It's a good question whether they even do survive leaving the  
> WikiSession and being executed in somebody else's context (the  
> HttpServletRequest comes to mind).  Now, since  
> SavePageTask.execute() invokes the parser, that means that user  
> plugins are also invoked, and if you've got someone relying on  
> HttpServletRequest parameters at the time of save parsing - well,  
> boom for them. (Not that I would see that anyone would do that.)

Oy.

Thinking out loud: Might I recommend that we encourage plugin  
developers NOT to assume the presence of an HTTP request in 3.0? It's  
going to be much cleaner simply to grab whatever parameters Stripes  
binds to the ActionBean via their accessors (they will correctly  
typed, among other things). E.g., if Stripes binds the "page" request  
parameter to an ActionBean's property of type WikiPage, why not just  
use getPage() to get it, rather than go through trouble of getting the  
request from the context, calling getParameter(), looking for nulls  
and misspellings etc...? So much could go wrong, and Stripes takes  
care of this for you. So, in 3.0 a developer commandment might be:

Thy plugins shalt not assume that HttpServletRequests are available,  
and shalt not parse its parameters.

In 2.8, clearly we **can't** make a "don't depend on the request"  
recommendation...

Aside: one of the things that pleases me the most about Stripes is  
that its parameter-binding glue allows us to decouple the request from  
the WikiContext. Likewise, the Resolution and related UrlBuilder  
classes decouple it from the response.

> The reason we call textToHTML() is to make sure the access rules and  
> metadata are correct after save.  This is needed because of the  
> CachingProvider, and is one of the reasons why our metadata system  
> is so crappy and kludgy and I can't wait to start fixing it in 3.0...

Hah! I knew it was something like that. It's never good to call a  
method just because it has magic "side effects"; ideally we'd have a  
non-kluge-y way to do thus... But there is no point in extracting the  
magic bits from the renderer code that (in essence) do the metadata  
refreshing. I completely agree that we can't solve this until 3.0.

> Looking at the issues involved, I think it probably makes more sense  
> to stuff this to 3.0 and see if we can clarify the role of the  
> WikiContext a bit more.  If WikiContext is going to be a Stripes  
> ActionBeanContext in the future, we've got to make sure that is  
> serializable too, of which I am not too sure about.

I do not completely understand what you mean here, but I do not think  
that WikiContext should be serializable. Today, it contains references  
to too several things that have time-dependent states  
(HttpServletRequests go out of scope; HttpSessions expire, etc.). In  
the future, it would be extremely difficult to make WikiContexts/ 
ActionBeans serializable because that would mean all of their  
properties would have to be serializable too. Not too hard with a  
concrete class, but probably impossible with an interface, which is  
what ActionBean is... would impede flexibility way too much.

Mime
View raw message