You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by sv...@apache.org on 2019/05/03 18:22:22 UTC

[wicket] 02/02: WICKET-6563 allow untouch of page

This is an automated email from the ASF dual-hosted git repository.

svenmeier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git

commit ee9af92be7c0d47ae6d7a2011efc88cb9006cf4e
Author: Sven Meier <sv...@apache.org>
AuthorDate: Tue Apr 30 18:55:46 2019 +0200

    WICKET-6563 allow untouch of page
---
 .../main/java/org/apache/wicket/Application.java   |  2 +-
 .../src/main/java/org/apache/wicket/Page.java      |  8 ++---
 .../src/main/java/org/apache/wicket/Session.java   |  3 +-
 .../request/handler/RenderPageRequestHandler.java  |  2 +-
 .../org/apache/wicket/mock/MockPageManager.java    | 10 +++++--
 .../java/org/apache/wicket/page/IPageManager.java  | 29 ++++++++----------
 .../apache/wicket/page/PageAccessSynchronizer.java | 14 ++++++---
 .../java/org/apache/wicket/page/PageManager.java   | 10 +++++--
 .../wicket/pageStore/DelegatingPageStore.java      |  6 ++++
 .../org/apache/wicket/pageStore/IPageStore.java    |  9 ++++++
 .../apache/wicket/pageStore/RequestPageStore.java  | 12 +++++++-
 .../core/request/mapper/TestMapperContext.java     |  2 +-
 .../DontStoreNotRenderedPageTestCase.java          |  4 +--
 .../html/TransparentWebMarkupContainerTest.java    |  4 +--
 .../wicket/page/PageAccessSynchronizerTest.java    |  2 +-
 .../wicket/page/PersistentPageManagerTest.java     |  2 +-
 .../wicket/pageStore/RequestPageStoreTest.java     | 34 ++++++++++++++++++----
 .../wicket/request/handler/PageProviderTest.java   | 10 +++----
 .../apache/wicket/examples/frames/BodyFrame.java   |  2 +-
 .../ajax/markup/html/modal/ModalWindow.java        |  2 +-
 20 files changed, 114 insertions(+), 53 deletions(-)

diff --git a/wicket-core/src/main/java/org/apache/wicket/Application.java b/wicket-core/src/main/java/org/apache/wicket/Application.java
index 035ee0b..82a046a 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Application.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Application.java
@@ -1523,7 +1523,7 @@ public abstract class Application implements UnboundListener, IEventSink, IMetad
 		{
 			session = newSession(requestCycle.getRequest(), requestCycle.getResponse());
 			ThreadContext.setSession(session);
-			internalGetPageManager().removeAllPages();
+			internalGetPageManager().clear();
 			sessionListeners.onCreated(session);
 		}
 		else
diff --git a/wicket-core/src/main/java/org/apache/wicket/Page.java b/wicket-core/src/main/java/org/apache/wicket/Page.java
index 7e74a4e..3f0f5b5 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Page.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Page.java
@@ -287,7 +287,7 @@ public abstract class Page extends MarkupContainer
 
 			if (isInitialization == false)
 			{
-				pageManager.addPage(this);
+				pageManager.touchPage(this);
 			}
 		}
 	}
@@ -298,7 +298,7 @@ public abstract class Page extends MarkupContainer
 		super.onInitialize();
 
 		final IPageManager pageManager = getSession().getPageManager();
