You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2012/04/23 14:18:45 UTC
git commit: WICKET-4441 PageProvider should create a new Page
instance if PageParameters are changed, even if a stored page exists.
Updated Branches:
refs/heads/master 98748dc30 -> 54c86ebbd
WICKET-4441 PageProvider should create a new Page instance if PageParameters are changed, even if a stored page exists.
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/54c86ebb
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/54c86ebb
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/54c86ebb
Branch: refs/heads/master
Commit: 54c86ebbd70dbd55639f91cf6a5800765b12d18c
Parents: 98748dc
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
Authored: Mon Apr 23 15:17:39 2012 +0300
Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
Committed: Mon Apr 23 15:17:39 2012 +0300
----------------------------------------------------------------------
.../wicket/core/request/handler/PageProvider.java | 24 +++++++++-
.../request/mapper/BookmarkableMapperTest.java | 14 +++--
.../core/request/mapper/MountedMapperTest.java | 14 +++--
.../core/request/mapper/PackageMapperTest.java | 14 ++++--
.../core/request/mapper/TestMapperContext.java | 16 +++++-
.../markup/html/link/MountedPageLinkTest.java | 1 +
.../wicket/request/handler/PageProviderTest.java | 38 +++++++++++++++
7 files changed, 100 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/wicket/blob/54c86ebb/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java
index 50402f6..6068e35 100644
--- a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java
+++ b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java
@@ -296,7 +296,10 @@ public class PageProvider implements IPageProvider
IRequestablePage storedPageInstance = getPageSource().getPageInstance(pageId);
if (storedPageInstance != null)
{
- if (pageClass == null || pageClass.equals(storedPageInstance.getClass()))
+ if (
+ (pageClass == null || pageClass.equals(storedPageInstance.getClass())) &&
+ (isPageParametersEmpty(pageParameters) || arePageParametersSame(storedPageInstance))
+ )
{
pageInstance = storedPageInstance;
pageInstanceIsFresh = false;
@@ -413,4 +416,23 @@ public class PageProvider implements IPageProvider
}
return pageInstanceIsFresh;
}
+
+ /**
+ * A helper method that compares the requested PageParameters with the ones in the stored
+ * page instance. {@code null} and empty PageParameters are considered equal.
+ *
+ * @param storedPageInstance
+ * the page instance with the original page parameters
+ * @return {@code true} if the indexed and named parameters are equal, {@code false} - otherwise
+ */
+ private boolean arePageParametersSame(IRequestablePage storedPageInstance) {
+ PageParameters currentCopy = new PageParameters(pageParameters);
+ PageParameters storedCopy = new PageParameters(storedPageInstance.getPageParameters());
+ return currentCopy.equals(storedCopy);
+ }
+
+ private boolean isPageParametersEmpty(PageParameters parameters)
+ {
+ return parameters == null || parameters.isEmpty();
+ }
}
http://git-wip-us.apache.org/repos/asf/wicket/blob/54c86ebb/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BookmarkableMapperTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BookmarkableMapperTest.java b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BookmarkableMapperTest.java
index af91e93..a634610 100644
--- a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BookmarkableMapperTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BookmarkableMapperTest.java
@@ -112,13 +112,14 @@ public class BookmarkableMapperTest extends AbstractMapperTest
IRequestHandler handler = encoder.mapRequest(getRequest(url));
assertTrue(handler instanceof RenderPageRequestHandler);
+ context.setCurrentPageParameters(((RenderPageRequestHandler)handler).getPageParameters());
IRequestablePage page = ((RenderPageRequestHandler)handler).getPage();
checkPage(page, 15);
PageParameters p = page.getPageParameters();
- assertEquals(0, p.getIndexedCount());
+ assertEquals(2, p.getIndexedCount());
- assertEquals(0, p.getNamedKeys().size());
+ assertEquals(2, p.getNamedKeys().size());
}
/**
@@ -154,7 +155,7 @@ public class BookmarkableMapperTest extends AbstractMapperTest
assertTrue(handler instanceof ListenerInterfaceRequestHandler);
ListenerInterfaceRequestHandler h = (ListenerInterfaceRequestHandler)handler;
-
+ context.setCurrentPageParameters(h.getPageParameters());
IRequestablePage page = h.getPage();
checkPage(page, 15);
@@ -162,9 +163,9 @@ public class BookmarkableMapperTest extends AbstractMapperTest
assertEquals("foo:bar", h.getComponent().getPageRelativePath());
PageParameters p = page.getPageParameters();
- assertEquals(0, p.getIndexedCount());
+ assertEquals(2, p.getIndexedCount());
- assertEquals(0, p.getNamedKeys().size());
+ assertEquals(2, p.getNamedKeys().size());
}
/**
@@ -204,6 +205,7 @@ public class BookmarkableMapperTest extends AbstractMapperTest
assertTrue(handler instanceof ListenerInterfaceRequestHandler);
ListenerInterfaceRequestHandler h = (ListenerInterfaceRequestHandler)handler;
+ context.setCurrentPageParameters(h.getPageParameters());
IRequestablePage page = h.getPage();
assertEquals(page.getRenderCount(), 5);
@@ -220,7 +222,7 @@ public class BookmarkableMapperTest extends AbstractMapperTest
context.setNextPageRenderCount(6);
IRequestHandler handler = encoder.mapRequest(getRequest(url));
-
+ context.setCurrentPageParameters(((ListenerInterfaceRequestHandler)handler).getPageParameters());
((IPageRequestHandler)handler).getPage();
}
http://git-wip-us.apache.org/repos/asf/wicket/blob/54c86ebb/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java
index 62dd866..76a253b 100644
--- a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java
@@ -156,13 +156,14 @@ public class MountedMapperTest extends AbstractMapperTest
IRequestHandler handler = encoder.mapRequest(getRequest(url));
assertTrue(handler instanceof RenderPageRequestHandler);
+ context.setCurrentPageParameters(((RenderPageRequestHandler)handler).getPageParameters());
IRequestablePage page = ((RenderPageRequestHandler)handler).getPage();
checkPage(page, 15);
PageParameters p = page.getPageParameters();
- assertEquals(0, p.getIndexedCount());
+ assertEquals(2, p.getIndexedCount());
- assertEquals(0, p.getNamedKeys().size());
+ assertEquals(2, p.getNamedKeys().size());
}
/**
@@ -177,7 +178,7 @@ public class MountedMapperTest extends AbstractMapperTest
assertTrue(handler instanceof ListenerInterfaceRequestHandler);
ListenerInterfaceRequestHandler h = (ListenerInterfaceRequestHandler)handler;
-
+ context.setCurrentPageParameters(h.getPageParameters());
IRequestablePage page = h.getPage();
checkPage(page, 15);
@@ -197,7 +198,7 @@ public class MountedMapperTest extends AbstractMapperTest
assertTrue(handler instanceof ListenerInterfaceRequestHandler);
ListenerInterfaceRequestHandler h = (ListenerInterfaceRequestHandler)handler;
-
+ context.setCurrentPageParameters(h.getPageParameters());
IRequestablePage page = h.getPage();
checkPage(page, 15);
@@ -205,9 +206,9 @@ public class MountedMapperTest extends AbstractMapperTest
assertEquals("foo:bar", h.getComponent().getPageRelativePath());
PageParameters p = page.getPageParameters();
- assertEquals(0, p.getIndexedCount());
+ assertEquals(2, p.getIndexedCount());
- assertEquals(0, p.getNamedKeys().size());
+ assertEquals(2, p.getNamedKeys().size());
}
/**
@@ -222,6 +223,7 @@ public class MountedMapperTest extends AbstractMapperTest
assertTrue(handler instanceof ListenerInterfaceRequestHandler);
ListenerInterfaceRequestHandler h = (ListenerInterfaceRequestHandler)handler;
+ context.setCurrentPageParameters(h.getPageParameters());
IRequestablePage page = h.getPage();
checkPage(page, 15);
http://git-wip-us.apache.org/repos/asf/wicket/blob/54c86ebb/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/PackageMapperTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/PackageMapperTest.java b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/PackageMapperTest.java
index a342bd5..094badd 100644
--- a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/PackageMapperTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/PackageMapperTest.java
@@ -156,13 +156,15 @@ public class PackageMapperTest extends AbstractMapperTest
IRequestHandler handler = encoder.mapRequest(getRequest(url));
assertTrue(handler instanceof RenderPageRequestHandler);
+
+ context.setCurrentPageParameters(((RenderPageRequestHandler)handler).getPageParameters());
IRequestablePage page = ((RenderPageRequestHandler)handler).getPage();
checkPage(page, 15);
PageParameters p = page.getPageParameters();
- assertEquals(0, p.getIndexedCount());
+ assertEquals(2, p.getIndexedCount());
- assertEquals(0, p.getNamedKeys().size());
+ assertEquals(2, p.getNamedKeys().size());
}
/**
@@ -197,6 +199,7 @@ public class PackageMapperTest extends AbstractMapperTest
assertTrue(handler instanceof ListenerInterfaceRequestHandler);
ListenerInterfaceRequestHandler h = (ListenerInterfaceRequestHandler)handler;
+ context.setCurrentPageParameters(h.getPageParameters());
IRequestablePage page = h.getPage();
checkPage(page, 15);
@@ -205,9 +208,9 @@ public class PackageMapperTest extends AbstractMapperTest
assertEquals("foo:bar", h.getComponent().getPageRelativePath());
PageParameters p = page.getPageParameters();
- assertEquals(0, p.getIndexedCount());
+ assertEquals(2, p.getIndexedCount());
- assertEquals(0, p.getNamedKeys().size());
+ assertEquals(2, p.getNamedKeys().size());
}
/**
@@ -245,6 +248,7 @@ public class PackageMapperTest extends AbstractMapperTest
assertTrue(handler instanceof ListenerInterfaceRequestHandler);
ListenerInterfaceRequestHandler h = (ListenerInterfaceRequestHandler)handler;
+ context.setCurrentPageParameters(h.getPageParameters());
IRequestablePage page = h.getPage();
assertEquals(page.getRenderCount(), 5);
@@ -261,9 +265,9 @@ public class PackageMapperTest extends AbstractMapperTest
context.setNextPageRenderCount(6);
IRequestHandler handler = encoder.mapRequest(getRequest(url));
+ context.setCurrentPageParameters(((ListenerInterfaceRequestHandler)handler).getPageParameters());
((IPageRequestHandler)handler).getPage();
-
}
/**
http://git-wip-us.apache.org/repos/asf/wicket/blob/54c86ebb/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/TestMapperContext.java
----------------------------------------------------------------------
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 b3e137a..98f5983 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
@@ -50,6 +50,7 @@ public class TestMapperContext implements IMapperContext
PageStoreManager pageManager;
private String appName;
private boolean createMockPageIfInstanceNotFound = true;
+ private PageParameters currentPageParameters = null;
/**
* Construct.
@@ -171,6 +172,11 @@ public class TestMapperContext implements IMapperContext
page.setCreatedBookmarkable(createdBookmarkable);
page.setRenderCount(nextPageRenderCount);
requestablePage = page;
+ if (currentPageParameters != null)
+ {
+ page.getPageParameters().overwriteWith(currentPageParameters);
+ currentPageParameters = null;
+ }
}
return requestablePage;
@@ -184,8 +190,7 @@ public class TestMapperContext implements IMapperContext
{
try
{
- MockPage page;
- page = (MockPage)pageClass.newInstance();
+ MockPage page = (MockPage)pageClass.newInstance();
page.setPageId(++idCounter);
page.setBookmarkable(true);
page.setCreatedBookmarkable(true);
@@ -219,6 +224,12 @@ public class TestMapperContext implements IMapperContext
return MockPage.class;
}
+ public TestMapperContext setCurrentPageParameters(PageParameters parameters)
+ {
+ this.currentPageParameters = parameters;
+ return this;
+ }
+
/**
*
* Adapts {@link PageProvider} to this {@link IMapperContext}
@@ -227,7 +238,6 @@ public class TestMapperContext implements IMapperContext
*/
public class TestPageProvider extends PageProvider
{
-
/**
* Construct.
*
http://git-wip-us.apache.org/repos/asf/wicket/blob/54c86ebb/wicket-core/src/test/java/org/apache/wicket/markup/html/link/MountedPageLinkTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/MountedPageLinkTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/MountedPageLinkTest.java
index a665352..22722e3 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/MountedPageLinkTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/MountedPageLinkTest.java
@@ -17,6 +17,7 @@
package org.apache.wicket.markup.html.link;
import org.apache.wicket.WicketTestCase;
+import org.apache.wicket.core.request.mapper.StalePageException;
import org.apache.wicket.protocol.http.PageExpiredException;
import org.apache.wicket.core.request.mapper.PageInstanceMapper;
import org.apache.wicket.request.mapper.parameter.PageParameters;
http://git-wip-us.apache.org/repos/asf/wicket/blob/54c86ebb/wicket-core/src/test/java/org/apache/wicket/request/handler/PageProviderTest.java
----------------------------------------------------------------------
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 fc22a58..1fdd331 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
@@ -34,6 +34,7 @@ import org.apache.wicket.markup.html.link.Link;
import org.apache.wicket.request.Url;
import org.apache.wicket.core.request.mapper.StalePageException;
import org.apache.wicket.core.request.mapper.TestMapperContext;
+import org.apache.wicket.request.mapper.parameter.PageParameters;
import org.apache.wicket.util.resource.IResourceStream;
import org.apache.wicket.util.resource.ResourceStreamNotFoundException;
import org.apache.wicket.util.resource.StringResourceStream;
@@ -252,6 +253,36 @@ public class PageProviderTest extends WicketTestCase
assertTrue(provider.isPageInstanceFresh());
}
+ /**
+ * https://issues.apache.org/jira/browse/WICKET-4441
+ *
+ * ProductPage is mounted at /product/${productId}.
+ * A request to /product/10?2 will show the product with id = 10.
+ * A following request to /product/20?2 (e.g. a bookmarked url sent by a colleague)
+ * will again show product with id=10 ignoring the fact that product with id=20 is
+ * requested because Wicket loads the old page with pageId=2 from the current session.
+ */
+ @Test
+ public void ignorePageFoundByIdIfItsClassMatchButPageParametersDont()
+ {
+ TestMapperContext mapperContext = new TestMapperContext();
+ PageParameters initialParameters = new PageParameters();
+ initialParameters.add("name1", "value1");
+ Page page = new TestPage(initialParameters);
+ mapperContext.getPageManager().touchPage(page);
+ mapperContext.getPageManager().commitRequest();
+
+ // by cleaning session cache we make sure of not being testing the same in-memory instance
+ mapperContext.cleanSessionCache();
+
+ PageParameters newParameters = new PageParameters();
+ newParameters.add("name2", "value2");
+ PageProvider provider = new PageProvider(page.getPageId(), TestPage.class, newParameters, 0);
+ assertFalse(provider.hasPageInstance());
+ assertEquals(TestPage.class, provider.getPageInstance().getClass());
+ assertTrue(provider.isPageInstanceFresh());
+ }
+
/** */
public static class TestPage extends WebPage implements IMarkupResourceStreamProvider
{
@@ -262,6 +293,13 @@ public class PageProviderTest extends WicketTestCase
/** */
public TestPage()
{
+ this(new PageParameters());
+ }
+
+ public TestPage(PageParameters parameters)
+ {
+ super(parameters);
+
add(link = new Link<Void>("link")
{
private static final long serialVersionUID = 1L;