Karthik
Karthik

Reputation: 149

Drools rule gets executed more times than expected

I'm new to Drools and am following along the book: Mastering JBoss Drools 6

Chapter 2 shows an example where we create a Customer and Order, classify customers based on their order size, give discounts and generate Coupons .

The example mentions:

Here are the Drools rules:

add-discount.drl

rule "High Range order - 10% discount"
    when
        $o: Order($lines: orderLines, orderLines.size >= 2, $customer: customer, discount == null)
        $c: Customer(category == Customer.Category.SILVER, this == $customer)
        forall(
            OrderLine(this memberOf $lines, $item: item)
            Item(this == $item, category == Item.Category.HIGH_RANGE)
        )
    then
        modify($o){
            setDiscount(new Discount(10.0))
        }
end

classify-customer-rules.drl

rule "Classify Customer by order size"
    when
        $o: Order( orderLines.size >= 5, $customer: customer )
        $c: Customer(this == $customer, category == Customer.Category.NA)
    then
        modify($c){
            setCategory(Customer.Category.SILVER)
        }
end

classify-items-rules.drl

rule "Classify Item - Low Range"
    when
        $i: Item(cost < 200, category == Category.NA)
    then
        $i.setCategory(Category.LOW_RANGE);
        update($i);
end

rule "Classify Item - Mid Range"
    when
        $i: Item(cost > 200 && cost < 500, category == Category.NA)
    then
        $i.setCategory(Category.MID_RANGE);
        update($i);
end

rule "Classify Item - High Range"
    when
        $i: Item(cost >= 500, category == Category.NA)
    then
        $i.setCategory(Category.HIGH_RANGE);
        update($i);
end

coupons-creation.drl

rule "Create Coupons for silver customer"
    when
        $o: Order($customer: customer)
        $c: Customer(this == $customer, category == Customer.Category.SILVER)
    then
        Coupon x = new Coupon($c, $o, Coupon.CouponType.POINTS);
        System.out.println(x);
        insert(x);
end

The number of rules executed should be 8. 5 items to classify (5 rules) + 1 customer to classify (1 rule) + 1 discount to add (1 rule) + 1 coupon to create (1 rule)

But the actual number of rules fired is 9. Here is the test I used to check.

@Test
    void orderDiscountTest() {
        KieSession kieSession = Util.getDefaultKieSession();
        Order o = ModelFactory.getOrderWithFiveHighRangeItems();

        kieSession.insert(o.getCustomer());
        kieSession.insert(o.getOrderLines().get(0));
        kieSession.insert(o.getOrderLines().get(1));
        kieSession.insert(o.getOrderLines().get(2));
        kieSession.insert(o.getOrderLines().get(3));
        kieSession.insert(o.getOrderLines().get(4));
        kieSession.insert(o.getOrderLines().get(0).getItem());
        kieSession.insert(o.getOrderLines().get(1).getItem());
        kieSession.insert(o.getOrderLines().get(2).getItem());
        kieSession.insert(o.getOrderLines().get(3).getItem());
        kieSession.insert(o.getOrderLines().get(4).getItem());
        kieSession.insert(o);

        int fired = kieSession.fireAllRules();

        assertEquals(8, fired);
        assertEquals(Customer.Category.SILVER, o.getCustomer().getCategory());
        assertNotNull(o.getDiscount());
        assertEquals(10.0, o.getDiscount().getPercentage());
        assertEquals(Item.Category.HIGH_RANGE, o.getOrderLines().get(0).getItem().getCategory());
        assertEquals(Item.Category.HIGH_RANGE, o.getOrderLines().get(1).getItem().getCategory());
        assertEquals(Item.Category.HIGH_RANGE, o.getOrderLines().get(2).getItem().getCategory());
        assertEquals(Item.Category.HIGH_RANGE, o.getOrderLines().get(3).getItem().getCategory());
        assertEquals(Item.Category.HIGH_RANGE, o.getOrderLines().get(4).getItem().getCategory());

        Collection<Coupon> coupons = Util.getFactsFromSession(kieSession, Coupon.class);
        coupons.forEach(x -> System.out.println(x));
        assertEquals(1, coupons.size());
    }

The coupon creation rule is fired twice. I do not understand why. I checked the number of coupons created after firing the rule like so:

Collection<Coupon> coupons = Util.getFactsFromSession(kieSession, Coupon.class);
coupons.forEach(x -> System.out.println(x));

and I also have a print statement in the coupon creation rule.

This is what I get:

12:54:47.607 [main] DEBUG org.drools.core.impl.KnowledgeBaseImpl - Starting Engine in PHREAK mode
12:54:47.730 [main] DEBUG org.drools.core.common.DefaultAgenda - State was INACTIVE is nw FIRING_ALL_RULES
12:54:47.732 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:54:47.795 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
Coupon created com.example.droolstut.model.Coupon@b14cb989
12:54:47.797 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:54:47.797 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:54:47.799 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:54:47.806 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
Coupon created com.example.droolstut.model.Coupon@424f842a
12:54:47.806 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:54:47.806 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:54:47.806 [main] DEBUG org.drools.core.common.DefaultAgenda - State was FIRING_ALL_RULES is nw HALTING
12:54:47.806 [main] DEBUG org.drools.core.common.DefaultAgenda - State was HALTING is nw INACTIVE
Coupon found: com.example.droolstut.model.Coupon@424f842a
Coupon found: com.example.droolstut.model.Coupon@424f842a

