fix null->primitive bindings in both reflective and bytecode generators.

cubita-io · Aug 10, 2010 · 432aed1 · 432aed1
1 parent d9112b3
commit 432aed1
Show file tree

Hide file tree

Showing 8 changed files with 128 additions and 19 deletions.
diff --git a/.classpath b/.classpath
@@ -1,8 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <classpath>
+	<classpathentry kind="src" path="src/test/java"/>
 	<classpathentry kind="src" path="src/main/java"/>
-	<classpathentry kind="src" output="target/test-classes" path="src/test/java"/>
-	<classpathentry kind="src" path="src/test/resources"/>
 	<classpathentry kind="lib" path="lib/asm.jar"/>
 	<classpathentry kind="lib" path="lib/commons-el-1.0.jar"/>
 	<classpathentry kind="lib" path="lib/jsp-api-2.1.jar"/>

diff --git a/META-INF/MANIFEST.MF b/META-INF/MANIFEST.MF
@@ -56,7 +56,7 @@ Export-Package: org.mvel2;uses:="org.mvel2.util,org.mvel2.conversion,o
 Bundle-Version: 2.0.18.SNAPSHOT
 Tool: Bnd-0.0.357
 Bundle-Name: mvel2
-Bnd-LastModified: 1281386698633
+Bnd-LastModified: 1281457587331
 Created-By: 1.6.0_20 (Apple Inc.)
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.mvel2

