feat(api): Refactors node and edge models

Author: tcjennings

src/lsst/cmservice/db/campaigns_v2.py

@@ -1,8 +1,10 @@
+"""ORM Models for v2 tables and objects."""
+
 from datetime import datetime
 from typing import Any
 from uuid import NAMESPACE_DNS, UUID, uuid5
 
-from pydantic import AliasChoices, ValidationInfo, model_validator
+from pydantic import AliasChoices
 from sqlalchemy.dialects import postgresql
 from sqlalchemy.ext.mutable import MutableDict, MutableList
 from sqlalchemy.types import PickleType
@@ -43,15 +45,6 @@ def jsonb_column(name: str, aliases: list[str] | None = None) -> Any:
     )
 
 
-# NOTES
-# - model validation is not triggered when table=True
-# - Every object model needs to have three flavors:
-#   1. the declarative model of the object's database table
-#   2. the model of the manifest when creating a new object
-#   3. the model of the manifest when updating an object
-#   4. a response model for APIs related to the object
-
-
 class BaseSQLModel(SQLModel):
     __table_args__ = {"schema": config.db.table_schema}
     metadata = metadata
@@ -72,26 +65,7 @@ class CampaignBase(BaseSQLModel):
     configuration: dict = jsonb_column("configuration", aliases=["configuration", "data", "spec"])
 
 
-class CampaignModel(CampaignBase):
-    """model used for resource creation."""
-
-    @model_validator(mode="before")
-    @classmethod
-    def custom_model_validator(cls, data: Any, info: ValidationInfo) -> Any:
-        """Validates the model based on different types of raw inputs,
-        where some default non-optional fields can be auto-populated.
-        """
-        if isinstance(data, dict):
-            if "name" not in data:
-                raise ValueError("'name' must be specified.")
-            if "namespace" not in data:
-                data["namespace"] = _default_campaign_namespace
-            if "id" not in data:
-                data["id"] = uuid5(namespace=data["namespace"], name=data["name"])
-        return data
-
-
-class Campaign(CampaignModel, table=True):
+class Campaign(CampaignBase, table=True):
     """Model used for database operations involving campaigns_v2 table rows"""
 
     __tablename__: str = "campaigns_v2"  # type: ignore[misc]
@@ -127,25 +101,7 @@ class NodeBase(BaseSQLModel):
     configuration: dict = jsonb_column("configuration", aliases=["configuration", "data", "spec"])
 
 
-class NodeModel(NodeBase):
-    """model validating class for Nodes"""
-
-    @model_validator(mode="before")
-    @classmethod
-    def custom_model_validator(cls, data: Any, info: ValidationInfo) -> Any:
-        if isinstance(data, dict):
-            if "version" not in data:
-                data["version"] = 1
-            if "name" not in data:
-                raise ValueError("'name' must be specified.")
-            if "namespace" not in data:
-                data["namespace"] = _default_campaign_namespace
-            if "id" not in data:
-                data["id"] = uuid5(namespace=data["namespace"], name=f"""{data["name"]}.{data["version"]}""")
-        return data
-
-
-class Node(NodeModel, table=True):
+class Node(NodeBase, table=True):
     __tablename__: str = "nodes_v2"  # type: ignore[misc]
 
     machine: UUID | None = Field(foreign_key="machines_v2.id", default=None, ondelete="CASCADE")
@@ -163,28 +119,12 @@ class EdgeBase(BaseSQLModel):
     configuration: dict = jsonb_column("configuration", aliases=["configuration", "data", "spec"])
 
 
-class EdgeModel(EdgeBase):
-    """model validating class for Edges"""
-
-    @model_validator(mode="before")
-    @classmethod
-    def custom_model_validator(cls, data: Any, info: ValidationInfo) -> Any:
-        if isinstance(data, dict):
-            if "name" not in data:
-                raise ValueError("'name' must be specified.")
-            if "namespace" not in data:
-                raise ValueError("Edges may only exist in a 'namespace'.")
-            if "id" not in data:
-                data["id"] = uuid5(namespace=data["namespace"], name=data["name"])
-        return data
-
-
-class EdgeResponseModel(EdgeModel):
+class EdgeResponseModel(EdgeBase):
     source: Any
     target: Any
 
 
-class Edge(EdgeModel, table=True):
+class Edge(EdgeBase, table=True):
     __tablename__: str = "edges_v2"  # type: ignore[misc]
 
 
@@ -216,24 +156,6 @@ class ManifestBase(BaseSQLModel):
     spec: dict = jsonb_column("spec", aliases=["spec", "configuration", "data"])
 
 
-class ManifestModel(ManifestBase):
-    """model validating class for Manifests"""
-
-    @model_validator(mode="before")
-    @classmethod
-    def custom_model_validator(cls, data: Any, info: ValidationInfo) -> Any:
-        if isinstance(data, dict):
-            if "version" not in data:
-                data["version"] = 1
-            if "name" not in data:
-                raise ValueError("'name' must be specified.")
-            if "namespace" not in data:
-                data["namespace"] = _default_campaign_namespace
-            if "id" not in data:
-                data["id"] = uuid5(namespace=data["namespace"], name=f"""{data["name"]}.{data["version"]}""")
-        return data
-
-
 class Manifest(ManifestBase, table=True):
     __tablename__: str = "manifests_v2"  # type: ignore[misc]
 