-		pageManager.addPage(this);
+		pageManager.touchPage(this);
 	}
 
 	/**
@@ -825,7 +825,7 @@ public abstract class Page extends MarkupContainer
 			getSession().getSessionStore().getSessionId(RequestCycle.get().getRequest(), true);
 
 			// Add/touch the response page in the session.
-			getSession().getPageManager().addPage(this);
+			getSession().getPageManager().touchPage(this);
 		}
 
 		if (getApplication().getDebugSettings().isOutputMarkupContainerClassName())
@@ -936,7 +936,7 @@ public abstract class Page extends MarkupContainer
 		setStatelessHint(false);
 
 		// make sure the page will be available on following request
-		getSession().getPageManager().addPage(this);
+		getSession().getPageManager().touchPage(this);
 
 		return new PageReference(numericId);
 	}
diff --git a/wicket-core/src/main/java/org/apache/wicket/Session.java b/wicket-core/src/main/java/org/apache/wicket/Session.java
index 60f5676..eb31dfe 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Session.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Session.java
@@ -38,7 +38,6 @@ import org.apache.wicket.feedback.FeedbackMessages;
 import org.apache.wicket.feedback.IFeedbackContributor;
 import org.apache.wicket.page.IPageManager;
 import org.apache.wicket.page.PageAccessSynchronizer;
-import org.apache.wicket.pageStore.IPageStore;
 import org.apache.wicket.request.Request;
 import org.apache.wicket.request.cycle.RequestCycle;
 import org.apache.wicket.session.ISessionStore;
@@ -292,7 +291,7 @@ public abstract class Session implements IClusterable, IEventSink, IMetadataCont
 	{
 		if (isTemporary() == false)
 		{
-			getPageManager().removeAllPages();
+			getPageManager().clear();
 		}
 	}
 
diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/RenderPageRequestHandler.java b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/RenderPageRequestHandler.java
index 642d40f..156d420 100644
--- a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/RenderPageRequestHandler.java
+++ b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/RenderPageRequestHandler.java
@@ -136,7 +136,7 @@ public class RenderPageRequestHandler
 			if (Session.exists())
 			{
 				// WICKET-5499
-				Session.get().getPageManager().addPage(pageProvider.getPageInstance());
+				Session.get().getPageManager().touchPage(pageProvider.getPageInstance());
 			}
 		}
 	}
diff --git a/wicket-core/src/main/java/org/apache/wicket/mock/MockPageManager.java b/wicket-core/src/main/java/org/apache/wicket/mock/MockPageManager.java
index 799f7b0..9c5216a 100644
--- a/wicket-core/src/main/java/org/apache/wicket/mock/MockPageManager.java
+++ b/wicket-core/src/main/java/org/apache/wicket/mock/MockPageManager.java
@@ -56,18 +56,24 @@ public class MockPageManager implements IPageManager
 	}
 
 	@Override
-	public void addPage(IManageablePage page)
+	public void touchPage(IManageablePage page)
 	{
 		pages.put(page.getPageId(), page);
 	}
 
 	@Override
-	public void removeAllPages()
+	public void clear()
 	{
 		pages.clear();
 	}
 
 	@Override
+	public void untouchPage(IManageablePage page)
+	{
+		pages.remove(page.getPageId());
+	}
+	
+	@Override
 	public void detach()
 	{
 	}
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/IPageManager.java b/wicket-core/src/main/java/org/apache/wicket/page/IPageManager.java
index 8f4cd0e..c675cd9 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/IPageManager.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/IPageManager.java
@@ -56,12 +56,21 @@ public interface IPageManager
 	 * @param page
 	 *            page to add
 	 */
-	void addPage(IManageablePage page);
+	void touchPage(IManageablePage page);
 	
 	/**
-	 * Remove all pages.
+	 * Marks page as non-changed.
+	 * Could be used in Ajax requests to avoid storing the page if no changes have happened.
+	 *
+	 * @param page
+	 *      the page that should <strong>not</strong> be stored in the page stores at the end of the request.
 	 */
-	void removeAllPages();
+	void untouchPage(IManageablePage page);
+
+	/**
+	 * Clear all pages.
+	 */
+	void clear();
 
 	/**
 	 * Detach at end of request.
@@ -79,18 +88,4 @@ public interface IPageManager
 	 * @return store or <code>null</code>
 	 */
 	IPageStore getPageStore();
-
-	/**
-	 * @deprecated will be removed in Wicket 10
-	 */
-	default void touchPage(IManageablePage page) {
-		addPage(page);
-	}
-
-	/**
-	 * @deprecated will be removed in Wicket 10
-	 */
-	default void clear() {
-		removeAllPages();
-	}
 }
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java b/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
index 8d69fda..a744bb7 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
@@ -280,17 +280,23 @@ public class PageAccessSynchronizer implements Serializable
 			}
 
 			@Override
-			public void addPage(IManageablePage page)
+			public void touchPage(IManageablePage page)
 			{
 				lockPage(page.getPageId());
 				
-				manager.addPage(page);
+				manager.touchPage(page);
 			}
 
 			@Override
