Support API aliases

tools/json_schema_compiler/feature_compiler.py

 # Copyright 2016 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 argparse
 import copy
 from datetime import datetime
 from functools import partial
 import os
 
@@ skipping 80 matching lines @@
 #               version_info::Channel::STABLE to channel. The keys in this map
 #               also serve as a list all of possible values.
 #   'allow_all': Only applicable for lists. If present, this will check for
 #                a value of "all" for a list value, and will replace it with
 #                the collection of all possible values. For instance, if a
 #                feature specifies 'contexts': 'all', the generated C++ will
 #                assign the list of Feature::BLESSED_EXTENSION_CONTEXT,
 #                Feature::BLESSED_WEB_PAGE_CONTEXT et al for contexts. If not
 #                specified, defaults to false.
 #   'values': A list of all possible allowed values for a given key.
+#   'shared': Boolean that, if set, ensures that only one of the associated
+#       features has the feature property set. Used primarily for complex
+#       features - for simple features, there is always at most one feature
+#       setting an option.
+#   'feature_reference': Only applicable for strings. An object that, if

[Devlin, 2016/11/21 16:55:53]

This feature reference code looks like it adds a lot of complication.  I wonder
if a simpler (and potentially more flexible) way of doing it would be to have a
final validation that happens once we parse all our features and is called
similar to validation.  Then, we could do something like:


def IsFeatureCrossReference(first_key, second_key, feature, all_features):
  val = feature.GetValue(first_key)
  if not val:
    return True
  if not val in all_features:
    return False
  xref_feature = all_features[val]
  xref_val = xref_feature.GetValue(second_key)
  if not xref_val or xref_val is not feature.name:
    return False
  return True

FINAL_VALIDATION = ([
  'APIFeature': [
    (partial(IsFeatureCrossReference, 'alias', 'source'),
     '<error message>')
  ]
])

(continued below)

This would allow us to later add any other validations that require knowledge of
other features more easily.

[tbarzic, 2016/11/21 23:49:47]

Done (though the error message returned using this approach is slightly less
precise :/).

+#       provided, verifies that the value is set to a name of another feature
+#       defined in the same features file. It can following options:
+#           'reverse_reference': String that should be equal to another feature
+#                option. If set, it verifies that the feature referenced by
+#                |feature_reference| value references this feature using
+#                |reverse_reference| option.
 # If a type definition does not have any restrictions (beyond the type itself),
 # an empty definition ({}) is used.
 FEATURE_GRAMMAR = (
   {
+    'alias': {
+      unicode: {},
+      'feature_reference': {
+        'reverse_reference': 'source'
+      },
+      'shared': True
+    },
     'blacklist': {
       list: {'subtype': unicode}
     },
     'channel': {
       unicode: {
         'enum_map': {
           'trunk': 'version_info::Channel::UNKNOWN',
           'canary': 'version_info::Channel::CANARY',
           'dev': 'version_info::Channel::DEV',
           'beta': 'version_info::Channel::BETA',
@@ skipping 75 matching lines @@
       }
     },
     'session_types': {
       list: {
         'enum_map': {
           'regular': 'FeatureSessionType::REGULAR',
           'kiosk': 'FeatureSessionType::KIOSK',
         }
       }
     },
+    'source': {
+      unicode: {},
+      'feature_reference': {
+        'reverse_reference': 'alias'
+      },
+      'shared': True
+    },
     'whitelist': {
       list: {'subtype': unicode}
     },
   })
 
 FEATURE_CLASSES = ['APIFeature', 'BehaviorFeature',
                    'ManifestFeature', 'PermissionFeature']
 
 def HasProperty(property_name, value):
   return property_name in value
@@ skipping 11 matching lines @@
   ],
   'APIFeature': [
     (partial(HasProperty, 'contexts'),
      'APIFeatures must specify at least one context')
   ],
   'ManifestFeature': [
     (partial(HasProperty, 'extension_types'),
      'ManifestFeatures must specify at least one extension type'),
     (partial(DoesNotHaveProperty, 'contexts'),
      'ManifestFeatures do not support contexts.'),
+    (partial(DoesNotHaveProperty, 'alias'),
+     'ManifestFeatures do not support alias.'),
+    (partial(DoesNotHaveProperty, 'source'),
+     'ManifestFeatures do not support source.'),
   ],
