Docserver: Consolidate features caching and access.

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

Index: chrome/common/extensions/docs/server2/api_list_data_source.py
diff --git a/chrome/common/extensions/docs/server2/api_list_data_source.py b/chrome/common/extensions/docs/server2/api_list_data_source.py
index 2e35ad631fb6a9b14e94c954b843e25d6fef53a5..4c8868cda6028f843792e209e61170dac082a761 100644
--- a/chrome/common/extensions/docs/server2/api_list_data_source.py
+++ b/chrome/common/extensions/docs/server2/api_list_data_source.py
@@ -4,6 +4,7 @@
 
 import os
 
+from third_party.json_schema_compiler.json_parse import Parse
 import third_party.json_schema_compiler.model as model
 import docs_server_utils as utils
 
@@ -11,75 +12,128 @@ class APIListDataSource(object):
   """ This class creates a list of chrome.* APIs and chrome.experimental.* APIs
   for extensions and apps that are used in the api_index.html and
   experimental.html pages.
-  |api_path| is the path to the API schemas.
-  |public_path| is the path to the public HTML templates.
-  An API is considered listable if it's in both |api_path| and |public_path| -
-  the API schemas may contain undocumentable APIs, and the public HTML templates
-  will contain non-API articles.
+  |file_system| is the host file system.
+  |public_template_path| is the relative path to public API HTML templates.
+  |api_features_path| is the path to _api_features.json.
+  |manifest_features_path| is the path to _manifest_features.json.
+  |permission_features_path| is the path to _permission_features.json.
+
+  An API is considered listable if it is listed in _api_features.json,
+  it has a corresponding HTML file in the public template path, and one of
+  the following conditions is met:
+    - It has no "dependencies" or "extension_types" properties in _api_features
+    - It has an "extension_types" property in _api_features with either/both
+      "extension"/"platform_app" values present.
+    - It has a dependency in _{api,manifest,permission}_features with an
+      "extension_types" property where either/both "extension"/"platform_app"
+      values are present.
   """
   class Factory(object):
-    def __init__(self, compiled_fs_factory, file_system, api_path, public_path):
-      self._compiled_fs = compiled_fs_factory.Create(self._ListAPIs,
-                                                     APIListDataSource)
+    def __init__(self,
+                 file_system,
+                 public_template_path,
+                 api_features_path,
+                 manifest_features_path,
+                 permission_features_path):
       self._file_system = file_system
-      def Normalize(string):
+      def NormalizePath(string):
         return string if string.endswith('/') else (string + '/')
-      self._api_path = Normalize(api_path)
-      self._public_path = Normalize(public_path)
+      self._public_template_path = NormalizePath(public_template_path)
+      self._api_features = Parse(file_system.ReadSingle(api_features_path))
+      self._manifest_features = Parse(
+          file_system.ReadSingle(manifest_features_path))
+      self._permission_features = Parse(
+          file_system.ReadSingle(permission_features_path))

[not at google - send to devlin, 2013/09/19 19:21:52]

It's preferable to do heavy lifting outside construction of these objects; all
model objects are aggressively cached (file system results etc) so that's fine,
and I'm not 100% confident that thing won't break if we run code during the
construction of a ServerInstance. Even so, best not to.

However, these features also need to be parsed. And indeed, they're being parsed
elsewhere.

Perhaps we should add a class like FeaturesBundle which has parsed features
files for manifest, permissions, and API (manifest using manifest_features, the
others using features_utility). Check out manifest data source; basically pull
that into a separate class. Manifest features need special treatment; I'm not
sure if the others do yet. Not yet, though I think they eventually will.

That will clean up code in a few places: here, manfiest_data_source, and
availablity_finder.

+      self._public_apis = {
+        'apps': self._GetPublicAPIs('apps'),
+        'extensions': self._GetPublicAPIs('extensions')
+      }
+      self._apis = self._GenerateAPIDict()
 
-    def _GetAPIsInSubdirectory(self, api_names, doc_type):
+    def _GetPublicAPIs(self, platform):
       public_templates = []
       for root, _, files in self._file_system.Walk(
-          self._public_path + doc_type):
+          self._public_template_path + platform):
         public_templates.extend(
             ('%s/%s' % (root, name)).lstrip('/') for name in files)
       template_names = set(os.path.splitext(name)[0]
                            for name in public_templates)
