Bug fixes to prevent parents and children to both be added to contain…

…er/component views.
structurizr · May 22, 2021 · b585eed · b585eed
1 parent d8e6c88
commit b585eed
Show file tree

Hide file tree

Showing 10 changed files with 187 additions and 12 deletions.
diff --git a/build.gradle b/build.gradle
@@ -14,7 +14,7 @@ subprojects { proj ->
 
     description = 'Structurizr'
     group = 'com.structurizr'
-    version = '1.9.3'
+    version = '1.9.4'
 
     repositories {
         mavenCentral()

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## 1.9.4 (22nd May 2021)
+
+- Bug fixes to prevent parents and children to both be added to container/component views.
+
 ## 1.9.3 (11th May 2021)
 
 - Added an `addTheme` method on `Configuration`.

diff --git a/structurizr-core/src/com/structurizr/view/ComponentView.java b/structurizr-core/src/com/structurizr/view/ComponentView.java
@@ -266,6 +266,7 @@ protected void checkElementCanBeAdded(Element element) {
             if (element.equals(getContainer().getParent())) {
                 throw new ElementNotPermittedInViewException("The software system in scope cannot be added to a component view.");
             } else {
+                checkParentAndChildrenHaveNotAlreadyBeenAdded((SoftwareSystem)element);
                 return;
             }
         }
@@ -274,11 +275,13 @@ protected void checkElementCanBeAdded(Element element) {
             if (element.equals(getContainer())) {
                 throw new ElementNotPermittedInViewException("The container in scope cannot be added to a component view.");
             } else {
+                checkParentAndChildrenHaveNotAlreadyBeenAdded((Container)element);
                 return;
             }
         }
 
         if (element instanceof Component) {
+            checkParentAndChildrenHaveNotAlreadyBeenAdded((Component)element);
             return;
         }
 

diff --git a/structurizr-core/src/com/structurizr/view/ContainerView.java b/structurizr-core/src/com/structurizr/view/ContainerView.java
@@ -181,11 +181,13 @@ protected void checkElementCanBeAdded(Element element) {
             if (element.equals(getSoftwareSystem())) {
                 throw new ElementNotPermittedInViewException("The software system in scope cannot be added to a container view.");
             } else {
+                checkParentAndChildrenHaveNotAlreadyBeenAdded((SoftwareSystem)element);
                 return;
             }
         }
 
         if (element instanceof Container) {
+            checkParentAndChildrenHaveNotAlreadyBeenAdded((Container)element);
             return;
         }
 

diff --git a/structurizr-core/src/com/structurizr/view/DeploymentView.java b/structurizr-core/src/com/structurizr/view/DeploymentView.java
@@ -314,7 +314,7 @@ protected void checkElementCanBeAdded(Element elementToBeAdded) {
             Set<String> softwareSystemIds = getElements().stream().map(ElementView::getElement).filter(e -> e instanceof SoftwareSystemInstance).map(e -> (SoftwareSystemInstance)e).map(SoftwareSystemInstance::getSoftwareSystemId).collect(Collectors.toSet());
 
             if (softwareSystemIds.contains(containerInstanceToBeAdded.getContainer().getSoftwareSystem().getId())) {
-                throw new ElementNotPermittedInViewException("The parent of " + elementToBeAdded.getName() + " is already in this view.");
+                throw new ElementNotPermittedInViewException("A parent of " + elementToBeAdded.getName() + " is already in this view.");
             }
         }
     }

diff --git a/structurizr-core/src/com/structurizr/view/View.java b/structurizr-core/src/com/structurizr/view/View.java
@@ -439,20 +439,31 @@ public ViewSet getViewSet() {
     protected abstract boolean canBeRemoved(Element element);
 
     final void checkParentAndChildrenHaveNotAlreadyBeenAdded(StaticStructureElement elementToBeAdded) {
-        // check the parent hasn't been added already
-        Set<String> elementIds = getElements().stream().map(ElementView::getElement).map(Element::getId).collect(Collectors.toSet());
+        // check a parent hasn't been added already
+        Set<String> idsOfElementsInView = getElements().stream().map(ElementView::getElement).map(Element::getId).collect(Collectors.toSet());
 
-        if (elementToBeAdded.getParent() != null) {
-            if (elementIds.contains(elementToBeAdded.getParent().getId())) {
-                throw new ElementNotPermittedInViewException("The parent of " + elementToBeAdded.getName() + " is already in this view.");
+        Element parent = elementToBeAdded.getParent();
+        while (parent != null) {
+            if (idsOfElementsInView.contains(parent.getId())) {
+                throw new ElementNotPermittedInViewException("A parent of " + elementToBeAdded.getName() + " is already in this view.");
             }
+
+            parent = parent.getParent();
         }
 
         // and now check a child hasn't been added already
-        Set<String> elementParentIds = getElements().stream().map(ElementView::getElement).filter(e -> e.getParent() != null).map(e -> e.getParent().getId()).collect(Collectors.toSet());
+        Set<String> elementParentIds = new HashSet<>();
+        for (ElementView elementView : getElements()) {
+            Element element = elementView.getElement();
+            parent = element.getParent();
+            while (parent != null) {
+                elementParentIds.add(parent.getId());
+                parent = parent.getParent();
+            }
+        }
 
         if (elementParentIds.contains(elementToBeAdded.getId())) {
-            throw new ElementNotPermittedInViewException("The child of " + elementToBeAdded.getName() + " is already in this view.");
+            throw new ElementNotPermittedInViewException("A child of " + elementToBeAdded.getName() + " is already in this view.");
         }
     }
 

diff --git a/structurizr-core/test/unit/com/structurizr/view/ComponentViewTests.java b/structurizr-core/test/unit/com/structurizr/view/ComponentViewTests.java
@@ -626,4 +626,114 @@ public void test_addContainer_ThrowsAnException_WhenTheContainerIsTheScopeOfTheV
         }
     }
 
+    @Test
+    public void test_addSoftwareSystem_ThrowsAnException_WhenAChildContainerIsAlreadyAdded() {
+        try {
+            SoftwareSystem softwareSystem1 = model.addSoftwareSystem("Software System 1");
+            Container container1 = softwareSystem1.addContainer("Container 1");
+            Component component1 = container1.addComponent("Component 1");
+
+            SoftwareSystem softwareSystem2 = model.addSoftwareSystem("Software System 2");
+            Container container2 = softwareSystem2.addContainer("Container 2");
+            Component component2 = container2.addComponent("Component 2");
+
+            ComponentView view = views.createComponentView(container1, "key", "Description");
+
+            view.add(container2);
+            view.add(softwareSystem2);
+
+            fail();
+        } catch (ElementNotPermittedInViewException e) {
+            assertEquals("A child of Software System 2 is already in this view.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void test_addSoftwareSystem_ThrowsAnException_WhenAChildComponentIsAlreadyAdded() {
+        try {
+            SoftwareSystem softwareSystem1 = model.addSoftwareSystem("Software System 1");
+            Container container1 = softwareSystem1.addContainer("Container 1");
+            Component component1 = container1.addComponent("Component 1");
+
+            SoftwareSystem softwareSystem2 = model.addSoftwareSystem("Software System 2");
+            Container container2 = softwareSystem2.addContainer("Container 2");
+            Component component2 = container2.addComponent("Component 2");
+
+            ComponentView view = views.createComponentView(container1, "key", "Description");
+
+            view.add(component2);
+            view.add(softwareSystem2);
+
+            fail();
+        } catch (ElementNotPermittedInViewException e) {
+            assertEquals("A child of Software System 2 is already in this view.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void test_addContainer_ThrowsAnException_WhenAChildComponentIsAlreadyAdded() {
+        try {
+            SoftwareSystem softwareSystem1 = model.addSoftwareSystem("Software System 1");
+            Container container1 = softwareSystem1.addContainer("Container 1");
+            Component component1 = container1.addComponent("Component 1");
+
+            SoftwareSystem softwareSystem2 = model.addSoftwareSystem("Software System 2");
+            Container container2 = softwareSystem2.addContainer("Container 2");
+            Component component2 = container2.addComponent("Component 2");
+
+            ComponentView view = views.createComponentView(container1, "key", "Description");
+
+            view.add(component2);
+            view.add(container2);
+
+            fail();
+        } catch (ElementNotPermittedInViewException e) {
+            assertEquals("A child of Container 2 is already in this view.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void test_addContainer_ThrowsAnException_WhenTheParentIsAlreadyAdded() {
+        try {
+            SoftwareSystem softwareSystem1 = model.addSoftwareSystem("Software System 1");
+            Container container1 = softwareSystem1.addContainer("Container 1");
+            Component component1 = container1.addComponent("Component 1");
+
+            SoftwareSystem softwareSystem2 = model.addSoftwareSystem("Software System 2");
+            Container container2 = softwareSystem2.addContainer("Container 2");
+            Component component2 = container2.addComponent("Component 2");
+
+            ComponentView view = views.createComponentView(container1, "key", "Description");
+
+            view.add(softwareSystem2);
+            view.add(container2);
+
+            fail();
+        } catch (ElementNotPermittedInViewException e) {
+            assertEquals("A parent of Container 2 is already in this view.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void test_addComponent_ThrowsAnException_WhenTheParentIsAlreadyAdded() {
+        try {
+            SoftwareSystem softwareSystem1 = model.addSoftwareSystem("Software System 1");
+            Container container1 = softwareSystem1.addContainer("Container 1");
+            Component component1 = container1.addComponent("Component 1");
+
+            SoftwareSystem softwareSystem2 = model.addSoftwareSystem("Software System 2");
+            Container container2 = softwareSystem2.addContainer("Container 2");
+            Component component2 = container2.addComponent("Component 2");
+
+            ComponentView view = views.createComponentView(container1, "key", "Description");
+
+            view.add(softwareSystem2);
+            view.add(component2);
+
+            fail();
+        } catch (ElementNotPermittedInViewException e) {
+            assertEquals("A parent of Component 2 is already in this view.", e.getMessage());
+        }
+    }
+
 }
diff --git a/structurizr-core/test/unit/com/structurizr/view/ContainerViewTests.java b/structurizr-core/test/unit/com/structurizr/view/ContainerViewTests.java
@@ -6,6 +6,9 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import static org.junit.Assert.*;
 
 public class ContainerViewTests extends AbstractWorkspaceTestBase {
@@ -306,4 +309,46 @@ public void test_addSoftwareSystem_ThrowsAnException_WhenTheSoftwareSystemIsTheS
         }
     }
 
+    @Test
+    public void test_addSoftwareSystem_ThrowsAnException_WhenAChildContainerIsAlreadyAdded() {
+        try {
+            SoftwareSystem softwareSystem1 = model.addSoftwareSystem("Software System 1");
+            Container container1 = softwareSystem1.addContainer("Container 1");
+
+            SoftwareSystem softwareSystem2 = model.addSoftwareSystem("Software System 2");
+            Container container2 = softwareSystem2.addContainer("Container 2");
+
+            ContainerView view = views.createContainerView(softwareSystem1, "key", "Description");
+
+            view.add(container1);
+            view.add(container2);
+            view.add(softwareSystem2);
+
+            fail();
+        } catch (ElementNotPermittedInViewException e) {
+            assertEquals("A child of Software System 2 is already in this view.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void test_addContainer_ThrowsAnException_WhenTheParentIsAlreadyAdded() {
+        try {
+            SoftwareSystem softwareSystem1 = model.addSoftwareSystem("Software System 1");
+            Container container1 = softwareSystem1.addContainer("Container 1");
+
+            SoftwareSystem softwareSystem2 = model.addSoftwareSystem("Software System 2");
+            Container container2 = softwareSystem2.addContainer("Container 2");
+
+            ContainerView view = views.createContainerView(softwareSystem1, "key", "Description");
+
+            view.add(container1);
+            view.add(softwareSystem2);
+            view.add(container2);
+
+            fail();
+        } catch (ElementNotPermittedInViewException e) {
+            assertEquals("A parent of Container 2 is already in this view.", e.getMessage());
+        }
+    }
+
 }
diff --git a/structurizr-core/test/unit/com/structurizr/view/DeploymentViewTests.java b/structurizr-core/test/unit/com/structurizr/view/DeploymentViewTests.java
@@ -532,7 +532,7 @@ public void test_addContainerInstance_ThrowsAnException_WhenTheParentSoftwareSys
             deploymentView.add(containerInstance);
             fail();
         } catch (ElementNotPermittedInViewException e) {
-            assertEquals("The parent of Container is already in this view.", e.getMessage());
+            assertEquals("A parent of Container is already in this view.", e.getMessage());
         }
     }
 

diff --git a/structurizr-core/test/unit/com/structurizr/view/DynamicViewTests.java b/structurizr-core/test/unit/com/structurizr/view/DynamicViewTests.java
@@ -149,7 +149,7 @@ public void test_add_ThrowsAnException_WhenTheParentOfAnElementHasAlreadyBeenAdd
             dynamicView.add(component1, component2);
             fail();
         } catch (Exception e) {
-            assertEquals("The parent of Component 2 is already in this view.", e.getMessage());
+            assertEquals("A parent of Component 2 is already in this view.", e.getMessage());
         }
     }
 
@@ -171,7 +171,7 @@ public void test_add_ThrowsAnException_WhenTheChildOfAnElementHasAlreadyBeenAdde
             dynamicView.add(component1, container2);
             fail();
         } catch (Exception e) {
-            assertEquals("The child of Container 2 is already in this view.", e.getMessage());
+            assertEquals("A child of Container 2 is already in this view.", e.getMessage());
         }
     }