Docserver: Add more support for object level availability in templates

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

Index: chrome/common/extensions/docs/server2/api_data_source.py
diff --git a/chrome/common/extensions/docs/server2/api_data_source.py b/chrome/common/extensions/docs/server2/api_data_source.py
index 6fdc6f6abd6d575845b47cd4a2e6e8c866077d3c..9ae272b83a29c5c49a432cd993fa31dab07effdf 100644
--- a/chrome/common/extensions/docs/server2/api_data_source.py
+++ b/chrome/common/extensions/docs/server2/api_data_source.py
@@ -98,11 +98,11 @@ class _APINodeCursor(object):
     self._node_availabilities = availability_finder.GetAPINodeAvailability(
         namespace_name)
     self._namespace_name = namespace_name
+    self._ignored_categories = []
 
   def _AssertIsValidCategory(self, category):
     assert category in _NODE_CATEGORIES, \
-        '%s is not a valid category. Full path: %s' % (category,
-                                                       self._lookup_path)
+        '%s is not a valid category. Full path: %s' % (category, str(self))
 
   def _GetParentPath(self):
     '''Returns the path pointing to this node's parent.
@@ -127,19 +127,18 @@ class _APINodeCursor(object):
     self._AssertIsValidCategory(self._lookup_path[-2])
     return self._lookup_path[:-2]
 
-  def _LookupNodeAvailability(self):
+  def _LookupNodeAvailability(self, lookup_path):
     '''Returns the ChannelInfo object for this node.
     '''
     return self._node_availabilities.Lookup(self._namespace_name,
-                                            *self._lookup_path).annotation
+                                            *lookup_path).annotation
 
   def _LookupParentNodeAvailability(self):

[not at google - send to devlin, 2014/07/07 23:00:07]

i don't see this called from anywhere?

     '''Returns the ChannelInfo object for this node's parent.
     '''
-    return self._node_availabilities.Lookup(self._namespace_name,
-                                            *self._GetParentPath()).annotation
+    return self._LookupNodeAvailability(self._GetParentPath())
 
-  def _CheckNamespacePrefix(self):
+  def _CheckNamespacePrefix(self, lookup_path):
     '''API schemas may prepend the namespace name to top-level types
     (e.g. declarativeWebRequest > types > declarativeWebRequest.IgnoreRules),
     but just the base name (here, 'IgnoreRules') will be in the |lookup_path|.
@@ -147,63 +146,73 @@ class _APINodeCursor(object):
     '''
     # lookup_path[0] is always the node category (e.g. types, functions, etc.).
     # Thus, lookup_path[1] is always the top-level node name.
-    self._AssertIsValidCategory(self._lookup_path[0])
-    base_name = self._lookup_path[1]
-    self._lookup_path[1] = '%s.%s' % (self._namespace_name, base_name)
+    self._AssertIsValidCategory(lookup_path[0])
+    base_name = lookup_path[1]
+    lookup_path[1] = '%s.%s' % (self._namespace_name, base_name)
     try:
-      node_availability = self._LookupNodeAvailability()
+      node_availability = self._LookupNodeAvailability(lookup_path)
       if node_availability is not None:
         return node_availability
     finally:
       # Restore lookup_path.
-      self._lookup_path[1] = base_name
+      lookup_path[1] = base_name
     return None
 
-  def _CheckEventCallback(self):
+  def _CheckEventCallback(self, lookup_path):
     '''Within API schemas, an event has a list of 'properties' that the event's
     callback expects. The callback itself is not explicitly represented in the
     schema. However, when creating an event node in _JSCModel, a callback node
     is generated and acts as the parent for the event's properties.
     Modify |lookup_path| to check the original schema format.
     '''
-    if 'events' in self._lookup_path:
-      assert 'callback' in self._lookup_path
-      callback_index = self._lookup_path.index('callback')
+    if 'events' in lookup_path:
+      assert 'callback' in lookup_path, str(self)

[not at google - send to devlin, 2014/07/07 23:00:07]

I think if you define __repr__ rather than (or in addition to) __str__ then you
can just use |self| here not |str(self)|

+      callback_index = lookup_path.index('callback')
       try:
-        self._lookup_path.pop(callback_index)
-        node_availability = self._LookupNodeAvailability()
+        lookup_path.pop(callback_index)
+        node_availability = self._LookupNodeAvailability(lookup_path)
       finally:
-        self._lookup_path.insert(callback_index, 'callback')
+        lookup_path.insert(callback_index, 'callback')
       return node_availability
     return None
 
-  def _LookupAvailability(self):
+  def _LookupAvailability(self, lookup_path):
     '''Runs all the lookup checks on self._lookup_path and
     returns the node availability if found, None otherwise.
     '''
     for lookup in (self._LookupNodeAvailability,
                    self._CheckEventCallback,
                    self._CheckNamespacePrefix):
-      node_availability = lookup()
+      node_availability = lookup(lookup_path)
       if node_availability is not None:
         return node_availability
     return None
 
+  def _GetCategory(self):
+    '''In addition to the defined _NODE_CATEGORIES, this function
+    may return 'parameters' as a category.

