Bug 724450 (BRMS-395)

Workaround Description: Removed: Do you have two choices:

1) If you use a "eval" in the first sentence, it's work:
rule "test"
when
//now the sentence below with "eval"
DroolsTest( eval(arg == (1 + 1) ));
(or eval(true); eval(true);)
then
System.out.println( "it works!" );
end

2)Creating a new rule for the second part of your conditional "OR" Added: Two choices:

1) If you use a "eval" in the first sentence, it's work:
rule "test"
when
//now the sentence below with "eval"
DroolsTest( eval(arg == (1 + 1) ));
(or eval(true); eval(true);)
then
System.out.println( "it works!" );
end

2)Creating a new rule for the second part of your conditional "OR"

Link: Added: This issue related JBRULES-2203

I talked to Porcelli about this issue, which made some tests on the parser. Apparently the Parser and TreeWalker are generating the data (descriptors) correctly.

Attached here two prints about this (thanks Porcelli).

Attachment: Added: debug1.png
Attachment: Added: debug2.png

Can you please show us a real-world rule that reproduces this issue (from customer) because the example rule even tho exposes a possible issue with drools is really such bad practice that  it's worth more to investigate into providing a better pattern(s) for customer first.

Below a real rule, from Customer:

rule "0701 - Guar Date Invalid"
dialect "mvel"
when
   ContinueProcessing( continueProcess == ( true ) )
   // it 's work if change to ->   cp : ContinueProcessing( eval( cp.isContinueProcess() )
   ( LenderLoan( eval( isBlank(guaranteeDate) )) or LenderLoan( eval( isStringDateNotAfter(guaranteeDate, "19651107") )) )
then
   LMError fact0 = new LMError();
   fact0.setCode( "0701" );
   insert(fact0 );
   System.out.println("Fired Rule: " + drools.getRule().getName());
end


function boolean isBlank(String arg) {    
   if(arg == null)
     return true;
   else 
    return (arg.trim().length() < 1);
}


function boolean isStringDateNotAfter(String inDate, String afterDate) {
   
 if (inDate == null)
      return true;
   
    //set the format to use as a constructor argument
    SimpleDateFormat dateFormat = new   SimpleDateFormat("yyyymmdd");
    String sDate = afterDate;
    Date d; 
    Date dIn;

  try {

    d = dateFormat.parse(sDate);
      dIn = dateFormat.parse(inDate);
   }
    catch (ParseException pe) {
      return true;
    }

    if (inDate.trim().length() != dateFormat.toPattern().length())
      return true;

    dateFormat.setLenient(false);
    
    try {
      //parse the inDate parameter
      dateFormat.parse(inDate.trim());
    }
    catch (ParseException pe) {
      return true;
    }
    
    boolean dTest = dIn.after(d);
 
    return  !dTest;
}

Thanks Alessandro, I have placed a comment on your support ticket on this.

Here is this comment: 
Rule from BRMS-935:

rule "0701 - Guar Date Invalid"
dialect "mvel"
when
   ContinueProcessing( continueProcess == ( true ) )
   // it 's work if change to ->   cp : ContinueProcessing( eval( cp.isContinueProcess() )
   ( LenderLoan( eval( isBlank(guaranteeDate) )) or LenderLoan( eval( isStringDateNotAfter(guaranteeDate, "19651107") )) )
then
   LMError fact0 = new LMError();
   fact0.setCode( "0701" );
   insert(fact0 );
   System.out.println("Fired Rule: " + drools.getRule().getName());
end

where isBlank and isStringDateNotAfter are functions defined in the drl.

Alessandro, this particular very unusual use of evals + or in the LHS pattern does expose an issue with the evaluation of the pattern in Drools. To fix the issue will require a big amount of time as it's not something easy to fix. This again is very unusual pattern, and if this was something common, it would have been exposed imo very long time ago.

The pattern written by the customer is not a good one. First of all, functions are designed to be used in the RHS of the rules. Currently, yes the only way
to call functions in the LHS is to call them inside an eval ( ... ) as customer is doing. This is something we are working on on changing in the future. In order to use "reusable functions" in the LHS you could have them as part of the fact model (or some global for example) or the better way would be to write a custom operator (see http://blog.athico.com/2010/06/creating-pluggable-oprators.html for example). Writing custom operators for isBlank and isStringDatNotAfter would be the ideal way for the customer to do, however writing these operators is not very easy, but the good thing is that we can help them write them if needed. 

Again, the use of eval in general should be avoided where possible and I think in this case it is indeed possible and should be done. Statements inside eval are not 
executed by the engine, but Java code. As such the engine cannot perform any optimizations or indexing. Using eval also in some cases leads to bad design of rules, for example to not having to properly notify the engine of changing object properties (modify blocks). 

So IMO the way to approach this customer issue is to explain to them that they have hit a very rare case where drools does fail to evaluate the LHS, however this case is so uncommon, as well as badly designed that in their best interest is not for us to give them a patch which makes their rule work (even though this is the long-term goal and I'll work on it), however for us to help them redesign their pattern given the info above and help them write the custom operators in the case they cannot figure it out right away.

Understood! Thanks Tiho

Release Notes Text: Added: resolved in 5.2

Hi Tihomir,

For the release notes, how was this resolved?

Thanks

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
https://bugzilla.redhat.com/show_bug.cgi?id=724450

When using the conditional 'or' element in the left handside of rule with a return value restriction the NullPointerException exception is thrown.

    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -1,3 +1 @@
-https://bugzilla.redhat.com/show_bug.cgi?id=724450
+When using the conditional 'or' element in the left handside of rule with a return value restriction the NullPointerException exception is thrown. This issue has been resolved.-
-When using the conditional 'or' element in the left handside of rule with a return value restriction the NullPointerException exception is thrown.

    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -1 +1 @@
-When using the conditional 'or' element in the left handside of rule with a return value restriction the NullPointerException exception is thrown. This issue has been resolved.+<remark>Need info from Tihomir - How was this issue resolved?</remark></para><para>When using the conditional 'or' element in the left handside of rule with a return value restriction the NullPointerException exception is thrown.

    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -1 +1 @@
-<remark>Need info from Tihomir - How was this issue resolved?</remark></para><para>When using the conditional 'or' element in the left handside of rule with a return value restriction the NullPointerException exception is thrown.+<remark>Need info from Tihomir - Was anything changed for this one or is the resolution that we helped the customer redesign their rules? Thanks.</remark></para><para>When using the conditional 'or' element in the left handside of rule with a return value restriction the NullPointerException exception was thrown.

    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -1 +1 @@
-<remark>Need info from Tihomir - Was anything changed for this one or is the resolution that we helped the customer redesign their rules? Thanks.</remark></para><para>When using the conditional 'or' element in the left handside of rule with a return value restriction the NullPointerException exception was thrown.+<remark>Need info from Tihomir - Was anything changed for this one or is the resolution that we helped the customer redesign their rules? Thanks.</remark></para><para>When using the conditional 'or' element in the left handside of rules with a return value restriction the NullPointerException exception was thrown.