Supporting more APIs with json_schema_compiler

tools/json_schema_compiler/model.py

Index: tools/json_schema_compiler/model.py
diff --git a/tools/json_schema_compiler/model.py b/tools/json_schema_compiler/model.py
index 2c356fff20d06f0815e7a2662586bdacbfa77e01..a13de5ea27366b0b07d7af67a298566537f101ba 100644
--- a/tools/json_schema_compiler/model.py
+++ b/tools/json_schema_compiler/model.py
@@ -3,18 +3,23 @@
 # found in the LICENSE file.
 
 import os.path
+import re
 
 class Model(object):
   """Model of all namespaces that comprise an API.
+
+  Properties:
+  - |namespaces| a map of a namespace name to its model.Namespace
   """
   def __init__(self):
     self.namespaces = {}
 
   def AddNamespace(self, json, source_file):
-    """Add a namespace's json to the model if it has a "compile" property set
-    to true. Returns the new namespace or None if a namespace wasn't added.
+    """Add a namespace's json to the model if it doesn't have "nocompile"
+    property set to true. Returns the new namespace or None if a namespace
+    wasn't added.
     """
-    if not json.get('compile'):
+    if json.get('nocompile', False):
       return None
     namespace = Namespace(json, source_file)
     self.namespaces[namespace.name] = namespace
@@ -22,74 +27,121 @@ class Model(object):
 
 class Namespace(object):
   """An API namespace.
+
+  Properties:
+  - |name| the name of the namespace
+  - |source_file| the file that contained the namespace definition
+  - |source_file_dir| the directory component of |source_file|
+  - |source_file_filename| the filename component of |source_file|
+  - |types| a map of type names to their model.Type
+  - |functions| a map of function names to their model.Function
   """
   def __init__(self, json, source_file):
     self.name = json['namespace']
     self.source_file = source_file
     self.source_file_dir, self.source_file_filename = os.path.split(source_file)
-    self.type_dependencies = {}
     self.types = {}
     self.functions = {}
     for type_json in json['types']:
       type_ = Type(type_json)
       self.types[type_.name] = type_
     for function_json in json['functions']:
-      if not function_json.get('nocompile'):
+      if not function_json.get('nocompile', False):
         function = Function(function_json)
         self.functions[function.name] = function
 
 class Type(object):
   """A Type defined in the json.
+
+  Properties:
+  - |name| the type name
+  - |description| the description of the type (if provided)
+  - |properties| a map of property names to their model.Property
   """
   def __init__(self, json):
     self.name = json['id']
     self.description = json.get('description')
     self.properties = {}
+    properties = json.get('properties')
     for prop_name, prop_json in json['properties'].items():
       self.properties[prop_name] = Property(prop_name, prop_json)
 
 class Callback(object):
   """A callback parameter to a Function.
+
+  Properties:
+  - |params| the parameters to this callback. There should only be one unless
+    the parameter is of type 'choices' in which case each choice is processed
+    as a separate parameter
   """
   def __init__(self, json):
     params = json['parameters']
+    self.params = []
     if len(params) == 0:
-      self.param = None
+      return
     elif len(params) == 1:
       param = params[0]
-      self.param = Property(param['name'], param)
+      if param.get('choices'):
+        for choice in param['choices']:
+          self.params.append(_Choice('callback', choice))
+      else:
+        self.params.append(Property(param['name'], param))
     else:
       raise AssertionError("Callbacks can have at most a single parameter")
 
 class Function(object):
   """A Function defined in the API.
+
+  Properties:
+  - |name| the function name
+  - |params| a list of parameters to the function (order matters). A separate
+    parameter is used for each choice of a 'choices' parameter.
+  - |description| a description of the function (if provided)
+  - |callback| the callback parameter to the function. There should be exactly
+    one
   """
   def __init__(self, json):
     self.name = json['name']
     self.params = []
     self.description = json['description']
     self.callback = None
-    self.type_dependencies = {}
     for param in json['parameters']:
       if param.get('type') == 'function':
-        assert (not self.callback), "Function has more than one callback"
+        assert (not self.callback), self.name + " has more than one callback"
         self.callback = Callback(param)
+      elif param.get('choices'):
+        for choice in param['choices']:
+          self.params.append(_Choice(self.name, choice))
       else:
         self.params.append(Property(param['name'], param))
-    assert (self.callback), "Function does not support callback"
+    assert (self.callback), self.name + " does not support callback"
 
-# TODO(calamity): handle Enum/choices
+# TODO(calamity): handle Enum
 class Property(object):
   """A property of a type OR a parameter to a function.
 
-  Members will change based on PropertyType. Check self.type_ to determine which
-  members actually exist.
+  Properties:
+  - |name| name of the property as in the json
+  - |unix_name| the unix_style_name of the property
+  - |optional| a boolean representing whether the property is optional
+  - |description| a description of the property (if provided)
+  - |type_| the model.PropertyType of this property
+  - |ref_type| the type that the REF property is referencing. Can be used to map to
+    its model.Type

[not at google - send to devlin, 2012/02/06 13:14:48]

line wrap

[calamity, 2012/02/08 00:52:31]

Done.

