From cf548f25338b3dff65fa818b5498009194c9a0fe Mon Sep 17 00:00:00 2001 From: Zac-HD Date: Thu, 16 Jul 2020 23:53:11 +1000 Subject: [PATCH] Fix #57 dependencies merging --- CHANGELOG.md | 3 ++ src/hypothesis_jsonschema/__init__.py | 2 +- src/hypothesis_jsonschema/_canonicalise.py | 62 +++++++++++++++++++--- tests/test_canonicalise.py | 37 +++++++++++++ 4 files changed, 95 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1427939..edef7c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +#### 0.17.2 - 2020-07-16 +- improved handling of overlapping `dependencies` keywords (#57) + #### 0.17.1 - 2020-07-16 - fixed an internal bug where results incorrectly depended on iteration order (#59) diff --git a/src/hypothesis_jsonschema/__init__.py b/src/hypothesis_jsonschema/__init__.py index bd031b6..a1b40ad 100644 --- a/src/hypothesis_jsonschema/__init__.py +++ b/src/hypothesis_jsonschema/__init__.py @@ -3,7 +3,7 @@ The only public API is `from_schema`; check the docstring for details. """ -__version__ = "0.17.1" +__version__ = "0.17.2" __all__ = ["from_schema"] from ._from_schema import from_schema diff --git a/src/hypothesis_jsonschema/_canonicalise.py b/src/hypothesis_jsonschema/_canonicalise.py index 00e1494..a11c9be 100644 --- a/src/hypothesis_jsonschema/_canonicalise.py +++ b/src/hypothesis_jsonschema/_canonicalise.py @@ -422,9 +422,25 @@ def canonicalish(schema: JSONType) -> Dict[str, Any]: "maxProperties", math.inf ): type_.remove("object") - # Remove no-op requires - if "required" in schema and not schema["required"]: - schema.pop("required") + # Discard dependencies values that don't restrict anything + for k, v in schema.get("dependencies", {}).copy().items(): + if v == [] or v == TRUTHY: + schema["dependencies"].pop(k) + # Remove no-op keywords + for kw, identity in { + "minItems": 0, + "items": {}, + "additionalItems": {}, + "dependencies": {}, + "minProperties": 0, + "properties": {}, + "propertyNames": {}, + "patternProperties": {}, + "additionalProperties": {}, + "required": [], + }.items(): + if kw in schema and schema[kw] == identity: + schema.pop(kw) # Canonicalise "required" schemas to remove redundancy if "object" in type_ and "required" in schema: assert isinstance(schema["required"], list) @@ -433,16 +449,18 @@ def canonicalish(schema: JSONType) -> Dict[str, Any]: # When the presence of a required property requires other properties via # dependencies, those properties can be moved to the base required keys. dep_names = { - k: sorted(v) + k: sorted(set(v)) for k, v in schema["dependencies"].items() if isinstance(v, list) } + schema["dependencies"].update(dep_names) while reqs.intersection(dep_names): for r in reqs.intersection(dep_names): reqs.update(dep_names.pop(r)) - for k, v in list(schema["dependencies"].items()): - if isinstance(v, list) and k not in dep_names: - schema["dependencies"].pop(k) + schema["dependencies"].pop(r) + # TODO: else merge schema-dependencies of required properties + # into the base schema after adding required back in and being + # careful to avoid an infinite loop... schema["required"] = sorted(reqs) max_ = schema.get("maxProperties", float("inf")) assert isinstance(max_, (int, float)) @@ -782,9 +800,37 @@ def merged(schemas: List[Any]) -> Optional[Schema]: s.pop("contains") if "not" in out and "not" in s and out["not"] != s["not"]: out["not"] = {"anyOf": [out["not"], s.pop("not")]} + if ( + "dependencies" in out + and "dependencies" in s + and out["dependencies"] != s["dependencies"] + ): + # Note: draft 2019-09 added separate keywords for name-dependencies + # and schema-dependencies, but when we add support for that it will + # be by canonicalising to the existing backwards-compatible keyword. + # + # In each dependencies dict, the keys are property names and the values + # are either a list of required names, or a schema that the whole + # instance must match. To merge a list and a schema, convert the + # former into a `required` key! + odeps = out["dependencies"] + for k, v in odeps.copy().items(): + if k in s["dependencies"]: + sval = s["dependencies"].pop(k) + if isinstance(v, list) and isinstance(sval, list): + odeps[k] = v + sval + continue + if isinstance(v, list): + v = {"required": v} + elif isinstance(sval, list): + sval = {"required": sval} + m = merged([v, sval]) + if m is None: + return None + odeps[k] = m + odeps.update(s.pop("dependencies")) # TODO: merge `items` schemas or lists-of-schemas - # TODO: merge dependencies # This loop handles the remaining cases. Notably, we do not attempt to # merge distinct values for: diff --git a/tests/test_canonicalise.py b/tests/test_canonicalise.py index 02451eb..6471507 100644 --- a/tests/test_canonicalise.py +++ b/tests/test_canonicalise.py @@ -356,6 +356,43 @@ def test_canonicalises_to_expected(schema, expected): [{"not": {"enum": [1, 2, 3]}}, {"not": {"enum": ["a", "b", "c"]}}], {"not": {"anyOf": [{"enum": ["a", "b", "c"]}, {"enum": [1, 2, 3]}]}}, ), + ( + [{"dependencies": {"a": ["b"]}}, {"dependencies": {"a": ["c"]}}], + {"dependencies": {"a": ["b", "c"]}}, + ), + ( + [{"dependencies": {"a": ["b"]}}, {"dependencies": {"b": ["c"]}}], + {"dependencies": {"a": ["b"], "b": ["c"]}}, + ), + ( + [ + {"dependencies": {"a": ["b"]}}, + {"dependencies": {"a": {"properties": {"b": {"type": "string"}}}}}, + ], + { + "dependencies": { + "a": {"required": ["b"], "properties": {"b": {"type": "string"}}} + }, + }, + ), + ( + [ + {"dependencies": {"a": {"properties": {"b": {"type": "string"}}}}}, + {"dependencies": {"a": ["b"]}}, + ], + { + "dependencies": { + "a": {"required": ["b"], "properties": {"b": {"type": "string"}}} + }, + }, + ), + ( + [ + {"dependencies": {"a": {"pattern": "a"}}}, + {"dependencies": {"a": {"pattern": "b"}}}, + ], + None, + ), ] + [ ([{lo: 0, hi: 9}, {lo: 1, hi: 10}], {lo: 1, hi: 9})