From 6431d52a2ed3f81c816c4c2fbed251b9bbca2334 Mon Sep 17 00:00:00 2001
From: Barnaby Gray <barnaby.gray@artirix.com>
Date: Wed, 12 Feb 2014 09:09:55 +0000
Subject: [PATCH] Implement topological sort using Cormen/Tarjan algorithm to
 handle a->b->c dependencies and detect a->b->c->a cycles.

---
 fig/project.py             | 45 +++++++++++++++++++-------------------
 tests/sort_service_test.py | 45 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/fig/project.py b/fig/project.py
index 43d52983a..235826d28 100644
--- a/fig/project.py
+++ b/fig/project.py
@@ -7,31 +7,30 @@ log = logging.getLogger(__name__)
 
 
 def sort_service_dicts(services):
-    # Get all services that are dependant on another.
-    dependent_services = [s for s in services if s.get('links')]
-    flatten_links = sum([s['links'] for s in dependent_services], [])
-    # Get all services that are not linked to and don't link to others.
-    non_dependent_sevices = [s for s in services if s['name'] not in flatten_links and not s.get('links')]
+    # Topological sort (Cormen/Tarjan algorithm).
+    unmarked = services[:]
+    temporary_marked = set()
     sorted_services = []
-    # Topological sort.
-    while dependent_services:
-        n = dependent_services.pop()
-        # Check if a service is dependent on itself, if so raise an error.
-        if n['name'] in n.get('links', []):
-            raise DependencyError('A service can not link to itself: %s' % n['name'])
-        sorted_services.append(n)
-        for l in n['links']:
-            # Get the linked service.
-            linked_service = next(s for s in services if l == s['name'])
-            # Check that there isn't a circular import between services.
-            if n['name'] in linked_service.get('links', []):
-                raise DependencyError('Circular import between %s and %s' % (n['name'], linked_service['name']))
-            # Check the linked service has no links and is not already in the
-            # sorted service list.
-            if not linked_service.get('links') and linked_service not in sorted_services:
-                sorted_services.insert(0, linked_service)
-    return non_dependent_sevices + sorted_services
 
+    def visit(n):
+        if n['name'] in temporary_marked:
+            if n['name'] in n.get('links', []):
+                raise DependencyError('A service can not link to itself: %s' % n['name'])
+            else:
+                raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked))
+        if n in unmarked:
+            temporary_marked.add(n['name'])
+            dependents = [m for m in services if n['name'] in m.get('links', [])]
+            for m in dependents:
+                visit(m)
+            temporary_marked.remove(n['name'])
+            unmarked.remove(n)
+            sorted_services.insert(0, n)
+
+    while unmarked:
+        visit(unmarked[-1])
+
+    return sorted_services
 
 class Project(object):
     """
diff --git a/tests/sort_service_test.py b/tests/sort_service_test.py
index d5218cd36..13cff89d1 100644
--- a/tests/sort_service_test.py
+++ b/tests/sort_service_test.py
@@ -44,6 +44,27 @@ class SortServiceTest(unittest.TestCase):
         self.assertEqual(sorted_services[1]['name'], 'postgres')
         self.assertEqual(sorted_services[2]['name'], 'web')
 
+    def test_sort_service_dicts_3(self):
+        services = [
+            {
+                'name': 'child'
+            },
+            {
+                'name': 'parent',
+                'links': ['child']
+            },
+            {
+                'links': ['parent'],
+                'name': 'grandparent'
+            },
+        ]
+
+        sorted_services = sort_service_dicts(services)
+        self.assertEqual(len(sorted_services), 3)
+        self.assertEqual(sorted_services[0]['name'], 'child')
+        self.assertEqual(sorted_services[1]['name'], 'parent')
+        self.assertEqual(sorted_services[2]['name'], 'grandparent')
+
     def test_sort_service_dicts_circular_imports(self):
         services = [
             {
@@ -87,6 +108,30 @@ class SortServiceTest(unittest.TestCase):
         else:
             self.fail('Should have thrown an DependencyError')
 
+    def test_sort_service_dicts_circular_imports_3(self):
+        services = [
+            {
+                'links': ['b'],
+                'name': 'a'
+            },
+            {
+                'name': 'b',
+                'links': ['c']
+            },
+            {
+                'name': 'c',
+                'links': ['a']
+            }
+        ]
+
+        try:
+            sort_service_dicts(services)
+        except DependencyError as e:
+            self.assertIn('a', e.msg)
+            self.assertIn('b', e.msg)
+        else:
+            self.fail('Should have thrown an DependencyError')
+
     def test_sort_service_dicts_self_imports(self):
         services = [
             {