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: WikiActionBeans & context factories (changed subject from ClassUtil thread)
Date Sun, 27 Jan 2008 17:36:01 GMT
Janne -- thanks for suffering through that rather lengthy e-mail, and  
for replying in detail. I've added a few clarifying comments below.

>> The new class hierarchy for WikiContext and related types, for 3.0,  
>> looks like this:
>>
>>    WikiActionBean extends ActionBean [Stripes]
>>
>>    AbstractActionBean implements WikiActionBean
>>     |
>>     +--WikiContext extends AbstractActionBean
>>     |   |
>>     |   +--ViewActionBean extends WikiContext
>>     |   +--EditActionBean extends WikiContext
>>    ... ... ...
>>     +--GroupActionBean extends AbstractActionBean
>>     |
>>     +--UserPreferencesActionBean extends AbstractActionBean
>>    ...
>>
>> [NOTE: it would be nice if we could deprecate "WikiContext" in  
>> favor of, for example, "PageActionBean" to clearly denote that it's  
>> about page activities. Maybe PageActionBean becomes the interface,  
>> and we start coding to that...]
>
> I am not totally sure of this.  I would very much like to keep our  
> API signatures the same as much as possible, and that means lugging  
> WikiContext around under the name "WikiContext".

All right, all right. Damned backwards compatibility... :)

> Also, since GroupAction might also need to render something, or  
> UserPreferences, you will need to invent a WikiContext for them as  
> well.  So wouldn't it make sense to just make WikiContext ==  
> AbstractActionBean?
>
> Or more likely - make AbstractActionBean the concrete subclass, and  
> WikiContext the interface towards lower layers.

AbstractActionBean is going to need some basic "scaffolding" that will  
affect all ActionBeans, whether they are a page-related ActionBean  
(aka WikiContext) or not. So I don't think WikiContext ==  
AbstractActionBean.

The real question is, and I think you hit on it, is whether  
WikiContext is an interface or an abstract subclass. It doesn't matter  
to me. I suppose I have a slight preference for making it an abstract  
subclass of AbstractActionBean, and maybe introducing a page-specific  
interface (e.g., "PageActionBean"). Alternatively, we could also make  
WikiContext an interface, and put the pagecontext-specific stuff in  
"AbstractWikiContext." Dunno.

Either way, the class hierarchy probably won't be *too* different from  
what I've proposed, no? :)

> I think I would like to introduce WikiContextFactory already in 2.8,  
> and just deprecate the WikiContext constructors; then make them  
> protected altogether in 3.0.

That's a good idea. We don't NEED to keep things compatible with what  
I've got in own local branch, but if you wanted to, might I suggest  
putting it in the "action" package, calling it WikiActionBeanFactory,  
and with the following regular (non-static) methods?

   public WikiContext newActionBean( HttpServletRequest request,  
HttpServletResponse response, WikiPage page)

   public WikiContext newActionBean( WikiPage page)

Calling these methods "newWikiContext" or "createWikiContext" is also  
fine, because they only return page-oriented contexts. :)

Putting all of the ActionBeans in the "action" package, including the  
WikiActionBeanFactory, means that we could make the actual class  
constructors protected later on.

In 3.0, by the way, WikiActionBeanFactory would also gain another  
method...

     public WikiActionBean newActionBean(HttpServletRequest request,  
HttpServletResponse response, Class<? extends WikiActionBean>  
beanClass) throws WikiException

...for all WikiActionBean classes generally. This is what I've got  
locally now...

>> - The setRequestContext() method disappears (it was only used in  
>> like 5 places anyway), because the WikiContext subclasses *are* the  
>> contexts. getReqeustContext() merely looks up a special JSPWiki  
>> class-level annotation that specifies the context. (For example:  
>> @WikiRequestContext("view"))
>
> Yes, exactly.

Note: I view the "request context" idea as being a transition step.  
When we break up the different page-related WikiContexts (and group  
contexts...) into separate concrete classes, you can simply do  
"instanceof classname" to see if you are in the right context. Request  
contexts have served their purpose well, but we ought to be  
encouraging developer to use something more typesafe in the 3.0  
timeframe... in this case, the class itself. :)

> Also, I'd like to resolve https://issues.apache.org/jira/browse/JSPWIKI-117 
>  somehow.  One of the ideas would be that we would have a proper  
> WikiPage also associated with each action.

