[DROOLS-970] fix Date coercion when the constraint uses a declaration…

… or an instance field
apache · Nov 4, 2015 · cc9a5ea · cc9a5ea
1 parent b757717
commit cc9a5ea
Show file tree

Hide file tree

Showing 11 changed files with 295 additions and 194 deletions.
diff --git a/drools-compiler/src/main/java/org/drools/compiler/builder/impl/ClassHierarchyManager.java b/drools-compiler/src/main/java/org/drools/compiler/builder/impl/ClassHierarchyManager.java
@@ -190,7 +190,7 @@ public void inheritFields( PackageRegistry pkgRegistry,
                             TypeFieldDescr inheritedFlDescr = new TypeFieldDescr(
                                     name,
                                     new PatternDescr(
-                                            inspector.getFieldTypes().get(name).getName()));
+                                            inspector.getFieldType(name).getName()));
                             inheritedFlDescr.setInherited(!Modifier.isAbstract( inspector.getGetterMethods().get( name ).getModifiers() ));
 
                             if (!tDescr.getFields().containsKey(inheritedFlDescr.getFieldName()))
@@ -346,7 +346,7 @@ protected boolean mergeFields( String simpleSuperTypeName,
                             TypeFieldDescr inheritedFlDescr = new TypeFieldDescr(
                                     name,
                                     new PatternDescr(
-                                            inspector.getFieldTypes().get(name).getName()));
+                                            inspector.getFieldType(name).getName()));
                             inheritedFlDescr.setInherited(!Modifier.isAbstract(inspector.getGetterMethods().get(name).getModifiers()));
 
                             if (!fieldMap.containsKey(inheritedFlDescr.getFieldName()))

