Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate validation on AsdfFile.tree assignment, AsdfFile.__init__ and AsdfFile.resolve_references #1691

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ The ASDF Standard is at v1.6.0
- Cleanup ``asdf.util`` including deprecating: ``human_list``
``resolve_name`` ``minversion`` and ``iter_subclasses`` [#1688]

- Deprecate validation on ``AsdfFile.tree`` assignment. Please
use ``AsdfFile.validate`` to validate the tree [#1691]

- Deprecate validation during ``AsdfFile.resolve_references``. Please
use ``AsdfFile.validate`` to validate the tree [#1691]

- Deprecate ``asdf.asdf`` and ``AsdfFile.resolve_and_inline`` [#1690]

- Deprecate automatic calling of ``AsdfFile.find_references`` during
Expand Down
35 changes: 30 additions & 5 deletions asdf/_asdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,17 @@ def __init__(
self._tree = tree.tree
self.find_references(_warning_msg=find_ref_warning_msg)
else:
self.tree = tree
self._tree = AsdfObject(tree)
try:
self.validate()
except ValidationError:
warnings.warn(
"Validation during AsdfFile.__init__ is deprecated. "
"Please use AsdfFile.validate to validate the tree",
AsdfDeprecationWarning,
)
raise

self.find_references(_warning_msg=find_ref_warning_msg)

self._comments = []
Expand Down Expand Up @@ -570,7 +580,13 @@ def tree(self):
def tree(self, tree):
asdf_object = AsdfObject(tree)
# Only perform custom validation if the tree is not empty
self._validate(asdf_object, custom=bool(tree))
try:
self._validate(asdf_object, custom=bool(tree))
except ValidationError:
warnings.warn(
"Validation on tree assignment is deprecated. Please use AsdfFile.validate", AsdfDeprecationWarning
)
raise
self._tree = asdf_object

def keys(self):
Expand Down Expand Up @@ -1232,9 +1248,18 @@ def resolve_references(self, **kwargs):
a ASDF file after this operation means it will have no
external references, and will be completely self-contained.
"""
# Set to the property self.tree so the resulting "complete"
# tree will be validated.
self.tree = reference.resolve_references(self._tree, self)
if len(kwargs):
warnings.warn("Passing kwargs to resolve_references is deprecated and does nothing", AsdfDeprecationWarning)
self._tree = reference.resolve_references(self._tree, self)
try:
self.validate()
except ValidationError:
warnings.warn(
"Validation during resolve_references is deprecated. "
"Please use AsdfFile.validate after resolve_references to validate the resolved tree",
AsdfDeprecationWarning,
)
raise

def resolve_and_inline(self):
"""
Expand Down
32 changes: 31 additions & 1 deletion asdf/_tests/test_deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import pytest

import asdf
from asdf.exceptions import AsdfDeprecationWarning
from asdf.exceptions import AsdfDeprecationWarning, ValidationError


def test_asdf_stream_deprecation():
Expand Down Expand Up @@ -84,3 +84,33 @@ def test_find_references_during_open_deprecation(tmp_path):
def test_asdf_util_is_primitive_deprecation():
with pytest.warns(AsdfDeprecationWarning, match="asdf.util.is_primitive is deprecated"):
asdf.util.is_primitive(1)


def test_AsdfFile_tree_assignment_validation_deprecation():
af = asdf.AsdfFile()
with pytest.warns(AsdfDeprecationWarning, match="Validation on tree assignment is deprecated"), pytest.raises(
ValidationError
):
af.tree = {"history": 42}


def test_AsdfFile_resolve_references_validation_deprecation():
af = asdf.AsdfFile()
af._tree["history"] = 42
with pytest.warns(
AsdfDeprecationWarning, match="Validation during resolve_references is deprecated"
), pytest.raises(ValidationError):
af.resolve_references()


def test_AsdfFile_resolve_references_kwargs_deprecation():
af = asdf.AsdfFile()
with pytest.warns(AsdfDeprecationWarning, match="Passing kwargs to resolve_references is deprecated"):
af.resolve_references(foo=42)


def test_AsdfFile_init_validation_deprecation():
with pytest.warns(AsdfDeprecationWarning, match="Validation during AsdfFile.__init__ is deprecated"), pytest.raises(
ValidationError
):
asdf.AsdfFile({"history": 42})
54 changes: 24 additions & 30 deletions asdf/_tests/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -844,26 +844,20 @@ def test_schema_resolved_via_entry_points():
def test_max_min_literals(num):
msg = r"Integer value .* is too large to safely represent as a literal in ASDF"

tree = {
"test_int": num,
}

af = asdf.AsdfFile()
af["test_int"] = num
with pytest.raises(ValidationError, match=msg):
asdf.AsdfFile(tree)

tree = {
"test_list": [num],
}
af.validate()

af = asdf.AsdfFile()
af["test_list"] = [num]
with pytest.raises(ValidationError, match=msg):
asdf.AsdfFile(tree)

tree = {
num: "test_key",
}
af.validate()

af = asdf.AsdfFile()
af[num] = "test_key"
with pytest.raises(ValidationError, match=msg):
asdf.AsdfFile(tree)
af.validate()


@pytest.mark.parametrize("num", [constants.MAX_NUMBER + 1, constants.MIN_NUMBER - 1])
Expand Down Expand Up @@ -930,8 +924,10 @@ def test_mapping_supported_key_types(keys, version):
)
def test_mapping_unsupported_key_types(keys, version):
for key in keys:
af = asdf.AsdfFile(version=version)
af[key] = "value"
with pytest.raises(ValidationError, match=r"Mapping key .* is not permitted"):
asdf.AsdfFile({key: "value"}, version=version)
af.validate()


def test_nested_array():
Expand Down Expand Up @@ -1022,10 +1018,10 @@ def test_custom_validation_bad(tmp_path):
ff.write_to(asdf_file)

# Creating file using custom schema should fail
with pytest.raises(ValidationError, match=r".* is a required property"), asdf.AsdfFile(
tree,
custom_schema=custom_schema_path,
):
af = asdf.AsdfFile(custom_schema=custom_schema_path)
af._tree = asdf.tags.core.AsdfObject(tree)
with pytest.raises(ValidationError, match=r".* is a required property"):
af.validate()
pass

# Opening file without custom schema should pass
Expand Down Expand Up @@ -1101,11 +1097,10 @@ def test_custom_validation_with_definitions_bad(tmp_path):
ff.write_to(asdf_file)

# Creating file with custom schema should fail
with pytest.raises(ValidationError, match=r".* is a required property"), asdf.AsdfFile(
tree,
custom_schema=custom_schema_path,
):
pass
af = asdf.AsdfFile(custom_schema=custom_schema_path)
af._tree = asdf.tags.core.AsdfObject(tree)
with pytest.raises(ValidationError, match=r".* is a required property"):
af.validate()

# Opening file without custom schema should pass
with asdf.open(asdf_file):
Expand Down Expand Up @@ -1145,11 +1140,10 @@ def test_custom_validation_with_external_ref_bad(tmp_path):
ff.write_to(asdf_file)

# Creating file with custom schema should fail
with pytest.raises(ValidationError, match=r"False is not valid under any of the given schemas"), asdf.AsdfFile(
tree,
custom_schema=custom_schema_path,
):
pass
af = asdf.AsdfFile(custom_schema=custom_schema_path)
af["foo"] = False
with pytest.raises(ValidationError, match=r"False is not valid under any of the given schemas"):
af.validate()

# Opening file without custom schema should pass
with asdf.open(asdf_file):
Expand Down
13 changes: 13 additions & 0 deletions docs/asdf/deprecations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@ Automatic calling of ``AsdfFile.find_references`` during calls to
``AsdfFile.__init__`` and ``asdf.open``. Call ``AsdfFile.find_references`` to
find references.

Several deprecations were added to ``AsdfFile`` methods that validate the
tree. In a future version of asdf these methods will not perform any tree
validation (please call ``AsdfFile.validate`` to validate the tree).
As this behavior is difficult to deprecate (without triggering warnings
for every call of the method) an ``AsdfDeprecationWarning`` will only
be issued on a failed validation during the following methods:

* ``AsdfFile.tree`` assignment
* ``AsdfFile.resolve_references``
* ``AsdfFile.__init__`` (when the ``tree`` argument is provided)

Providing ``kwargs`` to ``AsdfFile.resolve_references`` does nothing and is deprecated.

Version 3.0
===========

Expand Down
12 changes: 3 additions & 9 deletions docs/asdf/features.rst
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,9 @@ Schema validation

Schema validation is used to determine whether an ASDF file is well formed. All
ASDF files must conform to the schemas defined by the :ref:`ASDF Standard
<asdf-standard:asdf-standard>`. Schema validation occurs
when reading ASDF files (using `asdf.open`), writing them out
(using `AsdfFile.write_to` or `AsdfFile.update`), when assigning to the
root of the ASDF tree (assigning to `AsdfFile.tree`) and when constructing
a new `AsdfFile` object from a tree/dictionary (as in ``AsdfFile(tree)``).

.. note::
Schema validation does not occur when a leaf/branch of the tree is
assigned but can be manually triggered by using `AsdfFile.validate`.
<asdf-standard:asdf-standard>`. Schema validation can be run using `AsdfFile.validate`
and occurs when reading ASDF files (using `asdf.open`) and writing them out
(using `AsdfFile.write_to` or `AsdfFile.update`).

Schema validation also plays a role when using custom extensions (see
:ref:`using_extensions` and :ref:`extending_extensions`). Extensions must provide schemas
Expand Down
Loading