-			public void removeAllPages()
+			public void clear()
 			{
-				manager.removeAllPages();
+				manager.clear();
+			}
+			
+			@Override
+			public void untouchPage(IManageablePage page)
+			{
+				manager.untouchPage(page);
 			}
 			
 			@Override
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/PageManager.java b/wicket-core/src/main/java/org/apache/wicket/page/PageManager.java
index 52604cc..642b3c1 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/PageManager.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/PageManager.java
@@ -62,18 +62,24 @@ public class PageManager implements IPageManager
 	}
 
 	@Override
-	public void addPage(IManageablePage page)
+	public void touchPage(IManageablePage page)
 	{
 		store.addPage(createPageContext(), page);
 	}
 
 	@Override
-	public void removeAllPages()
+	public void clear()
 	{
 		store.removeAllPages(createPageContext());
 	}
 
 	@Override
+	public void untouchPage(IManageablePage page)
+	{
+		store.revertPage(createPageContext(), page);
+	}
+	
+	@Override
 	public void detach()
 	{
 		store.detach(createPageContext());
diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/DelegatingPageStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/DelegatingPageStore.java
index ba256a0..5ec3a79 100644
--- a/wicket-core/src/main/java/org/apache/wicket/pageStore/DelegatingPageStore.java
+++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/DelegatingPageStore.java
@@ -70,6 +70,12 @@ public abstract class DelegatingPageStore implements IPageStore
 	}
 
 	@Override
+	public void revertPage(IPageContext context, IManageablePage page)
+	{
+		delegate.revertPage(context, page);
+	}
+	
+	@Override
 	public void detach(IPageContext context)
 	{
 		delegate.detach(context);
diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/IPageStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/IPageStore.java
index bf2c798..50634a0 100644
--- a/wicket-core/src/main/java/org/apache/wicket/pageStore/IPageStore.java
+++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/IPageStore.java
@@ -74,6 +74,15 @@ public interface IPageStore
 	void removeAllPages(IPageContext context);
 
 	/**
+	 * Revert adding a page - optional operation. 
+	 *
+	 * @param page
+	 *      the page that should be reverted
+	 */
+	default void revertPage(IPageContext context, IManageablePage page) {
+	}
+
+	/**
 	 * Restores a page from storage.
 	 * 
 	 * @param context
diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/RequestPageStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/RequestPageStore.java
index 0355774..0c0da70 100644
--- a/wicket-core/src/main/java/org/apache/wicket/pageStore/RequestPageStore.java
+++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/RequestPageStore.java
@@ -16,6 +16,7 @@
  */
 package org.apache.wicket.pageStore;
 
+import java.util.ArrayList;
 import java.util.LinkedList;
 
 import org.apache.wicket.MetaDataKey;
@@ -78,6 +79,14 @@ public class RequestPageStore extends DelegatingPageStore
 	}
 
 	@Override
+	public void revertPage(IPageContext context, IManageablePage page)
+	{
+		getRequestData(context).remove(page);
+		
+		getDelegate().revertPage(context, page);
+	}
+	
+	@Override
 	public void detach(IPageContext context)
 	{
 		RequestData requestData = getRequestData(context);
@@ -125,7 +134,8 @@ public class RequestPageStore extends DelegatingPageStore
 
 		public Iterable<IManageablePage> pages()
 		{
-			return pages;
+			// must work on copy to prevent concurrent modification when page is re-added during detaching 
+			return new ArrayList<>(pages);
 		}
 
 		public IManageablePage get(int id)
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/TestMapperContext.java b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/TestMapperContext.java
index 4129c66..6f3be0f 100644
--- a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/TestMapperContext.java
+++ b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/TestMapperContext.java
@@ -78,7 +78,7 @@ public class TestMapperContext implements IMapperContext
 		pageContext.clearRequest();
 		MockPage other = new MockPage();
 		other.setPageId(Integer.MAX_VALUE);
-		getPageManager().addPage(other);
+		getPageManager().touchPage(other);
 		pageManager.detach();
 	}
 
