From 10f215ef359ffb90531b985e625da09f833f916a Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 10 Feb 2012 15:00:21 +0100 Subject: [PATCH 1/2] [IMP] conditional GET for concatenated JS and CSS files bzr revid: xmo@openerp.com-20120210140021-uh5t5egr00dxdwr8 --- addons/web/controllers/main.py | 114 ++++++++++++++++++++++++--------- 1 file changed, 82 insertions(+), 32 deletions(-) diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index d4ad330463a..e99f221dc57 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -8,6 +8,7 @@ import itertools import logging import operator import datetime +import hashlib import os import re import simplejson @@ -20,6 +21,7 @@ from cStringIO import StringIO import babel.messages.pofile import werkzeug.utils +import werkzeug.wrappers try: import xlwt except ImportError: @@ -35,21 +37,22 @@ openerpweb = common.http def concat_xml(file_list): """Concatenate xml files - return (concat,timestamp) - concat: concatenation of file content - timestamp: max(os.path.getmtime of file_list) + + :param list(str) file_list: list of files to check + :returns: (concatenation_result, checksum) + :rtype: (str, str) """ + checksum = hashlib.new('sha1') if not file_list: - return '', None + return '', checksum.hexdigest() root = None - files_timestamp = 0 for fname in file_list: - ftime = os.path.getmtime(fname) - if ftime > files_timestamp: - files_timestamp = ftime - - xml = ElementTree.parse(fname).getroot() + with open(fname, 'rb') as fp: + contents = fp.read() + checksum.update(contents) + fp.seek(0) + xml = ElementTree.parse(fp).getroot() if root is None: root = ElementTree.Element(xml.tag) @@ -58,17 +61,21 @@ def concat_xml(file_list): for child in xml.getchildren(): root.append(child) - return ElementTree.tostring(root, 'utf-8'), files_timestamp + return ElementTree.tostring(root, 'utf-8'), checksum.hexdigest() def concat_files(file_list, reader=None, intersperse=""): - """ Concatenate file content - return (concat,timestamp) - concat: concatenation of file content, read by `reader` - timestamp: max(os.path.getmtime of file_list) + """ Concatenates contents of all provided files + + :param list(str) file_list: list of files to check + :param function reader: reading procedure for each file + :param str intersperse: string to intersperse between file contents + :returns: (concatenation_result, checksum) + :rtype: (str, str) """ + checksum = hashlib.new('sha1') if not file_list: - return '', None + return '', checksum.hexdigest() if reader is None: def reader(f): @@ -76,15 +83,13 @@ def concat_files(file_list, reader=None, intersperse=""): return fp.read() files_content = [] - files_timestamp = 0 for fname in file_list: - ftime = os.path.getmtime(fname) - if ftime > files_timestamp: - files_timestamp = ftime + contents = reader(fname) + checksum.update(contents) + files_content.append(contents) - files_content.append(reader(fname)) files_concat = intersperse.join(files_content) - return files_concat,files_timestamp + return files_concat, checksum.hexdigest() html_template = """ @@ -150,10 +155,24 @@ class WebClient(openerpweb.Controller): def qweblist(self, req, mods=None): return self.manifest_list(req, mods, 'qweb') + def get_last_modified(self, files): + """ Returns the modification time of the most recently modified + file provided + + :param list(str) files: names of files to check + :return: most recent modification time amongst the fileset + :rtype: datetime.datetime + """ + return max(datetime.datetime.fromtimestamp(os.path.getmtime(f)) + for f in files) + @openerpweb.httprequest def css(self, req, mods=None): - files = list(self.manifest_glob(req, mods, 'css')) + last_modified = self.get_last_modified(f[0] for f in files) + if req.httprequest.if_modified_since and req.httprequest.if_modified_since >= last_modified: + return werkzeug.wrappers.Response(status=304) + file_map = dict(files) rx_import = re.compile(r"""@import\s+('|")(?!'|"|/|https?://)""", re.U) @@ -182,23 +201,54 @@ class WebClient(openerpweb.Controller): ) return data - content,timestamp = concat_files((f[0] for f in files), reader) - # TODO use timestamp to set Last mofified date and E-tag - return req.make_response(content, [('Content-Type', 'text/css')]) + content, checksum = concat_files((f[0] for f in files), reader) + if req.httprequest.if_none_match and checksum in req.httprequest.if_none_match: + return werkzeug.wrappers.Response(status=304) + + response = req.make_response(content, [('Content-Type', 'text/css')]) + response.cache_control.must_revalidate = True + response.cache_control.max_age = 0 + response.last_modified = last_modified + response.set_etag(checksum) + return response @openerpweb.httprequest def js(self, req, mods=None): files = [f[0] for f in self.manifest_glob(req, mods, 'js')] - content, timestamp = concat_files(files, intersperse=';') - # TODO use timestamp to set Last mofified date and E-tag - return req.make_response(content, [('Content-Type', 'application/javascript')]) + last_modified = self.get_last_modified(files) + print 'last modified', last_modified + print 'if modified since', req.httprequest.if_modified_since + if req.httprequest.if_modified_since and req.httprequest.if_modified_since >= last_modified: + return werkzeug.wrappers.Response(status=304) + + content, checksum = concat_files(files, intersperse=';') + if req.httprequest.if_none_match and checksum in req.httprequest.if_none_match: + return werkzeug.wrappers.Response(status=304) + + resp = req.make_response(content, [('Content-Type', 'application/javascript')]) + resp.cache_control.must_revalidate = True + resp.cache_control.max_age = 0 + resp.last_modified = last_modified + resp.set_etag(checksum) + return resp @openerpweb.httprequest def qweb(self, req, mods=None): files = [f[0] for f in self.manifest_glob(req, mods, 'qweb')] - content,timestamp = concat_xml(files) - # TODO use timestamp to set Last mofified date and E-tag - return req.make_response(content, [('Content-Type', 'text/xml')]) + last_modified = self.get_last_modified(files) + if req.httprequest.if_modified_since and req.httprequest.if_modified_since >= last_modified: + return werkzeug.wrappers.Response(status=304) + + content,checksum = concat_xml(files) + if req.httprequest.if_none_match and checksum in req.httprequest.if_none_match: + return werkzeug.wrappers.Response(status=304) + + response = req.make_response(content, [('Content-Type', 'text/xml')]) + response.cache_control.must_revalidate = True + response.cache_control.max_age = 0 + response.last_modified = last_modified + response.set_etag(checksum) + return response @openerpweb.httprequest From 3b565386c2eb197157f5772db3b300c1b037193f Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 10 Feb 2012 15:13:38 +0100 Subject: [PATCH 2/2] [IMP] move all the making-stuff-conditional-based-on-other-stuff code to a dedicated method of the WebClient controller provides a single point of configuration if/when we decide to change the cache-control stuff, mostly bzr revid: xmo@openerp.com-20120210141338-3aib9ls3b72gr4ez --- addons/web/controllers/main.py | 58 +++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index e99f221dc57..38fadcf745d 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -166,6 +166,30 @@ class WebClient(openerpweb.Controller): return max(datetime.datetime.fromtimestamp(os.path.getmtime(f)) for f in files) + def make_conditional(self, req, response, last_modified=None, etag=None): + """ Makes the provided response conditional based upon the request, + and mandates revalidation from clients + + Uses Werkzeug's own :meth:`ETagResponseMixin.make_conditional`, after + setting ``last_modified`` and ``etag`` correctly on the response object + + :param req: OpenERP request + :type req: web.common.http.WebRequest + :param response: Werkzeug response + :type response: werkzeug.wrappers.Response + :param datetime.datetime last_modified: last modification date of the response content + :param str etag: some sort of checksum of the content (deep etag) + :return: the response object provided + :rtype: werkzeug.wrappers.Response + """ + response.cache_control.must_revalidate = True + response.cache_control.max_age = 0 + if last_modified: + response.last_modified = last_modified + if etag: + response.set_etag(etag) + return response.make_conditional(req.httprequest) + @openerpweb.httprequest def css(self, req, mods=None): files = list(self.manifest_glob(req, mods, 'css')) @@ -202,15 +226,10 @@ class WebClient(openerpweb.Controller): return data content, checksum = concat_files((f[0] for f in files), reader) - if req.httprequest.if_none_match and checksum in req.httprequest.if_none_match: - return werkzeug.wrappers.Response(status=304) - response = req.make_response(content, [('Content-Type', 'text/css')]) - response.cache_control.must_revalidate = True - response.cache_control.max_age = 0 - response.last_modified = last_modified - response.set_etag(checksum) - return response + return self.make_conditional( + req, req.make_response(content, [('Content-Type', 'text/css')]), + last_modified, checksum) @openerpweb.httprequest def js(self, req, mods=None): @@ -222,15 +241,10 @@ class WebClient(openerpweb.Controller): return werkzeug.wrappers.Response(status=304) content, checksum = concat_files(files, intersperse=';') - if req.httprequest.if_none_match and checksum in req.httprequest.if_none_match: - return werkzeug.wrappers.Response(status=304) - resp = req.make_response(content, [('Content-Type', 'application/javascript')]) - resp.cache_control.must_revalidate = True - resp.cache_control.max_age = 0 - resp.last_modified = last_modified - resp.set_etag(checksum) - return resp + return self.make_conditional( + req, req.make_response(content, [('Content-Type', 'application/javascript')]), + last_modified, checksum) @openerpweb.httprequest def qweb(self, req, mods=None): @@ -240,16 +254,10 @@ class WebClient(openerpweb.Controller): return werkzeug.wrappers.Response(status=304) content,checksum = concat_xml(files) - if req.httprequest.if_none_match and checksum in req.httprequest.if_none_match: - return werkzeug.wrappers.Response(status=304) - - response = req.make_response(content, [('Content-Type', 'text/xml')]) - response.cache_control.must_revalidate = True - response.cache_control.max_age = 0 - response.last_modified = last_modified - response.set_etag(checksum) - return response + return self.make_conditional( + req, req.make_response(content, [('Content-Type', 'text/xml')]), + last_modified, checksum) @openerpweb.httprequest def home(self, req, s_action=None, **kw):