+  - |item_type| a model.Property representing the type of each element in an
+    ARRAY
+  - |properties| the properties of an OBJECT parameter
   """
-  def __init__(self, name, json):
+  def __init__(self, name, json, unix_name=None, is_choice=False):
+    if not re.match('^[a-z][a-zA-Z0-9]*$', name):
+      raise AssertionError('Name %s must be lowerCamelCase' % name)
     self.name = name
+    self.unix_name = unix_name
+    self.is_choice = is_choice
+    if not self.unix_name:
+      self.unix_name = _UnixName(self.name)
     self.optional = json.get('optional', False)
     self.description = json.get('description')
-    # TODO(calamity) maybe check for circular refs? could that be a problem?
     if '$ref' in json:
       self.ref_type = json['$ref']
       self.type_ = PropertyType.REF
@@ -97,38 +149,70 @@ class Property(object):
       json_type = json['type']
       if json_type == 'string':
         self.type_ = PropertyType.STRING
-      elif json_type ==  'boolean':
+      elif json_type == 'any':
+        self.type_ = PropertyType.ANY
+      elif json_type == 'boolean':
         self.type_ = PropertyType.BOOLEAN
       elif json_type == 'integer':
         self.type_ = PropertyType.INTEGER
-      elif json_type == 'double':
+      elif json_type == 'number':
         self.type_ = PropertyType.DOUBLE
       elif json_type == 'array':
-        self.item_type = Property(name + "_inner", json['items'])
+        self.item_type = Property(name + "Element", json['items'])
         self.type_ = PropertyType.ARRAY
       elif json_type == 'object':
         self.properties = {}
         self.type_ = PropertyType.OBJECT
         for key, val in json['properties'].items():
-          self.properties[key] = Property(key, val)
+          if 'choices' in val:
+            for choice in val['choices']:
+              choice_property = _Choice(self.name, choice)
+              self.properties[choice_property.name] = choice_property
+          else:
+            self.properties[key] = Property(key, val)
       else:
         raise NotImplementedError(json_type)
-    elif 'choices' in json:
-      self.type_ = PropertyType.CHOICES
-      self.choices = {}
+    else:
+      raise NotImplementedError(json)
 
 class PropertyType(object):
   """Enum of different types of properties/parameters.
   """
   class _Info(object):
-    def __init__(self, is_fundamental):
+    def __init__(self, is_fundamental, name):
       self.is_fundamental = is_fundamental
+      self.name = name
+
+    def __repr__(self):
+      return self.name
+
+  INTEGER = _Info(True, "INTEGER")
+  DOUBLE = _Info(True, "DOUBLE")
+  BOOLEAN = _Info(True, "BOOLEAN")
+  STRING = _Info(True, "STRING")
+  ARRAY = _Info(False, "ARRAY")
+  REF = _Info(False, "REF")
+  CHOICE = _Info(False, "CHOICE")
+  OBJECT = _Info(False, "OBJECT")
+  ANY = _Info(False, "ANY")
 
-  INTEGER = _Info(True)
-  DOUBLE = _Info(True)
-  BOOLEAN = _Info(True)
-  STRING = _Info(True)
-  ARRAY = _Info(False)
-  REF = _Info(False)
-  CHOICES = _Info(False)
-  OBJECT = _Info(False)
+def _UnixName(name):
+  """Returns the unix_style name for a given string.
+  """
+  return '_'.join([x.lower()
+      for x in re.findall('[A-Z][a-z_]*', name[0].upper() + name[1:])])
+
+def _Choice(name, json):

[not at google - send to devlin, 2012/02/06 13:14:48]

I made a comment here earlier about structuring this method differently, but it
seems to have gotten lost.  No matter, since I've come to realise something
after looking at that checking code.  Sorry to have not have formed an opinion
about this earlier... you're going to hate me...

but the way choices are done at the moment is hard to understand and easy to
misuse.  There is logic in in the model to expand choice parameters, but not
enough (as we saw earlier) and it's not applicable to all scenarios anyway.

To write this differently, how about:
 - Have another PropertyType CHOICE.
 - If a Property is CHOICE, have another member of Property called like
"choices", an array of possible Properties.
 - Code that uses parameters needs to know how to expand out choices.  A lot of
this, I think, should be able to be abstracted nicely away into the
CppTypeGenerator class; like a method along the lines of ExpandChoicesInParams
or something which returns an array of Properties, like function.params, but
expanded in the same way you're doing here, basically.

(and then the h/cc generator classes in ToValue which used function.params and
assumed the Choices were taken care of would use
cpp_type_generator.ExpandChoicesInParams(function.params) instead)

Yeah, there will be a few non-trivial changes to do something like this, but
nothing overly structural, and I think overall it will result in easier to
follow code.

And... you'll need tests for it.

[not at google - send to devlin, 2012/02/06 13:25:34]

Sorry, I should have said an array of possible PropertyTypes.

This is tough though, since for array/object you also need the information about
the items/properties.

Perhaps you'll need to eventually refactor this so that PropertyType is a class
not an enum, so that it can have those set.  That's a big change though, and we
can talk about it tomorrow.  If all that's needed to get this out the door now
is for this actually to be an array of Propertys after all (with those redundant
extra properties like descrition/name/unix_name/etc copyied over) then let's do
that for now.

[calamity, 2012/02/08 00:52:31]

Done.

+  """Creates a Property for an item under 'choices' in the json.
+  """
+  raise NotImplementedError('%s\nChoices is not fully implemented' % json)

[not at google - send to devlin, 2012/02/06 13:14:48]

Choices is implemented for return values right?  Or one of them?

I just meant to raise a NotImplementedError where you said it didn't work, not
for _all_ attempts to use choices :)

That's a bit harsh on yourself...

[calamity, 2012/02/08 00:52:31]

It's done now, but I thought it made sense to disable choices everywhere since
having NotImplemented anywhere would break as soon as you had choices anyway.

+  property_type = json.get('type')
+  if not property_type:
+    property_type = json['$ref'].lower()
+  if not property_type:
+    raise NotImplementedError(json)
+  # Append choice type to the unix_name
+  return Property(name, json, is_choice=True,
+      unix_name=_UnixName('%s%s' %
+      (name, property_type[0].upper() + property_type[1:]))
+  )