Doc server manifest page generation

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

Index: chrome/common/extensions/docs/server2/intro_data_source.py
diff --git a/chrome/common/extensions/docs/server2/intro_data_source.py b/chrome/common/extensions/docs/server2/intro_data_source.py
index 53034de158b96cc522a2abb5cde5c5cfb00ca76a..3b86bbbdaf8a29f8779f2236b339579c6dd04fbe 100644
--- a/chrome/common/extensions/docs/server2/intro_data_source.py
+++ b/chrome/common/extensions/docs/server2/intro_data_source.py
@@ -3,13 +3,13 @@
 # found in the LICENSE file.
 
 from HTMLParser import HTMLParser
+import functools

[not at google - send to devlin, 2013/05/03 15:56:25]

f < H

[jshumway, 2013/05/10 02:08:36]

Done.

 import logging
 import os
 import re
 
 from docs_server_utils import FormatKey
 from file_system import FileNotFoundError
-import compiled_file_system as compiled_fs
 from third_party.handlebar import Handlebar
 
 # TODO(kalman): rename this HTMLDataSource or other, then have separate intro
@@ -59,61 +59,85 @@ class _IntroParser(HTMLParser):
     elif self._recent_tag in ['h2', 'h3']:
       self._current_heading['title'] += data
 
+def _MakeIntroDict(ref_resolver, 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 = ref_resolver.ResolveAllLinks(intro, namespace=api_name)
+  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))
+  # 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,
+  }
+
 class IntroDataSource(object):
-  """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.
+  """ Allows a template to access the contents of a directory or one of its
+  subdirectories.
+
+  Each IntroDataSource has a dictionary of sub IntroDataSources that are used to
+  access files in subdirectories. Each child IDS shares a cache and identity_fs
+  with its parent. If subdirectory paths were added to base_paths instead,
+  lookup time would have really bad time complexity.
   """
   class Factory(object):
     def __init__(self, compiled_fs_factory, ref_resolver_factory, base_paths):
-      self._cache = compiled_fs_factory.Create(self._MakeIntroDict,
-                                               IntroDataSource)
-      self._ref_resolver = ref_resolver_factory.Create()
+      self._cache = compiled_fs_factory.Create(
+        functools.partial(_MakeIntroDict, ref_resolver_factory.Create()),
+        IntroDataSource)
       self._base_paths = base_paths
-
-    def _MakeIntroDict(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)
-      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))
-      # 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,
-      }
+      self._identity_fs = compiled_fs_factory.CreateIdentity(IntroDataSource)

[not at google - send to devlin, 2013/05/03 15:56:25]

hm ok, I don't think any of the changes to this file are actually needed after I
submit https://codereview.chromium.org/14856006/ which changes preview.py to use
LocalFileSystem. The bug appears to be that something in the fake fetchers is
failing to resolve the /, whereas I just tested this and it works without
changes within that patch.

one thing to check though - that SubversionFileSystem can actually handle / in
paths. Could you add a test in there?

[jshumway, 2013/05/10 02:08:36]

I reverted my changes to this file and everything worked. I also added the test.

 
     def Create(self):
-      return IntroDataSource(self._cache, self._base_paths)
+      return IntroDataSource(self._cache, self._identity_fs, self._base_paths)
 
-  def __init__(self, cache, base_paths):
+  def __init__(self, cache, identity_fs, base_paths):
     self._cache = cache
     self._base_paths = base_paths
+    self._identity_fs = identity_fs
+
+    # Datasources for subdirectories.
+    self._subIDS = {}
 
   def get(self, key):
     path = FormatKey(key)
+
     def get_from_base_path(base_path):
-      return self._cache.GetFromFile('%s/%s' % (base_path, path))
+      return self._cache.GetFromFile(os.path.join(base_path, path))

[not at google - send to devlin, 2013/05/03 15:56:25]

caches always use '/' as a path separator, the classes that care about OS paths
(e.g. LocalFileSystem) convert it themselves.

[jshumway, 2013/05/10 02:08:36]

My bad

+
     for base_path in self._base_paths:
       try:
         return get_from_base_path(base_path)
       except FileNotFoundError:
         continue
+
+    # File was not found, check if key is a subdirectory.
+    for base_path in self._base_paths:
+      subpath = os.path.join(base_path, key)
+      try:
+        subfiles = self._identity_fs.GetFromFileListing(subpath)
+      except FileNotFoundError:
+        continue
+
+      if subfiles:
+        self._subIDS[subpath] = IntroDataSource(
+          self._cache, self._identity_fs, [subpath])
+        return self._subIDS[subpath]
+
     # Not found. Do the first operation again so that we get a stack trace - we
     # know that it'll fail.
     get_from_base_path(self._base_paths[0])
-    raise AssertionError()
+    raise AssertionError