From 63374488bd5763ac196f924d7549fd81ac547e9c Mon Sep 17 00:00:00 2001 From: Dan Allen Date: Tue, 19 Oct 2010 19:14:17 -0400 Subject: [PATCH] WELDX-181 enforce JavaBean conventions - enforce the JavaBean conventions when creating a property from a method - write tests to validate that method is handled propertly - include reference to JavaBean spec document --- .../properties/MethodPropertyImpl.java | 64 +++++++++-- .../extensions/properties/Properties.java | 4 + .../test/properties/ClassToIntrospect.java | 70 ++++++++++++ .../properties/PropertyFromMethodTest.java | 106 ++++++++++++++++++ 4 files changed, 232 insertions(+), 12 deletions(-) create mode 100644 impl/src/test/java/org/jboss/weld/extensions/test/properties/ClassToIntrospect.java create mode 100644 impl/src/test/java/org/jboss/weld/extensions/test/properties/PropertyFromMethodTest.java diff --git a/impl/src/main/java/org/jboss/weld/extensions/properties/MethodPropertyImpl.java b/impl/src/main/java/org/jboss/weld/extensions/properties/MethodPropertyImpl.java index 595f1a5..6f86dd6 100644 --- a/impl/src/main/java/org/jboss/weld/extensions/properties/MethodPropertyImpl.java +++ b/impl/src/main/java/org/jboss/weld/extensions/properties/MethodPropertyImpl.java @@ -15,31 +15,71 @@ * * @author Pete Muir * @author Shane Bryzak + * @author Dan Allen */ class MethodPropertyImpl implements MethodProperty { + private static final String GETTER_METHOD_PREFIX = "get"; + private static final String SETTER_METHOD_PREFIX = "set"; + private static final String BOOLEAN_GETTER_METHOD_PREFIX = "is"; + + private static final int GETTER_METHOD_PREFIX_LENGTH = GETTER_METHOD_PREFIX.length(); + private static final int SETTER_METHOD_PREFIX_LENGTH = SETTER_METHOD_PREFIX.length(); + private static final int BOOLEAN_GETTER_METHOD_PREFIX_LENGTH = BOOLEAN_GETTER_METHOD_PREFIX.length(); + private final Method getterMethod; private final String propertyName; private final Method setterMethod; public MethodPropertyImpl(Method method) { - if (method.getName().startsWith("get")) + final String accessorMethodPrefix; + final String propertyNameInAccessorMethod; + if (method.getName().startsWith(GETTER_METHOD_PREFIX)) { - this.propertyName = Introspector.decapitalize(method.getName().substring(3)); + if (method.getReturnType() == Void.TYPE) + { + throw new IllegalArgumentException("Invalid accessor method, must have return value if starts with 'get'. Method: " + method); + } + else if (method.getParameterTypes().length > 0) + { + throw new IllegalArgumentException("Invalid accessor method, must have zero arguments if starts with 'get'. Method: " + method); + } + propertyNameInAccessorMethod = method.getName().substring(GETTER_METHOD_PREFIX_LENGTH); + accessorMethodPrefix = GETTER_METHOD_PREFIX; } - else if (method.getName().startsWith("set")) + else if (method.getName().startsWith(SETTER_METHOD_PREFIX)) { - this.propertyName = Introspector.decapitalize(method.getName().substring(3)); + if (method.getReturnType() != Void.TYPE) + { + throw new IllegalArgumentException("Invalid accessor method, must not have return value if starts with 'set'. Method: " + method); + } + else if (method.getParameterTypes().length != 1) + { + throw new IllegalArgumentException("Invalid accessor method, must have one argument if starts with 'set'. Method: " + method); + } + propertyNameInAccessorMethod = method.getName().substring(SETTER_METHOD_PREFIX_LENGTH); + accessorMethodPrefix = SETTER_METHOD_PREFIX; } - else if (method.getName().startsWith("is")) + else if (method.getName().startsWith(BOOLEAN_GETTER_METHOD_PREFIX)) { - this.propertyName = Introspector.decapitalize(method.getName().substring(2)); + if (method.getReturnType() != Boolean.class || !method.getReturnType().isPrimitive()) + { + throw new IllegalArgumentException("Invalid accessor method, must return boolean primitive if starts with 'is'. Method: " + method); + } + propertyNameInAccessorMethod = method.getName().substring(BOOLEAN_GETTER_METHOD_PREFIX_LENGTH); + accessorMethodPrefix = BOOLEAN_GETTER_METHOD_PREFIX; } else { throw new IllegalArgumentException("Invalid accessor method, must start with 'get', 'set' or 'is'. " + "Method: " + method); } + if (propertyNameInAccessorMethod.length() == 0 || !Character.isUpperCase(propertyNameInAccessorMethod.charAt(0))) + { + throw new IllegalArgumentException("Invalid accessor method, prefix '" + accessorMethodPrefix + + "' must be followed a non-empty property name, capitalized. Method: " + method); + } + this.propertyName = Introspector.decapitalize(propertyNameInAccessorMethod); this.getterMethod = getGetterMethod(method.getDeclaringClass(), propertyName); this.setterMethod = getSetterMethod(method.getDeclaringClass(), propertyName); } @@ -86,9 +126,9 @@ private static Method getSetterMethod(Class clazz, String name) for (Method method : methods) { String methodName = method.getName(); - if (methodName.startsWith("set") && method.getParameterTypes().length == 1) + if (methodName.startsWith(SETTER_METHOD_PREFIX) && method.getParameterTypes().length == 1) { - if (Introspector.decapitalize(methodName.substring(3)).equals(name)) + if (Introspector.decapitalize(methodName.substring(SETTER_METHOD_PREFIX_LENGTH)).equals(name)) { return method; } @@ -104,16 +144,16 @@ private static Method getGetterMethod(Class clazz, String name) String methodName = method.getName(); if (method.getParameterTypes().length == 0) { - if (methodName.startsWith("get")) + if (methodName.startsWith(GETTER_METHOD_PREFIX)) { - if (Introspector.decapitalize(methodName.substring(3)).equals(name)) + if (Introspector.decapitalize(methodName.substring(GETTER_METHOD_PREFIX_LENGTH)).equals(name)) { return method; } } - else if (methodName.startsWith("is")) + else if (methodName.startsWith(BOOLEAN_GETTER_METHOD_PREFIX)) { - if (Introspector.decapitalize(methodName.substring(2)).equals(name)) + if (Introspector.decapitalize(methodName.substring(BOOLEAN_GETTER_METHOD_PREFIX_LENGTH)).equals(name)) { return method; } diff --git a/impl/src/main/java/org/jboss/weld/extensions/properties/Properties.java b/impl/src/main/java/org/jboss/weld/extensions/properties/Properties.java index 86e41a7..d97db58 100644 --- a/impl/src/main/java/org/jboss/weld/extensions/properties/Properties.java +++ b/impl/src/main/java/org/jboss/weld/extensions/properties/Properties.java @@ -36,6 +36,8 @@ public static FieldProperty createProperty(Field field) * @param method * @return * @throws IllegalArgumentException if the method does not match JavaBean conventions + * + * @see http://www.oracle.com/technetwork/java/javase/documentation/spec-136004.html */ public static MethodProperty createProperty(Method method) { @@ -49,6 +51,8 @@ public static MethodProperty createProperty(Method method) * @param member * @return * @throws IllegalArgumentException if the method does not match JavaBean conventions + * + * @see http://www.oracle.com/technetwork/java/javase/documentation/spec-136004.html */ public static Property createProperty(Member member) { diff --git a/impl/src/test/java/org/jboss/weld/extensions/test/properties/ClassToIntrospect.java b/impl/src/test/java/org/jboss/weld/extensions/test/properties/ClassToIntrospect.java new file mode 100644 index 0000000..246d19a --- /dev/null +++ b/impl/src/test/java/org/jboss/weld/extensions/test/properties/ClassToIntrospect.java @@ -0,0 +1,70 @@ +package org.jboss.weld.extensions.test.properties; + +import java.net.URL; + +public class ClassToIntrospect +{ + private String name; + + private String p; + + private URL URL; + + public String getName() + { + return name; + } + + public void setName(String name) + { + this.name = name; + } + + public String getP() + { + return p; + } + + public void setP(String p) + { + this.p = p; + } + + public String getTitle() + { + return "Hero"; + } + + public String get() + { + return null; + } + + public boolean is() + { + return false; + } + + public void getFooBar() + { + } + + public void setSalary(Double base, Double bonus) + { + } + + public URL getURL() + { + return URL; + } + + public void setURL(URL URL) + { + this.URL = URL; + } + + public Boolean isValid() + { + return false; + } +} diff --git a/impl/src/test/java/org/jboss/weld/extensions/test/properties/PropertyFromMethodTest.java b/impl/src/test/java/org/jboss/weld/extensions/test/properties/PropertyFromMethodTest.java new file mode 100644 index 0000000..71dd277 --- /dev/null +++ b/impl/src/test/java/org/jboss/weld/extensions/test/properties/PropertyFromMethodTest.java @@ -0,0 +1,106 @@ +package org.jboss.weld.extensions.test.properties; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.lang.reflect.Method; +import java.net.URL; + +import org.jboss.weld.extensions.properties.Properties; +import org.jboss.weld.extensions.properties.Property; +import org.junit.Test; + +/** + * Verify that only valid properties are permitted, as per the JavaBean specification. + * + * @author Dan Allen + * @see http://www.oracle.com/technetwork/java/javase/documentation/spec-136004.html + */ +public class PropertyFromMethodTest +{ + @Test + public void testValidPropertyGetterMethod() throws Exception + { + Method getter = ClassToIntrospect.class.getMethod("getName"); + Property p = Properties.createProperty(getter); + assertNotNull(p); + assertEquals("name", p.getName()); + assertEquals(getter, p.getMember()); + } + + @Test + public void testValidPropertySetterMethod() throws Exception + { + Property p = Properties.createProperty(ClassToIntrospect.class.getMethod("setName", String.class)); + assertNotNull(p); + assertEquals("name", p.getName()); + } + + @Test + public void testReadOnlyProperty() throws Exception + { + Property p = Properties.createProperty(ClassToIntrospect.class.getMethod("getTitle")); + assertNotNull(p); + assertEquals("title", p.getName()); + assertTrue(p.isReadOnly()); + } + + @Test(expected = IllegalArgumentException.class) + public void testEmptyPropertyGetterMethod() throws Exception + { + Properties.createProperty(ClassToIntrospect.class.getMethod("get")); + } + + @Test(expected = IllegalArgumentException.class) + public void testEmptyBooleanPropertyGetterMethod() throws Exception + { + Properties.createProperty(ClassToIntrospect.class.getMethod("is")); + } + + @Test(expected = IllegalArgumentException.class) + public void testNonPrimitiveBooleanPropertyIsMethod() throws Exception + { + Properties.createProperty(ClassToIntrospect.class.getMethod("isValid")); + } + + @Test + public void testSingleCharPropertyGetterMethod() throws Exception + { + Method getter = ClassToIntrospect.class.getMethod("getP"); + Property p = Properties.createProperty(getter); + assertNotNull(p); + assertEquals("p", p.getName()); + assertEquals(getter, p.getMember()); + } + + @Test + public void testSingleCharPropertySetterMethod() throws Exception + { + Property p = Properties.createProperty(ClassToIntrospect.class.getMethod("setP", String.class)); + assertNotNull(p); + assertEquals("p", p.getName()); + } + + @Test(expected = IllegalArgumentException.class) + public void testGetterMethodWithVoidReturnType() throws Exception + { + Properties.createProperty(ClassToIntrospect.class.getMethod("getFooBar")); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetterMethodWithMultipleParameters() throws Exception + { + Properties.createProperty(ClassToIntrospect.class.getMethod("setSalary", Double.class, Double.class)); + } + + @Test + public void testAcronymProperty() throws Exception + { + Method getter = ClassToIntrospect.class.getMethod("getURL"); + Property p = Properties.createProperty(getter); + assertNotNull(p); + assertEquals("URL", p.getName()); + assertEquals(getter, p.getMember()); + } +}