Re: wicket git commit: simplify filter handling: don't pass the chain around, use the boolean return in #doFilter() instead

Fri, 09 Oct 2015 04:57:56 -0700 

Hi Emond,

thanks for the clarification.
I've added a comment to the code, so I won't try to change this again next year ;). 

Regards
Sven

On 09.10.2015 13:27, Emond Papegaaij wrote:

Hi Sven,

 From what I see in your commit, the chain is no longer called from within the
Wicket context. We use nested filters that need access to parts of the Wicket
context.

Best regards,
Emond

On Friday 09 October 2015 12:20:47 Sven Meier wrote:

Hi,

forwarding to the chain is just located in a central place now:

      @Override
      public void doFilter(final ServletRequest request, final
ServletResponse response,
          final FilterChain chain) throws IOException, ServletException
      {
          if (!processRequest(request, response)) {
              chain.doFilter(request, response);
          }
      }

Please explain - how does this prevent anything?

Have fun
Sven

On 09.10.2015 10:12, Martin Grigorov wrote:

Hi Sven,

Why this change?
This makes it impossible to use another Filter after WicketFilter?

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

On Fri, Oct 9, 2015 at 10:10 AM, <svenme...@apache.org> wrote:

Repository: wicket

Updated Branches:
    refs/heads/master 3b54aa802 -> 0d5011480

simplify filter handling: don't pass the chain around, use the boolean
return in #doFilter() instead

Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/0d501148
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/0d501148
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/0d501148

Branch: refs/heads/master
Commit: 0d50114800b44c888ec3d719e06427d4f322925c
Parents: 3b54aa8
Author: Sven Meier <svenme...@apache.org>
Authored: Fri Oct 9 10:08:44 2015 +0200
Committer: Sven Meier <svenme...@apache.org>
Committed: Fri Oct 9 10:08:44 2015 +0200

----------------------------------------------------------------------

   .../wicket/protocol/http/WicketFilter.java      | 24
   +++++++-------------
   .../wicket/protocol/http/WicketServlet.java     |  4 ++--
   .../wicket/protocol/http/WicketFilterTest.java  |  8 ++-----
   3 files changed, 12 insertions(+), 24 deletions(-)

----------------------------------------------------------------------



http://git-wip-us.apache.org/repos/asf/wicket/blob/0d501148/wicket-core/s
rc/main/java/org/apache/wicket/protocol/http/WicketFilter.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.
java
b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter
.java index d73a4a7..197603d 100644
---
a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.
java +++
b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.
java @@ -132,13 +132,12 @@ public class WicketFilter implements Filter

           *
           * @param request
           * @param response

-        * @param chain

           * @return false, if the request could not be processed
           * @throws IOException
           * @throws ServletException
           */

-       boolean processRequest(ServletRequest request, final
ServletResponse response,
-               final FilterChain chain) throws IOException,
ServletException
+       boolean processRequest(ServletRequest request, final
ServletResponse response)
+               throws IOException, ServletException

          {
final ThreadContext previousThreadContext = 

ThreadContext.detach();

@@ -169,10 +168,6 @@ public class WicketFilter implements Filter

                          if (shouldIgnorePath(httpServletRequest))
                          {
log.debug("Ignoring request {}", 

httpServletRequest.getRequestURL());
-                               if (chain != null)
-                               {
-                                       chain.doFilter(request,
response);
-                               }

                                  return false;
} 

@@ -201,7 +196,7 @@ public class WicketFilter implements Filter

                                  RequestCycle requestCycle =

application.createRequestCycle(webRequest, webResponse);

                                  res = processRequestCycle(requestCycle,

webResponse, httpServletRequest,
-                                       httpServletResponse, chain);
+                                       httpServletResponse);

                          }
                          else
                          {

@@ -246,14 +241,13 @@ public class WicketFilter implements Filter

           * @param webResponse
           * @param httpServletRequest
           * @param httpServletResponse

-        * @param chain

           * @return false, if the request could not be processed
           * @throws IOException
           * @throws ServletException
           */
protected boolean processRequestCycle(RequestCycle requestCycle, 

WebResponse webResponse,
-               HttpServletRequest httpServletRequest,
HttpServletResponse
httpServletResponse,
-               final FilterChain chain) throws IOException,
ServletException
+               HttpServletRequest httpServletRequest,
HttpServletResponse
httpServletResponse)
+                       throws IOException, ServletException

          {
// Assume we are able to handle the request 
                  boolean res = true;

@@ -264,10 +258,6 @@ public class WicketFilter implements Filter

                  }
                  else
                  {

-                       if (chain != null)
-                       {
-                               chain.doFilter(httpServletRequest,
httpServletResponse);
-                       }

                          res = false;
} 
                  return res;

