new pages

[Janne Jalkanen <Ja...@ecyrd.com>]

Heya!

Could you Andrew walk me through exactly how new pages are supposed to  
be created? As far as I can tell, everything just explodes if you try  
to edit a page which does not exist, and there really isn't a place to  
call ContentManager.addPage() either, since the WikiPage gets set in  
WikiPageConverter, and non-existent page is considered an error.

/Janne

Re: new pages

[Andrew Jaquith <an...@gmail.com>]

Janne --

I just checked in a big batch that fixes many of the issues relating
to creating new pages. Editing works again, among other things. I
changed the function of the WikiPageTypeConverter as described in
option (1), with special pages constituting a special case. :)

Andrew

On Fri, Apr 17, 2009 at 2:44 PM, Janne Jalkanen
<ja...@ecyrd.com> wrote:
>>
>> The JCR Javadocs seem to be ambiguous about whether nodes are
>> persisted when they are created, but they are very clear that they
>> must be persisted when Node.save() is called. I did, indeed, just need
>> to call ContentManager.addPage(). Seems to work just dandy.
>
> Actually, JSR-170 spec says about Node.addNode()
>
> "An ItemExistsException will be thrown either immediately (by this method),
> or on save, if an item at the specified path already exists and same name
> siblings are not allowed. Implementations may differ on when this validation
> is performed. "
>
> The end result might be a bit confusing if same name siblings were allowed,
> and the JCR repo would just silently add multiple copies.  It means that we
> should choose a nodetype which does not allow same name siblings. At the
> moment everything is stored with nt:unstructured, which does allow such
> beasts.
>
> I'm not sure whether calling addNode() would actually work with other repos
> than Priha. Priha does not validate until save(). But now is difficult to
> think what a good solution would be, too much noise around.
>
> /Janne

Re: new pages

[Janne Jalkanen <ja...@ecyrd.com>]

>
> The JCR Javadocs seem to be ambiguous about whether nodes are
> persisted when they are created, but they are very clear that they
> must be persisted when Node.save() is called. I did, indeed, just need
> to call ContentManager.addPage(). Seems to work just dandy.

Actually, JSR-170 spec says about Node.addNode()

"An ItemExistsException will be thrown either immediately (by this  
method), or on save, if an item at the specified path already exists  
and same name siblings are not allowed. Implementations may differ on  
when this validation is performed. "

The end result might be a bit confusing if same name siblings were  
allowed, and the JCR repo would just silently add multiple copies.  It  
means that we should choose a nodetype which does not allow same name  
siblings. At the moment everything is stored with nt:unstructured,  
which does allow such beasts.

I'm not sure whether calling addNode() would actually work with other  
repos than Priha. Priha does not validate until save(). But now is  
difficult to think what a good solution would be, too much noise around.

/Janne

Re: new pages

[Andrew Jaquith <an...@gmail.com>]

> 1) should be fairly easy to do - you can call ContentManager.addPage() to
> get a valid, unsaved WikiPage, which won't be persisted until you call
> save().  It does not matter that multiple Sessions call addPage() either,
> since possible conflicts aren't checked until persistence time.  It's also a
> fairly lightweight operation, since at least in Priha, Nodes are small.

The JCR Javadocs seem to be ambiguous about whether nodes are
persisted when they are created, but they are very clear that they
must be persisted when Node.save() is called. I did, indeed, just need
to call ContentManager.addPage(). Seems to work just dandy.

Re: new pages

[Janne Jalkanen <ja...@ecyrd.com>]

> I think (1) is a better option because it means we fix the problem
> once in WikiPageTypeConverter, and don't have to write special
> handling code in every WikiPage-related ActionBean. However, I don't
> fully understand how this would interact with ContentManager. Can you
> create new "temporary" Nodes that aren't persisted? Do we need a
> WikiPage method like isSaved() or similar to indicate whether it's
> been persisted to JCR? Dunno how hard this is.

Well, 2) means that the page name has to be stored as an attribute as  
well, since you can no longer call m_page.getName().

1) should be fairly easy to do - you can call ContentManager.addPage()  
to get a valid, unsaved WikiPage, which won't be persisted until you  
call save().  It does not matter that multiple Sessions call addPage()  
either, since possible conflicts aren't checked until persistence  
time.  It's also a fairly lightweight operation, since at least in  
Priha, Nodes are small.

/Janne

Re: new pages

[Andrew Jaquith <an...@gmail.com>]

I've started implementing option (2) below. It seems to help. However,
EditActionBean (and thus editing) is still hosed because of some work
I was doing integrating it with SpamFilter. I dropped that for a
little while to focus on core bug-fixes after JCR landed, but now I'm
revisiting it. With luck, I'll be able to get the editing features
working again over the next week.

Andrew

