Skip to content

Commit

Permalink
Do not include dependencies on down transitions
Browse files Browse the repository at this point in the history
  • Loading branch information
codebeige committed Dec 9, 2023
1 parent 48210ee commit d6391a7
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 76 deletions.
62 changes: 22 additions & 40 deletions src/moira/transition.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -31,44 +31,27 @@
(if (all? ks) (keys app) ks))

(defn enqueue-modules
"Returns an [[moira.context|interceptor]] that inserts `ks` into `::modules`,
consequently scheduling them as targets for execution."
"Returns an [[moira.context|interceptor]] that inserts `ks` into `::modules`
to schedule them as targets for a transition. The order of execution is
determined by the dependency graph, so each module is guaranteed to be
inserted after its dependencies.
[ks]
If the `include-deps?` option is enabled, the queue will include missing
dependencies. The `reverse?` option inverts execution order."

{:name ::enqueue-modules
:enter (fn [{::keys [app] :as ctx}]
(->> ks
(resolve-module-ks app)
(update ctx ::modules context/into-queue)))})

(defn enqueue-modules-with-deps
"Returns an [[moira.context|interceptor]] that inserts `ks` and all their
dependencies into `::modules`, consequently scheduling them as targets for
execution.
Order is determined by the dependency graph so that each module is guaranteed
to be inserted after all its dependencies."

[ks]
[ks & {:keys [include-deps? reverse?]}]

{:name ::enqueue-modules-with-deps
{:name ::enqueue-modules
:enter (fn [{::keys [app] :as ctx}]
(->> ks
(resolve-module-ks app)
(module/dependency-chain app)
(update ctx ::modules context/into-queue)))})

(def reverse-modules
"[[moira.context|Interceptor]] that reverses the order of `::modules`
scheduled as targets for execution.
[[down]] uses this interceptor to ensure each dependency is targeted after
all its dependent modules."

{:name ::reverse-modules
:enter (fn [ctx]
(update ctx ::modules #(context/into-queue [] (reverse %))))})
(let [ks* (resolve-module-ks app ks)
emit (fn [deps]
(cond->> deps
(not include-deps?) (filter (set ks*))
reverse? reverse))]
(->> ks*
(module/dependency-chain app)
emit
(update ctx ::modules context/into-queue))))})

(defn execute-txs-1
"Dequeue the next item from `::modules` and [[moira.module/execute|execute]]
Expand Down Expand Up @@ -105,7 +88,7 @@

(defn execute
"Wrap `app` with a [[moira.context|context]] for execution and apply the
transition `txs` on each module respectively. Returns a `Promise` that
interceptor chain `txs` on each module respectively. Returns a `Promise` that
resolves to the updated `system-map`."

[app txs]
Expand All @@ -121,7 +104,7 @@
Dependencies are guaranteed to be updated first. `:app-log` is injected, if
not already present, and automatically added to each module's `:deps`. When a
circular dependency is detected, an error is thrown and no updates are
circular dependency is detected, an error is thrown, and no updates are
applied."

[app txs ks]
Expand All @@ -132,7 +115,7 @@

(execute app [log.txs/inject
log.txs/pause
(enqueue-modules-with-deps ks)
(enqueue-modules ks :include-deps? true)
(execute-txs txs)]))

(defn down
Expand All @@ -141,7 +124,7 @@
new application with updated module states.
Dependencies are guaranteed to be updated in reverse order. When a circular
dependency is detected, an error is thrown and no updates are applied."
dependency is detected, an error is thrown, and no updates are applied."

[app txs ks]

Expand All @@ -150,8 +133,7 @@
(s/valid? ::module-ks ks)]}

(execute app [log.txs/pause
(enqueue-modules-with-deps ks)
reverse-modules
(enqueue-modules ks :reverse? true)
(execute-txs txs)]))

(defn tx
Expand Down
24 changes: 11 additions & 13 deletions test/moira/application_test.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,20 @@
(deftest stop-modules-test
(let [app (application/create {:module-a {:deps #{:module-b}
:state ["init-a" "start-a"]
:stop pop
:stop #(conj % "stop-a")
:tags #{:started}}
:module-b {:state ["init-b" "start-b"]
:stop pop
:stop #(conj % "stop-b")
:tags #{:started}}})]
(async-let [app-state (application/stop! app [:module-a])]
(testing "module is updated"
(is (= ["init-a"] (get-in app-state [:module-a :state]))))
(testing "dependencies are updated"
(is (= ["init-b"] (get-in app-state [:module-b :state]))))
(is (= ["init-a" "start-a" "stop-a"]
(get-in app-state [:module-a :state]))))
(testing "dependencies are not updated"
(is (= ["init-b" "start-b"]
(get-in app-state [:module-b :state]))))
(testing "modules are untagged"
(is (= #{}
(get-in app-state [:module-a :tags])
(get-in app-state [:module-b :tags]))))
(is (= #{} (get-in app-state [:module-a :tags]))))
(testing "update state"
(is (= app-state @app))))))

Expand Down Expand Up @@ -127,12 +127,10 @@
(async-let [app-state (application/pause! app [:module-a :module-c])]
(testing "module is updated"
(is (= ["started-a" "pause-a"] (get-in app-state [:module-a :state]))))
(testing "dependencies are updated"
(is (= ["started-b" "pause-b"] (get-in app-state [:module-b :state]))))
(testing "dependencies are not updated"
(is (= ["started-b"] (get-in app-state [:module-b :state]))))
(testing "modules are tagged"
(is (= #{:started :paused}
(get-in app-state [:module-a :tags])
(get-in app-state [:module-b :tags]))))
(is (= #{:started :paused} (get-in app-state [:module-a :tags]))))
(testing "idle modules are not paused"
(is (= ["not-started"] (get-in app-state [:module-c :state])))
(is (= #{} (get-in app-state [:module-c :tags]))))
Expand Down
34 changes: 11 additions & 23 deletions test/moira/transition_test.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,31 @@
(list :module-a :module-b) [:module-a :module-b :module-c :module-d]
[:module-a :module-b] [:module-a :module-b :module-c :module-d]))))

(deftest enqueue-module-with-deps-test
(let [{:keys [enter]} (transition/enqueue-modules-with-deps [:module-c :module-d])]
(testing "push modules onto queue"
(are [before after] (= after (-> {::transition/modules before}
enter
(get ::transition/modules)
util/peek-seq))
nil [:module-c :module-d]
#queue [:module-a :module-b] [:module-a :module-b :module-c :module-d]
(list :module-a :module-b) [:module-a :module-b :module-c :module-d]
[:module-a :module-b] [:module-a :module-b :module-c :module-d]))))

(deftest enqueue-dependency-tree-test
(let [app {:module-a {:deps #{:module-c}}
:module-b {:deps #{:module-a}}
:module-c {}
:module-d {}}
{:keys [enter]} (transition/enqueue-modules-with-deps [:module-b])
{:keys [enter]} (transition/enqueue-modules [:module-b]
{:include-deps? true})
ctx {::transition/app app}]
(testing "inject chain of dependencies in order"
(is (= [:module-c :module-a :module-b]
(-> (enter ctx)
(get ::transition/modules)
util/peek-seq))))))

(deftest reverse-modules-test
(let [{:keys [enter]} transition/reverse-modules
ctx {::transition/modules #queue [:module-a :module-b :module-c]}]
(deftest enqueue-modules-reversed-test
(let [{:keys [enter]} (transition/enqueue-modules [:module-b :module-d]
{:include-deps? true
:reverse? true})
ctx {::transition/app {:module-a {:deps #{:module-c}}
:module-b {:deps #{:module-a}}
:module-c {}
:module-d {}}}]
(testing "reverse order of modules"
(is (= [:module-c :module-b :module-a]
(-> (enter ctx)
(get ::transition/modules)
util/peek-seq))))
(testing "restore original order by reversing again"
(is (= [:module-a :module-b :module-c]
(is (= [:module-d :module-b :module-a :module-c]
(-> (enter ctx)
(enter ctx)
(get ::transition/modules)
util/peek-seq))))))

Expand Down

0 comments on commit d6391a7

Please sign in to comment.