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..c179b902be3f1a946b220ab03323219f731c0e15 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,24 +27,36 @@ 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

[Yoyo Zhou, 2012/02/08 19:04:12]

This seems redundant given the 2 below it.

[calamity, 2012/02/09 00:56:52]

It's really just for convenience, otherwise, I'd need to import os.path and join
the split version back together.

+  - |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']
@@ -50,46 +67,70 @@ class Type(object):
 
 class Callback(object):
   """A callback parameter to a Function.
+
+  Properties:
+  - |params| the parameters to this callback.
   """
   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)
+      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)
       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. This shouldn't change since
+    it is the key used to access DictionaryValues
+  - |unix_name| the unix_style_name of the property. Used as variable name
+  - |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
+  - |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):
+    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 = _UnixName(self.name)
+    self._unix_name_used = False
     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,14 +138,16 @@ 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 = {}
@@ -114,21 +157,61 @@ class Property(object):
       else:
         raise NotImplementedError(json_type)
     elif 'choices' in json:
-      self.type_ = PropertyType.CHOICES
+      assert len(json['choices']), 'Choices has no choices\n%s' % json
       self.choices = {}
+      self.type_ = PropertyType.CHOICES
+      for choice_json in json['choices']:
+        choice = Property(self.name, choice_json)
+        # A choice needs to have its unix_name set elsewhere

[Yoyo Zhou, 2012/02/08 19:04:12]

Can you discuss where SetUnixName might be called from? As it is, I can't find
it.

[calamity, 2012/02/09 00:56:52]

unix_name is set as a property of this class (line 189 in this patch). When the
property accessed, (i.e prop.unix_name) the setter or getter is called depending
on context.

tl;dr python magic

+        choice._unix_name = None
+        # The existence of any single choice is optional
+        choice.optional = True
+        self.choices[choice.type_] = choice
+    else:
+      raise NotImplementedError(json)
+
+  def GetUnixName(self):
+    """Gets the property's unix_name. Raises AttributeError if not set.
+    """
+    if self._unix_name is None:
+      raise AttributeError('No unix_name set on %s' % self.name)
+    self._unix_name_used = True
+    return self._unix_name
+
+  def SetUnixName(self, unix_name):
+    """Set the property's unix_name. Raises AttributeError if the unix_name has
+    already been used (GetUnixName has been called).
+    """
+    if self._unix_name_used:
+      raise AttributeError(
+          'Cannot set the unix_name on %s; it is already used elsewhere as %s')

[Yoyo Zhou, 2012/02/08 19:04:12]

missing substitutions here?

[calamity, 2012/02/09 00:56:52]

Done.

+    self._unix_name = unix_name
+  unix_name = property(GetUnixName, SetUnixName)

[not at google - send to devlin, 2012/02/08 05:02:08]

Getter/Setter can be a bit simpler to take advantage of their complementary
access restrictions (you don't need the comments, I'm just adding them for
illustrative purposes).

===

def GetUnicName(self):
  # There has to be a valid _unix_name since we set one in the
  # constructor, and SetUnixName ensures that it's only ever
  # changed to something valid.
  self._unix_name_used = True
  return self._unix_name

def SetUnixName(self, unix_name):
  if unix_name is None
    raise AssertionErrorOrSomething(...)
  if self._unix_name_used
    raise AttributeError(...)
  # This is up to you, but it seems like you only want to be
  # able to set it once, too.
  self._unix_name_used = true
  self._unix_name = unix_name

[calamity, 2012/02/08 07:01:18]

If the property is a choice, we unset it. GetUnixName then throws the exception
on attempted access.

We probably don't want to set more than once but we don't really care (unless it
gets used). Actually I'm going to allow this because it lets me avoid a copy in
GetExpandedChoicesInParams.

 
 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")
+  CHOICES = _Info(False, "CHOICES")
+  OBJECT = _Info(False, "OBJECT")
+  ANY = _Info(False, "ANY")
+
+def _UnixName(name):
+  """Returns the unix_style name for a given string.

[Yoyo Zhou, 2012/02/08 19:04:12]

Comment here that the name is expected to be lowerCamelCase.

[calamity, 2012/02/09 00:56:52]

Done.

+  """
+  return '_'.join([x.lower()
+      for x in re.findall('[A-Z][a-z_]*', name[0].upper() + name[1:])])
 
-  INTEGER = _Info(True)
-  DOUBLE = _Info(True)
-  BOOLEAN = _Info(True)
-  STRING = _Info(True)
-  ARRAY = _Info(False)
-  REF = _Info(False)
-  CHOICES = _Info(False)
-  OBJECT = _Info(False)