src/lsst/cmservice/db/manifests_v2.py

@@ -5,6 +5,7 @@
 """
 
 from typing import Self
+from uuid import uuid4
 
 from pydantic import AliasChoices, BaseModel, ConfigDict, Field, ValidationInfo, model_validator
 
@@ -99,3 +100,43 @@ def custom_model_validator(self, info: ValidationInfo) -> Self:
             raise ValueError("Campaigns may only be created from a <campaign> manifest")
 
         return self
+
+
+class EdgeMetadata(ManifestMetadata):
+    """Metadata model for an Edge Manifest.
+
+    A default random alphanumeric 8-byte name is generated if no name provided.
+    """
+
+    name: str = Field(default_factory=lambda: uuid4().hex[:8])
+
+
+class EdgeSpec(ManifestSpec):
+    """Spec model for an Edge Manifest."""
+
+    source: str
+    target: str
+
+
+class EdgeManifest(Manifest[EdgeMetadata, EdgeSpec]):
+    """validating model for Edges"""
+
+    @model_validator(mode="after")
+    def custom_model_validator(self, info: ValidationInfo) -> Self:
+        """Validate an Edge Manifest after a model has been created."""
+        if self.kind is not ManifestKind.edge:
+            raise ValueError("Edges may only be created from an <edge> manifest")
+
+        return self
+
+
+class NodeManifest(Manifest[VersionedMetadata, ManifestSpec]):
+    """validating model for Nodes"""
+
+    @model_validator(mode="after")
+    def custom_model_validator(self, info: ValidationInfo) -> Self:
+        """Validate a Node Manifest after a model has been created."""
+        if self.kind is not ManifestKind.node:
+            raise ValueError("Nodes may only be created from an <node> manifest")
+
+        return self

src/lsst/cmservice/routers/v2/edges.py

@@ -6,16 +6,15 @@
 
 from collections.abc import Sequence
 from typing import Annotated
-from uuid import UUID, uuid4, uuid5
+from uuid import UUID, uuid5
 
 from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response
 from sqlmodel import select
 from sqlmodel.ext.asyncio.session import AsyncSession
 
-from ...common.enums import ManifestKind
 from ...common.logging import LOGGER
 from ...db.campaigns_v2 import Campaign, Edge
-from ...db.manifests_v2 import ManifestWrapper
+from ...db.manifests_v2 import EdgeManifest
 from ...db.session import db_session_dependency
 
 # TODO should probably bind a logger to the fastapi app or something
@@ -110,7 +109,7 @@ async def read_edge_resource(
 async def create_edge_resource(
     request: Request,
     response: Response,
-    manifest: ManifestWrapper,
+    manifest: EdgeManifest,
     session: Annotated[AsyncSession, Depends(db_session_dependency)],
 ) -> Edge:
     """Creates a new edge from a Manifest.
