Gilad Chaplik 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)
please change the commit header to a single line
....................................................
File
frontend/webadmin/modules/frontend-overlay/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/CachingFilter.java
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);
IMHO, once you wrap an object you should only address wrapper.
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);
doFilter should be called in any condition, so its should be outside the '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: }
why? IMHO this filter should not enforce the next filters, it lives only in
this context. and this makes the code less readable in the doFilter
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) {
please do.
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
....................................................
File
frontend/webadmin/modules/frontend-overlay/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/UserportalDynamicHostingServlet.java
Line 25: protected void initQueryParams(VdcQueryParametersBase queryParams,
String sessionId) {
Line 26: super.initQueryParams(queryParams, sessionId);
Line 27:
Line 28: // All UserPortal queries are filtered according to user
permissions
Line 29: queryParams.setFiltered(true);
I know, but the client always fills the parameters with that, and we override
it anyway.
nevermind
Line 30: }
Line 31:
--
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 <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