On Thu, Apr 16, 2009 at 6:18 PM, Andrew Jaquith
<an...@gmail.com> wrote:
> Funny, I was drafting an e-mail on this very subject. Here it is...
>
> ---
>
> As the 3.0 trunk starts to settle down, I'm headed back to the JSP
> tier to do some work. I wanted to get your thoughts on
> WikiPageTypeConverter -- an important component of how JSPWiki
> interacts with the Stripes layer. I'm thinking of changing how it
> works so that it resolves non-existent WikiPages.
>
> Background: the Stripes TypeConverter classes are pretty nifty. They
> extract parameters from the request stream and convert them into
> objects the target ActionBean wants. For example, if an ActionBean has
> a get/set property called "counter" and its return type is an integer,
> Stripes will convert the value of the request parameter "counter"
> (which is a String) into the correct integer, and cause the
> ActionBean's setCounter(int) method to be called with the converted
> integer value. For JSPWiki, I've created TypeConverter classes for
> wiki Groups, WikiPages, and Principals. The net effect is that when
> one of our ActionBeans is invoked by user's browser in an HttpRequest,
> all of the parameters are magically parsed, converted into their
> correct target types, and bound to the ActionBean. Once the properties
> are converted and populated, they become available for an event
> handler method to use.  It works like magic.
>
> Now, the WikiPageTypeConverter I wrote is pretty interesting. When we
> see a String representing the name of a WikiPage, we look it up in the
> ContentManager. If it's not found, we return a null. Meaning, "we
> didn't find the page because it's not in the repository." The
> consequences of this is what is causing the problem. What this means
> is that ActionBeans with a WikiPage property on them won't have a
> WikiPage bound to them when we finally invoke the event handler
> method. For example, EditActionBean's "edit" method (which starts the
> editing screen) gives an NPE if we try to edit (create!) a page that
> doesn't exist, because the TypeConverter looked up the page, and not
> finding it, returned null as it should have.
>
> This is clearly a problem.
>
> There are two ways of dealing with this: (1) changing the behavior of
> WikiPageTypeConverter so that it returns a valid WikiPage object
> whenever it is presented with a non-null page name, even if the page
> does not exist. The other solution is to (2) ensure that the
> WikiActionBean that depends on the WikiPage has the correct code to
> detect the non-existent page condition, and then can create a new
> WikiPage to fill that gap. ViewActionBean provides an example of (2).
>
> I think (1) is a better option because it means we fix the problem
> once in WikiPageTypeConverter, and don't have to write special
> handling code in every WikiPage-related ActionBean. However, I don't
> fully understand how this would interact with ContentManager. Can you
> create new "temporary" Nodes that aren't persisted? Do we need a
> WikiPage method like isSaved() or similar to indicate whether it's
> been persisted to JCR? Dunno how hard this is.
>
> Andrew
>
> On Thu, Apr 16, 2009 at 5:05 PM, Janne Jalkanen
> <Ja...@ecyrd.com> wrote:
>> Heya!
>>
>> Could you Andrew walk me through exactly how new pages are supposed to be
>> created? As far as I can tell, everything just explodes if you try to edit a
>> page which does not exist, and there really isn't a place to call
>> ContentManager.addPage() either, since the WikiPage gets set in
>> WikiPageConverter, and non-existent page is considered an error.
>>
>> /Janne
>>
>

Re: new pages

[Andrew Jaquith <an...@gmail.com>]

Funny, I was drafting an e-mail on this very subject. Here it is...

---

As the 3.0 trunk starts to settle down, I'm headed back to the JSP
tier to do some work. I wanted to get your thoughts on
WikiPageTypeConverter -- an important component of how JSPWiki
interacts with the Stripes layer. I'm thinking of changing how it
works so that it resolves non-existent WikiPages.

Background: the Stripes TypeConverter classes are pretty nifty. They
extract parameters from the request stream and convert them into
objects the target ActionBean wants. For example, if an ActionBean has
a get/set property called "counter" and its return type is an integer,
Stripes will convert the value of the request parameter "counter"
(which is a String) into the correct integer, and cause the
ActionBean's setCounter(int) method to be called with the converted
integer value. For JSPWiki, I've created TypeConverter classes for
wiki Groups, WikiPages, and Principals. The net effect is that when
one of our ActionBeans is invoked by user's browser in an HttpRequest,
all of the parameters are magically parsed, converted into their
correct target types, and bound to the ActionBean. Once the properties
are converted and populated, they become available for an event
handler method to use.  It works like magic.

Now, the WikiPageTypeConverter I wrote is pretty interesting. When we
see a String representing the name of a WikiPage, we look it up in the
ContentManager. If it's not found, we return a null. Meaning, "we
didn't find the page because it's not in the repository." The
consequences of this is what is causing the problem. What this means
is that ActionBeans with a WikiPage property on them won't have a
WikiPage bound to them when we finally invoke the event handler
method. For example, EditActionBean's "edit" method (which starts the
editing screen) gives an NPE if we try to edit (create!) a page that
doesn't exist, because the TypeConverter looked up the page, and not
finding it, returned null as it should have.

This is clearly a problem.

There are two ways of dealing with this: (1) changing the behavior of
WikiPageTypeConverter so that it returns a valid WikiPage object
whenever it is presented with a non-null page name, even if the page
does not exist. The other solution is to (2) ensure that the
WikiActionBean that depends on the WikiPage has the correct code to
detect the non-existent page condition, and then can create a new
WikiPage to fill that gap. ViewActionBean provides an example of (2).

I think (1) is a better option because it means we fix the problem
once in WikiPageTypeConverter, and don't have to write special
handling code in every WikiPage-related ActionBean. However, I don't
fully understand how this would interact with ContentManager. Can you
create new "temporary" Nodes that aren't persisted? Do we need a
WikiPage method like isSaved() or similar to indicate whether it's
been persisted to JCR? Dunno how hard this is.

Andrew

On Thu, Apr 16, 2009 at 5:05 PM, Janne Jalkanen
<Ja...@ecyrd.com> wrote:
> Heya!
>
> Could you Andrew walk me through exactly how new pages are supposed to be
> created? As far as I can tell, everything just explodes if you try to edit a
> page which does not exist, and there really isn't a place to call
> ContentManager.addPage() either, since the WikiPage gets set in
> WikiPageConverter, and non-existent page is considered an error.
>
> /Janne
>