-  'BehaviorFeature': [],
+  'BehaviorFeature': [
+    (partial(DoesNotHaveProperty, 'alias'),
+     'BehaviorFeatures do not support alias.'),
+    (partial(DoesNotHaveProperty, 'source'),
+     'BehaviorFeatures do not support source.'),
+   ],
   'PermissionFeature': [
     (partial(HasProperty, 'extension_types'),
      'PermissionFeatures must specify at least one extension type'),
     (partial(DoesNotHaveProperty, 'contexts'),
      'PermissionFeatures do not support contexts.'),
+    (partial(DoesNotHaveProperty, 'alias'),
+     'PermissionFeatures do not support alias.'),
+    (partial(DoesNotHaveProperty, 'source'),
+     'PermissionFeatures do not support source.'),
   ],
 })
 
 # These keys are used to find the parents of different features, but are not
 # compiled into the features themselves.
 IGNORED_KEYS = ['default_parent']
 
 # By default, if an error is encountered, assert to stop the compilation. This
 # can be disabled for testing.
 ENABLE_ASSERTIONS = True
 
 # JSON parsing returns all strings of characters as unicode types. For testing,
 # we can enable converting all string types to unicode to avoid writing u''
 # everywhere.
 STRINGS_TO_UNICODE = False
 
 class Feature(object):
   """A representation of a single simple feature that can handle all parsing,
   validation, and code generation.
   """
   def __init__(self, name):
     self.name = name
     self.has_parent = False
     self.errors = []
     self.feature_values = {}
+    self.shared_values = {}
+    self.feature_references = []
 
   def _GetType(self, value):
     """Returns the type of the given value. This can be different than type() if
     STRINGS_TO_UNICODE is enabled.
     """
     t = type(value)
     if not STRINGS_TO_UNICODE:
       return t
     if t is str:
       return unicode
     return t
 
   def _AddError(self, error):
     """Adds an error to the feature. If ENABLE_ASSERTIONS is active, this will
     also assert to stop the compilation process (since errors should never be
     found in production).
     """
     self.errors.append(error)
     if ENABLE_ASSERTIONS:
       assert False, error
 
   def _AddKeyError(self, key, error):
     """Adds an error relating to a particular key in the feature.
     """
     self._AddError('Error parsing feature "%s" at key "%s": %s' %
                        (self.name, key, error))
 
   def _GetCheckedValue(self, key, expected_type, expected_values,
-                       enum_map, value):
+                       enum_map, feature_reference, value):
     """Returns a string to be used in the generated C++ code for a given key's
     python value, or None if the value is invalid. For example, if the python
     value is True, this returns 'true', for a string foo, this returns "foo",
     and for an enum, this looks up the C++ definition in the enum map.
       key: The key being parsed.
       expected_type: The expected type for this value, or None if any type is
                      allowed.
       expected_values: The list of allowed values for this value, or None if any
                        value is allowed.
       enum_map: The map from python value -> cpp value for all allowed values,
                or None if no special mapping should be made.
+      feature_reference: Object set if the value should represent a feature

[Devlin, 2016/11/21 16:55:53]

(With the above, we can remove this and most "feature_reference" code so that we
don't need to have knowledge of it everywhere)

[tbarzic, 2016/11/21 23:49:47]

Done.

+          reference. If set, the value is expected to be string referencing
+          another feature in the feature file. While it will be checked that the
+          value is string, validation that the value references an existing
+          feature will have to be postponed until all features are parsed - the
+          info needed to verify the key value will be cached in
+          |self.feature_references|.
+          The object may have "reverse_reference" property set - in that case
+          the referenced feature should reference this feature by
+          "reverse_reference" property.
       value: The value to check.
     """
     valid = True
     if expected_values and value not in expected_values:
       self._AddKeyError(key, 'Illegal value: "%s"' % value)
       valid = False
 
     t = self._GetType(value)
     if expected_type and t is not expected_type:
       self._AddKeyError(key, 'Illegal value: "%s"' % value)
       valid = False
 
+    if feature_reference and not t in [str, unicode]:
+      self._AddKeyError(key, 'Illegal value for feature reference %s' % value)
+      valid = False
+
     if not valid:
       return None
 
     if enum_map:
       return enum_map[value]
 
     if t in [str, unicode]:
