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/07/17 13:33:55 UTC

Re: git commit: Code beautifying: method WebPageRenderer.respond

Hi Andrea,

On Wed, Jul 16, 2014 at 8:33 PM, <ad...@apache.org> wrote:

> Repository: wicket
> Updated Branches:
>   refs/heads/master c24d830cd -> bb9c1044e
>
>
> Code beautifying: method WebPageRenderer.respond
>

What exactly "beautifying" means ?
It is a bit hard to see what is the actual change. It doesn't look like
applying Wicket's code format.


>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/bb9c1044
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/bb9c1044
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/bb9c1044
>
> Branch: refs/heads/master
> Commit: bb9c1044e5f1ba8902aa69073ef9fb236802d277
> Parents: c24d830
> Author: andrea del bene <ad...@apache.org>
> Authored: Wed Jul 16 19:33:16 2014 +0200
> Committer: andrea del bene <ad...@apache.org>
> Committed: Wed Jul 16 19:33:16 2014 +0200
>
> ----------------------------------------------------------------------
>  .../request/handler/render/WebPageRenderer.java | 169 +++++++++----------
>  1 file changed, 82 insertions(+), 87 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/bb9c1044/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
> index 0b5dee4..af9dbee 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
> @@ -198,99 +198,94 @@ public class WebPageRenderer extends PageRenderer
>                         // if there is saved response for this URL render
> it
>
> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>                 }
> +               else if (shouldRenderPageAndWriteResponse(requestCycle,
> currentUrl, targetUrl))
> +               {
> +                       BufferedWebResponse response =
> renderPage(currentUrl, requestCycle);
> +                       if (response != null)
> +                       {
> +
> response.writeTo((WebResponse)requestCycle.getResponse());
> +                       }
> +               }
> +               else if (shouldRedirectToTargetUrl(requestCycle,
> currentUrl, targetUrl))
> +               {
> +
> +                       redirectTo(targetUrl, requestCycle);
> +
> +                       // note: if we had session here we would render
> the page to buffer and then
> +                       // redirect to URL generated *after* page has been
> rendered (the statelessness
> +                       // may change during render). this would save one
> redirect because now we have
> +                       // to render to URL generated *before* page is
> rendered, render the page, get
> +                       // URL after render and if the URL is different
> (meaning page is not stateless),
> +                       // save the buffer and redirect again (which is
> pretty much what the next step
> +                       // does)
> +               }
>                 else
>                 {
> -                       if (shouldRenderPageAndWriteResponse(requestCycle,
> currentUrl, targetUrl)) //
> +                       if (isRedirectToBuffer() == false &&
> logger.isDebugEnabled())
> +                       {
> +                               String details = String
> +                                       .format(
> +                                               "redirect strategy: '%s',
> isAjax: '%s', redirect policy: '%s', "
> +                                                       + "current url:
> '%s', target url: '%s', is new: '%s', is stateless: '%s', is temporary:
> '%s'",
> +
> Application.get().getRequestCycleSettings().getRenderStrategy(),
> +                                               isAjax(requestCycle),
> getRedirectPolicy(), currentUrl, targetUrl,
> +                                               isNewPageInstance(),
> isPageStateless(), isSessionTemporary());
> +                               logger
> +                                       .debug("Falling back to
> Redirect_To_Buffer render strategy because none of the conditions "
> +                                               + "matched. Details: " +
> details);
> +                       }
> +
> +                       // force creation of possible stateful page to get
> the final target url
> +                       getPage();
> +
> +                       Url beforeRenderUrl =
> requestCycle.mapUrlFor(getRenderPageRequestHandler());
> +
> +                       // redirect to buffer
> +                       BufferedWebResponse response =
> renderPage(beforeRenderUrl, requestCycle);
> +
> +                       if (response == null)
> +                       {
> +                               return;
> +                       }
> +
> +                       // the url might have changed after page has been
> rendered (e.g. the
> +                       // stateless flag might have changed because
> stateful components
> +                       // were added)
> +                       final Url afterRenderUrl = requestCycle
> +                               .mapUrlFor(getRenderPageRequestHandler());
> +
> +                       if
> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
> false)
>                         {
> -                               BufferedWebResponse response =
> renderPage(currentUrl, requestCycle);
> -                               if (response != null)
> -                               {
> -
> response.writeTo((WebResponse)requestCycle.getResponse());
> -                               }
> +                               // the amount of segments is different -
> generated relative URLs
> +                               // will not work, we need to rerender the
> page. This can happen
> +                               // with IRequestHandlers that produce
> different URLs with
> +                               // different amount of segments for
> stateless and stateful pages
> +                               response = renderPage(afterRenderUrl,
> requestCycle);
> +                       }
> +
> +                       if (currentUrl.equals(afterRenderUrl))
> +                       {
> +                               // no need to redirect when both urls are
> exactly the same
> +
> response.writeTo((WebResponse)requestCycle.getResponse());
> +                       }
> +                       // if page is still stateless after render
> +                       else if (isPageStateless() &&
> !enableRedirectForStatelessPage())
> +                       {
> +                               // we don't want the redirect to happen
> for stateless page
> +                               // example:
> +                               // when a normal mounted stateful page is
> hit at /mount/point
> +                               // wicket renders the page to buffer and
> redirects to /mount/point?12
> +                               // but for stateless page the redirect is
> not necessary
> +                               // also for listener interface on stateful
> page we want to redirect
> +                               // after the listener is invoked, but on
> stateless page the user
> +                               // must ask for redirect explicitly
> +
> response.writeTo((WebResponse)requestCycle.getResponse());
>                         }
>                         else
>                         {
> -                               if
> (shouldRedirectToTargetUrl(requestCycle, currentUrl, targetUrl))
> -                               {
> -                                       redirectTo(targetUrl,
> requestCycle);
> -
> -                                       // note: if we had session here we
> would render the page to buffer and then
> -                                       // redirect to URL generated
> *after* page has been rendered (the statelessness
> -                                       // may change during render). this
> would save one redirect because now we have
> -                                       // to render to URL generated
> *before* page is rendered, render the page, get
> -                                       // URL after render and if the URL
> is different (meaning page is not stateless),
> -                                       // save the buffer and redirect
> again (which is pretty much what the next step
> -                                       // does)
> -                               }
> -                               else
> -                               {
> -                                       if (isRedirectToBuffer() == false
> && logger.isDebugEnabled())
> -                                       {
> -                                               String details = String
> -                                                       .format(
> -                                                               "redirect
> strategy: '%s', isAjax: '%s', redirect policy: '%s', "
> -                                                                       +
> "current url: '%s', target url: '%s', is new: '%s', is stateless: '%s', is
> temporary: '%s'",
> -
> Application.get().getRequestCycleSettings().getRenderStrategy(),
> -
> isAjax(requestCycle), getRedirectPolicy(), currentUrl, targetUrl,
> -
> isNewPageInstance(), isPageStateless(), isSessionTemporary());
> -                                               logger
> -                                                       .debug("Falling
> back to Redirect_To_Buffer render strategy because none of the conditions "
> -                                                               +
> "matched. Details: " + details);
> -                                       }
> -
> -                                       // force creation of possible
> stateful page to get the final target url
> -                                       getPage();
> -
> -                                       Url beforeRenderUrl =
> requestCycle.mapUrlFor(getRenderPageRequestHandler());
> -
> -                                       // redirect to buffer
> -                                       BufferedWebResponse response =
> renderPage(beforeRenderUrl, requestCycle);
> -
> -                                       if (response == null)
> -                                       {
> -                                               return;
> -                                       }
> -
> -                                       // the url might have changed
> after page has been rendered (e.g. the
> -                                       // stateless flag might have
> changed because stateful components
> -                                       // were added)
> -                                       final Url afterRenderUrl =
> requestCycle
> -
> .mapUrlFor(getRenderPageRequestHandler());
> -
> -                                       if
> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
> false)
> -                                       {
> -                                               // the amount of segments
> is different - generated relative URLs
> -                                               // will not work, we need
> to rerender the page. This can happen
> -                                               // with IRequestHandlers
> that produce different URLs with
> -                                               // different amount of
> segments for stateless and stateful pages
> -                                               response =
> renderPage(afterRenderUrl, requestCycle);
> -                                       }
> -
> -                                       if
> (currentUrl.equals(afterRenderUrl))
> -                                       {
> -                                               // no need to redirect
> when both urls are exactly the same
> -
> response.writeTo((WebResponse)requestCycle.getResponse());
> -                                       }
> -                                       // if page is still stateless
> after render
> -                                       else if (isPageStateless() &&
> !enableRedirectForStatelessPage())
> -                                       {
> -                                               // we don't want the
> redirect to happen for stateless page
> -                                               // example:
> -                                               // when a normal mounted
> stateful page is hit at /mount/point
> -                                               // wicket renders the page
> to buffer and redirects to /mount/point?12
> -                                               // but for stateless page
> the redirect is not necessary
> -                                               // also for listener
> interface on stateful page we want to redirect
> -                                               // after the listener is
> invoked, but on stateless page the user
> -                                               // must ask for redirect
> explicitly
> -
> response.writeTo((WebResponse)requestCycle.getResponse());
> -                                       }
> -                                       else
> -                                       {
> -
> storeBufferedResponse(afterRenderUrl, response);
> -
> -                                               redirectTo(afterRenderUrl,
> requestCycle);
> -                                       }
> -                               }
> +                               storeBufferedResponse(afterRenderUrl,
> response);
> +
> +                               redirectTo(afterRenderUrl, requestCycle);
>                         }
>                 }
>         }
>
>

Re: git commit: Code beautifying: method WebPageRenderer.respond

Posted by Andrea Del Bene <an...@gmail.com>.
You spotted me :)! I could also use pmd rules to make it clear from the 
start:

http://pmd.sourceforge.net/pmd-4.3/rules/design.html#AvoidDeeplyNestedIfStmts
> You just removed two pairs of curly braces (to turn 'else { if {} }'
> into 'else if {}') and re-indented the code?
> “The truth is rarely pure and never simple.” ― Oscar Wilde
>
>
> On Thu, Jul 17, 2014 at 11:53 AM, Andrea Del Bene <an...@gmail.com> wrote:
>> I admit I've changed a very sensible part of the code :), however after
>> working with it I realized that we could reduce its complexity removing a
>> couple of nesting levels "merging" them in the outer if...else block. I
>> think this improve code quality and its readability. Let me explain you what
>> I did. The code was:
>>
>>          if (bufferedResponse != null)
>>          {
>>              logger
>>                  .warn("The Buffered response should be handled by
>> BufferedResponseRequestHandler");
>>
>>              // if there is saved response for this URL render it
>> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>>          }
>>          else
>>          {
>>              if (shouldRenderPageAndWriteResponse(requestCycle, currentUrl,
>> targetUrl)) //
>>              {
>>
>>                  BufferedWebResponse response = renderPage(currentUrl,
>> requestCycle);
>>                  if (response != null)
>>                  {
>> response.writeTo((WebResponse)requestCycle.getResponse());
>>
>>                  }
>>              }
>>              else
>>              {
>>                  if (shouldRedirectToTargetUrl(requestCycle, currentUrl,
>> targetUrl))
>>                  {
>>                      redirectTo(targetUrl, requestCycle);
>>
>>
>>                      // note: if we had session here we would render the page
>> to buffer and then
>>                      // redirect to URL generated *after* page has been
>> rendered (the statelessness
>>                      // may change during render). this would save one
>> redirect because now we have
>>                      // to render to URL generated *before* page is rendered,
>> render the page, get
>>                      // URL after render and if the URL is different (meaning
>> page is not stateless),
>>                      // save the buffer and redirect again (which is pretty
>> much what the next step
>>                      // does)
>>                  }
>>                   else
>> ......etc.....
>>
>> now is
>>
>>
>>          if (bufferedResponse != null)
>>          {
>>              logger
>>                  .warn("The Buffered response should be handled by
>> BufferedResponseRequestHandler");
>>
>>              // if there is saved response for this URL render it
>> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>>          }
>>          else if (shouldRenderPageAndWriteResponse(requestCycle, currentUrl,
>> targetUrl))
>>          {
>>
>>              BufferedWebResponse response = renderPage(currentUrl,
>> requestCycle);
>>              if (response != null)
>>              {
>> response.writeTo((WebResponse)requestCycle.getResponse());
>>
>>              }
>>          }
>>          else if (shouldRedirectToTargetUrl(requestCycle, currentUrl,
>> targetUrl))
>>          {
>>
>>              redirectTo(targetUrl, requestCycle);
>>
>>
>>              // note: if we had session here we would render the page to
>> buffer and then
>>              // redirect to URL generated *after* page has been rendered (the
>> statelessness
>>              // may change during render). this would save one redirect
>> because now we have
>>              // to render to URL generated *before* page is rendered, render
>> the page, get
>>              // URL after render and if the URL is different (meaning page is
>> not stateless),
>>              // save the buffer and redirect again (which is pretty much what
>> the next step
>>              // does)
>>          }
>>          else
>>          {
>>
>> two of the 'if' statements next to an 'else' have been merged with this one.
>>
>>
>>
>>
>> On 17/07/2014 16:26, Martin Grigorov wrote:
>>> what is/was useless ?
>>> now I am even more confused what your "beautification" did with this
>>> really
>>> important and fragile part of Wicket code :-)
>>>
>>> I'm not saying that the new code is badly formatted. I was asking whether
>>> "beautification" really means "code formatting"
>>>
>>> Martin Grigorov
>>> Wicket Training and Consulting
>>> https://twitter.com/mtgrigorov
>>>
>>>
>>> On Thu, Jul 17, 2014 at 5:10 PM, Andrea Del Bene <an...@gmail.com>
>>> wrote:
>>>
>>>> Method respond had high level of "if...else" nesting, a couple of them
>>>> useless. in which way the new code doesn't respect Wicket's code format?
>>>> Maybe my IDE did something wrong.
>>>>
>>>>    Hi Andrea,
>>>>> On Wed, Jul 16, 2014 at 8:33 PM, <ad...@apache.org> wrote:
>>>>>
>>>>>    Repository: wicket
>>>>>> Updated Branches:
>>>>>>      refs/heads/master c24d830cd -> bb9c1044e
>>>>>>
>>>>>>
>>>>>> Code beautifying: method WebPageRenderer.respond
>>>>>>
>>>>>>    What exactly "beautifying" means ?
>>>>> It is a bit hard to see what is the actual change. It doesn't look like
>>>>> applying Wicket's code format.
>>>>>
>>>>>
>>>>>    Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>>>>>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/bb9c1044
>>>>>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/bb9c1044
>>>>>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/bb9c1044
>>>>>>
>>>>>> Branch: refs/heads/master
>>>>>> Commit: bb9c1044e5f1ba8902aa69073ef9fb236802d277
>>>>>> Parents: c24d830
>>>>>> Author: andrea del bene <ad...@apache.org>
>>>>>> Authored: Wed Jul 16 19:33:16 2014 +0200
>>>>>> Committer: andrea del bene <ad...@apache.org>
>>>>>> Committed: Wed Jul 16 19:33:16 2014 +0200
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>>     .../request/handler/render/WebPageRenderer.java | 169
>>>>>> +++++++++----------
>>>>>>     1 file changed, 82 insertions(+), 87 deletions(-)
>>>>>> ----------------------------------------------------------------------
>>>>>>
>>>>>>
>>>>>>
>>>>>> http://git-wip-us.apache.org/repos/asf/wicket/blob/
>>>>>> bb9c1044/wicket-core/src/main/java/org/apache/wicket/
>>>>>> request/handler/render/WebPageRenderer.java
>>>>>> ----------------------------------------------------------------------
>>>>>> diff --git
>>>>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>>> WebPageRenderer.java
>>>>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>>> WebPageRenderer.java
>>>>>> index 0b5dee4..af9dbee 100644
>>>>>> ---
>>>>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>>> WebPageRenderer.java
>>>>>> +++
>>>>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>>> WebPageRenderer.java
>>>>>> @@ -198,99 +198,94 @@ public class WebPageRenderer extends PageRenderer
>>>>>>                            // if there is saved response for this URL
>>>>>> render
>>>>>> it
>>>>>>
>>>>>> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>>>>>>                    }
>>>>>> +               else if (shouldRenderPageAndWriteResponse(requestCycle,
>>>>>> currentUrl, targetUrl))
>>>>>> +               {
>>>>>> +                       BufferedWebResponse response =
>>>>>> renderPage(currentUrl, requestCycle);
>>>>>> +                       if (response != null)
>>>>>> +                       {
>>>>>> +
>>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>>> +                       }
>>>>>> +               }
>>>>>> +               else if (shouldRedirectToTargetUrl(requestCycle,
>>>>>> currentUrl, targetUrl))
>>>>>> +               {
>>>>>> +
>>>>>> +                       redirectTo(targetUrl, requestCycle);
>>>>>> +
>>>>>> +                       // note: if we had session here we would render
>>>>>> the page to buffer and then
>>>>>> +                       // redirect to URL generated *after* page has
>>>>>> been
>>>>>> rendered (the statelessness
>>>>>> +                       // may change during render). this would save
>>>>>> one
>>>>>> redirect because now we have
>>>>>> +                       // to render to URL generated *before* page is
>>>>>> rendered, render the page, get
>>>>>> +                       // URL after render and if the URL is different
>>>>>> (meaning page is not stateless),
>>>>>> +                       // save the buffer and redirect again (which is
>>>>>> pretty much what the next step
>>>>>> +                       // does)
>>>>>> +               }
>>>>>>                    else
>>>>>>                    {
>>>>>> -                       if (shouldRenderPageAndWriteRespon
>>>>>> se(requestCycle,
>>>>>> currentUrl, targetUrl)) //
>>>>>> +                       if (isRedirectToBuffer() == false &&
>>>>>> logger.isDebugEnabled())
>>>>>> +                       {
>>>>>> +                               String details = String
>>>>>> +                                       .format(
>>>>>> +                                               "redirect strategy:
>>>>>> '%s',
>>>>>> isAjax: '%s', redirect policy: '%s', "
>>>>>> +                                                       + "current url:
>>>>>> '%s', target url: '%s', is new: '%s', is stateless: '%s', is temporary:
>>>>>> '%s'",
>>>>>> +
>>>>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>>>>> +                                               isAjax(requestCycle),
>>>>>> getRedirectPolicy(), currentUrl, targetUrl,
>>>>>> +                                               isNewPageInstance(),
>>>>>> isPageStateless(), isSessionTemporary());
>>>>>> +                               logger
>>>>>> +                                       .debug("Falling back to
>>>>>> Redirect_To_Buffer render strategy because none of the conditions "
>>>>>> +                                               + "matched. Details: "
>>>>>> +
>>>>>> details);
>>>>>> +                       }
>>>>>> +
>>>>>> +                       // force creation of possible stateful page to
>>>>>> get
>>>>>> the final target url
>>>>>> +                       getPage();
>>>>>> +
>>>>>> +                       Url beforeRenderUrl =
>>>>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>>>>> +
>>>>>> +                       // redirect to buffer
>>>>>> +                       BufferedWebResponse response =
>>>>>> renderPage(beforeRenderUrl, requestCycle);
>>>>>> +
>>>>>> +                       if (response == null)
>>>>>> +                       {
>>>>>> +                               return;
>>>>>> +                       }
>>>>>> +
>>>>>> +                       // the url might have changed after page has
>>>>>> been
>>>>>> rendered (e.g. the
>>>>>> +                       // stateless flag might have changed because
>>>>>> stateful components
>>>>>> +                       // were added)
>>>>>> +                       final Url afterRenderUrl = requestCycle
>>>>>> +                               .mapUrlFor(
>>>>>> getRenderPageRequestHandler());
>>>>>> +
>>>>>> +                       if
>>>>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>>>>> false)
>>>>>>                            {
>>>>>> -                               BufferedWebResponse response =
>>>>>> renderPage(currentUrl, requestCycle);
>>>>>> -                               if (response != null)
>>>>>> -                               {
>>>>>> -
>>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>>> -                               }
>>>>>> +                               // the amount of segments is different
>>>>>> -
>>>>>> generated relative URLs
>>>>>> +                               // will not work, we need to rerender
>>>>>> the
>>>>>> page. This can happen
>>>>>> +                               // with IRequestHandlers that produce
>>>>>> different URLs with
>>>>>> +                               // different amount of segments for
>>>>>> stateless and stateful pages
>>>>>> +                               response = renderPage(afterRenderUrl,
>>>>>> requestCycle);
>>>>>> +                       }
>>>>>> +
>>>>>> +                       if (currentUrl.equals(afterRenderUrl))
>>>>>> +                       {
>>>>>> +                               // no need to redirect when both urls
>>>>>> are
>>>>>> exactly the same
>>>>>> +
>>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>>> +                       }
>>>>>> +                       // if page is still stateless after render
>>>>>> +                       else if (isPageStateless() &&
>>>>>> !enableRedirectForStatelessPage())
>>>>>> +                       {
>>>>>> +                               // we don't want the redirect to happen
>>>>>> for stateless page
>>>>>> +                               // example:
>>>>>> +                               // when a normal mounted stateful page
>>>>>> is
>>>>>> hit at /mount/point
>>>>>> +                               // wicket renders the page to buffer
>>>>>> and
>>>>>> redirects to /mount/point?12
>>>>>> +                               // but for stateless page the redirect
>>>>>> is
>>>>>> not necessary
>>>>>> +                               // also for listener interface on
>>>>>> stateful
>>>>>> page we want to redirect
>>>>>> +                               // after the listener is invoked, but
>>>>>> on
>>>>>> stateless page the user
>>>>>> +                               // must ask for redirect explicitly
>>>>>> +
>>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>>>                            }
>>>>>>                            else
>>>>>>                            {
>>>>>> -                               if
>>>>>> (shouldRedirectToTargetUrl(requestCycle, currentUrl, targetUrl))
>>>>>> -                               {
>>>>>> -                                       redirectTo(targetUrl,
>>>>>> requestCycle);
>>>>>> -
>>>>>> -                                       // note: if we had session here
>>>>>> we
>>>>>> would render the page to buffer and then
>>>>>> -                                       // redirect to URL generated
>>>>>> *after* page has been rendered (the statelessness
>>>>>> -                                       // may change during render).
>>>>>> this
>>>>>> would save one redirect because now we have
>>>>>> -                                       // to render to URL generated
>>>>>> *before* page is rendered, render the page, get
>>>>>> -                                       // URL after render and if the
>>>>>> URL
>>>>>> is different (meaning page is not stateless),
>>>>>> -                                       // save the buffer and redirect
>>>>>> again (which is pretty much what the next step
>>>>>> -                                       // does)
>>>>>> -                               }
>>>>>> -                               else
>>>>>> -                               {
>>>>>> -                                       if (isRedirectToBuffer() ==
>>>>>> false
>>>>>> && logger.isDebugEnabled())
>>>>>> -                                       {
>>>>>> -                                               String details = String
>>>>>> -                                                       .format(
>>>>>> -
>>>>>> "redirect
>>>>>> strategy: '%s', isAjax: '%s', redirect policy: '%s', "
>>>>>> -
>>>>>> +
>>>>>> "current url: '%s', target url: '%s', is new: '%s', is stateless: '%s',
>>>>>> is
>>>>>> temporary: '%s'",
>>>>>> -
>>>>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>>>>> -
>>>>>> isAjax(requestCycle), getRedirectPolicy(), currentUrl, targetUrl,
>>>>>> -
>>>>>> isNewPageInstance(), isPageStateless(), isSessionTemporary());
>>>>>> -                                               logger
>>>>>> -                                                       .debug("Falling
>>>>>> back to Redirect_To_Buffer render strategy because none of the
>>>>>> conditions "
>>>>>> -                                                               +
>>>>>> "matched. Details: " + details);
>>>>>> -                                       }
>>>>>> -
>>>>>> -                                       // force creation of possible
>>>>>> stateful page to get the final target url
>>>>>> -                                       getPage();
>>>>>> -
>>>>>> -                                       Url beforeRenderUrl =
>>>>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>>>>> -
>>>>>> -                                       // redirect to buffer
>>>>>> -                                       BufferedWebResponse response =
>>>>>> renderPage(beforeRenderUrl, requestCycle);
>>>>>> -
>>>>>> -                                       if (response == null)
>>>>>> -                                       {
>>>>>> -                                               return;
>>>>>> -                                       }
>>>>>> -
>>>>>> -                                       // the url might have changed
>>>>>> after page has been rendered (e.g. the
>>>>>> -                                       // stateless flag might have
>>>>>> changed because stateful components
>>>>>> -                                       // were added)
>>>>>> -                                       final Url afterRenderUrl =
>>>>>> requestCycle
>>>>>> -
>>>>>> .mapUrlFor(getRenderPageRequestHandler());
>>>>>> -
>>>>>> -                                       if
>>>>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>>>>> false)
>>>>>> -                                       {
>>>>>> -                                               // the amount of
>>>>>> segments
>>>>>> is different - generated relative URLs
>>>>>> -                                               // will not work, we
>>>>>> need
>>>>>> to rerender the page. This can happen
>>>>>> -                                               // with
>>>>>> IRequestHandlers
>>>>>> that produce different URLs with
>>>>>> -                                               // different amount of
>>>>>> segments for stateless and stateful pages
>>>>>> -                                               response =
>>>>>> renderPage(afterRenderUrl, requestCycle);
>>>>>> -                                       }
>>>>>> -
>>>>>> -                                       if
>>>>>> (currentUrl.equals(afterRenderUrl))
>>>>>> -                                       {
>>>>>> -                                               // no need to redirect
>>>>>> when both urls are exactly the same
>>>>>> -
>>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>>> -                                       }
>>>>>> -                                       // if page is still stateless
>>>>>> after render
>>>>>> -                                       else if (isPageStateless() &&
>>>>>> !enableRedirectForStatelessPage())
>>>>>> -                                       {
>>>>>> -                                               // we don't want the
>>>>>> redirect to happen for stateless page
>>>>>> -                                               // example:
>>>>>> -                                               // when a normal
>>>>>> mounted
>>>>>> stateful page is hit at /mount/point
>>>>>> -                                               // wicket renders the
>>>>>> page
>>>>>> to buffer and redirects to /mount/point?12
>>>>>> -                                               // but for stateless
>>>>>> page
>>>>>> the redirect is not necessary
>>>>>> -                                               // also for listener
>>>>>> interface on stateful page we want to redirect
>>>>>> -                                               // after the listener
>>>>>> is
>>>>>> invoked, but on stateless page the user
>>>>>> -                                               // must ask for
>>>>>> redirect
>>>>>> explicitly
>>>>>> -
>>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>>> -                                       }
>>>>>> -                                       else
>>>>>> -                                       {
>>>>>> -
>>>>>> storeBufferedResponse(afterRenderUrl, response);
>>>>>> -
>>>>>> -
>>>>>> redirectTo(afterRenderUrl,
>>>>>> requestCycle);
>>>>>> -                                       }
>>>>>> -                               }
>>>>>> +                               storeBufferedResponse(afterRenderUrl,
>>>>>> response);
>>>>>> +
>>>>>> +                               redirectTo(afterRenderUrl,
>>>>>> requestCycle);
>>>>>>                            }
>>>>>>                    }
>>>>>>            }
>>>>>>
>>>>>>
>>>>>>


Re: git commit: Code beautifying: method WebPageRenderer.respond

Posted by tetsuo <ro...@gmail.com>.
You just removed two pairs of curly braces (to turn 'else { if {} }'
into 'else if {}') and re-indented the code?
“The truth is rarely pure and never simple.” ― Oscar Wilde


On Thu, Jul 17, 2014 at 11:53 AM, Andrea Del Bene <an...@gmail.com> wrote:
> I admit I've changed a very sensible part of the code :), however after
> working with it I realized that we could reduce its complexity removing a
> couple of nesting levels "merging" them in the outer if...else block. I
> think this improve code quality and its readability. Let me explain you what
> I did. The code was:
>
>         if (bufferedResponse != null)
>         {
>             logger
>                 .warn("The Buffered response should be handled by
> BufferedResponseRequestHandler");
>
>             // if there is saved response for this URL render it
> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>         }
>         else
>         {
>             if (shouldRenderPageAndWriteResponse(requestCycle, currentUrl,
> targetUrl)) //
>             {
>
>                 BufferedWebResponse response = renderPage(currentUrl,
> requestCycle);
>                 if (response != null)
>                 {
> response.writeTo((WebResponse)requestCycle.getResponse());
>
>                 }
>             }
>             else
>             {
>                 if (shouldRedirectToTargetUrl(requestCycle, currentUrl,
> targetUrl))
>                 {
>                     redirectTo(targetUrl, requestCycle);
>
>
>                     // note: if we had session here we would render the page
> to buffer and then
>                     // redirect to URL generated *after* page has been
> rendered (the statelessness
>                     // may change during render). this would save one
> redirect because now we have
>                     // to render to URL generated *before* page is rendered,
> render the page, get
>                     // URL after render and if the URL is different (meaning
> page is not stateless),
>                     // save the buffer and redirect again (which is pretty
> much what the next step
>                     // does)
>                 }
>                  else
> ......etc.....
>
> now is
>
>
>         if (bufferedResponse != null)
>         {
>             logger
>                 .warn("The Buffered response should be handled by
> BufferedResponseRequestHandler");
>
>             // if there is saved response for this URL render it
> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>         }
>         else if (shouldRenderPageAndWriteResponse(requestCycle, currentUrl,
> targetUrl))
>         {
>
>             BufferedWebResponse response = renderPage(currentUrl,
> requestCycle);
>             if (response != null)
>             {
> response.writeTo((WebResponse)requestCycle.getResponse());
>
>             }
>         }
>         else if (shouldRedirectToTargetUrl(requestCycle, currentUrl,
> targetUrl))
>         {
>
>             redirectTo(targetUrl, requestCycle);
>
>
>             // note: if we had session here we would render the page to
> buffer and then
>             // redirect to URL generated *after* page has been rendered (the
> statelessness
>             // may change during render). this would save one redirect
> because now we have
>             // to render to URL generated *before* page is rendered, render
> the page, get
>             // URL after render and if the URL is different (meaning page is
> not stateless),
>             // save the buffer and redirect again (which is pretty much what
> the next step
>             // does)
>         }
>         else
>         {
>
> two of the 'if' statements next to an 'else' have been merged with this one.
>
>
>
>
> On 17/07/2014 16:26, Martin Grigorov wrote:
>>
>> what is/was useless ?
>> now I am even more confused what your "beautification" did with this
>> really
>> important and fragile part of Wicket code :-)
>>
>> I'm not saying that the new code is badly formatted. I was asking whether
>> "beautification" really means "code formatting"
>>
>> Martin Grigorov
>> Wicket Training and Consulting
>> https://twitter.com/mtgrigorov
>>
>>
>> On Thu, Jul 17, 2014 at 5:10 PM, Andrea Del Bene <an...@gmail.com>
>> wrote:
>>
>>> Method respond had high level of "if...else" nesting, a couple of them
>>> useless. in which way the new code doesn't respect Wicket's code format?
>>> Maybe my IDE did something wrong.
>>>
>>>   Hi Andrea,
>>>>
>>>> On Wed, Jul 16, 2014 at 8:33 PM, <ad...@apache.org> wrote:
>>>>
>>>>   Repository: wicket
>>>>>
>>>>> Updated Branches:
>>>>>     refs/heads/master c24d830cd -> bb9c1044e
>>>>>
>>>>>
>>>>> Code beautifying: method WebPageRenderer.respond
>>>>>
>>>>>   What exactly "beautifying" means ?
>>>>
>>>> It is a bit hard to see what is the actual change. It doesn't look like
>>>> applying Wicket's code format.
>>>>
>>>>
>>>>   Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>>>>>
>>>>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/bb9c1044
>>>>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/bb9c1044
>>>>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/bb9c1044
>>>>>
>>>>> Branch: refs/heads/master
>>>>> Commit: bb9c1044e5f1ba8902aa69073ef9fb236802d277
>>>>> Parents: c24d830
>>>>> Author: andrea del bene <ad...@apache.org>
>>>>> Authored: Wed Jul 16 19:33:16 2014 +0200
>>>>> Committer: andrea del bene <ad...@apache.org>
>>>>> Committed: Wed Jul 16 19:33:16 2014 +0200
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>>    .../request/handler/render/WebPageRenderer.java | 169
>>>>> +++++++++----------
>>>>>    1 file changed, 82 insertions(+), 87 deletions(-)
>>>>> ----------------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>> http://git-wip-us.apache.org/repos/asf/wicket/blob/
>>>>> bb9c1044/wicket-core/src/main/java/org/apache/wicket/
>>>>> request/handler/render/WebPageRenderer.java
>>>>> ----------------------------------------------------------------------
>>>>> diff --git
>>>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>> WebPageRenderer.java
>>>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>> WebPageRenderer.java
>>>>> index 0b5dee4..af9dbee 100644
>>>>> ---
>>>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>> WebPageRenderer.java
>>>>> +++
>>>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>> WebPageRenderer.java
>>>>> @@ -198,99 +198,94 @@ public class WebPageRenderer extends PageRenderer
>>>>>                           // if there is saved response for this URL
>>>>> render
>>>>> it
>>>>>
>>>>> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>>>>>                   }
>>>>> +               else if (shouldRenderPageAndWriteResponse(requestCycle,
>>>>> currentUrl, targetUrl))
>>>>> +               {
>>>>> +                       BufferedWebResponse response =
>>>>> renderPage(currentUrl, requestCycle);
>>>>> +                       if (response != null)
>>>>> +                       {
>>>>> +
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> +                       }
>>>>> +               }
>>>>> +               else if (shouldRedirectToTargetUrl(requestCycle,
>>>>> currentUrl, targetUrl))
>>>>> +               {
>>>>> +
>>>>> +                       redirectTo(targetUrl, requestCycle);
>>>>> +
>>>>> +                       // note: if we had session here we would render
>>>>> the page to buffer and then
>>>>> +                       // redirect to URL generated *after* page has
>>>>> been
>>>>> rendered (the statelessness
>>>>> +                       // may change during render). this would save
>>>>> one
>>>>> redirect because now we have
>>>>> +                       // to render to URL generated *before* page is
>>>>> rendered, render the page, get
>>>>> +                       // URL after render and if the URL is different
>>>>> (meaning page is not stateless),
>>>>> +                       // save the buffer and redirect again (which is
>>>>> pretty much what the next step
>>>>> +                       // does)
>>>>> +               }
>>>>>                   else
>>>>>                   {
>>>>> -                       if (shouldRenderPageAndWriteRespon
>>>>> se(requestCycle,
>>>>> currentUrl, targetUrl)) //
>>>>> +                       if (isRedirectToBuffer() == false &&
>>>>> logger.isDebugEnabled())
>>>>> +                       {
>>>>> +                               String details = String
>>>>> +                                       .format(
>>>>> +                                               "redirect strategy:
>>>>> '%s',
>>>>> isAjax: '%s', redirect policy: '%s', "
>>>>> +                                                       + "current url:
>>>>> '%s', target url: '%s', is new: '%s', is stateless: '%s', is temporary:
>>>>> '%s'",
>>>>> +
>>>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>>>> +                                               isAjax(requestCycle),
>>>>> getRedirectPolicy(), currentUrl, targetUrl,
>>>>> +                                               isNewPageInstance(),
>>>>> isPageStateless(), isSessionTemporary());
>>>>> +                               logger
>>>>> +                                       .debug("Falling back to
>>>>> Redirect_To_Buffer render strategy because none of the conditions "
>>>>> +                                               + "matched. Details: "
>>>>> +
>>>>> details);
>>>>> +                       }
>>>>> +
>>>>> +                       // force creation of possible stateful page to
>>>>> get
>>>>> the final target url
>>>>> +                       getPage();
>>>>> +
>>>>> +                       Url beforeRenderUrl =
>>>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>>>> +
>>>>> +                       // redirect to buffer
>>>>> +                       BufferedWebResponse response =
>>>>> renderPage(beforeRenderUrl, requestCycle);
>>>>> +
>>>>> +                       if (response == null)
>>>>> +                       {
>>>>> +                               return;
>>>>> +                       }
>>>>> +
>>>>> +                       // the url might have changed after page has
>>>>> been
>>>>> rendered (e.g. the
>>>>> +                       // stateless flag might have changed because
>>>>> stateful components
>>>>> +                       // were added)
>>>>> +                       final Url afterRenderUrl = requestCycle
>>>>> +                               .mapUrlFor(
>>>>> getRenderPageRequestHandler());
>>>>> +
>>>>> +                       if
>>>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>>>> false)
>>>>>                           {
>>>>> -                               BufferedWebResponse response =
>>>>> renderPage(currentUrl, requestCycle);
>>>>> -                               if (response != null)
>>>>> -                               {
>>>>> -
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> -                               }
>>>>> +                               // the amount of segments is different
>>>>> -
>>>>> generated relative URLs
>>>>> +                               // will not work, we need to rerender
>>>>> the
>>>>> page. This can happen
>>>>> +                               // with IRequestHandlers that produce
>>>>> different URLs with
>>>>> +                               // different amount of segments for
>>>>> stateless and stateful pages
>>>>> +                               response = renderPage(afterRenderUrl,
>>>>> requestCycle);
>>>>> +                       }
>>>>> +
>>>>> +                       if (currentUrl.equals(afterRenderUrl))
>>>>> +                       {
>>>>> +                               // no need to redirect when both urls
>>>>> are
>>>>> exactly the same
>>>>> +
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> +                       }
>>>>> +                       // if page is still stateless after render
>>>>> +                       else if (isPageStateless() &&
>>>>> !enableRedirectForStatelessPage())
>>>>> +                       {
>>>>> +                               // we don't want the redirect to happen
>>>>> for stateless page
>>>>> +                               // example:
>>>>> +                               // when a normal mounted stateful page
>>>>> is
>>>>> hit at /mount/point
>>>>> +                               // wicket renders the page to buffer
>>>>> and
>>>>> redirects to /mount/point?12
>>>>> +                               // but for stateless page the redirect
>>>>> is
>>>>> not necessary
>>>>> +                               // also for listener interface on
>>>>> stateful
>>>>> page we want to redirect
>>>>> +                               // after the listener is invoked, but
>>>>> on
>>>>> stateless page the user
>>>>> +                               // must ask for redirect explicitly
>>>>> +
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>>                           }
>>>>>                           else
>>>>>                           {
>>>>> -                               if
>>>>> (shouldRedirectToTargetUrl(requestCycle, currentUrl, targetUrl))
>>>>> -                               {
>>>>> -                                       redirectTo(targetUrl,
>>>>> requestCycle);
>>>>> -
>>>>> -                                       // note: if we had session here
>>>>> we
>>>>> would render the page to buffer and then
>>>>> -                                       // redirect to URL generated
>>>>> *after* page has been rendered (the statelessness
>>>>> -                                       // may change during render).
>>>>> this
>>>>> would save one redirect because now we have
>>>>> -                                       // to render to URL generated
>>>>> *before* page is rendered, render the page, get
>>>>> -                                       // URL after render and if the
>>>>> URL
>>>>> is different (meaning page is not stateless),
>>>>> -                                       // save the buffer and redirect
>>>>> again (which is pretty much what the next step
>>>>> -                                       // does)
>>>>> -                               }
>>>>> -                               else
>>>>> -                               {
>>>>> -                                       if (isRedirectToBuffer() ==
>>>>> false
>>>>> && logger.isDebugEnabled())
>>>>> -                                       {
>>>>> -                                               String details = String
>>>>> -                                                       .format(
>>>>> -
>>>>> "redirect
>>>>> strategy: '%s', isAjax: '%s', redirect policy: '%s', "
>>>>> -
>>>>> +
>>>>> "current url: '%s', target url: '%s', is new: '%s', is stateless: '%s',
>>>>> is
>>>>> temporary: '%s'",
>>>>> -
>>>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>>>> -
>>>>> isAjax(requestCycle), getRedirectPolicy(), currentUrl, targetUrl,
>>>>> -
>>>>> isNewPageInstance(), isPageStateless(), isSessionTemporary());
>>>>> -                                               logger
>>>>> -                                                       .debug("Falling
>>>>> back to Redirect_To_Buffer render strategy because none of the
>>>>> conditions "
>>>>> -                                                               +
>>>>> "matched. Details: " + details);
>>>>> -                                       }
>>>>> -
>>>>> -                                       // force creation of possible
>>>>> stateful page to get the final target url
>>>>> -                                       getPage();
>>>>> -
>>>>> -                                       Url beforeRenderUrl =
>>>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>>>> -
>>>>> -                                       // redirect to buffer
>>>>> -                                       BufferedWebResponse response =
>>>>> renderPage(beforeRenderUrl, requestCycle);
>>>>> -
>>>>> -                                       if (response == null)
>>>>> -                                       {
>>>>> -                                               return;
>>>>> -                                       }
>>>>> -
>>>>> -                                       // the url might have changed
>>>>> after page has been rendered (e.g. the
>>>>> -                                       // stateless flag might have
>>>>> changed because stateful components
>>>>> -                                       // were added)
>>>>> -                                       final Url afterRenderUrl =
>>>>> requestCycle
>>>>> -
>>>>> .mapUrlFor(getRenderPageRequestHandler());
>>>>> -
>>>>> -                                       if
>>>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>>>> false)
>>>>> -                                       {
>>>>> -                                               // the amount of
>>>>> segments
>>>>> is different - generated relative URLs
>>>>> -                                               // will not work, we
>>>>> need
>>>>> to rerender the page. This can happen
>>>>> -                                               // with
>>>>> IRequestHandlers
>>>>> that produce different URLs with
>>>>> -                                               // different amount of
>>>>> segments for stateless and stateful pages
>>>>> -                                               response =
>>>>> renderPage(afterRenderUrl, requestCycle);
>>>>> -                                       }
>>>>> -
>>>>> -                                       if
>>>>> (currentUrl.equals(afterRenderUrl))
>>>>> -                                       {
>>>>> -                                               // no need to redirect
>>>>> when both urls are exactly the same
>>>>> -
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> -                                       }
>>>>> -                                       // if page is still stateless
>>>>> after render
>>>>> -                                       else if (isPageStateless() &&
>>>>> !enableRedirectForStatelessPage())
>>>>> -                                       {
>>>>> -                                               // we don't want the
>>>>> redirect to happen for stateless page
>>>>> -                                               // example:
>>>>> -                                               // when a normal
>>>>> mounted
>>>>> stateful page is hit at /mount/point
>>>>> -                                               // wicket renders the
>>>>> page
>>>>> to buffer and redirects to /mount/point?12
>>>>> -                                               // but for stateless
>>>>> page
>>>>> the redirect is not necessary
>>>>> -                                               // also for listener
>>>>> interface on stateful page we want to redirect
>>>>> -                                               // after the listener
>>>>> is
>>>>> invoked, but on stateless page the user
>>>>> -                                               // must ask for
>>>>> redirect
>>>>> explicitly
>>>>> -
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> -                                       }
>>>>> -                                       else
>>>>> -                                       {
>>>>> -
>>>>> storeBufferedResponse(afterRenderUrl, response);
>>>>> -
>>>>> -
>>>>> redirectTo(afterRenderUrl,
>>>>> requestCycle);
>>>>> -                                       }
>>>>> -                               }
>>>>> +                               storeBufferedResponse(afterRenderUrl,
>>>>> response);
>>>>> +
>>>>> +                               redirectTo(afterRenderUrl,
>>>>> requestCycle);
>>>>>                           }
>>>>>                   }
>>>>>           }
>>>>>
>>>>>
>>>>>
>

Re: git commit: Code beautifying: method WebPageRenderer.respond

Posted by Martin Grigorov <mg...@apache.org>.
OK. Thanks!

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Thu, Jul 17, 2014 at 5:53 PM, Andrea Del Bene <an...@gmail.com>
wrote:

> I admit I've changed a very sensible part of the code :), however after
> working with it I realized that we could reduce its complexity removing a
> couple of nesting levels "merging" them in the outer if...else block. I
> think this improve code quality and its readability. Let me explain you
> what I did. The code was:
>
>         if (bufferedResponse != null)
>         {
>             logger
>                 .warn("The Buffered response should be handled by
> BufferedResponseRequestHandler");
>
>             // if there is saved response for this URL render it
> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>         }
>         else
>         {
>             if (shouldRenderPageAndWriteResponse(requestCycle,
> currentUrl, targetUrl)) //
>             {
>
>                 BufferedWebResponse response = renderPage(currentUrl,
> requestCycle);
>                 if (response != null)
>                 {
> response.writeTo((WebResponse)requestCycle.getResponse());
>
>                 }
>             }
>             else
>             {
>                 if (shouldRedirectToTargetUrl(requestCycle, currentUrl,
> targetUrl))
>                 {
>                     redirectTo(targetUrl, requestCycle);
>
>
>                     // note: if we had session here we would render the
> page to buffer and then
>                     // redirect to URL generated *after* page has been
> rendered (the statelessness
>                     // may change during render). this would save one
> redirect because now we have
>                     // to render to URL generated *before* page is
> rendered, render the page, get
>                     // URL after render and if the URL is different
> (meaning page is not stateless),
>                     // save the buffer and redirect again (which is pretty
> much what the next step
>                     // does)
>                 }
>                  else
> ......etc.....
>
> now is
>
>
>         if (bufferedResponse != null)
>         {
>             logger
>                 .warn("The Buffered response should be handled by
> BufferedResponseRequestHandler");
>
>             // if there is saved response for this URL render it
> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>         }
>         else if (shouldRenderPageAndWriteResponse(requestCycle,
> currentUrl, targetUrl))
>         {
>
>             BufferedWebResponse response = renderPage(currentUrl,
> requestCycle);
>             if (response != null)
>             {
> response.writeTo((WebResponse)requestCycle.getResponse());
>
>             }
>         }
>         else if (shouldRedirectToTargetUrl(requestCycle, currentUrl,
> targetUrl))
>         {
>
>             redirectTo(targetUrl, requestCycle);
>
>
>             // note: if we had session here we would render the page to
> buffer and then
>             // redirect to URL generated *after* page has been rendered
> (the statelessness
>             // may change during render). this would save one redirect
> because now we have
>             // to render to URL generated *before* page is rendered,
> render the page, get
>             // URL after render and if the URL is different (meaning page
> is not stateless),
>             // save the buffer and redirect again (which is pretty much
> what the next step
>             // does)
>         }
>         else
>         {
>
> two of the 'if' statements next to an 'else' have been merged with this
> one.
>
>
>
>
> On 17/07/2014 16:26, Martin Grigorov wrote:
>
>> what is/was useless ?
>> now I am even more confused what your "beautification" did with this
>> really
>> important and fragile part of Wicket code :-)
>>
>> I'm not saying that the new code is badly formatted. I was asking whether
>> "beautification" really means "code formatting"
>>
>> Martin Grigorov
>> Wicket Training and Consulting
>> https://twitter.com/mtgrigorov
>>
>>
>> On Thu, Jul 17, 2014 at 5:10 PM, Andrea Del Bene <an...@gmail.com>
>> wrote:
>>
>>  Method respond had high level of "if...else" nesting, a couple of them
>>> useless. in which way the new code doesn't respect Wicket's code format?
>>> Maybe my IDE did something wrong.
>>>
>>>   Hi Andrea,
>>>
>>>> On Wed, Jul 16, 2014 at 8:33 PM, <ad...@apache.org> wrote:
>>>>
>>>>   Repository: wicket
>>>>
>>>>> Updated Branches:
>>>>>     refs/heads/master c24d830cd -> bb9c1044e
>>>>>
>>>>>
>>>>> Code beautifying: method WebPageRenderer.respond
>>>>>
>>>>>   What exactly "beautifying" means ?
>>>>>
>>>> It is a bit hard to see what is the actual change. It doesn't look like
>>>> applying Wicket's code format.
>>>>
>>>>
>>>>   Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>>>>
>>>>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/bb9c1044
>>>>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/bb9c1044
>>>>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/bb9c1044
>>>>>
>>>>> Branch: refs/heads/master
>>>>> Commit: bb9c1044e5f1ba8902aa69073ef9fb236802d277
>>>>> Parents: c24d830
>>>>> Author: andrea del bene <ad...@apache.org>
>>>>> Authored: Wed Jul 16 19:33:16 2014 +0200
>>>>> Committer: andrea del bene <ad...@apache.org>
>>>>> Committed: Wed Jul 16 19:33:16 2014 +0200
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>>    .../request/handler/render/WebPageRenderer.java | 169
>>>>> +++++++++----------
>>>>>    1 file changed, 82 insertions(+), 87 deletions(-)
>>>>> ----------------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>> http://git-wip-us.apache.org/repos/asf/wicket/blob/
>>>>> bb9c1044/wicket-core/src/main/java/org/apache/wicket/
>>>>> request/handler/render/WebPageRenderer.java
>>>>> ----------------------------------------------------------------------
>>>>> diff --git
>>>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>> WebPageRenderer.java
>>>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>> WebPageRenderer.java
>>>>> index 0b5dee4..af9dbee 100644
>>>>> ---
>>>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>> WebPageRenderer.java
>>>>> +++
>>>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>>> WebPageRenderer.java
>>>>> @@ -198,99 +198,94 @@ public class WebPageRenderer extends PageRenderer
>>>>>                           // if there is saved response for this URL
>>>>> render
>>>>> it
>>>>>
>>>>> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>>>>>                   }
>>>>> +               else if (shouldRenderPageAndWriteRespon
>>>>> se(requestCycle,
>>>>> currentUrl, targetUrl))
>>>>> +               {
>>>>> +                       BufferedWebResponse response =
>>>>> renderPage(currentUrl, requestCycle);
>>>>> +                       if (response != null)
>>>>> +                       {
>>>>> +
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> +                       }
>>>>> +               }
>>>>> +               else if (shouldRedirectToTargetUrl(requestCycle,
>>>>> currentUrl, targetUrl))
>>>>> +               {
>>>>> +
>>>>> +                       redirectTo(targetUrl, requestCycle);
>>>>> +
>>>>> +                       // note: if we had session here we would render
>>>>> the page to buffer and then
>>>>> +                       // redirect to URL generated *after* page has
>>>>> been
>>>>> rendered (the statelessness
>>>>> +                       // may change during render). this would save
>>>>> one
>>>>> redirect because now we have
>>>>> +                       // to render to URL generated *before* page is
>>>>> rendered, render the page, get
>>>>> +                       // URL after render and if the URL is different
>>>>> (meaning page is not stateless),
>>>>> +                       // save the buffer and redirect again (which is
>>>>> pretty much what the next step
>>>>> +                       // does)
>>>>> +               }
>>>>>                   else
>>>>>                   {
>>>>> -                       if (shouldRenderPageAndWriteRespon
>>>>> se(requestCycle,
>>>>> currentUrl, targetUrl)) //
>>>>> +                       if (isRedirectToBuffer() == false &&
>>>>> logger.isDebugEnabled())
>>>>> +                       {
>>>>> +                               String details = String
>>>>> +                                       .format(
>>>>> +                                               "redirect strategy:
>>>>> '%s',
>>>>> isAjax: '%s', redirect policy: '%s', "
>>>>> +                                                       + "current url:
>>>>> '%s', target url: '%s', is new: '%s', is stateless: '%s', is temporary:
>>>>> '%s'",
>>>>> +
>>>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>>>> +                                               isAjax(requestCycle),
>>>>> getRedirectPolicy(), currentUrl, targetUrl,
>>>>> +                                               isNewPageInstance(),
>>>>> isPageStateless(), isSessionTemporary());
>>>>> +                               logger
>>>>> +                                       .debug("Falling back to
>>>>> Redirect_To_Buffer render strategy because none of the conditions "
>>>>> +                                               + "matched. Details: "
>>>>> +
>>>>> details);
>>>>> +                       }
>>>>> +
>>>>> +                       // force creation of possible stateful page to
>>>>> get
>>>>> the final target url
>>>>> +                       getPage();
>>>>> +
>>>>> +                       Url beforeRenderUrl =
>>>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>>>> +
>>>>> +                       // redirect to buffer
>>>>> +                       BufferedWebResponse response =
>>>>> renderPage(beforeRenderUrl, requestCycle);
>>>>> +
>>>>> +                       if (response == null)
>>>>> +                       {
>>>>> +                               return;
>>>>> +                       }
>>>>> +
>>>>> +                       // the url might have changed after page has
>>>>> been
>>>>> rendered (e.g. the
>>>>> +                       // stateless flag might have changed because
>>>>> stateful components
>>>>> +                       // were added)
>>>>> +                       final Url afterRenderUrl = requestCycle
>>>>> +                               .mapUrlFor(
>>>>> getRenderPageRequestHandler());
>>>>> +
>>>>> +                       if
>>>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>>>> false)
>>>>>                           {
>>>>> -                               BufferedWebResponse response =
>>>>> renderPage(currentUrl, requestCycle);
>>>>> -                               if (response != null)
>>>>> -                               {
>>>>> -
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> -                               }
>>>>> +                               // the amount of segments is different
>>>>> -
>>>>> generated relative URLs
>>>>> +                               // will not work, we need to rerender
>>>>> the
>>>>> page. This can happen
>>>>> +                               // with IRequestHandlers that produce
>>>>> different URLs with
>>>>> +                               // different amount of segments for
>>>>> stateless and stateful pages
>>>>> +                               response = renderPage(afterRenderUrl,
>>>>> requestCycle);
>>>>> +                       }
>>>>> +
>>>>> +                       if (currentUrl.equals(afterRenderUrl))
>>>>> +                       {
>>>>> +                               // no need to redirect when both urls
>>>>> are
>>>>> exactly the same
>>>>> +
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> +                       }
>>>>> +                       // if page is still stateless after render
>>>>> +                       else if (isPageStateless() &&
>>>>> !enableRedirectForStatelessPage())
>>>>> +                       {
>>>>> +                               // we don't want the redirect to happen
>>>>> for stateless page
>>>>> +                               // example:
>>>>> +                               // when a normal mounted stateful page
>>>>> is
>>>>> hit at /mount/point
>>>>> +                               // wicket renders the page to buffer
>>>>> and
>>>>> redirects to /mount/point?12
>>>>> +                               // but for stateless page the redirect
>>>>> is
>>>>> not necessary
>>>>> +                               // also for listener interface on
>>>>> stateful
>>>>> page we want to redirect
>>>>> +                               // after the listener is invoked, but
>>>>> on
>>>>> stateless page the user
>>>>> +                               // must ask for redirect explicitly
>>>>> +
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>>                           }
>>>>>                           else
>>>>>                           {
>>>>> -                               if
>>>>> (shouldRedirectToTargetUrl(requestCycle, currentUrl, targetUrl))
>>>>> -                               {
>>>>> -                                       redirectTo(targetUrl,
>>>>> requestCycle);
>>>>> -
>>>>> -                                       // note: if we had session here
>>>>> we
>>>>> would render the page to buffer and then
>>>>> -                                       // redirect to URL generated
>>>>> *after* page has been rendered (the statelessness
>>>>> -                                       // may change during render).
>>>>> this
>>>>> would save one redirect because now we have
>>>>> -                                       // to render to URL generated
>>>>> *before* page is rendered, render the page, get
>>>>> -                                       // URL after render and if the
>>>>> URL
>>>>> is different (meaning page is not stateless),
>>>>> -                                       // save the buffer and redirect
>>>>> again (which is pretty much what the next step
>>>>> -                                       // does)
>>>>> -                               }
>>>>> -                               else
>>>>> -                               {
>>>>> -                                       if (isRedirectToBuffer() ==
>>>>> false
>>>>> && logger.isDebugEnabled())
>>>>> -                                       {
>>>>> -                                               String details = String
>>>>> -                                                       .format(
>>>>> -
>>>>> "redirect
>>>>> strategy: '%s', isAjax: '%s', redirect policy: '%s', "
>>>>> -
>>>>>   +
>>>>> "current url: '%s', target url: '%s', is new: '%s', is stateless: '%s',
>>>>> is
>>>>> temporary: '%s'",
>>>>> -
>>>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>>>> -
>>>>> isAjax(requestCycle), getRedirectPolicy(), currentUrl, targetUrl,
>>>>> -
>>>>> isNewPageInstance(), isPageStateless(), isSessionTemporary());
>>>>> -                                               logger
>>>>> -                                                       .debug("Falling
>>>>> back to Redirect_To_Buffer render strategy because none of the
>>>>> conditions "
>>>>> -                                                               +
>>>>> "matched. Details: " + details);
>>>>> -                                       }
>>>>> -
>>>>> -                                       // force creation of possible
>>>>> stateful page to get the final target url
>>>>> -                                       getPage();
>>>>> -
>>>>> -                                       Url beforeRenderUrl =
>>>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>>>> -
>>>>> -                                       // redirect to buffer
>>>>> -                                       BufferedWebResponse response =
>>>>> renderPage(beforeRenderUrl, requestCycle);
>>>>> -
>>>>> -                                       if (response == null)
>>>>> -                                       {
>>>>> -                                               return;
>>>>> -                                       }
>>>>> -
>>>>> -                                       // the url might have changed
>>>>> after page has been rendered (e.g. the
>>>>> -                                       // stateless flag might have
>>>>> changed because stateful components
>>>>> -                                       // were added)
>>>>> -                                       final Url afterRenderUrl =
>>>>> requestCycle
>>>>> -
>>>>> .mapUrlFor(getRenderPageRequestHandler());
>>>>> -
>>>>> -                                       if
>>>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>>>> false)
>>>>> -                                       {
>>>>> -                                               // the amount of
>>>>> segments
>>>>> is different - generated relative URLs
>>>>> -                                               // will not work, we
>>>>> need
>>>>> to rerender the page. This can happen
>>>>> -                                               // with
>>>>> IRequestHandlers
>>>>> that produce different URLs with
>>>>> -                                               // different amount of
>>>>> segments for stateless and stateful pages
>>>>> -                                               response =
>>>>> renderPage(afterRenderUrl, requestCycle);
>>>>> -                                       }
>>>>> -
>>>>> -                                       if
>>>>> (currentUrl.equals(afterRenderUrl))
>>>>> -                                       {
>>>>> -                                               // no need to redirect
>>>>> when both urls are exactly the same
>>>>> -
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> -                                       }
>>>>> -                                       // if page is still stateless
>>>>> after render
>>>>> -                                       else if (isPageStateless() &&
>>>>> !enableRedirectForStatelessPage())
>>>>> -                                       {
>>>>> -                                               // we don't want the
>>>>> redirect to happen for stateless page
>>>>> -                                               // example:
>>>>> -                                               // when a normal
>>>>> mounted
>>>>> stateful page is hit at /mount/point
>>>>> -                                               // wicket renders the
>>>>> page
>>>>> to buffer and redirects to /mount/point?12
>>>>> -                                               // but for stateless
>>>>> page
>>>>> the redirect is not necessary
>>>>> -                                               // also for listener
>>>>> interface on stateful page we want to redirect
>>>>> -                                               // after the listener
>>>>> is
>>>>> invoked, but on stateless page the user
>>>>> -                                               // must ask for
>>>>> redirect
>>>>> explicitly
>>>>> -
>>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>> -                                       }
>>>>> -                                       else
>>>>> -                                       {
>>>>> -
>>>>> storeBufferedResponse(afterRenderUrl, response);
>>>>> -
>>>>> -
>>>>> redirectTo(afterRenderUrl,
>>>>> requestCycle);
>>>>> -                                       }
>>>>> -                               }
>>>>> +                               storeBufferedResponse(afterRenderUrl,
>>>>> response);
>>>>> +
>>>>> +                               redirectTo(afterRenderUrl,
>>>>> requestCycle);
>>>>>                           }
>>>>>                   }
>>>>>           }
>>>>>
>>>>>
>>>>>
>>>>>
>

Re: git commit: Code beautifying: method WebPageRenderer.respond

Posted by Andrea Del Bene <an...@gmail.com>.
I admit I've changed a very sensible part of the code :), however after 
working with it I realized that we could reduce its complexity removing 
a couple of nesting levels "merging" them in the outer if...else block. 
I think this improve code quality and its readability. Let me explain 
you what I did. The code was:

         if (bufferedResponse != null)
         {
             logger
                 .warn("The Buffered response should be handled by 
BufferedResponseRequestHandler");
             // if there is saved response for this URL render it
bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
         }
         else
         {
             if (shouldRenderPageAndWriteResponse(requestCycle, 
currentUrl, targetUrl)) //
             {
                 BufferedWebResponse response = renderPage(currentUrl, 
requestCycle);
                 if (response != null)
                 {
response.writeTo((WebResponse)requestCycle.getResponse());
                 }
             }
             else
             {
                 if (shouldRedirectToTargetUrl(requestCycle, currentUrl, 
targetUrl))
                 {
                     redirectTo(targetUrl, requestCycle);

                     // note: if we had session here we would render the 
page to buffer and then
                     // redirect to URL generated *after* page has been 
rendered (the statelessness
                     // may change during render). this would save one 
redirect because now we have
                     // to render to URL generated *before* page is 
rendered, render the page, get
                     // URL after render and if the URL is different 
(meaning page is not stateless),
                     // save the buffer and redirect again (which is 
pretty much what the next step
                     // does)
                 }
                  else
......etc.....

now is


         if (bufferedResponse != null)
         {
             logger
                 .warn("The Buffered response should be handled by 
BufferedResponseRequestHandler");
             // if there is saved response for this URL render it
bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
         }
         else if (shouldRenderPageAndWriteResponse(requestCycle, 
currentUrl, targetUrl))
         {
             BufferedWebResponse response = renderPage(currentUrl, 
requestCycle);
             if (response != null)
             {
response.writeTo((WebResponse)requestCycle.getResponse());
             }
         }
         else if (shouldRedirectToTargetUrl(requestCycle, currentUrl, 
targetUrl))
         {

             redirectTo(targetUrl, requestCycle);

             // note: if we had session here we would render the page to 
buffer and then
             // redirect to URL generated *after* page has been rendered 
(the statelessness
             // may change during render). this would save one redirect 
because now we have
             // to render to URL generated *before* page is rendered, 
render the page, get
             // URL after render and if the URL is different (meaning 
page is not stateless),
             // save the buffer and redirect again (which is pretty much 
what the next step
             // does)
         }
         else
         {

two of the 'if' statements next to an 'else' have been merged with this one.



On 17/07/2014 16:26, Martin Grigorov wrote:
> what is/was useless ?
> now I am even more confused what your "beautification" did with this really
> important and fragile part of Wicket code :-)
>
> I'm not saying that the new code is badly formatted. I was asking whether
> "beautification" really means "code formatting"
>
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
>
>
> On Thu, Jul 17, 2014 at 5:10 PM, Andrea Del Bene <an...@gmail.com>
> wrote:
>
>> Method respond had high level of "if...else" nesting, a couple of them
>> useless. in which way the new code doesn't respect Wicket's code format?
>> Maybe my IDE did something wrong.
>>
>>   Hi Andrea,
>>> On Wed, Jul 16, 2014 at 8:33 PM, <ad...@apache.org> wrote:
>>>
>>>   Repository: wicket
>>>> Updated Branches:
>>>>     refs/heads/master c24d830cd -> bb9c1044e
>>>>
>>>>
>>>> Code beautifying: method WebPageRenderer.respond
>>>>
>>>>   What exactly "beautifying" means ?
>>> It is a bit hard to see what is the actual change. It doesn't look like
>>> applying Wicket's code format.
>>>
>>>
>>>   Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>>>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/bb9c1044
>>>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/bb9c1044
>>>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/bb9c1044
>>>>
>>>> Branch: refs/heads/master
>>>> Commit: bb9c1044e5f1ba8902aa69073ef9fb236802d277
>>>> Parents: c24d830
>>>> Author: andrea del bene <ad...@apache.org>
>>>> Authored: Wed Jul 16 19:33:16 2014 +0200
>>>> Committer: andrea del bene <ad...@apache.org>
>>>> Committed: Wed Jul 16 19:33:16 2014 +0200
>>>>
>>>> ----------------------------------------------------------------------
>>>>    .../request/handler/render/WebPageRenderer.java | 169
>>>> +++++++++----------
>>>>    1 file changed, 82 insertions(+), 87 deletions(-)
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/wicket/blob/
>>>> bb9c1044/wicket-core/src/main/java/org/apache/wicket/
>>>> request/handler/render/WebPageRenderer.java
>>>> ----------------------------------------------------------------------
>>>> diff --git
>>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>> WebPageRenderer.java
>>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>> WebPageRenderer.java
>>>> index 0b5dee4..af9dbee 100644
>>>> ---
>>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>> WebPageRenderer.java
>>>> +++
>>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>>> WebPageRenderer.java
>>>> @@ -198,99 +198,94 @@ public class WebPageRenderer extends PageRenderer
>>>>                           // if there is saved response for this URL
>>>> render
>>>> it
>>>>
>>>> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>>>>                   }
>>>> +               else if (shouldRenderPageAndWriteResponse(requestCycle,
>>>> currentUrl, targetUrl))
>>>> +               {
>>>> +                       BufferedWebResponse response =
>>>> renderPage(currentUrl, requestCycle);
>>>> +                       if (response != null)
>>>> +                       {
>>>> +
>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>> +                       }
>>>> +               }
>>>> +               else if (shouldRedirectToTargetUrl(requestCycle,
>>>> currentUrl, targetUrl))
>>>> +               {
>>>> +
>>>> +                       redirectTo(targetUrl, requestCycle);
>>>> +
>>>> +                       // note: if we had session here we would render
>>>> the page to buffer and then
>>>> +                       // redirect to URL generated *after* page has
>>>> been
>>>> rendered (the statelessness
>>>> +                       // may change during render). this would save one
>>>> redirect because now we have
>>>> +                       // to render to URL generated *before* page is
>>>> rendered, render the page, get
>>>> +                       // URL after render and if the URL is different
>>>> (meaning page is not stateless),
>>>> +                       // save the buffer and redirect again (which is
>>>> pretty much what the next step
>>>> +                       // does)
>>>> +               }
>>>>                   else
>>>>                   {
>>>> -                       if (shouldRenderPageAndWriteRespon
>>>> se(requestCycle,
>>>> currentUrl, targetUrl)) //
>>>> +                       if (isRedirectToBuffer() == false &&
>>>> logger.isDebugEnabled())
>>>> +                       {
>>>> +                               String details = String
>>>> +                                       .format(
>>>> +                                               "redirect strategy: '%s',
>>>> isAjax: '%s', redirect policy: '%s', "
>>>> +                                                       + "current url:
>>>> '%s', target url: '%s', is new: '%s', is stateless: '%s', is temporary:
>>>> '%s'",
>>>> +
>>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>>> +                                               isAjax(requestCycle),
>>>> getRedirectPolicy(), currentUrl, targetUrl,
>>>> +                                               isNewPageInstance(),
>>>> isPageStateless(), isSessionTemporary());
>>>> +                               logger
>>>> +                                       .debug("Falling back to
>>>> Redirect_To_Buffer render strategy because none of the conditions "
>>>> +                                               + "matched. Details: " +
>>>> details);
>>>> +                       }
>>>> +
>>>> +                       // force creation of possible stateful page to
>>>> get
>>>> the final target url
>>>> +                       getPage();
>>>> +
>>>> +                       Url beforeRenderUrl =
>>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>>> +
>>>> +                       // redirect to buffer
>>>> +                       BufferedWebResponse response =
>>>> renderPage(beforeRenderUrl, requestCycle);
>>>> +
>>>> +                       if (response == null)
>>>> +                       {
>>>> +                               return;
>>>> +                       }
>>>> +
>>>> +                       // the url might have changed after page has been
>>>> rendered (e.g. the
>>>> +                       // stateless flag might have changed because
>>>> stateful components
>>>> +                       // were added)
>>>> +                       final Url afterRenderUrl = requestCycle
>>>> +                               .mapUrlFor(
>>>> getRenderPageRequestHandler());
>>>> +
>>>> +                       if
>>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>>> false)
>>>>                           {
>>>> -                               BufferedWebResponse response =
>>>> renderPage(currentUrl, requestCycle);
>>>> -                               if (response != null)
>>>> -                               {
>>>> -
>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>> -                               }
>>>> +                               // the amount of segments is different -
>>>> generated relative URLs
>>>> +                               // will not work, we need to rerender the
>>>> page. This can happen
>>>> +                               // with IRequestHandlers that produce
>>>> different URLs with
>>>> +                               // different amount of segments for
>>>> stateless and stateful pages
>>>> +                               response = renderPage(afterRenderUrl,
>>>> requestCycle);
>>>> +                       }
>>>> +
>>>> +                       if (currentUrl.equals(afterRenderUrl))
>>>> +                       {
>>>> +                               // no need to redirect when both urls are
>>>> exactly the same
>>>> +
>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>> +                       }
>>>> +                       // if page is still stateless after render
>>>> +                       else if (isPageStateless() &&
>>>> !enableRedirectForStatelessPage())
>>>> +                       {
>>>> +                               // we don't want the redirect to happen
>>>> for stateless page
>>>> +                               // example:
>>>> +                               // when a normal mounted stateful page is
>>>> hit at /mount/point
>>>> +                               // wicket renders the page to buffer and
>>>> redirects to /mount/point?12
>>>> +                               // but for stateless page the redirect is
>>>> not necessary
>>>> +                               // also for listener interface on
>>>> stateful
>>>> page we want to redirect
>>>> +                               // after the listener is invoked, but on
>>>> stateless page the user
>>>> +                               // must ask for redirect explicitly
>>>> +
>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>>                           }
>>>>                           else
>>>>                           {
>>>> -                               if
>>>> (shouldRedirectToTargetUrl(requestCycle, currentUrl, targetUrl))
>>>> -                               {
>>>> -                                       redirectTo(targetUrl,
>>>> requestCycle);
>>>> -
>>>> -                                       // note: if we had session here
>>>> we
>>>> would render the page to buffer and then
>>>> -                                       // redirect to URL generated
>>>> *after* page has been rendered (the statelessness
>>>> -                                       // may change during render).
>>>> this
>>>> would save one redirect because now we have
>>>> -                                       // to render to URL generated
>>>> *before* page is rendered, render the page, get
>>>> -                                       // URL after render and if the
>>>> URL
>>>> is different (meaning page is not stateless),
>>>> -                                       // save the buffer and redirect
>>>> again (which is pretty much what the next step
>>>> -                                       // does)
>>>> -                               }
>>>> -                               else
>>>> -                               {
>>>> -                                       if (isRedirectToBuffer() == false
>>>> && logger.isDebugEnabled())
>>>> -                                       {
>>>> -                                               String details = String
>>>> -                                                       .format(
>>>> -                                                               "redirect
>>>> strategy: '%s', isAjax: '%s', redirect policy: '%s', "
>>>> -                                                                       +
>>>> "current url: '%s', target url: '%s', is new: '%s', is stateless: '%s',
>>>> is
>>>> temporary: '%s'",
>>>> -
>>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>>> -
>>>> isAjax(requestCycle), getRedirectPolicy(), currentUrl, targetUrl,
>>>> -
>>>> isNewPageInstance(), isPageStateless(), isSessionTemporary());
>>>> -                                               logger
>>>> -                                                       .debug("Falling
>>>> back to Redirect_To_Buffer render strategy because none of the
>>>> conditions "
>>>> -                                                               +
>>>> "matched. Details: " + details);
>>>> -                                       }
>>>> -
>>>> -                                       // force creation of possible
>>>> stateful page to get the final target url
>>>> -                                       getPage();
>>>> -
>>>> -                                       Url beforeRenderUrl =
>>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>>> -
>>>> -                                       // redirect to buffer
>>>> -                                       BufferedWebResponse response =
>>>> renderPage(beforeRenderUrl, requestCycle);
>>>> -
>>>> -                                       if (response == null)
>>>> -                                       {
>>>> -                                               return;
>>>> -                                       }
>>>> -
>>>> -                                       // the url might have changed
>>>> after page has been rendered (e.g. the
>>>> -                                       // stateless flag might have
>>>> changed because stateful components
>>>> -                                       // were added)
>>>> -                                       final Url afterRenderUrl =
>>>> requestCycle
>>>> -
>>>> .mapUrlFor(getRenderPageRequestHandler());
>>>> -
>>>> -                                       if
>>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>>> false)
>>>> -                                       {
>>>> -                                               // the amount of segments
>>>> is different - generated relative URLs
>>>> -                                               // will not work, we need
>>>> to rerender the page. This can happen
>>>> -                                               // with IRequestHandlers
>>>> that produce different URLs with
>>>> -                                               // different amount of
>>>> segments for stateless and stateful pages
>>>> -                                               response =
>>>> renderPage(afterRenderUrl, requestCycle);
>>>> -                                       }
>>>> -
>>>> -                                       if
>>>> (currentUrl.equals(afterRenderUrl))
>>>> -                                       {
>>>> -                                               // no need to redirect
>>>> when both urls are exactly the same
>>>> -
>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>> -                                       }
>>>> -                                       // if page is still stateless
>>>> after render
>>>> -                                       else if (isPageStateless() &&
>>>> !enableRedirectForStatelessPage())
>>>> -                                       {
>>>> -                                               // we don't want the
>>>> redirect to happen for stateless page
>>>> -                                               // example:
>>>> -                                               // when a normal mounted
>>>> stateful page is hit at /mount/point
>>>> -                                               // wicket renders the
>>>> page
>>>> to buffer and redirects to /mount/point?12
>>>> -                                               // but for stateless page
>>>> the redirect is not necessary
>>>> -                                               // also for listener
>>>> interface on stateful page we want to redirect
>>>> -                                               // after the listener is
>>>> invoked, but on stateless page the user
>>>> -                                               // must ask for redirect
>>>> explicitly
>>>> -
>>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>> -                                       }
>>>> -                                       else
>>>> -                                       {
>>>> -
>>>> storeBufferedResponse(afterRenderUrl, response);
>>>> -
>>>> -
>>>> redirectTo(afterRenderUrl,
>>>> requestCycle);
>>>> -                                       }
>>>> -                               }
>>>> +                               storeBufferedResponse(afterRenderUrl,
>>>> response);
>>>> +
>>>> +                               redirectTo(afterRenderUrl, requestCycle);
>>>>                           }
>>>>                   }
>>>>           }
>>>>
>>>>
>>>>


Re: git commit: Code beautifying: method WebPageRenderer.respond

Posted by Martin Grigorov <mg...@apache.org>.
what is/was useless ?
now I am even more confused what your "beautification" did with this really
important and fragile part of Wicket code :-)

I'm not saying that the new code is badly formatted. I was asking whether
"beautification" really means "code formatting"

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Thu, Jul 17, 2014 at 5:10 PM, Andrea Del Bene <an...@gmail.com>
wrote:

>
> Method respond had high level of "if...else" nesting, a couple of them
> useless. in which way the new code doesn't respect Wicket's code format?
> Maybe my IDE did something wrong.
>
>  Hi Andrea,
>>
>> On Wed, Jul 16, 2014 at 8:33 PM, <ad...@apache.org> wrote:
>>
>>  Repository: wicket
>>> Updated Branches:
>>>    refs/heads/master c24d830cd -> bb9c1044e
>>>
>>>
>>> Code beautifying: method WebPageRenderer.respond
>>>
>>>  What exactly "beautifying" means ?
>> It is a bit hard to see what is the actual change. It doesn't look like
>> applying Wicket's code format.
>>
>>
>>  Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/bb9c1044
>>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/bb9c1044
>>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/bb9c1044
>>>
>>> Branch: refs/heads/master
>>> Commit: bb9c1044e5f1ba8902aa69073ef9fb236802d277
>>> Parents: c24d830
>>> Author: andrea del bene <ad...@apache.org>
>>> Authored: Wed Jul 16 19:33:16 2014 +0200
>>> Committer: andrea del bene <ad...@apache.org>
>>> Committed: Wed Jul 16 19:33:16 2014 +0200
>>>
>>> ----------------------------------------------------------------------
>>>   .../request/handler/render/WebPageRenderer.java | 169
>>> +++++++++----------
>>>   1 file changed, 82 insertions(+), 87 deletions(-)
>>> ----------------------------------------------------------------------
>>>
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/wicket/blob/
>>> bb9c1044/wicket-core/src/main/java/org/apache/wicket/
>>> request/handler/render/WebPageRenderer.java
>>> ----------------------------------------------------------------------
>>> diff --git
>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>> WebPageRenderer.java
>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>> WebPageRenderer.java
>>> index 0b5dee4..af9dbee 100644
>>> ---
>>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>> WebPageRenderer.java
>>> +++
>>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/
>>> WebPageRenderer.java
>>> @@ -198,99 +198,94 @@ public class WebPageRenderer extends PageRenderer
>>>                          // if there is saved response for this URL
>>> render
>>> it
>>>
>>> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>>>                  }
>>> +               else if (shouldRenderPageAndWriteResponse(requestCycle,
>>> currentUrl, targetUrl))
>>> +               {
>>> +                       BufferedWebResponse response =
>>> renderPage(currentUrl, requestCycle);
>>> +                       if (response != null)
>>> +                       {
>>> +
>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>> +                       }
>>> +               }
>>> +               else if (shouldRedirectToTargetUrl(requestCycle,
>>> currentUrl, targetUrl))
>>> +               {
>>> +
>>> +                       redirectTo(targetUrl, requestCycle);
>>> +
>>> +                       // note: if we had session here we would render
>>> the page to buffer and then
>>> +                       // redirect to URL generated *after* page has
>>> been
>>> rendered (the statelessness
>>> +                       // may change during render). this would save one
>>> redirect because now we have
>>> +                       // to render to URL generated *before* page is
>>> rendered, render the page, get
>>> +                       // URL after render and if the URL is different
>>> (meaning page is not stateless),
>>> +                       // save the buffer and redirect again (which is
>>> pretty much what the next step
>>> +                       // does)
>>> +               }
>>>                  else
>>>                  {
>>> -                       if (shouldRenderPageAndWriteRespon
>>> se(requestCycle,
>>> currentUrl, targetUrl)) //
>>> +                       if (isRedirectToBuffer() == false &&
>>> logger.isDebugEnabled())
>>> +                       {
>>> +                               String details = String
>>> +                                       .format(
>>> +                                               "redirect strategy: '%s',
>>> isAjax: '%s', redirect policy: '%s', "
>>> +                                                       + "current url:
>>> '%s', target url: '%s', is new: '%s', is stateless: '%s', is temporary:
>>> '%s'",
>>> +
>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>> +                                               isAjax(requestCycle),
>>> getRedirectPolicy(), currentUrl, targetUrl,
>>> +                                               isNewPageInstance(),
>>> isPageStateless(), isSessionTemporary());
>>> +                               logger
>>> +                                       .debug("Falling back to
>>> Redirect_To_Buffer render strategy because none of the conditions "
>>> +                                               + "matched. Details: " +
>>> details);
>>> +                       }
>>> +
>>> +                       // force creation of possible stateful page to
>>> get
>>> the final target url
>>> +                       getPage();
>>> +
>>> +                       Url beforeRenderUrl =
>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>> +
>>> +                       // redirect to buffer
>>> +                       BufferedWebResponse response =
>>> renderPage(beforeRenderUrl, requestCycle);
>>> +
>>> +                       if (response == null)
>>> +                       {
>>> +                               return;
>>> +                       }
>>> +
>>> +                       // the url might have changed after page has been
>>> rendered (e.g. the
>>> +                       // stateless flag might have changed because
>>> stateful components
>>> +                       // were added)
>>> +                       final Url afterRenderUrl = requestCycle
>>> +                               .mapUrlFor(
>>> getRenderPageRequestHandler());
>>> +
>>> +                       if
>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>> false)
>>>                          {
>>> -                               BufferedWebResponse response =
>>> renderPage(currentUrl, requestCycle);
>>> -                               if (response != null)
>>> -                               {
>>> -
>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>> -                               }
>>> +                               // the amount of segments is different -
>>> generated relative URLs
>>> +                               // will not work, we need to rerender the
>>> page. This can happen
>>> +                               // with IRequestHandlers that produce
>>> different URLs with
>>> +                               // different amount of segments for
>>> stateless and stateful pages
>>> +                               response = renderPage(afterRenderUrl,
>>> requestCycle);
>>> +                       }
>>> +
>>> +                       if (currentUrl.equals(afterRenderUrl))
>>> +                       {
>>> +                               // no need to redirect when both urls are
>>> exactly the same
>>> +
>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>> +                       }
>>> +                       // if page is still stateless after render
>>> +                       else if (isPageStateless() &&
>>> !enableRedirectForStatelessPage())
>>> +                       {
>>> +                               // we don't want the redirect to happen
>>> for stateless page
>>> +                               // example:
>>> +                               // when a normal mounted stateful page is
>>> hit at /mount/point
>>> +                               // wicket renders the page to buffer and
>>> redirects to /mount/point?12
>>> +                               // but for stateless page the redirect is
>>> not necessary
>>> +                               // also for listener interface on
>>> stateful
>>> page we want to redirect
>>> +                               // after the listener is invoked, but on
>>> stateless page the user
>>> +                               // must ask for redirect explicitly
>>> +
>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>>                          }
>>>                          else
>>>                          {
>>> -                               if
>>> (shouldRedirectToTargetUrl(requestCycle, currentUrl, targetUrl))
>>> -                               {
>>> -                                       redirectTo(targetUrl,
>>> requestCycle);
>>> -
>>> -                                       // note: if we had session here
>>> we
>>> would render the page to buffer and then
>>> -                                       // redirect to URL generated
>>> *after* page has been rendered (the statelessness
>>> -                                       // may change during render).
>>> this
>>> would save one redirect because now we have
>>> -                                       // to render to URL generated
>>> *before* page is rendered, render the page, get
>>> -                                       // URL after render and if the
>>> URL
>>> is different (meaning page is not stateless),
>>> -                                       // save the buffer and redirect
>>> again (which is pretty much what the next step
>>> -                                       // does)
>>> -                               }
>>> -                               else
>>> -                               {
>>> -                                       if (isRedirectToBuffer() == false
>>> && logger.isDebugEnabled())
>>> -                                       {
>>> -                                               String details = String
>>> -                                                       .format(
>>> -                                                               "redirect
>>> strategy: '%s', isAjax: '%s', redirect policy: '%s', "
>>> -                                                                       +
>>> "current url: '%s', target url: '%s', is new: '%s', is stateless: '%s',
>>> is
>>> temporary: '%s'",
>>> -
>>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>>> -
>>> isAjax(requestCycle), getRedirectPolicy(), currentUrl, targetUrl,
>>> -
>>> isNewPageInstance(), isPageStateless(), isSessionTemporary());
>>> -                                               logger
>>> -                                                       .debug("Falling
>>> back to Redirect_To_Buffer render strategy because none of the
>>> conditions "
>>> -                                                               +
>>> "matched. Details: " + details);
>>> -                                       }
>>> -
>>> -                                       // force creation of possible
>>> stateful page to get the final target url
>>> -                                       getPage();
>>> -
>>> -                                       Url beforeRenderUrl =
>>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>>> -
>>> -                                       // redirect to buffer
>>> -                                       BufferedWebResponse response =
>>> renderPage(beforeRenderUrl, requestCycle);
>>> -
>>> -                                       if (response == null)
>>> -                                       {
>>> -                                               return;
>>> -                                       }
>>> -
>>> -                                       // the url might have changed
>>> after page has been rendered (e.g. the
>>> -                                       // stateless flag might have
>>> changed because stateful components
>>> -                                       // were added)
>>> -                                       final Url afterRenderUrl =
>>> requestCycle
>>> -
>>> .mapUrlFor(getRenderPageRequestHandler());
>>> -
>>> -                                       if
>>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>>> false)
>>> -                                       {
>>> -                                               // the amount of segments
>>> is different - generated relative URLs
>>> -                                               // will not work, we need
>>> to rerender the page. This can happen
>>> -                                               // with IRequestHandlers
>>> that produce different URLs with
>>> -                                               // different amount of
>>> segments for stateless and stateful pages
>>> -                                               response =
>>> renderPage(afterRenderUrl, requestCycle);
>>> -                                       }
>>> -
>>> -                                       if
>>> (currentUrl.equals(afterRenderUrl))
>>> -                                       {
>>> -                                               // no need to redirect
>>> when both urls are exactly the same
>>> -
>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>> -                                       }
>>> -                                       // if page is still stateless
>>> after render
>>> -                                       else if (isPageStateless() &&
>>> !enableRedirectForStatelessPage())
>>> -                                       {
>>> -                                               // we don't want the
>>> redirect to happen for stateless page
>>> -                                               // example:
>>> -                                               // when a normal mounted
>>> stateful page is hit at /mount/point
>>> -                                               // wicket renders the
>>> page
>>> to buffer and redirects to /mount/point?12
>>> -                                               // but for stateless page
>>> the redirect is not necessary
>>> -                                               // also for listener
>>> interface on stateful page we want to redirect
>>> -                                               // after the listener is
>>> invoked, but on stateless page the user
>>> -                                               // must ask for redirect
>>> explicitly
>>> -
>>> response.writeTo((WebResponse)requestCycle.getResponse());
>>> -                                       }
>>> -                                       else
>>> -                                       {
>>> -
>>> storeBufferedResponse(afterRenderUrl, response);
>>> -
>>> -
>>> redirectTo(afterRenderUrl,
>>> requestCycle);
>>> -                                       }
>>> -                               }
>>> +                               storeBufferedResponse(afterRenderUrl,
>>> response);
>>> +
>>> +                               redirectTo(afterRenderUrl, requestCycle);
>>>                          }
>>>                  }
>>>          }
>>>
>>>
>>>
>

Re: git commit: Code beautifying: method WebPageRenderer.respond

Posted by Andrea Del Bene <an...@gmail.com>.
Method respond had high level of "if...else" nesting, a couple of them 
useless. in which way the new code doesn't respect Wicket's code format? 
Maybe my IDE did something wrong.
> Hi Andrea,
>
> On Wed, Jul 16, 2014 at 8:33 PM, <ad...@apache.org> wrote:
>
>> Repository: wicket
>> Updated Branches:
>>    refs/heads/master c24d830cd -> bb9c1044e
>>
>>
>> Code beautifying: method WebPageRenderer.respond
>>
> What exactly "beautifying" means ?
> It is a bit hard to see what is the actual change. It doesn't look like
> applying Wicket's code format.
>
>
>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/bb9c1044
>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/bb9c1044
>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/bb9c1044
>>
>> Branch: refs/heads/master
>> Commit: bb9c1044e5f1ba8902aa69073ef9fb236802d277
>> Parents: c24d830
>> Author: andrea del bene <ad...@apache.org>
>> Authored: Wed Jul 16 19:33:16 2014 +0200
>> Committer: andrea del bene <ad...@apache.org>
>> Committed: Wed Jul 16 19:33:16 2014 +0200
>>
>> ----------------------------------------------------------------------
>>   .../request/handler/render/WebPageRenderer.java | 169 +++++++++----------
>>   1 file changed, 82 insertions(+), 87 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/bb9c1044/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
>> index 0b5dee4..af9dbee 100644
>> ---
>> a/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
>> +++
>> b/wicket-core/src/main/java/org/apache/wicket/request/handler/render/WebPageRenderer.java
>> @@ -198,99 +198,94 @@ public class WebPageRenderer extends PageRenderer
>>                          // if there is saved response for this URL render
>> it
>>
>> bufferedResponse.writeTo((WebResponse)requestCycle.getResponse());
>>                  }
>> +               else if (shouldRenderPageAndWriteResponse(requestCycle,
>> currentUrl, targetUrl))
>> +               {
>> +                       BufferedWebResponse response =
>> renderPage(currentUrl, requestCycle);
>> +                       if (response != null)
>> +                       {
>> +
>> response.writeTo((WebResponse)requestCycle.getResponse());
>> +                       }
>> +               }
>> +               else if (shouldRedirectToTargetUrl(requestCycle,
>> currentUrl, targetUrl))
>> +               {
>> +
>> +                       redirectTo(targetUrl, requestCycle);
>> +
>> +                       // note: if we had session here we would render
>> the page to buffer and then
>> +                       // redirect to URL generated *after* page has been
>> rendered (the statelessness
>> +                       // may change during render). this would save one
>> redirect because now we have
>> +                       // to render to URL generated *before* page is
>> rendered, render the page, get
>> +                       // URL after render and if the URL is different
>> (meaning page is not stateless),
>> +                       // save the buffer and redirect again (which is
>> pretty much what the next step
>> +                       // does)
>> +               }
>>                  else
>>                  {
>> -                       if (shouldRenderPageAndWriteResponse(requestCycle,
>> currentUrl, targetUrl)) //
>> +                       if (isRedirectToBuffer() == false &&
>> logger.isDebugEnabled())
>> +                       {
>> +                               String details = String
>> +                                       .format(
>> +                                               "redirect strategy: '%s',
>> isAjax: '%s', redirect policy: '%s', "
>> +                                                       + "current url:
>> '%s', target url: '%s', is new: '%s', is stateless: '%s', is temporary:
>> '%s'",
>> +
>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>> +                                               isAjax(requestCycle),
>> getRedirectPolicy(), currentUrl, targetUrl,
>> +                                               isNewPageInstance(),
>> isPageStateless(), isSessionTemporary());
>> +                               logger
>> +                                       .debug("Falling back to
>> Redirect_To_Buffer render strategy because none of the conditions "
>> +                                               + "matched. Details: " +
>> details);
>> +                       }
>> +
>> +                       // force creation of possible stateful page to get
>> the final target url
>> +                       getPage();
>> +
>> +                       Url beforeRenderUrl =
>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>> +
>> +                       // redirect to buffer
>> +                       BufferedWebResponse response =
>> renderPage(beforeRenderUrl, requestCycle);
>> +
>> +                       if (response == null)
>> +                       {
>> +                               return;
>> +                       }
>> +
>> +                       // the url might have changed after page has been
>> rendered (e.g. the
>> +                       // stateless flag might have changed because
>> stateful components
>> +                       // were added)
>> +                       final Url afterRenderUrl = requestCycle
>> +                               .mapUrlFor(getRenderPageRequestHandler());
>> +
>> +                       if
>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>> false)
>>                          {
>> -                               BufferedWebResponse response =
>> renderPage(currentUrl, requestCycle);
>> -                               if (response != null)
>> -                               {
>> -
>> response.writeTo((WebResponse)requestCycle.getResponse());
>> -                               }
>> +                               // the amount of segments is different -
>> generated relative URLs
>> +                               // will not work, we need to rerender the
>> page. This can happen
>> +                               // with IRequestHandlers that produce
>> different URLs with
>> +                               // different amount of segments for
>> stateless and stateful pages
>> +                               response = renderPage(afterRenderUrl,
>> requestCycle);
>> +                       }
>> +
>> +                       if (currentUrl.equals(afterRenderUrl))
>> +                       {
>> +                               // no need to redirect when both urls are
>> exactly the same
>> +
>> response.writeTo((WebResponse)requestCycle.getResponse());
>> +                       }
>> +                       // if page is still stateless after render
>> +                       else if (isPageStateless() &&
>> !enableRedirectForStatelessPage())
>> +                       {
>> +                               // we don't want the redirect to happen
>> for stateless page
>> +                               // example:
>> +                               // when a normal mounted stateful page is
>> hit at /mount/point
>> +                               // wicket renders the page to buffer and
>> redirects to /mount/point?12
>> +                               // but for stateless page the redirect is
>> not necessary
>> +                               // also for listener interface on stateful
>> page we want to redirect
>> +                               // after the listener is invoked, but on
>> stateless page the user
>> +                               // must ask for redirect explicitly
>> +
>> response.writeTo((WebResponse)requestCycle.getResponse());
>>                          }
>>                          else
>>                          {
>> -                               if
>> (shouldRedirectToTargetUrl(requestCycle, currentUrl, targetUrl))
>> -                               {
>> -                                       redirectTo(targetUrl,
>> requestCycle);
>> -
>> -                                       // note: if we had session here we
>> would render the page to buffer and then
>> -                                       // redirect to URL generated
>> *after* page has been rendered (the statelessness
>> -                                       // may change during render). this
>> would save one redirect because now we have
>> -                                       // to render to URL generated
>> *before* page is rendered, render the page, get
>> -                                       // URL after render and if the URL
>> is different (meaning page is not stateless),
>> -                                       // save the buffer and redirect
>> again (which is pretty much what the next step
>> -                                       // does)
>> -                               }
>> -                               else
>> -                               {
>> -                                       if (isRedirectToBuffer() == false
>> && logger.isDebugEnabled())
>> -                                       {
>> -                                               String details = String
>> -                                                       .format(
>> -                                                               "redirect
>> strategy: '%s', isAjax: '%s', redirect policy: '%s', "
>> -                                                                       +
>> "current url: '%s', target url: '%s', is new: '%s', is stateless: '%s', is
>> temporary: '%s'",
>> -
>> Application.get().getRequestCycleSettings().getRenderStrategy(),
>> -
>> isAjax(requestCycle), getRedirectPolicy(), currentUrl, targetUrl,
>> -
>> isNewPageInstance(), isPageStateless(), isSessionTemporary());
>> -                                               logger
>> -                                                       .debug("Falling
>> back to Redirect_To_Buffer render strategy because none of the conditions "
>> -                                                               +
>> "matched. Details: " + details);
>> -                                       }
>> -
>> -                                       // force creation of possible
>> stateful page to get the final target url
>> -                                       getPage();
>> -
>> -                                       Url beforeRenderUrl =
>> requestCycle.mapUrlFor(getRenderPageRequestHandler());
>> -
>> -                                       // redirect to buffer
>> -                                       BufferedWebResponse response =
>> renderPage(beforeRenderUrl, requestCycle);
>> -
>> -                                       if (response == null)
>> -                                       {
>> -                                               return;
>> -                                       }
>> -
>> -                                       // the url might have changed
>> after page has been rendered (e.g. the
>> -                                       // stateless flag might have
>> changed because stateful components
>> -                                       // were added)
>> -                                       final Url afterRenderUrl =
>> requestCycle
>> -
>> .mapUrlFor(getRenderPageRequestHandler());
>> -
>> -                                       if
>> (beforeRenderUrl.getSegments().equals(afterRenderUrl.getSegments()) ==
>> false)
>> -                                       {
>> -                                               // the amount of segments
>> is different - generated relative URLs
>> -                                               // will not work, we need
>> to rerender the page. This can happen
>> -                                               // with IRequestHandlers
>> that produce different URLs with
>> -                                               // different amount of
>> segments for stateless and stateful pages
>> -                                               response =
>> renderPage(afterRenderUrl, requestCycle);
>> -                                       }
>> -
>> -                                       if
>> (currentUrl.equals(afterRenderUrl))
>> -                                       {
>> -                                               // no need to redirect
>> when both urls are exactly the same
>> -
>> response.writeTo((WebResponse)requestCycle.getResponse());
>> -                                       }
>> -                                       // if page is still stateless
>> after render
>> -                                       else if (isPageStateless() &&
>> !enableRedirectForStatelessPage())
>> -                                       {
>> -                                               // we don't want the
>> redirect to happen for stateless page
>> -                                               // example:
>> -                                               // when a normal mounted
>> stateful page is hit at /mount/point
>> -                                               // wicket renders the page
>> to buffer and redirects to /mount/point?12
>> -                                               // but for stateless page
>> the redirect is not necessary
>> -                                               // also for listener
>> interface on stateful page we want to redirect
>> -                                               // after the listener is
>> invoked, but on stateless page the user
>> -                                               // must ask for redirect
>> explicitly
>> -
>> response.writeTo((WebResponse)requestCycle.getResponse());
>> -                                       }
>> -                                       else
>> -                                       {
>> -
>> storeBufferedResponse(afterRenderUrl, response);
>> -
>> -                                               redirectTo(afterRenderUrl,
>> requestCycle);
>> -                                       }
>> -                               }
>> +                               storeBufferedResponse(afterRenderUrl,
>> response);
>> +
>> +                               redirectTo(afterRenderUrl, requestCycle);
>>                          }
>>                  }
>>          }
>>
>>