6763051: MXBean: Incorrect type names for parametrized dealing with arrays (openType)

6713777: developer diagnosability of errors in uncompliant mxbean interfaces

Reviewed-by: dfuchs
This commit is contained in:
Eamonn McManus 2008-10-28 18:21:36 +01:00
parent 333adf3ae1
commit 540b83b6b1
5 changed files with 323 additions and 22 deletions

View File

@ -26,7 +26,8 @@
package com.sun.jmx.mbeanserver;
import static com.sun.jmx.mbeanserver.Util.*;
import java.lang.annotation.ElementType;
import static com.sun.jmx.mbeanserver.MXBeanIntrospector.typeName;
import javax.management.openmbean.MXBeanMappingClass;
import static javax.management.openmbean.SimpleType.*;
@ -247,8 +248,10 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
public synchronized MXBeanMapping mappingForType(Type objType,
MXBeanMappingFactory factory)
throws OpenDataException {
if (inProgress.containsKey(objType))
throw new OpenDataException("Recursive data structure");
if (inProgress.containsKey(objType)) {
throw new OpenDataException(
"Recursive data structure, including " + typeName(objType));
}
MXBeanMapping mapping;
@ -259,6 +262,8 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
inProgress.put(objType, objType);
try {
mapping = makeMapping(objType, factory);
} catch (OpenDataException e) {
throw openDataException("Cannot convert type: " + typeName(objType), e);
} finally {
inProgress.remove(objType);
}
@ -411,7 +416,7 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
MXBeanMappingFactory factory)
throws OpenDataException {
final String objTypeName = objType.toString();
final String objTypeName = typeName(objType);
final MXBeanMapping keyMapping = factory.mappingForType(keyType, factory);
final MXBeanMapping valueMapping = factory.mappingForType(valueType, factory);
final OpenType<?> keyOpenType = keyMapping.getOpenType();
@ -926,6 +931,7 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
concatenating each Builder's explanation of why it
isn't applicable. */
final StringBuilder whyNots = new StringBuilder();
Throwable possibleCause = null;
find:
for (CompositeBuilder[] relatedBuilders : builders) {
for (int i = 0; i < relatedBuilders.length; i++) {
@ -935,6 +941,9 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
foundBuilder = builder;
break find;
}
Throwable cause = builder.possibleCause();
if (cause != null)
possibleCause = cause;
if (whyNot.length() > 0) {
if (whyNots.length() > 0)
whyNots.append("; ");
@ -945,10 +954,12 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
}
}
if (foundBuilder == null) {
final String msg =
String msg =
"Do not know how to make a " + targetClass.getName() +
" from a CompositeData: " + whyNots;
throw new InvalidObjectException(msg);
if (possibleCause != null)
msg += ". Remaining exceptions show a POSSIBLE cause.";
throw invalidObjectException(msg, possibleCause);
}
compositeBuilder = foundBuilder;
}
@ -996,6 +1007,16 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
abstract String applicable(Method[] getters)
throws InvalidObjectException;
/** If the subclass returns an explanation of why it is not applicable,
it can additionally indicate an exception with details. This is
potentially confusing, because the real problem could be that one
of the other subclasses is supposed to be applicable but isn't.
But the advantage of less information loss probably outweighs the
disadvantage of possible confusion. */
Throwable possibleCause() {
return null;
}
abstract Object fromCompositeData(CompositeData cd,
String[] itemNames,
MXBeanMapping[] converters)
@ -1031,8 +1052,8 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
if (fromMethod.getReturnType() != getTargetClass()) {
final String msg =
"Method from(CompositeData) returns " +
fromMethod.getReturnType().getName() +
" not " + targetClass.getName();
typeName(fromMethod.getReturnType()) +
" not " + typeName(targetClass);
throw new InvalidObjectException(msg);
}
@ -1083,6 +1104,7 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
try {
getterConverters[i].checkReconstructible();
} catch (InvalidObjectException e) {
possibleCause = e;
return "method " + getters[i].getName() + " returns type " +
"that cannot be mapped back from OpenData";
}
@ -1090,6 +1112,11 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
return "";
}
@Override
Throwable possibleCause() {
return possibleCause;
}
final Object fromCompositeData(CompositeData cd,
String[] itemNames,
MXBeanMapping[] converters) {
@ -1097,6 +1124,7 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
}
private final MXBeanMapping[] getterConverters;
private Throwable possibleCause;
}
/** Builder for when the target class has a setter for every getter. */
@ -1227,10 +1255,16 @@ public class DefaultMXBeanMappingFactory extends MXBeanMappingFactory {
for (int i = 0; i < propertyNames.length; i++) {
String propertyName = propertyNames[i];
if (!getterMap.containsKey(propertyName)) {
final String msg =
String msg =
"@ConstructorProperties includes name " + propertyName +
" which does not correspond to a property: " +
constr;
" which does not correspond to a property";
for (String getterName : getterMap.keySet()) {
if (getterName.equalsIgnoreCase(propertyName)) {
msg += " (differs only in case from property " +
getterName + ")";
}
}
msg += ": " + constr;
throw new InvalidObjectException(msg);
}
int getterIndex = getterMap.get(propertyName);

View File

@ -391,26 +391,26 @@ class MXBeanIntrospector extends MBeanIntrospector<ConvertingMethod> {
if (type instanceof Class<?>)
return ((Class<?>) type).getName();
else
return genericTypeString(type);
return typeName(type);
}
private static String genericTypeString(Type type) {
static String typeName(Type type) {
if (type instanceof Class<?>) {
Class<?> c = (Class<?>) type;
if (c.isArray())
return genericTypeString(c.getComponentType()) + "[]";
return typeName(c.getComponentType()) + "[]";
else
return c.getName();
} else if (type instanceof GenericArrayType) {
GenericArrayType gat = (GenericArrayType) type;
return genericTypeString(gat.getGenericComponentType()) + "[]";
return typeName(gat.getGenericComponentType()) + "[]";
} else if (type instanceof ParameterizedType) {
ParameterizedType pt = (ParameterizedType) type;
StringBuilder sb = new StringBuilder();
sb.append(genericTypeString(pt.getRawType())).append("<");
sb.append(typeName(pt.getRawType())).append("<");
String sep = "";
for (Type t : pt.getActualTypeArguments()) {
sb.append(sep).append(genericTypeString(t));
sb.append(sep).append(typeName(t));
sep = ", ";
}
return sb.append(">").toString();

View File

@ -361,7 +361,13 @@ public class MBeanServerInvocationHandler implements InvocationHandler {
if (p != null)
return p;
}
p = new MXBeanProxy(mxbeanInterface, mappingFactory);
try {
p = new MXBeanProxy(mxbeanInterface, mappingFactory);
} catch (IllegalArgumentException e) {
String msg = "Cannot make MXBean proxy for " +
mxbeanInterface.getName() + ": " + e.getMessage();
throw new IllegalArgumentException(msg, e.getCause());
}
classToProxy.put(mxbeanInterface, new WeakReference<MXBeanProxy>(p));
return p;
}

View File

@ -0,0 +1,237 @@
/*
* @test
* @bug 6713777
* @summary Test that exception messages include all relevant information
* @author Eamonn McManus
*/
import java.beans.ConstructorProperties;
import java.io.File;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import javax.management.JMX;
import javax.management.MBeanServer;
import javax.management.MBeanServerFactory;
import javax.management.NotCompliantMBeanException;
import javax.management.ObjectName;
public class ExceptionDiagnosisTest {
private static volatile String failure;
// ------ Illegal MXBeans ------
// Test that all of BdelloidMXBean, Rotifer, and File appear in the
// exception messages. File is not an allowed type because of recursive
// getters like "File getParentFile()".
public static interface BdelloidMXBean {
public Rotifer getRotifer();
}
public static class Bdelloid implements BdelloidMXBean {
public Rotifer getRotifer() {
return null;
}
}
public static class Rotifer {
public File getFile() {
return null;
}
}
// Test that all of IndirectHashMapMXBean, HashMapContainer, and
// HashMap<String,String> appear in the exception messages.
// HashMap<String,String> is not an allowed type because only the
// java.util interface such as Map are allowed with generic parameters,
// not their concrete implementations like HashMap.
public static interface IndirectHashMapMXBean {
public HashMapContainer getContainer();
}
public static class IndirectHashMap implements IndirectHashMapMXBean {
public HashMapContainer getContainer() {
return null;
}
}
public static class HashMapContainer {
public HashMap<String, String> getHashMap() {return null;}
}
// ------ MXBeans that are legal but where proxies are not ------
// Test that all of BlimMXBean, BlimContainer, Blim, and Blam appear
// in the exception messages for a proxy for this MXBean. Blam is
// legal in MXBeans but is not reconstructible so you cannot make
// a proxy for BlimMXBean.
public static interface BlimMXBean {
public BlimContainer getBlimContainer();
}
public static class BlimImpl implements BlimMXBean {
public BlimContainer getBlimContainer() {
return null;
}
}
public static class BlimContainer {
public Blim getBlim() {return null;}
public void setBlim(Blim blim) {}
}
public static class Blim {
public Blam getBlam() {return null;}
public void setBlam(Blam blam) {}
}
public static class Blam {
public Blam(int x) {}
public int getX() {return 0;}
}
// ------ Property name differing only in case ------
public static interface CaseProbMXBean {
public CaseProb getCaseProb();
}
public static class CaseProbImpl implements CaseProbMXBean {
public CaseProb getCaseProb() {return null;}
}
public static class CaseProb {
@ConstructorProperties({"urlPath"})
public CaseProb(String urlPath) {}
public String getURLPath() {return null;}
}
public static void main(String[] args) throws Exception {
testMXBeans(new Bdelloid(), BdelloidMXBean.class, Rotifer.class, File.class);
testMXBeans(new IndirectHashMap(),
IndirectHashMapMXBean.class, HashMapContainer.class,
HashMapContainer.class.getMethod("getHashMap").getGenericReturnType());
testProxies(new BlimImpl(), BlimMXBean.class, BlimMXBean.class,
BlimContainer.class, Blim.class, Blam.class);
testCaseProb();
if (failure == null)
System.out.println("TEST PASSED");
else
throw new Exception("TEST FAILED: " + failure);
}
private static void testMXBeans(Object mbean, Type... expectedTypes)
throws Exception {
try {
MBeanServer mbs = MBeanServerFactory.newMBeanServer();
ObjectName name = new ObjectName("a:b=c");
mbs.registerMBean(mbean, name);
fail("No exception from registerMBean for " + mbean);
} catch (NotCompliantMBeanException e) {
checkExceptionChain("MBean " + mbean, e, expectedTypes);
}
}
private static <T> void testProxies(
Object mbean, Class<T> mxbeanClass, Type... expectedTypes)
throws Exception {
MBeanServer mbs = MBeanServerFactory.newMBeanServer();
ObjectName name = new ObjectName("a:b=c");
mbs.registerMBean(mbean, name);
T proxy = JMX.newMXBeanProxy(mbs, name, mxbeanClass);
List<Method> methods = new ArrayList<Method>();
for (Method m : mxbeanClass.getMethods()) {
if (m.getDeclaringClass() == mxbeanClass)
methods.add(m);
}
if (methods.size() != 1) {
fail("TEST BUG: expected to find exactly one method in " +
mxbeanClass.getName() + ": " + methods);
}
Method getter = methods.get(0);
try {
try {
getter.invoke(proxy);
fail("No exception from proxy method " + getter.getName() +
" in " + mxbeanClass.getName());
} catch (InvocationTargetException e) {
Throwable cause = e.getCause();
if (cause instanceof Exception)
throw (Exception) cause;
else
throw (Error) cause;
}
} catch (IllegalArgumentException e) {
checkExceptionChain(
"Proxy for " + mxbeanClass.getName(), e, expectedTypes);
}
}
private static void testCaseProb() throws Exception {
MBeanServer mbs = MBeanServerFactory.newMBeanServer();
ObjectName name = new ObjectName("a:b=c");
Object mbean = new CaseProbImpl();
mbs.registerMBean(new CaseProbImpl(), name);
CaseProbMXBean proxy = JMX.newMXBeanProxy(mbs, name, CaseProbMXBean.class);
try {
CaseProb prob = proxy.getCaseProb();
fail("No exception from proxy method getCaseProb");
} catch (IllegalArgumentException e) {
String messageChain = messageChain(e);
if (messageChain.contains("URLPath")) {
System.out.println("Message chain contains URLPath as required: "
+ messageChain);
} else {
fail("Exception chain for CaseProb does not mention property" +
" URLPath differing only in case");
System.out.println("Full stack trace:");
e.printStackTrace(System.out);
}
}
}
private static void checkExceptionChain(
String what, Throwable e, Type[] expectedTypes) {
System.out.println("Exceptions in chain for " + what + ":");
for (Throwable t = e; t != null; t = t.getCause())
System.out.println(".." + t);
String messageChain = messageChain(e);
// Now check that each of the classes is mentioned in those messages
for (Type type : expectedTypes) {
String name = (type instanceof Class) ?
((Class<?>) type).getName() : type.toString();
if (!messageChain.contains(name)) {
fail("Exception chain for " + what + " does not mention " +
name);
System.out.println("Full stack trace:");
e.printStackTrace(System.out);
}
}
System.out.println();
}
private static String messageChain(Throwable t) {
String msg = "//";
for ( ; t != null; t = t.getCause())
msg += " " + t.getMessage() + " //";
return msg;
}
private static void fail(String why) {
failure = why;
System.out.println("FAIL: " + why);
}
}

View File

@ -40,6 +40,8 @@ import javax.management.MBeanServer;
import javax.management.MBeanServerFactory;
import javax.management.ObjectName;
import javax.management.StandardMBean;
import javax.management.openmbean.TabularData;
import javax.management.openmbean.TabularType;
public class TypeNameTest {
public static interface TestMXBean {
@ -63,7 +65,7 @@ public class TypeNameTest {
}
};
static String failure;
static volatile String failure;
public static void main(String[] args) throws Exception {
TestMXBean testImpl = (TestMXBean) Proxy.newProxyInstance(
@ -74,24 +76,46 @@ public class TypeNameTest {
mbs.registerMBean(mxbean, name);
MBeanInfo mbi = mbs.getMBeanInfo(name);
MBeanAttributeInfo[] mbais = mbi.getAttributes();
boolean sawTabular = false;
for (MBeanAttributeInfo mbai : mbais) {
String attrName = mbai.getName();
String attrTypeName = (String) mbai.getDescriptor().getFieldValue("originalType");
String fieldName = attrName + "Name";
Field nameField = TestMXBean.class.getField(fieldName);
String expectedTypeName = (String) nameField.get(null);
if (expectedTypeName.equals(attrTypeName)) {
System.out.println("OK: " + attrName + ": " + attrTypeName);
} else {
failure = "For attribute " + attrName + " expected type name \"" +
fail("For attribute " + attrName + " expected type name \"" +
expectedTypeName + "\", found type name \"" + attrTypeName +
"\"";
System.out.println("FAIL: " + failure);
"\"");
}
if (mbai.getType().equals(TabularData.class.getName())) {
sawTabular = true;
TabularType tt = (TabularType) mbai.getDescriptor().getFieldValue("openType");
if (tt.getTypeName().equals(attrTypeName)) {
System.out.println("OK: TabularType name for " + attrName);
} else {
fail("For attribute " + attrName + " expected TabularType " +
"name \"" + attrTypeName + "\", found \"" +
tt.getTypeName());
}
}
}
if (!sawTabular)
fail("Test bug: did not test TabularType name");
if (failure == null)
System.out.println("TEST PASSED");
else
throw new Exception("TEST FAILED: " + failure);
}
private static void fail(String why) {
System.out.println("FAIL: " + why);
failure = why;
}
}