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 <[email protected]> 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, <[email protected]> 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 <[email protected]> >>> Authored: Wed Jul 16 19:33:16 2014 +0200 >>> Committer: andrea del bene <[email protected]> >>> 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); >>> } >>> } >>> } >>> >>> >>> >
