diff --git a/project.clj b/project.clj index b8c5a47b..682e1fe2 100644 --- a/project.clj +++ b/project.clj @@ -6,7 +6,9 @@ :dependencies [[org.clojure/clojure "1.7.0"] [prismatic/schema "1.1.6"]] :profiles {:dev {:dependencies [[org.clojure/math.combinatorics "0.1.3"] - [org.clojure/data.fressian "0.2.1"]]} + [org.clojure/data.fressian "0.2.1"]] + :java-source-paths ["src/test/java"] + :global-vars {*warn-on-reflection* true}} :provided {:dependencies [[org.clojure/clojurescript "1.7.170"]]} :recent-clj {:dependencies [^:replace [org.clojure/clojure "1.9.0"] ^:replace [org.clojure/clojurescript "1.9.946"]]} diff --git a/src/main/clojure/clara/rules/compiler.clj b/src/main/clojure/clara/rules/compiler.clj index 52ce53e7..50ceae58 100644 --- a/src/main/clojure/clara/rules/compiler.clj +++ b/src/main/clojure/clara/rules/compiler.clj @@ -3,17 +3,11 @@ This is the Clara rules compiler, translating raw data structures into compiled versions and functions. Most users should use only the clara.rules namespace." (:require [clara.rules.engine :as eng] - [clara.rules.listener :as listener] - [clara.rules.platform :as platform] [clara.rules.schema :as schema] - [clojure.core.reducers :as r] - [clojure.reflect :as reflect] - [clojure.set :as s] [clojure.set :as set] [clojure.string :as string] [clojure.walk :as walk] - [schema.core :as sc] - [schema.macros :as sm]) + [schema.core :as sc]) (:import [clara.rules.engine ProductionNode QueryNode @@ -27,7 +21,6 @@ AccumulateNode AccumulateWithJoinFilterNode LocalTransport - LocalSession Accumulator ISystemFact] [java.beans @@ -179,10 +172,13 @@ ;; Iterate through the bean properties, returning tuples and the corresponding methods. (for [^PropertyDescriptor property (seq (.. java.beans.Introspector (getBeanInfo cls) - (getPropertyDescriptors)))] - - [(symbol (string/replace (.. property (getName)) #"_" "-")) ; Replace underscore with idiomatic dash. - (symbol (str "." (.. property (getReadMethod) (getName))))]))) + (getPropertyDescriptors))) + :let [read-method (.getReadMethod property)] + ;; In the event that there the class has an indexed property without a basic accessor we will simply skip + ;; the accessor as we will not know how to retrieve the value. see https://github.com/cerner/clara-rules/issues/446 + :when read-method] + [(symbol (string/replace (.getName property) #"_" "-")) ; Replace underscore with idiomatic dash. + (symbol (str "." (.getName read-method)))]))) (defn effective-type [type] (if (compiling-cljs?) @@ -765,7 +761,7 @@ ;; have any new facts as input. See: ;; https://github.com/cerner/clara-rules/issues/357 [#{} - (apply s/union (classify-variables constraints))] + (apply set/union (classify-variables constraints))] ;; It is not a negation, so simply classify variables. (classify-variables constraints)) @@ -775,10 +771,10 @@ (:result-binding leaf-condition) (conj (symbol (name (:result-binding leaf-condition))))) ;; All variables bound in this condition. - all-bound (s/union bound bound-with-result-bindings) + all-bound (set/union bound bound-with-result-bindings) ;; Unbound variables, minus those that have been bound elsewhere in this condition. - all-unbound (s/difference (s/union unbound-variables unbound) all-bound)] + all-unbound (set/difference (set/union unbound-variables unbound) all-bound)] {:bound all-bound :unbound all-unbound @@ -812,8 +808,8 @@ ;; Unsatisfied conditions remain, so find ones we can satisfy. (let [satisfied? (fn [classified-condition] - (clojure.set/subset? (:unbound classified-condition) - bound-variables)) + (set/subset? (:unbound classified-condition) + bound-variables)) ;; Find non-accumulator conditions that are satisfied. We defer ;; accumulators until later in the rete network because they @@ -821,8 +817,8 @@ ;; in the network are satisfied. satisfied-non-accum? (fn [classified-condition] (and (not (:is-accumulator classified-condition)) - (clojure.set/subset? (:unbound classified-condition) - bound-variables))) + (set/subset? (:unbound classified-condition) + bound-variables))) has-satisfied-non-accum (some satisfied-non-accum? remaining-conditions) @@ -834,16 +830,16 @@ (remove satisfied-non-accum? remaining-conditions) (remove satisfied? remaining-conditions)) - updated-bindings (apply clojure.set/union bound-variables + updated-bindings (apply set/union bound-variables (map :bound newly-satisfied))] ;; If no existing variables can be satisfied then the production is invalid. (when (empty? newly-satisfied) ;; Get the subset of variables that cannot be satisfied. - (let [unsatisfiable (clojure.set/difference - (apply clojure.set/union (map :unbound still-unsatisfied)) - bound-variables)] + (let [unsatisfiable (set/difference + (apply set/union (map :unbound still-unsatisfied)) + bound-variables)] (throw (ex-info (str "Using variable that is not previously bound. This can happen " "when an expression uses a previously unbound variable, " "or if a variable is referenced in a nested part of a parent " @@ -925,7 +921,7 @@ (conj constraint-bindings (:fact-binding condition)) constraint-bindings) - new-bindings (s/difference (variables-as-keywords (:constraints condition)) + new-bindings (set/difference (variables-as-keywords (:constraints condition)) parent-bindings) join-filter-bindings (if join-filter-expressions @@ -936,12 +932,12 @@ {:node-type node-type :condition condition :new-bindings new-bindings - :used-bindings (s/union cond-bindings join-filter-bindings)} + :used-bindings (set/union cond-bindings join-filter-bindings)} (seq env) (assoc :env env) ;; Add the join bindings to join, accumulator or negation nodes. - (#{:join :negation :accumulator} node-type) (assoc :join-bindings (s/intersection cond-bindings parent-bindings)) + (#{:join :negation :accumulator} node-type) (assoc :join-bindings (set/intersection cond-bindings parent-bindings)) accumulator (assoc :accumulator accumulator) @@ -949,7 +945,7 @@ join-filter-expressions (assoc :join-filter-expressions join-filter-expressions) - join-filter-bindings (assoc :join-filter-join-bindings (s/intersection join-filter-bindings parent-bindings))))) + join-filter-bindings (assoc :join-filter-join-bindings (set/intersection join-filter-bindings parent-bindings))))) (sc/defn ^:private add-node :- schema/BetaGraph "Adds a node to the beta graph." @@ -1028,8 +1024,8 @@ ;; 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) + (variables-as-keywords negation-expr) + ancestor-bindings) ancestor-bindings-insertion-form (into {} (map (fn [binding] @@ -1105,7 +1101,7 @@ {:keys [result-binding fact-binding]} expression - all-bindings (cond-> (s/union bindings (:used-bindings node)) + all-bindings (cond-> (set/union bindings (:used-bindings node)) result-binding (conj result-binding) fact-binding (conj fact-binding)) @@ -1626,9 +1622,9 @@ pending-id)) updated-edges (into {} (for [[dependent-id dependencies] node-deps] - [dependent-id (s/difference dependencies newly-satisfied-ids)]))] + [dependent-id (set/difference dependencies newly-satisfied-ids)]))] - (recur (s/difference pending-ids newly-satisfied-ids) + (recur (set/difference pending-ids newly-satisfied-ids) updated-edges (concat sorted-nodes newly-satisfied-ids)))))] diff --git a/src/test/clojure/clara/test_java_facts.clj b/src/test/clojure/clara/test_java_facts.clj new file mode 100644 index 00000000..7961ad78 --- /dev/null +++ b/src/test/clojure/clara/test_java_facts.clj @@ -0,0 +1,50 @@ +(ns clara.test-java-facts + (:require [clara.tools.testing-utils :as tu] + [clara.rules :as rules] + [clojure.test :refer [is deftest run-tests testing use-fixtures]] + [clara.rules.compiler :as com]) + (:import [clara.test.facts + BeanTestFact])) + +;; A test to demonstrate that a Pojo with indexed property accessors can be used as an alpha root in a session, +;; see https://github.com/cerner/clara-rules/issues/446 +(tu/def-rules-test test-basic-rule + {:rules [kansas-rule [[[BeanTestFact + (= ?locs locations) + (some #(= "Kansas" %) ?locs)]] + (rules/insert! "Kansas Exists")]] + :queries [string-query [[] [[?s <- String]]]] + + :sessions [empty-session [kansas-rule string-query] {}]} + + (let [locs (doto (make-array String 2) + (aset 0 "Florida") + (aset 1 "Kansas"))] + (let [session-strings (map :?s + (-> empty-session + (rules/insert (BeanTestFact. locs)) + (rules/fire-rules) + (rules/query string-query)))] + (is (= ["Kansas Exists"] session-strings))))) + +;; Using an indexed property accessor that doesn't have a standard accessor will throw exception as clara cannot resolve +;; the usage of the accessor. See https://github.com/cerner/clara-rules/issues/446 +(deftest test-indexed-property-accessor + (let [rule-using-unsupported-accessor {:ns-name (ns-name *ns*) + :lhs [{:type BeanTestFact :constraints [`(contains? ~'roadConditions "Slippery")]}] + :rhs `(rules/insert! "doesn't matter") + :name "rule-using-unsupported-accessor"}] + (try + (com/mk-session [[rule-using-unsupported-accessor]]) + (is false "An exception should be thrown") + (catch Exception e + (loop [exc e] + (cond + (re-find #"Failed compiling alpha node" (.getMessage exc)) + :success + + (.getCause exc) + (recur (.getCause exc)) + + :else + (is false "Exception didn't contain a message containing alpha node failure"))))))) \ No newline at end of file diff --git a/src/test/java/clara/test/facts/BeanTestFact.java b/src/test/java/clara/test/facts/BeanTestFact.java new file mode 100644 index 00000000..dcd2813a --- /dev/null +++ b/src/test/java/clara/test/facts/BeanTestFact.java @@ -0,0 +1,39 @@ +package clara.test.facts; + +/** + * A Java Pojo for the express purpose of testing the behavior of compilation and execution of rules. + * + * This class should not be included in the released artifact, if it did make it into a released artifact it should be + * ignored and not consumed as it may be moved/removed without warning to consumers. + */ +public class BeanTestFact { + private String[] locations; + private String[] roadConditions; + + public BeanTestFact(String[] locations) { + this.locations = locations; + } + + // Standard and Indexed property accessors + public void setLocations(String[] locations) { + this.locations = locations; + } + public String[] getLocations() { + return locations; + } + public void setLocations(int pos, String location) { + locations[pos] = location; + } + public String getLocations(int pos){ + return locations[pos]; + } + + // Partial Indexed property accessor, ie. no standard accessor + // See https://github.com/cerner/clara-rules/issues/446 for more details + public void setRoadConditions(int pos, String condition) { + roadConditions[pos] = condition; + } + public String getRoadConditions(int pos){ + return roadConditions[pos]; + } +}