Extensions docs server: Design changes, partial template support.

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

Index: chrome/common/extensions/docs/server2/echo_handler.py
diff --git a/chrome/common/extensions/docs/server2/echo_handler.py b/chrome/common/extensions/docs/server2/echo_handler.py
index 20ae79cfb5e4d6d8adfa6c16009406c72a8fbc9f..adfd5d0d78d957f396188370b2a443b04c249c24 100755
--- a/chrome/common/extensions/docs/server2/echo_handler.py
+++ b/chrome/common/extensions/docs/server2/echo_handler.py
@@ -21,7 +21,7 @@ from google.appengine.ext.webapp.util import run_wsgi_app
 from branch_utility import BranchUtility
 from local_fetcher import LocalFetcher
 from subversion_fetcher import SubversionFetcher
-from template_fetcher import TemplateFetcher
+from template_data_source import TemplateDataSource
 from template_utility import TemplateUtility
 
 EXTENSIONS_PATH = 'chrome/common/extensions/'
@@ -30,48 +30,70 @@ STATIC_PATH = DOCS_PATH + 'static/'
 PUBLIC_TEMPLATE_PATH = DOCS_PATH + 'template2/public/'
 PRIVATE_TEMPLATE_PATH = DOCS_PATH + 'template2/private/'
 
-class MainPage(webapp.RequestHandler):
-  def _FetchResource(self, fetcher, branch, path):
-    result = fetcher.FetchResource(branch, path)
-    for key in result.headers:
-      self.response.headers[key] = result.headers[key]
-    return result.content
+# Global cache of instances because the Server is recreated for every request.
+SERVER_INSTANCES = {}
 
-  def get(self):
-    path = self.request.path
-    path = path.replace('/chrome/extensions/', '')
+class Instance(object):

[Aaron Boodman, 2012/06/06 23:49:59]

Consider moving this into its own class. Easier to test.

[not at google - send to devlin, 2012/06/07 04:43:57]

Yes please. Might make sense to rename ServerInstance or something when you do.

[cduvall, 2012/06/08 00:39:23]

Done.

+  def __init__(self, data_source, fetcher):
+    self._data_source = data_source
+    self._fetcher = fetcher
+    self._template_util = TemplateUtility()

[not at google - send to devlin, 2012/06/07 04:43:57]

Usually it doesn't make sense to instantiate util classes, looking at
TemplateUtility, RenderTemplate (and any future util methods) should just be
def'd at the top level then imported without the "from".

[cduvall, 2012/06/08 00:39:23]

Done.

+
+  def Run(self, path, request_handler):
     parts = path.split('/')
     filename = parts[-1]
+    template = self._data_source[filename]
+    if template:
+      content = self._template_util.RenderTemplate(template,
+          '{"test": "Hello"}', self._data_source)
+    else:
+      logging.info('Template not found for: ' + filename)
+      try:
+        result = self._fetcher.FetchResource(STATIC_PATH + filename)
+        for key in result.headers:
+          request_handler.response.headers[key] = result.headers[key]
+        content = result.content
+      except:
+        # TODO should be an actual not found page.
+        content = 'File not found.'
+    request_handler.response.out.write(content)
 
-    if len(path) > 0 and path[0] == '/':
-      path = path.strip('/')
 
-    branch_util = BranchUtility(urlfetch)
-    channel_name = branch_util.GetChannelNameFromPath(path)
-    branch = branch_util.GetBranchNumberForChannelName(channel_name)
+class Server(webapp.RequestHandler):
+  def __init__(self):
+    self._branch_util = BranchUtility(urlfetch)
 
-    if channel_name == 'local':
+  def _GetInstanceForBranch(self, branch):
+    if branch in SERVER_INSTANCES:
+      return SERVER_INSTANCES[branch]
+    if branch == 'local':
       fetcher = LocalFetcher(EXTENSIONS_PATH)
+      # No cache for local doc server.
+      cache_timeout = 0
     else:
-      fetcher = SubversionFetcher(EXTENSIONS_PATH, urlfetch)
-    template_fetcher = TemplateFetcher(branch, fetcher)
-    template_util = TemplateUtility()
+      fetcher = SubversionFetcher(branch, EXTENSIONS_PATH, urlfetch)
+      cache_timeout = 300

[not at google - send to devlin, 2012/06/07 04:43:57]

nit: cache_timeout_seconds

[cduvall, 2012/06/08 00:39:23]

Done.

+    data_source = TemplateDataSource(fetcher,
+        [PUBLIC_TEMPLATE_PATH, PRIVATE_TEMPLATE_PATH], cache_timeout)

[not at google - send to devlin, 2012/06/07 04:43:57]

nit: parameters all on same line, or all on vertically aligned separate lines.
So, here... vertical obviously :)

[cduvall, 2012/06/08 00:39:23]

Done.

+    SERVER_INSTANCES[branch] = Instance(data_source, fetcher)
+    return SERVER_INSTANCES[branch]
 
-    template = template_fetcher[PUBLIC_TEMPLATE_PATH + filename]
-    if template:
-      content = template_util.RenderTemplate(template, '{"test": "Hello"}')
-    else:
-      logging.info('Template for %s not found.' % filename)
+  def _HandleRequest(self, path):
+    channel_name = self._branch_util.GetChannelNameFromPath(path)

[not at google - send to devlin, 2012/06/07 04:43:57]

ditto comment about instantiating util classes.

[cduvall, 2012/06/08 00:39:23]

Done.

+    branch = self._branch_util.GetBranchNumberForChannelName(channel_name)
 
-      try:
-        content = self._FetchResource(fetcher, branch, STATIC_PATH + filename)
-      except:
-        content = 'File not found.'
+    instance = self._GetInstanceForBranch(branch)
+    instance.Run(path, self)

[not at google - send to devlin, 2012/06/07 04:43:57]

nit: can inline instance

[cduvall, 2012/06/08 00:39:23]

Done.

 
-    self.response.out.write(content)
+  def get(self):
+    path = self.request.path
+    path = path.replace('/chrome/extensions/', '')
+    if len(path) > 0 and path[0] == '/':
+      path = path.strip('/')
+    self._HandleRequest(path)
 
 application = webapp.WSGIApplication([
-  ('/.*', MainPage),
+  ('/.*', Server),
 ], debug=False)
 
 def main():