Rev 425: Add some tests for the ChangeLogUI, and implement a method for cheesing HEAD requests. in http://bazaar.launchpad.net/~jameinel/loggerhead/less_work_for_head_716217

John Arbash Meinel john at arbash-meinel.com
Thu Feb 10 05:05:44 UTC 2011


At http://bazaar.launchpad.net/~jameinel/loggerhead/less_work_for_head_716217

------------------------------------------------------------
revno: 425
revision-id: john at arbash-meinel.com-20110210050543-8q6rvwj0plt5di8f
parent: john at arbash-meinel.com-20110210023315-515pkynlfpfs3cvm
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: less_work_for_head_716217
timestamp: Wed 2011-02-09 23:05:43 -0600
message:
  Add some tests for the ChangeLogUI, and implement a method for cheesing HEAD requests.
  
  Basically, if we can answer quickly that HEAD is ok, do so, but allow fallbacks
  that do it the way we used to, rather than requiring we handle all processing
  immediately.
-------------- next part --------------
=== modified file 'loggerhead/controllers/__init__.py'
--- a/loggerhead/controllers/__init__.py	2009-10-17 08:59:33 +0000
+++ b/loggerhead/controllers/__init__.py	2011-02-10 05:05:43 +0000
@@ -49,8 +49,18 @@
 
 
 class TemplatedBranchView(object):
+    """Base class for most views.
+
+    This is based on the idea that .get_values() will populate a dict with
+    values for a template, which will then be expanded and returned for a given
+    request.
+
+    :cvar template_path: Subclasses set this to the template that will be
+        used for this particular view.
+    """
 
     template_path = None
+    sets_extra_headers = True
 
     def __init__(self, branch, history_callable):
         self._branch = branch
@@ -65,6 +75,25 @@
         self.__history = self._history_callable()
         return self.__history
 
+
+    def validate_call(self, path, kwargs, headers):
+        """Make sure the request being made is valid.
+
+        This is done for HEAD requests. Instead of calling get_values and
+        rendering the template, we validate the request, and return just the
+        headers.
+
+        Classes which implement validate_call should take care to make sure the
+        headers they set will be identical to headers set for get_values().
+        Probably the best way to do this is to have get_values() call
+        validate_call directly, and only set header data there.
+
+        :return: True if you know the call is valid, raise an exception if
+            there is a problem (preferabbly an HTTPException), and False if we
+            should just process the call as normal.
+        """
+        return False
+
     def __call__(self, environ, start_response):
         z = time.time()
         kwargs = dict(parse_querystring(environ))
@@ -90,13 +119,16 @@
         vals.update(templatefunctions)
         headers = {}
 
+        if (environ.get('REQUEST_METHOD', 'GET') == 'HEAD'):
+            if self.validate_call(path, kwargs, headers):
+                self._call_start_response(start_response, headers)
+                return []
+
         vals.update(self.get_values(path, kwargs, headers))
 
         self.log.info('Getting information for %s: %r secs' % (
             self.__class__.__name__, time.time() - z))
-        if 'Content-Type' not in headers:
-            headers['Content-Type'] = 'text/html'
-        writer = start_response("200 OK", headers.items())
+        writer = self._call_start_response(start_response, headers)
         template = load_template(self.template_path)
         z = time.time()
         w = BufferingWriter(writer, 8192)
@@ -107,6 +139,13 @@
                 self.__class__.__name__, time.time() - z, w.bytes))
         return []
 
+    def _call_start_response(self, start_response, headers):
+        if 'Content-Type' not in headers:
+            headers['Content-Type'] = 'text/html'
+        # Since HEAD is supposed to match GET, shouldn't we be doing something
+        # like sorting the headers?
+        return start_response("200 OK", headers.items())
+
     def get_revid(self):
         h = self._history
         if h is None:

=== modified file 'loggerhead/controllers/changelog_ui.py'
--- a/loggerhead/controllers/changelog_ui.py	2010-04-22 08:52:59 +0000
+++ b/loggerhead/controllers/changelog_ui.py	2011-02-10 05:05:43 +0000
@@ -35,6 +35,14 @@
 
     template_path = 'loggerhead.templates.changelog'
 
+    def validate_call(self, path, kwargs, headers):
+        if path is not None or kwargs:
+            # This includes a file-id, abort for now
+            return False
+        # Make sure we have a valid revid
+        self.get_revid()
+        return True
+
     def get_values(self, path, kwargs, headers):
         history = self._history
         revid = self.get_revid()

=== modified file 'loggerhead/tests/test_controllers.py'
--- a/loggerhead/tests/test_controllers.py	2009-03-19 18:48:15 +0000
+++ b/loggerhead/tests/test_controllers.py	2011-02-10 05:05:43 +0000
@@ -1,4 +1,9 @@
+from cStringIO import StringIO
+
+from bzrlib import errors
+
 from loggerhead.apps.branch import BranchWSGIApp
+from loggerhead.controllers.changelog_ui import ChangeLogUI
 from loggerhead.controllers.annotate_ui import AnnotateUI
 from loggerhead.controllers.inventory_ui import InventoryUI
 from loggerhead.controllers.revision_ui import RevisionUI
@@ -6,6 +11,60 @@
 from loggerhead import util
 
 
+class TestChangeLogUI(BasicTests):
+
+    def make_branch_and_changelog_ui(self, tree_shape):
+        tree = self.make_branch_and_tree('.')
+        self.build_tree(tree_shape)
+        tree.smart_add([])
+        tree.commit('simple message')
+        tree.branch.lock_read()
+        self.addCleanup(tree.branch.unlock)
+        branch_app = BranchWSGIApp(tree.branch, '')
+        # These are usually set in BranchWSGIApp.app(), which is set from env
+        # settings set by BranchesFromTransportRoot, so we fake it.
+        branch_app._static_url_base = '/'
+        branch_app._url_base = '/'
+        return tree.branch, ChangeLogUI(branch_app, branch_app.get_history)
+
+    def consume_app(self, app, extra_environ=None):
+        env = {'SCRIPT_NAME': '/changes', 'PATH_INFO': ''}
+        if extra_environ is not None:
+            env.update(extra_environ)
+        body = StringIO()
+        start = []
+        def start_response(status, headers, exc_info=None):
+            start.append((status, headers, exc_info))
+            return body.write
+        extra_content = list(app(env, start_response))
+        body.writelines(extra_content)
+        return start[0], body.getvalue()
+
+    def test_smoke(self):
+        bzrbranch, change_ui = self.make_branch_and_changelog_ui(
+            ['file'])
+        start, content = self.consume_app(change_ui)
+        self.assertEqual(('200 OK', [('Content-Type', 'text/html')], None),
+                         start)
+        self.assertContainsRe(content, 'simple message')
+
+    def test_simple_head_doesnt_yield_body(self):
+        bzrbranch, change_ui = self.make_branch_and_changelog_ui(
+            ['file'])
+        start, content = self.consume_app(change_ui,
+                            extra_environ={'REQUEST_METHOD': 'HEAD'})
+        self.assertEqual(('200 OK', [('Content-Type', 'text/html')], None),
+                         start)
+        self.assertEqualDiff('', content)
+
+    def test_simple_head_bad_revid(self):
+        bzrbranch, change_ui = self.make_branch_and_changelog_ui(
+            ['file'])
+        self.assertRaises(errors.NoSuchRevision, self.consume_app,
+            change_ui, extra_environ={'REQUEST_METHOD': 'HEAD',
+                                      'PATH_INFO': '/9999'})
+
+
 class TestInventoryUI(BasicTests):
 
     def make_bzrbranch_and_inventory_ui_for_tree_shape(self, shape):



More information about the bazaar-commits mailing list