feat: deserialisation of DeploymentNodes

Author: yt-ms

src/structurizr/model/container.py

@@ -114,7 +114,10 @@ def hydrate(
         software_system: "SoftwareSystem",
         model: "Model",
     ) -> "Container":
-        """Hydrate a new Container instance from its IO."""
+        """Hydrate a new Container instance from its IO.
+
+        This will also automatically register with the model.
+        """
         container = cls(
             **cls.hydrate_arguments(container_io),
             parent=software_system,

src/structurizr/model/deployment_element.py

@@ -17,6 +17,7 @@
 
 
 from abc import ABC
+from typing import Optional
 
 from .element import Element, ElementIO
 
@@ -36,7 +37,7 @@ class DeploymentElementIO(ElementIO, ABC):
 
     """
 
-    environment: str = DEFAULT_DEPLOYMENT_ENVIRONMENT
+    environment: Optional[str] = DEFAULT_DEPLOYMENT_ENVIRONMENT
 
 
 class DeploymentElement(Element, ABC):
@@ -54,3 +55,11 @@ def __init__(
         """Initialize a deployment element."""
         super().__init__(**kwargs)
         self.environment = environment
+
+    @classmethod
+    def hydrate_arguments(cls, deployment_element_io: DeploymentElementIO) -> dict:
+        """Build constructor arguments from IO."""
+        return {
+            **super().hydrate_arguments(deployment_element_io),
+            "environment": deployment_element_io.environment,
+        }

src/structurizr/model/deployment_node.py

@@ -111,16 +111,17 @@ def __iadd__(self, node: "DeploymentNode") -> "DeploymentNode":
         if node in self._children:
             return self
 
-        # if self.get_component_with_name(component.name):
-        #     raise ValueError(
-        #         f"Component with name {component.name} already exists in {self}."
-        #     )
+        if any(node.name == child.name for child in self.children):
+            raise ValueError(
+                f"A deployment node with the name '{node.name}' already "
+                f"exists in node '{self.name}'."
+            )
 
         if node.parent is None:
             node.parent = self
         elif node.parent is not self:
             raise ValueError(
-                f"DeploymentNode with name {node.name} already has parent "
+                f"DeploymentNode with name '{node.name}' already has parent "
                 f"{node.parent}. Cannot add to {self}."
             )
         self._children.add(node)
@@ -135,16 +136,18 @@ def hydrate(
         model: "Model",
         parent: "DeploymentNode" = None,
     ) -> "DeploymentNode":
-        """Hydrate a new DeploymentNode instance from its IO."""
+        """Hydrate a new DeploymentNode instance from its IO.
+
+        This will also automatically register with the model.
+        """
         node = cls(
             parent=parent,
-            name=deployment_node_io.name,
-            description=deployment_node_io.description,
+            **cls.hydrate_arguments(deployment_node_io),
         )
         model += node
 
         for child_io in deployment_node_io.children:
-            child_node = DeploymentNode.hydrate(child_io, model, node)
+            child_node = DeploymentNode.hydrate(child_io, model=model, parent=node)
             node += child_node
 
         return node

src/structurizr/model/model.py

@@ -21,12 +21,11 @@
 
 from pydantic import Field
 
-from structurizr.model.deployment_node import DeploymentNode
-
 from ..abstract_base import AbstractBase
 from ..base_model import BaseModel
 from .container import Container
 from .container_instance import ContainerInstance
+from .deployment_node import DeploymentNode, DeploymentNodeIO
 from .element import Element
 from .enterprise import Enterprise, EnterpriseIO
 from .implied_relationship_strategies import (
@@ -69,12 +68,11 @@ class ModelIO(BaseModel):
         alias="softwareSystems",
         description="The set of software systems belonging to this model.",
     )
-    # TODO:
-    # deployment_nodes: List[DeploymentNodeIO] = Field(
-    #     default=(),
-    #     alias="deploymentNodes",
-    #     description="The set of deployment nodes belonging to this model.",
-    # )
+    deployment_nodes: List[DeploymentNodeIO] = Field(
+        default=(),
+        alias="deploymentNodes",
+        description="The set of top-level deployment nodes belonging to this model.",
+    )
 
 
 class Model(AbstractBase):
@@ -156,9 +154,10 @@ def hydrate(cls, model_io: ModelIO) -> "Model":
         for software_system_io in model_io.software_systems:
             model += SoftwareSystem.hydrate(software_system_io, model=model)
 
-        # for deployment_node_io in model_io.deployment_nodes:
-        #     deployment_node = DeploymentNode.hydrate(deployment_node_io)
-        #     model.add_deployment_node(deployment_node=deployment_node)
+        for deployment_node_io in model_io.deployment_nodes:
+            DeploymentNode.hydrate(
+                deployment_node_io, model=model
+            )  # Auto-registers with the model
 
         for element in model.get_elements():
             for relationship in element.relationships:
@@ -215,6 +214,8 @@ def add_software_system(self, **kwargs) -> SoftwareSystem:
 
     def __iadd__(self, element: Element) -> "Model":
         """Add a newly constructed element to the model."""
+        if element in self.get_elements():
+            return self
         if isinstance(element, Person):
             if any(element.name == p.name for p in self.people):
                 raise ValueError(
@@ -229,12 +230,12 @@ def __iadd__(self, element: Element) -> "Model":
                 )
         elif isinstance(element, DeploymentNode):
             if any(
-                element.name == d.name and isinstance(d, DeploymentNode)
-                for d in self.get_elements()
+                element.name == d.name and element.environment == d.environment
+                for d in self.deployment_nodes
             ):
                 raise ValueError(
                     f"A deployment node with the name '{element.name}' already "
-                    f"exists in the model."
+                    f"exists in environment '{element.environment}' of the model."
                 )
         elif element.parent is None:
             raise ValueError(

tests/integration/test_model_deployment_node_deserialization.py

@@ -0,0 +1,44 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Ensure that relationships in Elements and in the Model are consistent.
+
+See https://github.com/Midnighter/structurizr-python/issues/31.
+"""
+
+from pathlib import Path
+
+import pytest
+
+from structurizr import Workspace
+
+
+DEFINITIONS = Path(__file__).parent / "data" / "workspace_definition"
+
+
+@pytest.mark.parametrize(
+    "filename",
+    ["BigBank.json"],
+)
+def test_model_deserialises_deployment_nodes(filename: str):
+    """Ensure deserialisaton of deployment nodes works."""
+    path = DEFINITIONS / filename
+    workspace = Workspace.load(path)
+    model = workspace.model
+
+    db_server = model.get_element("59")
+    assert db_server.name == "Docker Container - Database Server"
+    assert db_server is not None
+    assert db_server.model is model
+    assert db_server.parent.name == "Developer Laptop"
+    assert db_server.parent.parent is None

tests/unit/model/test_deployment_node.py

@@ -66,6 +66,33 @@ def test_deployment_node_adds_to_children(model_with_node):
     assert child in top_node.children
 
 
+def test_deployment_node_adding_same_child_twice_is_ok(model_with_node):
+    """Test adding the same child twice is just ignored."""
+    top_node = model_with_node.empty_node
+    child = top_node.add_deployment_node(name="child")
+    top_node += child
+    assert len(top_node.children) == 1
+
+
+def test_deployment_node_add_child_with_existing_parent(model_with_node: MockModel):
+    """Check that adding a node with an existing parent fails."""
+    top_node = model_with_node.empty_node
+    other_parent = DeploymentNode(name="OtherParent")
+    with pytest.raises(ValueError, match="DeploymentNode .* already has parent."):
+        top_node += DeploymentNode(name="child", parent=other_parent)
+
+
+def test_deployment_node_cant_have_two_children_with_the_same_name(model_with_node):
+    """Make sure you can't have two children with the same name in the same node."""
+    top_node = model_with_node.empty_node
+    top_node.add_deployment_node(name="child")
+    with pytest.raises(
+        ValueError,
+        match="A deployment node with the name 'child' already exists in node 'Empty'.",
+    ):
+        top_node.add_deployment_node(name="child")
+
+
 def test_deployment_node_serialization_of_recursive_nodes(model_with_node):
     """Check that nodes within nodes are handled with (de)serialisation."""
     top_node = model_with_node.empty_node

tests/unit/model/test_model.py

@@ -88,13 +88,18 @@ def test_model_add_top_level_deployment_node(empty_model: Model):
 
 
 def test_model_cant_add_two_deployment_nodes_with_same_name(empty_model: Model):
-    """Make sure that deployment nodes (at any level) can't share a name."""
-    node = empty_model.add_deployment_node(name="node1")
+    """Make sure that deployment nodes at the top level can't have the same name.
+
+    Unless they're in different environments.
+    """
+    empty_model.add_deployment_node(name="node1", environment="Live")
+    empty_model.add_deployment_node(name="node1", environment="Dev")  # Different env
     with pytest.raises(
         ValueError,
-        match="A deployment node with the name 'node1' already exists in the model.",
+        match="A deployment node with the name 'node1' already "
+        "exists in environment 'Live' of the model.",
     ):
-        node.add_deployment_node(name="node1")
+        empty_model.add_deployment_node(name="node1", environment="Live")
 
 
 def test_model_add_lower_level_deployment_node(empty_model: Model):