oracle-samples/clara-rules

Left-retract on ProductionNode causes excessive retractions for tokens with both pending activations and logical insertions

Closed this issue · 4 comments

In the implementation of left-retract on ProductionNode [1] we currently take every token that is being retracted, attempt to remove a corresponding pending activation, and then attempt to retract a logical insertion that was based on that token. As a result a single retracted token can cause both a pending activation and a previous logical insertion to be removed. From a user-facing perspective, in scenarios where one of a fact should be retracted two could be retracted instead. To give a concrete example, I'd expect the following test to pass, but it doesn't:

(deftest test-remove-pending-activation-with-equal-previous-insertion
  (let [lousy-weather-rule (dsl/parse-rule [[?cw <- ColdAndWindy]]
                                           (insert! (->LousyWeather)))

        lousy-weather-query (dsl/parse-query [] [[LousyWeather]])

        empty-session (mk-session [lousy-weather-rule lousy-weather-query] :cache false)

        end-session (-> empty-session
                        (insert (->ColdAndWindy 10 10))
                        fire-rules
                        (insert (->ColdAndWindy 10 10))
                        (retract (->ColdAndWindy 10 10))
                        fire-rules)]

    (is (= (query end-session lousy-weather-query)
           [{}]))))
FAIL in (test-remove-pending-activation-with-equal-previous-insertion) (test_rules.clj:5013)
expected: (= (query end-session lousy-weather-query) [{}])
  actual: (not (= () [{}]))
nil
  1. https://github.com/cerner/clara-rules/blob/0.13.0-RC5/src/main/clojure/clara/rules/engine.cljc#L319

I have a draft of a fix for this at WilliamParker@7c78128

@WilliamParker I had a few comments there. However, overall this was a sneaky issue and a good find.

I have created a PR that addresses this issue at #252

The above PR has been merged, which resolves this issue. Closing.