+      if feature_reference:
+        self.feature_references.append({
+          'value': str(value),
+          'key': key,
+          'reverse_reference': feature_reference.get('reverse_reference')})
       return '"%s"' % str(value)
     if t is int:
       return str(value)
     if t is bool:
       return 'true' if value else 'false'
     assert False, 'Unsupported type: %s' % value
 
   def _ParseKey(self, key, value, grammar):
     """Parses the specific key according to the grammar rule for that key if it
     is present in the json value.
       key: The key to parse.
       value: The full value for this feature.
       grammar: The rule for the specific key.
     """
     if key not in value:
       return
     v = value[key]
 
     is_all = False
     if v == 'all' and list in grammar and 'allow_all' in grammar[list]:
       v = []
       is_all = True
 
+    if 'shared' in grammar and key in self.shared_values:
+      self._AddKeyError(key, 'Key can be set at most once per feature.')
+      return
+
     value_type = self._GetType(v)
     if value_type not in grammar:
       self._AddKeyError(key, 'Illegal value: "%s"' % v)
       return
 
     expected = grammar[value_type]
     expected_values = None
     enum_map = None
     if 'values' in expected:
       expected_values = expected['values']
     elif 'enum_map' in expected:
       enum_map = expected['enum_map']
       expected_values = enum_map.keys()
 
     if is_all:
       v = copy.deepcopy(expected_values)
 
     expected_type = None
     if value_type is list and 'subtype' in expected:
       expected_type = expected['subtype']
 
+    feature_reference = None
+    if 'feature_reference' in grammar:
+      feature_reference = {}
+      feature_reference['reverse_reference'] = grammar['feature_reference'].get(
+          'reverse_reference')
+
     cpp_value = None
     # If this value is a list, iterate over each entry and validate. Otherwise,
     # validate the single value.
     if value_type is list:
       cpp_value = []
       for sub_value in v:
         cpp_sub_value = self._GetCheckedValue(key, expected_type,
                                               expected_values, enum_map,
+                                              feature_reference,
                                               sub_value)
         if cpp_sub_value:
           cpp_value.append(cpp_sub_value)
       if cpp_value:
         cpp_value = '{' + ','.join(cpp_value) + '}'
     else:
       cpp_value = self._GetCheckedValue(key, expected_type, expected_values,
-                                        enum_map, v)
+                                        enum_map, feature_reference, v)
 
     if cpp_value:
-      self.feature_values[key] = cpp_value
+      if 'shared' in grammar:
+        self.shared_values[key] = cpp_value
+      else:
+        self.feature_values[key] = cpp_value
     elif key in self.feature_values:
       # If the key is empty and this feature inherited a value from its parent,
       # remove the inherited value.
       del self.feature_values[key]
 
   def SetParent(self, parent):
     """Sets the parent of this feature, and inherits all properties from that
     parent.
     """
     assert not self.feature_values, 'Parents must be set before parsing'
     self.feature_values = copy.deepcopy(parent.feature_values)
     self.has_parent = True
 
+  def SetSharedValues(self, values):
+    self.shared_values = values
+
   def Parse(self, parsed_json):
     """Parses the feature from the given json value."""
     for key in parsed_json.keys():
       if key not in FEATURE_GRAMMAR:
         self._AddKeyError(key, 'Unrecognized key')
     for key, key_grammar in FEATURE_GRAMMAR.iteritems():
       self._ParseKey(key, parsed_json, key_grammar)
 
   def Validate(self, feature_class):
+    feature_values = self.GetAllFeatureValues()
     for validator, error in (VALIDATION[feature_class] + VALIDATION['all']):
-      if not validator(self.feature_values):
+      if not validator(feature_values):
         self._AddError(error)
 
+  def _GetCodeForFeatureValues(self, feature_values):
+    """ Gets the Code object for setting feature values for this object. """
+    c = Code()
+    for key in sorted(feature_values.keys()):
+      if key in IGNORED_KEYS:
+        continue;
+      c.Append('feature->set_%s(%s);' % (key, feature_values[key]))
+    return c
+
   def GetCode(self, feature_class):
     """Returns the Code object for generating this feature."""
     c = Code()
     c.Append('%s* feature = new %s();' % (feature_class, feature_class))
     c.Append('feature->set_name("%s");' % self.name)
