Docserver: Implement the content providers infrastructure, where a

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
index 25b6d3ded6c869c0d5778e87beb888d559412c24..c65058914e76de316559bed73a6a49702afeb4ee 100644
--- a/chrome/common/extensions/docs/server2/render_servlet.py
+++ b/chrome/common/extensions/docs/server2/render_servlet.py
@@ -5,20 +5,30 @@
 from fnmatch import fnmatch
 import logging
 import mimetypes
+import posixpath

[not at google - send to devlin, 2013/11/01 20:45:18]

You may want to review this file as if it were newly added. It's changed very
dramatically, though it amounts to moving most of it into ContentProvider.

+import traceback
 from urlparse import urlsplit
 
 from data_source_registry import CreateDataSources
 from file_system import FileNotFoundError
+from redirector import Redirector
 from servlet import Servlet, Response
 from svn_constants import DOCS_PATH, PUBLIC_TEMPLATE_PATH
+from third_party.handlebar import Handlebar
+
+
+def _MakeHeaders(content_type):
+  return {
+    'x-frame-options': 'sameorigin',
+    'content-type': content_type,
+    'cache-control': 'max-age=300',
+  }
 
-def _IsBinaryMimetype(mimetype):
-  return any(
-    mimetype.startswith(prefix) for prefix in ['audio', 'image', 'video'])
 
 class RenderServlet(Servlet):
   '''Servlet which renders templates.
   '''
+
   class Delegate(object):
     def CreateServerInstance(self):
       raise NotImplementedError(self.__class__)
@@ -33,82 +43,64 @@ class RenderServlet(Servlet):
     # TODO(kalman): a consistent path syntax (even a Path class?) so that we
     # can stop being so conservative with stripping and adding back the '/'s.
     path = self._request.path.lstrip('/')
-
-    if path.split('/')[-1] == 'redirects.json':
-      return Response.Ok('')
-
     server_instance = self._delegate.CreateServerInstance()
 
-    redirect = server_instance.redirector.Redirect(self._request.host, path)
-    if redirect is not None:
-      return Response.Redirect(redirect)
-
-    canonical_result = server_instance.path_canonicalizer.Canonicalize(path)
-    redirect = canonical_result.path.lstrip('/')
-    if path != redirect:
-      return Response.Redirect('/' + redirect,
-                               permanent=canonical_result.permanent)
-
-    trunk_fs = server_instance.host_file_system_provider.GetTrunk()
-    template_renderer = server_instance.template_renderer
-    template_cache = server_instance.compiled_fs_factory.ForTemplates(trunk_fs)
-
-    content = None
-    content_type = None
-
     try:
-      # At this point, any valid paths ending with '/' have been redirected.
-      # Therefore, the response should be a 404 Not Found.
-      if path.endswith('/'):
-        pass
-      elif fnmatch(path, 'extensions/examples/*.zip'):
-        zip_path = DOCS_PATH + path[len('extensions'):-len('.zip')]
-        content = server_instance.directory_zipper.Zip(zip_path).Get()
-        content_type = 'application/zip'
-      elif path.startswith('extensions/examples/'):
-        mimetype = mimetypes.guess_type(path)[0] or 'text/plain'
-        content = trunk_fs.ReadSingle(
-            '%s/%s' % (DOCS_PATH, path[len('extensions/'):]),
-            binary=_IsBinaryMimetype(mimetype)).Get()
-        content_type = mimetype
-      elif path.startswith('static/'):
-        mimetype = mimetypes.guess_type(path)[0] or 'text/plain'
-        content = trunk_fs.ReadSingle(
-            '%s/%s' % (DOCS_PATH, path),
-            binary=_IsBinaryMimetype(mimetype)).Get()
-        content_type = mimetype
-      elif path.endswith('.html'):
-        content = template_renderer.Render(
-            template_cache.GetFromFile(
-                '%s/%s' % (PUBLIC_TEMPLATE_PATH, path)).Get(),
-            self._request)
-        content_type = 'text/html'
-      else:
-        content = None
+      return self._GetSuccessResponse(path, server_instance)
     except FileNotFoundError:
-      content = None
-
-    headers = {'x-frame-options': 'sameorigin'}
-    if content is None:
-      def render_template_or_none(path):
+      # Maybe it didn't find the file because its canonical location is
+      # somewhere else; this is distinct from "redirects", which are typically
+      # explicit. This is implicit.
+      canonical_result = server_instance.path_canonicalizer.Canonicalize(path)
+      redirect = canonical_result.path.lstrip('/')
+      if path != redirect:
+        return Response.Redirect('/' + redirect,
+                                 permanent=canonical_result.permanent)
+
+      # Not found for reals. Find the closest 404.html file and serve that;
+      # e.g. if the path is extensions/manifest/typo.html then first look for
+      # extensions/manifest/404.html, then extensions/404.html, then 404.html.
+      #
+      # Failing that just print 'Not Found' but that should preferrably never
+      # happen, because it would look really bad.
+      path_components = path.split('/')
+      for i in xrange(len(path_components) - 1, -1, -1):
         try:
-          return template_renderer.Render(
-              template_cache.GetFromFile(
-                  '%s/%s' % (PUBLIC_TEMPLATE_PATH, path)).Get(),
-              self._request)
-        except FileNotFoundError:
-          return None
-      content = (render_template_or_none('%s/404.html' %
-                                         path.split('/', 1)[0]) or
-                 render_template_or_none('extensions/404.html') or
-                 'Not found')
-      return Response.NotFound(content, headers=headers)
-
-    if not content:
+          path_404 = posixpath.join(*(path_components[0:i] + ['404.html']))
+          response = self._GetSuccessResponse(path_404, server_instance)
+          return Response.NotFound(response.content.ToString(),
+                                   headers=response.headers)
+        except FileNotFoundError: continue
+      logging.error('No 404.html found in %s' % path)
+      return Response.NotFound('Not Found', headers=_MakeHeaders('text/plain'))
+
+  def _GetSuccessResponse(self, path, server_instance):
+    '''Returns the Response from trying to render |path| with
+    |server_instance|.  If |path| isn't found then a FileNotFoundError will be
+    raised, such that the only responses that will be returned from this method
+    are Ok and Redirect.
+    '''
+    content_provider, path_within_content_provider = (
+        server_instance.content_providers.GetByServlet(path))
+    if content_provider is None:
+      content_provider = server_instance.content_providers.GetDefault()
+    else:
+      path = path_within_content_provider

[Jeffrey Yasskin, 2013/11/04 21:21:18]

It looks like GetByServlet(path) returns |path| when it can't find a servlet, so
you might not need this else branch.

[not at google - send to devlin, 2013/11/04 23:34:49]

Good point.

+
+    redirect = Redirector(
+        server_instance.compiled_fs_factory,
+        content_provider.file_system).Redirect(self._request.host, path)
+    if redirect is not None:
+      return Response.Redirect(redirect, permanent=False)
+
+    content_info = content_provider.GetContentInfo(
+        self._request.host, path).Get()
+    if not content_info.content:
       logging.error('%s had empty content' % path)
 
-    headers.update({
-      'content-type': content_type,
-      'cache-control': 'max-age=300',
-    })
-    return Response.Ok(content, headers=headers)
+    if isinstance(content_info.content, Handlebar):
+      content_info.content = server_instance.template_renderer.Render(
+          content_info.content, self._request)
+
+    return Response.Ok(content_info.content,
+                       headers=_MakeHeaders(content_info.content_type))