diff --git a/wicket-core/src/test/java/org/apache/wicket/dontstoreunrendered/DontStoreNotRenderedPageTestCase.java b/wicket-core/src/test/java/org/apache/wicket/dontstoreunrendered/DontStoreNotRenderedPageTestCase.java
index 857c715..2eb35c7 100644
--- a/wicket-core/src/test/java/org/apache/wicket/dontstoreunrendered/DontStoreNotRenderedPageTestCase.java
+++ b/wicket-core/src/test/java/org/apache/wicket/dontstoreunrendered/DontStoreNotRenderedPageTestCase.java
@@ -58,10 +58,10 @@ public abstract class DontStoreNotRenderedPageTestCase extends WicketTestCase
 					return new MockPageManager()
 					{
 						@Override
-						public void addPage(IManageablePage page)
+						public void touchPage(IManageablePage page)
 						{
 							assertFalse(page instanceof PageB, "PageB should not be touched!");
-							super.addPage(page);
+							super.touchPage(page);
 						}
 					};
 				};
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java
index 39d81c3..f4eeac6 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java
@@ -125,10 +125,10 @@ public class TransparentWebMarkupContainerTest extends WicketTestCase
 						return new MockPageManager()
 						{
 							@Override
-							public void addPage(IManageablePage page)
+							public void touchPage(IManageablePage page)
 							{
 								page = WicketObjects.cloneObject(page);
-								super.addPage(page);
+								super.touchPage(page);
 							}
 						};
 					}
diff --git a/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java b/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java
index ad697ed..c686d6a 100644
--- a/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java
@@ -323,7 +323,7 @@ class PageAccessSynchronizerTest
 
 		int pageId = 1;
 		IManageablePage page = new MockPage(pageId);
-		synchronizedPageManager.addPage(page);
+		synchronizedPageManager.touchPage(page);
 		synchronizedPageManager.getPage(pageId);
 		PageLock pageLock2 = locks.get(Integer.valueOf(pageId));
 		assertNotNull(pageLock2);
diff --git a/wicket-core/src/test/java/org/apache/wicket/page/PersistentPageManagerTest.java b/wicket-core/src/test/java/org/apache/wicket/page/PersistentPageManagerTest.java
index eea83bf..d10dc4d 100644
--- a/wicket-core/src/test/java/org/apache/wicket/page/PersistentPageManagerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/page/PersistentPageManagerTest.java
@@ -65,7 +65,7 @@ class PersistentPageManagerTest
 
 		// add a page
 		TestPage toSerializePage = new TestPage();
-		pageManager.addPage(toSerializePage);
+		pageManager.touchPage(toSerializePage);
 		pageManager.detach();
 
 		// get the stored SessionEntry
diff --git a/wicket-core/src/test/java/org/apache/wicket/pageStore/RequestPageStoreTest.java b/wicket-core/src/test/java/org/apache/wicket/pageStore/RequestPageStoreTest.java
index ab7a4bc..ff02195 100644
--- a/wicket-core/src/test/java/org/apache/wicket/pageStore/RequestPageStoreTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/pageStore/RequestPageStoreTest.java
@@ -34,7 +34,7 @@ public class RequestPageStoreTest
 {
 
 	@Test
-	void test()
+	void testAdd()
 	{
 		MockPageStore mockStore = new MockPageStore();
 		
@@ -42,9 +42,9 @@ public class RequestPageStoreTest
 
 		RequestPageStore store = new RequestPageStore(mockStore);
 		
-		MockPage page1 = new MockPage(2);
-		MockPage page2 = new MockPage(3);
-		MockPage page3 = new MockPage(4);
+		MockPage page1 = new MockPage(1);
+		MockPage page2 = new MockPage(2);
+		MockPage page3 = new MockPage(3);
 		
 		store.addPage(context, page1);
 		store.addPage(context, page2);
@@ -58,8 +58,32 @@ public class RequestPageStoreTest
 		
 		mockStore.getPages().clear();
 		
+		assertNull(store.getPage(context, 1), "no page in request store");
 		assertNull(store.getPage(context, 2), "no page in request store");
 		assertNull(store.getPage(context, 3), "no page in request store");
-		assertNull(store.getPage(context, 4), "no page in request store");
+	}
+	
+	@Test
+	void testUntouch()
+	{
+		MockPageStore mockStore = new MockPageStore();
+		
+		MockPageContext context = new MockPageContext();
+
+		RequestPageStore store = new RequestPageStore(mockStore);
+		
+		MockPage page = new MockPage(1);
+		
+		store.addPage(context, page);
+		
+		store.revertPage(context, page);
+		
+		assertTrue(mockStore.getPages().isEmpty(), "no page delegated before detach");
+		
+		store.detach(context);
+		
+		assertEquals(0, mockStore.getPages().size(), "untouched page not delegated on detach");
+		
+		assertNull(store.getPage(context, 1), "no page in request store");
 	}
 }
