Multi Ref Support (#619)

aws-cloudformation · Nov 20, 2020 · 55c2ad0 · 55c2ad0
1 parent ae1f5ba
commit 55c2ad0
Show file tree

Hide file tree

Showing 9 changed files with 435 additions and 37 deletions.
diff --git a/requirements.txt b/requirements.txt
@@ -11,6 +11,8 @@ pytest-random-order>=1.0.4
 hypothesis>=4.32.3
 pytest-localserver>=0.5.0
 
+ordered-set>=4.0.2
+
 # commit hooks
 pre-commit>=1.18.1
 

diff --git a/setup.cfg b/setup.cfg
@@ -29,7 +29,7 @@ include_trailing_comma = true
 combine_as_imports = True
 force_grid_wrap = 0
 known_first_party = rpdk
-known_third_party = boto3,botocore,colorama,docker,hypothesis,jinja2,jsonschema,pkg_resources,pytest,pytest_localserver,setuptools,yaml
+known_third_party = boto3,botocore,colorama,docker,hypothesis,jinja2,jsonschema,ordered_set,pkg_resources,pytest,pytest_localserver,setuptools,yaml
 
 [tool:pytest]
 # can't do anything about 3rd part modules, so don't spam us

diff --git a/src/rpdk/core/jsonutils/flattener.py b/src/rpdk/core/jsonutils/flattener.py
@@ -1,8 +1,10 @@
 # pylint: disable=too-few-public-methods,raising-format-tuple
 import logging
 
+from ordered_set import OrderedSet
+
 from .pointer import fragment_decode
-from .utils import ConstraintError, FlatteningError, schema_merge, traverse
+from .utils import TYPE, ConstraintError, FlatteningError, schema_merge, traverse
 
 LOG = logging.getLogger(__name__)
 COMBINERS = ("oneOf", "anyOf", "allOf")
@@ -50,7 +52,6 @@ def _walk(self, sub_schema, property_path):
         except KeyError:
             # schemas without type are assumed to be objects
             json_type = sub_schema.get("type", "object")
-
             if json_type == "array":
                 sub_schema = self._flatten_array_type(sub_schema, property_path)
 
@@ -157,19 +158,24 @@ def _flatten_combiners(self, sub_schema, path):
             try:
                 schema_array = sub_schema.pop(arr_key)
             except KeyError:
-                continue
-            for i, nested_schema in enumerate(schema_array):
-                ref_path = path + (arr_key, i)
-                ref_path_is_used = ref_path in self._schema_map
-                walked_schema = self._walk(nested_schema, ref_path)
-
-                # if no other schema is referencing the ref_path,
-                # we no longer need the refkey since the properties will be squashed
-                if ref_path_is_used:
-                    resolved_schema = self._schema_map.get(ref_path)
-                else:
-                    resolved_schema = self._schema_map.pop(ref_path, walked_schema)
-                schema_merge(sub_schema, resolved_schema, path)
+                pass
+            else:
+                for i, nested_schema in enumerate(schema_array):
+
+                    ref_path = path + (arr_key, i)
+                    ref_path_is_used = ref_path in self._schema_map
+                    walked_schema = self._walk(nested_schema, ref_path)
+
+                    # if no other schema is referencing the ref_path,
+                    # we no longer need the refkey since the properties will be squashed
+                    if ref_path_is_used:
+                        resolved_schema = self._schema_map.get(ref_path)
+                    else:
+                        resolved_schema = self._schema_map.pop(ref_path, walked_schema)
+                    schema_merge(sub_schema, resolved_schema, path)
+
+        if isinstance(sub_schema.get(TYPE), OrderedSet):
+            sub_schema[TYPE] = list(sub_schema[TYPE])
         return sub_schema
 
     def _find_subschema_by_ref(self, ref_path):

diff --git a/src/rpdk/core/jsonutils/utils.py b/src/rpdk/core/jsonutils/utils.py
@@ -1,14 +1,27 @@
 from collections.abc import Mapping, Sequence
+from typing import Any
+
+from ordered_set import OrderedSet
 
 from .pointer import fragment_encode
 
-NON_MERGABLE_KEYS = ("$ref", "uniqueItems", "insertionOrder")
+NON_MERGABLE_KEYS = ("uniqueItems", "insertionOrder")
+TYPE = "type"
+REF = "$ref"
 
 
 class FlatteningError(Exception):
     pass
 
 
+def to_set(value: Any) -> OrderedSet:
+    return (
+        OrderedSet(value)
+        if isinstance(value, (list, OrderedSet))
+        else OrderedSet([value])
+    )
+
+
 class ConstraintError(FlatteningError, ValueError):
     def __init__(self, message, path, *args):
         self.path = fragment_encode(path)
@@ -98,7 +111,7 @@ def traverse(document, path_parts):
     return document, tuple(path), parent
 
 
-def schema_merge(target, src, path):  # noqa: C901
+def schema_merge(target, src, path):  # noqa: C901 # pylint: disable=R0912
     """Merges the src schema into the target schema in place.
 
     If there are duplicate keys, src will overwrite target.
@@ -110,39 +123,97 @@ def schema_merge(target, src, path):  # noqa: C901
     {}
     >>> schema_merge({'foo': 'a'}, {}, ())
     {'foo': 'a'}
+
     >>> schema_merge({}, {'foo': 'a'}, ())
     {'foo': 'a'}
+
     >>> schema_merge({'foo': 'a'}, {'foo': 'b'}, ())
     {'foo': 'b'}
+
     >>> schema_merge({'required': 'a'}, {'required': 'b'}, ())
     {'required': ['a', 'b']}
+
+    >>> a, b = {'$ref': 'a'}, {'foo': 'b'}
+    >>> schema_merge(a, b, ('foo',))
+    {'$ref': 'a', 'foo': 'b'}
+
     >>> a, b = {'$ref': 'a'}, {'type': 'b'}
     >>> schema_merge(a, b, ('foo',))
-    {'$ref': 'a', 'type': 'b'}
+    {'type': OrderedSet(['a', 'b'])}
+
+    >>> a, b = {'$ref': 'a'}, {'$ref': 'b'}
+    >>> schema_merge(a, b, ('foo',))
+    {'type': OrderedSet(['a', 'b'])}
+
+    >>> a, b = {'$ref': 'a'}, {'type': ['b', 'c']}
+    >>> schema_merge(a, b, ('foo',))
+    {'type': OrderedSet(['a', 'b', 'c'])}
+
+    >>> a, b = {'$ref': 'a'}, {'type': OrderedSet(['b', 'c'])}
+    >>> schema_merge(a, b, ('foo',))
+    {'type': OrderedSet(['a', 'b', 'c'])}
+
+    >>> a, b = {'type': ['a', 'b']}, {'$ref': 'c'}
+    >>> schema_merge(a, b, ('foo',))
+    {'type': OrderedSet(['a', 'b', 'c'])}
+
+    >>> a, b = {'type': OrderedSet(['a', 'b'])}, {'$ref': 'c'}
+    >>> schema_merge(a, b, ('foo',))
+    {'type': OrderedSet(['a', 'b', 'c'])}
+
     >>> a, b = {'Foo': {'$ref': 'a'}}, {'Foo': {'type': 'b'}}
     >>> schema_merge(a, b, ('foo',))
-    {'Foo': {'$ref': 'a', 'type': 'b'}}
+    {'Foo': {'type': OrderedSet(['a', 'b'])}}
+
     >>> schema_merge({'type': 'a'}, {'type': 'b'}, ()) # doctest: +NORMALIZE_WHITESPACE
-    {'type': ['a', 'b']}
+    {'type': OrderedSet(['a', 'b'])}
+
+    >>> schema_merge({'type': 'string'}, {'type': 'integer'}, ())
+    {'type': OrderedSet(['string', 'integer'])}
     """
     if not (isinstance(target, Mapping) and isinstance(src, Mapping)):
         raise TypeError("Both schemas must be dictionaries")
 
     for key, src_schema in src.items():
         try:
-            target_schema = target[key]
+            if key in (
+                REF,
+                TYPE,
+            ):  # $ref and type are treated similarly and unified
+                target_schema = target.get(key) or target.get(TYPE) or target[REF]
+            else:
+                target_schema = target[key]  # carry over existing properties
         except KeyError:
             target[key] = src_schema
         else:
             next_path = path + (key,)
             try:
                 target[key] = schema_merge(target_schema, src_schema, next_path)
             except TypeError:
-                if key == "type":
-                    if isinstance(target_schema, list):
-                        target_schema.append(src_schema)
-                        continue
-                    target[key] = [target_schema, src_schema]
+                if key in (TYPE, REF):  # combining multiple $ref and types
+                    src_set = to_set(src_schema)
+
+                    try:
+                        target[TYPE] = to_set(
+                            target[TYPE]
+                        )  # casting to ordered set as lib
+                        # implicitly converts strings to sets
+                        target[TYPE] |= src_set
+                    except (TypeError, KeyError):
+                        target_set = to_set(target_schema)
+                        target[TYPE] = target_set | src_set
+
+                    try:
+                        # check if there are conflicting $ref and type
+                        # at the same sub schema. Conflicting $ref could only
+                        # happen on combiners because method merges two json
+                        # objects without losing any previous info:
+                        # e.g. "oneOf": [{"$ref": "..#1.."},{"$ref": "..#2.."}] ->
+                        # { "ref": "..#1..", "type": [{},{}] }
+                        target.pop(REF)
+                    except KeyError:
+                        pass
+
                 elif key == "required":
                     target[key] = sorted(set(target_schema) | set(src_schema))
                 else:

diff --git a/src/rpdk/core/project.py b/src/rpdk/core/project.py
@@ -646,7 +646,6 @@ def __set_property_type(prop_type, single_type=True):
             prop[jsontype] = __join(prop.get(jsontype), type_json)
             prop[yamltype] = __join(prop.get(yamltype), type_yaml)
             prop[longformtype] = __join(prop.get(longformtype), type_longform)
-
             if "enum" in prop:
                 prop["allowedvalues"] = prop["enum"]
 
@@ -657,12 +656,19 @@ def __set_property_type(prop_type, single_type=True):
             prop_type = [prop_type]
             single_item = True
 
-        visited = set()
         for prop_item in prop_type:
-            if prop_item not in visited:
-                visited.add(prop_item)
+            if isinstance(prop_item, tuple):  # if tuple, then it's a ref
+                # using doc method to generate the mdo and reassign the ref
+                resolved = self._set_docs_properties(
+                    propname, {"$ref": prop_item}, proppath
+                )
+                prop[jsontype] = __join(prop.get(jsontype), resolved[jsontype])
+                prop[yamltype] = __join(prop.get(yamltype), resolved[yamltype])
+                prop[longformtype] = __join(
+                    prop.get(longformtype), resolved[longformtype]
+                )
+            else:
                 __set_property_type(prop_item, single_type=single_item)
-
         return prop
 
     def _upload(

diff --git a/tests/data/schema/target_output/multiref.md b/tests/data/schema/target_output/multiref.md
@@ -0,0 +1,126 @@
+# AWS::Color::Red
+
+a test schema
+
+## Syntax
+
+To declare this entity in your AWS CloudFormation template, use the following syntax:
+
+### JSON
+
+<pre>
+{
+    "Type" : "AWS::Color::Red",
+    "Properties" : {
+        "<a href="#primaryid" title="primaryID">primaryID</a>" : <i>String</i>,
+        "<a href="#propertywithmultipleconstraints" title="PropertyWithMultipleConstraints">PropertyWithMultipleConstraints</a>" : <i>String</i>,
+        "<a href="#propertywithmultiplemultiples" title="PropertyWithMultipleMultiples">PropertyWithMultipleMultiples</a>" : <i>String, Map, Integer, Boolean</i>,
+        "<a href="#propertywithmultipleprimitives" title="PropertyWithMultiplePrimitives">PropertyWithMultiplePrimitives</a>" : <i>Integer, String, Map</i>,
+        "<a href="#propertywithtwocomplextypes" title="PropertyWithTwoComplexTypes">PropertyWithTwoComplexTypes</a>" : <i><a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a></i>,
+        "<a href="#propertywithmultiplecomplextypes" title="PropertyWithMultipleComplexTypes">PropertyWithMultipleComplexTypes</a>" : <i><a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a>, <a href="complextypewithcircularref.md">ComplexTypeWithCircularRef</a></i>,
+        "<a href="#propertywithmultiplecomplextypesandoneprimitive" title="PropertyWithMultipleComplexTypesAndOnePrimitive">PropertyWithMultipleComplexTypesAndOnePrimitive</a>" : <i><a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a>, <a href="complextypewithcircularref.md">ComplexTypeWithCircularRef</a>, Map</i>,
+        "<a href="#propertywithcomplextypeandprimitive" title="PropertyWithComplexTypeAndPrimitive">PropertyWithComplexTypeAndPrimitive</a>" : <i><a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, Map</i>,
+        "<a href="#multiproperty3" title="MultiProperty3">MultiProperty3</a>" : <i>Integer, Map</i>
+    }
+}
+</pre>
+
+### YAML
+
+<pre>
+Type: AWS::Color::Red
+Properties:
+    <a href="#primaryid" title="primaryID">primaryID</a>: <i>String</i>
+    <a href="#propertywithmultipleconstraints" title="PropertyWithMultipleConstraints">PropertyWithMultipleConstraints</a>: <i>String</i>
+    <a href="#propertywithmultiplemultiples" title="PropertyWithMultipleMultiples">PropertyWithMultipleMultiples</a>: <i>String, Map, Integer, Boolean</i>
+    <a href="#propertywithmultipleprimitives" title="PropertyWithMultiplePrimitives">PropertyWithMultiplePrimitives</a>: <i>Integer, String, Map</i>
+    <a href="#propertywithtwocomplextypes" title="PropertyWithTwoComplexTypes">PropertyWithTwoComplexTypes</a>: <i><a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a></i>
+    <a href="#propertywithmultiplecomplextypes" title="PropertyWithMultipleComplexTypes">PropertyWithMultipleComplexTypes</a>: <i><a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a>, <a href="complextypewithcircularref.md">ComplexTypeWithCircularRef</a></i>
+    <a href="#propertywithmultiplecomplextypesandoneprimitive" title="PropertyWithMultipleComplexTypesAndOnePrimitive">PropertyWithMultipleComplexTypesAndOnePrimitive</a>: <i><a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a>, <a href="complextypewithcircularref.md">ComplexTypeWithCircularRef</a>, Map</i>
+    <a href="#propertywithcomplextypeandprimitive" title="PropertyWithComplexTypeAndPrimitive">PropertyWithComplexTypeAndPrimitive</a>: <i><a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, Map</i>
+    <a href="#multiproperty3" title="MultiProperty3">MultiProperty3</a>: <i>Integer, Map</i>
+</pre>
+
+## Properties
+
+#### primaryID
+
+_Required_: No
+
+_Type_: String
+
+_Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement)
+
+#### PropertyWithMultipleConstraints
+
+_Required_: No
+
+_Type_: String
+
+_Minimum_: <code>13</code>
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+
+#### PropertyWithMultipleMultiples
+
+_Required_: No
+
+_Type_: String, Map, Integer, Boolean
+
+_Minimum_: <code>13</code>
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+
+#### PropertyWithMultiplePrimitives
+
+_Required_: No
+
+_Type_: Integer, String, Map
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+
+#### PropertyWithTwoComplexTypes
+
+_Required_: No
+
+_Type_: <a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a>
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+
+#### PropertyWithMultipleComplexTypes
+
+_Required_: No
+
+_Type_: <a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a>, <a href="complextypewithcircularref.md">ComplexTypeWithCircularRef</a>
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+
+#### PropertyWithMultipleComplexTypesAndOnePrimitive
+
+_Required_: No
+
+_Type_: <a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, <a href="complextypewithmultipleprimitives.md">ComplexTypeWithMultiplePrimitives</a>, <a href="complextypewithcircularref.md">ComplexTypeWithCircularRef</a>, Map
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+
+#### PropertyWithComplexTypeAndPrimitive
+
+_Required_: No
+
+_Type_: <a href="complextypewithoneprimitive.md">ComplexTypeWithOnePrimitive</a>, Map
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+
+#### MultiProperty3
+
+_Required_: No
+
+_Type_: Integer, Map
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+
+## Return Values
+
+### Ref
+
+When you pass the logical ID of this resource to the intrinsic `Ref` function, Ref returns the primaryID.