You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2014/01/09 14:12:37 UTC
Null-ify PageParameters for stateful pages in AbstractBookmarkableMapper
Hi Sven,
What was the reason to move the null-yfing of the parsed PageParameters
from PageProvider to AbstractBookmarkableMapper in
https://git-wip-us.apache.org/repos/asf/wicket/repo?p=wicket.git;a=commitdiff;h=b3982a4beff352cd5e61521490a397a926c13eed
and
https://git-wip-us.apache.org/repos/asf/wicket/repo?p=wicket.git;a=commitdiff;h=5e1bf8d8169a8d01f041a1d2bf41a8b8fe170dbd
I am working on https://issues.apache.org/jira/browse/WICKET-4686 and I had
an issue
with org.apache.wicket.core.request.mapper.PackageMapperTest#decodeNamedParameters
because of this change. So I reverted it in branch sandbox/WICKET-4686.
My reasoning to null-ify in PageProvider is: if there is parsed pageId and
the page cannot loaded from the stores then it is expired, so the page
parameters are ignored and new page instance is created.
How do you decide to drop the parameters in the mapper ?
Other related tickets that I'd like to address for Wicket 7:
https://issues.apache.org/jira/browse/WICKET-5068
https://issues.apache.org/jira/browse/WICKET-5070
Martin Grigorov
Wicket Training and Consulting
Re: Null-ify PageParameters for stateful pages in AbstractBookmarkableMapper
Posted by Martin Grigorov <mg...@apache.org>.
Digging into the related code I see that if my new test has no listener
interface then everything is OK.
So I reverted the revert and it is as in 6.x again.
For the next tickets I may need to move it again.
Martin Grigorov
Wicket Training and Consulting
On Fri, Jan 10, 2014 at 12:07 PM, Martin Grigorov <mg...@apache.org>wrote:
> Hi,
>
>
> On Thu, Jan 9, 2014 at 3:38 PM, Sven Meier <sv...@meiers.net> wrote:
>
>> Hi Martin,
>>
>> it seemed reasonable to me to let the mapper decide to drop the
>> parameters. It already has all information at hand, why delay the decision
>> and put the burden on the PageProvider?
>>
>>
>> >My reasoning to null-ify in PageProvider is: if there is parsed pageId
>> and the page cannot loaded from the
>> >stores then it is expired, so the page parameters are ignored and new
>> page instance is created. How do
>> >you decide to drop the parameters in the mapper ?
>>
>> If there's a pageId, the request parameters are uninteresting for the
>> page provider, aren't they?
>>
>
> Maybe not: https://issues.apache.org/jira/browse/WICKET-4441
> I need to debug some more to see what params come with the following
> requests (request caused by click on a link for example).
>
> PackageMapperTest#decodeNamedParameters() is a new test introduced for
> WICKET-4686 and it also needs the parameters to be able to read the named
> parameters from the url.
>
>
>>
>> https://git-wip-us.apache.org/repos/asf/wicket/repo?p=
>> wicket.git;a=commitdiff;h=5e1bf8d8169a8d01f041a1d2bf41a8b8fe170dbd
>>
>> But feel free to revert this, if null-ifying the parameters in
>> PageProvider helps you in solving the puzzle of these settings. WICKET-5068
>> is really important IMHO.
>
>
> For now I'll revert it and keep an eye on it when dealing with the other
> tickets in this area.
>
> Thanks!
>
>
>>
>>
>> Sven
>>
>>
>> On 01/09/2014 02:12 PM, Martin Grigorov wrote:
>>
>>> Hi Sven,
>>>
>>> What was the reason to move the null-yfing of the parsed PageParameters
>>> from PageProvider to AbstractBookmarkableMapper in
>>> https://git-wip-us.apache.org/repos/asf/wicket/repo?p=
>>> wicket.git;a=commitdiff;h=b3982a4beff352cd5e61521490a397a926c13eed
>>>
>>> and
>>> https://git-wip-us.apache.org/repos/asf/wicket/repo?p=
>>> wicket.git;a=commitdiff;h=5e1bf8d8169a8d01f041a1d2bf41a8b8fe170dbd
>>>
>>> I am working on https://issues.apache.org/jira/browse/WICKET-4686 and I
>>> had
>>> an issue
>>> with org.apache.wicket.core.request.mapper.PackageMapperTest#
>>> decodeNamedParameters
>>> because of this change. So I reverted it in branch sandbox/WICKET-4686.
>>>
>>> My reasoning to null-ify in PageProvider is: if there is parsed pageId
>>> and
>>> the page cannot loaded from the stores then it is expired, so the page
>>> parameters are ignored and new page instance is created.
>>> How do you decide to drop the parameters in the mapper ?
>>>
>>> Other related tickets that I'd like to address for Wicket 7:
>>> https://issues.apache.org/jira/browse/WICKET-5068
>>> https://issues.apache.org/jira/browse/WICKET-5070
>>>
>>> Martin Grigorov
>>> Wicket Training and Consulting
>>>
>>>
>>
>
Re: Null-ify PageParameters for stateful pages in AbstractBookmarkableMapper
Posted by Martin Grigorov <mg...@apache.org>.
Hi,
On Thu, Jan 9, 2014 at 3:38 PM, Sven Meier <sv...@meiers.net> wrote:
> Hi Martin,
>
> it seemed reasonable to me to let the mapper decide to drop the
> parameters. It already has all information at hand, why delay the decision
> and put the burden on the PageProvider?
>
>
> >My reasoning to null-ify in PageProvider is: if there is parsed pageId
> and the page cannot loaded from the
> >stores then it is expired, so the page parameters are ignored and new
> page instance is created. How do
> >you decide to drop the parameters in the mapper ?
>
> If there's a pageId, the request parameters are uninteresting for the page
> provider, aren't they?
>
Maybe not: https://issues.apache.org/jira/browse/WICKET-4441
I need to debug some more to see what params come with the following
requests (request caused by click on a link for example).
PackageMapperTest#decodeNamedParameters() is a new test introduced for
WICKET-4686 and it also needs the parameters to be able to read the named
parameters from the url.
>
> https://git-wip-us.apache.org/repos/asf/wicket/repo?p=
> wicket.git;a=commitdiff;h=5e1bf8d8169a8d01f041a1d2bf41a8b8fe170dbd
>
> But feel free to revert this, if null-ifying the parameters in
> PageProvider helps you in solving the puzzle of these settings. WICKET-5068
> is really important IMHO.
For now I'll revert it and keep an eye on it when dealing with the other
tickets in this area.
Thanks!
>
>
> Sven
>
>
> On 01/09/2014 02:12 PM, Martin Grigorov wrote:
>
>> Hi Sven,
>>
>> What was the reason to move the null-yfing of the parsed PageParameters
>> from PageProvider to AbstractBookmarkableMapper in
>> https://git-wip-us.apache.org/repos/asf/wicket/repo?p=
>> wicket.git;a=commitdiff;h=b3982a4beff352cd5e61521490a397a926c13eed
>>
>> and
>> https://git-wip-us.apache.org/repos/asf/wicket/repo?p=
>> wicket.git;a=commitdiff;h=5e1bf8d8169a8d01f041a1d2bf41a8b8fe170dbd
>>
>> I am working on https://issues.apache.org/jira/browse/WICKET-4686 and I
>> had
>> an issue
>> with org.apache.wicket.core.request.mapper.PackageMapperTest#
>> decodeNamedParameters
>> because of this change. So I reverted it in branch sandbox/WICKET-4686.
>>
>> My reasoning to null-ify in PageProvider is: if there is parsed pageId and
>> the page cannot loaded from the stores then it is expired, so the page
>> parameters are ignored and new page instance is created.
>> How do you decide to drop the parameters in the mapper ?
>>
>> Other related tickets that I'd like to address for Wicket 7:
>> https://issues.apache.org/jira/browse/WICKET-5068
>> https://issues.apache.org/jira/browse/WICKET-5070
>>
>> Martin Grigorov
>> Wicket Training and Consulting
>>
>>
>
Re: Null-ify PageParameters for stateful pages in AbstractBookmarkableMapper
Posted by Sven Meier <sv...@meiers.net>.
Hi Martin,
it seemed reasonable to me to let the mapper decide to drop the
parameters. It already has all information at hand, why delay the
decision and put the burden on the PageProvider?
>My reasoning to null-ify in PageProvider is: if there is parsed pageId
and the page cannot loaded from the
>stores then it is expired, so the page parameters are ignored and new
page instance is created. How do
>you decide to drop the parameters in the mapper ?
If there's a pageId, the request parameters are uninteresting for the
page provider, aren't they?
https://git-wip-us.apache.org/repos/asf/wicket/repo?p=wicket.git;a=commitdiff;h=5e1bf8d8169a8d01f041a1d2bf41a8b8fe170dbd
But feel free to revert this, if null-ifying the parameters in
PageProvider helps you in solving the puzzle of these settings.
WICKET-5068 is really important IMHO.
Sven
On 01/09/2014 02:12 PM, Martin Grigorov wrote:
> Hi Sven,
>
> What was the reason to move the null-yfing of the parsed PageParameters
> from PageProvider to AbstractBookmarkableMapper in
> https://git-wip-us.apache.org/repos/asf/wicket/repo?p=wicket.git;a=commitdiff;h=b3982a4beff352cd5e61521490a397a926c13eed
>
> and
> https://git-wip-us.apache.org/repos/asf/wicket/repo?p=wicket.git;a=commitdiff;h=5e1bf8d8169a8d01f041a1d2bf41a8b8fe170dbd
>
> I am working on https://issues.apache.org/jira/browse/WICKET-4686 and I had
> an issue
> with org.apache.wicket.core.request.mapper.PackageMapperTest#decodeNamedParameters
> because of this change. So I reverted it in branch sandbox/WICKET-4686.
>
> My reasoning to null-ify in PageProvider is: if there is parsed pageId and
> the page cannot loaded from the stores then it is expired, so the page
> parameters are ignored and new page instance is created.
> How do you decide to drop the parameters in the mapper ?
>
> Other related tickets that I'd like to address for Wicket 7:
> https://issues.apache.org/jira/browse/WICKET-5068
> https://issues.apache.org/jira/browse/WICKET-5070
>
> Martin Grigorov
> Wicket Training and Consulting
>