diff --git a/wicket-core/src/test/java/org/apache/wicket/request/handler/PageProviderTest.java b/wicket-core/src/test/java/org/apache/wicket/request/handler/PageProviderTest.java
index 0bc9eb2..252760e 100644
--- a/wicket-core/src/test/java/org/apache/wicket/request/handler/PageProviderTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/request/handler/PageProviderTest.java
@@ -65,7 +65,7 @@ class PageProviderTest extends WicketTestCase
 
 		// storing test page
 		TestMapperContext mapperContext = new TestMapperContext();
-		mapperContext.getPageManager().addPage(testPage);
+		mapperContext.getPageManager().touchPage(testPage);
 		mapperContext.getPageManager().detach();
 
 		// by cleaning session cache we make sure of not being testing the same in-memory instance
@@ -80,7 +80,7 @@ class PageProviderTest extends WicketTestCase
 		// changing some sate
 		StatefullMockPage providedPage = (StatefullMockPage)pageProvider.getPageInstance();
 		providedPage.state = newState;
-		mapperContext.getPageManager().addPage(providedPage);
+		mapperContext.getPageManager().touchPage(providedPage);
 		mapperContext.getPageManager().detach();
 
 
@@ -216,7 +216,7 @@ class PageProviderTest extends WicketTestCase
 	{
 		TestMapperContext mapperContext = new TestMapperContext();
 		Page page = new TestPage();
-		mapperContext.getPageManager().addPage(page);
+		mapperContext.getPageManager().touchPage(page);
 		mapperContext.getPageManager().detach();
 
 		// by cleaning session cache we make sure of not being testing the same in-memory instance
@@ -239,7 +239,7 @@ class PageProviderTest extends WicketTestCase
 	{
 		TestMapperContext mapperContext = new TestMapperContext();
 		Page page = new TestPage();
-		mapperContext.getPageManager().addPage(page);
+		mapperContext.getPageManager().touchPage(page);
 		mapperContext.getPageManager().detach();
 
 		// by cleaning session cache we make sure of not being testing the same in-memory instance
@@ -256,7 +256,7 @@ class PageProviderTest extends WicketTestCase
 	{
 		TestMapperContext mapperContext = new TestMapperContext();
 		Page page = new TestPage();
-		mapperContext.getPageManager().addPage(page);
+		mapperContext.getPageManager().touchPage(page);
 		mapperContext.getPageManager().detach();
 
 		PageProvider pageProvider = new PageProvider(page.getPageId(), page.getRenderCount());
diff --git a/wicket-examples/src/main/java/org/apache/wicket/examples/frames/BodyFrame.java b/wicket-examples/src/main/java/org/apache/wicket/examples/frames/BodyFrame.java
index f557a59..4939eb9 100644
--- a/wicket-examples/src/main/java/org/apache/wicket/examples/frames/BodyFrame.java
+++ b/wicket-examples/src/main/java/org/apache/wicket/examples/frames/BodyFrame.java
@@ -41,7 +41,7 @@ public class BodyFrame extends WebPage
 	{
 		// create a new page instance, passing this 'master page' as an argument
 		LeftFrame leftFrame = new LeftFrame(this);
-		getSession().getPageManager().addPage(leftFrame);
+		getSession().getPageManager().touchPage(leftFrame);
 		// get the url to that page
 		IRequestHandler leftFrameHandler = new RenderPageRequestHandler(new PageProvider(leftFrame));
 		// and create a simple component that modifies it's src attribute to
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/ModalWindow.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/ModalWindow.java
index 8302650..808a9e8 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/ModalWindow.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/ModalWindow.java
@@ -1044,7 +1044,7 @@ public class ModalWindow extends Panel
 			CharSequence pageUrl;
 			RequestCycle requestCycle = RequestCycle.get();
 
-			page.getSession().getPageManager().addPage(page);
+			page.getSession().getPageManager().touchPage(page);
 			if (page.isPageStateless())
 			{
 				pageUrl = requestCycle.urlFor(page.getClass(), page.getPageParameters());