[not at google - send to devlin, 2014/07/07 23:00:06]

it doesn't look like this function can ever return 'parameters'

+    '''
+    if self._lookup_path[-2] in _NODE_CATEGORIES:
+      return self._lookup_path[-2]
+    if self._lookup_path[-1] == 'callback':

[not at google - send to devlin, 2014/07/07 23:00:06]

maybe add a quick comment why this is the case

+      return 'events'
+    if (self._lookup_path[-2] == 'parameters' or
+        self._lookup_path[-1].endswith('Type') or
+        'events' in self._lookup_path):
+      return 'properties'

[not at google - send to devlin, 2014/07/07 23:00:06]

again this check is quite specific (especially the 'Type' check - is there a
more principled way to write that?)

+    raise AssertionError('Could not classify node %s' % str(self))
+
   def GetAvailability(self):
     '''Returns availability information for this node.
     '''
-    node_availability = self._LookupAvailability()
+    if self._GetCategory() in self._ignored_categories:
+      return None
+    node_availability = self._LookupAvailability(self._lookup_path)
     if node_availability is None:
-      logging.warning('No availability found for: %s > %s' %
-          (self._namespace_name, ' > '.join(self._lookup_path)))
+      logging.warning('No availability found for: %s' % str(self))

[not at google - send to devlin, 2014/07/07 23:00:06]

I don't think you need the extra "str"-ing, right?

       return None
 
-    try:
-      current_path = self._lookup_path
-      self._lookup_path = self._GetParentPath()
-      parent_node_availability = self._LookupAvailability()
-    finally:
-      self._lookup_path = current_path
+    parent_node_availability = self._LookupAvailability(self._GetParentPath())
     # If the parent node availability couldn't be found, something
     # is very wrong.
     assert parent_node_availability is not None
@@ -214,16 +223,33 @@ class _APINodeCursor(object):
       return None
     return node_availability
 
-  def Descend(self, *path):
+  def Descend(self, *path, **kwargs):

[not at google - send to devlin, 2014/07/07 23:00:07]

annoying that you can't just specify 'ignore' directly. but a neater way to
implement this I think would be to go, on line 228,

ignore = kwargs.get('ignore')

and also please comment what "ignore" is in the docstring.

     '''Moves down the APISchemaGraph, following |path|.
     '''
     class scope(object):
+      def __init__(self2):
+        if kwargs:
+          assert 'ignore' in kwargs
+          self2._ignore = kwargs['ignore']

[not at google - send to devlin, 2014/07/07 23:00:06]

you shouldn't need to actually use self2 here at all, just store "ignore" as
part of the outer function scope.

+        else:
+          self2._ignore = ()
+
       def __enter__(self2):
+        if self2._ignore:
+          self._ignored_categories.extend(self2._ignore)
         self._lookup_path.extend(path)

[not at google - send to devlin, 2014/07/07 23:00:06]

if you're checking |path| below you might as well here too, for consistency.
doesn't make a whole lot of difference though of course.

+
       def __exit__(self2, _, __, ___):
-        self._lookup_path[:] = self._lookup_path[:-len(path)]
+        if self2._ignore:
+          self._ignored_categories[:] = (
+              self._ignored_categories[:-len(self2._ignore)])
+        if path:
+          self._lookup_path[:] = self._lookup_path[:-len(path)]
     return scope()
 
+  def __str__(self):
+    return '%s > %s' % (self._namespace_name, ' > '.join(self._lookup_path))
+
 
 class _JSCModel(object):
   '''Uses a Model from the JSON Schema Compiler and generates a dict that
@@ -300,6 +326,7 @@ class _JSCModel(object):
         'functions': self._GenerateFunctions(type_.functions),
         'events': self._GenerateEvents(type_.events),
         'id': _CreateId(type_, 'type'),
