You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Jeremy Thomerson <je...@wickettraining.com> on 2010/12/18 03:41:24 UTC

Re: svn commit: r1050433 - in /wicket/trunk/wicket/src: main/java/org/apache/wicket/request/handler/render/ main/java/org/apache/wicket/util/tester/ test/java/org/apache/wicket/request/handler/

A couple questions about this commit (inline):

On Fri, Dec 17, 2010 at 10:01 AM, <mg...@apache.org> wrote:

> Author: mgrigorov
> Date: Fri Dec 17 16:01:41 2010
> New Revision: 1050433
>
> URL: http://svn.apache.org/viewvc?rev=1050433&view=rev
> Log:
> WICKET-3252 StalePageException on non-versioned Page in Ajax request does
> not render ajax-response
>
> If a hit to stale page is made and the current request is an Ajax one then
> always redirect to the new page. It doesn't make sense to return the new
> page's markup in Ajax response
>

<snip>


> +       private boolean isAjax(RequestCycle requestCycle)
> +       {
> +               boolean isAjax = false;
> +
> +               Request request = requestCycle.getRequest();
> +               if (request instanceof WebRequest)
> +               {
> +                       WebRequest webRequest = (WebRequest)request;
> +                       isAjax = webRequest.isAjax();
> +               }
> +
> +               return isAjax;
> +       }
>

Is this new method needed?  I haven't looked at exactly where this renderer
is getting called, but can't we say the following?
boolean isAjax = AjaxRequestTarget.get() != null;

That's what we're doing elsewhere in the code.

+       public String getWicketAjaxBaserUrlFromLastRequest() throws
> IOException,
> +               ResourceStreamNotFoundException, ParseException
>

Typo here - "Baser" should be "Base".

Great work on writing a new test case for your change!

-- 
Jeremy Thomerson
http://wickettraining.com
*Need a CMS for Wicket?  Use Brix! http://brixcms.org*

Re: svn commit: r1050433 - in /wicket/trunk/wicket/src: main/java/org/apache/wicket/request/handler/render/ main/java/org/apache/wicket/util/tester/ test/java/org/apache/wicket/request/handler/

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

On Sat, Dec 18, 2010 at 3:41 AM, Jeremy Thomerson <jeremy@wickettraining.com
> wrote:

> A couple questions about this commit (inline):
>
> On Fri, Dec 17, 2010 at 10:01 AM, <mg...@apache.org> wrote:
>
> > Author: mgrigorov
> > Date: Fri Dec 17 16:01:41 2010
> > New Revision: 1050433
> >
> > URL: http://svn.apache.org/viewvc?rev=1050433&view=rev
> > Log:
> > WICKET-3252 StalePageException on non-versioned Page in Ajax request does
> > not render ajax-response
> >
> > If a hit to stale page is made and the current request is an Ajax one
> then
> > always redirect to the new page. It doesn't make sense to return the new
> > page's markup in Ajax response
> >
>
> <snip>
>
>
> > +       private boolean isAjax(RequestCycle requestCycle)
> > +       {
> > +               boolean isAjax = false;
> > +
> > +               Request request = requestCycle.getRequest();
> > +               if (request instanceof WebRequest)
> > +               {
> > +                       WebRequest webRequest = (WebRequest)request;
> > +                       isAjax = webRequest.isAjax();
> > +               }
> > +
> > +               return isAjax;
> > +       }
> >
>
> Is this new method needed?  I haven't looked at exactly where this renderer
> is getting called, but can't we say the following?
> boolean isAjax = AjaxRequestTarget.get() != null;
>
> That's what we're doing elsewhere in the code.
>
> Yes, it is needed. In this particular case the request is to a stale page,
so the Ajax request handler (target) is transformed to
RenderPageRequestHandler with a new
page instance. That's why we need to check on a lower level.

Actually this code for checking whether the current request is Ajax is used
in few more places. Maybe we have to move it in some util class, e.g.
RequestUtil.


> +       public String getWicketAjaxBaserUrlFromLastRequest() throws
> > IOException,
> > +               ResourceStreamNotFoundException, ParseException
> >
>
> Typo here - "Baser" should be "Base".
>
Fixed! Thanks!

>
> Great work on writing a new test case for your change!
>
Actually the test is provided by Pedro. Thanks, Pedro!

>
> --
> Jeremy Thomerson
> http://wickettraining.com
> *Need a CMS for Wicket?  Use Brix! http://brixcms.org*
>