Supporting more APIs with json_schema_compiler

tools/json_schema_compiler/model.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import os.path
+import re
 
 class Model(object):
   """Model of all namespaces that comprise an API.
   """
   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.
     """
-    if not json.get('compile'):
+    if not json.get('compile', False):
       return None
     namespace = Namespace(json, source_file)
     self.namespaces[namespace.name] = namespace
     return namespace
 
 class Namespace(object):
   """An API namespace.
   """
   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 function_json.get('compile', True):
         function = Function(function_json)
         self.functions[function.name] = function
 
 class Type(object):
   """A Type defined in the json.
   """
   def __init__(self, json):
     self.name = json['id']
     self.description = json.get('description')
     self.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.
   """
   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'):

[not at google - send to devlin, 2012/02/05 23:42:12]

I think you need to document what the available properties are throughout the
model, it's a bit hard to follow just by looking through the code.

I don't know what the standard python documentation structure looks like, but
something like (at the top)

"""Model of all namespaces that comprise an API.

Properties:
 - |namespaces| a map of namespace to its |Namespace| model
"""

For namespace

"""An API namespace

Properties:
 - |name| foo
 - |source_file| bar
"""

And, the reason I mention this,

"""A callback parameter to a Function.

Properties:
 - |params| what is params, why does each choice result in something being
appended to it?
"""

[calamity, 2012/02/06 11:51:18]

Done.

+        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.
   """
   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))

[not at google - send to devlin, 2012/02/05 23:42:12]

(ditto)

I see now why you are appending a different Choice for each choice, but this
definitely needs some extra explanation.

[calamity, 2012/02/06 11:51:18]

Done.

       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
 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.
   """
-  def __init__(self, name, json):
+  def __init__(self, name, json, is_choice=False):

[not at google - send to devlin, 2012/02/05 23:42:12]

is_choice no longer needed

[calamity, 2012/02/06 11:51:18]

I think I'll need something eventually in order to generate the type indicator
for which choice is populated. The choices code is certainly not finished.

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

Yeah, see the other couple of comments I've made regarding this elsewhere.

     self.name = name

[not at google - send to devlin, 2012/02/05 23:42:12]

I think for self-documentation sake, this should specify what the name is
expecting to be. which is... lowerCamelCase I think, right?

something like

if not re.match('^[a-z][a-zA-Z0-9]*$', name)
  throw AppropriateException(name + " must be lowerCamelCase")

[calamity, 2012/02/06 11:51:18]

Done.

+    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
     elif 'type' in json:
       json_type = json['type']
       if json_type == 'string':
         self.type_ = PropertyType.STRING
+      elif json_type ==  'any':

[not at google - send to devlin, 2012/02/05 23:42:12]

extra space

[calamity, 2012/02/06 11:51:18]

Done.

+        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 == 'double' or json_type == 'number':

[not at google - send to devlin, 2012/02/05 23:42:12]

We should remove support for "double" and fix up the JSON in the 1 place that
it's used.

(though might as well leave PropertyType as DOUBLE)

[calamity, 2012/02/06 11:51:18]

Done.

         self.type_ = PropertyType.DOUBLE
       elif json_type == 'array':
         self.item_type = Property(name + "_inner", 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']:
+              self.properties[key] = _Choice(self.name, choice)

[not at google - send to devlin, 2012/02/05 23:42:12]

doesn't this overwrite the same (and incorrect) key each time?

should be something like

if 'choices' in val:
 for choice in val['choices']:
   choice_property = _Choice(self.name, choice)
   self.properties[choice_property.name_] = choice_property

[calamity, 2012/02/06 11:51:18]

Done.

+          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
 
-  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 __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")
+
+def _UnixName(name):

[not at google - send to devlin, 2012/02/05 23:42:12]

document

[calamity, 2012/02/06 11:51:18]

Done.

+  return '_'.join([x.lower()
+      for x in re.findall('[A-Z][a-z_]*', name[0].upper() + name[1:])])

[not at google - send to devlin, 2012/02/05 23:42:12]

i'll just assume that works...

+
+def _Choice(name, json):

[not at google - send to devlin, 2012/02/05 23:42:12]

document

[calamity, 2012/02/06 11:51:18]

Done.

+  prop_type = json.get('type')
+  if not prop_type:
+    prop_type = json['$ref'].lower()

[not at google - send to devlin, 2012/02/05 23:42:12]

I think should structure this slightly differently to make the intent a bit
clearer (and so avoid having to mutate the property that is generated).

def _Choice(name, json):
  """documentation.
  """
  # Adjust name so that it reflects the type of the choice.
  # E.g. for a property with name "ids" with choices ["integer", "array"],
  # we want to generate properties "ids_integer" and "ids_array".
  property_type = json.get('type')
  if not property_type:
    property_type = json['$ref']
  if not property_type:
    throw NotImplementedError(json)
  full_name = "%s%s" % (name, propertyType[0].upper() + propertyType[1].lower())
  return Property(full_name, json)

+  prop = Property(name, json)
+  prop.unix_name = _UnixName('%s_%s' % (name, prop_type))
+  return prop