Alexander Wels has posted comments on this change.

Change subject: userportal: Added caching to user portal page webadmin: Added 
caching to webadmin page
......................................................................


Patch Set 1: (5 inline comments)

....................................................
File 
frontend/webadmin/modules/frontend-overlay/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/CachingFilter.java
Line 53:     public void doFilter(ServletRequest request, ServletResponse 
response,
Line 54:             FilterChain chain) throws IOException, ServletException {
Line 55:         // Cast to HttpServletRequest/Response.
Line 56:         final HttpServletRequest httpRequest = (HttpServletRequest) 
request;
Line 57:         final HttpServletResponse httpResponse = (HttpServletResponse) 
response;
We have bigger problems if those are not HttpServletRequest/Response
Line 58: 
Line 59:         if (cacheFilterPatternMathes(httpRequest)) {
Line 60:             HttpServletResponseWrapper responseWrapper = 
getCacheHeaderResponseWrapper(httpResponse);
Line 61:             httpResponse.setHeader(CACHE_CONTROL, CACHE_YEAR);


Line 59:         if (cacheFilterPatternMathes(httpRequest)) {
Line 60:             HttpServletResponseWrapper responseWrapper = 
getCacheHeaderResponseWrapper(httpResponse);
Line 61:             httpResponse.setHeader(CACHE_CONTROL, CACHE_YEAR);
Line 62:             httpResponse.setHeader(EXPIRES, getNowPlusYearHttpDate());
Line 63:             chain.doFilter(request, responseWrapper);
Makes no difference to me, I will change the call to address the wrapper, 
doesn't matter in the end.
Line 64:         } else if (noCacheFilterPatternMatches(httpRequest)) {
Line 65:             chain.doFilter(request, response);
Line 66:             httpResponse.setHeader(CACHE_CONTROL, NO_CACHE);
Line 67:             httpResponse.setHeader(EXPIRES, getYesterdayHttpDate());


Line 65:             chain.doFilter(request, response);
Line 66:             httpResponse.setHeader(CACHE_CONTROL, NO_CACHE);
Line 67:             httpResponse.setHeader(EXPIRES, getYesterdayHttpDate());
Line 68:         } else {
Line 69:             chain.doFilter(request, response);
It can't be outside of the if else, because in the first condition the response 
is the wrapper, in the second and else condition it is the original response. I 
can re-factor that a little bit and pull the chain outside of the if/else 
if/else
Line 70:         }
Line 71:     }
Line 72: 
Line 73:     private String getYesterdayHttpDate() {


Line 104:                     httpResponse.setHeader(name, value);
Line 105:                 }
Line 106:             }
Line 107:         };
Line 108:     }
Because JBOSS sets the expires header poorly further down the chain, we put 
this filter in place to set the expires properly, we don't want that 
overwritten by something down the chain.
Line 109: 
Line 110:     private boolean cacheFilterPatternMathes(HttpServletRequest 
httpRequest) {
Line 111:         boolean result = false;
Line 112:         if (null != cachePattern) {


Line 135:         List<String> filterParams = Collections.list(filterConfig
Line 136:                 .getInitParameterNames());
Line 137:         // No need to worry about concurrency, worst case scenario 
the same
Line 138:         // pattern is calculated a couple of times.
Line 139:         if (null == cachePattern) {
Done
Line 140:             // Get a list of parameters.
Line 141:             for (String paramName : filterParams) {
Line 142:                 if (CACHE.equals(paramName)) {
Line 143:                     cachePattern = Pattern.compile(filterConfig


--
To view, visit http://gerrit.ovirt.org/10449
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d8e02ae542a4aa37bd421bde5582c0f3e9820ad
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to