Merge pull request #1691 from braingram/deprecate_tree_assignment_val…

…idation

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

CHANGES.rst

@@ -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

asdf/_asdf.py

@@ -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 = []
@@ -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):
@@ -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):
         """

asdf/_tests/test_deprecated.py

@@ -5,7 +5,7 @@
 import pytest
 
 import asdf
-from asdf.exceptions import AsdfDeprecationWarning
+from asdf.exceptions import AsdfDeprecationWarning, ValidationError
 
 
 def test_asdf_stream_deprecation():
@@ -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})

asdf/_tests/test_schema.py

@@ -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])
@@ -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():
@@ -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
@@ -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):
@@ -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):

docs/asdf/deprecations.rst

@@ -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
 ===========
 

docs/asdf/features.rst

@@ -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