Docserver: Attach intro metadata to the template itself, not to an object which

contains the template.

BUG=320339
R=yoz@chromium.org, mkearney@chromium.org

Review URL: https://codereview.chromium.org/65643009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@235762 0039d316-1c4b-4281-b951-d872f2087c98

chrome/common/extensions/docs/server2/app.yaml

 application: chrome-apps-doc
-version: 2-42-1
+version: 2-42-2
 runtime: python27
 api_version: 1
 threadsafe: false

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

@@ -79,7 +79,8 @@ class CachingFileSystem(FileSystem):
     else:
       file_version = dir_stat.child_versions.get(file_path)
       if file_version is None:
-        raise FileNotFoundError('No stat found for %s in %s' % (path, dir_path))
+        raise FileNotFoundError('No stat found for %s in %s (found %s)' %
+                                (path, dir_path, dir_stat.child_versions))
       stat_info = StatInfo(file_version)
 
     return stat_info

chrome/common/extensions/docs/server2/cron.yaml

@@ -2,4 +2,4 @@ cron:
 - description: Repopulates all cached data.
   url: /_cron
   schedule: every 5 minutes
-  target: 2-42-1
+  target: 2-42-2

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

@@ -18,6 +18,20 @@ from third_party.handlebar import Handlebar
 _H1_REGEX = re.compile('<h1[^>.]*?>.*?</h1>', flags=re.DOTALL)
 
 
+class _HandlebarWithContext(Handlebar):
+  '''Extends Handlebar with a get() method so that templates can not only
+  render intros with {{+intro}} but also access properties on them, like
+  {{intro.title}}, {{intro.toc}}, etc.
+  '''
+
+  def __init__(self, text, name, context):
+    Handlebar.__init__(self, text, name=name)
+    self._context = context
+
+  def get(self, key):
+    return self._context.get(key)
+
+
 # TODO(kalman): rename this HTMLDataSource or other, then have separate intro
 # article data sources created as instances of it.
 class _IntroParser(HTMLParser):
@@ -62,48 +76,49 @@ class _IntroParser(HTMLParser):
     elif self._recent_tag in ['h2', 'h3']:
       self._current_heading['title'] += data
 
+
 class IntroDataSource(DataSource):
   '''This class fetches the intros for a given API. From this intro, a table
   of contents dictionary is created, which contains the headings in the intro.
   '''
 
-  def __init__(self, server_instance, _):
+  def __init__(self, server_instance, request):
+    self._template_renderer = server_instance.template_renderer
+    self._request = request
     self._cache = server_instance.compiled_fs_factory.Create(
         server_instance.host_file_system_provider.GetTrunk(),
-        self._MakeIntroDict,
+        self._MakeIntro,
         IntroDataSource)
     self._ref_resolver = server_instance.ref_resolver_factory.Create()
 
-  def _MakeIntroDict(self, intro_path, intro):
+  def _MakeIntro(self, intro_path, intro):
     # Guess the name of the API from the path to the intro.
     api_name = os.path.splitext(intro_path.split('/')[-1])[0]
-    intro_with_links = self._ref_resolver.ResolveAllLinks(intro,
-                                                          namespace=api_name)
-    # TODO(kalman): Do $ref replacement after rendering the template, not
-    # before, so that (a) $ref links can contain template annotations, and (b)
-    # we can use CompiledFileSystem.ForTemplates to create the templates and
-    # save ourselves some effort.
-    apps_parser = _IntroParser()
-    apps_parser.feed(Handlebar(intro_with_links).render(
-        { 'is_apps': True }).text)
-    extensions_parser = _IntroParser()
-    extensions_parser.feed(Handlebar(intro_with_links).render(
-        { 'is_apps': False }).text)
-    # TODO(cduvall): Use the normal template rendering system, so we can check
-    # errors.
-    if extensions_parser.page_title != apps_parser.page_title:
-      logging.error(
-          'Title differs for apps and extensions: Apps: %s, Extensions: %s.' %
-              (extensions_parser.page_title, apps_parser.page_title))
+    intro_with_links = self._ref_resolver.ResolveAllLinks(
+            intro, namespace=api_name)
+
+    # TODO(kalman): In order to pick up every header tag, and therefore make a
+    # complete TOC, the render context of the Handlebar needs to be passed
+    # through to here. Even if there were a mechanism to do this it would
+    # break caching; we'd need to do the TOC parsing *after* rendering the full
+    # template, and that would probably be expensive.
+    intro_parser = _IntroParser()
+    intro_parser.feed(
+        self._template_renderer.Render(Handlebar(intro_with_links),
+                                       self._request,
+                                       # Avoid nasty surprises.
+                                       data_sources=('partials', 'strings'),
+                                       warn=False))
+
     # The templates will render the heading themselves, so remove it from the
     # HTML content.
     intro_with_links = re.sub(_H1_REGEX, '', intro_with_links, count=1)
