[BVAL-149] internal metadata cache was incorrectly seeing all boolean properties defined by is* accessors as being one
Project: http://git-wip-us.apache.org/repos/asf/bval/repo Commit: http://git-wip-us.apache.org/repos/asf/bval/commit/df741b5d Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/df741b5d Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/df741b5d Branch: refs/heads/master Commit: df741b5da769576630b357ccd9536ee76dd90272 Parents: 889333a Author: Matt Benson <[email protected]> Authored: Wed Oct 19 22:06:07 2016 +0000 Committer: Matt Benson <[email protected]> Committed: Wed Oct 19 22:06:07 2016 +0000 ---------------------------------------------------------------------- .../java/org/apache/bval/model/MetaBean.java | 60 +++++++++++++------- .../org/apache/bval/jsr/ValidationTest.java | 22 +++++++ 2 files changed, 61 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/bval/blob/df741b5d/bval-core/src/main/java/org/apache/bval/model/MetaBean.java ---------------------------------------------------------------------- diff --git a/bval-core/src/main/java/org/apache/bval/model/MetaBean.java b/bval-core/src/main/java/org/apache/bval/model/MetaBean.java index ae5d9e5..19d484b 100644 --- a/bval-core/src/main/java/org/apache/bval/model/MetaBean.java +++ b/bval-core/src/main/java/org/apache/bval/model/MetaBean.java @@ -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 FeaturesCapable implements Cloneable, Features.Bea 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 FeaturesCapable implements Cloneable, Features.Bea } } - @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> { http://git-wip-us.apache.org/repos/asf/bval/blob/df741b5d/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java ---------------------------------------------------------------------- diff --git a/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java b/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java index d49bd60..71449fb 100644 --- a/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java +++ b/bval-jsr/src/test/java/org/apache/bval/jsr/ValidationTest.java @@ -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 ValidationTestBase { 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 { } }
