diff --git a/src/main/clojure/clara/rules/compiler.clj b/src/main/clojure/clara/rules/compiler.clj index f88a61ce..e2f8619a 100644 --- a/src/main/clojure/clara/rules/compiler.clj +++ b/src/main/clojure/clara/rules/compiler.clj @@ -983,16 +983,38 @@ "__" (gensym)) + ;; Insert the bindings from ancestors that are used in the negation + ;; in the NegationResult fact so that the [:not [NegationResult...]] + ;; condition can assert that the facts matching the negation + ;; have the necessary bindings. + ;; See https://github.com/cerner/clara-rules/issues/304 for more details + ;; and a case that behaves incorrectly without this check. + ancestor-bindings-in-negation-expr (set/intersection + (variables-as-keywords negation-expr) + ancestor-bindings) + + ancestor-bindings-insertion-form (into {} + (map (fn [binding] + [binding (-> binding + name + symbol)])) + ancestor-bindings-in-negation-expr) + + ancestor-binding->restriction-form (fn [b] + (list '= (-> b name symbol) + (list b 'ancestor-bindings))) + modified-expression `[:not {:type ~(if (compiling-cljs?) 'clara.rules.engine/NegationResult 'clara.rules.engine.NegationResult) - :constraints [(~'= ~gen-rule-name ~'gen-rule-name)]}] - - + :constraints [(~'= ~gen-rule-name ~'gen-rule-name) + ~@(map ancestor-binding->restriction-form + ancestor-bindings-in-negation-expr)]}] generated-rule (cond-> {:name gen-rule-name :lhs (concat previous-expressions [negation-expr]) - :rhs `(clara.rules/insert! (eng/->NegationResult ~gen-rule-name))} + :rhs `(clara.rules/insert! (eng/->NegationResult ~gen-rule-name + ~ancestor-bindings-insertion-form))} ;; Propagate properties like salience to the generated production. (:props production) (assoc :props (:props production)) diff --git a/src/main/clojure/clara/rules/engine.cljc b/src/main/clojure/clara/rules/engine.cljc index 436979fd..cb7d68c2 100644 --- a/src/main/clojure/clara/rules/engine.cljc +++ b/src/main/clojure/clara/rules/engine.cljc @@ -43,12 +43,12 @@ (do ;; A marker interface to identify internal facts. (definterface ISystemFact) - (defrecord NegationResult [gen-rule-name] + (defrecord NegationResult [gen-rule-name ancestor-bindings] ISystemFact)) :cljs (do - (defrecord NegationResult [gen-rule-name]) + (defrecord NegationResult [gen-rule-name ancestor-bindings]) ;; Make NegationResult a "system type" so that NegationResult ;; facts are special-cased when matching productions. This serves ;; the same purpose as implementing the ISystemFact Java interface diff --git a/src/test/clojure/clara/test_rules.clj b/src/test/clojure/clara/test_rules.clj index 6a20dfe7..46247440 100644 --- a/src/test/clojure/clara/test_rules.clj +++ b/src/test/clojure/clara/test_rules.clj @@ -475,6 +475,21 @@ (fire-rules))] (is (= [{:?l "MCI"}] (query end-session nested-negation-with-prior-bindings))) + (is (no-system-types? end-session))) + + ;; Match the nested negation for location ORD but not for MCI. + ;; See https://github.com/cerner/clara-rules/issues/304 + (let [end-session (-> s-with-nested + (insert + (->WindSpeed 10 "MCI") + (->Temperature 10 "MCI") + (->Cold 20) + (->WindSpeed 20 "ORD") + (->Temperature 20 "ORD")) + (fire-rules))] + + (is (= [{:?l "ORD"}] + (query end-session nested-negation-with-prior-bindings))) (is (no-system-types? end-session))))) (deftest test-complex-negation-custom-type @@ -2169,6 +2184,7 @@ (insert (->Cold 10) (->Hot 10)) (fire-rules) (query q)))] + (is (= (session->results (mk-session [r q])) ;; Validate that equal salience is handled correctly by ;; the activation-group-sort-fn when the user provides