incubator-jspwiki-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew R Jaquith <andrew.r.jaqu...@gmail.com>
Subject Re: WikiPage constructors
Date Wed, 10 Dec 2008 23:07:09 GMT
WikiContext should not be overloaded more than it is. Definitely a bad  
idea to have more than one WikiContext kicking around at a time, since  
they are performing double-duty now as Stripes ActionBeanContexts.

WikiPage, it seems to me, should be an interface, just like  
WikiContext has become. Factory methods (in PageManager, perhaps?) are  
better anyway.

It won't be that fun to change (about 75 cases where the WikiPage  
constructor is used), but that's ok. There are only 2 references in  
the 'plugin' package, suggesting that 3rd parties won't have to do too  
much, if anything. And there are no references in any JSPs to the  
constructor. Other than developers who work on the core, nobody else  
would be affected.

Andrew


On Dec 10, 2008, at 5:55 PM, Janne Jalkanen wrote:

> Folks,
>
> there is a minor problem with WikiPages and JCR.
>
> In the old days, the recommended pattern was to create a WikiPage  
> with new WikiPage(WikiEngine,String).  The problem is that with JCR,  
> this simply won't be possible, since the WikiPage ceases to be a  
> passive retainer of information, and will actually store a reference  
> to the JCR Node.  This has a couple of impacts:
>
> * WikiPages can no longer be cached (which wasn't a smart thing to  
> do previously either, though some internal components did so, mostly  
> to avoid hitting the repository.  Which won't of course be a problem  
> in the future.)
> * WikiPages cannot be directly constructed anymore - instead,  
> WikiPages will need to be fetched using WikiEngine.getPage() or  
> created with WikiEngine.createPage().  Otherwise, the JCR Node  
> reference would be null, and there would be a bunch of NPEs flying  
> around the code.
> * WikiPages will not be thread-safe (though no guarantees were given  
> previously either - they just happened to be)
>
> [In fact, that's exactly what it does right now, if you check my  
> latest commits to the jcr branch.  We have used the new WikiPage() - 
> idiom pretty liberally everywhere, and that is currently causing  
> some pain.]
>
> Now, this change would not be without its merits - it would allow  
> WikiPage to become an interface, and that is very nice (and almost  
> mandatory) if we consider the fact that we would like to have a  
> stable API.  Also, any trick I can think of around this problem  
> either involves overloading WikiContext more, or creating a  
> labyrinth of if( getNode() == null )  
> setNode 
> ( m_engine 
> .getContentManager 
> ().getSession 
> (m_engine 
> .getWikiContextFactory 
> ().newViewContext(null,null,this)).getRootNode().getNode(getName()))  
> -calls in most of the WikiPage methods.  Which could be somewhat  
> slow, and would also create two WikiContexts for each request...
>
> How do the fellow developers feel about this change?  Does this pose  
> insurmountable problems for anyone, or change any plans?
>
> /Janne


Mime
View raw message