diff --git a/drools-compiler/src/main/java/org/drools/compiler/builder/impl/TypeDeclarationFactory.java b/drools-compiler/src/main/java/org/drools/compiler/builder/impl/TypeDeclarationFactory.java
@@ -119,7 +119,7 @@ protected void checkRedeclaration( AbstractClassTypeDeclarationDescr typeDescr,
                                     kbuilder.addBuilderResult(new TypeDeclarationError(typeDescr, "New declaration of "+typeDescr.getType().getFullName() +
                                                                                                   " does not include field " + existingFieldName ) );
                                 } else {
-                                    String fldType = cfi.getFieldTypes().get( existingFieldName ).getName();
+                                    String fldType = cfi.getFieldType( existingFieldName ).getName();
                                     fldType = TypeDeclarationUtils.toBuildableType( fldType, kbuilder.getRootClassLoader() );
                                     TypeFieldDescr declaredField = typeDescr.getFields().get( existingFieldName );
                                     if ( ! fldType.equals( type.getTypeClassDef().getField( existingFieldName ).getTypeName() ) ) {

diff --git a/drools-compiler/src/main/java/org/drools/compiler/rule/builder/PatternBuilder.java b/drools-compiler/src/main/java/org/drools/compiler/rule/builder/PatternBuilder.java
@@ -882,7 +882,8 @@ protected Constraint buildExpression( final RuleBuildContext context,
         if ( simple && // simple means also relDescr is != null
                 !ClassObjectType.Map_ObjectType.isAssignableFrom( pattern.getObjectType() ) &&
                 !ClassObjectType.Match_ObjectType.isAssignableFrom( pattern.getObjectType() ) ) {
-            return buildRelationalExpression(context, pattern, relDescr, expr, aliases);
+            String normalizedExpr = normalizeExpression( context, pattern, relDescr, expr );
+            return buildRelationalExpression(context, pattern, relDescr, normalizedExpr, aliases);
         }
 
         // Either it's a complex expression, so do as predicate
@@ -932,19 +933,14 @@ private String rewriteCompositeExpressions(RuleBuildContext context, Pattern pat
 
     private String normalizeExpression( RuleBuildContext context, Pattern pattern, RelationalExprDescr subDescr, String subExpr ) {
         String leftValue = findLeftExpressionValue(subDescr);
-        InternalReadAccessor extractor = getFieldReadAccessor( context, subDescr, pattern, leftValue, null, true );
-        if (extractor == null) {
-            return subExpr;
-        }
-
-        ValueType vtype = extractor.getValueType();
         String operator = subDescr.getOperator();
 
-        if (vtype == ValueType.DATE_TYPE) {
+        if ( isDateType( context, pattern, leftValue ) ) {
             String rightValue = findRightExpressionValue( subDescr );
-            FieldValue fieldValue = getFieldValue(context, vtype, rightValue);
+            FieldValue fieldValue = getFieldValue(context, ValueType.DATE_TYPE, rightValue);
             return fieldValue != null ? normalizeDate( fieldValue, leftValue, operator ) : subExpr;
         }
+
         if (operator.equals( "str" )) {
             String rightValue = findRightExpressionValue( subDescr );
             return normalizeStringOperator( leftValue, rightValue, new LiteralRestrictionDescr( operator,
@@ -953,10 +949,26 @@ private String normalizeExpression( RuleBuildContext context, Pattern pattern, R
                                                                                                 rightValue,
                                                                                                 LiteralRestrictionDescr.TYPE_STRING ) );
         }
+
         // resolve ambiguity between mvel's "empty" keyword and constraints like: List(empty == ...)
         return normalizeEmptyKeyword( subExpr, operator );
     }
 
+    private boolean isDateType( RuleBuildContext context, Pattern pattern, String leftValue ) {
+        Declaration declaration = pattern.getDeclarations().get( leftValue );
+        if (declaration != null) {
+            return declaration.getValueType() == ValueType.DATE_TYPE;
+        }
+
+        if (pattern.getObjectType() instanceof FactTemplateObjectType) {
+            return ( (FactTemplateObjectType) pattern.getObjectType() ).getFactTemplate().getFieldTemplate( leftValue ).getValueType() == ValueType.DATE_TYPE;
+        }
+
+        Class<?> clazz = ((ClassObjectType) pattern.getObjectType()).getClassType();
+        Class<?> fieldType = context.getPkg().getClassFieldAccessorStore().getFieldType(clazz, leftValue);
+        return fieldType != null && ValueType.isDateType( fieldType );
+    }
+
     protected Constraint buildRelationalExpression( final RuleBuildContext context,
                                                     final Pattern pattern,
                                                     final RelationalExprDescr relDescr,

diff --git a/drools-compiler/src/test/java/org/drools/compiler/integrationtests/DateCoercionTest.java b/drools-compiler/src/test/java/org/drools/compiler/integrationtests/DateCoercionTest.java
@@ -0,0 +1,164 @@
+/*
+ * Copyright 2015 JBoss Inc
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.drools.compiler.integrationtests;
+
+import org.junit.Test;
+import org.kie.api.io.ResourceType;
+import org.kie.api.runtime.KieSession;
+import org.kie.internal.utils.KieHelper;
+
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+public class DateCoercionTest {
+
+    @Test
+    public void testDateCoercionWithOr() {
+        // DROOLS-296
+        String drl = "import java.util.Date\n" +
+                     "global java.util.List list\n" +
+                     "declare DateContainer\n" +
+                     "     date: Date\n" +
+                     "end\n" +
+                     "\n" +
+                     "rule Init when\n" +
+                     "then\n" +
+                     "    insert(new DateContainer(new Date(0)));" +
+                     "end\n" +
+                     "\n" +
+                     "rule \"Test rule\"\n" +
+                     "when\n" +
+                     "    $container: DateContainer( date >= \"15-Oct-2013\" || date <= \"01-Oct-2013\" )\n" +
+                     "then\n" +
+                     "    list.add(\"working\");\n" +
+                     "end\n";
+
+        KieSession ksession = new KieHelper().addContent( drl, ResourceType.DRL )
+                                             .build()
+                                             .newKieSession();
+
+        List<String> list = new ArrayList<String>();
+        ksession.setGlobal( "list", list );
+        ksession.fireAllRules();
+        assertEquals( 1, list.size() );
+        assertEquals( "working", list.get( 0 ) );
+    }
+
+    @Test
+    public void testDateCoercionWithVariable() {
+        // DROOLS-970
+        String drl = "import java.util.Date\n" +
+                     "global java.util.List list\n" +
+                     "declare DateContainer\n" +
+                     "     date: Date\n" +
+                     "end\n" +
+                     "\n" +
+                     "rule Init when\n" +
+                     "then\n" +
+                     "    insert(new DateContainer(new Date(0)));" +
+                     "end\n" +
+                     "\n" +
+                     "rule \"Test rule\"\n" +
+                     "when\n" +
+                     "    $container: DateContainer( $date: date, $date <= \"01-Oct-2013\" )\n" +
+                     "then\n" +
+                     "    list.add(\"working\");\n" +
+                     "end\n";
+
+        KieSession ksession = new KieHelper().addContent( drl, ResourceType.DRL )
+                                             .build()
+                                             .newKieSession();
+
+        List<String> list = new ArrayList<String>();
+        ksession.setGlobal( "list", list );
+        ksession.fireAllRules();
+        assertEquals( 1, list.size() );
+        assertEquals( "working", list.get( 0 ) );
+    }
+
+    @Test
+    public void testDateCoercionWithInstanceVariable() {
+        // DROOLS-970
+        String drl = "import " + DateContainer.class.getCanonicalName() + "\n" +
+                     "import java.util.Date\n" +
+                     "global java.util.List list\n" +
+                     "rule Init when\n" +
+                     "then\n" +
+                     "    insert(new DateContainer(new Date(0)));" +
+                     "end\n" +
+                     "\n" +
+                     "rule \"Test rule\"\n" +
+                     "when\n" +
+                     "    $container: DateContainer( date <= \"01-Oct-2013\" )\n" +
+                     "then\n" +
+                     "    list.add(\"working\");\n" +
+                     "end\n";
+
+        KieSession ksession = new KieHelper().addContent( drl, ResourceType.DRL )
+                                             .build()
+                                             .newKieSession();
+
+        List<String> list = new ArrayList<String>();
+        ksession.setGlobal( "list", list );
+        ksession.fireAllRules();
+        assertEquals( 1, list.size() );
+        assertEquals( "working", list.get( 0 ) );
+    }
+
+    public static class DateContainer {
+        public final Date date;
+
+        public DateContainer( Date date ) {
+            this.date = date;
+        }
+    }
+
+    @Test
+    public void testDateCoercionWithNestedOr() {
+        // BZ-1253575
+        String drl = "import java.util.Date\n" +
+                     "global java.util.List list\n" +
+                     "declare DateContainer\n" +
+                     "     date: Date\n" +
+                     "end\n" +
+                     "\n" +
+                     "rule Init when\n" +
+                     "then\n" +
+                     "    insert(new DateContainer(new Date( 1439882189744L )));" +
+                     "end\n" +
+                     "\n" +
+                     "rule \"Test rule\"\n" +
+                     "when\n" +
+                     "    $container: DateContainer( (date >= \"19-Jan-2014\" && date <= \"03-Dec-2015\" ) || (date >= \"17-Dec-2016\" && date <= \"02-Jan-2017\" ) )\n" +
+                     "then\n" +
+                     "    list.add(\"working\");\n" +
+                     "end\n";
+
+        KieSession ksession = new KieHelper().addContent( drl, ResourceType.DRL )
+                                             .build()
+                                             .newKieSession();
+
+        List<String> list = new ArrayList<String>();
+        ksession.setGlobal( "list", list );
+        ksession.fireAllRules();
+        assertEquals( 1, list.size() );
+        assertEquals( "working", list.get( 0 ) );
+    }
+}
diff --git a/drools-compiler/src/test/java/org/drools/compiler/integrationtests/Misc2Test.java b/drools-compiler/src/test/java/org/drools/compiler/integrationtests/Misc2Test.java
@@ -4713,68 +4713,6 @@ public void testCollectAndAccumulate() {
         assertEquals( 3, list.get( 3 ) );
     }
 
-    @Test
-    public void testDateCoercionWithOr() {
-        // DROOLS-296
-        String drl = "import java.util.Date\n" +
-                     "global java.util.List list\n" +
-                     "declare DateContainer\n" +
-                     "     date: Date\n" +
-                     "end\n" +
-                     "\n" +
-                     "rule Init when\n" +
-                     "then\n" +
-                     "    insert(new DateContainer(new Date(0)));" +
-                     "end\n" +
-                     "\n" +
-                     "rule \"Test rule\"\n" +
-                     "when\n" +
-                     "    $container: DateContainer( date >= \"15-Oct-2013\" || date <= \"01-Oct-2013\" )\n" +
-                     "then\n" +
-                     "    list.add(\"working\");\n" +
-                     "end\n";
-
-        KnowledgeBase kbase = loadKnowledgeBaseFromString( drl );
-        StatefulKnowledgeSession ksession = kbase.newStatefulKnowledgeSession();
-
-        List<String> list = new ArrayList<String>();
-        ksession.setGlobal( "list", list );
-        ksession.fireAllRules();
-        assertEquals( 1, list.size() );
-        assertEquals( "working", list.get( 0 ) );
-    }
-
-    @Test
-    public void testDateCoercionWithNestedOr() {
-        // BZ-1253575
-        String drl = "import java.util.Date\n" +
-                     "global java.util.List list\n" +
-                     "declare DateContainer\n" +
-                     "     date: Date\n" +
-                     "end\n" +
-                     "\n" +
-                     "rule Init when\n" +
-                     "then\n" +
-                     "    insert(new DateContainer(new Date( 1439882189744L )));" +
-                     "end\n" +
-                     "\n" +
-                     "rule \"Test rule\"\n" +
-                     "when\n" +
-                     "    $container: DateContainer( (date >= \"19-Jan-2014\" && date <= \"03-Dec-2015\" ) || (date >= \"17-Dec-2016\" && date <= \"02-Jan-2017\" ) )\n" +
-                     "then\n" +
-                     "    list.add(\"working\");\n" +
-                     "end\n";
-
-        KnowledgeBase kbase = loadKnowledgeBaseFromString( drl );
-        StatefulKnowledgeSession ksession = kbase.newStatefulKnowledgeSession();
-
-        List<String> list = new ArrayList<String>();
-        ksession.setGlobal( "list", list );
-        ksession.fireAllRules();
-        assertEquals( 1, list.size() );
-        assertEquals( "working", list.get( 0 ) );
-    }
-
     @Test
     public void testMatchingEventsInStreamMode() {
         // DROOLS-338

diff --git a/drools-core/src/main/java/org/drools/core/base/BaseClassFieldWriter.java b/drools-core/src/main/java/org/drools/core/base/BaseClassFieldWriter.java
@@ -59,7 +59,7 @@ public BaseClassFieldWriter(final Class< ? > clazz,
         try {
             final ClassFieldInspector inspector = new ClassFieldInspector( clazz );
             this.index = inspector.getFieldNames().get( fieldName );
-            this.fieldType = inspector.getFieldTypes().get( fieldName );
+            this.fieldType = inspector.getFieldType( fieldName );
             this.valueType = ValueType.determineValueType( this.fieldType );
         } catch ( final Exception e ) {
             throw new RuntimeException( e );

diff --git a/drools-core/src/main/java/org/drools/core/base/ClassFieldAccessorCache.java b/drools-core/src/main/java/org/drools/core/base/ClassFieldAccessorCache.java
@@ -84,8 +84,7 @@ public boolean equals(Object obj) {
             if ( cls == null ) {
                 if ( other.cls != null ) return false;
             } else if ( !cls.equals( other.cls ) ) return false;
-            if ( event != other.event ) return false;
-            return true;
+            return event == other.event;
         }
 
     }
@@ -168,9 +167,9 @@ public BaseClassFieldReader getReadAccessor(AccessorKey key,
                                                     Class cls) {
             BaseClassFieldReader reader = this.readCache.get( key );
             if ( reader == null ) {
-                reader = ClassFieldAccessorFactory.getInstance().getClassFieldReader( cls,
-                                                                                      key.getFieldName(),
-                                                                                      this );
+                reader = ClassFieldAccessorFactory.getClassFieldReader( cls,
+                                                                        key.getFieldName(),
+                                                                        this );
                 if ( reader != null ) {
                     BaseClassFieldReader existingReader = this.readCache.putIfAbsent( key,
                                                                                       reader );
@@ -188,9 +187,9 @@ public BaseClassFieldWriter getWriteAccessor(AccessorKey key,
                                                      Class cls) {
             BaseClassFieldWriter writer = this.writeCache.get( key );
             if ( writer == null ) {
-                writer = ClassFieldAccessorFactory.getInstance().getClassFieldWriter( cls,
-                                                                                      key.getFieldName(),
-                                                                                      this );
+                writer = ClassFieldAccessorFactory.getClassFieldWriter( cls,
+                                                                        key.getFieldName(),
+                                                                        this );
                 if ( writer != null ) {
                     BaseClassFieldWriter existingWriter = this.writeCache.putIfAbsent( key,
                                                                                        writer );