@@ -130,36 +129,23 @@ async def create_edge_resource(
         target: {ndoe name or id}
     ```
     """
-    if manifest.kind is not ManifestKind.edge:
-        raise HTTPException(status_code=422, detail="Edges may only be created from a 'edge' manifest")
-
-    edge_name: str
-    if (edge_name := manifest.metadata_.pop("name", None)) is None:
-        edge_name = uuid4().hex[:8]
-
-    # Validate manifest spec as an edge
-    # TODO match edge with jsonschema and validate
-    elif (source_node := manifest.spec.pop("source", None)) is None:
-        raise HTTPException(status_code=400, detail="Edges must have a source node'")
-    elif (target_node := manifest.spec.pop("target", None)) is None:
-        raise HTTPException(status_code=400, detail="Edges must have a target node'")
+    edge_name = manifest.metadata_.name
+    source_node = manifest.spec.source
+    target_node = manifest.spec.target
 
     # A edge must exist in the namespace of an existing campaign
-    edge_namespace: str = manifest.metadata_.pop("namespace", None)
-    if edge_namespace is None:
-        raise HTTPException(status_code=422, detail="Edges must be created in a campaign namespace.")
-    else:
-        try:
-            edge_namespace_uuid: UUID | None = UUID(edge_namespace)
-        except ValueError:
-            # get the campaign ID by its name to use as a namespace
-            edge_namespace_uuid = (
-                await session.exec(select(Campaign.id).where(Campaign.name == edge_namespace))
-            ).one_or_none()
-
-        # it is an error if the provided namespace (campaign) does not exist
-        if edge_namespace_uuid is None:
-            raise HTTPException(status_code=422, detail="Requested campaign namespace does not exist.")
+    edge_namespace: str = manifest.metadata_.namespace
+    try:
+        edge_namespace_uuid: UUID | None = UUID(edge_namespace)
+    except ValueError:
+        # get the campaign ID by its name to use as a namespace
+        edge_namespace_uuid = (
+            await session.exec(select(Campaign.id).where(Campaign.name == edge_namespace))
+        ).one_or_none()
+
+    # it is an error if the provided namespace (campaign) does not exist
+    if edge_namespace_uuid is None:
+        raise HTTPException(status_code=422, detail="Requested campaign namespace does not exist.")
 
     # an edge may specify the source and target nodes by name and version,
     # which means the UUID of these nodes is deterministic, or we could go to
@@ -180,7 +166,7 @@ async def create_edge_resource(
         namespace=edge_namespace_uuid,
         source=uuid5(edge_namespace_uuid, source_node),
         target=uuid5(edge_namespace_uuid, target_node),
-        configuration=manifest.spec,
+        configuration=manifest.spec.model_dump(),
     )
 
     # The merge operation is effectively an upsert should an edge matching the

src/lsst/cmservice/routers/v2/nodes.py

@@ -12,11 +12,10 @@
 from sqlmodel import col, select
 from sqlmodel.ext.asyncio.session import AsyncSession
 
-from ...common.enums import ManifestKind
 from ...common.jsonpatch import JSONPatch, JSONPatchError, apply_json_patch
 from ...common.logging import LOGGER
 from ...db.campaigns_v2 import Campaign, Node
-from ...db.manifests_v2 import ManifestWrapper
+from ...db.manifests_v2 import NodeManifest
 from ...db.session import db_session_dependency
 
 # TODO should probably bind a logger to the fastapi app or something
@@ -103,23 +102,11 @@ async def read_node_resource(
 async def create_node_resource(
     request: Request,
     response: Response,
-    manifest: ManifestWrapper,
+    manifest: NodeManifest,
     session: Annotated[AsyncSession, Depends(db_session_dependency)],
 ) -> Node:
-    # Validate the input by checking the "kind" of manifest is a node
-    if manifest.kind is not ManifestKind.node:
-        raise HTTPException(status_code=422, detail="Nodes may only be created from a 'node' manifest")
-    # and that the manifest includes any required fields, though this could
-    # just as well be a try/except ValueError around `_.model_validate()`
-    elif (node_name := manifest.metadata_.pop("name", None)) is None:
-        raise HTTPException(status_code=400, detail="Nodes must have a name set in '.metadata.name'")
-
-    # A node must exist in the namespace of an existing campaign
-    node_namespace: str = manifest.metadata_.pop("namespace", None)
-    if node_namespace is None:
-        raise HTTPException(
-            status_code=400, detail="Nodes must have a campaign namespace set in '.metadata.namespace'"
-        )
+    node_name = manifest.metadata_.name
+    node_namespace = manifest.metadata_.namespace
 
     try:
         node_namespace_uuid: UUID | None = UUID(node_namespace)
@@ -136,7 +123,7 @@ async def create_node_resource(
 
     # A node must be a new version if name+namespace already exists
     # - check db for node as name+namespace, get current version and increment
-    node_version = int(manifest.metadata_.pop("version", 0))
+    node_version = int(manifest.metadata_.version)
 
     s = (
         select(Node)
@@ -154,7 +141,7 @@ async def create_node_resource(
         name=node_name,
         namespace=node_namespace_uuid,
         version=node_version,
-        configuration=manifest.spec,
+        configuration=manifest.spec.model_dump(),
     )
 
     # Put the node in the database

tests/v2/test_edge_routes.py

@@ -48,7 +48,7 @@ async def test_edge_negative(aclient: AsyncClient) -> None:
         "cm-service/v2/edges",
         json={
             "kind": "edge",
-            "metadata": {"name": uuid4().hex[8:]},
+            "metadata": {"name": uuid4().hex[8:], "namespace": "test_campaign"},
             "spec": {
                 "source": "START",
             },
@@ -60,7 +60,7 @@ async def test_edge_negative(aclient: AsyncClient) -> None:
         "cm-service/v2/edges",
         json={
             "kind": "edge",
-            "metadata": {"name": uuid4().hex[8:]},
+            "metadata": {"name": uuid4().hex[8:], "namespace": "test_campaign"},
             "spec": {
                 "target": "END",
             },
@@ -78,7 +78,6 @@ async def test_edge_negative(aclient: AsyncClient) -> None:
         },
     )
     assert x.is_client_error
-    assert "campaign namespace" in x.text
 
     # negative: fail to delete an edge using a non-uuid string (like a name)
     x = await aclient.delete(
@@ -97,6 +96,7 @@ async def test_edge_lifecycle(aclient: AsyncClient) -> None:
         json={
             "kind": "campaign",
             "metadata": {"name": campaign_name},
+            "spec": {},
         },
     )
     assert x.is_success

tests/v2/test_node_routes.py

@@ -124,5 +124,7 @@ async def test_node_lifecycle(aclient: AsyncClient) -> None:
     assert x.is_success
     node = x.json()
 
-    # Edit a Node using RFCxxxx json-merge
-    ...
+    # test that the updates have been made and the version has been bumped
+    assert node["configuration"]["split"]["maxInFlight"] == 4
+    assert node["configuration"]["query"][-1]["dimension"] == "fact"
+    assert node["version"] == 2