[rules-users] (no subject)

[Jason Smith]

SUMMARY:
I am getting a NPE when using modify() with a JavaBean that implements PropertyChangeSupport.

I have included the original source and a proposed fix.  Stack trace is also included.

This applies to Drools 5.0.1, and has been verified to also be a problem for 5.1.0.M1.


DETAIL:

I have a rule that looks like this:


rule "Constrain to owner or published to public"

when

  $fact : ListFact(

  path not matches ".*owner.*",

  noopResponse == true

)

then

  modify($fact)

  {

    setPath( "[owner/username='' or publishlist/publish='All Users']")

  }

end


The $fact is a JavaBean that implements PropertyChangeSupport, and I've verified that everything is working correctly at the bean level.  When I run this rule, I get this exception:

Caused by: java.lang.NullPointerException:
        at org.drools.common.EqualityKey.removeFactHandle(EqualityKey.java:109)
        at org.drools.common.AbstractWorkingMemory.update(AbstractWorkingMemory.java:1435)
        at org.drools.common.AbstractWorkingMemory.update(AbstractWorkingMemory.java:1350)
        at org.drools.common.AbstractWorkingMemory.propertyChange(AbstractWorkingMemory.java:1577)
        at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:339)
        at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:276)
        at com.infotrustgroup.rules.RestFact.setPath(RestFact.java:264)
        at com.infotrustgroup.facts.ParametersFact.setPath(ParametersFact.java:105)
        at com.infotrustgroup.facts.ParametersFact.setPath(ParametersFact.java:74)
        at com.infotrustgroup.what.Rule_Constrain_to_owner_or_published_to_public_0.consequence(Rule_Constrain_to_owner_or_published_to_public_0.java:12)
        at com.infotrustgroup.what.Rule_Constrain_to_owner_or_published_to_public_0ConsequenceInvoker.evaluate(Rule_Constrain_to_owner_or_published_to_public_0ConsequenceInvoker.java:23)
        at org.drools.common.DefaultAgenda.fireActivation(DefaultAgenda.java:934)
        ... 38 more

The offending line is marked below.  It looks like maybe "this.instances" is null in the "else" clause.

    public void removeFactHandle(final InternalFactHandle handle) {
        if ( this.handle == handle ) {
            if ( this.instances == null ) {
                this.handle = null;
            } else {
                this.handle = (InternalFactHandle) this.instances.remove( 0 );
                if ( this.instances.isEmpty() ) {
                    this.instances = null;
                }
            }
        } else {
            ***this.instances.remove( handle );***
            if ( this.instances.isEmpty() ) {
                this.instances = null;
            }
        }
    }

It looks like someone else ran into this problem before, but only fixed it for the "if" portion, not the "else" portion.  I modified the code as follows, and this seems to fix the NPE.  Basically, I followed the pattern from the "if" portion.

    public void removeFactHandle(final InternalFactHandle handle) {
        if ( this.handle == handle ) {
            if ( this.instances == null ) {
                this.handle = null;
            } else {
                this.handle = (InternalFactHandle) this.instances.remove( 0 );
                if ( this.instances.isEmpty() ) {
                    this.instances = null;
                }
            }
        } else {
   if(this.instances == null)
   {
    this.handle = null;
   }
   else
   {
    this.instances.remove( handle );
    if ( this.instances.isEmpty() ) {
     this.instances = null;
    }
   }
        }
    }
Please read around the formatting.  Can someone verify if this is a correct fix, and if it should be applied to the codebase?  It seems to work for me.  It does not break any unit tests in drools-core 5.0.1 either.

Jason Smith
Software Engineer
InfoTrust Group, Inc.
500 Discovery Parkway, Suite 200
Superior, CO 80027
Office 303-627-6571
Fax 303-666-6711
Email jsmith at infotrustgroup.com<mailto:jsmith at infotrustgroup.com>
WEB www.infotrustgroup.com<http://www.infotrustgroup.com/>
This e-mail and all information included herein do not constitute a legal agreement accorded by INFOTRUST GROUP and its affiliates and subsidiaries.  All legal agreements must be formulated in writing by a legal representative of INFOTRUST GROUP. This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed.  If you have received this e-mail by mistake, please inform us and destroy this e-mail and any documents it might contain.  Please note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. Finally, the recipient should check this email and any attachments for the presence of viruses. The company accepts no liability for any damage caused by any virus transmitted by this email.  Thank you for your cooperation.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/rules-users/attachments/20091123/e2414224/attachment.html