So, it looks like the rule is executed twice, but only the second coupon is found post execution.

But, when I run the test in debug mode, the coupon rule is executed only once, and the total number of rules fired is 8 too.

Did I miss something? I'd appreciate any help.

EDIT 1:

I printed the order and customer after adding the not(Coupon(...)) condition. The item objects look different.

Order(orderId=1, date=Thu Jan 01 05:30:00 IST 1970, customer=Customer(customerId=1, age=40, name=John Doe, [email protected], category=SILVER), orderLines=[OrderLine(item=com.example.droolstut.model.Item@38c70399, quantity=1), OrderLine(item=com.example.droolstut.model.Item@5245e3d4, quantity=1), OrderLine(item=com.example.droolstut.model.Item@6bc4c40f, quantity=1), OrderLine(item=com.example.droolstut.model.Item@8543a44a, quantity=1), OrderLine(item=com.example.droolstut.model.Item@9ec28485, quantity=1)], state=PENDING, discount=null) Customer(customerId=1, age=40, name=John Doe, [email protected], category=SILVER)
com.example.droolstut.model.Coupon@2753dad2
12:16:02.399 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:16:02.400 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:16:02.402 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
12:16:02.408 [main] DEBUG org.drools.core.common.DefaultAgenda - Fire Loop
Order(orderId=1, date=Thu Jan 01 05:30:00 IST 1970, customer=Customer(customerId=1, age=40, name=John Doe, [email protected], category=SILVER), orderLines=[OrderLine(item=com.example.droolstut.model.Item@5dc27ea9, quantity=1), OrderLine(item=com.example.droolstut.model.Item@77415ee4, quantity=1), OrderLine(item=com.example.droolstut.model.Item@90c03f1f, quantity=1), OrderLine(item=com.example.droolstut.model.Item@aa3f1f5a, quantity=1), OrderLine(item=com.example.droolstut.model.Item@c3bdff95, quantity=1)], state=PENDING, discount=Discount(percentage=10.0)) Customer(customerId=1, age=40, name=John Doe, [email protected], category=SILVER)
com.example.droolstut.model.Coupon@10e5ecb2

EDIT 2:

I am using lombok for getters/setter/Equals & hashCode. Once I remove the @EqualsAndHashCode annotation and add only the equals method(), it works as expected. As soon as I add the hashCode() method, it fails.

@Override
    public int hashCode() {
        int hash = 3;
        hash = 17 * hash + Objects.hashCode(this.orderId);
        hash = 17 * hash + Objects.hashCode(this.date);
        hash = 17 * hash + Objects.hashCode(this.customer);
        hash = 17 * hash + Objects.hashCode(this.orderLines);
        hash = 17 * hash + Objects.hashCode(this.state);
        hash = 17 * hash + Objects.hashCode(this.discount);
        return hash;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            return false;
        }
        if (getClass() != obj.getClass()) {
            return false;
        }
        final Order other = (Order) obj;
        if (!Objects.equals(this.orderId, other.orderId)) {
            return false;
        }
        if (!Objects.equals(this.date, other.date)) {
            return false;
        }
        if (!Objects.equals(this.customer, other.customer)) {
            return false;
        }
        if (!Objects.equals(this.orderLines, other.orderLines)) {
            return false;
        }
        if (this.state != other.state) {
            return false;
        }
        if (!Objects.equals(this.discount, other.discount)) {
            return false;
        }
        return true;
    }

Upvotes: 0

Views: 233

Answers (1)

Roddy of the Frozen Peas
Roddy of the Frozen Peas

Reputation: 15190

By the time a book on Drools gets published, it's out of date. Drools has fantastic documentation that you should be using as your primary source, not a very old book on a very old version of Drools. (Current version of Drools is Drools 8. Your book is for Drools 6.)

The other reason I recommend people not use books is because they tend to be awful. This one is no exception


The reason you're firing more rules than you think is because both "High Range order - 10% discount" forces a reevaluation of working memory, and "Create Coupons for silver customer" doesn't have anything to stop it from triggering multiple times.

"High Range..." calls 'modify' which re-fires all rules with the modified item. Since "Create Coupon..." remains valid to fire, it will trigger twice. (It'll fire once, and then once "High Range..." forces the reevaluation of working memory, it'll fire a second time.) You'll also see that there's twice as many coupons in working memory as you'd expect -- there will be 2 coupons instead of one because you fired the coupon rule twice.

To keep this from happening, you need to make "Create Coupon..." not valid to fire twice. (That is, once it's fired, it can't be triggered again.) You can do this, for example, by not allowing two of the same coupon. Or you could do this by only allowing one coupon. Depends on your use case.

One way might be:

rule "Create Coupons for silver customer"
when
  $o: Order($customer: customer)
  $c: Customer(category == Customer.Category.SILVER)

  // don't fire if this coupon already exists; I guessed on the variable names
  not( Coupon( customer == $c, order == $o, type == Coupon.CouponType.POINTS) )
then
  Coupon x = new Coupon($c, $o, Coupon.CouponType.POINTS);
  insert(x);
end

In the Real World, you want to keep your rules simple. Don't stick the entire kitchen sink into a single KieBase. The single responsibility principle is key here. Your "Item classification" rules should be fired separately from your customer classification rules should be fired separately from your discount rules.

Also calling update is a great way to get yourself into infinite loops and greatly increase your memory/cpu overhead. At my last company it was considered not only a code smell and bad practice, but actually outright banned (we checked for it in code reviews.)

Upvotes: 2

Related Questions