Fixes a bug that allows relationships to be created between parents and children.

Author: simonbrowndotje

Structurizr.Core.Tests/Model/ModelTests.cs

@@ -155,6 +155,88 @@ public void Test_AddRelationship_AllowsMultipleRelationshipsBetweenElements()
             Assert.Equal(2, element1.Relationships.Count);
         }
 
+        [Fact]
+        public void Test_AddRelationship_ThrowsAnException_WhenTheDestinationIsAChildOfTheSource()
+        {
+            SoftwareSystem softwareSystem = Model.AddSoftwareSystem("Software System", "");
+            Container container = softwareSystem.AddContainer("Container", "", "");
+            Component component = container.AddComponent("Component", "", "");
+
+            try
+            {
+                softwareSystem.Uses(container, "Uses");
+                throw new TestFailedException();
+            }
+            catch (ArgumentException ae)
+            {
+                Assert.Equal("Relationships cannot be added between parents and children.", ae.Message);
+                Assert.Equal(0, softwareSystem.Relationships.Count);
+            }
+
+            try
+            {
+                container.Uses(component, "Uses");
+                throw new TestFailedException();
+            }
+            catch (ArgumentException ae)
+            {
+                Assert.Equal("Relationships cannot be added between parents and children.", ae.Message);
+                Assert.Equal(0, softwareSystem.Relationships.Count);
+            }
+
+            try
+            {
+                softwareSystem.Uses(component, "Uses");
+                throw new TestFailedException();
+            }
+            catch (ArgumentException ae)
+            {
+                Assert.Equal("Relationships cannot be added between parents and children.", ae.Message);
+                Assert.Equal(0, softwareSystem.Relationships.Count);
+            }
+        }
+
+        [Fact]
+        public void Test_AddRelationship_ThrowsAnException_WhenTheSourceIsAChildOfTheDestination()
+        {
+            SoftwareSystem softwareSystem = Model.AddSoftwareSystem("Software System", "");
+            Container container = softwareSystem.AddContainer("Container", "", "");
+            Component component = container.AddComponent("Component", "", "");
+
+            try
+            {
+                container.Uses(softwareSystem, "Uses");
+                throw new TestFailedException();
+            }
+            catch (ArgumentException ae)
+            {
+                Assert.Equal("Relationships cannot be added between parents and children.", ae.Message);
+                Assert.Equal(0, softwareSystem.Relationships.Count);
+            }
+
+            try
+            {
+                component.Uses(container, "Uses");
+                throw new TestFailedException();
+            }
+            catch (ArgumentException ae)
+            {
+                Assert.Equal("Relationships cannot be added between parents and children.", ae.Message);
+                Assert.Equal(0, softwareSystem.Relationships.Count);
+            }
+
+            try
+            {
+                component.Uses(softwareSystem, "Uses");
+                throw new TestFailedException();
+            }
+            catch (ArgumentException ae)
+            {
+                Assert.Equal("Relationships cannot be added between parents and children.", ae.Message);
+                Assert.Equal(0, softwareSystem.Relationships.Count);
+            }
+        }
+
         [Fact]
         public void Test_ModifyRelationship_ThrowsAnException_WhenARelationshipIsNotSpecified()
         {

Structurizr.Core/Model/Model.cs

@@ -339,7 +339,12 @@ internal Relationship AddRelationship(Element source, Element destination, strin
             {
                 throw new ArgumentException("The destination must be specified.");
             }
-            
+
+            if (IsChildOf(source, destination) || IsChildOf(destination, source))
+            {
+                throw new ArgumentException("Relationships cannot be added between parents and children.");
+            }
+
             Relationship relationship = new Relationship(source, destination, description, technology, interactionStyle);
             if (AddRelationship(relationship)) {
                 return relationship;
@@ -348,6 +353,26 @@ internal Relationship AddRelationship(Element source, Element destination, strin
             return null;
         }
 
+        private bool IsChildOf(Element e1, Element e2)
+        {
+            if (e1 is Person || e2 is Person) {
+                return false;
+            }
+
+            Element parent = e2.Parent;
+            while (parent != null)
+            {
+                if (parent.Id.Equals(e1.Id))
+                {
+                    return true;
+                }
+
+                parent = parent.Parent;
+            }
+
+            return false;
+        }
+
         private bool AddRelationship(Relationship relationship)
         {
             if (!relationship.Source.Has(relationship))
@@ -667,41 +692,10 @@ private bool propagatedRelationshipIsAllowed(Element source, Element destination
             {
                 return false;
             }
-    
-            if (source.Parent != null) {
-                if (destination.Equals(source.Parent))
-                {
-                    return false;
-                }
-    
-                if (source.Parent.Parent != null)
-                {
-                    if (destination.Equals(source.Parent.Parent))
-                    {
-                        return false;
-                    }
-                }
-            }
-    
-            if (destination.Parent != null)
-            {
-                if (source.Equals(destination.Parent))
-                {
-                    return false;
-                }
-    
-                if (destination.Parent.Parent != null)
-                {
-                    if (source.Equals(destination.Parent.Parent))
-                    {
-                        return false;
-                    }
-                }
-            }
-    
-            return true;
+
+            return !(IsChildOf(source, destination) || IsChildOf(destination, source));
         }
 
     }
 
-}
+}

docs/changelog.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.9.3 (unreleased)
+
+- Fixes a bug that allows relationships to be created between parents and children.
+
 ## 0.9.2 (16th September 2019)
 
 - Fixes a bug where element and relationship positioning is lost when workspaces are merged.