Avoid running after/enrich without a db effect

Previously both standard interceptors acted upon the `db` *coeffect* if
no *effect* was available. Per day8#453 and related discussion, there is little
reason in general to do this, since this context means that the `app-db`
should not change.

Both functions are changed to explicitly check for the presence of the
`db` effect. If it is not present in the context, the interceptor
function is not run.

The previous functionality around `nil` and `false` values for the `db`
effect is preserved: the interceptors *are* run in this case. Tests are
added to verify the behavior of both functions in the case of
non-presence, falsey and truthy values.

The relevant docstrings are tweaked to accommodate this change.

Closes day8#453.

CHANGES.md

@@ -1,5 +1,8 @@
 ## Unchanged
 
+#### Fixed
+
+  - `after` and `enrich` interceptors now no longer run if there is no `db` effect, rather than running against the `db` coffect. [#453](https://github.com/Day8/re-frame/issues/453)
 
 
 ## 0.10.6 (2018-09-03)

src/re_frame/std_interceptors.cljc

@@ -240,7 +240,7 @@
   return a modified `db`.
 
   Unlike the `after` interceptor which is only about side effects, `enrich`
-  expects `f` to process and alter the given `db` coeffect in some useful way,
+  expects `f` to process and alter the given `db` effect in some useful way,
   contributing to the derived data, flowing vibe.
 
   Example Use:
@@ -283,21 +283,19 @@
     :id :enrich
     :after (fn enrich-after
              [context]
-             (let [event (get-coeffect context :event)
-                   db    (if (contains? (:effects context) :db)
-                           (get-effect context :db) ;; If no db effect is returned, we provide the original coeffect.
-                           (get-coeffect context :db))]
-               (->> (f db event)
-                    (assoc-effect context :db))))))
+             (let [event  (get-coeffect context :event)
+                   db-fx? (contains? (:effects context) :db)
+                   db     (get-effect context :db)]
+               (cond-> context
+                 db-fx? (assoc-effect :db (f db event)))))))
 
 
 
 (defn after
   "returns an interceptor which runs a given function `f` in the `:after`
   position, presumably for side effects.
 
-  `f` is called with two arguments: the `:effects` value for `:db`
-  (or the `coeffect` value of db if no db effect is returned) and the event.
+  `f` is called with two arguments: the `:effects` value for `:db` and the event.
   Its return value is ignored, so `f` can only side-effect.
 
   Examples use can be seen in the /examples/todomvc:
@@ -308,12 +306,11 @@
     :id :after
     :after (fn after-after
              [context]
-             (let [db    (if (contains? (:effects context) :db)
-                           (get-in context [:effects :db])
-                           (get-in context [:coeffects :db]))
-                   event (get-in context [:coeffects :event])]
-               (f db event) ;; call f for side effects
-               context)))) ;; context is unchanged
+             (let [event  (get-coeffect context :event)
+                   db-fx? (contains? (:effects context) :db)
+                   db     (get-effect context :db)]
+               (when db-fx? (f db event))) ;; call f for side effects
+             context))) ;; context is unchanged
 
 
 (defn  on-changes

test/re_frame/interceptor_test.cljs

@@ -136,42 +136,48 @@
                 ((:after change-i))
                 (get-effect :db ::not-found))))))
 
+(defn- intercept-with-db-effect
+  "Used to test if interceptors run given a db effect."
+  [interceptor-f db-fx add?]
+  (cond-> (context [:a :b] [(interceptor-f)] {:a 1})
+    add? (assoc-effect :db db-fx)
+    true (interceptor/invoke-interceptors :before)
+    true interceptor/change-direction
+    true (interceptor/invoke-interceptors :after)))
+
 (deftest test-after
-  (testing "when no db effect is returned"
-    (let [after-db-val (atom nil)]
-      (-> (context [:a :b]
-                   [(after (fn [db] (reset! after-db-val db)))]
-                   {:a 1})
-          (interceptor/invoke-interceptors :before)
-          interceptor/change-direction
-          (interceptor/invoke-interceptors :after))
-      (is (= @after-db-val {:a 1}))))
-  (testing "when a false db effect is returned"
-    (let [after-db-val (atom :not-reset)]
-      (-> (context [:a :b]
-                   [(after (fn [db] (reset! after-db-val db)))]
-                   {:a 2})
-          (assoc-effect :db nil)
-          (interceptor/invoke-interceptors :before)
-          interceptor/change-direction
-          (interceptor/invoke-interceptors :after))
-      (is (= @after-db-val nil))))
-  (testing "when a nil db effect is returned"
-    (let [after-db-val (atom :not-reset)]
-      (-> (context [:a :b]
-                   [(after (fn [db] (reset! after-db-val db)))]
-                   {:a 3})
-          (assoc-effect :db false)
-          (interceptor/invoke-interceptors :before)
-          interceptor/change-direction
-          (interceptor/invoke-interceptors :after))
-      (is (= @after-db-val false)))))
+  (let [after-db-val (atom nil)
+        run-after    (partial intercept-with-db-effect
+                        #(after (fn [db] (reset! after-db-val db))))]
+    (testing "when no db effect is returned"
+      (reset! after-db-val :not-reset)
+      (run-after nil false)
+      (is (= @after-db-val :not-reset)))
+    (testing "when a nil db effect is returned"
+      (reset! after-db-val :not-reset)
+      (run-after nil true)
+      (is (= @after-db-val nil)))
+    (testing "when a false db effect is returned"
+      (reset! after-db-val :not-reset)
+      (run-after false true)
+      (is (= @after-db-val false)))
+    (testing "when a truthy db effect is returned"
+      (reset! after-db-val :not-reset)
+      (run-after :reset true)
+      (is (= @after-db-val :reset)))))
 
 (deftest test-enrich
-  (testing "when no db effect is returned"
-    (let [ctx (context [] [] {:a 1})]
-      (is (= ::not-found (get-effect ctx :db ::not-found)))
-      (-> ctx (:after (enrich (fn [db] (is (= db {:a 1})))))))))
+  (let [run-enrich (partial intercept-with-db-effect
+                      #(enrich (fn [db] (assoc db :enriched :added))))]
+    (testing "when no db effect is returned"
+      (let [ctx (run-enrich nil false)]
+        (is (= (-> ctx (get-effect :db) :enriched) nil))))
+    (testing "when nil db effect is returned"
+      (let [ctx (run-enrich nil true)]
+        (is (= (-> ctx (get-effect :db) :enriched) :added))))
+    (testing "when truthy db effect is returned"
+      (let [ctx (run-enrich {} true)]
+        (is (= (-> ctx (get-effect :db) :enriched) :added))))))
 
 (deftest test-update-coeffect
   (let [context {:effects {:db {:a 1}}