Author: chris.laprun(a)jboss.com
Date: 2008-07-17 16:53:57 -0400 (Thu, 17 Jul 2008)
New Revision: 11482
Modified:
modules/common/branches/JBP_COMMON_BRANCH_1_1/common/src/main/java/org/jboss/portal/common/mx/JavaBeanModelMBeanBuilder.java
modules/common/branches/JBP_COMMON_BRANCH_1_1/common/src/test/java/org/jboss/portal/test/common/JavaBeanModelMBeanBuilderTestCase.java
Log:
- Fixed a bug where overriden setters and getters would be improperly detected as
duplicated methods.
- Added more tests.
Modified:
modules/common/branches/JBP_COMMON_BRANCH_1_1/common/src/main/java/org/jboss/portal/common/mx/JavaBeanModelMBeanBuilder.java
===================================================================
---
modules/common/branches/JBP_COMMON_BRANCH_1_1/common/src/main/java/org/jboss/portal/common/mx/JavaBeanModelMBeanBuilder.java 2008-07-17
20:48:39 UTC (rev 11481)
+++
modules/common/branches/JBP_COMMON_BRANCH_1_1/common/src/main/java/org/jboss/portal/common/mx/JavaBeanModelMBeanBuilder.java 2008-07-17
20:53:57 UTC (rev 11482)
@@ -22,6 +22,13 @@
******************************************************************************/
package org.jboss.portal.common.mx;
+import javax.management.Descriptor;
+import javax.management.modelmbean.ModelMBeanAttributeInfo;
+import javax.management.modelmbean.ModelMBeanConstructorInfo;
+import javax.management.modelmbean.ModelMBeanInfo;
+import javax.management.modelmbean.ModelMBeanInfoSupport;
+import javax.management.modelmbean.ModelMBeanNotificationInfo;
+import javax.management.modelmbean.ModelMBeanOperationInfo;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
@@ -31,14 +38,6 @@
import java.util.Map;
import java.util.Set;
-import javax.management.Descriptor;
-import javax.management.modelmbean.ModelMBeanAttributeInfo;
-import javax.management.modelmbean.ModelMBeanConstructorInfo;
-import javax.management.modelmbean.ModelMBeanInfo;
-import javax.management.modelmbean.ModelMBeanInfoSupport;
-import javax.management.modelmbean.ModelMBeanNotificationInfo;
-import javax.management.modelmbean.ModelMBeanOperationInfo;
-
/**
* @author <a href="mailto:julien@jboss.org">Julien Viet</a>
@@ -46,7 +45,7 @@
* @version $Revision: 7452 $
*/
public class JavaBeanModelMBeanBuilder
-{
+{
private final static String CURRENCY_TIME_LIMIT = "currencyTimeLimit";
private final static String GET_METHOD = "getMethod";
private final static String SET_METHOD = "setMethod";
@@ -56,6 +55,9 @@
private ArrayList mmais;
private ArrayList mmois;
private String className;
+ private static final String GET = "get";
+ private static final String IS = "is";
+ private static final String SET = "set";
public JavaBeanModelMBeanBuilder(Class from, Class to) throws Exception
{
@@ -87,119 +89,49 @@
//
for (Class c = from;c != null && !c.equals(to);c = c.getSuperclass())
{
- Map currentClassGetters = new HashMap();
- Map currentClassSetters = new HashMap();
+ Map<String,Method> currentClassGetters = new HashMap<String,
Method>();
+ Map<String, Method> currentClassSetters = new HashMap<String,
Method>();
Method[] methods = c.getDeclaredMethods();
- for (int i = 0; i < methods.length; i++)
+ for (Method method : methods)
{
- Method method = methods[i];
int modifiers = method.getModifiers();
if (Modifier.isPublic(modifiers) &&
- !Modifier.isAbstract(modifiers) &&
- !Modifier.isStatic(modifiers))
+ !Modifier.isAbstract(modifiers) &&
+ !Modifier.isStatic(modifiers))
{
String methodName = method.getName();
Class returnType = method.getReturnType();
Class[] parameterTypes = method.getParameterTypes();
- if (methodName.startsWith("get") &&
- !void.class.equals(returnType) &&
- parameterTypes.length == 0 &&
- methodName.length() > 3)
- {
- String propertyName = methodName.substring(3);
- // Try to locate an existing setter for the same property
- Method beanSetter = (Method)currentClassSetters.get(propertyName);
- if (beanSetter == null)
- {
- beanSetter = (Method)beanSetters.get(propertyName);
- }
-
- // Check we do not have a setter with a different return type
- if (beanSetter != null &&
!beanSetter.getParameterTypes()[0].equals(returnType))
- {
- throw new IllegalArgumentException("Property " +
propertyName + " has a setter" +
- " type " + beanSetter.getParameterTypes()[0] + "
different from the corresponding" +
- " getter type " + returnType);
- }
-
- // Get an existing bean getter
- Method beanGetter = (Method)currentClassGetters.get(propertyName);
- if (beanGetter != null)
- {
- throw new IllegalArgumentException("Property " +
propertyName + " has two getters " +
- beanGetter + " and " + method);
- }
-
- //
- currentClassGetters.put(propertyName, method);
- }
- else if (methodName.startsWith("is") &&
- !void.class.equals(returnType) &&
- parameterTypes.length == 0 &&
- methodName.length() > 2)
+ int prefixLength = 0;
+ boolean isPotentialGetter = false;
+ if(methodName.startsWith(GET))
{
- String propertyName = methodName.substring(2);
-
- // Try to locate an existing setter for the same property
- Method beanSetter = (Method)beanSetters.get(propertyName);
- if (beanSetter != null)
- {
- beanSetter = (Method)currentClassSetters.get(propertyName);
- }
-
- // Check we do not have a setter with a different return type
- if (beanSetter != null &&
!beanSetter.getParameterTypes()[0].equals(returnType))
- {
- throw new IllegalArgumentException("Property " +
propertyName + " has a setter" +
- " type " + beanSetter.getParameterTypes()[0] + "
different from the corresponding" +
- " getter type " + returnType);
- }
-
- // Get an existing getter
- Method beanGetter = (Method)beanGetters.get(propertyName);
- if (beanGetter != null)
- {
- throw new IllegalArgumentException("Property " +
propertyName + " has two getters " +
- beanGetter + " and " + method);
- }
-
- //
- currentClassGetters.put(propertyName, method);
+ prefixLength = 3;
+ isPotentialGetter = true;
}
- else if (methodName.startsWith("set") &&
- void.class.equals(returnType) &&
- parameterTypes.length == 1 &&
- methodName.length() > 3)
+ else if(methodName.startsWith(IS))
{
- String propertyName = methodName.substring(3);
+ prefixLength = 2;
+ isPotentialGetter = true;
+ } else if (methodName.startsWith(SET))
+ {
+ prefixLength = 3;
+ }
- // Try to locate an existing getter
- Method beanGetter = (Method)beanGetters.get(propertyName);
- if (beanGetter == null)
+ if (methodName.length() > prefixLength)
+ {
+ if (isPotentialGetter && !void.class.equals(returnType)
&& parameterTypes.length == 0)
{
- beanGetter = (Method)currentClassGetters.get(propertyName);
+ processPropertyOperation(method, currentClassGetters,
currentClassSetters, beanSetters,
+ prefixLength, false);
}
-
- // Check we do not have a getter with a different return type
- if (beanGetter != null &&
!beanGetter.getReturnType().equals(parameterTypes[0]))
+ else if (methodName.startsWith(SET) &&
void.class.equals(returnType) && parameterTypes.length == 1)
{
- throw new IllegalArgumentException("Property " +
propertyName + " has a setter" +
- " type " + parameterTypes[0] + " different from the
corresponding" +
- " getter type " + beanGetter.getReturnType());
+ processPropertyOperation(method, currentClassSetters,
currentClassGetters, beanGetters,
+ prefixLength, true);
}
-
- // Get an existing setter
- Method beanSetter = (Method)beanSetters.get(propertyName);
- if (beanSetter != null)
- {
- throw new IllegalArgumentException("Property " +
propertyName + " cannot have two setters " +
- beanSetter + " and " + method);
- }
-
- //
- currentClassSetters.put(propertyName, method);
}
//
@@ -283,6 +215,63 @@
}
/**
+ * Process a property operation either setter or getter, checking for consistency.
"Reverse" operation is defined here
+ * as a setter for a getter operation, and a getter for a setter operation.
+ * Hence, if we are currently checking a getter,
<code>beanReverseOperations</code> will refer to known setters so far,
+ * <code>currentClassOperations</code> to the already known getter for
this class at this hierachical level,
+ * <code>currentClassReverseOperations</code> to the already known setter
for this class at this hierachical level.
+ *
+ * @param operation
+ * @param currentClassOperations known property operations for the hierarchy level
being currently examined
+ * @param currentClassReverseOperations known property "reverse" operations
for the hierarchy level being currently examined
+ * @param beanReverseOperations known "reverse" property operations for this
bean
+ * @param prefixLength
+ * @param isSetter
+ */
+ private void processPropertyOperation(Method operation, Map<String, Method>
currentClassOperations,
+ Map<String, Method>
currentClassReverseOperations, Map beanReverseOperations,
+ int prefixLength, boolean isSetter)
+ {
+ String propertyName = operation.getName().substring(prefixLength);
+
+ // Try to locate an existing setter for the same property
+ Method reverseOp = currentClassReverseOperations.get(propertyName);
+ if (reverseOp == null)
+ {
+ reverseOp = (Method)beanReverseOperations.get(propertyName);
+ }
+
+ // check that if we know a reverse operation, the types match
+ if (reverseOp != null)
+ {
+ Class opType = isSetter ? operation.getParameterTypes()[0] :
operation.getReturnType();
+ Class reverseOpType = isSetter ? reverseOp.getReturnType() :
reverseOp.getParameterTypes()[0];
+ if (!reverseOpType.equals(opType))
+ {
+ throw new IllegalArgumentException("Property " + propertyName +
" has a " + getterOrSetter(isSetter) +
+ " type " + reverseOpType + " different from the
corresponding " + getterOrSetter(!isSetter) +
+ " type " + opType);
+ }
+ }
+
+ // Check that we don't have twice the same operation
+ Method op = currentClassOperations.get(propertyName);
+ if (op != null)
+ {
+ throw new IllegalArgumentException("Property " + propertyName + "
has two " + getterOrSetter(isSetter) + "s " +
+ op + " and " + operation);
+ }
+
+ //
+ currentClassOperations.put(propertyName, operation);
+ }
+
+ private String getterOrSetter(boolean isSetter)
+ {
+ return isSetter ? "setter" : "getter";
+ }
+
+ /**
* Remove an interface from the management interface.
*/
public void remove(Class itf)
Modified:
modules/common/branches/JBP_COMMON_BRANCH_1_1/common/src/test/java/org/jboss/portal/test/common/JavaBeanModelMBeanBuilderTestCase.java
===================================================================
---
modules/common/branches/JBP_COMMON_BRANCH_1_1/common/src/test/java/org/jboss/portal/test/common/JavaBeanModelMBeanBuilderTestCase.java 2008-07-17
20:48:39 UTC (rev 11481)
+++
modules/common/branches/JBP_COMMON_BRANCH_1_1/common/src/test/java/org/jboss/portal/test/common/JavaBeanModelMBeanBuilderTestCase.java 2008-07-17
20:53:57 UTC (rev 11482)
@@ -25,20 +25,20 @@
import junit.framework.TestCase;
import org.jboss.portal.common.mx.JavaBeanModelMBeanBuilder;
+import javax.management.Attribute;
import javax.management.AttributeNotFoundException;
+import javax.management.Descriptor;
import javax.management.MBeanAttributeInfo;
import javax.management.MBeanOperationInfo;
import javax.management.MBeanServer;
import javax.management.MBeanServerFactory;
import javax.management.ObjectName;
-import javax.management.Attribute;
-import javax.management.Descriptor;
import javax.management.modelmbean.ModelMBeanInfo;
+import javax.management.modelmbean.ModelMBeanOperationInfo;
import javax.management.modelmbean.RequiredModelMBean;
-import javax.management.modelmbean.ModelMBeanOperationInfo;
+import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
-import java.util.Arrays;
/**
* @author <a href="mailto:julien@jboss.org">Julien Viet</a>
@@ -131,10 +131,26 @@
Set ops = getOperations(info);
Set expectedOps = new HashSet();
expectedOps.add(TestOperation.newGetter("getTest",
"java.util.Set"));
+ expectedOps.add(TestOperation.newGetter("isSet", "boolean"));
+ expectedOps.add(TestOperation.newSetter("setSet", "boolean"));
+ expectedOps.add(TestOperation.newSetter("setFoo",
"java.lang.String"));
assertEquals(expectedOps, ops);
}
+ public void testMismatchedPropertyOperations()
+ {
+ try
+ {
+ JavaBeanModelMBeanBuilder builder = new
JavaBeanModelMBeanBuilder(TestMismatchedSetterGetter.class, Object.class);
+ fail();
+ }
+ catch (Exception expected)
+ {
+ // expected
+ }
+ }
+
public void testAttributesAreNotCached() throws Exception
{
AttributesAreNotCached aanc = new AttributesAreNotCached();
@@ -337,6 +353,21 @@
//nothing
return new HashSet();
}
+
+ public boolean isSet()
+ {
+ return false;
+ }
+
+ public void setSet(boolean set)
+ {
+ // nothing
+ }
+
+ public void setFoo(String foo)
+ {
+ // nothing
+ }
}
public class TestOverridenExtend extends TestOverridenBase
@@ -346,8 +377,31 @@
//nothing
return new HashSet();
}
+
+ public boolean isSet()
+ {
+ return true;
+ }
+
+ public void setFoo(String foo)
+ {
+ // nothing
+ }
}
+ public class TestMismatchedSetterGetter
+ {
+ public void setFoo(String foo)
+ {
+ // nothing
+ }
+
+ public boolean getFoo()
+ {
+ return false;
+ }
+ }
+
public static class TestAttribute
{