From b6e7be0a8cc6a465f66c4e70adf365175918ebc7 Mon Sep 17 00:00:00 2001
From: Charlie Harding <charlie_harding@icloud.com>
Date: Mon, 20 Apr 2020 13:32:00 +0100
Subject: [PATCH] [Validator] Block self-references (without @nullable) in
 foreign keys

---
 src/main/scala/temple/DSL/semantics/Tarjan.scala        | 7 +++++--
 src/main/scala/temple/DSL/semantics/Validator.scala     | 4 ++--
 src/test/scala/temple/DSL/semantics/ValidatorTest.scala | 9 +++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/main/scala/temple/DSL/semantics/Tarjan.scala b/src/main/scala/temple/DSL/semantics/Tarjan.scala
index 43f35dea0..dee7cca53 100644
--- a/src/main/scala/temple/DSL/semantics/Tarjan.scala
+++ b/src/main/scala/temple/DSL/semantics/Tarjan.scala
@@ -17,12 +17,14 @@ class Tarjan[T] private (graph: Map[T, Iterable[T]]) {
     sccBuffer.toSet
   }
 
+  private def neighbours(node: T): Iterable[T] = graph.getOrElse(node, Nil)
+
   private def visit(node: T): Unit = {
     // Set the depth index for v to the smallest unused index
     index(node) = index.size
     lowLink(node) = index(node)
     stack.push(node)
-    for (neighbour <- graph.getOrElse(node, Nil)) {
+    for (neighbour <- neighbours(node)) {
       if (!index.contains(neighbour)) { // neighbour has not yet been visited; recurse on it
         visit(neighbour)
         lowLink(node) = math.min(lowLink(node), lowLink(neighbour))
@@ -34,7 +36,8 @@ class Tarjan[T] private (graph: Map[T, Iterable[T]]) {
     if (lowLink(node) == index(node)) {
       // Pop all elements from the stack until and including this element itself
       val scc = (stack.popWhile(_ != node) :+ stack.pop()).toSet
-      sccBuffer += scc
+      // Only add single elements if they have a self-loop
+      if (scc.sizeIs != 1 || neighbours(node).iterator.contains(node)) sccBuffer += scc
     }
   }
 
diff --git a/src/main/scala/temple/DSL/semantics/Validator.scala b/src/main/scala/temple/DSL/semantics/Validator.scala
index 290e5c107..768f2713b 100644
--- a/src/main/scala/temple/DSL/semantics/Validator.scala
+++ b/src/main/scala/temple/DSL/semantics/Validator.scala
@@ -215,10 +215,10 @@ private class Validator private (templefile: Templefile) {
   private val referenceCycles: Set[Set[String]] = {
     val graph = templefile.providedBlockNames.map { blockName =>
       blockName -> templefile.getBlock(blockName).attributes.values.collect {
-        case Attribute(ForeignKey(references), _, annotations) => references
+        case Attribute(ForeignKey(references), _, _) => references
       }
     }.toMap
-    Tarjan(graph).filter(_.sizeIs > 1)
+    Tarjan(graph)
   }
 
   def validate(): Seq[String] = {
diff --git a/src/test/scala/temple/DSL/semantics/ValidatorTest.scala b/src/test/scala/temple/DSL/semantics/ValidatorTest.scala
index 525a873c0..f032f7c18 100644
--- a/src/test/scala/temple/DSL/semantics/ValidatorTest.scala
+++ b/src/test/scala/temple/DSL/semantics/ValidatorTest.scala
@@ -313,6 +313,15 @@ class ValidatorTest extends FlatSpec with Matchers {
   }
 
   it should "find cycles in dependencies" in {
+    validationErrors(
+      Templefile(
+        "MyProject",
+        services = Map(
+          "Box" -> ServiceBlock(Map("box" -> Attribute(ForeignKey("Box")))),
+        ),
+      ),
+    ) shouldBe Set("Cycle(s) were detected in foreign keys, between elements: { Box }")
+
     validationErrors(
       Templefile(
         "MyProject",