-    return {
-      'intro': Handlebar(intro_with_links),
-      'title': apps_parser.page_title,
-      'apps_toc': apps_parser.toc,
-      'extensions_toc': extensions_parser.toc,
+
+    context = {
+      'title': intro_parser.page_title,
+      'toc': intro_parser.toc,
     }
+    return _HandlebarWithContext(intro_with_links, intro_path, context)
 
   def get(self, key):
     path = FormatKey(key)

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

@@ -17,16 +17,16 @@ class IntroDataSourceTest(unittest.TestCase):
   def testIntro(self):
     intro_data_source = IntroDataSource(self._server_instance, None)
     data = intro_data_source.get('test')
-    self.assertEqual('hi', data['title'])
+    self.assertEqual('hi', data.get('title'))
     # TODO(kalman): test links.
     expected_toc = [{'subheadings': [{'link': '', 'title': u'inner'}],
                      'link': '',
                      'title': u'first'},
                     {'subheadings': [], 'link': '', 'title': u'second'}]
-    self.assertEqual(expected_toc, data['apps_toc'])
-    self.assertEqual(expected_toc, data['extensions_toc'])
+    self.assertEqual(expected_toc, data.get('toc'))
     self.assertEqual('you<h2>first</h2><h3>inner</h3><h2>second</h2>',
-                     data['intro'].render().text)
+                     data.Render().text)
+
 
 if __name__ == '__main__':
   unittest.main()

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

@@ -10,6 +10,7 @@ import traceback
 from urlparse import urlsplit
 
 from data_source_registry import CreateDataSources
+from environment import IsPreviewServer
 from file_system import FileNotFoundError
 from redirector import Redirector
 from servlet import Servlet, Response
@@ -47,6 +48,8 @@ class RenderServlet(Servlet):
     try:
       return self._GetSuccessResponse(path, server_instance)
     except FileNotFoundError:
+      if IsPreviewServer():
+        logging.error(traceback.format_exc())
       # 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.

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

@@ -16,21 +16,37 @@ class TemplateRenderer(object):
   def __init__(self, server_instance):
     self._server_instance = server_instance
 
-  def Render(self, template, request):
+  def Render(self, template, request, data_sources=None, warn=True):
+    '''Renders |template| using |request|.
+    Specify |data_sources| to only include the DataSources with the given names
+    when rendering the template.
+    Specify |warn| whether to warn (e.g. logging.warning) if there are template
+    rendering errors. It may be useful to disable this when errors are expected,
+    for example when doing an initial page render to determine document title.
+    '''
     assert isinstance(template, Handlebar), type(template)
-    server_instance = self._server_instance
-    render_context = {
-      'api_list': server_instance.api_list_data_source_factory.Create(),
-      'apis': server_instance.api_data_source_factory.Create(request),
+    render_context = self._CreateDataSources(request)
+    if data_sources is not None:
+      render_context = dict((name, d) for name, d in render_context.iteritems()
+                            if name in data_sources)
+    render_context.update({
       'apps_samples_url': GITHUB_BASE,
-      'base_path': server_instance.base_path,
+      'base_path': self._server_instance.base_path,
       'extensions_samples_url': EXTENSIONS_SAMPLES,
-      'samples': server_instance.samples_data_source_factory.Create(request),
-      'static': server_instance.base_path + 'static',
-    }
-    render_context.update(CreateDataSources(server_instance, request=request))
-    render_data = template.render(render_context)
-    if render_data.errors:
+      'static': self._server_instance.base_path + 'static',
+    })
+    render_data = template.Render(render_context)
+    if warn and render_data.errors:
       logging.error('Handlebar error(s) rendering %s:\n%s' %
           (template._name, '  \n'.join(render_data.errors)))
     return render_data.text
