You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jspwiki.apache.org by aj...@apache.org on 2009/10/18 00:49:05 UTC

svn commit: r826317 - in /incubator/jspwiki/trunk: src/java/org/apache/wiki/WikiEngine.java src/java/org/apache/wiki/content/ContentManager.java tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java

Author: ajaquith
Date: Sat Oct 17 22:49:04 2009
New Revision: 826317

URL: http://svn.apache.org/viewvc?rev=826317&view=rev
Log:
Fixed JSPWikiMarkupParser rendering speed test and a few Javadocs. No version bump.

Modified:
    incubator/jspwiki/trunk/src/java/org/apache/wiki/WikiEngine.java
    incubator/jspwiki/trunk/src/java/org/apache/wiki/content/ContentManager.java
    incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java

Modified: incubator/jspwiki/trunk/src/java/org/apache/wiki/WikiEngine.java
URL: http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/WikiEngine.java?rev=826317&r1=826316&r2=826317&view=diff
==============================================================================
--- incubator/jspwiki/trunk/src/java/org/apache/wiki/WikiEngine.java (original)
+++ incubator/jspwiki/trunk/src/java/org/apache/wiki/WikiEngine.java Sat Oct 17 22:49:04 2009
@@ -1869,7 +1869,8 @@
     }
 
     /**
-     *  Creates a new WikiPage object.
+     *  Creates a new WikiPage object without saving it to the repository.
+     *  To save the page, call {@link WikiPage#save()}.
      *  
      *  @param name the WikiName of the object to create
      *  @return a new WikiPage object

Modified: incubator/jspwiki/trunk/src/java/org/apache/wiki/content/ContentManager.java
URL: http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/content/ContentManager.java?rev=826317&r1=826316&r2=826317&view=diff
==============================================================================
--- incubator/jspwiki/trunk/src/java/org/apache/wiki/content/ContentManager.java (original)
+++ incubator/jspwiki/trunk/src/java/org/apache/wiki/content/ContentManager.java Sat Oct 17 22:49:04 2009
@@ -1455,7 +1455,7 @@
     }
     
     /**
-     *  Adds new content to the repository.  To update, get a page, modify
+     *  Adds new content to the repository without saving it. To update, get a page, modify
      *  it, then store it back using save().
      *  
      *  @param path the WikiName
@@ -1470,7 +1470,7 @@
     }
 
     /**
-     *  Add new content to the repository to a particular JCR path.
+     *  Add new content to the repository to a particular JCR path, without saving it.
      *  
      *  @param path
      *  @param jcrPath

Modified: incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java
URL: http://svn.apache.org/viewvc/incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java?rev=826317&r1=826316&r2=826317&view=diff
==============================================================================
--- incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java (original)
+++ incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java Sat Oct 17 22:49:04 2009
@@ -2327,7 +2327,14 @@
     {
         Benchmark sw = new Benchmark();
         sw.start();
+        
+        // Create the page and save it to disk.
+        if ( !testEngine.pageExists( PAGE_NAME ) )
+        {
+            testEngine.saveText( PAGE_NAME, brokenPageText );
+        }
 
+        // Test the rendering speed
         for( int i = 0; i < 100; i++ )
         {
             translate( brokenPageText );



Re: svn commit: r826317 - in /incubator/jspwiki/trunk: src/java/org/apache/wiki/WikiEngine.java src/java/org/apache/wiki/content/ContentManager.java tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java

Posted by Janne Jalkanen <Ja...@ecyrd.com>.
> But I am not sure the solution is quite as easy as creating a new
> version and saving the old one. Not all saves are guaranteed to
> complete because of the workflow stuff. It might be better to simply
> have WikiEngine.createNode() look for the existence of Nodes whose
> isNew() returns true, and return it instead (which would mean the
> contents might get overwritten, but whatever).

Yup, this should work...  The other possibility would be to throw a  
PageExistsException (which would enforce one to be a bit more careful  
with the code), but perhaps returning the already-existing -page is  
nicer.

/Janne

Re: svn commit: r826317 - in /incubator/jspwiki/trunk: src/java/org/apache/wiki/WikiEngine.java src/java/org/apache/wiki/content/ContentManager.java tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java

Posted by Andrew Jaquith <an...@gmail.com>.
Sorry. I should have said: ContentManager.addPage().

On 10/18/09, Andrew Jaquith <an...@gmail.com> wrote:
> Just checked in a fix. I added some code to ContentManager.addNode()
> that checks for the existence of an unsaved Node. If one is found, it
> is re-used in the WikiPage that is returned to the caller. Works
> nicely and doesn't break anything.
>
> On 10/18/09, Andrew Jaquith <an...@gmail.com> wrote:
>> I agree that this might potentially be a bug -- or more correctly put,
>> a not-very-well-documented way for a developer to get into trouble.
>>
>> But I am not sure the solution is quite as easy as creating a new
>> version and saving the old one. Not all saves are guaranteed to
>> complete because of the workflow stuff. It might be better to simply
>> have WikiEngine.createNode() look for the existence of Nodes whose
>> isNew() returns true, and return it instead (which would mean the
>> contents might get overwritten, but whatever).
>>
>> Andrew
>>
>> On Sat, Oct 17, 2009 at 6:52 PM, Janne Jalkanen
>> <ja...@ecyrd.com> wrote:
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> ======================================================================
>>>> ---
>>>> incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java
>>>> (original)
>>>> +++
>>>> incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java
>>>> Sat Oct 17 22:49:04 2009
>>>> @@ -2327,7 +2327,14 @@
>>>>    {
>>>>        Benchmark sw = new Benchmark();
>>>>        sw.start();
>>>> +
>>>> +        // Create the page and save it to disk.
>>>> +        if ( !testEngine.pageExists( PAGE_NAME ) )
>>>> +        {
>>>> +            testEngine.saveText( PAGE_NAME, brokenPageText );
>>>> +        }
>>>>
>>>> +        // Test the rendering speed
>>>
>>> Not sure whether this is the right fix - I mean, saveText() should
>>> actually
>>> check for this itself, and in fact, it should save a new version of the
>>> page
>>> whenever it's called - NOT create a new SNS.  Yes, it fixes the test,
>>> but
>>> I
>>> have a feeling that it only hides a real bug.
>>>
>>> /Janne
>>>
>>
>

Re: svn commit: r826317 - in /incubator/jspwiki/trunk: src/java/org/apache/wiki/WikiEngine.java src/java/org/apache/wiki/content/ContentManager.java tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java

Posted by Andrew Jaquith <an...@gmail.com>.
Just checked in a fix. I added some code to ContentManager.addNode()
that checks for the existence of an unsaved Node. If one is found, it
is re-used in the WikiPage that is returned to the caller. Works
nicely and doesn't break anything.

On 10/18/09, Andrew Jaquith <an...@gmail.com> wrote:
> I agree that this might potentially be a bug -- or more correctly put,
> a not-very-well-documented way for a developer to get into trouble.
>
> But I am not sure the solution is quite as easy as creating a new
> version and saving the old one. Not all saves are guaranteed to
> complete because of the workflow stuff. It might be better to simply
> have WikiEngine.createNode() look for the existence of Nodes whose
> isNew() returns true, and return it instead (which would mean the
> contents might get overwritten, but whatever).
>
> Andrew
>
> On Sat, Oct 17, 2009 at 6:52 PM, Janne Jalkanen
> <ja...@ecyrd.com> wrote:
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> ======================================================================
>>> ---
>>> incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java
>>> (original)
>>> +++
>>> incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java
>>> Sat Oct 17 22:49:04 2009
>>> @@ -2327,7 +2327,14 @@
>>>    {
>>>        Benchmark sw = new Benchmark();
>>>        sw.start();
>>> +
>>> +        // Create the page and save it to disk.
>>> +        if ( !testEngine.pageExists( PAGE_NAME ) )
>>> +        {
>>> +            testEngine.saveText( PAGE_NAME, brokenPageText );
>>> +        }
>>>
>>> +        // Test the rendering speed
>>
>> Not sure whether this is the right fix - I mean, saveText() should
>> actually
>> check for this itself, and in fact, it should save a new version of the
>> page
>> whenever it's called - NOT create a new SNS.  Yes, it fixes the test, but
>> I
>> have a feeling that it only hides a real bug.
>>
>> /Janne
>>
>

Re: svn commit: r826317 - in /incubator/jspwiki/trunk: src/java/org/apache/wiki/WikiEngine.java src/java/org/apache/wiki/content/ContentManager.java tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java

Posted by Andrew Jaquith <an...@gmail.com>.
I agree that this might potentially be a bug -- or more correctly put,
a not-very-well-documented way for a developer to get into trouble.

But I am not sure the solution is quite as easy as creating a new
version and saving the old one. Not all saves are guaranteed to
complete because of the workflow stuff. It might be better to simply
have WikiEngine.createNode() look for the existence of Nodes whose
isNew() returns true, and return it instead (which would mean the
contents might get overwritten, but whatever).

Andrew

On Sat, Oct 17, 2009 at 6:52 PM, Janne Jalkanen
<ja...@ecyrd.com> wrote:
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> ======================================================================
>> ---
>> incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java
>> (original)
>> +++
>> incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java
>> Sat Oct 17 22:49:04 2009
>> @@ -2327,7 +2327,14 @@
>>    {
>>        Benchmark sw = new Benchmark();
>>        sw.start();
>> +
>> +        // Create the page and save it to disk.
>> +        if ( !testEngine.pageExists( PAGE_NAME ) )
>> +        {
>> +            testEngine.saveText( PAGE_NAME, brokenPageText );
>> +        }
>>
>> +        // Test the rendering speed
>
> Not sure whether this is the right fix - I mean, saveText() should actually
> check for this itself, and in fact, it should save a new version of the page
> whenever it's called - NOT create a new SNS.  Yes, it fixes the test, but I
> have a feeling that it only hides a real bug.
>
> /Janne
>

Re: svn commit: r826317 - in /incubator/jspwiki/trunk: src/java/org/apache/wiki/WikiEngine.java src/java/org/apache/wiki/content/ContentManager.java tests/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java

Posted by Janne Jalkanen <ja...@ecyrd.com>.
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/ 
> JSPWikiMarkupParserTest.java (original)
> +++ incubator/jspwiki/trunk/tests/java/org/apache/wiki/parser/ 
> JSPWikiMarkupParserTest.java Sat Oct 17 22:49:04 2009
> @@ -2327,7 +2327,14 @@
>     {
>         Benchmark sw = new Benchmark();
>         sw.start();
> +
> +        // Create the page and save it to disk.
> +        if ( !testEngine.pageExists( PAGE_NAME ) )
> +        {
> +            testEngine.saveText( PAGE_NAME, brokenPageText );
> +        }
>
> +        // Test the rendering speed

Not sure whether this is the right fix - I mean, saveText() should  
actually check for this itself, and in fact, it should save a new  
version of the page whenever it's called - NOT create a new SNS.  Yes,  
it fixes the test, but I have a feeling that it only hides a real bug.

/Janne