You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Pedro Santos <pe...@gmail.com> on 2013/01/06 23:05:48 UTC

Need more context on the recreateMountedPagesAfterExpiry flag

Hi,

we have the recreateMountedPagesAfterExpiry flag at page settings to tell
mounted mapper to encode enough info at callback urls so the mounted page
can be recreated after expired and the callback be invoked.

Why this flag works only for mounted pages? It looks we are segmenting too
much a functionality. I think that a more consistent way of to honor such
flag is by improving AbstractBookmarkableMapper itself and not only one of
its extension (MountedMapper).

Let me know what you think so we can improve bookmarkable mapper.

Cheers,

Pedro Santos

Re: Need more context on the recreateMountedPagesAfterExpiry flag

Posted by Pedro Santos <pe...@gmail.com>.
Thx Martin, I cleaned up the code.

I found one more issue, our bookmarkable mapper isn't testing if the page
is bookmarkable or not at AbstractBookmarkableMapper#checkPageClass, which
can make the mapper to create URLs that would be mapped to handlers that
couldn't be resolved; already fixed it on the branch [1].

Also checked in some test expectations so you can have an idea of what will
change [2]. There are more 50 exactly same expectation to be updated still
:) Lets first make sure we are in the right path.

1 -
https://github.com/apache/wicket/commit/1ec0b8b3637feb19da4f87b485e96bfc7ce4e992
2 -
https://github.com/apache/wicket/commit/fba8bddd98da943ea74c9c04d90f17c75417c2fe

Cheers,

Pedro Santos


On Wed, Jan 9, 2013 at 6:35 PM, Martin Grigorov <mg...@apache.org>wrote:

> I've commented in the commit thread.
>
>
> On Wed, Jan 9, 2013 at 9:53 PM, Pedro Santos <pe...@gmail.com> wrote:
>
> > Nice, I moved the logic to AbstractBookmarkableMapper in this branch:
> > sandbox/bookmarkable-callback-url
> >
> > Let me know if the changes are ok before I start to update our test cases
> > expectations to assert the new encoded URL.
> >
>
> Please do these additional changes in the same branch so we can see what
> expectations have changed.
>
>
> >
> > Cheers,
> >
> >
> > Pedro Santos
> >
> >
> > On Sun, Jan 6, 2013 at 8:34 PM, Martin Grigorov <mgrigorov@apache.org
> > >wrote:
> >
> > > Hi Pedro,
> > >
> > > I agree with you.
> > > https://issues.apache.org/jira/browse/WICKET-4686 is similar - the
> > support
> > > for path parameters should be moved the same way too.
> > >
> > >
> > > On Mon, Jan 7, 2013 at 12:05 AM, Pedro Santos <pe...@gmail.com>
> > wrote:
> > >
> > > > Hi,
> > > >
> > > > we have the recreateMountedPagesAfterExpiry flag at page settings to
> > tell
> > > > mounted mapper to encode enough info at callback urls so the mounted
> > page
> > > > can be recreated after expired and the callback be invoked.
> > > >
> > > > Why this flag works only for mounted pages? It looks we are
> segmenting
> > > too
> > > > much a functionality. I think that a more consistent way of to honor
> > such
> > > > flag is by improving AbstractBookmarkableMapper itself and not only
> one
> > > of
> > > > its extension (MountedMapper).
> > > >
> > > > Let me know what you think so we can improve bookmarkable mapper.
> > > >
> > > > Cheers,
> > > >
> > > > Pedro Santos
> > > >
> > >
> > >
> > >
> > > --
> > > Martin Grigorov
> > > jWeekend
> > > Training, Consulting, Development
> > > http://jWeekend.com <http://jweekend.com/>
> > >
> >
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>
>

Re: Need more context on the recreateMountedPagesAfterExpiry flag

Posted by Martin Grigorov <mg...@apache.org>.
I've commented in the commit thread.


On Wed, Jan 9, 2013 at 9:53 PM, Pedro Santos <pe...@gmail.com> wrote:

