From ddff7e400096bef5d7cb78f9da04aec1870b085e Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 27 Nov 2024 13:33:06 -0500 Subject: [PATCH 1/8] apply hotfix for schema info collection combiner handling --- asdf/_node_info.py | 97 +++++++++++++++++++++++++++++++++------- asdf/_tests/test_info.py | 37 ++++++++++----- asdf/exceptions.py | 6 +++ 3 files changed, 113 insertions(+), 27 deletions(-) diff --git a/asdf/_node_info.py b/asdf/_node_info.py index f45ef3b13..18551ef6d 100644 --- a/asdf/_node_info.py +++ b/asdf/_node_info.py @@ -1,6 +1,7 @@ import re from collections import namedtuple +from .exceptions import AsdfSchemaResolutionError from .schema import load_schema from .treeutil import get_children @@ -20,6 +21,82 @@ def _filter_tree(info, filters): return len(info.children) > 0 or all(f(info.node, info.identifier) for f in filters) +def _get_matching_schema_property(schema, key): + if "properties" in schema: + props = schema["properties"] + if key in props: + return props[key] + if "patternProperties" in props: + patterns = props["patternProperties"] + for regex in patterns: + if re.search(regex, key): + return patterns[regex] + return None + + +def _get_subschema_for_property(schema, key): + # This does NOT handle $ref the expectation is that the schema + # is loaded with resolve_references=True + applicable = [] + + # first check properties and patternProperties + subschema = _get_matching_schema_property(schema, key) + if subschema is not None: + applicable.append(subschema) + + # next handle schema combiners + if "not" in schema: + # since we're only concerned here with if the schema applies + # it doesn't matter if the schema is nested in a not + subschema = _get_subschema_for_property(schema["not"], key) + if subschema is not None: + applicable.append(subschema) + + for combiner in ("allOf", "oneOf", "anyOf"): + for combined_schema in schema.get(combiner, []): + subschema = _get_subschema_for_property(combined_schema, key) + if subschema is not None: + applicable.append(subschema) + + if not applicable: + return None + + if len(applicable) > 1: + msg = ( + f"schema info could not be determined for {key} since " + f"{len(applicable)} possibly applicable schemas were found." + ) + raise AsdfSchemaResolutionError(msg) + + return applicable[0] + + +def _get_schema_key(schema, key): + applicable = [] + if key in schema: + applicable.append(schema[key]) + if "not" in schema: + possible = _get_schema_key(schema["not"], key) + if possible is not None: + applicable.append(possible) + for combiner in ("allOf", "oneOf", "anyOf"): + for combined_schema in schema.get(combiner, []): + possible = _get_schema_key(combined_schema, key) + if possible is not None: + applicable.append(possible) + if not applicable: + return None + + if len(applicable) > 1: + msg = ( + f"schema info could not be determined for {key} since " + f"{len(applicable)} possibly applicable schemas were found." + ) + raise AsdfSchemaResolutionError(msg) + + return applicable[0] + + def create_tree(key, node, identifier="root", filters=None, refresh_extension_manager=False): """ Create a `NodeSchemaInfo` tree which can be filtered from a base node. @@ -214,22 +291,12 @@ def parent_node(self): @property def info(self): - if self.schema is not None: - return self.schema.get(self.key, None) - - return None + if self.schema is None: + return None + return _get_schema_key(self.schema, self.key) def get_schema_for_property(self, identifier): - subschema = self.schema.get("properties", {}).get(identifier, None) - if subschema is not None: - return subschema - - subschema = self.schema.get("properties", {}).get("patternProperties", None) - if subschema: - for key in subschema: - if re.search(key, identifier): - return subschema[key] - return {} + return _get_subschema_for_property(self.schema, identifier) or {} def set_schema_for_property(self, parent, identifier): """Extract a subschema from the parent for the identified property""" @@ -241,7 +308,7 @@ def set_schema_from_node(self, node, extension_manager): tag_def = extension_manager.get_tag_definition(node._tag) schema_uri = tag_def.schema_uris[0] - schema = load_schema(schema_uri) + schema = load_schema(schema_uri, resolve_references=True) self.schema = schema diff --git a/asdf/_tests/test_info.py b/asdf/_tests/test_info.py index 94353051b..469bc1b45 100644 --- a/asdf/_tests/test_info.py +++ b/asdf/_tests/test_info.py @@ -168,8 +168,8 @@ def manifest_extension(tmp_path): description: Some silly description type: integer archive_catalog: - datatype: int - destination: [ScienceCommon.silly] + datatype: int + destination: [ScienceCommon.silly] clown: title: clown name description: clown description @@ -231,14 +231,14 @@ def manifest_extension(tmp_path): title: Attribute1 Title type: string archive_catalog: - datatype: str - destination: [ScienceCommon.attribute1] + datatype: str + destination: [ScienceCommon.attribute1] attribute2: title: Attribute2 Title type: string archive_catalog: - datatype: str - destination: [ScienceCommon.attribute2] + datatype: str + destination: [ScienceCommon.attribute2] ... """ @@ -251,19 +251,29 @@ def manifest_extension(tmp_path): type: object title: object with info support 3 title description: object description +allOf: + - $ref: drink_ref-1.0.0 +... +""" + drink_ref_schema = """ +%YAML 1.1 +--- +$schema: "asdf://stsci.edu/schemas/asdf/asdf-schema-1.1.0" +id: "asdf://somewhere.org/asdf/schemas/drink_ref-1.0.0" properties: attributeOne: title: AttributeOne Title description: AttributeOne description type: string archive_catalog: - datatype: str - destination: [ScienceCommon.attributeOne] + datatype: str + destination: [ScienceCommon.attributeOne] attributeTwo: - title: AttributeTwo Title - description: AttributeTwo description - type: string - archive_catalog: + allOf: + - title: AttributeTwo Title + description: AttributeTwo description + type: string + archive_catalog: datatype: str destination: [ScienceCommon.attributeTwo] ... @@ -278,6 +288,9 @@ def manifest_extension(tmp_path): spath = tmp_path / "schemas" / "drink-1.0.0.yaml" with open(spath, "w") as fschema: fschema.write(drink_schema) + spath = tmp_path / "schemas" / "drink_ref-1.0.0.yaml" + with open(spath, "w") as fschema: + fschema.write(drink_ref_schema) os.mkdir(tmp_path / "manifests") mpath = str(tmp_path / "manifests" / "foo_manifest-1.0.yaml") with open(mpath, "w") as fmanifest: diff --git a/asdf/exceptions.py b/asdf/exceptions.py index 0851a8baa..fefcc5e26 100644 --- a/asdf/exceptions.py +++ b/asdf/exceptions.py @@ -84,3 +84,9 @@ class AsdfSerializationError(RepresenterError): that the object does not have a supporting asdf Converter and needs to be manually converted to a supported type. """ + + +class AsdfSchemaResolutionError(ValueError): + """ + An attempt to lookup schema for an attribute failed. + """ From 8e2ea1d9cdd063dfbed82ebd5bb77fe75aff5714 Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 27 Nov 2024 14:47:18 -0500 Subject: [PATCH 2/8] add changelog --- changes/1875.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/1875.bugfix.rst diff --git a/changes/1875.bugfix.rst b/changes/1875.bugfix.rst new file mode 100644 index 000000000..89960753a --- /dev/null +++ b/changes/1875.bugfix.rst @@ -0,0 +1 @@ +Improve ``schema_info`` handling of schemas with combiners (allOf, anyOf, etc). From 42545f35dd96fe98f86561a6b4bfa95830008c7a Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 27 Nov 2024 16:04:07 -0500 Subject: [PATCH 3/8] rename exception to AsdfInfoResolutionError --- asdf/_node_info.py | 6 +++--- asdf/exceptions.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/asdf/_node_info.py b/asdf/_node_info.py index 18551ef6d..490beff80 100644 --- a/asdf/_node_info.py +++ b/asdf/_node_info.py @@ -1,7 +1,7 @@ import re from collections import namedtuple -from .exceptions import AsdfSchemaResolutionError +from .exceptions import AsdfInfoResolutionError from .schema import load_schema from .treeutil import get_children @@ -66,7 +66,7 @@ def _get_subschema_for_property(schema, key): f"schema info could not be determined for {key} since " f"{len(applicable)} possibly applicable schemas were found." ) - raise AsdfSchemaResolutionError(msg) + raise AsdfInfoResolutionError(msg) return applicable[0] @@ -92,7 +92,7 @@ def _get_schema_key(schema, key): f"schema info could not be determined for {key} since " f"{len(applicable)} possibly applicable schemas were found." ) - raise AsdfSchemaResolutionError(msg) + raise AsdfInfoResolutionError(msg) return applicable[0] diff --git a/asdf/exceptions.py b/asdf/exceptions.py index fefcc5e26..cfdef90b5 100644 --- a/asdf/exceptions.py +++ b/asdf/exceptions.py @@ -86,7 +86,7 @@ class AsdfSerializationError(RepresenterError): """ -class AsdfSchemaResolutionError(ValueError): +class AsdfInfoResolutionError(ValueError): """ An attempt to lookup schema for an attribute failed. """ From 6ddf99b86163f9390905511a9ff3a295d020d24d Mon Sep 17 00:00:00 2001 From: Brett Date: Mon, 2 Dec 2024 12:23:32 -0500 Subject: [PATCH 4/8] add tests for info schema resolution --- asdf/_node_info.py | 8 +++----- asdf/_tests/test_info.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/asdf/_node_info.py b/asdf/_node_info.py index 490beff80..0d820213a 100644 --- a/asdf/_node_info.py +++ b/asdf/_node_info.py @@ -46,7 +46,7 @@ def _get_subschema_for_property(schema, key): # next handle schema combiners if "not" in schema: - # since we're only concerned here with if the schema applies + # Since we're only concerned here with if the schema applies # it doesn't matter if the schema is nested in a not subschema = _get_subschema_for_property(schema["not"], key) if subschema is not None: @@ -75,10 +75,8 @@ def _get_schema_key(schema, key): applicable = [] if key in schema: applicable.append(schema[key]) - if "not" in schema: - possible = _get_schema_key(schema["not"], key) - if possible is not None: - applicable.append(possible) + # Here we don't consider any subschema under "not" to avoid + # false positives for keys like "type" etc. for combiner in ("allOf", "oneOf", "anyOf"): for combined_schema in schema.get(combiner, []): possible = _get_schema_key(combined_schema, key) diff --git a/asdf/_tests/test_info.py b/asdf/_tests/test_info.py index 469bc1b45..b3df11c96 100644 --- a/asdf/_tests/test_info.py +++ b/asdf/_tests/test_info.py @@ -4,8 +4,10 @@ import tempfile import numpy as np +import pytest import asdf +from asdf.exceptions import AsdfInfoResolutionError from asdf.extension import ExtensionManager, ExtensionProxy, ManifestExtension from asdf.resource import DirectoryResourceMapping @@ -715,3 +717,33 @@ def __str__(self): assert "(NewlineStr)\n" in captured.out assert "(CarriageReturnStr)\n" in captured.out assert "(NiceStr): nice\n" in captured.out + + +@pytest.mark.parametrize( + "schema, expected", + [ + ({"title": "foo"}, "foo"), + ({"allOf": [{"title": "foo"}]}, "foo"), + ({"oneOf": [{"title": "foo"}]}, "foo"), + ({"anyOf": [{"title": "foo"}]}, "foo"), + ({"not": {"title": "foo"}}, None), + ], +) +def test_node_info(schema, expected): + ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) + assert ni.info == expected + + +@pytest.mark.parametrize( + "schema", + [ + {"allOf": [{"title": "foo"}, {"title": "bar"}]}, + {"oneOf": [{"title": "foo"}, {"title": "bar"}]}, + {"anyOf": [{"title": "foo"}, {"title": "bar"}]}, + {"allOf": [{"title": "foo"}, {"title": "bar"}]}, + ], +) +def test_node_info_failure(schema): + ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) + with pytest.raises(AsdfInfoResolutionError): + ni.info From f5ade03c6b9b3e1d46be1d8ab78cb320e3d88008 Mon Sep 17 00:00:00 2001 From: Brett Date: Mon, 2 Dec 2024 15:32:01 -0500 Subject: [PATCH 5/8] more tests --- asdf/_node_info.py | 7 +++-- asdf/_tests/test_info.py | 61 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/asdf/_node_info.py b/asdf/_node_info.py index 0d820213a..0a88ddaa2 100644 --- a/asdf/_node_info.py +++ b/asdf/_node_info.py @@ -46,11 +46,12 @@ def _get_subschema_for_property(schema, key): # next handle schema combiners if "not" in schema: - # Since we're only concerned here with if the schema applies - # it doesn't matter if the schema is nested in a not subschema = _get_subschema_for_property(schema["not"], key) if subschema is not None: - applicable.append(subschema) + # We can't resolve a valid subschema under a "not" since + # we'd have to know how to invert a schema + msg = f"schema info could not be determined for {key} since " f"it is nested under a 'not'." + raise AsdfInfoResolutionError(msg) for combiner in ("allOf", "oneOf", "anyOf"): for combined_schema in schema.get(combiner, []): diff --git a/asdf/_tests/test_info.py b/asdf/_tests/test_info.py index b3df11c96..7b28961ad 100644 --- a/asdf/_tests/test_info.py +++ b/asdf/_tests/test_info.py @@ -719,6 +719,65 @@ def __str__(self): assert "(NiceStr): nice\n" in captured.out +@pytest.mark.parametrize( + "schema, expected", + [ + ({"properties": {"foo": {"type": "object"}}}, {"type": "object"}), + ({"allOf": [{"properties": {"foo": {"type": "object"}}}]}, {"type": "object"}), + ({"oneOf": [{"properties": {"foo": {"type": "object"}}}]}, {"type": "object"}), + ({"anyOf": [{"properties": {"foo": {"type": "object"}}}]}, {"type": "object"}), + ], +) +def test_node_property(schema, expected): + ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) + assert ni.get_schema_for_property("foo") == expected + + +@pytest.mark.parametrize( + "schema, msg", + [ + ({"not": {"properties": {"foo": {"type": "object"}}}}, "nested under a 'not'"), + ( + {"properties": {"foo": {"type": "object"}}, "allOf": [{"properties": {"foo": {"type": "object"}}}]}, + "2 possibly applicable schemas", + ), + ( + {"properties": {"foo": {"type": "object"}}, "anyOf": [{"properties": {"foo": {"type": "object"}}}]}, + "2 possibly applicable schemas", + ), + ( + {"properties": {"foo": {"type": "object"}}, "oneOf": [{"properties": {"foo": {"type": "object"}}}]}, + "2 possibly applicable schemas", + ), + ( + { + "allOf": [{"properties": {"foo": {"type": "object"}}}], + "anyOf": [{"properties": {"foo": {"type": "object"}}}], + }, + "2 possibly applicable schemas", + ), + ( + { + "anyOf": [{"properties": {"foo": {"type": "object"}}}], + "oneOf": [{"properties": {"foo": {"type": "object"}}}], + }, + "2 possibly applicable schemas", + ), + ( + { + "oneOf": [{"properties": {"foo": {"type": "object"}}}], + "allOf": [{"properties": {"foo": {"type": "object"}}}], + }, + "2 possibly applicable schemas", + ), + ], +) +def test_node_property_error(schema, msg): + ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) + with pytest.raises(AsdfInfoResolutionError, match=msg): + ni.get_schema_for_property("foo") + + @pytest.mark.parametrize( "schema, expected", [ @@ -745,5 +804,5 @@ def test_node_info(schema, expected): ) def test_node_info_failure(schema): ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) - with pytest.raises(AsdfInfoResolutionError): + with pytest.raises(AsdfInfoResolutionError, match="2 possibly applicable schemas"): ni.info From e3ee3cda6e1e2b3d06dec10eda231789a9660704 Mon Sep 17 00:00:00 2001 From: Brett Date: Mon, 2 Dec 2024 15:37:03 -0500 Subject: [PATCH 6/8] update changelog with mention of exception --- changes/1875.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/1875.bugfix.rst b/changes/1875.bugfix.rst index 89960753a..fcdc9d162 100644 --- a/changes/1875.bugfix.rst +++ b/changes/1875.bugfix.rst @@ -1 +1 @@ -Improve ``schema_info`` handling of schemas with combiners (allOf, anyOf, etc). +Improve ``schema_info`` handling of schemas with combiners (allOf, anyOf, etc). Raise AsdfInfoResolutionError for ambiguous info. From afa9cbd78e7672548ae95ab069c6428c3a2155c8 Mon Sep 17 00:00:00 2001 From: Brett Date: Tue, 10 Dec 2024 09:13:52 -0500 Subject: [PATCH 7/8] dial back severity of ambiguity --- asdf/_asdf.py | 4 +++ asdf/_node_info.py | 33 +++++------------ asdf/_tests/test_info.py | 77 ++++++++++++---------------------------- asdf/exceptions.py | 6 ---- changes/1875.bugfix.rst | 2 +- 5 files changed, 37 insertions(+), 85 deletions(-) diff --git a/asdf/_asdf.py b/asdf/_asdf.py index 434733f87..b63349dbd 100644 --- a/asdf/_asdf.py +++ b/asdf/_asdf.py @@ -1378,6 +1378,10 @@ def schema_info(self, key="description", path=None, preserve_list=True, refresh_ """ Get a nested dictionary of the schema information for a given key, relative to the path. + This method will only return unambiguous info. If a property is subject to multiple + subschemas or contains ambiguous entries (multiple titles) no result will be returned + for that property. + Parameters ---------- key : str diff --git a/asdf/_node_info.py b/asdf/_node_info.py index 0a88ddaa2..048e3ad1b 100644 --- a/asdf/_node_info.py +++ b/asdf/_node_info.py @@ -1,7 +1,6 @@ import re from collections import namedtuple -from .exceptions import AsdfInfoResolutionError from .schema import load_schema from .treeutil import get_children @@ -50,8 +49,7 @@ def _get_subschema_for_property(schema, key): if subschema is not None: # We can't resolve a valid subschema under a "not" since # we'd have to know how to invert a schema - msg = f"schema info could not be determined for {key} since " f"it is nested under a 'not'." - raise AsdfInfoResolutionError(msg) + return None for combiner in ("allOf", "oneOf", "anyOf"): for combined_schema in schema.get(combiner, []): @@ -59,17 +57,10 @@ def _get_subschema_for_property(schema, key): if subschema is not None: applicable.append(subschema) - if not applicable: - return None - - if len(applicable) > 1: - msg = ( - f"schema info could not be determined for {key} since " - f"{len(applicable)} possibly applicable schemas were found." - ) - raise AsdfInfoResolutionError(msg) - - return applicable[0] + # only return the subschema if we found exactly 1 applicable + if len(applicable) == 1: + return applicable[0] + return None def _get_schema_key(schema, key): @@ -83,17 +74,11 @@ def _get_schema_key(schema, key): possible = _get_schema_key(combined_schema, key) if possible is not None: applicable.append(possible) - if not applicable: - return None - if len(applicable) > 1: - msg = ( - f"schema info could not be determined for {key} since " - f"{len(applicable)} possibly applicable schemas were found." - ) - raise AsdfInfoResolutionError(msg) - - return applicable[0] + # only return the property if we found exactly 1 applicable + if len(applicable) == 1: + return applicable[0] + return None def create_tree(key, node, identifier="root", filters=None, refresh_extension_manager=False): diff --git a/asdf/_tests/test_info.py b/asdf/_tests/test_info.py index 7b28961ad..cb8917c46 100644 --- a/asdf/_tests/test_info.py +++ b/asdf/_tests/test_info.py @@ -7,7 +7,6 @@ import pytest import asdf -from asdf.exceptions import AsdfInfoResolutionError from asdf.extension import ExtensionManager, ExtensionProxy, ManifestExtension from asdf.resource import DirectoryResourceMapping @@ -734,48 +733,29 @@ def test_node_property(schema, expected): @pytest.mark.parametrize( - "schema, msg", + "schema", [ - ({"not": {"properties": {"foo": {"type": "object"}}}}, "nested under a 'not'"), - ( - {"properties": {"foo": {"type": "object"}}, "allOf": [{"properties": {"foo": {"type": "object"}}}]}, - "2 possibly applicable schemas", - ), - ( - {"properties": {"foo": {"type": "object"}}, "anyOf": [{"properties": {"foo": {"type": "object"}}}]}, - "2 possibly applicable schemas", - ), - ( - {"properties": {"foo": {"type": "object"}}, "oneOf": [{"properties": {"foo": {"type": "object"}}}]}, - "2 possibly applicable schemas", - ), - ( - { - "allOf": [{"properties": {"foo": {"type": "object"}}}], - "anyOf": [{"properties": {"foo": {"type": "object"}}}], - }, - "2 possibly applicable schemas", - ), - ( - { - "anyOf": [{"properties": {"foo": {"type": "object"}}}], - "oneOf": [{"properties": {"foo": {"type": "object"}}}], - }, - "2 possibly applicable schemas", - ), - ( - { - "oneOf": [{"properties": {"foo": {"type": "object"}}}], - "allOf": [{"properties": {"foo": {"type": "object"}}}], - }, - "2 possibly applicable schemas", - ), + {"not": {"properties": {"foo": {"type": "object"}}}}, + {"properties": {"foo": {"type": "object"}}, "allOf": [{"properties": {"foo": {"type": "object"}}}]}, + {"properties": {"foo": {"type": "object"}}, "anyOf": [{"properties": {"foo": {"type": "object"}}}]}, + {"properties": {"foo": {"type": "object"}}, "oneOf": [{"properties": {"foo": {"type": "object"}}}]}, + { + "allOf": [{"properties": {"foo": {"type": "object"}}}], + "anyOf": [{"properties": {"foo": {"type": "object"}}}], + }, + { + "anyOf": [{"properties": {"foo": {"type": "object"}}}], + "oneOf": [{"properties": {"foo": {"type": "object"}}}], + }, + { + "oneOf": [{"properties": {"foo": {"type": "object"}}}], + "allOf": [{"properties": {"foo": {"type": "object"}}}], + }, ], ) -def test_node_property_error(schema, msg): +def test_node_property_error(schema): ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) - with pytest.raises(AsdfInfoResolutionError, match=msg): - ni.get_schema_for_property("foo") + assert ni.get_schema_for_property("foo") == {} @pytest.mark.parametrize( @@ -786,23 +766,12 @@ def test_node_property_error(schema, msg): ({"oneOf": [{"title": "foo"}]}, "foo"), ({"anyOf": [{"title": "foo"}]}, "foo"), ({"not": {"title": "foo"}}, None), + ({"allOf": [{"title": "foo"}, {"title": "bar"}]}, None), + ({"oneOf": [{"title": "foo"}, {"title": "bar"}]}, None), + ({"anyOf": [{"title": "foo"}, {"title": "bar"}]}, None), + ({"allOf": [{"title": "foo"}, {"title": "bar"}]}, None), ], ) def test_node_info(schema, expected): ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) assert ni.info == expected - - -@pytest.mark.parametrize( - "schema", - [ - {"allOf": [{"title": "foo"}, {"title": "bar"}]}, - {"oneOf": [{"title": "foo"}, {"title": "bar"}]}, - {"anyOf": [{"title": "foo"}, {"title": "bar"}]}, - {"allOf": [{"title": "foo"}, {"title": "bar"}]}, - ], -) -def test_node_info_failure(schema): - ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) - with pytest.raises(AsdfInfoResolutionError, match="2 possibly applicable schemas"): - ni.info diff --git a/asdf/exceptions.py b/asdf/exceptions.py index cfdef90b5..0851a8baa 100644 --- a/asdf/exceptions.py +++ b/asdf/exceptions.py @@ -84,9 +84,3 @@ class AsdfSerializationError(RepresenterError): that the object does not have a supporting asdf Converter and needs to be manually converted to a supported type. """ - - -class AsdfInfoResolutionError(ValueError): - """ - An attempt to lookup schema for an attribute failed. - """ diff --git a/changes/1875.bugfix.rst b/changes/1875.bugfix.rst index fcdc9d162..89960753a 100644 --- a/changes/1875.bugfix.rst +++ b/changes/1875.bugfix.rst @@ -1 +1 @@ -Improve ``schema_info`` handling of schemas with combiners (allOf, anyOf, etc). Raise AsdfInfoResolutionError for ambiguous info. +Improve ``schema_info`` handling of schemas with combiners (allOf, anyOf, etc). From e39d1e18b83e3c2b3099cac7bff7a4e85f34ebc6 Mon Sep 17 00:00:00 2001 From: Brett Date: Thu, 9 Jan 2025 11:04:22 -0500 Subject: [PATCH 8/8] clarify naming, add docstrings --- asdf/_node_info.py | 73 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 8 deletions(-) diff --git a/asdf/_node_info.py b/asdf/_node_info.py index 048e3ad1b..ccbb196c8 100644 --- a/asdf/_node_info.py +++ b/asdf/_node_info.py @@ -20,32 +20,70 @@ def _filter_tree(info, filters): return len(info.children) > 0 or all(f(info.node, info.identifier) for f in filters) -def _get_matching_schema_property(schema, key): +def _get_matching_schema_property(schema, property_name): + """ + Extract a property subschema for a given property_name. + This function does not descend into the schema (beyond + looking for a "properties" key) and does not support + schema combiners. + + Parameters + ---------- + schema : dict + A dictionary containing a JSONSCHEMA + property_name : str + The name of the property to extract + + Returns + ------- + dict or None + The property subschema at the provided name or + ``None`` if the property doesn't exist. + """ if "properties" in schema: props = schema["properties"] - if key in props: - return props[key] + if property_name in props: + return props[property_name] if "patternProperties" in props: patterns = props["patternProperties"] for regex in patterns: - if re.search(regex, key): + if re.search(regex, property_name): return patterns[regex] return None -def _get_subschema_for_property(schema, key): +def _get_subschema_for_property(schema, property_name): + """ + Extract a property subschema for a given property_name. + This function will attempt to consider schema combiners + and will return None on an ambiguous result. + + Parameters + ---------- + schema : dict + A dictionary containing a JSONSCHEMA + property_name : str + The name of the property to extract + + Returns + ------- + dict or None + The property subschema at the provided name or + ``None`` if the property doesn't exist or is + ambiguous (has more than one subschema or is nested in a not). + """ # This does NOT handle $ref the expectation is that the schema # is loaded with resolve_references=True applicable = [] # first check properties and patternProperties - subschema = _get_matching_schema_property(schema, key) + subschema = _get_matching_schema_property(schema, property_name) if subschema is not None: applicable.append(subschema) # next handle schema combiners if "not" in schema: - subschema = _get_subschema_for_property(schema["not"], key) + subschema = _get_subschema_for_property(schema["not"], property_name) if subschema is not None: # We can't resolve a valid subschema under a "not" since # we'd have to know how to invert a schema @@ -53,7 +91,7 @@ def _get_subschema_for_property(schema, key): for combiner in ("allOf", "oneOf", "anyOf"): for combined_schema in schema.get(combiner, []): - subschema = _get_subschema_for_property(combined_schema, key) + subschema = _get_subschema_for_property(combined_schema, property_name) if subschema is not None: applicable.append(subschema) @@ -64,6 +102,25 @@ def _get_subschema_for_property(schema, key): def _get_schema_key(schema, key): + """ + Extract a subschema at a given key. + This function will attempt to consider schema combiners + (allOf, oneOf, anyOf) and will return None on an + ambiguous result (where more than 1 match is found). + + Parameters + ---------- + schema : dict + A dictionary containing a JSONSCHEMA + key : str + The key under which the subschema is stored + + Returns + ------- + dict or None + The subschema at the provided key or + ``None`` if the key doesn't exist or is ambiguous. + """ applicable = [] if key in schema: applicable.append(schema[key])