Order transitive dependencies

Currently, transitive dependencies are being started in the order they
were loaded, not in dependency order. This can cause issues in
situations where new states which are needed for substitutes are
loaded much later than the original state that is being overridden to
depend on those new states.

An example of this is in a test environment where you may have
configuration overrides that you wouldn't define in your production
overrides that are loaded from dev-only code. Without this change, the
new test that was written failed because the newly added dependency was
not loaded first.

I had to do a bit of a hack to remove a state defined by defstate-let me
know if there is a better way of doing this.

src/mount/extensions/common_deps.clj

@@ -7,25 +7,28 @@
 (defn- transitive
   "Given a graph and a node, returns all the transitive nodes."
   [graph node]
-  (loop [unexpanded (graph node)
-         expanded #{}]
-    (if-let [[node & more] (seq unexpanded)]
-      (if (contains? expanded node)
-        (recur more expanded)
-        (recur (concat more (graph node))
-               (conj expanded node)))
-      expanded)))
+  (letfn [(search [cur]
+            (->> (get graph cur)
+                 (map search)
+                 (reduce into [cur])))]
+    (search node)))
+
+(defn ^:no-doc transitives
+  "Filters and orders a given list of states in transitive dependency order
+  based upon the given dependency graphs."
+  [var graphs states]
+  (let [var-kw       (utils/var->keyword var)
+        dependencies (reverse (transitive (:dependencies graphs) var-kw))
+        dependents   (transitive (:dependents graphs) var-kw)
+        concatted    (distinct (concat dependencies dependents))]
+    (filter (set states) concatted)))
 
 (defn ^:no-doc with-transitives*
   "Calls 0-arity function `f`, while `*states*` has been bound to the
   transitive dependencies and dependents of the given state `var`."
   [var graphs f]
-  (let [var-kw       (utils/var->keyword var)
-        dependencies (transitive (:dependencies graphs) var-kw)
-        dependents   (transitive (:dependents graphs) var-kw)
-        concatted    (set (concat dependencies dependents))]
-    (binding [mount/*states* (atom (filter (conj concatted var-kw) @mount/*states*))]
-      (f))))
+  (binding [mount/*states* (atom (transitives var graphs @mount/*states*))]
+    (f)))
 
 (defmacro with-transitives
   "Wraps the `body` having the `*states*` extension point bound to the

test/mount/extensions/common_deps_test.clj

@@ -0,0 +1,57 @@
+(ns mount.extensions.common-deps-test
+  (:require
+   [clojure.test :refer [deftest testing is]]
+   [mount.extensions.common-deps :as sut]))
+
+(def system "system")
+
+(def graphs
+  {:dependencies {::overrides   #{::overrides-a ::overrides-b}
+                  ::config-a    #{}
+                  ::overrides-a #{::config-a}
+                  ::config-b    #{}
+                  ::registry    #{}
+                  ::config      #{::overrides ::env}
+                  ::system      #{::config ::debug ::db}
+                  ::debug       #{}
+                  ::db          #{::config}
+                  ::env         #{}
+                  ::overrides-b #{::config-b}}
+   :dependents   {::overrides   #{::config}
+                  ::config-a    #{::overrides-a}
+                  ::overrides-a #{::overrides}
+                  ::config-b    #{::overrides-b}
+                  ::registry    #{}
+                  ::config      #{::system ::db}
+                  ::system      #{}
+                  ::debug       #{::system}
+                  ::db          #{::system}
+                  ::env         #{::config}
+                  ::overrides-b #{::overrides}}})
+
+(def states
+  [::overrides
+   ::config-a
+   ::overrides-a
+   ::config-b
+   ::registry
+   ::config
+   ::system
+   ::debug
+   ::db
+   ::env
+   ::overrides-b])
+
+(deftest transitives-test
+  (testing "ordering active states in dependency order"
+    (is (= [::debug
+            ::env
+            ::config-b
+            ::overrides-b
+            ::config-a
+            ::overrides-a
+            ::overrides
+            ::config
+            ::db
+            ::system]
+           (sut/transitives #'system graphs states)))))

test/mount/extensions/explicit_deps_test.clj

@@ -1,7 +1,8 @@
 (ns mount.extensions.explicit-deps-test
   (:require [clojure.test :refer (deftest is use-fixtures testing)]
             [mount.extensions.explicit-deps :as deps]
-            [mount.lite :refer (start state status stop with-substitutes)]
+            [mount.lite :as mount :refer (defstate start state status stop with-substitutes)]
+            [mount.utils :as utils]
             [mount.lite-test.test-state-1 :as ts1 :refer (state-1)]
             [mount.lite-test.test-state-2 :as ts2 :refer (state-2)]
             [mount.lite-test.test-state-2-extra :as ts2e :refer (state-2-a state-2-b)]
@@ -11,9 +12,25 @@
 
 (use-fixtures :each (fn [f] (stop) (f) (stop)))
 
+(declare someone)
+
+(defn def-someone
+  []
+  (defstate someone
+    :start "i am someone"
+    :dependencies nil))
+
+(defn undef-someone
+  []
+  (ns-unmap *ns* 'someone)
+  (swap! mount/*states*
+         #(remove %2 %1)
+         #(= % (utils/var->keyword #'someone))))
+
 ;;; Tests.
 
 (def dont-need-anyone (state :start "appeltaart" :dependencies nil))
+(def i-need-someone (state :start (str @someone " yay!") :dependencies [#'someone]))
 
 (deftest build-graphs-test
   (testing "no substitutes used"
@@ -40,6 +57,15 @@
     (deps/start #'state-3))
   (is (status) {#'state-1 :stopped #'state-2 :started #'state-3 :started}))
 
+(deftest start-with-overridden-dependency
+  (try
+    (def-someone)
+    (with-substitutes [#'state-2-a i-need-someone]
+      (deps/start #'state-2-a))
+    (is (status) {#'state-1 :started #'state-2 :started #'someone :started #'state-2-a :started})
+    (finally
+      (undef-someone))))
+
 (deftest stop-test
   (with-substitutes [#'state-2 dont-need-anyone]
     (start))