Yes -- I am aware of the concatenation problem with editing after v1. I took a quick look at it, and haven't been able to find the root cause of the bug yet. But it will yield, as they all do at some point. ;) If you have any space cycles, I'd appreciate a second pair of eyes on this issue.
Andrew On Mon, Nov 2, 2009 at 1:34 PM, Harry Metske <[email protected]> wrote: > very good catch ! > > We can now start editing pages again on an empty repo. > However :-), when we save a couple of versions, it looks like the contents > of versions of pages are concatenated. > I did some edits on the same page with the following results : > > (Text Saved ==> Result) > A => A > B => A A A B A A A > C => A A A B A A A C A A A B A A A > D => A A A B A A A C A A A B A A A D A A A B A A A C A A A B A A A > > regards, > Harry > > > 2009/11/2 <[email protected]> > >> Author: ajaquith >> Date: Mon Nov 2 03:45:48 2009 >> New Revision: 831797 >> >> URL: http://svn.apache.org/viewvc?rev=831797&view=rev >> Log: >> Fixed bug in ContentManager.pageExists() that caused tests for version 1 of >> pages to fail if there had been no other versions saved yet. Shockingly, we >> had never tested for this condition before. As a result, webapp page-viewing >> actions now work again. Related: added versioning to ViewActionBean, which >> hadn't been added previously because the versioning code hadn't stabilized >> until recently. >> >> Modified: >> incubator/jspwiki/trunk/ChangeLog >> incubator/jspwiki/trunk/src/java/org/apache/wiki/Release.java >> >> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/EditActionBean.java >> >> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/ViewActionBean.java >> >> incubator/jspwiki/trunk/src/java/org/apache/wiki/content/ContentManager.java >> >> incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/ViewActionBeanTest.java >> >> incubator/jspwiki/trunk/tests/java/org/apache/wiki/content/ContentManagerTest.java >> >> Modified: incubator/jspwiki/trunk/ChangeLog >> URL: >> http://svn.apache.org/viewvc/incubator/jspwiki/trunk/ChangeLog?rev=831797&r1=831796&r2=831797&view=diff >> >> ============================================================================== >> --- incubator/jspwiki/trunk/ChangeLog (original) >> +++ incubator/jspwiki/trunk/ChangeLog Mon Nov 2 03:45:48 2009 >> @@ -1,3 +1,15 @@ >> +2009-11-01 Andrew Jaquith <ajaquith AT apache DOT org> >> + >> + * 3.0.0-svn-176 >> + >> + * Fixed bug in ContentManager.pageExists() that caused tests >> + for version 1 of pages to fail if there had been no other >> + versions saved yet. Shockingly, we had never tested for this >> + condition before. As a result, webapp page-viewing actions >> + now work again. Related: added versioning to ViewActionBean, >> + which hadn't been added previously because the versioning >> + code hadn't stabilized until recently. >> + >> 2009-10-26 Janne Jalkanen <[email protected]> >> >> * Priha 0.6.0, no other changes. This new version functions >> >> Modified: incubator/jspwiki/trunk/src/java/org/apache/wiki/Release.java >> URL: >> http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/Release.java?rev=831797&r1=831796&r2=831797&view=diff >> >> ============================================================================== >> --- incubator/jspwiki/trunk/src/java/org/apache/wiki/Release.java >> (original) >> +++ incubator/jspwiki/trunk/src/java/org/apache/wiki/Release.java Mon Nov >> 2 03:45:48 2009 >> @@ -77,7 +77,7 @@ >> * <p> >> * If the build identifier is empty, it is not added. >> */ >> - public static final String BUILD = "175"; >> + public static final String BUILD = "176"; >> >> /** >> * This is the generic version string you should use >> >> Modified: >> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/EditActionBean.java >> URL: >> http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/action/EditActionBean.java?rev=831797&r1=831796&r2=831797&view=diff >> >> ============================================================================== >> --- >> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/EditActionBean.java >> (original) >> +++ >> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/EditActionBean.java >> Mon Nov 2 03:45:48 2009 >> @@ -468,7 +468,7 @@ >> + m_author + ", Host=" + >> getContext().getRequest().getRemoteAddr() ); >> >> // Set author information and other metadata >> - WikiPage modifiedPage = (WikiPage) wikiContext.getPage().clone(); >> + WikiPage modifiedPage = (WikiPage)page.clone(); >> modifiedPage.setAuthor( m_author ); >> if( m_changeNote != null ) >> { >> >> Modified: >> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/ViewActionBean.java >> URL: >> http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/action/ViewActionBean.java?rev=831797&r1=831796&r2=831797&view=diff >> >> ============================================================================== >> --- >> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/ViewActionBean.java >> (original) >> +++ >> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/ViewActionBean.java >> Mon Nov 2 03:45:48 2009 >> @@ -32,12 +32,15 @@ >> import net.sourceforge.stripes.validation.ValidationErrors; >> >> import org.apache.wiki.WikiEngine; >> +import org.apache.wiki.WikiProvider; >> import org.apache.wiki.api.WikiException; >> import org.apache.wiki.api.WikiPage; >> import org.apache.wiki.auth.permissions.PagePermission; >> +import org.apache.wiki.content.PageNotFoundException; >> import org.apache.wiki.log.Logger; >> import org.apache.wiki.log.LoggerFactory; >> import org.apache.wiki.ui.stripes.HandlerPermission; >> +import org.apache.wiki.ui.stripes.WikiActionBeanContext; >> import org.apache.wiki.ui.stripes.WikiRequestContext; >> >> /** >> @@ -51,6 +54,8 @@ >> >> private String m_renameTo = null; >> >> + private int m_version = WikiProvider.LATEST_VERSION; >> + >> public ViewActionBean() >> { >> super(); >> @@ -81,6 +86,15 @@ >> } >> >> /** >> + * Returns the version of the page. >> + * @return the version >> + */ >> + public int getVersion() >> + { >> + return m_version; >> + } >> + >> + /** >> * Handler that forwards to the page information display JSP >> * <code>/PageInfo.jsp</code>. >> * >> @@ -119,17 +133,18 @@ >> @After( stages = LifecycleStage.BindingAndValidation ) >> public Resolution resolvePage() throws WikiException >> { >> - WikiEngine engine = getContext().getEngine(); >> + WikiActionBeanContext context = getContext(); >> + WikiEngine engine = context.getEngine(); >> >> if( isSpecialPageView() ) >> { >> // The page might be null because it's a special page >> // WikiPageTypeConverter >> // refused to convert. If so, redirect. >> - String pageName = getContext().getRequest().getParameter( >> "page" ); >> + String pageName = context.getRequest().getParameter( "page" ); >> if( pageName != null ) >> { >> - URI uri = >> getContext().getEngine().getSpecialPageReference( pageName ); >> + URI uri = engine.getSpecialPageReference( pageName ); >> if( uri != null ) >> { >> return new RedirectResolution( uri.toString() ); >> @@ -149,34 +164,49 @@ >> } >> >> // If page still missing, it's an error condition >> - if( getPage() == null ) >> + WikiPage page = getPage(); >> + if( page == null ) >> { >> throw new WikiException( "Page not supplied, and WikiEngine >> does not define a front page! This is highly unusual." ); >> } >> >> // Is there an ALIAS attribute in the wiki page? >> - String specialUrl = (String) getPage().getAttribute( >> WikiPage.ALIAS ); >> + String specialUrl = (String) page.getAttribute( WikiPage.ALIAS ); >> if( specialUrl != null ) >> { >> - return new RedirectResolution( getContext().getViewURL( >> specialUrl ) ); >> + return new RedirectResolution( context.getViewURL( specialUrl >> ) ); >> } >> >> // Is there a REDIRECT attribute in the wiki page? >> - specialUrl = (String) getPage().getAttribute( WikiPage.REDIRECT ); >> + specialUrl = (String) page.getAttribute( WikiPage.REDIRECT ); >> if( specialUrl != null ) >> { >> - return new RedirectResolution( getContext().getViewURL( >> specialUrl ) ); >> + return new RedirectResolution( context.getViewURL( specialUrl >> ) ); >> } >> >> // Ok, the page exists. If attachment, make sure it's directed to >> the >> // "info" handler >> - WikiPage page = getPage(); >> - String handler = getContext().getEventName(); >> - if( getPage().isAttachment() && !"info".equals( handler ) ) >> + String handler = context.getEventName(); >> + if( page.isAttachment() && !"info".equals( handler ) ) >> { >> return new RedirectResolution( ViewActionBean.class, "info" >> ).addParameter( "page", page.getPath().toString() ); >> } >> >> + // Now, retrieve the requested page or attachment version >> + if ( engine.pageExists( page.getPath().toString(), m_version ) ) >> + { >> + try >> + { >> + page = engine.getPage( page.getPath().toString(), >> m_version ); >> + setPage( page ); >> + } >> + catch( PageNotFoundException e ) >> + { >> + // Shouldn't happen! >> + throw new WikiException( "Did not retrieve the page even >> though it exists. " >> + + " This is a BUG. ", e ); >> + } >> + } >> return null; >> } >> >> @@ -202,6 +232,16 @@ >> } >> >> /** >> + * Sets the version of the page to show. If not set, defaults to >> + * {...@link WikiProvider#LATEST_VERSION}. >> + * @param version the version >> + */ >> + public void setVersion( int version ) >> + { >> + m_version = version; >> + } >> + >> + /** >> * Default handler that simply forwards the user back to the display >> JSP >> * <code>/Wiki.jsp</code>. Every ActionBean needs a default handler to >> * function properly, so we use this (very simple) one. >> >> 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=831797&r1=831796&r2=831797&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 >> Mon Nov 2 03:45:48 2009 >> @@ -950,17 +950,33 @@ >> return false; >> } >> >> - if ( version == WikiProvider.LATEST_VERSION ) >> + // Is Node's current version the one we want? >> + if ( node.hasProperty( JCRWikiPage.ATTR_VERSION ) ) >> + { >> + Property versionProperty = node.getProperty( >> JCRWikiPage.ATTR_VERSION ); >> + try >> + { >> + if ( version == versionProperty.getLong() ) >> + { >> + return true; >> + } >> + } >> + catch ( ValueFormatException e ) { >> + throw new ProviderException("Property " + >> JCRWikiPage.ATTR_VERSION + >> + " for node " + jcrPath + " >> cannot be " + >> + " coerced to a Long. This >> is strange." ); >> + } >> + } >> + >> + boolean noVersions = !node.hasNode( WIKI_VERSIONS ); >> + if ( version == WikiProvider.LATEST_VERSION || noVersions ) >> { >> return !isNew( node ); >> } >> >> String v = Integer.toString( version ); >> - if ( node.hasNode( WIKI_VERSIONS ) ) >> - { >> - Node versions = node.getNode( WIKI_VERSIONS ); >> - return versions.hasNode( v ) && !isNew( versions.getNode( >> v ) ); >> - } >> + Node versions = node.getNode( WIKI_VERSIONS ); >> + return versions.hasNode( v ) && !isNew( versions.getNode( v ) >> ); >> } >> catch ( PathNotFoundException e ) >> { >> @@ -971,7 +987,6 @@ >> { >> throw new ProviderException( "Unable to check for page >> existence", e ); >> } >> - return false; >> } >> >> /** >> >> Modified: >> incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/ViewActionBeanTest.java >> URL: >> http://svn.apache.org/viewvc/incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/ViewActionBeanTest.java?rev=831797&r1=831796&r2=831797&view=diff >> >> ============================================================================== >> --- >> incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/ViewActionBeanTest.java >> (original) >> +++ >> incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/ViewActionBeanTest.java >> Mon Nov 2 03:45:48 2009 >> @@ -115,22 +115,79 @@ >> >> >> public void testView() throws Exception { >> - // Save page Test >> - m_engine.saveText("Test", "This is a test."); >> - WikiPage page = m_engine.getPage("Test"); >> - assertNotNull("Did not save page Test!", page); >> - >> - // Set the 'page' request parameter to 'Test'... >> - MockRoundtrip trip = m_engine.guestTrip( "/Wiki.action"); >> - trip.setParameter("page", "Test"); >> - trip.execute("view"); >> + String pageName = "ViewActionBeanTest"; >> + >> + // Save test page >> + m_engine.saveText( pageName, "This is a test." ); >> + WikiPage page = m_engine.getPage( pageName ); >> + assertNotNull("Did not save page " + pageName + "!", page); >> + >> + // Set the 'page' request parameter to 'ViewActionBeanTest'... >> + MockRoundtrip trip = m_engine.guestTrip( "/Wiki.action" ); >> + trip.setParameter( "page", pageName ); >> + trip.execute( "view" ); >> >> // ...we should automatically see Test bound to the ActionBean >> (nice!) >> - ViewActionBean bean = trip.getActionBean(ViewActionBean.class); >> + ViewActionBean bean = trip.getActionBean( ViewActionBean.class ); >> assertEquals( page, bean.getPage() ); >> >> // ...and the destination should be Wiki.jsp (aka display JSP) >> - assertEquals("/Wiki.jsp", trip.getDestination() ); >> + assertEquals( "/Wiki.jsp", trip.getDestination() ); >> + } >> + >> + public void testViewVersion() throws Exception { >> + String pageName = "ViewActionBeanTest"; >> + m_engine.deletePage( pageName ); >> + >> + // Save test page >> + m_engine.saveText( pageName, "This is the first version." ); >> + WikiPage page = m_engine.getPage( pageName ); >> + assertNotNull("Did not save page " + pageName + "!", page); >> + >> + // Go get the first version >> + MockRoundtrip trip = m_engine.guestTrip( "/Wiki.action" ); >> + trip.setParameter( "page", pageName ); >> + trip.execute( "view" ); >> + ViewActionBean bean = trip.getActionBean( ViewActionBean.class ); >> + assertEquals( page, bean.getPage() ); >> + String pageText = page.getContentAsString(); >> + assertEquals( "This is the first version.\r\n", pageText ); >> + >> + // Save a second version >> + m_engine.saveText( pageName, "This is the second version." ); >> + WikiPage pageV1 = m_engine.getPage( pageName, 1 ); >> + WikiPage pageV2 = m_engine.getPage( pageName, 2 ); >> + >> + // Go get the first version again >> + trip = m_engine.guestTrip( "/Wiki.action" ); >> + trip.setParameter( "page", pageName ); >> + trip.setParameter( "version", "1" ); >> + trip.execute( "view" ); >> + bean = trip.getActionBean( ViewActionBean.class ); >> + assertEquals( pageV1, bean.getPage() ); >> + pageText = pageV1.getContentAsString(); >> + assertEquals( "This is the first version.\r\n", pageText ); >> + >> + // Go get the second version >> + trip = m_engine.guestTrip( "/Wiki.action" ); >> + trip.setParameter( "page", pageName ); >> + trip.setParameter( "version", "2" ); >> + trip.execute( "view" ); >> + bean = trip.getActionBean( ViewActionBean.class ); >> + assertEquals( pageV2, bean.getPage() ); >> + pageText = pageV2.getContentAsString(); >> + assertEquals( "This is the second version.\r\n", pageText ); >> + >> + // Go get the "latest" version >> + trip = m_engine.guestTrip( "/Wiki.action" ); >> + trip.setParameter( "page", pageName ); >> + trip.execute( "view" ); >> + bean = trip.getActionBean( ViewActionBean.class ); >> + assertEquals( pageV2, bean.getPage() ); >> + pageText = pageV2.getContentAsString(); >> + assertEquals( "This is the second version.\r\n", pageText ); >> + >> + m_engine.deletePage( pageName ); >> } >> >> public static Test suite() >> >> Modified: >> incubator/jspwiki/trunk/tests/java/org/apache/wiki/content/ContentManagerTest.java >> URL: >> http://svn.apache.org/viewvc/incubator/jspwiki/trunk/tests/java/org/apache/wiki/content/ContentManagerTest.java?rev=831797&r1=831796&r2=831797&view=diff >> >> ============================================================================== >> --- >> incubator/jspwiki/trunk/tests/java/org/apache/wiki/content/ContentManagerTest.java >> (original) >> +++ >> incubator/jspwiki/trunk/tests/java/org/apache/wiki/content/ContentManagerTest.java >> Mon Nov 2 03:45:48 2009 >> @@ -29,6 +29,7 @@ >> import junit.framework.TestSuite; >> >> import org.apache.wiki.TestEngine; >> +import org.apache.wiki.WikiProvider; >> import org.apache.wiki.api.WikiException; >> import org.apache.wiki.api.WikiPage; >> import org.apache.wiki.providers.ProviderException; >> @@ -134,6 +135,24 @@ >> assertEquals( 2, allPages.size() ); >> } >> >> + public void testPageExists() throws Exception >> + { >> + WikiPath path = WikiPath.valueOf( "ContentManagerTest-PageExists" >> ); >> + >> + // Save a new page >> + m_engine.saveText( path.toString(), "This is the first version" ); >> + assertTrue( m_mgr.pageExists( path, WikiProvider.LATEST_VERSION ) >> ); >> + assertTrue( m_mgr.pageExists( path, 1 ) ); >> + >> + // Save another version >> + m_engine.saveText( path.toString(), "This is the second version" >> ); >> + assertTrue( m_mgr.pageExists( path, WikiProvider.LATEST_VERSION ) >> ); >> + assertTrue( m_mgr.pageExists( path, 2 ) ); >> + assertTrue( m_mgr.pageExists( path, 1 ) ); >> + >> + m_engine.deletePage( path.toString() ); >> + } >> + >> public void testVersions() throws Exception >> { >> String content = "Test Content"; >> >> >> >