Mmph. As appealing as the idea might be to associate every activity in  
JSPWiki with a "wiki page," certain activities (group, user prefs) are  
NOT pages. They are different things entirely -- actions. Also,  
another implication of equating actions with pages is that you can  
have more than one action per page! I can think of lots of scenarios  
where you might want render multiple pages together, or display  
multiple admin tasks on the same JSP. For this reason, I think it's  
best NOT to equate 1 JSP : 1 ActionBean : 1 "wiki page" (specially  
named or not). It's more like 1 : many : many.

> users to use as they please.  I like the fact that you can say, in  
> wikitext, "You can login using the [System/Login] -page", instead of  
> having to cut-n-paste the whole URL.

I like this feature too, but it's probably fairer to say that [System/ 
Login] refer to specific JSPs that compose one or more ActionBeans  
together.

>> - hasAccess()/requiredPermission(). This are handled by our own  
>> Stripes interceptor/controller (WikiInterceptor) in combination  
>> with event-level annotations. So these methods should absolutely be  
>> eliminated.
>
> I am not totally certain of this.  There needs to be a way to ask  
> the system "does user X has Y access to page Z?"  Maybe WikiContext  
> is not the right place for them, but there needs to be a good, clear  
> method for doing this.

Agreed. In my opinion, we are going to get better results (simpler,  
more flexible) if we handle this through annotations at the ActionBean  
level. What we need to make sure about is that we can do this, even  
outside of a classic servlet context. The servlet scenario works quite  
well, though, via the Interceptor idea.

>> <stripes:useActionBean  
>> beanclass="com.ecyrd.jspwiki.action.____ActionBean"/>
>
> This is, IMO, really the only way.  Stripes documentation recommends  
> another way, but in my experiments I found your way to be simply the  
> best one.   Least confusion all around.

It's also dead simple for JSP developers. And it allows multiple  
ActionBeans on a single JSP. In my local branch, I've been working on  
a technique for stashing more than one ActionBean in the request, and  
retrieving it in WikiTagBase. Works pretty well!

> Stripes has a problem in the sense that the distribution of work  
> between the ActionBean and the corresponding JSP page is not always  
> clear.  Sometimes you use the JSP page as the entry point, sometimes  
> the ActionBean (especially the validator tends to sometimes give the  
> wrong URLs, exposing the underlying actions).

This might be one of those things that is best handled through  
convention and clear devleoper guidelines. Like you, I feel the  
<useActionBean> tag is the best "entry" technique.

>> The question is, how do we deal with this when embedding; is it  
>> sufficient just to have a factory method which has an explicit  
>> argument for the WikiEngine / WikiPage?

As I've suggested above, the WikiActionBeanFactory factory method...

public WikiContext newActionBean( WikiPage page)

...could do this. That's what my local build does.

>> 2) Injection by DispatcherServlet due to POST/GET to /dispatcher, / 
>> action/* or *.action URLs. This method appears to be used, in  
>> particular, by generated Stripes form elements. It is rare that a  
>> user would actually specify one of these URLs directly.
>
> Yes, except for validation.  Stripes will automatically get you the  
> *.action URL, if the content does not validate, which can be  
> confusing.  I haven't yet found a way around this.

I don't think this is too bad, really. The user probably isn't going  
to pay attention to the fact that it's posted to a .action URL.

> The question is - does this JSTL mechanism work outside a HTTP  
> context?

Great question! I don't know the complete answer yet, but it's clear  
that we need to ensure that it works outside of an HTTP context. What  
I will say, though, is that I've successfully built unit tests that  
work outside of a servlet environment. So that does suggest that there  
is at least one technique that might work for embedding. We'll need to  
do some work here, but it shouldn't be too bad (see http://stripesframework.org/jira/browse/STS-324)

. As it happens, I know Tim Fennell (lead developer of Stripes), so I  
could always put the arm on him. :)

> Reminds me, we also might want to stash the page and the WikiContext  
> into the http request for brevity - it's a pain to always say "$ 
> {actionBean.context.wikiContext.page.version}" - much easier to say  
> "${page.version}".

The strategy I've taken so far, in my test builds, has been to have  
the Stripes WikiInterceptor stash the WikiActionBean in (which might  
well be a WikiContext!) in the *page context* under the name  
"wikiActionBean" (WikiTagBase.ACTIONBEAN, really). Later, WikiTagBase  
pulls it back out of the page context. There are probably some other  
ways to do this; regardless, we can, I am sure, find out some very  
simple page attribute names that would make JSTL expressions nice and  
easy, like ${wikiContext} or ${wikiPage}. To be determined!

> BTW, I've been meaning to start a "JSPWiki 3.0 Design" page on  
> jspwiki.org...  It might be a good idea to create one.

I'll start contributing all of this stuff, then...

>
>
> /Janne
>
>


Mime
View raw message