-    for key in sorted(self.feature_values.keys()):
-      if key in IGNORED_KEYS:
-        continue;
-      c.Append('feature->set_%s(%s);' % (key, self.feature_values[key]))
+    c.Concat(self._GetCodeForFeatureValues(self.GetAllFeatureValues()))
     return c
 
+  def AsParent(self):
+    """ Returns the feature values that should be inherited by children features
+    when this feature is set as parent.
+    """
+    return self
+
+  def GetAllFeatureValues(self):
+    """ Gets all values set for this feature. """
+    values = self.feature_values.copy()
+    values.update(self.shared_values)
+    return values
+
+  def ValidateFeatureReferences(self, features):
+    """ Validates feature values that reference another feature - the validation
+    cannot be done when individual features are compiled (like it is done for
+    other value types), as whole feature set is not know at that point.
+    """
+    for ref in self.feature_references:
+      # Validate that a feature referenced by this feature exists.
+      if not ref['value'] in features:
+        self._AddKeyError(ref['key'], '%s is not a feature.' % ref['value'])
+        return
+
+      # Verify that referenced feature references this feature, if reverse
+      # reference is required.
+      reverse_ref = ref['reverse_reference']
+      if reverse_ref:
+        referenced_feature_values = features[ref['value']].GetAllFeatureValues()
+        reverse_ref_value = referenced_feature_values.get(reverse_ref)
+        if (reverse_ref_value != ('"%s"' % self.name)):
+          self._AddKeyError(ref['key'],
+              'Referenced feature ("%s") should have "%s" set to "%s".' %
+                  (ref['value'], reverse_ref, self.name))
+
+  def GetErrors(self):
+    return self.errors;
+
+class ComplexFeature(Feature):
+  """ Complex feature - feature that is comprised of list of features.
+  Overall complex feature is available if any of contained
+  feature is available.
+  """
+  def __init__(self, name, shared_values):

[Devlin, 2016/11/21 16:55:53]

self.shared_values should start empty, so no need to pass in

[tbarzic, 2016/11/21 23:49:47]

Done.

+    Feature.__init__(self, name)
+    self.shared_values = shared_values
+    self.feature_list = []
+
+  def GetCode(self, feature_class):
+    c = Code()
+    c.Append('std::vector<Feature*> features;')
+    for f in self.feature_list:
+      c.Sblock('{')
+      c.Concat(f.GetCode(feature_class))
+      c.Append('features.push_back(feature);')
+      c.Eblock('}')
+    c.Append('ComplexFeature* feature(new ComplexFeature(&features));')
+    c.Append('feature->set_name("%s");' % self.name)
+    c.Concat(self._GetCodeForFeatureValues(self.GetAllFeatureValues()))
+    return c
+
+  def AsParent(self):
+    for p in self.feature_list:
+      if 'default_parent' in p.feature_values:
+        parent = p
+        break
+    assert parent, 'No default parent found for %s' % self.name
+    return parent
+
+  def ValidateFeatureReferences(self, available_features):
+    for feature in self.feature_list:
+      feature.ValidateFeatureReferences(available_features)
+
+  def GetAllFeatureValues(self):
+    return self.shared_values.copy()
+
+  def GetErrors(self):
+    errors = None
+    for feature in self.feature_list:
+      if not errors:
+        errors = []
+      errors.extend(feature.GetErrors())
+    return errors
+
 class FeatureCompiler(object):
   """A compiler to load, parse, and generate C++ code for a number of
   features.json files."""
   def __init__(self, chrome_root, source_files, feature_class,
                provider_class, out_root, out_base_filename):
     # See __main__'s ArgumentParser for documentation on these properties.
     self._chrome_root = chrome_root
     self._source_files = source_files
     self._feature_class = feature_class
     self._provider_class = provider_class
@@ skipping 47 matching lines @@
       parent_name = feature_name[:sep]
 
     if sep == -1:
       # TODO(devlin): It'd be kind of nice to be able to assert that the
       # deduced parent name is in our features, but some dotted features don't
       # have parents and also don't have noparent, e.g. system.cpu. We should
       # probably just noparent them so that we can assert this.
       #   raise KeyError('Could not find parent "%s" for feature "%s".' %
       #                      (parent_name, feature_name))
       return None
