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();
- }
- }
}