Skip to content

Commit f54a1e5

Browse files
ammokhovammokhov
authored
Input/Output comparison must traverse raw schema (#820)
1 parent ae2212a commit f54a1e5

File tree

4 files changed

+157
-9
lines changed

4 files changed

+157
-9
lines changed

‎src/rpdk/core/contract/resource_client.py‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
get_temporary_credentials,
2323
)
2424
from ..jsonutils.pointer import fragment_decode, fragment_list
25-
from ..jsonutils.utils import item_hash, traverse
25+
from ..jsonutils.utils import item_hash, traverse, traverse_raw_schema
2626

2727
LOG = logging.getLogger(__name__)
2828

@@ -348,7 +348,7 @@ def generate_invalid_update_example(self, create_model):
348348
example = override_properties(self.invalid_strategy.example(), overrides)
349349
return {**create_model, **example}
350350

351-
def compare(self, inputs, outputs):
351+
def compare(self, inputs, outputs, path=()):
352352
assertion_error_message = (
353353
"All properties specified in the request MUST "
354354
"be present in the model returned, and they MUST"
@@ -358,25 +358,29 @@ def compare(self, inputs, outputs):
358358
try:
359359
if isinstance(inputs, dict):
360360
for key in inputs:
361+
new_path = path + (key,)
361362
if isinstance(inputs[key], dict):
362-
self.compare(inputs[key], outputs[key])
363+
self.compare(inputs[key], outputs[key], new_path)
363364
elif isinstance(inputs[key], list):
364365
assert len(inputs[key]) == len(outputs[key])
365-
is_ordered = self._schema["properties"][key].get(
366+
367+
is_ordered = traverse_raw_schema(self._schema, new_path).get(
366368
"insertionOrder", True
367369
)
368-
self.compare_collection(inputs[key], outputs[key], is_ordered)
370+
self.compare_collection(
371+
inputs[key], outputs[key], is_ordered, new_path
372+
)
369373
else:
370374
assert inputs[key] == outputs[key], assertion_error_message
371375
else:
372376
assert inputs == outputs, assertion_error_message
373377
except Exception as exception:
374378
raise AssertionError(assertion_error_message) from exception
375379

376-
def compare_collection(self, inputs, outputs, is_ordered):
380+
def compare_collection(self, inputs, outputs, is_ordered, path):
377381
if is_ordered:
378382
for index in range(len(inputs)): # pylint: disable=C0200
379-
self.compare(inputs[index], outputs[index])
383+
self.compare(inputs[index], outputs[index], path)
380384
return
381385

382386
assert {item_hash(item) for item in inputs} == {

‎src/rpdk/core/jsonutils/utils.py‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import hashlib
22
import json
3+
import logging
34
from collections.abc import Mapping, Sequence
45
from typing import Any
56

7+
from nested_lookup import nested_lookup
68
from ordered_set import OrderedSet
79

8-
from .pointer import fragment_encode
10+
from .pointer import fragment_decode, fragment_encode
11+
12+
LOG = logging.getLogger(__name__)
913

1014
NON_MERGABLE_KEYS = ("uniqueItems", "insertionOrder")
1115
TYPE = "type"
@@ -127,6 +131,60 @@ def traverse(document, path_parts):
127131
return document, tuple(path), parent
128132

129133

134+
def _resolve_ref(sub_schema, definitions):
135+
# resolve $ref
136+
ref = nested_lookup(REF, sub_schema)
137+
if ref:
138+
# [0] should be a single $ref in subschema on the top level
139+
# [-1] $ref must follow #/definitions/object
140+
sub_schema = definitions[fragment_decode(ref[0])[-1]]
141+
# resolve properties
142+
properties = nested_lookup("properties", sub_schema)
143+
if properties:
144+
sub_schema = properties[0]
145+
return sub_schema
146+
147+
148+
# pylint: disable=C0301
149+
def traverse_raw_schema(schema: dict, path: tuple):
150+
"""Traverse the raw json schema resolving $ref
151+
152+
:raises TypeError: either schema is not of type dict
153+
:raises ConstraintError: the schema tries to override "type" or "$ref"
154+
155+
>>> traverse_raw_schema({"properties": {"bar": [42]}}, tuple())
156+
{'bar': [42]}
157+
>>> traverse_raw_schema({"properties": {"bar": [42]}}, ("bar",))
158+
[42]
159+
160+
>>> traverse_raw_schema({"definitions": {"bar": {"type": "boolean"}},"properties": {"bar": {"$ref": "#/definitions/bar"}}}, ("bar",))
161+
{'type': 'boolean'}
162+
163+
>>> traverse_raw_schema({"definitions":{"b":[1],"f":{"properties":{"b":{"$ref":"#/definitions/b"}}}},"properties":{"f":{"$ref":"#/definitions/f"}}},("f", "b")) # noqa: B950
164+
[1]
165+
166+
>>> traverse_raw_schema({}, ("foo"))
167+
{}
168+
>>> traverse_raw_schema([], ["foo"])
169+
Traceback (most recent call last):
170+
...
171+
TypeError: Schema must be a dictionary
172+
"""
173+
if not isinstance(schema, Mapping):
174+
raise TypeError("Schema must be a dictionary")
175+
176+
try:
177+
properties = schema["properties"]
178+
definitions = schema.get("definitions", {})
179+
sub_properties = properties
180+
for step in path:
181+
sub_properties = _resolve_ref(sub_properties[step], definitions)
182+
return sub_properties
183+
except KeyError as e:
184+
LOG.debug("Malformed Schema or incorrect path provided\n%s\n%s", path, e)
185+
return {}
186+
187+
130188
def schema_merge(target, src, path): # noqa: C901 # pylint: disable=R0912
131189
"""Merges the src schema into the target schema in place.
132190

‎src/rpdk/core/project.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,12 +551,12 @@ def _add_resources_content_to_zip(self, zip_file):
551551
cli_metadata["cli-version"] = __version__
552552
zip_file.writestr(CFN_METADATA_FILENAME, json.dumps(cli_metadata))
553553

554+
# pylint: disable=R1732
554555
def _create_context_manager(self, dry_run):
555556
# if it's a dry run, keep the file; otherwise can delete after upload
556557
if dry_run:
557558
return self._get_zip_file_path().open("wb")
558559

559-
# pylint: disable=consider-using-with
560560
return TemporaryFile("w+b")
561561

562562
def _get_zip_file_path(self):

‎tests/contract/test_resource_client.py‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,92 @@ def test_compare_should_throw_exception(resource_client):
14131413
},
14141414
{"properties": {"CollectionToCompare": {"insertionOrder": False}}},
14151415
),
1416+
(
1417+
{
1418+
"Collection": {
1419+
"PropertyA": {"A": True},
1420+
"CollectionToCompare": ["item1", "item2", "item3"],
1421+
}
1422+
},
1423+
{
1424+
"Collection": {
1425+
"PropertyA": {"A": True},
1426+
"CollectionToCompare": ["item3", "item2", "item1"],
1427+
}
1428+
},
1429+
{
1430+
"definitions": {
1431+
"PropertyA": {
1432+
"type": "object",
1433+
"additionalProperties": False,
1434+
"properties": {"A": {"type": "boolean"}},
1435+
},
1436+
"Collection": {
1437+
"type": "object",
1438+
"additionalProperties": False,
1439+
"properties": {
1440+
"PropertyA": {"$ref": "#/definitions/PropertyA"},
1441+
"CollectionToCompare": {
1442+
"insertionOrder": False,
1443+
"type": "array",
1444+
"items": {"type": "string", "minItems": 1},
1445+
},
1446+
},
1447+
},
1448+
},
1449+
"properties": {"Collection": {"$ref": "#/definitions/Collection"}},
1450+
},
1451+
),
1452+
(
1453+
{
1454+
"Collections": [
1455+
{
1456+
"InnerCollection": {
1457+
"Items": ["item2", "item1"],
1458+
"IntegerProperty": 10,
1459+
}
1460+
}
1461+
]
1462+
},
1463+
{
1464+
"Collections": [
1465+
{
1466+
"InnerCollection": {
1467+
"Items": ["item1", "item2"],
1468+
"IntegerProperty": 10,
1469+
}
1470+
}
1471+
]
1472+
},
1473+
{
1474+
"definitions": {
1475+
"InnerCollection": {
1476+
"type": "object",
1477+
"properties": {
1478+
"Items": {
1479+
"type": "array",
1480+
"insertionOrder": False,
1481+
"items": {"type": "string"},
1482+
},
1483+
"IntegerProperty": {"type": "integer"},
1484+
},
1485+
},
1486+
"Collection": {
1487+
"type": "object",
1488+
"properties": {
1489+
"InnerCollection": {"$ref": "#/definitions/InnerCollection"}
1490+
},
1491+
},
1492+
},
1493+
"properties": {
1494+
"Collections": {
1495+
"type": "array",
1496+
"uniqueItems": True,
1497+
"items": {"$ref": "#/definitions/Collection"},
1498+
},
1499+
},
1500+
},
1501+
),
14161502
],
14171503
)
14181504
def test_compare_collection(resource_client, inputs, outputs, schema_fragment):

0 commit comments

Comments
 (0)