-    parent_value = self._features[parent_name]
-    parent = parent_value
-    if type(parent_value) is list:
-      for p in parent_value:
-        if 'default_parent' in p.feature_values:
-          parent = p
-          break
-      assert parent, 'No default parent found for %s' % parent_name
-    return parent
+    return self._features[parent_name].AsParent()
 
   def _CompileFeature(self, feature_name, feature_value):
     """Parses a single feature."""
     if 'nocompile' in feature_value:
       assert feature_value['nocompile'], (
           'nocompile should only be true; otherwise omit this key.')
       return
 
-    def parse_and_validate(name, value, parent):
+    def parse_and_validate(name, value, parent, shared_values):
       try:
         feature = Feature(name)
         if parent:
           feature.SetParent(parent)
+        feature.SetSharedValues(shared_values)

[Devlin, 2016/11/21 16:55:53]

Ideally, this should be handled as part of feature.SetParent

[tbarzic, 2016/11/21 23:49:47]

parent might not be set

         feature.Parse(value)
         feature.Validate(self._feature_class)
         return feature
       except:
         print('Failure to parse feature "%s"' % feature_name)
         raise
 
     parent = self._FindParent(feature_name, feature_value)
+    shared_values = copy.deepcopy(parent.shared_values) if parent else {}
+
     # Handle complex features, which are lists of simple features.
     if type(feature_value) is list:
-      feature_list = []
+      feature = ComplexFeature(feature_name, shared_values)
+
       # This doesn't handle nested complex features. I think that's probably for
       # the best.
       for v in feature_value:
-        feature_list.append(parse_and_validate(feature_name, v, parent))
-      self._features[feature_name] = feature_list
-      return
-
-    self._features[feature_name] = parse_and_validate(
-                                       feature_name, feature_value, parent)
+        feature.feature_list.append(
+            parse_and_validate(feature_name, v, parent, feature.shared_values))
+      self._features[feature_name] = feature
+    else:
+      self._features[feature_name] = parse_and_validate(
+          feature_name, feature_value, parent, shared_values)
 
   def Compile(self):
     """Parses all features after loading the input files."""
     self._Load();
     # Iterate over in sorted order so that parents come first.
     for k in sorted(self._json.keys()):
       self._CompileFeature(k, self._json[k])
+    # Validate values that reference feature

[Devlin, 2016/11/21 16:55:53]

Then here

def _FinalValidation(self):
  for name, feature in self._features.items():
     for validator, error in (FINAL_VALIDATION[self._feature_class] +
FINAL_VALIDATION['all']):
       if not validator(self._features, feature):
         self._AddError(error)

def Compile(self):
  ....
  self._FinalValidation()

[tbarzic, 2016/11/21 23:49:47]

Done.

+    for key in self._features:
+      self._features[key].ValidateFeatureReferences(self._features)
 
   def Render(self):
     """Returns the Code object for the body of the .cc file, which handles the
     initialization of all features."""
     c = Code()
     c.Append('%s::%s() {' % (self._provider_class, self._provider_class))
     c.Sblock()
     for k in sorted(self._features.keys()):
       c.Sblock('{')
       feature = self._features[k]
-      if type(feature) is list:
-        c.Append('std::vector<Feature*> features;')
-        for f in feature:
-          c.Sblock('{')
-          c.Concat(f.GetCode(self._feature_class))
-          c.Append('features.push_back(feature);')
-          c.Eblock('}')
-        c.Append('ComplexFeature* feature(new ComplexFeature(&features));')
-        c.Append('feature->set_name("%s");' % k)
-      else:
-        c.Concat(feature.GetCode(self._feature_class))
+      c.Concat(feature.GetCode(self._feature_class))
       c.Append('AddFeature("%s", feature);' % k)
       c.Eblock('}')
     c.Eblock('}')
     return c
 
   def Write(self):
     """Writes the output."""
     header_file_path = self._out_base_filename + '.h'
     cc_file_path = self._out_base_filename + '.cc'
     substitutions = ({
@@ skipping 42 matching lines @@
   parser.add_argument('source_files', type=str, nargs='+',
                       help='The source features.json files')
   args = parser.parse_args()
   if args.feature_class not in FEATURE_CLASSES:
     raise NameError('Unknown feature class: %s' % args.feature_class)
   c = FeatureCompiler(args.chrome_root, args.source_files, args.feature_class,
                       args.provider_class, args.out_root,
                       args.out_base_filename)
   c.Compile()
   c.Write()