diff --git a/src/main/java/org/mvel2/optimizers/impl/asm/ASMAccessorOptimizer.java b/src/main/java/org/mvel2/optimizers/impl/asm/ASMAccessorOptimizer.java
@@ -475,6 +475,9 @@ else if (member != null) {
 
                 Class targetType = meth.getParameterTypes()[0];
 
+
+                Label jmp = null;
+                Label jmp2 = new Label();
                 if (value != null && !targetType.isAssignableFrom(value.getClass())) {
                     if (!canConvert(targetType, value.getClass())) {
                         throw new ConversionException("cannot convert type: "
@@ -488,15 +491,52 @@ else if (member != null) {
                     else checkcast(targetType);
                     meth.invoke(ctx, convert(value, meth.getParameterTypes()[0]));
                 }
+//                else if (value == null && targetType.isPrimitive()) {
+//                    checkcast(targetType);
+//                    meth.invoke(ctx, PropertyTools.getPrimitiveInitialValue(targetType));
+//                }
                 else {
-                    checkcast(targetType);
+                    if (targetType.isPrimitive()) {
+
+                        if (value == null) value = PropertyTools.getPrimitiveInitialValue(targetType);
+
+
+                        jmp = new Label();
+                        assert debug("IFNOTNULL jmp");
+                        mv.visitJumpInsn(IFNONNULL, jmp);
+
+                        assert debug("ICONST_0");
+                        mv.visitInsn(ICONST_0);
+
+                        assert debug("INVOKEVIRTUAL " + getInternalName(meth.getDeclaringClass()) + "." + meth.getName());
+                        mv.visitMethodInsn(INVOKEVIRTUAL, getInternalName(meth.getDeclaringClass()), meth.getName(),
+                                getMethodDescriptor(meth));
+
+                        assert debug("GOTO jmp2");
+                        mv.visitJumpInsn(GOTO, jmp2);
+
+                        assert debug("jmp:");
+                        mv.visitLabel(jmp);
+
+                        assert debug("ALOAD 4");
+                        mv.visitVarInsn(ALOAD, 4);
+
+                        unwrapPrimitive(targetType);
+                    }
+                    else {
+                        checkcast(targetType);                        
+                    }
+
                     meth.invoke(ctx, value);
                 }
 
                 assert debug("INVOKEVIRTUAL " + getInternalName(meth.getDeclaringClass()) + "." + meth.getName());
                 mv.visitMethodInsn(INVOKEVIRTUAL, getInternalName(meth.getDeclaringClass()), meth.getName(),
                         getMethodDescriptor(meth));
 
+                assert debug("jmp2:");
+                mv.visitLabel(jmp2);
+
                 assert debug("ALOAD 4");
                 mv.visitVarInsn(ALOAD, 4);
             }

diff --git a/src/main/java/org/mvel2/optimizers/impl/refl/ReflectiveAccessorOptimizer.java b/src/main/java/org/mvel2/optimizers/impl/refl/ReflectiveAccessorOptimizer.java
@@ -35,10 +35,7 @@
 import org.mvel2.optimizers.impl.refl.collection.ListCreator;
 import org.mvel2.optimizers.impl.refl.collection.MapCreator;
 import org.mvel2.optimizers.impl.refl.nodes.*;
-import org.mvel2.util.ArrayTools;
-import org.mvel2.util.MethodStub;
-import org.mvel2.util.ParseTools;
-import org.mvel2.util.StringAppender;
+import org.mvel2.util.*;
 
 import java.lang.reflect.*;
 import java.util.List;
@@ -65,7 +62,7 @@ public class ReflectiveAccessorOptimizer extends AbstractOptimizer implements Ac
     private Object ctx;
     private Object thisRef;
     private Object val;
-
+    
     private VariableResolverFactory variableFactory;
 
     private static final int DONE = -1;
@@ -278,6 +275,9 @@ else if (member != null) {
 
                     meth.invoke(ctx, convert(value, meth.getParameterTypes()[0]));
                 }
+                else if (value == null && meth.getParameterTypes()[0].isPrimitive()) {
+                    meth.invoke(ctx, PropertyTools.getPrimitiveInitialValue(meth.getParameterTypes()[0]));
+                }
                 else {
                     meth.invoke(ctx, value);
                 }
@@ -295,10 +295,13 @@ else if (ctx instanceof Map) {
             }
         }
         catch (InvocationTargetException e) {
-            throw new PropertyAccessException("could not access property", e);
+            throw new PropertyAccessException("could not access property: " + new String(property), e);
         }
         catch (IllegalAccessException e) {
-            throw new PropertyAccessException("could not access property", e);
+            throw new PropertyAccessException("could not access property: " + new String(property), e);
+        }
+        catch (IllegalArgumentException e) {
+            throw new PropertyAccessException("error binding property: " + new String(property) + " (value <<" + value + ">>::" + (value == null ? "null" : value.getClass().getCanonicalName()) + ")");
         }
 
 

diff --git a/src/main/java/org/mvel2/util/PropertyTools.java b/src/main/java/org/mvel2/util/PropertyTools.java
@@ -18,20 +18,21 @@
 
 package org.mvel2.util;
 
-import static org.mvel2.DataConversion.canConvert;
 import org.mvel2.ParserContext;
 import org.mvel2.compiler.PropertyVerifier;
-import static org.mvel2.util.ParseTools.boxPrimitive;
 
-import static java.lang.String.valueOf;
 import java.lang.reflect.Field;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
-import static java.lang.reflect.Modifier.PUBLIC;
-import static java.lang.reflect.Modifier.isPublic;
 import java.util.Collection;
 import java.util.Map;
 
+import static java.lang.String.valueOf;
+import static java.lang.reflect.Modifier.PUBLIC;
+import static java.lang.reflect.Modifier.isPublic;
+import static org.mvel2.DataConversion.canConvert;
+import static org.mvel2.util.ParseTools.boxPrimitive;
+
 public class PropertyTools {
     public static boolean isEmpty(Object o) {
         if (o != null) {
@@ -159,6 +160,15 @@ else if (toCompare.getClass().isArray()) {
         }
         return false;
     }
+
+    public static Object getPrimitiveInitialValue(Class type) {
+        if (type == boolean.class) {
+            return false;
+        }
+        else {
+            return 0;
+        }
+    }
 
     public static boolean isAssignable(Class to, Class from) {
         return (to.isPrimitive() ? boxPrimitive(to) : to).isAssignableFrom(from.isPrimitive() ? boxPrimitive(from) : from);

diff --git a/src/test/java/org/mvel2/tests/AccessorBMModel.java b/src/test/java/org/mvel2/tests/AccessorBMModel.java
@@ -15,17 +15,22 @@
 
 public class AccessorBMModel implements Accessor {
     private ExecutableStatement p0;
+    private long foo;
 
     public PropertyHandler nullPropertyHandler;
     public PropertyHandler nullMethodHandler;
 
     public Object getValue(Object ctx, Object elCtx, VariableResolverFactory variableFactory) {
-    	StringAppender append = new StringAppender().append("foo").append(((String)ctx).toString().trim());
-    	return append;
+    		return null;
     }
 
     public Object setValue(Object ctx, Object elCtx, VariableResolverFactory variableFactory, Object value) {
-        GlobalListenerFactory.notifySetListeners(ctx, "foobie", variableFactory, value);
+        Foo foo = (Foo) ctx;
+
+        if (value == null) {
+        	foo.setCountTest(0);
+        }
+
         return value;
     }
 

diff --git a/src/test/java/org/mvel2/tests/core/CoreConfidenceTests.java b/src/test/java/org/mvel2/tests/core/CoreConfidenceTests.java
@@ -5172,4 +5172,47 @@ public void testAmbiguousGetName() {
 
         System.out.println(MVEL.executeExpression(s, vars));
     }
+
+    public void testBindingNullToPrimitiveTypes() {
+        Map<String, Object> vars = createTestMap();
+        ((Foo) vars.get("foo")).setCountTest(10);
+
+        OptimizerFactory.setDefaultOptimizer("reflective");
+        Serializable s = MVEL.compileSetExpression("foo.countTest");
+        MVEL.executeSetExpression(s, vars, null);
+
+        assertEquals(((Foo) vars.get("foo")).getCountTest(), 0);
+
+        OptimizerFactory.setDefaultOptimizer("ASM");
+        s = MVEL.compileSetExpression("foo.countTest");
+        MVEL.executeSetExpression(s, vars, null);
+
+        assertEquals(((Foo) vars.get("foo")).getCountTest(), 0);
+
+        MVEL.executeSetExpression(s, vars, null);
+
+        assertEquals(((Foo) vars.get("foo")).getCountTest(), 0);
+    }
+
+    public void testBindingNullToPrimitiveTypes2() {
+        Map<String, Object> vars = createTestMap();
+        ((Foo) vars.get("foo")).setCountTest(10);
+
+        OptimizerFactory.setDefaultOptimizer("reflective");
+        Serializable s = MVEL.compileSetExpression("foo.boolTest");
+        MVEL.executeSetExpression(s, vars, null);
+
+        assertFalse(((Foo) vars.get("foo")).isBoolTest());
+
+        OptimizerFactory.setDefaultOptimizer("ASM");
+        s = MVEL.compileSetExpression("foo.boolTest");
+        MVEL.executeSetExpression(s, vars, null);
+
+        assertFalse(((Foo) vars.get("foo")).isBoolTest());
+
+        MVEL.executeSetExpression(s, vars, null);
+
+        assertFalse(((Foo) vars.get("foo")).isBoolTest());
+    }
+
 }
diff --git a/src/test/java/org/mvel2/tests/core/res/Foo.java b/src/test/java/org/mvel2/tests/core/res/Foo.java
@@ -9,6 +9,7 @@ public class Foo {
     public String bValue = "";
     private String name = "dog";
     private int countTest = 0;
+    private boolean boolTest = true;
     private Collection collectionTest;
     private SampleBean sampleBean = new SampleBean();
 
@@ -69,6 +70,14 @@ public void setCountTest(int countTest) {
         this.countTest = countTest;
     }
 
+    public boolean isBoolTest() {
+        return boolTest;
+    }
+
+    public void setBoolTest(boolean boolTest) {
+        this.boolTest = boolTest;
+    }
+
     public boolean equals(Object o) {
         return o instanceof Foo;
     }