tolitius / mount

managing Clojure and ClojureScript app state since (reset)
Eclipse Public License 1.0
1.22k stars 88 forks source link

stopping substitues #68

Closed jlgeering closed 7 years ago

jlgeering commented 7 years ago

Versions

[org.clojure/clojure "1.8.0"]
[mount "0.1.10"]

Steps to reproduce the issue

(defn create [name from]
  (let [id (gensym)]
    (println "creating" name id "from" from)
    {:name name
     :id   id}))

(defn destroy [state from]
  (println "destroying" (:name state) (:id state) "from" from))

(defstate a :start (create "a" "start a")
            :stop  (destroy a "stop a"))

(defstate b :start (create "b" "start b")
            :stop  (destroy b "stop b"))

(defn info []
  (println "current states:")
  (println " " (mount.tools.graph/states-with-deps))
  (println "  a:" (:name a) (:id a))
  (println "  b:" (:name b) (:id b)))

(defn this-works-as-expected []
  (mount/start)
  (info)
  (mount/stop))

(defn this-is-not-stopping-b []
  (mount/start-with-states {#'user/a #'user/b})
  (info)
  (mount/stop))

no surprises when calling (this-works-as-expected)

user=> (this-works-as-expected)
creating a G__12960 from start a
creating b G__12961 from start b
current states:
  ({:name #'user/a, :order 1, :status #{:started}, :deps #{}} {:name #'user/b, :order 2, :status #{:started}, :deps #{}})
  a: a G__12960
  b: b G__12961
destroying b G__12961 from stop b
destroying a G__12960 from stop a
{:stopped ["#'user/b" "#'user/a"]}

but calling (this-is-not-stopping-b) does not properly stop the substitute

user=> (this-is-not-stopping-b)
creating b G__13029 from start b
current states:
  ({:name #'user/a, :order 1, :status #{:started}, :deps #{}} {:name #'user/b, :order 2, :status #{:stopped}, :deps #{}})
  a: b G__13029
  b: nil nil
destroying nil nil from stop b
{:stopped ["#'user/a"]}

Expected result

user=> (this-is-not-stopping-b)
...
destroying b G__13029 from stop b

Actual result

user=> (this-is-not-stopping-b)
...
destroying nil nil from stop b
jlgeering commented 7 years ago

mount/stop is calling "stop b" (which is good) with b as specified in :stop (destroy b "stop b") instead of calling "stop b" with a (which is an alias to b)

I think I'm looking for a way to :stop (destroy b-or-whatever-the-current-alias-is "stop b")

Is this a problem with mount, or did I misunderstand something in the doc?

jlgeering commented 7 years ago

I guess I'm looking for the proper way to do substitutions, i.e. how to translate this mount-lite example to mount

tolitius commented 7 years ago

good question :)

If you notice from your mount-lite [example](), you don't really swapping two existing states, but a state with a value. In the mount-lite example, the value is another state that is created within the substitution.

why doesn't it work?

The reason is :stop (destroy b "stop b")) in definition of the state b.

The reference to b is passed directly into the :stop function at the point of creating the #'user/b state. Which means even if we copy / substitute #'user/a with #'user/b, the :stop function will still be keep looking for a b that is nil, since only the a was started.

I thought about it, and called it a bug. Usually in tests I just use swap with values which do not need :stop functions.

But in this case it is an omission on my part.

how to make it work?

I just changed start-with-states to take "state values". You can read about here in the 0.1.11 branch.

Here is a slightly modified working example (this is of off 0.1.11-SNAPSHOT):

(defn create [name from]
  (let [id (gensym)]
    (println "creating" name id "from" from)
    {:name name
     :id   id}))

(defn destroy [state from]
  (println "destroying" (:name state) (:id state) "from" from))

(defstate a :start (create "a" "start a")
            :stop  (destroy a "stop a"))

(defn log []
  (println "current states:")
  (println " " (mount.tools.graph/states-with-deps))
  (println "  a:" (:name a) (:id a))
  (println "  b: is not needed as a separate state"))

(defn this-works-as-expected []
  (mount/start)
  (log)
  (mount/stop))

running with it:

boot.user=> (this-works-as-expected)
creating a G__4160 from start a
current states:
  ({:name #'boot.user/a, :order 1, :status #{:started}, :deps #{}})
  a: a G__4160
  b: is not needed as a separate state
destroying a G__4160 from stop a
{:stopped ["#'boot.user/a"]}

now the interesting part:

(defn this-is-now-stopping-b []
  (mount/start-with-states {#'boot.user/a {:start #(create "b" "start b")
                                           :stop #(destroy a "stop b")}})
  (log)
  (mount/stop))

running with it:

boot.user=> (this-is-now-stopping-b)
creating b G__4173 from start b
current states:
  ({:name #'boot.user/a, :order 1, :status #{:started}, :deps #{}})
  a: b G__4173
  b: is not needed as a separate state
destroying b G__4173 from stop b
{:stopped ["#'boot.user/a"]}

another way to do it

Is to use composable start functions:

(-> (swap-states {#'boot.user/a {:start #(create "b" "start b")
                                 :stop #(destroy a "stop b")}})
    mount/start)
creating b G__4191 from start b
{:started ["#'boot.user/a"]}
boot.user=>

boot.user=> (log)
current states:
  ({:name #'boot.user/a, :order 1, :status #{:started}, :deps #{}})
  a: b G__4191
  b: is not needed as a separate state
nil
boot.user=>

boot.user=> (mount/stop)
destroying b G__4191 from stop b
{:stopped ["#'boot.user/a"]}

thanks for finding a bug :)

let me know if it works for you (with 0.1.11-SNAPSHOT).

tolitius commented 7 years ago

closing this. feel free to reopen in case more clarification is needed.