Docserver: refactor the Handler/ServerInstance relationship into a servlet

chrome/common/extensions/docs/server2/render_servlet.py

Index: chrome/common/extensions/docs/server2/render_servlet.py
diff --git a/chrome/common/extensions/docs/server2/render_servlet.py b/chrome/common/extensions/docs/server2/render_servlet.py
new file mode 100644
index 0000000000000000000000000000000000000000..7ce761851aca378d8e16f70499d7ac5d4bea8f45
--- /dev/null
+++ b/chrome/common/extensions/docs/server2/render_servlet.py
@@ -0,0 +1,125 @@
+# Copyright 2013 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from fnmatch import fnmatch
+import logging
+import mimetypes
+import os
+import traceback
+
+from appengine_wrappers import IsDevServer
+from branch_utility import BranchUtility
+from file_system import FileNotFoundError
+from server_instance import ServerInstance
+from servlet import Servlet
+import svn_constants
+
+_DEFAULT_CHANNEL = 'stable'
+
+_ALWAYS_ONLINE = IsDevServer()
+
+def _IsBinaryMimetype(mimetype):
+  return any(mimetype.startswith(prefix)
+             for prefix in ['audio', 'image', 'video'])
+
+def AlwaysOnline(fn):
+  '''A function decorator which forces the rendering to be always online rather
+  than the default offline behaviour. Useful for testing.
+  '''
+  def impl(*args, **optargs):
+    global _ALWAYS_ONLINE
+    was_always_online = _ALWAYS_ONLINE
+    try:
+      _ALWAYS_ONLINE = True
+      return fn(*args, **optargs)
+    finally:
+      _ALWAYS_ONLINE = was_always_online
+  return impl
+
+class RenderServlet(Servlet):
+  '''Servlet which renders templates.
+  '''
+  def Get(self, server_instance=None):
+    path, request, response = (self._path, self._request, self._response)
+
+    # Redirect "extensions" and "extensions/" to "extensions/index.html", etc.
+    if os.path.splitext(path)[1] == '' and path.find('/') == -1:
+      path += '/'
+    if path.endswith('/'):
+      self.Redirect(path + 'index.html')
+      return
+
+    channel_name, real_path = BranchUtility.SplitChannelNameFromPath(path)
+
+    if channel_name == _DEFAULT_CHANNEL:
+      self.Redirect('/%s' % real_path)
+      return
+
+    if channel_name is None:
+      channel_name = _DEFAULT_CHANNEL
+
+    # AppEngine instances should never need to call out to SVN. That should
+    # only ever be done by the cronjobs, which then write the result into
+    # DataStore, which is as far as instances look. To enable this, crons can
+    # pass a custom (presumably online) ServerInstance into Get().
+    #
+    # Why? SVN is slow and a bit flaky. Cronjobs failing is annoying but
+    # temporary. Instances failing affects users, and is really bad.
+    #
+    # Anyway - to enforce this, we actually don't give instances access to SVN.
+    # If anything is missing from datastore, it'll be a 404. If the cronjobs
+    # don't manage to catch everything - uhoh. On the other hand, we'll figure
+    # it out pretty soon, and it also means that legitimate 404s are caught
+    # before a round trip to SVN.
+    if not server_instance:
+      # The ALWAYS_ONLINE thing is for tests and preview.py that shouldn't need
+      # to run the cron before rendering things.
+      constructor = (ServerInstance.CreateOnline if _ALWAYS_ONLINE else
+                     ServerInstance.GetOrCreateOffline)
+      server_instance = constructor(channel_name)
+
+    canonical_path = server_instance.path_canonicalizer.Canonicalize(real_path)
+    if real_path != canonical_path:
+      if channel_name is None:
+        self.Redirect(canonical_path);
+      else:
+        self.Redirect('%s/%s' % (channel_name, canonical_path))
+      return
+
+    templates = server_instance.template_data_source_factory.Create(request,

[方觉(Fang Jue), 2013/04/29 12:14:15]

These code (and below) will also be shared by PatchServlet, right? Maybe they
should be moved into a separate method (and then let PatchServlet inherit from
RenderServlet and override Get?)

[not at google - send to devlin, 2013/04/29 16:09:13]

As they say, composition over inheritance - composing these servlets is a little
bit weird but I'm not fussed. It *is* unfortunate to be creating these fake
request objects, I'm going to have a look at doing something more formal than
that. Probably removing the strict appengine format of these things, the
writing-to-out thing is partially what creates the messiness.

However - why will PatchServlet have to use a lot of this code? In the current
state of this patch you can pass a custom ServerInstance into RenderServlet.Get.
The PatchServlet should be able to call that with a ServerInstance that has the
patched subversion file system.

+                                                                    path)
+
+    content = None
+    try:
+      if fnmatch(path, 'extensions/examples/*.zip'):
+        content = server_instance.example_zipper.Create(
+            path[len('extensions/'):-len('.zip')])
+        response.headers['content-type'] = 'application/zip'
+      elif path.startswith('extensions/examples/'):
+        mimetype = mimetypes.guess_type(path)[0] or 'text/plain'
+        content = server_instance.content_cache.GetFromFile(
+            '%s/%s' % (svn_constants.DOCS_PATH, path[len('extensions/'):]),
+            binary=_IsBinaryMimetype(mimetype))
+        response.headers['content-type'] = 'text/plain'
+      elif path.startswith('static/'):
+        mimetype = mimetypes.guess_type(path)[0] or 'text/plain'
+        content = server_instance.content_cache.GetFromFile(
+            ('%s/%s' % (svn_constants.DOCS_PATH, path)),
+            binary=_IsBinaryMimetype(mimetype))
+        response.headers['content-type'] = mimetype
+      elif path.endswith('.html'):
+        content = templates.Render(path)
+    except FileNotFoundError as e:
+      logging.warning(traceback.format_exc())
+      content = None
+
+    response.headers['x-frame-options'] = 'sameorigin'
+    if content is None:
+      response.set_status(404);
+      response.out.write(templates.Render('404'))
+    else:
+      if not content:
+        logging.error('%s had empty content' % path)
+      response.headers['cache-control'] = 'max-age=300'
+      response.out.write(content)