[Issue454]: Add validation that queries provide the correct parameters (

#455)

CHANGELOG.md

@@ -3,6 +3,8 @@ This is a history of changes to clara-rules.
 # 0.21.0 SNAPSHOT
 * Add names to anonymous functions generated by rule compilation, see [issue 261](https://github.com/cerner/clara-rules/issues/261) and [issue 291](https://github.com/cerner/clara-rules/issues/291)
 * Add alpha node types, see [issue 237](https://github.com/cerner/clara-rules/issues/237)
+* Validate parameters provided to queries exist on query and compilation. 
+  Throws exception if query doesn't specify appropriate parameters. see [issue 454](https://github.com/cerner/clara-rules/issues/454)
 
 # 0.20.0
 * Add a flag to omit compilation context (used by the durability layer) after Session compilation to save space when not needed. Defaults to true. [issue 422](https://github.com/cerner/clara-rules/issues/422)

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

@@ -1292,15 +1292,26 @@
                  beta-with-nodes))
 
         ;; No more conditions to add, so connect the production.
-        (add-node beta-graph
-                  parent-ids
-                  (create-id-fn)
-                  (if (:rhs production)
+        (if (:rhs production)
+          ;; if its a production node simply add it
+          (add-node beta-graph
+                    parent-ids
+                    (create-id-fn)
                     {:node-type :production
                      :production production
-                     :bindings ancestor-bindings}
-                    {:node-type :query
-                     :query production}))))))
+                     :bindings ancestor-bindings})
+          ;; else its a query node and we need to validate that the query has at least the bindings
+          ;; specified in the parameters
+          (if (every? ancestor-bindings (:params production))
+            (add-node beta-graph
+                      parent-ids
+                      (create-id-fn)
+                      {:node-type :query
+                       :query production})
+            (throw (ex-info "Query does not contain bindings specified in parameters."
+                            {:expected-bindings (:params production)
+                             :available-bindings ancestor-bindings
+                             :query (:name production)}))))))))
 
 
 (sc/defn to-beta-graph :- schema/BetaGraph

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

@@ -1986,6 +1986,10 @@
     (let [query-node (get-in rulebase [:query-nodes query])]
       (when (= nil query-node)
         (platform/throw-error (str "The query " query " is invalid or not included in the rule base.")))
+      (when-not (= (into #{} (keys params)) ;; nil params should be equivalent to #{}
+                   (:param-keys query-node))
+        (platform/throw-error (str "The query " query " was not provided with the correct parameters, expected: "
+                                   (:param-keys query-node) ", provided: " (set (keys params)))))
 
       (->> (mem/get-tokens memory query-node params)
 

src/test/clojure/clara/test_compiler.clj

@@ -14,7 +14,8 @@
             ProductionNode
             NegationWithJoinFilterNode
             ExpressionJoinNode
-            RootJoinNode]))
+            RootJoinNode]
+           [clojure.lang ExceptionInfo]))
 
 ;; See https://github.com/cerner/clara-rules/pull/451 for more info
 (tu/def-rules-test test-nodes-have-named-fns
@@ -55,4 +56,20 @@
                                          "-"
                                          (:id node)))
                         (-> node-fn str m/demunge (str/split #"/") last)))
-          (str "For node: " node " and node-fn: " node-fn)))))
+          (str "For node: " node " and node-fn: " node-fn)))))
+
+;; See https://github.com/cerner/clara-rules/issues/454 for more info
+(deftest test-query-node-requires-bindings-exist
+  (let [;; (defquery a-query
+        ;;   [:?b]
+        ;;   [?c <- ::a-fact-type])
+        query {:lhs [{:type ::a-fact-type
+                      :constraints []
+                      :args []
+                      :fact-binding :?c}]
+               :params #{:?b}
+               :name "a-query"}]
+    (tu/assert-ex-data {:expected-bindings #{:?b}
+                        :available-bindings #{:?c}
+                        :query "a-query"}
+                       (r/mk-session [query]))))

src/test/common/clara/test_simple_rules.cljc

@@ -194,3 +194,33 @@
     (is (has-fact? (:cold @side-effect-holder) (->Temperature -10 "MCI") ))
 
     (is (has-fact? (:subzero @side-effect-holder) (->Temperature -10 "MCI")))))
+
+(def-rules-test test-query-failure-when-provided-invalid-parameters
+
+  {:queries [temp-query [[:?t] [[Temperature (= ?t temperature)]]]]
+
+   :sessions [empty-session [temp-query] {}]}
+
+  (let [session (-> empty-session
+                    (insert (->Temperature 10 "MCI"))
+                    ;; Ensure retracting a nonexistent item has no ill effects.
+                    (retract (->Temperature 15 "MCI"))
+                    fire-rules)
+
+        expected-msg #"was not provided with the correct parameters"]
+
+    ;; passivity test
+    (is (= #{{:?t 10}}
+           (set (query session temp-query :?t 10))))
+
+    (is (thrown-with-msg? #?(:clj IllegalArgumentException :cljs js/Error)
+                          expected-msg
+                          (query session temp-query :?another-param 42)))
+
+    (is (thrown-with-msg? #?(:clj IllegalArgumentException :cljs js/Error)
+                          expected-msg
+                          (query session temp-query)))
+
+    (is (thrown-with-msg? #?(:clj IllegalArgumentException :cljs js/Error)
+                          expected-msg
+                          (query session temp-query :?t 42 :?another-param 42)))))