Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ActivationRuleGroup implementation #122

Closed
SeanSWatkins opened this issue Dec 1, 2017 · 12 comments
Closed

Add ActivationRuleGroup implementation #122

SeanSWatkins opened this issue Dec 1, 2017 · 12 comments
Assignees
Milestone

Comments

@SeanSWatkins
Copy link

Hey everyone

Currently, composite rules work only if all the rules return true.

Is modifying this behaviour outside of the scope of easy-rules? I'd just like to know, if it is something that would be of interest, I would love to contribute.

My use case is such that I am using the easy-rules for form field validations (which is work beautifully for). The only issue I have is that I would like composite rules to fire is any of the internal rules return true vs all of them.

It might also be an interesting idea to allow the action to be fired on the reverse of a rule fire (i.e. allow a negative return to trigger the action - but perhaps this is for another issue)

@fmbenhassine
Copy link
Member

Hi,

This is indeed an interesting use case. Another user asked for a custom composite rule (see #108).

The issue is that I'm not sure these custom implementations should provided out of the box. Not even the current CompositeRule actually! I should have left it outside easy rules (may be in a tutorial as an example or something), but not in the core library.

There are a lot of custom use cases for a "composite" rule:

I am not sure we should end up with all of them inside the library. Do you see?

It might also be an interesting idea to allow the action to be fired on the reverse of a rule fire (i.e. allow a negative return to trigger the action - but perhaps this is for another issue)

This is related to #107 . I don't exclude adding this feature to easy rules.

@khandelwalankit
Copy link

khandelwalankit commented Dec 12, 2017

The issue is that I'm not sure these custom implementations should provided out of the box. Not even the current CompositeRule actually! I should have left it outside easy rules (may be in a tutorial as an example or something), but not in the core library.

@benas: I agree that CompositeRule has varied use cases and having it in core library limits the users to create their on set of conditions on composite rule. Instead i would suggest to have a RuleGroup feature. Each Rule can belong to one or many RuleGroup. Each RuleGroup can have its own condition to operate upon. By default each Rule belongs to a default group and it executed on firing rules of default group.

For example RuleGroup : Avengers
Rule1 Name: BlackPanther
Rule2 Name: IronMan
Rule3 Name: Thor
Rule4 Name: Hulk
Rule5 Name: Wasp

RuleGroup Avengers Condition : (BlackPanther||Wasp) && IronMan && Thor && Hulk

So each user can form its own set of CompositeRules with different conditions. Let me know what you think about it.

@SeanSWatkins
Copy link
Author

@benas Thanks for the feedback

I have been thinking about this quite extensively since I wrote the question.

I've used the current CompositeRule Implementation to create the use case I mentioned, which works pretty well. The only issue I had was that I wanted to know which Rule's actually fired - so I added the names of the Triggered Rule's to a list which I then compared in the evaluate method. (I can provide a code snippet if my explanation is not clear)
I agree that perhaps the CompositeRule should have been an example as the API is simple enough to be very clear.

With regards to a negation rule, wrapping a rule or composite rule in which returns the opposite boolean value is incredibly easy to implement so that is probably unnecessary as well. (Again, I can provide a code snippet need be)

As another note - I am using this lib with Kotlin - so creating a wrapper class whereby you pass the rule in as a function makes the change even more simple.

@fmbenhassine
Copy link
Member

@khandelwalankit

I agree that CompositeRule has varied use cases and having it in core library limits the users to create their on set of conditions on composite rule

Well, it is not that limiting, the composite rule is just 4 lines of code for the condition and 4 lines for the action 😄 One can always write another rule with custom behaviour. The point is that since I've added it to the core library, everyone now would like the to add its own variant to the core. If it was outside the library as an example, no one would do it.

Instead i would suggest to have a RuleGroup feature

good idea, a bit like drools activation group. I might consider adding it to the library but for next major release (not minor ones).

@fmbenhassine
Copy link
Member

@SeanSWatkins

The only issue I had was that I wanted to know which Rule's actually fired - so I added the names of the Triggered Rule's to a list which I then compared in the evaluate method

I think a RuleListener is a good candidate for such requirement.

With regards to a negation rule, wrapping a rule or composite rule in which returns the opposite boolean value is incredibly easy to implement so that is probably unnecessary as well.

Yes, as said in #107, I was not aware some rules engines would provide it (like IBM ODM). Drools on the other hand does not provide an 'else' statement. And I much tend to agree with Drools on this, because production systems are not designed to have an 'else' part.

As another note - I am using this lib with Kotlin - so creating a wrapper class whereby you pass the rule in as a function makes the change even more simple.

Oh cool! I'm a kotlin newbie but I would love to see an example.

@fmbenhassine
Copy link
Member

@SeanSWatkins

I would like composite rules to fire is any of the internal rules return true vs all of them.

Can you provide the implementation of your composite rule with 'any' semantics?

May be we can make the current one configurable with the strategy (all/any)?

@fmbenhassine
Copy link
Member

Hi All,

I think I found a good tradeoff to all your great suggestions!

I will create a new module called easy-rules-support in which we can put all these supporting classes. The current CompositeRule will be deprecated, made abstract (evaluate and execute methods) and moved to the new module. Each extending class should define how to trigger composing rules:

  • UnitRuleGroup: a composite rule that acts as a unit (all or nothing logic). This is the current implementation of CompositeRule
  • ActivationRuleGroup: similar to drools, this is a composite rule that will activate any rule of its composing rules (xor logic)
  • FlowRuleGroup: a composite rule that has primary rule and a set of other rules. If the primary rule is satisfied, the rule group will be triggered. A good idea from @dagframstad in Added PathRule that evaluates it's composite rules if it's primary ru… #108

The naming XXXRuleGroup is inspired by what is suggested by @khandelwalankit . I like this idea of rule group. For the base class, I think CompositeRule is the perfect name since we are implementing the composite design pattern here.

I really like how we are brainstorming all these ideas and I would like to thank you upfront. Please let me know what do you think!

Kr
Mahmoud

@SeanSWatkins
Copy link
Author

SeanSWatkins commented Dec 18, 2017

@benas I think the easy-rule-support idea is wonderful. Will definitely keep the core library leaner which is great.

An example class written in Kotlin which allows someone to just define a function could look something like this:

class RuleWrapper(private val name: String,
        // This is dependent on function you want to use to process the rule
                  private val f: (JsonObject) -> Boolean) : Rule {

    override fun getPriority() = 1

    override fun getName() = name

    override fun getDescription() = "RuleWrapper for any rule"

    override fun evaluate(facts: Facts?): Boolean {
        if (facts == null) return true
        // Cast the domain to a JsonObject and pass it through to the function passed in
        val domain = facts["domain"] as JsonObject
        return f(domain)
    }

    override fun compareTo(other: Rule?): Int {
        // This is pretty much taken verbatim from the BasicRule implementation
        if (other == null) return -1
        return when {
            priority < other.priority -> -1
            priority > other.priority -> 1
            else -> name.compareTo(other.name)
        }
    }

    override fun execute(facts: Facts?) {
        // Perform whatever needs to be done
        facts?.put("result", "executed")
    }
}

Example Rule Creation

val newRuleWrapperExample = RuleWrapper("CheckObjectHasEasyRuleAndItIsTrue") {
    it.has("easy_rule") && it.get("easy_rule").asBoolean
}

This is obviously very simplistic and you can set the priority, description and name via passing in values as one might need.

In order to negate the rule, or rather make the rule execute on false rather than true a simple change could be made by changing return f(domain) -> return !f(domain) and the effect would be the same.

Lastly, an example of how I created a 'composite rule' that executes all the rules that fire is as follows (I will just include the evaluate and execute methods as everything else is identical to the current CompositeRule implementation:

override fun evaluate(facts: Facts?): Boolean {
        val firedRules = mutableListOf<String>()
        if (!rules.isEmpty()) {
            rules.forEach {
                if (it.evaluate(facts)) {
                    firedRules.add(it.name)
                }
            }
            if (firedRules.size > 0) {
                facts?.put("fired_rules", firedRules)
                return true
            }
        }
        return false
    }

 override fun execute(facts: Facts?) {
        val firedRulesList = facts?.get("fired_rules") as List<*>
        rules.forEach {
            if (firedRulesList.contains(it.name)) {
                it.execute(facts)
            }
        }
    }

This is also in Kotlin but it would work similarly in Java.

@fmbenhassine
Copy link
Member

fmbenhassine commented Jan 6, 2018

I think the easy-rule-support idea is wonderful. Will definitely keep the core library leaner which is great.

Cool, I started the work in the easy-rules-support branch:

  • the new module is created
  • The current CompositeRule is deprecated
  • A new abstract CompositeRule is created in the new module
  • A new rule UnitRuleGroup is added as a subclass of CompositeRule to implement the "all or nothing semantics" (see here). This is exactly the same as the previous composite rule implementation (that is deprecated in the core).

With this preparatory work, we can move forward and add other subclasses of CompositeRule to implement other semantics discussed earlier:

With theses planned additions, the easy-rules-support will provide 3 implementations of CompositeRule. Of course, the API is public, so users can implement their own requirement if needed.

@fmbenhassine
Copy link
Member

The easy-rules-support module has been merged to the master branch and will be part of v3.2. Now we have two implementations: UnitRuleGroup and ConditionalRuleGroup.

I'd just like to know, if it is something that would be of interest, I would love to contribute.

@SeanSWatkins Are you willing to contribute the ActivationRuleGroup implementation?

@fmbenhassine fmbenhassine changed the title Composite Rule Any vs All Add ActivationRuleGroup implementation Apr 8, 2018
@fmbenhassine fmbenhassine modified the milestones: v3.2, v3.3 Apr 8, 2018
@fmbenhassine
Copy link
Member

This has been planned for v3.3. I'm releasing v3.2 with the new module + UnitRuleGroup and ConditionalRuleGroup. The ActivationRuleGroup implementation would sort rules by salience/priority and apply the first rule that applies (xor logic).

@fmbenhassine fmbenhassine modified the milestones: v3.3, v3.2 Apr 9, 2018
@fmbenhassine
Copy link
Member

I finally managed to get this done in v3.2 which is now released (see here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants