Fix schema id URI should not be the same as tag URI (#28)

.pre-commit-config.yaml

@@ -1,5 +1,6 @@
 # See https://pre-commit.com for more information
 # See https://pre-commit.com/hooks.html for more hooks
+default_install_hook_types: [pre-commit, pre-push, commit-msg]
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.6.0
@@ -21,6 +22,12 @@ repos:
     hooks:
       - id: black
 
+  - repo: https://github.com/compilerla/conventional-pre-commit
+    rev: v3.4.0
+    hooks:
+      - id: conventional-pre-commit
+        stages: [commit-msg]
+
 ci:
     # autofix_commit_msg: |
     #     [pre-commit.ci] auto fixes from pre-commit.com hooks

asdf_pydantic/examples/extensions.py

@@ -4,9 +4,9 @@
 
 from asdf_pydantic.converter import AsdfPydanticConverter
 from asdf_pydantic.examples.shapes import AsdfRectangle
-from asdf_pydantic.examples.tree import AsdfNode
+from asdf_pydantic.examples.tree import AsdfTreeNode
 
-AsdfPydanticConverter.add_models(AsdfRectangle, AsdfNode)
+AsdfPydanticConverter.add_models(AsdfRectangle, AsdfTreeNode)
 
 
 class ExampleExtension(Extension):

asdf_pydantic/examples/tree.py

@@ -11,7 +11,7 @@ class Node(BaseModel):
     child: Optional[Node] = None
 
 
-class AsdfNode(Node, AsdfPydanticModel):
-    _tag = "asdf://asdf-pydantic/examples/tags/node-1.0.0"
+class AsdfTreeNode(Node, AsdfPydanticModel):
+    _tag = "asdf://asdf-pydantic/examples/tags/tree-node-1.0.0"
 
-    child: Optional[Union[AsdfNode, Node]] = None
+    child: Optional[Union[AsdfTreeNode, Node]] = None

asdf_pydantic/model.py

@@ -72,7 +72,7 @@ def model_asdf_schema(
         )
         json_schema = schema_generator_instance.generate(cls.__pydantic_core_schema__)
 
-        return f"%YAML 1.1\n---\n{yaml.safe_dump(json_schema)}"
+        return f"%YAML 1.1\n---\n{yaml.safe_dump(json_schema, sort_keys=False)}"
 
     @classmethod
     @deprecated(

asdf_pydantic/schema.py

@@ -3,6 +3,17 @@
 from pydantic.json_schema import GenerateJsonSchema
 
 DEFAULT_ASDF_SCHEMA_REF_TEMPLATE = "#/definitions/{model}"
+DESIRED_ASDF_SCHEMA_KEY_ORDER = (
+    "$schema",
+    "id",
+    "title",
+    "type",
+    "properties",
+    "allOf",
+    "anyOf",
+    "required",
+    "$defs",
+)
 
 
 class GenerateAsdfSchema(GenerateJsonSchema):
@@ -32,8 +43,17 @@ def generate(self, schema, mode="validation"):
 
         if self.tag_uri:
             json_schema["$schema"] = self.schema_dialect
-            json_schema["id"] = self.tag_uri
-            json_schema["tag"] = f"tag:{self.tag_uri.split('://', maxsplit=2)[-1]}"
+            json_schema["id"] = f"{self.tag_uri}/schema"
+
+        # Order keys
+        json_schema = {
+            **{
+                key: json_schema[key]
+                for key in DESIRED_ASDF_SCHEMA_KEY_ORDER
+                if key in json_schema
+            },
+            **json_schema,  # Rest of the keys not in order list
+        }
 
         # TODO: Convert jsonschema 2020-12 to ASDF schema
         return json_schema

tests/convert_to_asdf_yaml_tree_test.py

@@ -1,25 +1,25 @@
 from __future__ import annotations
 
-from asdf_pydantic.examples.tree import AsdfNode, Node
+from asdf_pydantic.examples.tree import AsdfTreeNode, Node
 
 
 def test_sanity():
-    AsdfNode().asdf_yaml_tree() == {"child": None}
+    AsdfTreeNode().asdf_yaml_tree() == {"child": None}
 
 
 def test_should_not_convert_given_child_is_AsdfNode():
-    AsdfNode(child=AsdfNode()).asdf_yaml_tree() == {"child": AsdfNode()}
+    AsdfTreeNode(child=AsdfTreeNode()).asdf_yaml_tree() == {"child": AsdfTreeNode()}
 
 
 def test_should_convert_given_child_is_Node():
-    AsdfNode(child=Node()).asdf_yaml_tree() == {"child": {"child": None}}
+    AsdfTreeNode(child=Node()).asdf_yaml_tree() == {"child": {"child": None}}
 
 
 def test_given_mix_child_is_mix_of_AsdfNode_and_Node():
-    assert AsdfNode(child=AsdfNode(child=Node())).asdf_yaml_tree() == {
-        "child": AsdfNode(child=Node())
+    assert AsdfTreeNode(child=AsdfTreeNode(child=Node())).asdf_yaml_tree() == {
+        "child": AsdfTreeNode(child=Node())
     }
 
-    assert AsdfNode(child=Node(child=AsdfNode())).asdf_yaml_tree() == {
+    assert AsdfTreeNode(child=Node(child=AsdfTreeNode())).asdf_yaml_tree() == {
         "child": {"child": {"child": None}}
     }

tests/examples/test_node.py

@@ -0,0 +1,120 @@
+"""Tests example asdf-pydantic model for nodes of a graph/tree."""
+
+from __future__ import annotations
+
+import textwrap
+from unittest.mock import MagicMock, patch
+
+import asdf
+import asdf.exceptions
+import asdf.schema
+import pytest
+import yaml
+from asdf.extension import Extension
+
+from asdf_pydantic import AsdfPydanticConverter, AsdfPydanticModel
+
+
+class AsdfNode(AsdfPydanticModel):
+    """Model for a node in a graph/tree.
+
+    Nodes introduce self-referential types. Notice the type of the `child`
+    attribute it the node type itself. The ASDF schema for this model will require
+    self-referencing syntax. We assumes this form is valid for ASDF schemas:
+
+        ```yaml
+        ---
+        type: object
+        anyOf:
+        - $ref: "#/definitions/AsdfNode"
+        definitions:
+            AsdfNode:
+                type: object
+                properties:
+                name:
+                    type: string
+                child:
+                    anyOf:
+                    - $ref: "#/definitions/AsdfNode"
+                    - type: null
+        ```
+
+    The self-reference happens in ``definitions[AsdfNode].properties.child.anyOf[0]``
+    where the `$ref` is a special JSONSchema syntax that referes to the value,
+    `#/definitions/AsdfNode`. This value is a json path where `#` denotes "this
+    schema".
+    """
+
+    _tag = "asdf://asdf-pydantic/examples/tags/node-2.0.0"
+
+    name: str
+    child: AsdfNode | None = None
+
+
+@pytest.fixture()
+def asdf_extension():
+    """Registers an ASDF extension containing models for this test."""
+    AsdfPydanticConverter.add_models(AsdfNode)
+
+    class TestExtension(Extension):
+        extension_uri = "asdf://asdf-pydantic/examples/extensions/test-1.0.0"
+
+        converters = [AsdfPydanticConverter()]  # type: ignore
+        tags = [AsdfNode.get_tag_definition()]  # type: ignore
+
+    with asdf.config_context() as asdf_config:
+        asdf_config.add_resource_mapping(
+            {
+                yaml.safe_load(AsdfNode.model_asdf_schema())[
+                    "id"
+                ]: AsdfNode.model_asdf_schema()
+            }
+        )
+        asdf_config.add_extension(TestExtension())
+        yield asdf_config
+
+
+@pytest.mark.usefixtures("asdf_extension")
+def test_can_write_valid_asdf_file(tmp_path):
+    """Tests using the model to write an ASDF file validates its own schema."""
+    af = asdf.AsdfFile()
+    af["root"] = AsdfNode(name="foo", child=None)
+    af.validate()
+    af.write_to(tmp_path / "test.asdf")
+
+    with asdf.open(tmp_path / "test.asdf") as af:
+        assert af.tree
+
+
+@pytest.mark.usefixtures("asdf_extension")
+@patch.object(AsdfNode, "model_validate", MagicMock())  # Ignore pydantic validation
+def test_errors_reading_invalid_asdf_file(tmp_path):
+    """Tests ASDF validation fails when ASDF file does not match the schema."""
+    content = """\
+        #ASDF 1.0.0
+        #ASDF_STANDARD 1.5.0
+        %YAML 1.1
+        %TAG ! tag:stsci.edu:asdf/
+        --- !core/asdf-1.1.0
+        asdf_library: !core/software-1.0.0 {author: The ASDF Developers, homepage: 'http://github.com/asdf-format/asdf',
+            name: asdf, version: 3.4.0}
+        history:
+            extensions:
+            - !core/extension_metadata-1.0.0
+                extension_class: asdf.extension._manifest.ManifestExtension
+                extension_uri: asdf://asdf-format.org/core/extensions/core-1.5.0
+                manifest_software: !core/software-1.0.0 {name: asdf_standard, version: 1.1.1}
+                software: !core/software-1.0.0 {name: asdf, version: 3.4.0}
+            - !core/extension_metadata-1.0.0 {extension_class: tests.examples.test_node.setup_module.<locals>.TestExtension,
+                extension_uri: 'asdf://asdf-pydantic/examples/extensions/test-1.0.0'}
+        root: !<asdf://asdf-pydantic/examples/tags/node-1.0.0>
+            name: foo
+            child: 1
+        ...
+    """
+    with open(tmp_path / "test.asdf", "w") as f:
+        f.write(textwrap.dedent(content))
+
+    with pytest.raises(asdf.exceptions.ValidationError):
+        with asdf.open(tmp_path / "test.asdf") as af:
+            assert af.tree

tests/examples/test_rectangle.py

@@ -1,58 +1,90 @@
+import textwrap
+from unittest.mock import MagicMock, patch
+
 import asdf
 import pytest
+import yaml
 from asdf.extension import Extension
-from asdf.schema import check_schema, load_schema
-from yaml.scanner import ScannerError
 
-from asdf_pydantic import AsdfPydanticConverter
-from asdf_pydantic.examples.shapes import AsdfRectangle
+from asdf_pydantic import AsdfPydanticConverter, AsdfPydanticModel
+
+
+class AsdfRectangle(AsdfPydanticModel):
+    _tag = "asdf://asdf-pydantic/examples/tags/rectangle-1.0.0"
+    width: float
+    height: float
 
 
-def setup_module():
+@pytest.fixture()
+def asdf_extension():
+    """Registers an ASDF extension containing models for this test."""
     AsdfPydanticConverter.add_models(AsdfRectangle)
 
     class TestExtension(Extension):
-        extension_uri = "asdf://asdf-pydantic/examples/extensions/test-1.0.0"  # type: ignore
+        extension_uri = "asdf://asdf-pydantic/examples/extensions/test-1.0.0"
 
-        tags = [*AsdfPydanticConverter().tags]  # type: ignore
         converters = [AsdfPydanticConverter()]  # type: ignore
+        tags = [AsdfRectangle.get_tag_definition()]  # type: ignore
+
+    with asdf.config_context() as asdf_config:
+        asdf_config.add_resource_mapping(
+            {
+                yaml.safe_load(AsdfRectangle.model_asdf_schema())[
+                    "id"
+                ]: AsdfRectangle.model_asdf_schema()
+            }
+        )
+        asdf_config.add_extension(TestExtension())
+        yield asdf_config
+
+
+@pytest.mark.usefixtures("asdf_extension")
+def test_can_write_valid_asdf_file(tmp_path):
+    """Tests using the model to write an ASDF file validates its own schema."""
+    af = asdf.AsdfFile()
+    af["root"] = AsdfRectangle(width=42, height=10)
+    af.validate()
+    af.write_to(tmp_path / "test.asdf")
+
+    with asdf.open(tmp_path / "test.asdf") as af:
+        assert af.tree
+
+
+@pytest.mark.usefixtures("asdf_extension")
+@patch.object(
+    AsdfRectangle, "model_validate", MagicMock()
+)  # Ignore pydantic validation
+def test_errors_reading_invalid_asdf_file(tmp_path):
+    """Tests validation fails when ASDF file does not match the schema."""
+    content = """\
+        #ASDF 1.0.0
+        #ASDF_STANDARD 1.5.0
+        %YAML 1.1
+        %TAG ! tag:stsci.edu:asdf/
+        --- !core/asdf-1.1.0
+        asdf_library: !core/software-1.0.0 {
+            author: The ASDF Developers,
+            homepage: 'http://github.com/asdf-format/asdf',
+            name: asdf,
+            version: 2.14.3}
+        history:
+        extensions:
+        - !core/extension_metadata-1.0.0
+            extension_class: asdf.extension.BuiltinExtension
+            software: !core/software-1.0.0 {
+                name: asdf,
+                version: 2.14.3}
+        - !core/extension_metadata-1.0.0 {
+            extension_class: mypackage.shapes.ShapesExtension,
+            extension_uri: 'asdf://asdf-pydantic/shapes/extensions/shapes-1.0.0'}
+        rect: !<asdf://asdf-pydantic/shapes/tags/rectangle-1.0.0>
+            height: "10"
+            width: "42"
+        ...
+    """
+    with open(tmp_path / "test.asdf", "w") as f:
+        f.write(textwrap.dedent(content))
 
-    # HACK: The schema URI should be referenced from `AsdfRectangle._schema`.
-    # Then there should be a way to automatically add the schema to ASDF
-    # resources perhaps during AsdfPydanticConverter.add_models(). Further
-    # abstracting can be done later, perhaps defining a
-    # AsdfPydanticExtension.
-    asdf.get_config().add_resource_mapping(
-        {
-            "asdf://asdf-pydantic/shapes/schemas/rectangle-1.0.0": (
-                AsdfRectangle.model_asdf_schema().encode("utf-8")
-            )
-        }
-    )
-    asdf.get_config().add_extension(TestExtension())
-
-
-def test_schema():
-    try:
-        schema = load_schema("asdf://asdf-pydantic/shapes/schemas/rectangle-1.0.0")
-        check_schema(schema)
-    except ScannerError as e:
-        pytest.fail(f"{e}\n{AsdfRectangle.model_asdf_schema()}")
-
-    assert schema["$schema"] == "http://stsci.edu/schemas/asdf/asdf-schema-1.0.0"
-    assert schema["title"] == "AsdfRectangle"
-    assert schema["id"] == "asdf://asdf-pydantic/examples/tags/rectangle-1.0.0"
-    assert schema["tag"] == "tag:asdf-pydantic/examples/tags/rectangle-1.0.0"
-    assert schema["type"] == "object"
-    assert schema["properties"] == {
-        "width": {
-            "type": "number",
-            "title": "Width",
-        },
-        "height": {
-            "type": "number",
-            "title": "Height",
-        },
-    }
-
-    assert schema["required"] == ["width", "height"]
+    with pytest.raises(asdf.exceptions.ValidationError):
+        with asdf.open(tmp_path / "test.asdf") as af:
+            assert af.tree