+
+  def _CreateDataSources(self, request):
+    server_instance = self._server_instance
+    data_sources = CreateDataSources(server_instance, request=request)
+    data_sources.update({
+      'api_list': server_instance.api_list_data_source_factory.Create(),
+      'apis': server_instance.api_data_source_factory.Create(request),
+      'samples': server_instance.samples_data_source_factory.Create(request),
+    })
+    return data_sources

chrome/common/extensions/docs/templates/articles/manifest.html

 <h1>Manifest File Format</h1>
 
-
 <p>
 Every extension, installable web app, and theme has a
 <a href="http://www.json.org">JSON</a>-formatted manifest file,

chrome/common/extensions/docs/templates/private/api_table.html

@@ -4,9 +4,9 @@
    <tr>
      <td><a href="{{a.name}}.html">{{a.name}}</a></td>
      <td>{{{a.description}}}</td>
-     {{?a.is_stable}}
+     {{?is_stable}}
      <td>{{a.version}}</td>
-     {{/a.is_stable}}
+     {{/is_stable}}
    </tr>
   {{/apis}}
 </table>

chrome/common/extensions/docs/templates/private/standard_apps_api.html

@@ -13,21 +13,20 @@
         {{?api.channelWarning.trunk +partials.warning_trunk/}}
         {{?api.channelWarning.dev +partials.warning_dev/}}
         {{?api.channelWarning.beta +partials.warning_beta/}}
