Skip to content

Commit

Permalink
ISSUE-352: Durability fails to deserialize rules containing condition…
Browse files Browse the repository at this point in the history
…s with an empty constraint (#353)
  • Loading branch information
EthanEChristian authored and WilliamParker committed Oct 27, 2017
1 parent 6b83688 commit de62503
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 9 deletions.
10 changes: 8 additions & 2 deletions src/main/clojure/clara/rules/compiler.clj
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@
(throw (ex-info (str "Malformed variable binding for " variables ". No associated value.")
{:variables (map keyword variables)})))

(if binds-variables?

(cond
binds-variables?
;; Bind each variable with the first value we encounter.
;; The additional equality checks are handled below so which value
;; we bind to is not important. So an expression like (= ?x value-1 value-2) will
Expand All @@ -287,8 +287,14 @@
compiled-rest)
)

;; A contraint that is empty doesn't need to be added as a check,
;; simply move on to the rest
(empty? exp)
compiled-rest

;; No variables to unify, so simply check the expression and
;; move on to the rest.
:else
`(if ~exp ~compiled-rest nil))))))


Expand Down
23 changes: 20 additions & 3 deletions src/main/clojure/clara/rules/durability/fressian.clj
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,23 @@
(read [_ rdr tag component-count]
(read-with-meta rdr #(apply list %))))}}

"clj/emptylist"
{:class clojure.lang.PersistentList$EmptyList
:writer (reify WriteHandler
(write [_ w o]
(let [m (meta o)]
(do
(.writeTag w "clj/emptylist" 1)
(if m
(.writeObject w m)
(.writeNull w))))))
:readers {"clj/emptylist"
(reify ReadHandler
(read [_ rdr tag component-count]
(let [m (read-meta rdr)]
(cond-> '()
m (with-meta m)))))}}

"clj/aseq"
{:class clojure.lang.ASeq
:writer (reify WriteHandler
Expand Down Expand Up @@ -264,7 +281,7 @@
(reify ReadHandler
(read [_ rdr tag component-count]
(read-with-meta rdr #(into {} %))))}}

"clj/treeset"
{:class clojure.lang.PersistentTreeSet
:writer (reify WriteHandler
Expand All @@ -289,7 +306,7 @@
(if m
(with-meta s m)
s))))}}

"clj/treemap"
{:class clojure.lang.PersistentTreeMap
:writer (reify WriteHandler
Expand Down Expand Up @@ -347,7 +364,7 @@
(let [s (symbol (.readObject rdr) (.readObject rdr))
m (read-meta rdr)]
(cond-> s
m (with-meta m)))))}}
m (with-meta m)))))}}

"clj/record"
{:class clojure.lang.IRecord
Expand Down
7 changes: 6 additions & 1 deletion src/test/clojure/clara/durability_rules.clj
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@

(defrule find-wind-speeds-without-temp
"Rule using NegationNode"
[?w <- WindSpeed (= ?loc location)]
[?w <- WindSpeed
;; Empty constraint and a constraint containing an empty list to test serializing an EmptyList,
;; see Issue 352 for more information
()
(not= this ())
(= ?loc location)]
[:not [Temperature (= ?loc location)]]
=>
(insert! (->UnpairedWindSpeed ?w)))
Expand Down
48 changes: 45 additions & 3 deletions src/test/common/clara/test_dsl.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
query]]
[clara.rules.accumulators :as acc]
[clara.rules.testfacts :refer [->Temperature ->Cold ->WindSpeed
->ColdAndWindy map->FlexibleFields]]
->ColdAndWindy map->FlexibleFields
->First ->Second ->Third ->Fourth]]
[clojure.test :refer [is deftest run-tests testing use-fixtures]]
[clara.rules.accumulators]
[schema.test :as st])
Expand All @@ -19,7 +20,11 @@
Cold
WindSpeed
ColdAndWindy
FlexibleFields]))
FlexibleFields
First
Second
Third
Fourth]))

:cljs
(ns clara.test-dsl
Expand All @@ -34,7 +39,11 @@
->Temperature Temperature
->Cold Cold
->WindSpeed WindSpeed
->ColdAndWindy ColdAndWindy]]
->ColdAndWindy ColdAndWindy
->First First
->Second Second
->Third Third
->Fourth Fourth]]
[clara.rules.accumulators :as acc]
[clara.tools.testing-utils :as tu]
[cljs.test]
Expand Down Expand Up @@ -440,3 +449,36 @@
"Validate that the ?t binding from the previous accumulator result is used, rather
than the binding in the ColdAndWindy condition that would create a ?t binding if one were
not already present"))

;; Test for https://github.com/cerner/clara-rules/issues/352
;; A test to validate the construction of rules containing empty constraints
(def-rules-test test-condition-with-empty-constraint
{:rules [rule-without-constraint [[[First]]
(swap! side-effect-holder conj :no-constraints)]
rule-with-an-empty [[[Second ()]]
(swap! side-effect-holder conj :one-empty-constraint)]
rule-with-two-empty [[[Third () ()]]
(swap! side-effect-holder conj :two-empty-constraint)]
rule-with-empty-and-non-empty [[[Fourth () (not= this ())]]
(swap! side-effect-holder conj :one-empty-one-populated)]
rule-with-non-empty-and-empty [[[Cold (not= this ()) ()]]
(swap! side-effect-holder conj :one-populated-one-empty)]]
:sessions [empty-session [rule-without-constraint
rule-with-an-empty
rule-with-two-empty
rule-with-empty-and-non-empty
rule-with-non-empty-and-empty] {}]}
(reset! side-effect-holder [])
(let [session (-> empty-session
(insert (->First)
(->Second)
(->Third)
(->Fourth)
(->Cold -10))
fire-rules)]
(is (= (frequencies @side-effect-holder)
(frequencies [:no-constraints
:one-empty-constraint
:two-empty-constraint
:one-empty-one-populated
:one-populated-one-empty])))))

0 comments on commit de62503

Please sign in to comment.