+        'availability': self._GetAvailabilityTemplate()
       }
       self._RenderTypeInformation(type_, type_dict)
       return type_dict
@@ -309,7 +336,8 @@ class _JSCModel(object):
       return [self._GenerateFunction(f) for f in functions.values()]
 
   def _GenerateFunction(self, function):
-    with self._current_node.Descend(function.simple_name):
+    with self._current_node.Descend(function.simple_name,
+                                    ignore=('types', 'properties')):
       function_dict = {
         'name': function.simple_name,
         'description': function.description,
@@ -322,8 +350,10 @@ class _JSCModel(object):
       self._AddCommonProperties(function_dict, function)
       if function.returns:
         function_dict['returns'] = self._GenerateType(function.returns)
-      for param in function.params:
-        function_dict['parameters'].append(self._GenerateProperty(param))
+    with self._current_node.Descend(function.simple_name):
+      with self._current_node.Descend('parameters'):
+        for param in function.params:
+          function_dict['parameters'].append(self._GenerateProperty(param))
       if function.callback is not None:
         # Show the callback as an extra parameter.
         function_dict['parameters'].append(
@@ -343,11 +373,12 @@ class _JSCModel(object):
               if e.supports_dom]
 
   def _GenerateEvent(self, event):
-    with self._current_node.Descend(event.simple_name):
+    with self._current_node.Descend(event.simple_name, ignore=('properties',)):
       event_dict = {
         'name': event.simple_name,
         'description': event.description,
-        'filters': [self._GenerateProperty(f) for f in event.filters],
+        'filters': [self._GenerateProperty(f)
+                    for f in event.filters],

[not at google - send to devlin, 2014/07/07 23:00:07]

this can go back to 1 line?

         'conditions': [self._GetLink(condition)
                        for condition in event.conditions],
         'actions': [self._GetLink(action) for action in event.actions],
@@ -356,6 +387,7 @@ class _JSCModel(object):
         'properties': [],
         'id': _CreateId(event, 'event'),
         'byName': {},
+        'availability': self._GetAvailabilityTemplate()
       }
       self._AddCommonProperties(event_dict, event)
       # Add the Event members to each event in this object.
@@ -395,58 +427,68 @@ class _JSCModel(object):
       'optional': callback.optional,
       'parameters': []
     }
-    for param in callback.params:
-      callback_dict['parameters'].append(self._GenerateProperty(param))
+    with self._current_node.Descend('parameters',
+                                    callback.simple_name,
+                                    'parameters'):
+      for param in callback.params:
+        callback_dict['parameters'].append(self._GenerateProperty(param))
     if (len(callback_dict['parameters']) > 0):
       callback_dict['parameters'][-1]['last'] = True
     return callback_dict
 
   def _GenerateProperties(self, properties):
-    return [self._GenerateProperty(v) for v in properties.values()]
+    with self._current_node.Descend('properties'):
+      return [self._GenerateProperty(v) for v in properties.values()]
 
   def _GenerateProperty(self, property_):
-    if not hasattr(property_, 'type_'):
-      for d in dir(property_):
-        if not d.startswith('_'):
-          print ('%s -> %s' % (d, getattr(property_, d)))
-    type_ = property_.type_
-
-    # Make sure we generate property info for arrays, too.
-    # TODO(kalman): what about choices?
-    if type_.property_type == model.PropertyType.ARRAY:
-      properties = type_.item_type.properties
-    else:
-      properties = type_.properties
-
-    property_dict = {
-      'name': property_.simple_name,
-      'optional': property_.optional,
-      'description': property_.description,
-      'properties': self._GenerateProperties(type_.properties),
-      'functions': self._GenerateFunctions(type_.functions),
-      'parameters': [],
-      'returns': None,
-      'id': _CreateId(property_, 'property'),
-    }
-    self._AddCommonProperties(property_dict, property_)
-
-    if type_.property_type == model.PropertyType.FUNCTION:
-      function = type_.function
-      for param in function.params:
-        property_dict['parameters'].append(self._GenerateProperty(param))
-      if function.returns:
-        property_dict['returns'] = self._GenerateType(function.returns)
+    with self._current_node.Descend(property_.simple_name,
+                                    ignore=('properties',)):
+      if not hasattr(property_, 'type_'):
+        for d in dir(property_):
+          if not d.startswith('_'):
+            print ('%s -> %s' % (d, getattr(property_, d)))
+      type_ = property_.type_
+
+      # Make sure we generate property info for arrays, too.
+      # TODO(kalman): what about choices?
+      if type_.property_type == model.PropertyType.ARRAY:
+        properties = type_.item_type.properties
+      else:
+        properties = type_.properties
 
