Hi,
I think that WebResponseExceptionsTest#expirePage must fail
because clicking the link should re-create the page because
TestExpirePage is bookmarkable.
I have made different changes to Wicket which I am not so sure about
but I am ready to share, and in order to get this test to pass, I had
to use a non-bookmarkable TestExpirePage:
public class TestExpirePage extends WebPage
{
private static final long serialVersionUID = 1L;
private int state;
/**
* Construct.
*/
public TestExpirePage(int state)
{
this.state = state;
...
then in the test:
tester.startPage(new TestExpirePage(1));
and this test must succeed.
More importantly we still need an additional test that positively
verifies that a bookmarkable page does NOT expire with
PageExpiredException but with the same page re-created after session
expiry.
Kind Regards
Bernard
On Mon, 19 Aug 2013 17:32:45 +0300, you wrote:
>Hi Emond,
>
>
>On Mon, Aug 19, 2013 at 12:10 PM, <[email protected]> wrote:
>
>> WICKET-4997: render bookmarkable urls for bookmarkable pages (not
>> stateless)
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/e99bf147
>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/e99bf147
>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/e99bf147
>>
>> Branch: refs/heads/wicket-4997
>> Commit: e99bf1476403ff13e409ba28331a8291a03ca25f
>> Parents: 7d8313f
>> Author: Emond Papegaaij <[email protected]>
>> Authored: Mon Aug 19 11:07:41 2013 +0200
>> Committer: Emond Papegaaij <[email protected]>
>> Committed: Mon Aug 19 11:10:02 2013 +0200
>>
>> ----------------------------------------------------------------------
>> .../main/java/org/apache/wicket/Component.java | 4 +-
>> .../mapper/AbstractBookmarkableMapper.java | 13 +++++--
>> .../core/request/mapper/MountedMapper.java | 41 ++++++++------------
>> 3 files changed, 29 insertions(+), 29 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/e99bf147/wicket-core/src/main/java/org/apache/wicket/Component.java
>> ----------------------------------------------------------------------
>> diff --git a/wicket-core/src/main/java/org/apache/wicket/Component.java
>> b/wicket-core/src/main/java/org/apache/wicket/Component.java
>> index 8fa63a3..116eb86 100644
>> --- a/wicket-core/src/main/java/org/apache/wicket/Component.java
>> +++ b/wicket-core/src/main/java/org/apache/wicket/Component.java
>> @@ -3336,7 +3336,7 @@ public abstract class Component
>> Page page = getPage();
>> PageAndComponentProvider provider = new
>> PageAndComponentProvider(page, this, parameters);
>> IRequestHandler handler;
>> - if (page.isPageStateless())
>> + if (page.isBookmarkable())
>> {
>> handler = new
>> BookmarkableListenerInterfaceRequestHandler(provider, listener, id);
>> }
>> @@ -3379,7 +3379,7 @@ public abstract class Component
>> Page page = getPage();
>> PageAndComponentProvider provider = new
>> PageAndComponentProvider(page, this, parameters);
>> IRequestHandler handler;
>> - if (page.isPageStateless())
>> + if (page.isBookmarkable())
>>
>
>I think this change is OK.
>Maybe we can improve it a bit by using Application.get().getPageSettings().
>getRecreateMountedPagesAfterExpiry() in the checks above ?
>
>With the new check as you can see the produced urls contain the class name.
>Some users don't like this.
>Application.get().getPageSettings().getRecreateMountedPagesAfterExpiry()
>return true by default. If someone doesn't like the extra info in the
>produced urls then she can disable it this way.
>
>
>> {
>> handler = new
>> BookmarkableListenerInterfaceRequestHandler(provider, listener);
>> }
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/e99bf147/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
>> b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
>> index 93c22d2..bbe2e1c 100644
>> ---
>> a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
>> +++
>> b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
>> @@ -210,8 +210,7 @@ public abstract class AbstractBookmarkableMapper
>> extends AbstractComponentMapper
>> PageProvider provider = new
>> PageProvider(pageInfo.getPageId(), pageClass, pageParameters,
>> renderCount);
>> provider.setPageSource(getContext());
>> - if (provider.isNewPageInstance() &&
>> -
>> !WebApplication.get().getPageSettings().getRecreateMountedPagesAfterExpiry())
>> + if (provider.isNewPageInstance() &&
>> !getRecreateMountedPagesAfterExpiry())
>> {
>> throw new
>> PageExpiredException(String.format("Bookmarkable page id '%d' has expired.",
>> pageInfo.getPageId()));
>> @@ -222,6 +221,11 @@ public abstract class AbstractBookmarkableMapper
>> extends AbstractComponentMapper
>> }
>> }
>>
>> + boolean getRecreateMountedPagesAfterExpiry()
>> + {
>> + return
>> WebApplication.get().getPageSettings().getRecreateMountedPagesAfterExpiry();
>> + }
>> +
>> /**
>> * Creates a {@code IRequestHandler} that processes a listener
>> request.
>> *
>> @@ -420,8 +424,11 @@ public abstract class AbstractBookmarkableMapper
>> extends AbstractComponentMapper
>>
>> requestListenerInterfaceToString(handler.getListenerInterface()),
>> handler.getComponentPath(),
>> handler.getBehaviorIndex());
>>
>> + PageParameters parameters =
>> getRecreateMountedPagesAfterExpiry() ? new PageParameters(
>> +
>> handler.getPage().getPageParameters()).mergeWith(handler.getPageParameters())
>> + : handler.getPageParameters();
>> UrlInfo urlInfo = new UrlInfo(new
>> PageComponentInfo(pageInfo, componentInfo),
>> - pageClass, handler.getPageParameters());
>> + pageClass, parameters);
>> return buildUrl(urlInfo);
>> }
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/e99bf147/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
>> b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
>> index 9b1db28..19212b6 100644
>> ---
>> a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
>> +++
>> b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
>> @@ -19,7 +19,6 @@ package org.apache.wicket.core.request.mapper;
>> import java.util.ArrayList;
>> import java.util.List;
>>
>> -import org.apache.wicket.Application;
>> import org.apache.wicket.RequestListenerInterface;
>> import
>> org.apache.wicket.core.request.handler.ListenerInterfaceRequestHandler;
>> import org.apache.wicket.request.IRequestHandler;
>> @@ -49,21 +48,21 @@ import org.apache.wicket.util.string.Strings;
>> * are matched before optional parameters, and optional parameters eager
>> (from left to right).
>> * <p>
>> * Decodes and encodes the following URLs:
>> - *
>> + *
>> * <pre>
>> * Page Class - Render (BookmarkablePageRequestHandler for mounted pages)
>> * /mount/point
>> * (these will redirect to hybrid alternative if page is not stateless)
>> - *
>> + *
>> * IPage Instance - Render Hybrid (RenderPageRequestHandler for mounted
>> pages)
>> * /mount/point?2
>> - *
>> + *
>> * IPage Instance - Bookmarkable Listener
>> (BookmarkableListenerInterfaceRequestHandler for mounted pages)
>> * /mount/point?2-click-foo-bar-baz
>> * /mount/point?2-5.click.1-foo-bar-baz (1 is behavior index, 5 is
>> render count)
>> * (these will redirect to hybrid if page is not stateless)
>> * </pre>
>> - *
>> + *
>> * @author Matej Knopp
>> */
>> public class MountedMapper extends AbstractBookmarkableMapper
>> @@ -143,7 +142,7 @@ public class MountedMapper extends
>> AbstractBookmarkableMapper
>>
>> /**
>> * Construct.
>> - *
>> + *
>> * @param mountPath
>> * @param pageClass
>> */
>> @@ -154,32 +153,32 @@ public class MountedMapper extends
>> AbstractBookmarkableMapper
>>
>> /**
>> * Construct.
>> - *
>> + *
>> * @param mountPath
>> * @param pageClassProvider
>> */
>> @Deprecated
>> public MountedMapper(String mountPath,
>> - ClassProvider<? extends IRequestablePage>
>> pageClassProvider)
>> + ClassProvider<? extends IRequestablePage>
>> pageClassProvider)
>> {
>> this(mountPath, new
>> ClassReference(pageClassProvider.get()), new PageParametersEncoder());
>> }
>>
>> /**
>> * Construct.
>> - *
>> + *
>> * @param mountPath
>> * @param pageClassProvider
>> */
>> public MountedMapper(String mountPath,
>> - IProvider<Class<? extends IRequestablePage>>
>> pageClassProvider)
>> + IProvider<Class<? extends IRequestablePage>>
>> pageClassProvider)
>> {
>> this(mountPath, pageClassProvider, new
>> PageParametersEncoder());
>> }
>>
>> /**
>> * Construct.
>> - *
>> + *
>> * @param mountPath
>> * @param pageClass
>> * @param pageParametersEncoder
>> @@ -192,7 +191,7 @@ public class MountedMapper extends
>> AbstractBookmarkableMapper
>>
>> /**
>> * Construct.
>> - *
>> + *
>> * @param mountPath
>> * @param pageClassProvider
>> * @param pageParametersEncoder
>> @@ -202,20 +201,19 @@ public class MountedMapper extends
>> AbstractBookmarkableMapper
>> ClassProvider<? extends IRequestablePage>
>> pageClassProvider,
>> IPageParametersEncoder pageParametersEncoder)
>> {
>> - this(mountPath, new
>> ClassReference(pageClassProvider.get()),
>> - pageParametersEncoder);
>> + this(mountPath, new
>> ClassReference(pageClassProvider.get()), pageParametersEncoder);
>> }
>>
>> /**
>> * Construct.
>> - *
>> + *
>> * @param mountPath
>> * @param pageClassProvider
>> * @param pageParametersEncoder
>> */
>> public MountedMapper(String mountPath,
>> - IProvider<Class<? extends IRequestablePage>>
>> pageClassProvider,
>> - IPageParametersEncoder pageParametersEncoder)
>> + IProvider<Class<? extends IRequestablePage>>
>> pageClassProvider,
>> + IPageParametersEncoder pageParametersEncoder)
>> {
>> Args.notEmpty(mountPath, "mountPath");
>> Args.notNull(pageClassProvider, "pageClassProvider");
>> @@ -427,11 +425,6 @@ public class MountedMapper extends
>> AbstractBookmarkableMapper
>> return url;
>> }
>>
>> - boolean getRecreateMountedPagesAfterExpiry()
>> - {
>> - return
>> Application.get().getPageSettings().getRecreateMountedPagesAfterExpiry();
>> - }
>> -
>> /**
>> * @see
>> AbstractBookmarkableMapper#buildUrl(AbstractBookmarkableMapper.UrlInfo)
>> */
>> @@ -483,7 +476,7 @@ public class MountedMapper extends
>> AbstractBookmarkableMapper
>> /**
>> * Check if the URL is for home page and the home page class match
>> mounted class. If so,
>> * redirect to mounted URL.
>> - *
>> + *
>> * @param url
>> * @return request handler or <code>null</code>
>> */
>> @@ -504,7 +497,7 @@ public class MountedMapper extends
>> AbstractBookmarkableMapper
>> * If this method returns <code>true</code> and application home
>> page class is same as the class
>> * mounted with this encoder, request to home page will result in
>> a redirect to the mounted
>> * path.
>> - *
>> + *
>> * @return whether this encode should respond to home page request
>> when home page class is same
>> * as mounted class.
>> */
>>
>>
