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
+import re
 
 from code import Code
 import json_parse
 
 # The template for the header file of the generated FeatureProvider.
 HEADER_FILE_TEMPLATE = """
 // Copyright %(year)s 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.
 
@@ skipping 71 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.
 # If a type definition does not have any restrictions (beyond the type itself),
 # an empty definition ({}) is used.
 FEATURE_GRAMMAR = (
   {
+    'alias': {
+      unicode: {},
+      '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: {},
+      'shared': True
+    },
     'whitelist': {
       list: {'subtype': unicode}
     },
   })
 
 FEATURE_CLASSES = ['APIFeature', 'BehaviorFeature',
                    'ManifestFeature', 'PermissionFeature']
 
 def HasProperty(property_name, value):
   return property_name in value
 
 def HasAtLeastOneProperty(property_names, value):
   return any([HasProperty(name, value) for name in property_names])
 
 def DoesNotHaveProperty(property_name, value):
   return property_name not in value
 
+def IsFeatureCrossReference(property_name, reverse_property_name, feature,
+                            all_features):
+  feature_values = feature.GetAllFeatureValues()

[Devlin, 2016/11/23 15:56:13]

This would be a lot more efficient as a GetValue() implementation to avoid
copying all the entries.

[tbarzic, 2016/11/23 20:16:41]

Done.

+  value = feature_values.get(property_name)
+  if not value:
+    return True
+  # String property values will be wrapped in "", strip those.
+  value_regex = re.compile('^"(.+)"$')
+  parsed_value = value_regex.match(value)
+  if not parsed_value:
+    return False
+
+  referenced_feature = all_features.get(parsed_value.group(1))
+  if not referenced_feature:
+    return False
+  referenced_feature_values = referenced_feature.GetAllFeatureValues()
+  reverse_reference_value = referenced_feature_values.get(reverse_property_name)
+  if not reverse_reference_value:
+    return False
+  # Don't validate reverse reference value for child features - chances are that
+  # the value was inherited from a feature parent, in which case it won't match
+  # current feature name.
+  if feature.has_parent:
+    return True
+  return reverse_reference_value == ('"%s"' % feature.name)
+
 VALIDATION = ({
   'all': [
     (partial(HasAtLeastOneProperty, ['channel', 'dependencies']),
      'Features must specify either a channel or dependencies'),
   ],
   '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.'),
   ],
 })
 
+FINAL_VALIDATION = ({
+  'all': [],
+  'APIFeature': [
+    (partial(IsFeatureCrossReference, 'alias', 'source'),
+     'A feature alias property should reference a feature whose source '
+     'property references it back.'),
+    (partial(IsFeatureCrossReference, 'source', 'alias'),
+     'A feature source property should reference a feature whose alias '
+     'property references it back.')
+
+  ],
+  'ManifestFeature': [],
+  'BehaviorFeature': [],
+  'PermissionFeature': []
+})
+
 # 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 = {}
 
   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):
+  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))
+    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):
     """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.
@@ skipping 36 matching lines @@
     """
     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']
@@ skipping 19 matching lines @@
                                               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)
 
     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):
-        self._AddError(error)
+      if not validator(feature_values):
+        self.AddError(error)
+
+  def _GetCodeForFeatureValues(self, feature_values):

[Devlin, 2016/11/23 15:56:13]

This should be either a static function or a function separate from the feature
class.

[tbarzic, 2016/11/23 20:16:41]

Done.

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

[Devlin, 2016/11/23 15:56:13]

As this is written now, won't this result in each simple feature of a complex
feature setting the shared properties?  (I think this will be fixed with the
suggestion for shared_values below)

     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 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):
+    Feature.__init__(self, name)
+    self.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):
+    parent = None
+    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 GetAllFeatureValues(self):
+    return self.shared_values.copy()
+
+  def GetErrors(self):
+    errors = copy.copy(self.errors)
+    for feature in self.feature_list:
+      if not errors:

[Devlin, 2016/11/23 15:56:13]

When is this the case?

[tbarzic, 2016/11/23 20:16:41]

when error is [].
I initially wrote this assuming errors is None by default, and forgot to remove
it once I realized that's not the case.

+        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
     self._out_root = out_root
     self._out_base_filename = out_base_filename
 
     # The json value for the feature files.
     self._json = {}
     # The parsed features.
     self._features = {}
 
-  def _Load(self):
+  def Load(self):
     """Loads and parses the source from each input file and puts the result in
     self._json."""
     for f in self._source_files:
       abs_source_file = os.path.join(self._chrome_root, f)
       try:
         with open(abs_source_file, 'r') as f:
           f_json = json_parse.Parse(f.read())
       except:
         print('FAILED: Exception encountered while loading "%s"' %
                   abs_source_file)
@@ skipping 28 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/23 15:56:13]

We shouldn't have to call this each time.  Instead, let's pass the shared_values
object that we use into Parse().  So for complex features, we'd pass the complex
feature's shared values in, and otherwise we'd pass the simple feature's shared
values in.

Does that make sense?

[tbarzic, 2016/11/23 20:16:41]

Sure.

         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)
+      feature.SetSharedValues(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
+        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)
 
-    self._features[feature_name] = parse_and_validate(
-                                       feature_name, feature_value, parent)
+  def _FinalValidation(self):
+    validators = FINAL_VALIDATION['all'] + FINAL_VALIDATION[self._feature_class]
+    for name, feature in self._features.items():
+      for validator, error in validators:
+        if not validator(feature, self._features):
+          feature.AddError(error)
 
   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])
+    self._FinalValidation()
 
   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 40 matching lines @@
       'out_base_filename', type=str,
       help='The base filename for the C++ files (.h and .cc will be appended)')
   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.Load()
   c.Compile()
   c.Write()