-    value = property_.value
-    if value is not None:
-      if isinstance(value, int):
-        property_dict['value'] = _FormatValue(value)
+      property_dict = {
+        'name': property_.simple_name,
+        'optional': property_.optional,
+        'description': property_.description,
+        'properties': self._GenerateProperties(type_.properties),
+        'functions': self._GenerateFunctions(type_.functions),
+        'parameters': [],
+        'returns': None,
+        'id': _CreateId(property_, 'property'),
+        'availability': self._GetAvailabilityTemplate()
+      }
+      self._AddCommonProperties(property_dict, property_)
+
+    with self._current_node.Descend(property_.simple_name):
+      if type_.property_type == model.PropertyType.FUNCTION:
+        function = type_.function
+        with self._current_node.Descend('parameters'):
+          for param in function.params:
+            property_dict['parameters'].append(self._GenerateProperty(param))
+        if function.returns:
+          with self._current_node.Descend(ignore=('types', 'properties')):
+            property_dict['returns'] = self._GenerateType(function.returns)
+
+      value = property_.value
+      if value is not None:
+        if isinstance(value, int):
+          property_dict['value'] = _FormatValue(value)
+        else:
+          property_dict['value'] = value
       else:
-        property_dict['value'] = value
-    else:
-      self._RenderTypeInformation(type_, property_dict)
+        self._RenderTypeInformation(type_, property_dict)
 
-    return property_dict
+      return property_dict
 
   def _GenerateCallbackProperty(self, callback):
     property_dict = {
@@ -463,27 +505,28 @@ class _JSCModel(object):
     return property_dict
 
   def _RenderTypeInformation(self, type_, dst_dict):
-    dst_dict['is_object'] = type_.property_type == model.PropertyType.OBJECT
-    if type_.property_type == model.PropertyType.CHOICES:
-      dst_dict['choices'] = self._GenerateTypes(type_.choices)
-      # We keep track of which == last for knowing when to add "or" between
-      # choices in templates.
-      if len(dst_dict['choices']) > 0:
-        dst_dict['choices'][-1]['last'] = True
-    elif type_.property_type == model.PropertyType.REF:
-      dst_dict['link'] = self._GetLink(type_.ref_type)
-    elif type_.property_type == model.PropertyType.ARRAY:
-      dst_dict['array'] = self._GenerateType(type_.item_type)
-    elif type_.property_type == model.PropertyType.ENUM:
-      dst_dict['enum_values'] = [
-          {'name': value.name, 'description': value.description}
-          for value in type_.enum_values]
-      if len(dst_dict['enum_values']) > 0:
-        dst_dict['enum_values'][-1]['last'] = True
-    elif type_.instance_of is not None:
-      dst_dict['simple_type'] = type_.instance_of
-    else:
-      dst_dict['simple_type'] = type_.property_type.name
+    with self._current_node.Descend(*[], ignore=('types', 'properties')):

[not at google - send to devlin, 2014/07/07 23:00:06]

I don't think the *[] is necessary.

+      dst_dict['is_object'] = type_.property_type == model.PropertyType.OBJECT
+      if type_.property_type == model.PropertyType.CHOICES:
+        dst_dict['choices'] = self._GenerateTypes(type_.choices)
+        # We keep track of which == last for knowing when to add "or" between
+        # choices in templates.
+        if len(dst_dict['choices']) > 0:
+          dst_dict['choices'][-1]['last'] = True
+      elif type_.property_type == model.PropertyType.REF:
+        dst_dict['link'] = self._GetLink(type_.ref_type)
+      elif type_.property_type == model.PropertyType.ARRAY:
+        dst_dict['array'] = self._GenerateType(type_.item_type)
+      elif type_.property_type == model.PropertyType.ENUM:
+        dst_dict['enum_values'] = [
+            {'name': value.name, 'description': value.description}
+            for value in type_.enum_values]
+        if len(dst_dict['enum_values']) > 0:
+          dst_dict['enum_values'][-1]['last'] = True
+      elif type_.instance_of is not None:
+        dst_dict['simple_type'] = type_.instance_of
+      else:
+        dst_dict['simple_type'] = type_.property_type.name
 
   def _GetIntroTableList(self):
     '''Create a generic data structure that can be traversed by the templates