Mailing-List: contact commits-help@bval.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@bval.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1765727 - in /bval/trunk:
 bval-core/src/main/java/org/apache/bval/model/MetaBean.java
 bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java
Date: Wed, 19 Oct 2016 22:06:07 -0000
To: commits@bval.apache.org
From: mbenson@apache.org
Message-Id: <20161019220607.BE5783A05B6@svn01-us-west.apache.org>
archived-at: Wed, 19 Oct 2016 22:11:18 -0000

Author: mbenson
Date: Wed Oct 19 22:06:07 2016
New Revision: 1765727

URL: http://svn.apache.org/viewvc?rev=1765727&view=rev
Log:
[BVAL-149] internal metadata cache was incorrectly seeing all boolean properties defined by is* accessors as being one

Modified:
    bval/trunk/bval-core/src/main/java/org/apache/bval/model/MetaBean.java
    bval/trunk/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java

Modified: bval/trunk/bval-core/src/main/java/org/apache/bval/model/MetaBean.java
URL: http://svn.apache.org/viewvc/bval/trunk/bval-core/src/main/java/org/apache/bval/model/MetaBean.java?rev=1765727&r1=1765726&r2=1765727&view=diff
==============================================================================
--- bval/trunk/bval-core/src/main/java/org/apache/bval/model/MetaBean.java (original)
+++ bval/trunk/bval-core/src/main/java/org/apache/bval/model/MetaBean.java Wed Oct 19 22:06:07 2016
@@ -16,10 +16,6 @@
  */
 package org.apache.bval.model;
 
-import org.apache.bval.util.reflection.Reflection;
-import org.apache.commons.weaver.privilizer.Privilizing;
-import org.apache.commons.weaver.privilizer.Privilizing.CallTo;
-
 import java.beans.Introspector;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
@@ -30,6 +26,11 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.TreeMap;
 
+import org.apache.bval.util.reflection.Reflection;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.weaver.privilizer.Privilizing;
+import org.apache.commons.weaver.privilizer.Privilizing.CallTo;
+
 /**
  * Description: the meta description of a bean or class. the class/bean itself can have a map of features and an array
  * of metaproperties.<br/>
@@ -279,20 +280,16 @@ public class MetaBean extends FeaturesCa
             Class<?> clazz = beanClass;
             while (clazz != null && clazz != Object.class) {
                 for (final Field f : Reflection.getDeclaredFields(clazz)) {
-                    i++;
                     final String name = f.getName();
                     if (!fields.containsKey(name)) {
-                        fields.put(name, i);
+                        fields.put(name, Integer.valueOf(++i));
                     }
                 }
                 for (final Method m : clazz.getDeclaredMethods()) {
-                    if (m.getName().startsWith("get") && Void.TYPE != m.getReturnType() && m.getParameterTypes().length == 0) {
-                        final String name = Introspector.decapitalize(m.getName().substring("get".length()));
-                        if (!name.isEmpty()) {
-                            i++;
-                            if (!fields.containsKey(name)) {
-                                fields.put(name, i);
-                            }
+                    final String name = getPropertyName(m);
+                    if (StringUtils.isNotEmpty(name)) {
+                        if (!fields.containsKey(name)) {
+                            fields.put(name, Integer.valueOf(++i));
                         }
                     }
                 }
@@ -300,18 +297,39 @@ public class MetaBean extends FeaturesCa
             }
         }
 
-        @Override
-        public int compare(final String o1, final String o2) {
-            return fieldIndex(o1) - fieldIndex(o2);
+        private String getPropertyName(Method potentialAccessor) {
+            if (potentialAccessor.getParameterTypes().length == 0) {
+                final String name = potentialAccessor.getName();
+                if (Boolean.TYPE.equals(potentialAccessor.getReturnType())
+                    && potentialAccessor.getName().startsWith("is")) {
+                    return Introspector.decapitalize(name.substring(2));
+                }
+                if (!Void.TYPE.equals(potentialAccessor.getReturnType())
+                    && potentialAccessor.getName().startsWith("get")) {
+                    return Introspector.decapitalize(name.substring(3));
+                }
+            }
+            return null;
         }
 
-        private int fieldIndex(final String o2) {
-            final Integer idx = fields.get(o2);
-            if (idx == null) {
-                return Integer.MIN_VALUE; // to avoid collision and false positive in get() due to equals
+        @Override
+        public int compare(final String o1, final String o2) {
+            final Integer i1 = fields.get(o1);
+            final Integer i2 = fields.get(o2);
+            if (i1 == null) {
+                if (i2 == null) {
+                    // java.util.TreeMap requires that the comparator be consistent with #equals(),
+                    // therefore we must not incorrectly report 0 comparison for different property names
+                    return StringUtils.compare(o1, o2);
+                }
+                return -1;
             }
-            return idx;
+            if (i2 == null) {
+                return 1;
+            }
+            return i1.intValue() - i2.intValue();
         }
+
     }
 
     protected static class MethodComparator implements Comparator<Method> {

Modified: bval/trunk/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java
URL: http://svn.apache.org/viewvc/bval/trunk/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java?rev=1765727&r1=1765726&r2=1765727&view=diff
==============================================================================
--- bval/trunk/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java (original)
+++ bval/trunk/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java Wed Oct 19 22:06:07 2016
@@ -37,6 +37,7 @@ import java.util.Map;
 import java.util.Set;
 
 import javax.validation.ConstraintViolation;
+import javax.validation.constraints.AssertTrue;
 import javax.validation.constraints.NotNull;
 import javax.validation.constraints.Size;
 import javax.validation.groups.Default;
@@ -713,6 +714,27 @@ public class ValidationTest extends Vali
     	assertTrue(errors.isEmpty());
     }
 
+    @Test
+    public void testValidatePrimitiveBooleanPropertyNameIssue149() {
+        Set<ConstraintViolation<Issue149Subject>> violations = validator.validate(new Issue149Subject());
+        assertEquals(1, violations.size());
+        ConstraintViolation<Issue149Subject> violation = violations.iterator().next();
+        assertEquals("false", violation.getMessage());
+        assertEquals("booleanFalse", violation.getPropertyPath().toString());
+    }
+
+    public static class Issue149Subject {
+        @AssertTrue(message = "true")
+        public boolean isBooleanTrue() {
+            return true;
+        }
+
+        @AssertTrue(message = "false")
+        public boolean isBooleanFalse() {
+            return false;
+        }
+    }
+
     private static class TestCloneableClass implements Cloneable {
     }
 }