Bug 1339604: Allow nesting of keyed-by values; r?kmoir draft
authorDustin J. Mitchell <dustin@mozilla.com>
Thu, 16 Feb 2017 22:09:17 +0000
changeset 485598 26248164337e94eb8d35fd06350bfc7f4aa3179d
parent 485597 a656eb5c2b814ebbb45fad27fcff81348d5f5953
child 546061 1ec0e4f86080ad3ca18e73c68d4b7e22b6f2b548
push id45778
push userdmitchell@mozilla.com
push dateThu, 16 Feb 2017 22:11:10 +0000
reviewerskmoir
bugs1339604
milestone54.0a1
Bug 1339604: Allow nesting of keyed-by values; r?kmoir MozReview-Commit-ID: LRzDWauipGQ
taskcluster/taskgraph/test/test_util_schema.py
taskcluster/taskgraph/util/schema.py
--- a/taskcluster/taskgraph/test/test_util_schema.py
+++ b/taskcluster/taskgraph/test/test_util_schema.py
@@ -48,16 +48,42 @@ class TestResolveKeyedBy(unittest.TestCa
             resolve_keyed_by({'x': 10}, 'x.y', 'n'),
             {'x': 10})
 
     def test_no_by_not_by(self):
         self.assertEqual(
             resolve_keyed_by({'x': {'a': 10}}, 'x', 'n'),
             {'x': {'a': 10}})
 
+    def test_nested(self):
+        x = {
+            'by-foo': {
+                'F1': {
+                    'by-bar': {
+                        'B1': 11,
+                        'B2': 12,
+                    },
+                },
+                'F2': 20,
+                'default': 0,
+            },
+        }
+        self.assertEqual(
+            resolve_keyed_by({'x': x}, 'x', 'x', foo='F1', bar='B1'),
+            {'x': 11})
+        self.assertEqual(
+            resolve_keyed_by({'x': x}, 'x', 'x', foo='F1', bar='B2'),
+            {'x': 12})
+        self.assertEqual(
+            resolve_keyed_by({'x': x}, 'x', 'x', foo='F2'),
+            {'x': 20})
+        self.assertEqual(
+            resolve_keyed_by({'x': x}, 'x', 'x', foo='F99', bar='B1'),
+            {'x': 0})
+
     def test_no_by_empty_dict(self):
         self.assertEqual(
             resolve_keyed_by({'x': {}}, 'x', 'n'),
             {'x': {}})
 
     def test_no_by_not_only_by(self):
         self.assertEqual(
             resolve_keyed_by({'x': {'by-y': True, 'a': 10}}, 'x', 'n'),
--- a/taskcluster/taskgraph/util/schema.py
+++ b/taskcluster/taskgraph/util/schema.py
@@ -29,88 +29,110 @@ def optionally_keyed_by(*arguments):
     """
     Mark a schema value as optionally keyed by any of a number of fields.  The
     schema is the last argument, and the remaining fields are taken to be the
     field names.  For example:
 
         'some-value': optionally_keyed_by(
             'test-platform', 'build-platform',
             Any('a', 'b', 'c'))
+
+    The resulting schema will allow nesting of `by-test-platform` and
+    `by-build-platform` in either order.
     """
-    subschema = arguments[-1]
+    schema = arguments[-1]
     fields = arguments[:-1]
-    options = [subschema]
-    for field in fields:
-        options.append({'by-' + field: {basestring: subschema}})
-    return voluptuous.Any(*options)
+
+    # build the nestable schema by generating schema = Any(schema,
+    # by-fld1, by-fld2, by-fld3) once for each field.  So we don't allow
+    # infinite nesting, but one level of nesting for each field.
+    for _ in arguments:
+        options = [schema]
+        for field in fields:
+            options.append({'by-' + field: {basestring: schema}})
+        schema = voluptuous.Any(*options)
+    return schema
 
 
 def resolve_keyed_by(item, field, item_name, **extra_values):
     """
     For values which can either accept a literal value, or be keyed by some
     other attribute of the item, perform that lookup and replacement in-place
     (modifying `item` directly).  The field is specified using dotted notation
     to traverse dictionaries.
 
-    For example, given item
+    For example, given item::
 
         job:
             test-platform: linux128
             chunks:
                 by-test-platform:
                     macosx-10.11/debug: 13
                     win.*: 6
                     default: 12
 
     a call to `resolve_keyed_by(item, 'job.chunks', item['thing-name'])
-    would mutate item in-place to
+    would mutate item in-place to::
 
         job:
             chunks: 12
 
     The `item_name` parameter is used to generate useful error messages.
 
     If extra_values are supplied, they represent additional values available
     for reference from by-<field>.
+
+    Items can be nested as deeply as the schema will allow::
+
+        chunks:
+            by-test-platform:
+                win.*:
+                    by-project:
+                        ash: ..
+                        cedar: ..
+                linux: 13
+                default: 12
     """
     # find the field, returning the item unchanged if anything goes wrong
     container, subfield = item, field
     while '.' in subfield:
         f, subfield = subfield.split('.', 1)
         if f not in container:
             return item
         container = container[f]
         if not isinstance(container, dict):
             return item
 
     if subfield not in container:
         return item
     value = container[subfield]
-    if not isinstance(value, dict) or len(value) != 1 or not value.keys()[0].startswith('by-'):
-        return item
+    while True:
+        if not isinstance(value, dict) or len(value) != 1 or not value.keys()[0].startswith('by-'):
+            return item
 
-    keyed_by = value.keys()[0][3:]  # strip off 'by-' prefix
-    key = extra_values.get(keyed_by) if keyed_by in extra_values else item[keyed_by]
-    alternatives = value.values()[0]
+        keyed_by = value.keys()[0][3:]  # strip off 'by-' prefix
+        key = extra_values.get(keyed_by) if keyed_by in extra_values else item[keyed_by]
+        alternatives = value.values()[0]
 
-    # exact match
-    if key in alternatives:
-        container[subfield] = alternatives[key]
-        return item
+        # exact match
+        if key in alternatives:
+            value = container[subfield] = alternatives[key]
+            continue
 
-    # regular expression match
-    matches = [(k, v) for k, v in alternatives.iteritems() if re.match(k + '$', key)]
-    if len(matches) > 1:
+        # regular expression match
+        matches = [(k, v) for k, v in alternatives.iteritems() if re.match(k + '$', key)]
+        if len(matches) > 1:
+            raise Exception(
+                "Multiple matching values for {} {!r} found while "
+                "determining item {} in {}".format(
+                    keyed_by, key, field, item_name))
+        elif matches:
+            value = container[subfield] = matches[0][1]
+            continue
+
+        # default
+        if 'default' in alternatives:
+            value = container[subfield] = alternatives['default']
+            continue
+
         raise Exception(
-            "Multiple matching values for {} {!r} found while determining item {} in {}".format(
+            "No {} matching {!r} nor 'default' found while determining item {} in {}".format(
                 keyed_by, key, field, item_name))
-    elif matches:
-        container[subfield] = matches[0][1]
-        return item
-
-    # default
-    if 'default' in alternatives:
-        container[subfield] = alternatives['default']
-        return item
-
-    raise Exception(
-        "No {} matching {!r} nor 'default' found while determining item {} in {}".format(
-            keyed_by, key, field, item_name))