You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by pa...@apache.org on 2013/08/19 11:10:40 UTC
[4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
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 <em...@topicus.nl>
Authored: Mon Aug 19 11:07:41 2013 +0200
Committer: Emond Papegaaij <em...@topicus.nl>
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())
{
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.
*/
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Bernard <bh...@gmail.com>.
Thanks very much Emond for the good work!
I found some behavior confusing at first but I agree with Emond -
correctness provides the best result for PageExpiredException recovery
and keeps the framework clean.
After Emond has improved PageExpiredException recovery, there is still
one other area that could be addressed for a major improvement:
PageParameters missing from re-created Page
https://issues.apache.org/jira/browse/WICKET-5068
I am suggesting there a method that lets Component override a default
when it wants to process parameters during PageExpiredException
recovery in case the system thinks it should not do so.
Regards,
Bernard
On Tue, 20 Aug 2013 17:20:58 +0200, you wrote:
>Well, there is no urlFor method that takes a page, but if you use urlFor with
>a RenderPageRequestHandler, the resulting url will be the same as this
>method is not affected. Only the methods for RequestListenerInterface urls
>are changed. For those, the url will be the bookmarkable version (with a
>page id by the way). If you don't want that, your page should override
>isBookmarkable and return false if it was constructed with the model
>constructor.
>
>I don't think it's possible to prevent this. The current behavior was based
>on isPageStateless, which was incorrect anyway. The result for your page
>would have been the same if it is stateless.
>Page.wasCreatedBookmarkable also does not give the desired result, as it
>is only set when the page was created as a result of opening a
>bookmarkable url, not by using a bookmarkable constructor.
>
>Best regards,
>Emond
>
>On Tuesday 20 August 2013 08:09:48 Igor Vaynberg wrote:
>> so what happens to the pages that are both bookmarkable and not?
>>
>> public class EditCustomerPage {
>> public EditCustomerPage(PageParameters params) {
>> this(getEntity(params, "customer"));
>> }
>>
>> public EditCustomerPage(IModel<Customer> customer) {
>> ...
>> }
>> }
>>
>> what url will i get now when i say urlFor(new
>> EditCustomerPage(customerModel)) ?
>>
>> -igor
>>
>>
>> On Tue, Aug 20, 2013 at 5:39 AM, Emond Papegaaij
><emond.papegaaij@topicus.nl
>> > wrote:
>> >
>> > On Monday 19 August 2013 17:32:45 Martin Grigorov wrote:
>> > > Hi Emond,
>> > >
>> > > 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.
>> >
>> > I've thought some more about this, and I think the current behavior is
>ok.
>> > If
>> > you don't want bookmarkable urls, but your page has a bookmarkable
>> > constructor, you should override isBookmarkable. The setting is more
>> > about request handling than it is about rendering urls. The reason the
>> > pageparameters are not rendered in the url is, that they would
>otherwise
>> > be rendered as query parameters, overriding other (such as form)
>> > parameters.
>> >
>> > I'll merge the branch somewhere tomorrow and forward port it to 7 if
>> > nobody objects.
>> >
>> > Best regards,
>> > Emond
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for bookmarkable pages (not stateless)
Posted by Emond Papegaaij <em...@topicus.nl>.
Well, there is no urlFor method that takes a page, but if you use urlFor with
a RenderPageRequestHandler, the resulting url will be the same as this
method is not affected. Only the methods for RequestListenerInterface urls
are changed. For those, the url will be the bookmarkable version (with a
page id by the way). If you don't want that, your page should override
isBookmarkable and return false if it was constructed with the model
constructor.
I don't think it's possible to prevent this. The current behavior was based
on isPageStateless, which was incorrect anyway. The result for your page
would have been the same if it is stateless.
Page.wasCreatedBookmarkable also does not give the desired result, as it
is only set when the page was created as a result of opening a
bookmarkable url, not by using a bookmarkable constructor.
Best regards,
Emond
On Tuesday 20 August 2013 08:09:48 Igor Vaynberg wrote:
> so what happens to the pages that are both bookmarkable and not?
>
> public class EditCustomerPage {
> public EditCustomerPage(PageParameters params) {
> this(getEntity(params, "customer"));
> }
>
> public EditCustomerPage(IModel<Customer> customer) {
> ...
> }
> }
>
> what url will i get now when i say urlFor(new
> EditCustomerPage(customerModel)) ?
>
> -igor
>
>
> On Tue, Aug 20, 2013 at 5:39 AM, Emond Papegaaij
<emond.papegaaij@topicus.nl
> > wrote:
> >
> > On Monday 19 August 2013 17:32:45 Martin Grigorov wrote:
> > > Hi Emond,
> > >
> > > 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.
> >
> > I've thought some more about this, and I think the current behavior is
ok.
> > If
> > you don't want bookmarkable urls, but your page has a bookmarkable
> > constructor, you should override isBookmarkable. The setting is more
> > about request handling than it is about rendering urls. The reason the
> > pageparameters are not rendered in the url is, that they would
otherwise
> > be rendered as query parameters, overriding other (such as form)
> > parameters.
> >
> > I'll merge the branch somewhere tomorrow and forward port it to 7 if
> > nobody objects.
> >
> > Best regards,
> > Emond
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Igor Vaynberg <ig...@gmail.com>.
so what happens to the pages that are both bookmarkable and not?
public class EditCustomerPage {
public EditCustomerPage(PageParameters params) {
this(getEntity(params, "customer"));
}
public EditCustomerPage(IModel<Customer> customer) {
...
}
}
what url will i get now when i say urlFor(new
EditCustomerPage(customerModel)) ?
-igor
On Tue, Aug 20, 2013 at 5:39 AM, Emond Papegaaij <emond.papegaaij@topicus.nl
> wrote:
> On Monday 19 August 2013 17:32:45 Martin Grigorov wrote:
> > Hi Emond,
> >
> > 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.
>
> I've thought some more about this, and I think the current behavior is ok.
> If
> you don't want bookmarkable urls, but your page has a bookmarkable
> constructor, you should override isBookmarkable. The setting is more
> about request handling than it is about rendering urls. The reason the
> pageparameters are not rendered in the url is, that they would otherwise
> be rendered as query parameters, overriding other (such as form)
> parameters.
>
> I'll merge the branch somewhere tomorrow and forward port it to 7 if
> nobody objects.
>
> Best regards,
> Emond
>
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for bookmarkable pages (not stateless)
Posted by Emond Papegaaij <em...@topicus.nl>.
On Monday 19 August 2013 17:32:45 Martin Grigorov wrote:
> Hi Emond,
>
> 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.
I've thought some more about this, and I think the current behavior is ok. If
you don't want bookmarkable urls, but your page has a bookmarkable
constructor, you should override isBookmarkable. The setting is more
about request handling than it is about rendering urls. The reason the
pageparameters are not rendered in the url is, that they would otherwise
be rendered as query parameters, overriding other (such as form)
parameters.
I'll merge the branch somewhere tomorrow and forward port it to 7 if
nobody objects.
Best regards,
Emond
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for bookmarkable
pages (not stateless)
Posted by Michael Mosmann <mi...@mosmann.de>.
Am 22.08.13 11:25, schrieb Bernard:
> Hi,
> On Thu, 22 Aug 2013 11:21:41 +0300, you wrote:
>
>> On Thu, Aug 22, 2013 at 11:08 AM, Emond Papegaaij <
>> emond.papegaaij@topicus.nl> wrote:
> snip
>>> The mismatch is in the fact that the PageParameters object is stored with
>>> the page while it resembles the parameters the page was created with. If
>>> a page changes (for example, by updating its model object), the
>>> PageParameters no longer match with the state of the page. That's why I'd
>>> rather not store them in the page at all but recreate them when needed.
>>>
>> I don't see why Page's parameters should be related 1:1 to page's model.
>> The parameters are just the data that has been provided for the page
>> construction. Nothing more.
>> Later they could be used for url generation or not.
>> The page may use some of the parameters to create its model or not.
>>
> snip
>
> I can see Martijn's point, and if one defines this as a convention, as
> it already is, then no more confusion.
>
> But I think it is a good concept to generate PageParameters from the
> state of the page.
... which can be complicated if you want to preserve any state of your
page in an external representation through page parameters. I had build
something like that and it was a pain (did this for SEO)...
> It would be good practice.
.. i am not sure about that.
> I already use public
> static methods MyPage#createParameters(MyEntity something) which
> encapsulates this so my page instance would just call the static
> method with the parameters from the state of the page.
>
> So how could this be enforced? The framework could insist on
> overriding such a method if it detects a PageParameters constructor.
.. sometimes you do not want to present internal state.
> Unfortunately that constructor is abused at the moment. See
> WICKET-5069.
>
> It would not be performant so caching would be needed.
>
> Perhaps update the instance var pageparameters with the result of a
> getPageParameters() call on modelChanged()?
I think we mixing different kind of things here.. If you can externalize
the state of a page into a page parameter string representation, then
your page does not have any state. Every link on such a page should be
therefor a BookmarkablePageLink with an different set of page parameters
which describe the transition to a different client side state. You must
then redirect every form submit or non-bookmarkable links to a page with
page parameters describing the new state. This way there will be no
PageExpired exception.
This way you use wicket as a component based page renderer.. which is
ok, but not a strength of wicket.
We can make it easier to provide classes and functions if this is what
you want. But i fear that there is not the one solution, because there
are often very surprising requirements.
Michael:)
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Bernard <bh...@gmail.com>.
Hi,
On Thu, 22 Aug 2013 11:21:41 +0300, you wrote:
>On Thu, Aug 22, 2013 at 11:08 AM, Emond Papegaaij <
>emond.papegaaij@topicus.nl> wrote:
snip
>> The mismatch is in the fact that the PageParameters object is stored with
>> the page while it resembles the parameters the page was created with. If
>> a page changes (for example, by updating its model object), the
>> PageParameters no longer match with the state of the page. That's why I'd
>> rather not store them in the page at all but recreate them when needed.
>>
>
>I don't see why Page's parameters should be related 1:1 to page's model.
>The parameters are just the data that has been provided for the page
>construction. Nothing more.
>Later they could be used for url generation or not.
>The page may use some of the parameters to create its model or not.
>
snip
I can see Martijn's point, and if one defines this as a convention, as
it already is, then no more confusion.
But I think it is a good concept to generate PageParameters from the
state of the page. It would be good practice. I already use public
static methods MyPage#createParameters(MyEntity something) which
encapsulates this so my page instance would just call the static
method with the parameters from the state of the page.
So how could this be enforced? The framework could insist on
overriding such a method if it detects a PageParameters constructor.
Unfortunately that constructor is abused at the moment. See
WICKET-5069.
It would not be performant so caching would be needed.
Perhaps update the instance var pageparameters with the result of a
getPageParameters() call on modelChanged()?
Kind Regards,
Bernard
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Martin Grigorov <mg...@apache.org>.
On Thu, Aug 22, 2013 at 11:08 AM, Emond Papegaaij <
emond.papegaaij@topicus.nl> wrote:
> On Thursday 22 August 2013 10:43:57 Martin Grigorov wrote:
> > On Wed, Aug 21, 2013 at 10:10 PM, Emond Papegaaij
> <emond.papegaaij@gmail.com
> > > wrote:
> > > For Wicket 7, we might want to take a look at the PageParameters and
> > > mounts
> > > because they hold several caveats. The most important is that the
> > > lifecycle
> > > of PageParameters if different than that of the Page they belong to.
> > > PageParameters are bound to a request, whereas a page instance
> can live
> > > for
> > > many requests. Also, it is possible to modify the PageParameters
> passed to
> >
> > PageParameters has a confusing name, e.g. PageParameters are
> passed to
> > IResource#respond(Attributes#getPageParameters).
> > Here is how I understand the lifecycle - when a page is instantiated
> Wicket
> > may pass PageParameters to its constructor. Those PageParameters
> have the
> > same lifecycle as the page itself. They are reachable via
> > Page#getPageParameters.
> > When a link is clicked its parameters are reachable in #onClick via
> > getRequest.getRequestParameters. Using getPage.getPageParameters
> will
> > return the parameters for the page creation, not for the link's click, or
> > form submit.
>
> The mismatch is in the fact that the PageParameters object is stored with
> the page while it resembles the parameters the page was created with. If
> a page changes (for example, by updating its model object), the
> PageParameters no longer match with the state of the page. That's why I'd
> rather not store them in the page at all but recreate them when needed.
>
I don't see why Page's parameters should be related 1:1 to page's model.
The parameters are just the data that has been provided for the page
construction. Nothing more.
Later they could be used for url generation or not.
The page may use some of the parameters to create its model or not.
> > > the super contructor, but those changes will be overridden by Wicket
> after
> >
> > I don't see what you mean here.
>
> DefaultPageFactory:204 If you change the PageParameter instance in the
> construtor, wicket will overwrite it.
>
> > > constructing the page. Finally, you can override getPageParameters in
> your
> >
> > Yes. Overriding this method may return wrong data later.
> > See
> >
> https://cwiki.apache.org/confluence/display/WICKET/Wicket+7.0+Roadmap#
> Wicket
> > 7.0Roadmap-PageParametersimprovements -
> > here I suggest to make the Page's parameters immutable. I like this
> change
> > A LOT, but as you can see in the patch in
> > https://issues.apache.org/jira/browse/WICKET-4774 it requires too many
> API
> > breaks :-/
>
> If we do not store the PageParamters in the page anymore, there will be no
> need for a mutable PageParameters object. Still, the API break will be
> huge.
>
> > > page, which will confuse Wicket even more. Also, PageParameters only
> > > handle
> > > strings, which is quite limited. I don't know how to improve this.
> Perhaps
> >
> > What do you mean by this ? The data coming from the client is not typed,
> so
> > using String is the most natural choice. You can use StringValue#toXyz()
> to
> > convert it to other primitives.
>
> JAX-RS is able to convert any object from and to Strings using some well
> known patterns: A constructor taking a string or a static valueOf(String)
> method.
>
> Best regards,
> Emond
>
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for bookmarkable pages (not stateless)
Posted by Emond Papegaaij <em...@topicus.nl>.
On Thursday 22 August 2013 10:43:57 Martin Grigorov wrote:
> On Wed, Aug 21, 2013 at 10:10 PM, Emond Papegaaij
<emond.papegaaij@gmail.com
> > wrote:
> > For Wicket 7, we might want to take a look at the PageParameters and
> > mounts
> > because they hold several caveats. The most important is that the
> > lifecycle
> > of PageParameters if different than that of the Page they belong to.
> > PageParameters are bound to a request, whereas a page instance
can live
> > for
> > many requests. Also, it is possible to modify the PageParameters
passed to
>
> PageParameters has a confusing name, e.g. PageParameters are
passed to
> IResource#respond(Attributes#getPageParameters).
> Here is how I understand the lifecycle - when a page is instantiated
Wicket
> may pass PageParameters to its constructor. Those PageParameters
have the
> same lifecycle as the page itself. They are reachable via
> Page#getPageParameters.
> When a link is clicked its parameters are reachable in #onClick via
> getRequest.getRequestParameters. Using getPage.getPageParameters
will
> return the parameters for the page creation, not for the link's click, or
> form submit.
The mismatch is in the fact that the PageParameters object is stored with
the page while it resembles the parameters the page was created with. If
a page changes (for example, by updating its model object), the
PageParameters no longer match with the state of the page. That's why I'd
rather not store them in the page at all but recreate them when needed.
> > the super contructor, but those changes will be overridden by Wicket
after
>
> I don't see what you mean here.
DefaultPageFactory:204 If you change the PageParameter instance in the
construtor, wicket will overwrite it.
> > constructing the page. Finally, you can override getPageParameters in
your
>
> Yes. Overriding this method may return wrong data later.
> See
>
https://cwiki.apache.org/confluence/display/WICKET/Wicket+7.0+Roadmap#
Wicket
> 7.0Roadmap-PageParametersimprovements -
> here I suggest to make the Page's parameters immutable. I like this
change
> A LOT, but as you can see in the patch in
> https://issues.apache.org/jira/browse/WICKET-4774 it requires too many
API
> breaks :-/
If we do not store the PageParamters in the page anymore, there will be no
need for a mutable PageParameters object. Still, the API break will be
huge.
> > page, which will confuse Wicket even more. Also, PageParameters only
> > handle
> > strings, which is quite limited. I don't know how to improve this.
Perhaps
>
> What do you mean by this ? The data coming from the client is not typed,
so
> using String is the most natural choice. You can use StringValue#toXyz()
to
> convert it to other primitives.
JAX-RS is able to convert any object from and to Strings using some well
known patterns: A constructor taking a string or a static valueOf(String)
method.
Best regards,
Emond
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Martin Grigorov <mg...@apache.org>.
Hi,
On Wed, Aug 21, 2013 at 10:10 PM, Emond Papegaaij <emond.papegaaij@gmail.com
> wrote:
> Martijn and I discussed some more about this, and we've came to the
> conclusion that we probably should not try to fix this in Wicket 6. The
> cause of this issue (and several others) is a mismatch between Page,
> PageParameters and statefulness that cannot be fixed without breaking the
> API in several ways. The reason Component.urlFor checks isPageStateless is
> that a stateful page cannot be recreated from a bookmarkable url. If you
> do, you will loose (some of) its state. However, looking at it again, I'm
> starting to think we should at least to try improve the situation for
> Wicket 6.
>
> At the moment, there is a discrepancy between mounted and bookmarkable
> pages. Consider MyPage, a bookmarkable but stateful page with a Link. The
> href for this link will be something like
> /wicket/page?2-1.ILinkListener-link. Now, if I mount this page under
> /mypage, the href for the link will suddenly become
> /mypage?2-1.ILinkListener-link. I don't see why a mounted stateful page can
> and should be treated differently than a bookmarkable stateful page. IMHO,
> they are 2 forms of the same pattern. Therefore, I've just pushed an update
> to the wicket-4997 that tries to improve the situation by using
> bookmarkable urls for stateless pages (as it was), but also for
> bookmarkable pages that were created bookmarkable. This seems to give much
> better results in the testcases. I would really appreciate it if some more
> people could take a look at this, because url rendering is quite a
> sensitive subject.
>
> For Wicket 7, we might want to take a look at the PageParameters and mounts
> because they hold several caveats. The most important is that the lifecycle
> of PageParameters if different than that of the Page they belong to.
> PageParameters are bound to a request, whereas a page instance can live for
> many requests. Also, it is possible to modify the PageParameters passed to
>
PageParameters has a confusing name, e.g. PageParameters are passed to
IResource#respond(Attributes#getPageParameters).
Here is how I understand the lifecycle - when a page is instantiated Wicket
may pass PageParameters to its constructor. Those PageParameters have the
same lifecycle as the page itself. They are reachable via
Page#getPageParameters.
When a link is clicked its parameters are reachable in #onClick via
getRequest.getRequestParameters. Using getPage.getPageParameters will
return the parameters for the page creation, not for the link's click, or
form submit.
> the super contructor, but those changes will be overridden by Wicket after
>
I don't see what you mean here.
> constructing the page. Finally, you can override getPageParameters in your
>
Yes. Overriding this method may return wrong data later.
See
https://cwiki.apache.org/confluence/display/WICKET/Wicket+7.0+Roadmap#Wicket7.0Roadmap-PageParametersimprovements
-
here I suggest to make the Page's parameters immutable. I like this change
A LOT, but as you can see in the patch in
https://issues.apache.org/jira/browse/WICKET-4774 it requires too many API
breaks :-/
> page, which will confuse Wicket even more. Also, PageParameters only handle
> strings, which is quite limited. I don't know how to improve this. Perhaps
>
What do you mean by this ? The data coming from the client is not typed, so
using String is the most natural choice. You can use StringValue#toXyz() to
convert it to other primitives.
> we can borrow some ideas from JAX-RS? On the other hand, they don't have to
> deal with annoying things like state :)
>
I don't quite like the repetitive approach in JAX-RS -
@ParamValue("number") int number -, note that there is "number" twice, but
if you like it then you can create a new IPageFactory impl that does this
for you.
>
> Best regards,
> Emond
>
>
>
> On Wed, Aug 21, 2013 at 8:54 AM, Bernard <bh...@gmail.com> wrote:
>
> > 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, <pa...@apache.org> 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 <em...@topicus.nl>
> > >> Authored: Mon Aug 19 11:07:41 2013 +0200
> > >> Committer: Emond Papegaaij <em...@topicus.nl>
> > >> 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.
> > >> */
> > >>
> > >>
> >
> >
>
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Bernard <bh...@gmail.com>.
Hi Emond and Martijn,
Thanks for your great work. This is really tricky time-consuming
stuff, and I am glad that you follow this up all the way. I am not a
Wicket framework specialist and don't have as much inside knowledge as
I would like, so please excuse my ignorance in some cases.
On Wed, 21 Aug 2013 21:10:37 +0200, you wrote:
>Martijn and I discussed some more about this, and we've came to the
>conclusion that we probably should not try to fix this in Wicket 6. The
>cause of this issue (and several others) is a mismatch between Page,
>PageParameters and statefulness that cannot be fixed without breaking the
>API in several ways.
I agree. This needs time and refinement, and while you are at it,
perhaps some other issues should be addressed with it to get a better
return for the changes.
>The reason Component.urlFor checks isPageStateless is
>that a stateful page cannot be recreated from a bookmarkable url. If you
>do, you will loose (some of) its state.
But this is not a problem. From my limited perspective, I was only
trying to address the survivability of a page after expiry. So we can
only gain never lose. I am fully aware that some if not all state can
be lost if the page is re-created using a link URL that is created
with a different urlFor(). That is until we start discussing client
state.
Often all is required is that the page is created without state, and
obviously a bookmarkable constructor must exist because of the
isBookmarkable() switch. This bookmarkable constructor should
guarantee that a page is valid otherwise what is the point of coding
it and what is the point of defining what bookmarkable is.
>However, looking at it again, I'm
>starting to think we should at least to try improve the situation for
>Wicket 6.
Great. I wasn't really expecting that.
>
>At the moment, there is a discrepancy between mounted and bookmarkable
>pages. Consider MyPage, a bookmarkable but stateful page with a Link. The
>href for this link will be something like
>/wicket/page?2-1.ILinkListener-link. Now, if I mount this page under
>/mypage, the href for the link will suddenly become
>/mypage?2-1.ILinkListener-link. I don't see why a mounted stateful page can
>and should be treated differently than a bookmarkable stateful page. IMHO,
>they are 2 forms of the same pattern. Therefore, I've just pushed an update
>to the wicket-4997 that tries to improve the situation by using
>bookmarkable urls for stateless pages (as it was), but also for
>bookmarkable pages that were created bookmarkable. This seems to give much
>better results in the testcases. I would really appreciate it if some more
>people could take a look at this, because url rendering is quite a
>sensitive subject.
This is also my understanding. This is a sound concept, and it should
be one of our guidelines.
>
>For Wicket 7, we might want to take a look at the PageParameters and mounts
>because they hold several caveats. The most important is that the lifecycle
>of PageParameters if different than that of the Page they belong to.
>PageParameters are bound to a request, whereas a page instance can live for
>many requests. Also, it is possible to modify the PageParameters passed to
>the super contructor, but those changes will be overridden by Wicket after
>constructing the page. Finally, you can override getPageParameters in your
>page, which will confuse Wicket even more. Also, PageParameters only handle
>strings, which is quite limited. I don't know how to improve this. Perhaps
>we can borrow some ideas from JAX-RS? On the other hand, they don't have to
>deal with annoying things like state :)
Obviously, this is very tricky, and to understand this, I simplify
this a little to not get confused. I am considering PageParameters
client state that gets passed between pages via links and requests.
Although they might not be strictly needed if the page becomes
stateful. But we need them so that people can bookmark the page in the
browser, and for recovery. So if there is for example
?orderId=123&lineItemId=5 then I would expect that these parameters
never cease to exist.
Your comments about the complexity remind me of WICKET-4441:
"PageProvider should create a new Page instance if PageParameters are
changed, even if a stored page exists."
So what you are raising is the fact that client state competes with
server state. We would need a policy for that. That policy would
perhaps help to create a bridge to some viable client state.
Who is the genious to solve this? I don't have any ideas yet. I just
hoped that we get incremental improvements by means of clarity and
consistency. Therefore I thought that WICKET-5068 could be the next
step.
Kind Regards,
Bernard
>
>Best regards,
>Emond
>
>
>
>On Wed, Aug 21, 2013 at 8:54 AM, Bernard <bh...@gmail.com> wrote:
>
>> 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, <pa...@apache.org> 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 <em...@topicus.nl>
>> >> Authored: Mon Aug 19 11:07:41 2013 +0200
>> >> Committer: Emond Papegaaij <em...@topicus.nl>
>> >> 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.
>> >> */
>> >>
>> >>
>>
>>
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Emond Papegaaij <em...@gmail.com>.
Martijn and I discussed some more about this, and we've came to the
conclusion that we probably should not try to fix this in Wicket 6. The
cause of this issue (and several others) is a mismatch between Page,
PageParameters and statefulness that cannot be fixed without breaking the
API in several ways. The reason Component.urlFor checks isPageStateless is
that a stateful page cannot be recreated from a bookmarkable url. If you
do, you will loose (some of) its state. However, looking at it again, I'm
starting to think we should at least to try improve the situation for
Wicket 6.
At the moment, there is a discrepancy between mounted and bookmarkable
pages. Consider MyPage, a bookmarkable but stateful page with a Link. The
href for this link will be something like
/wicket/page?2-1.ILinkListener-link. Now, if I mount this page under
/mypage, the href for the link will suddenly become
/mypage?2-1.ILinkListener-link. I don't see why a mounted stateful page can
and should be treated differently than a bookmarkable stateful page. IMHO,
they are 2 forms of the same pattern. Therefore, I've just pushed an update
to the wicket-4997 that tries to improve the situation by using
bookmarkable urls for stateless pages (as it was), but also for
bookmarkable pages that were created bookmarkable. This seems to give much
better results in the testcases. I would really appreciate it if some more
people could take a look at this, because url rendering is quite a
sensitive subject.
For Wicket 7, we might want to take a look at the PageParameters and mounts
because they hold several caveats. The most important is that the lifecycle
of PageParameters if different than that of the Page they belong to.
PageParameters are bound to a request, whereas a page instance can live for
many requests. Also, it is possible to modify the PageParameters passed to
the super contructor, but those changes will be overridden by Wicket after
constructing the page. Finally, you can override getPageParameters in your
page, which will confuse Wicket even more. Also, PageParameters only handle
strings, which is quite limited. I don't know how to improve this. Perhaps
we can borrow some ideas from JAX-RS? On the other hand, they don't have to
deal with annoying things like state :)
Best regards,
Emond
On Wed, Aug 21, 2013 at 8:54 AM, Bernard <bh...@gmail.com> wrote:
> 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, <pa...@apache.org> 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 <em...@topicus.nl>
> >> Authored: Mon Aug 19 11:07:41 2013 +0200
> >> Committer: Emond Papegaaij <em...@topicus.nl>
> >> 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.
> >> */
> >>
> >>
>
>
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Bernard <bh...@gmail.com>.
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, <pa...@apache.org> 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 <em...@topicus.nl>
>> Authored: Mon Aug 19 11:07:41 2013 +0200
>> Committer: Emond Papegaaij <em...@topicus.nl>
>> 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.
>> */
>>
>>
Re: [4/4] git commit: WICKET-4997: render bookmarkable urls for
bookmarkable pages (not stateless)
Posted by Martin Grigorov <mg...@apache.org>.
Hi Emond,
On Mon, Aug 19, 2013 at 12:10 PM, <pa...@apache.org> 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 <em...@topicus.nl>
> Authored: Mon Aug 19 11:07:41 2013 +0200
> Committer: Emond Papegaaij <em...@topicus.nl>
> 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.
> */
>
>