feat: throw condition exception on TestNode exception

CHANGELOG.md

@@ -1,5 +1,8 @@
 This is a history of changes to clara-rules.
 
+# 0.22.0-SNAPSHOT
+* Try and catch TestNode expression evaluation so that exceptions thrown are re-thrown wrapped in a condition exception which includes production name and bindings information. See [PR 471](https://github.com/cerner/clara-rules/pull/471).
+
 # 0.21.1
 * Add support to specify query binding arguments as symbols instead of only keywords so that defquery syntax looks closer to function definition syntax. See [PR 463](https://github.com/cerner/clara-rules/pull/463).
 

CONTRIBUTORS.md

@@ -11,6 +11,7 @@ Community
 - David Goeke [@dgoeke]
 - Dave Dixon [@sparkofreason]
 - Baptiste Fontaine [@bfontaine]
+- Jose Gomez [@k13gomez]
 
 [@rbrush]: https://github.com/rbrush
 [@mrrodriguez]: https://github.com/mrrodriguez

src/main/clojure/clara/rules/compiler.clj

@@ -415,17 +415,18 @@
         (list `-> '?__token__ :bindings binding-key)))
 
 ;; FIXME: add env...
-(defn compile-test [node-id tests]
-  (let [binding-keys (variables-as-keywords tests)
+(defn compile-test [node-id constraints]
+  (let [binding-keys (variables-as-keywords constraints)
         assignments (mapcat build-token-assignment binding-keys)
 
         ;; Hardcoding the node-type and fn-type as we would only ever expect 'compile-test' to be used for this scenario
         fn-name (mk-node-fn-name "TestNode" node-id "TE")]
 
-    `(fn ~fn-name [~'?__token__]
-       (let [~@assignments]
-
-        (and ~@tests)))))
+    `(let [handler# (fn ~fn-name [~'?__token__]
+                      (let [~@assignments]
+                        (and ~@constraints)))]
+       {:handler handler#
+        :constraints '~constraints})))
 
 (defn compile-action
   "Compile the right-hand-side action of a rule, returning a function to execute it."

src/main/clojure/clara/rules/engine.cljc

@@ -488,13 +488,15 @@
 
 (defn- throw-condition-exception
   "Adds a useful error message when executing a constraint node raises an exception."
-  [{:keys [cause node fact env bindings]}]
+  [{:keys [cause node fact env bindings] :as args}]
   (let [bindings-description (if (empty? bindings)
-                               "with no bindings\n"
+                               "with no bindings"
                                (str "with bindings\n  " bindings))
+        facts-description (if (contains? args :fact)
+                            (str "when processing fact\n " (pr-str fact))
+                            "with no fact")
         message-header (string/join ["Condition exception raised.\n"
-                                     "when processing fact\n"
-                                     (str "  " (pr-str fact) "\n")
+                                     (str facts-description "\n")
                                      (str bindings-description "\n")
                                      "Conditions:\n"])
         conditions-and-rules (get-conditions-and-rule-names node)
@@ -924,6 +926,16 @@
                              constraints)]
       [:not (into [type] full-constraints)])))
 
+(defn- test-node-matches
+  [node test-handler env token]
+  (let [test-result (try
+                      (test-handler token)
+                      (catch #?(:clj Exception :cljs :default) e
+                        (throw-condition-exception {:cause e
+                                                    :node node
+                                                    :env env
+                                                    :bindings (:bindings token)})))]
+    test-result))
 
 ;; The test node represents a Rete extension in which an arbitrary test condition is run
 ;; against bindings from ancestor nodes. Since this node
@@ -937,15 +949,22 @@
      memory
      listener
      children
-     (filter test tokens)))
+     (platform/eager-for
+      [token tokens
+       :when (test-node-matches node (:handler test) {} token)]
+      token)))
 
   (left-retract [node join-bindings tokens memory transport listener]
     (l/left-retract! listener node tokens)
     (retract-tokens transport memory listener children tokens))
 
   (get-join-keys [node] [])
 
-  (description [node] (str "TestNode -- " (:text test))))
+  (description [node] (str "TestNode -- " (:text test)))
+
+  IConditionNode
+  (get-condition-description [this]
+    (into [:test] (:constraints test))))
 
 (defn- do-accumulate
   "Runs the actual accumulation.  Returns the accumulated value."

src/main/clojure/clara/tools/testing_utils.cljc

@@ -154,26 +154,32 @@
      :std std}))
 
 #?(:clj
-   (defn ex-data-search [^Exception e edata]
-     (loop [non-matches []
-            e e]
-       (cond
+   (defn ex-data-search
+     ([^Exception e edata]
+      (ex-data-search e nil edata))
+     ([^Exception e emsg edata]
+      (loop [non-matches []
+             e e]
+        (cond
          ;; Found match.
-         (= edata
-            (select-keys (ex-data e)
-                         (keys edata)))
+         (and (= edata
+                 (select-keys (ex-data e)
+                              (keys edata)))
+              (or (= emsg
+                     (.getMessage e))
+                  (nil? emsg)))
          :success
 
          ;; Keep searching, record any non-matching ex-data.
          (.getCause e)
          (recur (if-let [ed (ex-data e)]
-                  (conj non-matches ed)
+                  (conj non-matches {(.getMessage e) ed})
                   non-matches)
                 (.getCause e))
 
          ;; Can't find a match.
          :else
-         non-matches))))
+         non-matches)))))
 
 #?(:clj
    (defn get-all-ex-data
@@ -190,19 +196,22 @@
                     (get-ex-chain e))))))
 
 #?(:clj
-   (defmacro assert-ex-data [expected-ex-data form]
-     `(try
-        ~form
-        (is false
-            (str "Exception expected to be thrown when evaluating: " \newline
-                 '~form))
-        (catch Exception e#
-          (let [res# (ex-data-search e# ~expected-ex-data)]
-            (is (= :success res#)
-                (str "Exception msg found: " \newline
-                     e# \newline
-                     "Non matches found: " \newline
-                     res#)))))))
+   (defmacro assert-ex-data
+     ([expected-ex-data form]
+      `(assert-ex-data nil ~expected-ex-data ~form))
+     ([expected-ex-message expected-ex-data form]
+      `(try
+         ~form
+         (is false
+             (str "Exception expected to be thrown when evaluating: " \newline
+                  '~form))
+         (catch Exception e#
+           (let [res# (ex-data-search e# ~expected-ex-message ~expected-ex-data)]
+             (is (= :success res#)
+                 (str "Exception msg found: " \newline
+                      e# \newline
+                      "Non matches found: " \newline
+                      res#))))))))
 
 #?(:clj
    (defn ex-data-maps

src/test/clojure/clara/test_compiler.clj

@@ -43,7 +43,7 @@
   (let [get-node-fns (fn [node]
                        (condp instance? node
                          AlphaNode [(:activation node)]
-                         TestNode [(:test node)]
+                         TestNode [(-> node :test :handler)]
                          AccumulateNode []
                          AccumulateWithJoinFilterNode [(:join-filter-fn node)]
                          ProductionNode [(:rhs node)]
@@ -72,4 +72,4 @@
     (tu/assert-ex-data {:expected-bindings #{:?b}
                         :available-bindings #{:?c}
                         :query "a-query"}
-                       (r/mk-session [query]))))
+                       (r/mk-session [query]))))

src/test/clojure/clara/test_durability.clj

@@ -12,8 +12,9 @@
             [clojure.test :refer :all]
             [clara.rules.compiler :as com]
             [clara.tools.testing-utils :as tu])
-  (:import [clara.rules.testfacts
-            Temperature]))
+  (:import [clara.rules.testfacts Temperature]
+           [clara.rules.engine TestNode]))
+
 
 (use-fixtures :once st/validate-schemas)
 
@@ -338,6 +339,36 @@
              restored-qresults1
              restored-qresults2)))))
 
+(defn get-test-nodes
+  [session]
+  (->> session
+       eng/components
+       :rulebase
+       :id-to-node
+       vals
+       (filter (partial instance? TestNode))))
+
+(deftest test-durability-testnode-serde
+  (let [s (mk-session 'clara.durability-rules)
+        rb (-> s eng/components :rulebase)
+        deserialized1 (rb-serde s nil)
+        ;; Need a session to do the 2nd round of SerDe.
+        restored1 (d/assemble-restored-session deserialized1 {})
+        deserialized2 (rb-serde restored1 nil)
+        restored2 (d/assemble-restored-session deserialized2 {})]
+    (testing "ensure that a TestNode's ICondition/get-condition-description implementation survives serialization and deserialization"
+      (is (= [[:test '(not-empty ?ts)]]
+             (map eng/get-condition-description (get-test-nodes s))
+             (map eng/get-condition-description (get-test-nodes restored1))
+             (map eng/get-condition-description (get-test-nodes restored2)))))
+    (testing "ensure that a TestNode which has gone through SerDe still works correctly by inserting a new fact, re-firing the rules, and then querying to ensure the test node worked"
+      (let [{:keys [fired-session]} (session-test restored2)]
+        (is (= [{:?his (->TemperatureHistory [50 40 30 20 10])}]
+               (-> fired-session
+                   (insert (->Temperature 10 "ORD"))
+                   (fire-rules)
+                   (query dr/temp-his))))))))
+
 (deftest test-assemble-restored-session-opts
   (let [orig (mk-session 'clara.durability-rules)
 

src/test/clojure/clara/test_rules.clj

@@ -2361,10 +2361,26 @@
                   (query check-exception))
               (is false "Scope binding test is expected to throw an exception.")
               (catch Exception e e))]
-
       (is (= {:?w 10, :?t nil}
              (-> e (ex-data) :bindings))))))
 
+(deftest test-include-details-when-exception-is-thrown-in-test-filter
+  (testing "when a condition exception is thrown ensure it contains the necessary details and text"
+    (let [check-exception (assoc (dsl/parse-query [] [[WindSpeed (= ?w windspeed)]
+                                                      [Temperature (= ?t temperature)]
+                                                      [:test (> ?t ?w)]])
+                                 :name "my-test-query")]
+
+      (assert-ex-data "Condition exception raised.\nwith no fact\nwith bindings\n  {:?w 10, :?t nil}\nConditions:\n\n1. [:test (> ?t ?w)]\n   queries:\n     my-test-query\n"
+                      {:bindings {:?w 10, :?t nil}
+                       :fact nil
+                       :env {}
+                       :conditions-and-rules {[:test '(> ?t ?w)] #{[:query "my-test-query"]}}}
+                      (-> (mk-session [check-exception])
+                          (insert (->WindSpeed 10 "MCI") (->Temperature nil "MCI"))
+                          (fire-rules)
+                          (query check-exception))))))
+
 ;;; Test that we can properly assemble a session, insert facts, fire rules,
 ;;; and run a query with keyword-named productions.
 (deftest test-simple-insert-data-with-keyword-names