-        {{?intro.apps_toc}}
-        {{+partials.table_of_contents toc_items:intro.apps_toc
-                                      has_toc:true
+        {{?intro}}
+        {{+partials.table_of_contents toc_items:intro.toc
                                       api:api
                                       samples_list:api.samples.apps
                                       title:strings.apps_title/}}
-        {{:intro.apps_toc}}
-        {{+partials.table_of_contents has_toc:false
+        {{:intro}}
+        {{+partials.table_of_contents toc_items:false
                                       api:api
                                       samples_list:api.samples.apps
                                       title:strings.apps_title/}}
-        {{/intro.apps_toc}}
+        {{/intro}}
         {{+partials.intro_table api:api/}}
 {{- This is unindented because it contains <pre> tags -}}
-{{?intro +intro.intro is_apps:true/}}
+{{?intro +intro platform:strings.app is_apps:true/}}
         {{+partials.api_reference samples_list:api.samples.apps
                                   title:strings.apps_title
                                   api:api/}}

chrome/common/extensions/docs/templates/private/standard_apps_article.html

@@ -10,21 +10,10 @@
       {{+partials.sidenav items:sidenavs.apps/}}
       <div id="gc-pagecontent">
         <h1 class="page_title">{{article.title}}</h1>
-        {{?article.apps_toc}}
-        {{+partials.table_of_contents toc_items:article.apps_toc
-                                      has_toc:true
+        {{+partials.table_of_contents toc_items:article.toc
                                       title:strings.apps_title/}}
-        {{/article.apps_toc}}
 {{- This may contain <pre> tags so it is not indented -}}
-{{+article.intro
-    apis:apis
-    beta_apis:beta_apis
-    dev_apis:dev_apis
-    manifest_source:manifest_source
-    permissions:permissions
-    platform:strings.app
-    stable_apis:stable_apis
-    is_apps:true/}}
+{{+article platform:strings.app is_apps:true/}}
       </div>
     </div>
   </body>

chrome/common/extensions/docs/templates/private/standard_extensions_api.html

@@ -13,21 +13,20 @@
         {{?api.channelWarning.trunk +partials.warning_trunk/}}
         {{?api.channelWarning.dev +partials.warning_dev/}}
         {{?api.channelWarning.beta +partials.warning_beta/}}
-        {{?intro.extensions_toc}}
-        {{+partials.table_of_contents toc_items:intro.extensions_toc
-                                      has_toc:true
+        {{?intro}}
+        {{+partials.table_of_contents toc_items:intro.toc
                                       api:api
                                       samples_list:api.samples.extensions
                                       title:strings.extensions_title/}}
-        {{:intro.extensions_toc}}
-        {{+partials.table_of_contents has_toc:false
+        {{:intro}}
+        {{+partials.table_of_contents toc_items:false
                                       api:api
                                       samples_list:api.samples.extensions
                                       title:strings.extensions_title/}}
-        {{/intro.extensions_toc}}
+        {{/intro}}
         {{+partials.intro_table api:api/}}
 {{- This is unindented because it contains <pre> tags -}}
-{{?intro +intro.intro is_apps:false/}}
+{{?intro +intro platform:strings.app is_apps:false/}}
         {{+partials.api_reference samples_list:api.samples.extensions
                                   title:strings.extensions_title
                                   api:api/}}

chrome/common/extensions/docs/templates/private/standard_extensions_article.html

@@ -10,21 +10,10 @@
       {{+partials.sidenav items:sidenavs.extensions/}}
       <div id="gc-pagecontent">
         <h1 class="page_title">{{article.title}}</h1>
-        {{?article.extensions_toc}}
-        {{+partials.table_of_contents toc_items:article.extensions_toc
-                                      has_toc:true
+        {{+partials.table_of_contents toc_items:article.toc
                                       title:strings.extensions_title/}}
-        {{/article.extensions_toc}}
 {{- This may contain <pre> tags so it is not indented -}}
-{{+article.intro
-    apis:apis
-    beta_apis:beta_apis
-    dev_apis:dev_apis
-    manifest_source:manifest_source
-    permissions:permissions
-    platform:strings.extension
-    stable_apis:stable_apis
-    is_apps:false/}}
+{{+article platform:strings.extension is_apps:false /}}
       </div>
     </div>
   </body>

chrome/common/extensions/docs/templates/private/table_of_contents.html

 <div id="toc">
   <ol>
-    {{?has_toc}}
+    {{?toc_items}}
     {{#i:toc_items}}
     <li>
       <a href="#{{i.link}}">{{{i.title}}}</a>
@@ -11,7 +11,7 @@
       {{/}}
     </li>
     {{/toc_items}}
-    {{/has_toc}}
+    {{/toc_items}}
     {{?api}}
     <div class="api-reference">
       <li>

chrome/common/extensions/docs/templates/public/apps/api_index.html

 {{+partials.standard_apps_article
-    article:intros.api_index
-    is_apps:true
+    article:(intros.api_index
              stable_apis:api_list.apps.chrome.stable
              beta_apis:api_list.apps.chrome.beta
-    dev_apis:api_list.apps.chrome.dev/}}
+             dev_apis:api_list.apps.chrome.dev) /}}

chrome/common/extensions/docs/templates/public/apps/declare_permissions.html

-{{+partials.standard_apps_article article:intros.declare_permissions
-                                  permissions:permissions.declare_apps/}}
+{{+partials.standard_apps_article
+    article:(intros.declare_permissions
+             permissions:permissions.declare_apps)/}}

chrome/common/extensions/docs/templates/public/apps/experimental.html

 {{+partials.standard_apps_article
-    article:intros.experimental
-    apis:api_list.apps.experimental/}}
+    article:(intros.experimental apis:api_list.apps.experimental) /}}

chrome/common/extensions/docs/templates/public/apps/manifest.html

-{{+partials.standard_apps_article article:intros.manifest
-                                  manifest_source:manifest_source.apps/}}
+{{+partials.standard_apps_article
+    article:(intros.manifest manifest_source:manifest_source.apps) /}}

chrome/common/extensions/docs/templates/public/extensions/api_index.html

 {{+partials.standard_extensions_article 
-    article:intros.api_index 
+    article:(intros.api_index 
              stable_apis:api_list.extensions.chrome.stable 
              beta_apis:api_list.extensions.chrome.beta 
-    dev_apis:api_list.extensions.chrome.dev/}}
+             dev_apis:api_list.extensions.chrome.dev) /}}

chrome/common/extensions/docs/templates/public/extensions/declare_permissions.html

-{{+partials.standard_extensions_article article:intros.declare_permissions
-                                        permissions:permissions.declare_extensions/}}
+{{+partials.standard_extensions_article
+    article:(intros.declare_permissions
+             permissions:permissions.declare_extensions)/}}

chrome/common/extensions/docs/templates/public/extensions/experimental.html

 {{+partials.standard_extensions_article
-    article:intros.experimental
-    apis:api_list.extensions.experimental/}}
+    article:(intros.experimental apis:api_list.extensions.experimental)/}}

chrome/common/extensions/docs/templates/public/extensions/manifest.html

 {{+partials.standard_extensions_article
-    article:intros.manifest
-    manifest_source:manifest_source.extensions/}}
+    article:(intros.manifest manifest_source:manifest_source.extensions) /}}