feat: ensure view keys can't be duplicated or null

Author: yt-ms

src/structurizr/view/filtered_view.py

@@ -26,6 +26,8 @@
 
 
 class FilterMode(Enum):
+    """indicates whether elements/relationships are being included or excluded."""
+
     Include = "Include"
     Exclude = "Exclude"
 
@@ -51,8 +53,8 @@ class FilteredView(AbstractView):
     """
     Represent the filtered view from the C4 model.
 
-    A FilteredView is a view that is based on another view, but adding or removing
-    specific elements specified by tags.
+    A FilteredView is a view that is based on another view, but including or excluding
+    elements and relationships from the base view according to their tags.
 
     Attributes:
         view: the view which this FilteredView is based on

src/structurizr/view/view_set.py

@@ -200,12 +200,11 @@ def create_system_landscape_view(
             **kwargs: Provide keyword arguments for instantiating a
                 `SystemLandscapeView` (recommended).
         """
-        # TODO:
-        # assertThatTheViewKeyIsSpecifiedAndUnique(key);
         if system_landscape_view is None:
             system_landscape_view = SystemLandscapeView(
                 model=self.get_model(), **kwargs
             )
+        self._ensure_key_is_specific_and_unique(system_landscape_view.key)
         system_landscape_view.set_viewset(self)
         self.system_landscape_views.add(system_landscape_view)
         return system_landscape_view
@@ -224,9 +223,9 @@ def create_system_context_view(
         """
         # TODO:
         # assertThatTheSoftwareSystemIsNotNull(softwareSystem);
-        # assertThatTheViewKeyIsSpecifiedAndUnique(key);
         if system_context_view is None:
             system_context_view = SystemContextView(**kwargs)
+        self._ensure_key_is_specific_and_unique(system_context_view.key)
         system_context_view.set_viewset(self)
         self.system_context_views.add(system_context_view)
         return system_context_view
@@ -245,9 +244,9 @@ def create_container_view(
         """
         # TODO:
         # assertThatTheSoftwareSystemIsNotNull(softwareSystem);
-        # assertThatTheViewKeyIsSpecifiedAndUnique(key);
         if container_view is None:
             container_view = ContainerView(**kwargs)
+        self._ensure_key_is_specific_and_unique(container_view.key)
         container_view.set_viewset(self)
         self.container_views.add(container_view)
         return container_view
@@ -264,10 +263,9 @@ def create_component_view(
             **kwargs: Provide keyword arguments for instantiating a `ComponentView`
                 (recommended).
         """
-        # TODO:
-        # AssertThatTheViewKeyIsUnique(key);
         if component_view is None:
             component_view = ComponentView(**kwargs)
+        self._ensure_key_is_specific_and_unique(component_view.key)
         component_view.set_viewset(self)
         self.component_views.add(component_view)
         return component_view
@@ -279,9 +277,8 @@ def create_deployment_view(self, **kwargs) -> DeploymentView:
         Args:
             **kwargs: Provide keyword arguments for instantiating a `DeploymentView`
         """
-        # TODO:
-        # AssertThatTheViewKeyIsUnique(key);
         deployment_view = DeploymentView(**kwargs)
+        self._ensure_key_is_specific_and_unique(deployment_view.key)
         deployment_view.set_viewset(self)
         deployment_view.set_model(self.model)
         self.deployment_views.add(deployment_view)
@@ -294,9 +291,8 @@ def create_dynamic_view(self, **kwargs) -> DynamicView:
         Args:
             **kwagrs: Provide keyword arguments for instantiating a `DynamicView`.
         """
-        # TODO:
-        # AssertThatTheViewKeyIsUnique(key);
         dynamic_view = DynamicView(**kwargs)
+        self._ensure_key_is_specific_and_unique(dynamic_view.key)
         dynamic_view.set_viewset(self)
         dynamic_view.set_model(self.model)
         self.dynamic_views.add(dynamic_view)
@@ -309,9 +305,8 @@ def create_filtered_view(self, **kwargs) -> FilteredView:
         Args:
             **kwargs: Provide keyword arguments for instantiating a `FilteredView`.
         """
-        # TODO:
-        # AssertThatTheViewKeyIsUnique(key);
         filtered_view = FilteredView(**kwargs)
+        self._ensure_key_is_specific_and_unique(filtered_view.key)
         filtered_view.set_viewset(self)
         self.filtered_views.add(filtered_view)
         return filtered_view
@@ -338,6 +333,8 @@ def __getitem__(self, key: str) -> AbstractView:
 
     def copy_layout_information_from(self, source: "ViewSet") -> None:
         """Copy all the layout information from a source ViewSet."""
+
+        # Note that filtered views don't have any layout information to copy.
         for source_view in source.system_landscape_views:
             destination_view = self._find_system_landscape_view(source_view)
             if destination_view:
@@ -368,6 +365,12 @@ def copy_layout_information_from(self, source: "ViewSet") -> None:
             if destination_view:
                 destination_view.copy_layout_information_from(source_view)
 
+    def _ensure_key_is_specific_and_unique(self, key: str) -> None:
+        if key is None or key == "":
+            raise ValueError("A key must be specified.")
+        if self.get_view(key) is not None:
+            raise ValueError(f"View already exists in workspace with key '{key}'.")
+
     def _find_system_landscape_view(
         self, view: SystemLandscapeView
     ) -> Optional[SystemLandscapeView]:

tests/unit/view/test_view_set.py

@@ -43,7 +43,7 @@ def test_view_set_construction(empty_model):
 def test_adding_dynamic_view(empty_model):
     """Check adding dyanmic view initialises it properly."""
     viewset = ViewSet(model=empty_model)
-    view = viewset.create_dynamic_view(description="test")
+    view = viewset.create_dynamic_view(key="dyn1", description="test")
     assert view.model is empty_model
     assert view.get_viewset() is viewset
     assert view.description == "test"
@@ -125,3 +125,28 @@ def test_getting_view_by_key(empty_viewset):
     assert viewset["container1"] is container_view
     with pytest.raises(KeyError, match="No view with key"):
         viewset["bogus"]
+
+
+def test_key_constraints(empty_viewset):
+    """Test the common constraints on view keys when creating views."""
+    viewset = empty_viewset
+    system1 = viewset.model.add_software_system(name="sys1")
+
+    # No key
+    with pytest.raises(ValueError, match="A key must be specified"):
+        viewset.create_container_view(description="container", software_system=system1)
+
+    # Empty key
+    with pytest.raises(ValueError, match="A key must be specified"):
+        viewset.create_container_view(
+            key="", description="container", software_system=system1
+        )
+
+    # Duplicate key
+    viewset.create_container_view(
+        key="container1", description="container", software_system=system1
+    )
+    with pytest.raises(ValueError, match="View already exists"):
+        viewset.create_container_view(
+            key="container1", description="container", software_system=system1
+        )