Docserver: when checking for the existence of an API on a branch, just look at

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

Index: chrome/common/extensions/docs/server2/availability_finder.py
diff --git a/chrome/common/extensions/docs/server2/availability_finder.py b/chrome/common/extensions/docs/server2/availability_finder.py
index e88523b454fe76797af87c71be3a9116a77e36bc..5c03c38d2e0c7dd22c62428b5a9ed262eccd2bbd 100644
--- a/chrome/common/extensions/docs/server2/availability_finder.py
+++ b/chrome/common/extensions/docs/server2/availability_finder.py
@@ -2,7 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import collections
+from collections import Mapping
 import os
 
 from api_schema_graph import APISchemaGraph
@@ -26,7 +26,7 @@ def _GetChannelFromFeatures(api_name, file_system, path):
 
   if feature is None:
     return None
-  if isinstance(feature, collections.Mapping):
+  if isinstance(feature, Mapping):
     # The channel information exists as a solitary dict.
     return feature.get('channel')
   # The channel information dict is nested within a list for whitelisting
@@ -55,38 +55,52 @@ def _GetChannelFromPermissionFeatures(api_name, file_system):
       '%s/_permission_features.json' % API_PATH)
 
 
-def _GetApiSchema(api_name, file_system):
-  '''Searches |file_system| for |api_name|'s API schema data, and parses and
-  returns it if found.
+def _GetApiSchemaFilename(api_name, file_system):
+  '''Gets the name of the file which contains the schema for |api_name| in
+  |file_system|, or None if the API is not found. Note that this may be the
+  single _EXTENSION_API file which all APIs share in older versions of Chrome.
   '''
-  file_names = file_system.ReadSingle('%s/' % API_PATH)
-  # API names can be represented in unix_style and camelCase formats.
-  possibilities = (api_name, UnixName(api_name))
+  def under_api_path(path):
+    return '%s/%s' % (API_PATH, path)
 
-  def get_file_data(file_name):
-    return file_system.ReadSingle('%s/%s' % (API_PATH, file_name))
+  file_names = file_system.ReadSingle(under_api_path(''))
 
   if _EXTENSION_API in file_names:
     # Prior to Chrome version 18, extension_api.json contained all API schema

[Jeffrey Yasskin, 2013/10/02 00:19:17]

Do we still need this code?

[not at google - send to devlin, 2013/10/02 00:40:02]

Yes, this is the class responsible for stepping back through history to find out
when APIs were added, changed, etc. It specifically needs to deal with legacy
cases.

     # data, which replaced the current implementation of individual API files.
+    # We're forced to parse this (very large) file to determine if the API
+    # exists in it.
     #
-    # TODO(epeterson) This file will be parsed a lot, but the data remains the
-    # same for each API. Avoid doing unnecessary work by re-parsing.
-    # (see http://crbug.com/295812)
-    extension_api_json = json_parse.Parse(get_file_data(_EXTENSION_API))
-    api = [api for api in extension_api_json if api['namespace'] == api_name]
-    return api if api else None
+    # TODO(epeterson) Avoid doing unnecessary work by re-parsing.
+    # See http://crbug.com/295812.
+    extension_api_json = json_parse.Parse(
+        file_system.ReadSingle('%s/%s'% (API_PATH, _EXTENSION_API)))
+    if any(api['namespace'] == api_name for api in extension_api_json):
+      return under_api_path(_EXTENSION_API)
+    return None
+
+  api_file_names = [
+      file_name for file_name in file_names
+      if os.path.splitext(file_name)[0] in (api_name, UnixName(api_name))]
+  assert len(api_file_names) < 2
+  return under_api_path(api_file_names[0]) if api_file_names else None
+
 
-  def check_file(file_name):
-    return os.path.splitext(file_name)[0] in (api_name, UnixName(api_name))
+def _GetApiSchema(api_name, file_system):
+  '''Searches |file_system| for |api_name|'s API schema data, and parses and
+  returns it if found.
+  '''
+  api_file_name = _GetApiSchemaFilename(api_name, file_system)
+  if api_file_name is None:
+    return None
 
-  for file_name in file_names:
-    if check_file(file_name):
-      if file_name.endswith('idl'):
-        idl_data = idl_parser.IDLParser().ParseData(get_file_data(file_name))
-        return idl_schema.IDLSchema(idl_data).process()
-      return json_parse.Parse(get_file_data(file_name))
-  return None
+  api_file_text = file_system.ReadSingle(api_file_name)
+  if api_file_name == _EXTENSION_API:
+    return [api for api in json_parse.Parse(api_file_text)
+            if api['namespace'] == api_name][0]

[Jeffrey Yasskin, 2013/10/02 00:19:17]

It looks like the old code would return None if there were no matching API name.
This will throw an exception instead. Is that a good idea?

[not at google - send to devlin, 2013/10/02 00:40:02]

It still does return None; _GetApiSchemaFilename would return None if it's not
found in extension_api.json, so this method would return None. It might as well
crash here.

HOWEVER I actually think this code is kind of wrong anyway (see below), but I'll
write it in such a way to make that clearer.

+  if api_file_name.endswith('.idl'):
+    return idl_parser.IDLParser().ParseData(api_file_text)
+  return json_parse.Parse(api_file_text)  # [0]?

[Jeffrey Yasskin, 2013/10/02 00:19:17]

Figure out the "[0]?" before checking this in.

[not at google - send to devlin, 2013/10/02 00:40:02]

Done.

 
 
 class AvailabilityFinder(object):
@@ -139,7 +153,7 @@ class AvailabilityFinder(object):
       # Fall back to a check for file system existence if the API is not
       # stable in any of the _features.json files, or if the _features files
       # do not exist (version 19 and earlier).
-      return _GetApiSchema(api_name, file_system) is not None
+      return _GetApiSchemaFilename(api_name, file_system) is not None

[Jeffrey Yasskin, 2013/10/02 00:19:17]

Replace these with "ApiExists(api_name, file_system)" or something similar,
rather than encoding that in a filename is not None check.

[not at google - send to devlin, 2013/10/02 00:40:02]

Done.

 
   def _CheckChannelAvailability(self, api_name, file_system, channel_name):
     '''Searches through the _features files in a given |file_system| and
@@ -155,7 +169,7 @@ class AvailabilityFinder(object):
         or _GetChannelFromPermissionFeatures(api_name, features_fs)
         or _GetChannelFromManifestFeatures(api_name, features_fs))
     if (available_channel is None and
-        _GetApiSchema(api_name, file_system) is not None):
+        _GetApiSchemaFilename(api_name, file_system) is not None):
       # If an API is not represented in any of the _features files, but exists
       # in the filesystem, then assume it is available in this version.
       # The windows API is an example of this.
@@ -208,6 +222,7 @@ class AvailabilityFinder(object):
     if availability_graph is not None:
       return availability_graph
 
+
     availability_graph = APISchemaGraph()
     trunk_graph = APISchemaGraph(_GetApiSchema(api_name,
                                                self._host_file_system))
@@ -225,9 +240,8 @@ class AvailabilityFinder(object):
       # version and trunk.
       return trunk_graph != version_graph
 
-    self._file_system_iterator.Ascending(
-        self.GetApiAvailability(api_name),
-        update_availability_graph)
+    self._file_system_iterator.Ascending(self.GetApiAvailability(api_name),
+                                         update_availability_graph)
 
     self._node_level_object_store.Set(api_name, availability_graph)
     return availability_graph