[portal-commits] JBoss Portal SVN: r7452 - in trunk/common: src/main/org/jboss/portal/common/mx and 1 other directories.

Author: julien(a)jboss.com Date: 2007-06-17 15:40:31 -0400 (Sun, 17 Jun 2007) New Revision: 7452 Modified: trunk/common/build.xml trunk/common/src/main/org/jboss/portal/common/mx/JavaBeanModelMBeanBuilder.java trunk/common/src/main/org/jboss/portal/test/common/JavaBeanModelMBeanBuilderTestCase.java Log: - improved a bit the implementation of JavaBeanModelMBeanBuilder by removing unnecessary creation of HashMap objects - fixed bug when an overriden getter produces 2 getter operations in the model mbean operation infos metadata Modified: trunk/common/build.xml =================================================================== --- trunk/common/build.xml 2007-06-17 16:39:12 UTC (rev 7451) +++ trunk/common/build.xml 2007-06-17 19:40:31 UTC (rev 7452) @@ -211,6 +211,7 @@ </x-sysproperty> <x-test> +<!-- <test todir="${test.reports}" name="org.jboss.portal.test.common.net.URLNavigatorTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.common.net.URLToolsTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.common.io.IOToolsTestCase"/> @@ -239,7 +240,9 @@ <test todir="${test.reports}" name="org.jboss.portal.test.common.StringTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.common.JarTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.common.PathTestCase"/> +--> <test todir="${test.reports}" name="org.jboss.portal.test.common.JavaBeanModelMBeanBuilderTestCase"/> +<!-- <test todir="${test.reports}" name="org.jboss.portal.test.common.ParameterMapTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.common.LocalizedStringTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.common.ImplodeTestCase"/> @@ -249,6 +252,7 @@ <test todir="${test.reports}" name="org.jboss.portal.test.common.ToolsTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.common.IteratorStatusTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.common.PathMapperTestCase"/> +--> </x-test> <x-classpath> <pathelement location="${build.classes}"/> Modified: trunk/common/src/main/org/jboss/portal/common/mx/JavaBeanModelMBeanBuilder.java =================================================================== --- trunk/common/src/main/org/jboss/portal/common/mx/JavaBeanModelMBeanBuilder.java 2007-06-17 16:39:12 UTC (rev 7451) +++ trunk/common/src/main/org/jboss/portal/common/mx/JavaBeanModelMBeanBuilder.java 2007-06-17 19:40:31 UTC (rev 7452) @@ -82,7 +82,7 @@ // Map beanGetters = new HashMap(); Map beanSetters = new HashMap(); - Set beanMethods = new HashSet(); + Map beanMethods = new HashMap(); // for (Class c = from;c != null && !c.equals(to);c = c.getSuperclass()) @@ -108,22 +108,31 @@ methodName.length() > 3) { String propertyName = methodName.substring(3); - Map setters = new HashMap(); - setters.putAll(beanSetters); - setters.putAll(currentClassSetters); - Method beanSetter = (Method)setters.get(propertyName); + + // 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") && @@ -132,22 +141,31 @@ methodName.length() > 2) { String propertyName = methodName.substring(2); - Map setters = new HashMap(); - setters.putAll(beanSetters); - setters.putAll(currentClassSetters); - Method beanSetter = (Method)setters.get(propertyName); + + // 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); } else if (methodName.startsWith("set") && @@ -156,29 +174,36 @@ methodName.length() > 3) { String propertyName = methodName.substring(3); - Map getters = new HashMap(); - getters.putAll(beanGetters); - getters.putAll(currentClassGetters); - Method beanGetter = (Method)getters.get(propertyName); - + + // Try to locate an existing getter + Method beanGetter = (Method)beanGetters.get(propertyName); + if (beanGetter == null) + { + beanGetter = (Method)currentClassGetters.get(propertyName); + } + + // Check we do not have a getter with a different return type if (beanGetter != null && !beanGetter.getReturnType().equals(parameterTypes[0])) { throw new IllegalArgumentException("Property " + propertyName + " has a setter" + " type " + parameterTypes[0] + " different from the corresponding" + " getter type " + beanGetter.getReturnType()); } + + // 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); } - beanSetters.put(propertyName, method); + + // currentClassSetters.put(propertyName, method); } // - beanMethods.add(method); + beanMethods.put(new MethodKey(method), method); } } beanGetters.putAll(currentClassGetters); @@ -239,7 +264,7 @@ // Methods->Operations mmois = new ArrayList(); - for (Iterator i = beanMethods.iterator(); i.hasNext();) + for (Iterator i = beanMethods.values().iterator(); i.hasNext();) { Method method = (Method)i.next(); @@ -279,22 +304,6 @@ (ModelMBeanOperationInfo[])mmois.toArray(new ModelMBeanOperationInfo[mmois.size()]), new ModelMBeanNotificationInfo[0]); - // Comment out the code that adds the service proxy unwrapper - // don't remove that though it may be usefull later -// try -// { -// // Add the specific interceptor that will unwrap proxies during dependency injection -// Descriptor mbeanDesc = info.getDescriptor("", "mbean"); -// Descriptor interceptorDesc = new DescriptorSupport(); -// interceptorDesc.setField("code", ProxyUnwrapperInterceptor.class.getName()); -// mbeanDesc.setField("interceptors", new Descriptor[]{interceptorDesc}); -// info.setDescriptor(mbeanDesc, "mbean"); -// } -// catch (MBeanException e) -// { -// log.error("Not able to setup the proxy unwrapper interceptor", e); -// } - // return info; } @@ -308,4 +317,67 @@ { return new JavaBeanModelMBeanBuilder(o.getClass(), null).getInfo(); } + + /** + * Key a method during the lookup operations + */ + private static class MethodKey + { + + /** . */ + private final String name; + + /** . */ + private final Class[] parameterTypes; + + /** . */ + private final int hashCode; + + public MethodKey(Method method) + { + this.name = method.getName(); + this.parameterTypes = method.getParameterTypes(); + + // Compute hash code + int hashCode = method.getName().hashCode(); + for (int i = 0;i < parameterTypes.length;i++) + { + Class parameterType = parameterTypes[i]; + hashCode = hashCode * 43 + parameterType.hashCode(); + } + this.hashCode = hashCode; + } + + public int hashCode() + { + return hashCode; + } + + public boolean equals(Object obj) + { + if (obj == this) + { + return true; + } + if (obj instanceof MethodKey) + { + MethodKey that = (MethodKey)obj; + + // + if (that.name.equals(this.name) && that.parameterTypes.length == this.parameterTypes.length) + { + for (int i = 0;i < that.parameterTypes.length;i++) + { + if (!that.parameterTypes[i].equals(this.parameterTypes[i])) + { + return false; + } + } + return true; + } + } + return false; + } + } + } Modified: trunk/common/src/main/org/jboss/portal/test/common/JavaBeanModelMBeanBuilderTestCase.java =================================================================== --- trunk/common/src/main/org/jboss/portal/test/common/JavaBeanModelMBeanBuilderTestCase.java 2007-06-17 16:39:12 UTC (rev 7451) +++ trunk/common/src/main/org/jboss/portal/test/common/JavaBeanModelMBeanBuilderTestCase.java 2007-06-17 19:40:31 UTC (rev 7452) @@ -32,8 +32,10 @@ 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.RequiredModelMBean; +import javax.management.modelmbean.ModelMBeanOperationInfo; import java.util.HashSet; import java.util.Set; import java.util.Arrays; @@ -62,7 +64,7 @@ assertEquals(0, ops.length); } - public void testOneMethod() throws Exception + public void testSimpleClass() throws Exception { JavaBeanModelMBeanBuilder builder = new JavaBeanModelMBeanBuilder(TestClass.class, Object.class); ModelMBeanInfo info = builder.getInfo(); @@ -83,12 +85,12 @@ // Set expectedOps = new HashSet(); - expectedOps.add(new TestOperation("void", "operation", new String[]{})); - expectedOps.add(new TestOperation("void", "operation", new String[]{"java.lang.String"})); - expectedOps.add(new TestOperation("void", "setString", new String[]{"java.lang.String"})); - expectedOps.add(new TestOperation("java.lang.String", "getString", new String[]{})); - expectedOps.add(new TestOperation("void", "setBoolean", new String[]{"boolean"})); - expectedOps.add(new TestOperation("boolean", "isBoolean", new String[]{})); + expectedOps.add(TestOperation.newOperation("void", "operation", new String[]{})); + expectedOps.add(TestOperation.newOperation("void", "operation", new String[]{"java.lang.String"})); + expectedOps.add(TestOperation.newSetter("setString", "java.lang.String")); + expectedOps.add(TestOperation.newGetter("getString", "java.lang.String")); + expectedOps.add(TestOperation.newSetter("setBoolean", "boolean")); + expectedOps.add(TestOperation.newGetter("isBoolean", "boolean")); assertEquals(expectedOps, ops); } @@ -103,7 +105,7 @@ // Set expectedOps = new HashSet(); - expectedOps.add(new TestOperation("void", "overridenOperation", new String[]{})); + expectedOps.add(TestOperation.newOperation("void", "overridenOperation", new String[]{})); assertEquals(expectedOps, ops); } @@ -114,15 +116,23 @@ JavaBeanModelMBeanBuilder builder = new JavaBeanModelMBeanBuilder(OverloadedSetter.class, Object.class); fail(); } - catch (Exception e) + catch (Exception expected) { // expected } } - public void testGetterOverride() throws Exception + public void testOverridenGetter() throws Exception { JavaBeanModelMBeanBuilder builder = new JavaBeanModelMBeanBuilder(TestOverridenExtend.class, Object.class); + ModelMBeanInfo info = builder.getInfo(); + + // + Set ops = getOperations(info); + Set expectedOps = new HashSet(); + expectedOps.add(TestOperation.newGetter("getTest", "java.util.Set")); + assertEquals(expectedOps, ops); + } public void testAttributesAreNotCached() throws Exception @@ -207,7 +217,7 @@ for (int i = 0; i < ops.length; i++) { MBeanOperationInfo op = ops[i]; - set.add(new TestOperation(op)); + set.add(new TestOperation((ModelMBeanOperationInfo)op)); } return set; } @@ -320,15 +330,39 @@ } } + public class TestOverridenBase + { + public Set getTest() + { + //nothing + return new HashSet(); + } + } + + public class TestOverridenExtend extends TestOverridenBase + { + public Set getTest() + { + //nothing + return new HashSet(); + } + } + public static class TestAttribute { + + /** . */ public final String type; + + /** . */ public final String name; + public TestAttribute(MBeanAttributeInfo info) { this.type = info.getType(); this.name = info.getName(); } + public TestAttribute(String type, String name) { if (type == null) @@ -342,15 +376,18 @@ this.type = type; this.name = name; } + public int hashCode() { return type.hashCode() * 43 + name.hashCode(); } + public boolean equals(Object obj) { TestAttribute that = (TestAttribute)obj; return type.equals(that.type) && name.equals(that.name); } + public String toString() { return "Attribute[" + type + "," + name + "]"; @@ -359,21 +396,50 @@ public static class TestOperation { + + /** . */ public final String returnType; + + /** . */ public final String name; + + /** . */ public final String[] argTypes; - public TestOperation(MBeanOperationInfo info) + + /** . */ + public final String role; + + public TestOperation(ModelMBeanOperationInfo info) { - returnType = info.getReturnType(); - name = info.getName(); - argTypes = new String[info.getSignature().length]; + Descriptor desc = info.getDescriptor(); + + this.returnType = info.getReturnType(); + this.name = info.getName(); + this.argTypes = new String[info.getSignature().length]; + this.role = (String)desc.getFieldValue("role"); for (int i = 0; i < argTypes.length; i++) { argTypes[i] = info.getSignature()[i].getType(); } } - public TestOperation(String returnType, String name, String[] argTypes) + + public static TestOperation newOperation(String returnType, String name, String[] argTypes) { + return new TestOperation(returnType, name, argTypes, "operation"); + } + + public static TestOperation newGetter(String name, String type) + { + return new TestOperation(type, name, new String[0], "getter"); + } + + public static TestOperation newSetter(String name, String type) + { + return new TestOperation("void", name, new String[]{type}, "setter"); + } + + public TestOperation(String returnType, String name, String[] argTypes, String role) + { if (returnType == null) { throw new IllegalArgumentException(); @@ -396,10 +462,12 @@ this.returnType = returnType; this.name = name; this.argTypes = argTypes; + this.role = role; } + public int hashCode() { - int code = returnType.hashCode() * 43 + name.hashCode(); + int code = (returnType.hashCode() * 43 + name.hashCode()) * 43 + role.hashCode(); for (int i = 0; i < argTypes.length; i++) { String argType = argTypes[i]; @@ -407,14 +475,16 @@ } return code; } + public boolean equals(Object obj) { TestOperation that = (TestOperation)obj; - return returnType.equals(that.returnType) && name.equals(that.name) && Arrays.equals(argTypes, that.argTypes); + return returnType.equals(that.returnType) && name.equals(that.name) && role.equals(that.role) && Arrays.equals(argTypes, that.argTypes); } + public String toString() { - StringBuffer tmp = new StringBuffer("Operation[").append(returnType).append(",").append(name); + StringBuffer tmp = new StringBuffer("Operation[").append(returnType).append(",").append(name).append(",").append(role); for (int i = 0; i < argTypes.length; i++) { tmp.append(",").append(argTypes[i]); @@ -423,24 +493,4 @@ return tmp.toString(); } } - - public class TestOverridenBase - { - public Set getTest() - { - //nothing - return new HashSet(); - } - - - } - - public class TestOverridenExtend extends TestOverridenBase - { - public Set getTest() - { - //nothing - return new HashSet(); - } - } }