-      experimental_apis = []
-      chrome_apis = []
-      private_apis = []
-      for template_name in sorted(template_names):
-        if model.UnixName(template_name) not in api_names:
-          continue
-        entry = {'name': template_name.replace('_', '.')}
-        if template_name.startswith('experimental'):
-          experimental_apis.append(entry)
-        elif template_name.endswith('Private'):
-          private_apis.append(entry)
-        else:
-          chrome_apis.append(entry)
-      if len(chrome_apis):
-        chrome_apis[-1]['last'] = True
-      if len(experimental_apis):
-        experimental_apis[-1]['last'] = True
-      if len(private_apis):
-        private_apis[-1]['last'] = True
-      return {
-        'chrome': chrome_apis,
-        'experimental': experimental_apis,
-        'private': private_apis
+      return map(lambda name: name.replace('_', '.'), template_names)
+
+    def _GetExtensionTypesForDependency(self, dependency):
+      feature_sources = {
+        'api': self._api_features,
+        'manifest': self._manifest_features,
+        'permission': self._permission_features
       }
+      feature_type, feature_name = dependency.split(':')

[not at google - send to devlin, 2013/09/19 19:21:52]

This is the sort of stuff that feature_utility c/should handle if it doesn't
already.

+      feature = feature_sources[feature_type].get(feature_name, None)
+      if feature is None:
+        logging.error('Unable to resolve feature dependency %s' % dependency)
+        return []
+      if type(feature) is list:

[not at google - send to devlin, 2013/09/19 19:21:52]

isinstance(feature, list):

but again feature_utility should handle it.

+        extension_types = []
+        for branch in feature:
+          extension_types.extend(branch.get('extension_types', []))
+      else:
+        extension_types = feature.get('extension_types', [])
+      return extension_types
+
+    def _GetAPIPlatforms(self, api):
+      def MapExtensionTypesToAPIPlatforms(extension_types):
+        mapping = { 'extension': 'extensions', 'platform_app': 'apps' }
+        platforms = set()
+        for extension_type in extension_types:
+          if extension_type in mapping:
+            platforms.add(mapping[extension_type])
+        return platforms
+      extension_types = api.get('extension_types', None)
+      if extension_types is not None:
+        return MapExtensionTypesToAPIPlatforms(extension_types)
+      platforms = set(['apps', 'extensions'])

[not at google - send to devlin, 2013/09/19 19:21:52]

when creating fixed-length lists use (...) not [...]. Involves double-parens
here but whatever.

[Ken Rockot(use gerrit already), 2013/09/20 20:53:37]

Done.

+      for dependency in api.get('dependencies', []):

[not at google - send to devlin, 2013/09/19 19:21:52]

() not []

[Ken Rockot(use gerrit already), 2013/09/20 20:53:37]

Done.

+        platforms = platforms.intersection(MapExtensionTypesToAPIPlatforms(
+            self._GetExtensionTypesForDependency(dependency)))
+      return list(platforms)
+
+    def _GetAPICategory(self, api, platform):
+      name = api['name']
+      if name.startswith('experimental.'):
+        return 'experimental'
+      elif (name.endswith('Private') or
+           name not in self._public_apis[platform]):
+        return 'private'
+      else:
+        return 'chrome'

[not at google - send to devlin, 2013/09/19 19:21:52]

no else's after return's.

[Ken Rockot(use gerrit already), 2013/09/20 20:53:37]

Done.

 
-    def _ListAPIs(self, base_dir, apis):
-      api_names = set(utils.SanitizeAPIName(name) for name in apis)
-      return {
-        'apps': self._GetAPIsInSubdirectory(api_names, 'apps'),
-        'extensions': self._GetAPIsInSubdirectory(api_names, 'extensions')
+    def _GenerateAPIDict(self):
+      def MakeEmptyAPIDict():
+        return { 'chrome': [], 'experimental': [], 'private': [] }
+      api_dict = {
+        'apps': MakeEmptyAPIDict(),
+        'extensions': MakeEmptyAPIDict()
       }
+      for (name, api) in self._api_features.items():

[not at google - send to devlin, 2013/09/19 19:21:52]

name, api not (name, api)

iteritems not items.

[Ken Rockot(use gerrit already), 2013/09/20 20:53:37]

Done.

+        api['name'] = name
+        for platform in self._GetAPIPlatforms(api):
+          api_dict[platform][self._GetAPICategory(api, platform)].append(api)
+      for platform in api_dict.keys():

[not at google - send to devlin, 2013/09/19 19:21:52]

iterkeys

[Ken Rockot(use gerrit already), 2013/09/20 20:53:37]

Done.

+        for category in api_dict[platform]:
+          api_dict[platform][category][-1]['last'] = True
+      return api_dict
 
     def Create(self):
-      return APIListDataSource(self._compiled_fs, self._api_path)
+      return APIListDataSource(self._apis)
 
-  def __init__(self, compiled_fs, api_path):
-    self._compiled_fs = compiled_fs
-    self._api_path = api_path
+  def __init__(self, apis):
+    self._apis = apis
 
   def GetAllNames(self):
-    names = []
+    apis = []
     for platform in ['apps', 'extensions']:
       for category in ['chrome', 'experimental', 'private']:
-        names.extend(self.get(platform).get(category))
-    return [api_name['name'] for api_name in names]
+        apis.extend(self._apis[platform][category])
+    return [api['name'] for api in apis]
 
   def get(self, key):
-    return self._compiled_fs.GetFromFileListing(self._api_path)[key]
+    return self._apis.get(key)