6774918: @NotificationInfo is ineffective on MBeans that cannot send notifications
Reviewed-by: jfdenise
--- a/jdk/src/share/classes/com/sun/jmx/mbeanserver/MBeanInjector.java Fri Dec 05 21:59:09 2008 -0800
+++ b/jdk/src/share/classes/com/sun/jmx/mbeanserver/MBeanInjector.java Tue Dec 09 12:01:07 2008 +0100
@@ -47,6 +47,10 @@
import javax.management.SendNotification;
public class MBeanInjector {
+ // There are no instances of this class
+ private MBeanInjector() {
+ }
+
private static Class<?>[] injectedClasses = {
MBeanServer.class, ObjectName.class, SendNotification.class,
};
--- a/jdk/src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java Fri Dec 05 21:59:09 2008 -0800
+++ b/jdk/src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java Tue Dec 09 12:01:07 2008 +0100
@@ -44,7 +44,6 @@
import javax.management.ImmutableDescriptor;
import javax.management.IntrospectionException;
import javax.management.InvalidAttributeValueException;
-import javax.management.JMX;
import javax.management.MBean;
import javax.management.MBeanAttributeInfo;
import javax.management.MBeanConstructorInfo;
@@ -538,6 +537,14 @@
}
}
+ /*
+ * Return the array of MBeanNotificationInfo for the given MBean object.
+ * If the object implements NotificationBroadcaster and its
+ * getNotificationInfo() method returns a non-empty array, then that
+ * is the result. Otherwise, if the object has a @NotificationInfo
+ * or @NotificationInfos annotation, then its contents form the result.
+ * Otherwise, the result is null.
+ */
static MBeanNotificationInfo[] findNotifications(Object moi) {
if (moi instanceof NotificationBroadcaster) {
MBeanNotificationInfo[] mbn =
@@ -553,6 +560,13 @@
}
return result;
}
+ } else {
+ try {
+ if (!MBeanInjector.injectsSendNotification(moi))
+ return null;
+ } catch (NotCompliantMBeanException e) {
+ throw new RuntimeException(e);
+ }
}
return findNotificationsFromAnnotations(moi.getClass());
}
--- a/jdk/src/share/classes/javax/management/NotificationInfo.java Fri Dec 05 21:59:09 2008 -0800
+++ b/jdk/src/share/classes/javax/management/NotificationInfo.java Tue Dec 09 12:01:07 2008 +0100
@@ -44,7 +44,13 @@
* "com.example.notifs.destroy"})
* public interface CacheMBean {...}
*
- * public class Cache implements CacheMBean {...}
+ * public class Cache
+ * extends NotificationBroadcasterSupport implements CacheMBean {
+ * public Cache() {
+ * super(); // do not supply any MBeanNotificationInfo[]
+ * }
+ * ...
+ * }
* </pre>
*
* <pre>
@@ -52,7 +58,11 @@
* {@link MBean @MBean}
* {@code @NotificationInfo}(types={"com.example.notifs.create",
* "com.example.notifs.destroy"})
- * public class Cache {...}
+ * public class Cache {
+ * <a href="MBeanRegistration.html#injection">{@code @Resource}</a>
+ * private volatile SendNotification sendNotification;
+ * ...
+ * }
* </pre>
*
* <p>Each {@code @NotificationInfo} produces an {@link
@@ -64,6 +74,13 @@
* several {@code @NotificationInfo} annotations into a containing
* {@link NotificationInfos @NotificationInfos} annotation.
*
+ * <p>The {@code @NotificationInfo} and {@code @NotificationInfos} annotations
+ * are ignored on an MBean that is not a {@linkplain JMX#isNotificationSource
+ * notification source} or that implements {@link NotificationBroadcaster} and
+ * returns a non-empty array from its {@link
+ * NotificationBroadcaster#getNotificationInfo() getNotificationInfo()}
+ * method.</p>
+ *
* <p>The {@code NotificationInfo} and {@code NotificationInfos}
* annotations can be applied to the MBean implementation class, or to
* any parent class or interface. These annotations on a class take
@@ -71,7 +88,8 @@
* If an MBean does not have these annotations on its class or any
* superclass, then superinterfaces are examined. It is an error for
* more than one superinterface to have these annotations, unless one
- * of them is a child of all the others.</p>
+ * of them is a descendant of all the others; registering such an erroneous
+ * MBean will cause a {@link NotCompliantMBeanException}.</p>
*/
@Documented
@Inherited
--- a/jdk/test/javax/management/Introspector/AnnotatedNotificationInfoTest.java Fri Dec 05 21:59:09 2008 -0800
+++ b/jdk/test/javax/management/Introspector/AnnotatedNotificationInfoTest.java Tue Dec 09 12:01:07 2008 +0100
@@ -38,19 +38,34 @@
import javax.management.Description;
import javax.management.Descriptor;
import javax.management.ImmutableDescriptor;
+import javax.management.ListenerNotFoundException;
import javax.management.MBean;
import javax.management.MBeanInfo;
import javax.management.MBeanNotificationInfo;
import javax.management.MBeanServer;
import javax.management.MXBean;
+import javax.management.Notification;
+import javax.management.NotificationBroadcaster;
import javax.management.NotificationBroadcasterSupport;
+import javax.management.NotificationFilter;
import javax.management.NotificationInfo;
import javax.management.NotificationInfos;
+import javax.management.NotificationListener;
import javax.management.ObjectName;
import javax.management.SendNotification;
public class AnnotatedNotificationInfoTest {
- // Data for the first test. This tests that MBeanNotificationInfo
+
+ static final Descriptor expectedDescriptor = new ImmutableDescriptor(
+ "foo=bar", "descriptionResourceBundleBaseName=bundle",
+ "descriptionResourceKey=key");
+ static final MBeanNotificationInfo expected = new MBeanNotificationInfo(
+ new String[] {"foo", "bar"},
+ AttributeChangeNotification.class.getName(),
+ "description",
+ expectedDescriptor);
+
+ // Data for the first kind of test. This tests that MBeanNotificationInfo
// is correctly derived from @NotificationInfo.
// Every static field called mbean* is expected to be an MBean
// with a single MBeanNotificationInfo that has the same value
@@ -254,11 +269,48 @@
private static Object mbeanMXBean2 = new MXBean2();
- // Classes for the second test. This tests the simplest case, which is
- // the first example in the javadoc for @NotificationInfo. Notice that
- // this MBean is not a NotificationBroadcaster and does not inject a
- // SendNotification! That should possibly be an error, but it's currently
- // allowed by the spec.
+ // Test that @NotificationInfo and @NotificationInfos are ignored if
+ // the MBean returns a non-empty MBeanNotificationInfo[] from its
+ // NotificationBroadcaster.getNotifications() implementation.
+
+ @NotificationInfo(types={"blim", "blam"})
+ public static interface Explicit1MBean {}
+
+ public static class Explicit1
+ extends NotificationBroadcasterSupport implements Explicit1MBean {
+ public Explicit1() {
+ super(expected);
+ }
+ }
+
+ private static Object mbeanExplicit1 = new Explicit1();
+
+ @NotificationInfos(
+ {
+ @NotificationInfo(types="blim"), @NotificationInfo(types="blam")
+ }
+ )
+ public static interface Explicit2MXBean {}
+
+ public static class Explicit2
+ implements NotificationBroadcaster, Explicit2MXBean {
+ public void addNotificationListener(NotificationListener listener,
+ NotificationFilter filter, Object handback) {}
+
+ public void removeNotificationListener(NotificationListener listener)
+ throws ListenerNotFoundException {}
+
+ public MBeanNotificationInfo[] getNotificationInfo() {
+ return new MBeanNotificationInfo[] {expected};
+ }
+ }
+
+ // Data for the second kind of test. This tests that @NotificationInfo is
+ // ignored if the MBean is not a notification source. Every static
+ // field called ignoredMBean* is expected to be an MBean on which
+ // isInstanceOf(NotificationBroadcaster.class.getName() is false,
+ // addNotificationListener produces an exception, and the
+ // MBeanNotificationInfo array is empty.
@NotificationInfo(types={"com.example.notifs.create",
"com.example.notifs.destroy"})
public static interface CacheMBean {
@@ -271,6 +323,73 @@
}
}
+ private static Object ignoredMBean1 = new Cache();
+
+ @NotificationInfos(
+ @NotificationInfo(types={"foo", "bar"})
+ )
+ public static interface Cache2MBean {
+ public int getCachedNum();
+ }
+
+ public static class Cache2 implements Cache2MBean {
+ public int getCachedNum() {
+ return 0;
+ }
+ }
+
+ private static Object ignoredMBean2 = new Cache2();
+
+ private static final NotificationListener nullListener =
+ new NotificationListener() {
+ public void handleNotification(
+ Notification notification, Object handback) {}
+ };
+
+ // Test that inheriting inconsistent @NotificationInfo annotations is
+ // an error, but not if they are overridden by a non-empty getNotifications()
+
+ @NotificationInfo(types={"blim"})
+ public static interface Inconsistent1 {}
+
+ @NotificationInfo(types={"blam"})
+ public static interface Inconsistent2 {}
+
+ public static interface InconsistentMBean extends Inconsistent1, Inconsistent2 {}
+
+ public static class Inconsistent
+ extends NotificationBroadcasterSupport implements InconsistentMBean {}
+
+ public static class Consistent
+ extends Inconsistent implements NotificationBroadcaster {
+ public void addNotificationListener(NotificationListener listener,
+ NotificationFilter filter, Object handback) {}
+
+ public void removeNotificationListener(NotificationListener listener)
+ throws ListenerNotFoundException {}
+
+ public MBeanNotificationInfo[] getNotificationInfo() {
+ return new MBeanNotificationInfo[] {expected};
+ }
+ }
+
+ private static Object mbeanConsistent = new Consistent();
+
+ @NotificationInfo(
+ types = {"foo", "bar"},
+ notificationClass = AttributeChangeNotification.class,
+ description = @Description(
+ value = "description",
+ bundleBaseName = "bundle",
+ key = "key"),
+ descriptorFields = {"foo=bar"})
+ public static interface Consistent2MBean extends Inconsistent1, Inconsistent2 {}
+
+ public static class Consistent2
+ extends NotificationBroadcasterSupport implements Consistent2MBean {}
+
+ private static Object mbeanConsistent2 = new Consistent2();
+
public static void main(String[] args) throws Exception {
if (!AnnotatedNotificationInfoTest.class.desiredAssertionStatus())
throw new Exception("Test must be run with -ea");
@@ -278,37 +397,46 @@
MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
ObjectName on = new ObjectName("a:b=c");
- Descriptor expectedDescriptor = new ImmutableDescriptor(
- "foo=bar", "descriptionResourceBundleBaseName=bundle",
- "descriptionResourceKey=key");
- MBeanNotificationInfo expected = new MBeanNotificationInfo(
- new String[] {"foo", "bar"},
- AttributeChangeNotification.class.getName(),
- "description",
- expectedDescriptor);
-
System.out.println("Testing MBeans...");
for (Field mbeanField :
AnnotatedNotificationInfoTest.class.getDeclaredFields()) {
- if (!mbeanField.getName().startsWith("mbean"))
+ boolean notifier;
+ if (mbeanField.getName().startsWith("mbean"))
+ notifier = true;
+ else if (mbeanField.getName().startsWith("ignoredMBean"))
+ notifier = false;
+ else
continue;
System.out.println("..." + mbeanField.getName());
Object mbean = mbeanField.get(null);
mbs.registerMBean(mbean, on);
MBeanInfo mbi = mbs.getMBeanInfo(on);
MBeanNotificationInfo[] mbnis = mbi.getNotifications();
- assert mbnis.length == 1 : mbnis.length;
- assert mbnis[0].equals(expected) : mbnis[0];
+ if (notifier) {
+ assert mbnis.length == 1 : mbnis.length;
+ assert mbnis[0].equals(expected) : mbnis[0];
+ } else {
+ assert mbnis.length == 0 : mbnis.length;
+ assert !mbs.isInstanceOf(on, NotificationBroadcaster.class.getName());
+ try {
+ mbs.addNotificationListener(on, nullListener, null, null);
+ assert false : "addNotificationListener works";
+ } catch (Exception e) {
+ // OK: addNL correctly refused
+ }
+ }
mbs.unregisterMBean(on);
}
- mbs.registerMBean(new Cache(), on);
- MBeanInfo mbi = mbs.getMBeanInfo(on);
- MBeanNotificationInfo[] mbnis = mbi.getNotifications();
- assert mbnis.length == 1 : mbnis.length;
- String[] types = mbnis[0].getNotifTypes();
- String[] expectedTypes =
- CacheMBean.class.getAnnotation(NotificationInfo.class).types();
- assert Arrays.equals(types, expectedTypes) : Arrays.toString(types);
+ // Test that inconsistent @NotificationInfo annotations produce an
+ // error.
+ try {
+ mbs.registerMBean(new Inconsistent(), on);
+ System.out.println(mbs.getMBeanInfo(on));
+ assert false : "Inconsistent @NotificationInfo not detected";
+ } catch (Exception e) {
+ System.out.println(
+ "Inconsistent @NotificationInfo correctly produced " + e);
+ }
}
}