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 {