@@ -281,7 +271,9 @@ public class WicketFilter implements Filter

          public void doFilter(final ServletRequest request, final

ServletResponse response,

                  final FilterChain chain) throws IOException,

ServletException

          {

-               processRequest(request, response, chain);
+               if (!processRequest(request, response)) {
+                       chain.doFilter(request, response);
+               }

          }
/** 

http://git-wip-us.apache.org/repos/asf/wicket/blob/0d501148/wicket-core/s
rc/main/java/org/apache/wicket/protocol/http/WicketServlet.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketServlet
.java
b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketServle
t.java index 5768c4f..2a8731e 100644
---
a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketServlet
.java +++
b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketServlet
.java @@ -134,7 +134,7 @@ public class WicketServlet extends

HttpServlet>>

          public final void doGet(final HttpServletRequest servletRequest,
final HttpServletResponse servletResponse) throws 

ServletException, IOException

          {

-               if (wicketFilter.processRequest(servletRequest,
servletResponse, null) == false)
+               if (wicketFilter.processRequest(servletRequest,
servletResponse) == false)

                  {
fallback(servletRequest, servletResponse); } 

@@ -156,7 +156,7 @@ public class WicketServlet extends HttpServlet

          public final void doPost(final HttpServletRequest
          servletRequest,
final HttpServletResponse servletResponse) throws 

ServletException, IOException

          {

-               if (wicketFilter.processRequest(servletRequest,
servletResponse, null) == false)
+               if (wicketFilter.processRequest(servletRequest,
servletResponse) == false)

                  {
fallback(servletRequest, servletResponse); } 

http://git-wip-us.apache.org/repos/asf/wicket/blob/0d501148/wicket-core/s
rc/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilterT
est.java
b/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilter
Test.java index 7fc36a7..f1b4ce9 100644
---
a/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilterT
est.java +++
b/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilterT
est.java @@ -484,27 +484,23 @@ public class WicketFilterTest extends
Assert>>
                                  return

(String)invocation.getArguments()[0];

                          }
}); 

-               FilterChain chain = mock(FilterChain.class);

                  // execute 3 requests - 1 for bla.js, 1 for bla.css and
                  1

for bla.img

                  for (int i = 0; i < 3; i++)
                  {

-                       boolean isProcessed =
filter.processRequest(request, response, chain);
+                       boolean isProcessed =
filter.processRequest(request, response);

                          assertFalse(isProcessed);
                          verify(application,

Mockito.never()).newWebRequest(Matchers.eq(request),

                                  Matchers.anyString());
verify(application, 

Mockito.never()).newWebResponse(Matchers.any(WebRequest.class),

                                  Matchers.eq(response));

-                       verify(chain, Mockito.times(i +
1)).doFilter(request, response);

                  }
// execute the request to /something/real 

-               boolean isProcessed = filter.processRequest(request,
response, chain);
+               boolean isProcessed = filter.processRequest(request,
response);

                  assertTrue(isProcessed);
                  verify(application).newWebRequest(Matchers.eq(request),

Matchers.anyString());

verify(application).newWebResponse(Matchers.any(WebRequest.class),
Matchers.eq(response));
-               // the request is processed so the chain is not executed
-               verify(chain, Mockito.times(3)).doFilter(request,
response);

          }
/**

Reply via email to