> Nice, I moved the logic to AbstractBookmarkableMapper in this branch:
> sandbox/bookmarkable-callback-url
>
> Let me know if the changes are ok before I start to update our test cases
> expectations to assert the new encoded URL.
>

Please do these additional changes in the same branch so we can see what
expectations have changed.


>
> Cheers,
>
>
> Pedro Santos
>
>
> On Sun, Jan 6, 2013 at 8:34 PM, Martin Grigorov <mgrigorov@apache.org
> >wrote:
>
> > Hi Pedro,
> >
> > I agree with you.
> > https://issues.apache.org/jira/browse/WICKET-4686 is similar - the
> support
> > for path parameters should be moved the same way too.
> >
> >
> > On Mon, Jan 7, 2013 at 12:05 AM, Pedro Santos <pe...@gmail.com>
> wrote:
> >
> > > Hi,
> > >
> > > we have the recreateMountedPagesAfterExpiry flag at page settings to
> tell
> > > mounted mapper to encode enough info at callback urls so the mounted
> page
> > > can be recreated after expired and the callback be invoked.
> > >
> > > Why this flag works only for mounted pages? It looks we are segmenting
> > too
> > > much a functionality. I think that a more consistent way of to honor
> such
> > > flag is by improving AbstractBookmarkableMapper itself and not only one
> > of
> > > its extension (MountedMapper).
> > >
> > > Let me know what you think so we can improve bookmarkable mapper.
> > >
> > > Cheers,
> > >
> > > Pedro Santos
> > >
> >
> >
> >
> > --
> > Martin Grigorov
> > jWeekend
> > Training, Consulting, Development
> > http://jWeekend.com <http://jweekend.com/>
> >
>



-- 
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>

Re: Need more context on the recreateMountedPagesAfterExpiry flag

Posted by Pedro Santos <pe...@gmail.com>.
Nice, I moved the logic to AbstractBookmarkableMapper in this branch:
sandbox/bookmarkable-callback-url

Let me know if the changes are ok before I start to update our test cases
expectations to assert the new encoded URL.

Cheers,


Pedro Santos


On Sun, Jan 6, 2013 at 8:34 PM, Martin Grigorov <mg...@apache.org>wrote:

> Hi Pedro,
>
> I agree with you.
> https://issues.apache.org/jira/browse/WICKET-4686 is similar - the support
> for path parameters should be moved the same way too.
>
>
> On Mon, Jan 7, 2013 at 12:05 AM, Pedro Santos <pe...@gmail.com> wrote:
>
> > Hi,
> >
> > we have the recreateMountedPagesAfterExpiry flag at page settings to tell
> > mounted mapper to encode enough info at callback urls so the mounted page
> > can be recreated after expired and the callback be invoked.
> >
> > Why this flag works only for mounted pages? It looks we are segmenting
> too
> > much a functionality. I think that a more consistent way of to honor such
> > flag is by improving AbstractBookmarkableMapper itself and not only one
> of
> > its extension (MountedMapper).
> >
> > Let me know what you think so we can improve bookmarkable mapper.
> >
> > Cheers,
> >
> > Pedro Santos
> >
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>
>

Re: Need more context on the recreateMountedPagesAfterExpiry flag

Posted by Martin Grigorov <mg...@apache.org>.
Hi Pedro,

I agree with you.
https://issues.apache.org/jira/browse/WICKET-4686 is similar - the support
for path parameters should be moved the same way too.


On Mon, Jan 7, 2013 at 12:05 AM, Pedro Santos <pe...@gmail.com> wrote:

> Hi,
>
> we have the recreateMountedPagesAfterExpiry flag at page settings to tell
> mounted mapper to encode enough info at callback urls so the mounted page
> can be recreated after expired and the callback be invoked.
>
> Why this flag works only for mounted pages? It looks we are segmenting too
> much a functionality. I think that a more consistent way of to honor such
> flag is by improving AbstractBookmarkableMapper itself and not only one of
> its extension (MountedMapper).
>
> Let me know what you think so we can improve bookmarkable mapper.
>
> Cheers,
>
> Pedro Santos
>



-- 
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>