Issue #2727 - Revisit JMX MBean lookup behavior.

Improved lookup of MBean constructor, now done only once.
Cleaned up tests.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.jmx;
 
 import java.io.IOException;
+import java.lang.reflect.Constructor;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -34,7 +35,9 @@ import javax.management.InstanceNotFoundException;
 import javax.management.MBeanRegistrationException;
 import javax.management.MBeanServer;
 import javax.management.ObjectName;
+import javax.management.modelmbean.ModelMBean;
 
+import org.eclipse.jetty.util.Loader;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.component.Container;
@@ -151,7 +154,7 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable, De
         if (mbean instanceof ObjectMBean)
             ((ObjectMBean)mbean).setMBeanContainer(container);
         if (LOG.isDebugEnabled())
-            LOG.debug("mbeanFor {} is {}", o, mbean);
+            LOG.debug("MBean for {} is {}", o, mbean);
         return mbean;
     }
 
@@ -161,7 +164,11 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable, De
             return null;
         MetaData metaData = getMetaData(container, klass);
         if (metaData != null)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug("Found cached {}", metaData);
             return metaData;
+        }
         return newMetaData(container, klass);
     }
 
@@ -175,12 +182,12 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable, De
         if (klass == null)
             return null;
         if (klass == Object.class)
-            return new MetaData(klass, null, Collections.emptyList());
+            return new MetaData(klass, null, null, Collections.emptyList());
 
         List<MetaData> interfaces = Arrays.stream(klass.getInterfaces())
-                .map(iClass -> findMetaData(container, iClass))
+                .map(intf -> findMetaData(container, intf))
                 .collect(Collectors.toList());
-        MetaData metaData = new MetaData(klass, findMetaData(container, klass.getSuperclass()), interfaces);
+        MetaData metaData = new MetaData(klass, findConstructor(klass), findMetaData(container, klass.getSuperclass()), interfaces);
 
         if (container != null)
         {
@@ -195,6 +202,29 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable, De
         return metaData;
     }
 
+    private static Constructor<?> findConstructor(Class<?> klass)
+    {
+        String pName = klass.getPackage().getName();
+        String cName = klass.getName().substring(pName.length() + 1);
+        String mName = pName + ".jmx." + cName + "MBean";
+        try
+        {
+            Class<?> mbeanClass = Loader.loadClass(mName);
+            Constructor<?> constructor = ModelMBean.class.isAssignableFrom(mbeanClass)
+                    ? mbeanClass.getConstructor()
+                    : mbeanClass.getConstructor(Object.class);
+            if (LOG.isDebugEnabled())
+                LOG.debug("Found MBean wrapper: {} for {}", mName, klass.getName());
+            return constructor;
+        }
+        catch (Throwable x)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug("MBean wrapper not found: {} for {}", mName, klass.getName());
+            return null;
+        }
+    }
+
     /**
      * Lookup an object name by instance
      *

jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java

@@ -45,7 +45,6 @@ import javax.management.ObjectName;
 import javax.management.ReflectionException;
 import javax.management.modelmbean.ModelMBean;
 
-import org.eclipse.jetty.util.Loader;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.annotation.ManagedOperation;
@@ -59,21 +58,24 @@ class MetaData
 
     private final Map<String, AttributeInfo> _attributes = new HashMap<>();
     private final Map<String, OperationInfo> _operations = new HashMap<>();
+    private final Class<?> _klass;
     private final MetaData _parent;
     private final List<MetaData> _interfaces;
     private final Constructor<?> _constructor;
     private final MBeanInfo _info;
 
-    MetaData(Class<?> klass, MetaData parent, List<MetaData> interfaces)
+    MetaData(Class<?> klass, Constructor<?> constructor, MetaData parent, List<MetaData> interfaces)
     {
+        _klass = klass;
         _parent = parent;
         _interfaces = interfaces;
-        _constructor = findConstructor(klass);
+        _constructor = constructor;
         if (_constructor != null)
             parseMethods(klass, _constructor.getDeclaringClass());
         else
             parseMethods(klass);
         _info = buildMBeanInfo(klass);
+
     }
 
     Object newInstance(Object bean)
@@ -93,7 +95,7 @@ class MetaData
         return _info;
     }
 
-    Object getAttribute(String name, ObjectMBean mbean) throws AttributeNotFoundException, ReflectionException
+    Object getAttribute(String name, ObjectMBean mbean) throws AttributeNotFoundException, ReflectionException, MBeanException
     {
         AttributeInfo info = findAttribute(name);
         if (info == null)
@@ -101,7 +103,7 @@ class MetaData
         return info.getAttribute(mbean);
     }
 
-    void setAttribute(Attribute attribute, ObjectMBean mbean) throws AttributeNotFoundException, ReflectionException
+    void setAttribute(Attribute attribute, ObjectMBean mbean) throws AttributeNotFoundException, ReflectionException, MBeanException
     {
         if (attribute == null)
             return;
@@ -114,6 +116,8 @@ class MetaData
 
     private AttributeInfo findAttribute(String name)
     {
+        if (name == null)
+            return null;
         AttributeInfo result = _attributes.get(name);
         if (result != null)
             return result;
@@ -153,24 +157,6 @@ class MetaData
         return null;
     }
 
-    private static Constructor<?> findConstructor(Class<?> klass)
-    {
-        try
-        {
-            String pName = klass.getPackage().getName();
-            String cName = klass.getName().substring(pName.length() + 1);
-            String mName = pName + ".jmx." + cName + "MBean";
-            Class<?> mbeanClass = Loader.loadClass(mName);
-            return ModelMBean.class.isAssignableFrom(mbeanClass)
-                    ? mbeanClass.getConstructor()
-                    : mbeanClass.getConstructor(Object.class);
-        }
-        catch (Throwable x)
-        {
-            return null;
-        }
-    }
-
     private static Object newInstance(Constructor<?> constructor, Object bean)
     {
         try
@@ -190,6 +176,7 @@ class MetaData
     {
         for (Class<?> klass : classes)
         {
+            // Only work on the public method of the class, not of the hierarchy.
             for (Method method : klass.getDeclaredMethods())
             {
                 if (!Modifier.isPublic(method.getModifiers()))
@@ -198,12 +185,16 @@ class MetaData
                 if (attribute != null)
                 {
                     AttributeInfo info = new AttributeInfo(attribute, method);
+                    if (LOG.isDebugEnabled())
+                        LOG.debug("Found attribute for {}: {}", klass.getName(), info);
                     _attributes.put(info._name, info);
                 }
                 ManagedOperation operation = method.getAnnotation(ManagedOperation.class);
                 if (operation != null)
                 {
                     OperationInfo info = new OperationInfo(operation, method);
+                    if (LOG.isDebugEnabled())
+                        LOG.debug("Found operation for {}: {}", klass.getName(), info);
                     _operations.put(info._name, info);
                 }
             }
@@ -284,6 +275,21 @@ class MetaData
             _parent.collectMBeanOperationInfos(operationInfos);
     }
 
+    private static MBeanException toMBeanException(InvocationTargetException x)
+    {
+        Throwable cause = x.getCause();
+        if (cause instanceof Exception)
+            return new MBeanException((Exception)cause);
+        else
+            return new MBeanException(x);
+    }
+
+    @Override
+    public String toString()
+    {
+        return String.format("%s@%x[%s]", getClass().getSimpleName(), hashCode(), _klass.getName());
+    }
+
     private static class AttributeInfo
     {
         private final String _name;
@@ -318,7 +324,7 @@ class MetaData
                             _setter != null, getter.getName().startsWith("is"));
         }
 
-        Object getAttribute(ObjectMBean mbean) throws ReflectionException
+        Object getAttribute(ObjectMBean mbean) throws ReflectionException, MBeanException
         {
             try
             {
@@ -338,13 +344,17 @@ class MetaData
                     names[i] = mbean.findObjectName(Array.get(result, i));
                 return names;
             }
+            catch (InvocationTargetException x)
+            {
+                throw toMBeanException(x);
+            }
             catch (Exception x)
             {
                 throw new ReflectionException(x);
             }
         }
 
-        void setAttribute(Object value, ObjectMBean mbean) throws ReflectionException
+        void setAttribute(Object value, ObjectMBean mbean) throws ReflectionException, MBeanException
         {
             try
             {
@@ -370,6 +380,10 @@ class MetaData
                     Array.set(result, i, mbean.findBean(names[i]));
                 _setter.invoke(target, result);
             }
+            catch (InvocationTargetException x)
+            {
+                throw toMBeanException(x);
+            }
             catch (Exception x)
             {
                 throw new ReflectionException(x);
@@ -404,6 +418,13 @@ class MetaData
 
             return setter;
         }
+
+        @Override
+        public String toString()
+        {
+            return String.format("%s@%x[%s,proxied=%b,convert=%b]", getClass().getSimpleName(), hashCode(),
+                    _name, _proxied, _convert);
+        }
     }
 
     private static class OperationInfo
@@ -464,11 +485,7 @@ class MetaData
             }
             catch (InvocationTargetException x)
             {
-                Throwable cause = x.getCause();
+                throw toMBeanException(x);
-                if (cause instanceof Exception)
-                    throw new MBeanException((Exception)cause);
-                else
-                    throw new MBeanException(new RuntimeException(cause));
             }
             catch (Exception x)
             {
@@ -498,5 +515,12 @@ class MetaData
             }
             return result;
         }
+
+        @Override
+        public String toString()
+        {
+            return String.format("%s@%x[%s,proxied=%b,convert=%b]", getClass().getSimpleName(), hashCode(),
+                    _name, _proxied, _convert);
+        }
     }
 }

jetty-jmx/src/main/java/org/eclipse/jetty/jmx/ObjectMBean.java

@@ -139,7 +139,7 @@ public class ObjectMBean implements DynamicMBean
     }
 
     @Override
-    public Object getAttribute(String name) throws AttributeNotFoundException, ReflectionException
+    public Object getAttribute(String name) throws AttributeNotFoundException, ReflectionException, MBeanException
     {
         ClassLoader prevLoader = Thread.currentThread().getContextClassLoader();
         try
@@ -164,14 +164,15 @@ public class ObjectMBean implements DynamicMBean
             }
             catch (Throwable x)
             {
-                LOG.info(x);
+                if (LOG.isDebugEnabled())
+                    LOG.debug(x);
             }
         }
         return results;
     }
 
     @Override
-    public void setAttribute(Attribute attribute) throws AttributeNotFoundException, ReflectionException
+    public void setAttribute(Attribute attribute) throws AttributeNotFoundException, ReflectionException, MBeanException
     {
         ClassLoader prevLoader = Thread.currentThread().getContextClassLoader();
         try
@@ -197,7 +198,8 @@ public class ObjectMBean implements DynamicMBean
             }
             catch (Throwable x)
             {
-                LOG.info(x);
+                if (LOG.isDebugEnabled())
+                    LOG.debug(x);
             }
         }
         return results;
@@ -227,7 +229,7 @@ public class ObjectMBean implements DynamicMBean
         return _mbeanContainer.findBean(objectName);
     }
 
-    private MetaData metaData()
+    MetaData metaData()
     {
         if (_metaData == null)
             _metaData = MBeanContainer.findMetaData(_mbeanContainer, _managed.getClass());

jetty-jmx/src/test/java/com/acme/DerivedExtended.java

@@ -25,7 +25,6 @@ import org.eclipse.jetty.util.annotation.ManagedOperation;
 @ManagedObject(value = "Test the mbean extended stuff")
 public class DerivedExtended extends Derived
 {
-
     private String doodle4 = "doodle4";
 
     @ManagedAttribute(value = "The doodle4 name of something", name = "doodle4", setter = "setDoodle4")

jetty-jmx/src/test/java/org/eclipse/jetty/jmx/MBeanContainerTest.java

@@ -18,24 +18,23 @@
 
 package org.eclipse.jetty.jmx;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
-import com.acme.Managed;
-
 import java.lang.management.ManagementFactory;
 
 import javax.management.MBeanServer;
 import javax.management.ObjectName;
 
+import com.acme.Managed;
 import org.eclipse.jetty.util.component.Container;
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 public class MBeanContainerTest
 {
     private MBeanContainer mbeanContainer;
@@ -54,27 +53,21 @@ public class MBeanContainerTest
     @Test
     public void testMakeName()
     {
-        // given
         beanName = "mngd:bean";
 
-        // when
         beanName = mbeanContainer.makeName(beanName);
 
-        // then
         assertEquals("mngd_bean", beanName, "Bean name should be mngd_bean");
     }
 
     @Test
     public void testFindBean()
     {
-        // given
         managed = getManaged();
 
-        // when
         objectName = mbeanContainer.findMBean(managed);
         assertNotNull(objectName);
 
-        // then
         assertEquals(managed, mbeanContainer.findBean(objectName), "Bean must be added");
         assertNull(mbeanContainer.findBean(null), "It must return null as there is no bean with the name null");
     }
@@ -104,40 +97,31 @@ public class MBeanContainerTest
     @Test
     public void testDomain()
     {
-        // given
         String domain = "Test";
 
-        // when
         mbeanContainer.setDomain(domain);
 
-        // then
         assertEquals(domain, mbeanContainer.getDomain(), "Domain name must be Test");
     }
 
     @Test
-    public void testBeanAdded() throws Exception
+    public void testBeanAdded()
     {
-        // given
         setBeanAdded();
 
-        // when
         objectName = mbeanContainer.findMBean(managed);
 
-        // then
         assertTrue(mbeanServer.isRegistered(objectName), "Bean must have been registered");
     }
 
     @Test
-    public void testBeanAddedNullCheck() throws Exception
+    public void testBeanAddedNullCheck()
     {
-        // given
         setBeanAdded();
         Integer mbeanCount = mbeanServer.getMBeanCount();
 
-        // when
         mbeanContainer.beanAdded(null, null);
 
-        // then
         assertEquals(mbeanCount, mbeanServer.getMBeanCount(), "MBean count must not change after beanAdded(null, null) call");
     }
 
@@ -150,15 +134,12 @@ public class MBeanContainerTest
     }
 
     @Test
-    public void testBeanRemoved() throws Exception
+    public void testBeanRemoved()
     {
-        // given
         setUpBeanRemoved();
 
-        // when
         mbeanContainer.beanRemoved(null, managed);
 
-        // then
         assertNull(mbeanContainer.findMBean(managed), "Bean shouldn't be registered with container as we removed the bean");
     }
 
@@ -200,30 +181,24 @@ public class MBeanContainerTest
     }
 
     @Test
-    public void testDestroy() throws Exception
+    public void testDestroy()
     {
-        // given
         setUpDestroy();
 
-        // when
         objectName = mbeanContainer.findMBean(managed);
         mbeanContainer.destroy();
 
-        // then
         assertFalse(mbeanContainer.getMBeanServer().isRegistered(objectName), "Unregistered bean - managed");
     }
 
     @Test
     public void testDestroyInstanceNotFoundException() throws Exception
     {
-        // given
         setUpDestroy();
 
-        // when
         objectName = mbeanContainer.findMBean(managed);
         mbeanContainer.getMBeanServer().unregisterMBean(objectName);
 
-        // then
         assertFalse(mbeanContainer.getMBeanServer().isRegistered(objectName), "Unregistered bean - managed");
         // this flow covers InstanceNotFoundException. Actual code just eating
         // the exception. i.e Actual code just printing the stacktrace, whenever

jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java

@@ -26,202 +26,126 @@ import javax.management.MBeanOperationInfo;
 import javax.management.MBeanParameterInfo;
 
 import com.acme.Derived;
-import org.eclipse.jetty.util.log.Log;
+import com.acme.Managed;
-import org.eclipse.jetty.util.log.Logger;
-import org.eclipse.jetty.util.thread.QueuedThreadPool;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class ObjectMBeanTest
 {
-    private static final Logger LOG = Log.getLogger(ObjectMBeanTest.class);
+    private MBeanContainer container;
-
-    private static MBeanContainer container;
 
     @BeforeEach
-    public void before() throws Exception
+    public void before()
     {
         container = new MBeanContainer(ManagementFactory.getPlatformMBeanServer());
     }
 
     @AfterEach
-    public void after() throws Exception
+    public void after()
     {
         container.destroy();
         container = null;
     }
 
-    /*
+    @Test
-     * this test uses the com.acme.Derived test classes
+    public void testMetaDataCaching()
-     */
+    {
+        Derived derived = new Derived();
+        ObjectMBean derivedMBean = (ObjectMBean)container.mbeanFor(derived);
+        ObjectMBean derivedMBean2 = (ObjectMBean)container.mbeanFor(derived);
+        assertNotSame(derivedMBean, derivedMBean2);
+        assertSame(derivedMBean.metaData(), derivedMBean2.metaData());
+    }
+
     @Test
     public void testDerivedAttributes() throws Exception
     {
         Derived derived = new Derived();
-        ObjectMBean mbean = (ObjectMBean)ObjectMBean.mbeanFor(derived);
+        Managed managed = derived.getManagedInstance();
+        ObjectMBean derivedMBean = (ObjectMBean)container.mbeanFor(derived);
+        ObjectMBean managedMBean = (ObjectMBean)container.mbeanFor(managed);
 
-        ObjectMBean managed = (ObjectMBean)ObjectMBean.mbeanFor(derived.getManagedInstance());
+        container.beanAdded(null, derived);
-        mbean.setMBeanContainer(container);
+        container.beanAdded(null, managed);
-        managed.setMBeanContainer(container);
 
-        container.beanAdded(null,derived);
+        MBeanInfo derivedInfo = derivedMBean.getMBeanInfo();
-        container.beanAdded(null,derived.getManagedInstance());
+        assertNotNull(derivedInfo);
+        MBeanInfo managedInfo = managedMBean.getMBeanInfo();
+        assertNotNull(managedInfo);
 
-        MBeanInfo toss = managed.getMBeanInfo();
+        assertEquals("com.acme.Derived", derivedInfo.getClassName(), "name does not match");
+        assertEquals("Test the mbean stuff", derivedInfo.getDescription(), "description does not match");
+        assertEquals(6, derivedInfo.getAttributes().length, "attribute count does not match");
+        assertEquals("Full Name", derivedMBean.getAttribute("fname"), "attribute values does not match");
 
-        assertNotNull(mbean.getMBeanInfo());
+        derivedMBean.setAttribute(new Attribute("fname", "Fuller Name"));
-
+        assertEquals("Fuller Name", derivedMBean.getAttribute("fname"), "set attribute value does not match");
-        MBeanInfo info = mbean.getMBeanInfo();
+        assertEquals("goop", derivedMBean.getAttribute("goop"), "proxy attribute values do not match");
-
-        assertEquals("com.acme.Derived", info.getClassName(), "name does not match");
-        assertEquals("Test the mbean stuff", info.getDescription(), "description does not match");
-
-        // for ( MBeanAttributeInfo i : info.getAttributes())
-        // {
-        // LOG.debug(i.toString());
-        // }
-
-        /*
-         * 2 attributes from lifecycle and 2 from Derived and 1 from MBean
-         */
-        assertEquals(6, info.getAttributes().length, "attribute count does not match");
-
-        assertEquals("Full Name", mbean.getAttribute("fname"), "attribute values does not match");
-
-        mbean.setAttribute(new Attribute("fname","Fuller Name"));
-
-        assertEquals("Fuller Name", mbean.getAttribute("fname"), "set attribute value does not match");
-
-        assertEquals("goop", mbean.getAttribute("goop"), "proxy attribute values do not match");
-
-        // Thread.sleep(100000);
     }
 
     @Test
     public void testDerivedOperations() throws Exception
     {
         Derived derived = new Derived();
-        ObjectMBean mbean = (ObjectMBean)ObjectMBean.mbeanFor(derived);
+        ObjectMBean mbean = (ObjectMBean)container.mbeanFor(derived);
 
-        mbean.setMBeanContainer(container);
+        container.beanAdded(null, derived);
-
-        container.beanAdded(null,derived);
 
         MBeanInfo info = mbean.getMBeanInfo();
-
         assertEquals(5, info.getOperations().length, "operation count does not match");
 
-        MBeanOperationInfo[] opinfos = info.getOperations();
+        MBeanOperationInfo[] operationInfos = info.getOperations();
         boolean publish = false;
         boolean doodle = false;
         boolean good = false;
-        for (int i = 0; i < opinfos.length; ++i)
+        for (MBeanOperationInfo operationInfo : operationInfos)
         {
-            MBeanOperationInfo opinfo = opinfos[i];
+            if ("publish".equals(operationInfo.getName()))
-
-            if ("publish".equals(opinfo.getName()))
             {
                 publish = true;
-                assertEquals("publish something", opinfo.getDescription(), "description doesn't match");
+                assertEquals("publish something", operationInfo.getDescription(), "description doesn't match");
             }
 
-            if ("doodle".equals(opinfo.getName()))
+            if ("doodle".equals(operationInfo.getName()))
             {
                 doodle = true;
-                assertEquals("Doodle something", opinfo.getDescription(), "description doesn't match");
+                assertEquals("Doodle something", operationInfo.getDescription(), "description doesn't match");
-
+                MBeanParameterInfo[] parameterInfos = operationInfo.getSignature();
-                MBeanParameterInfo[] pinfos = opinfo.getSignature();
+                assertEquals("A description of the argument", parameterInfos[0].getDescription(), "parameter description doesn't match");
-
+                assertEquals("doodle", parameterInfos[0].getName(), "parameter name doesn't match");
-                assertEquals("A description of the argument", pinfos[0].getDescription(), "parameter description doesn't match");
-                assertEquals("doodle", pinfos[0].getName(), "parameter name doesn't match");
             }
 
-            // This is a proxied operation on the JMX wrapper
+            // This is a proxied operation on the MBean wrapper.
-            if ("good".equals(opinfo.getName()))
+            if ("good".equals(operationInfo.getName()))
             {
                 good = true;
-
+                assertEquals("test of proxy operations", operationInfo.getDescription(), "description does not match");
-                assertEquals("test of proxy operations", opinfo.getDescription(), "description does not match");
+                assertEquals("not bad", mbean.invoke("good", new Object[]{}, new String[]{}), "execution contexts wrong");
-                assertEquals("not bad",mbean.invoke("good",new Object[] {}, new String[] {}), "execution contexts wrong");
             }
         }
 
         assertTrue(publish, "publish operation was not not found");
         assertTrue(doodle, "doodle operation was not not found");
         assertTrue(good, "good operation was not not found");
-
     }
 
     @Test
-    public void testDerivedObjectAttributes() throws Exception
+    public void testMethodNameMining()
     {
-        Derived derived = new Derived();
+        assertEquals("fullName", MetaData.toAttributeName("getFullName"));
-        ObjectMBean mbean = (ObjectMBean)ObjectMBean.mbeanFor(derived);
+        assertEquals("fullName", MetaData.toAttributeName("getfullName"));
-
+        assertEquals("fullName", MetaData.toAttributeName("isFullName"));
-        ObjectMBean managed = (ObjectMBean)ObjectMBean.mbeanFor(derived.getManagedInstance());
+        assertEquals("fullName", MetaData.toAttributeName("isfullName"));
-        mbean.setMBeanContainer(container);
+        assertEquals("fullName", MetaData.toAttributeName("setFullName"));
-        managed.setMBeanContainer(container);
+        assertEquals("fullName", MetaData.toAttributeName("setfullName"));
-
+        assertEquals("fullName", MetaData.toAttributeName("FullName"));
-        assertNotNull(mbean.getMBeanInfo());
+        assertEquals("fullName", MetaData.toAttributeName("fullName"));
-
-        container.beanAdded(null,derived);
-        container.beanAdded(null,derived.getManagedInstance());
-        container.beanAdded(null,mbean);
-        container.beanAdded(null,managed);
-
-        // Managed managedInstance = (Managed)mbean.getAttribute("managedInstance");
-        // assertNotNull(managedInstance);
-        // assertEquals("foo", managedInstance.getManaged(), "managed instance returning nonsense");
-
     }
-
-    @Test
-    @Disabled("ignore, used in testing jconsole atm")
-    public void testThreadPool() throws Exception
-    {
-
-        Derived derived = new Derived();
-        ObjectMBean mbean = (ObjectMBean)ObjectMBean.mbeanFor(derived);
-
-        ObjectMBean managed = (ObjectMBean)ObjectMBean.mbeanFor(derived.getManagedInstance());
-        mbean.setMBeanContainer(container);
-        managed.setMBeanContainer(container);
-
-        QueuedThreadPool qtp = new QueuedThreadPool();
-
-        ObjectMBean bqtp = (ObjectMBean)ObjectMBean.mbeanFor(qtp);
-
-        bqtp.getMBeanInfo();
-
-        container.beanAdded(null,derived);
-        container.beanAdded(null,derived.getManagedInstance());
-        container.beanAdded(null,mbean);
-        container.beanAdded(null,managed);
-        container.beanAdded(null,qtp);
-
-        Thread.sleep(10000000);
-
-    }
-
-    @Test
-    public void testMethodNameMining() throws Exception
-    {
-        ObjectMBean mbean = new ObjectMBean(new Derived());
-
-        assertEquals("fullName",MetaData.toAttributeName("getFullName"));
-        assertEquals("fullName",MetaData.toAttributeName("getfullName"));
-        assertEquals("fullName",MetaData.toAttributeName("isFullName"));
-        assertEquals("fullName",MetaData.toAttributeName("isfullName"));
-        assertEquals("fullName",MetaData.toAttributeName("setFullName"));
-        assertEquals("fullName",MetaData.toAttributeName("setfullName"));
-        assertEquals("fullName",MetaData.toAttributeName("FullName"));
-        assertEquals("fullName",MetaData.toAttributeName("fullName"));
-    }
-
 }

jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanUtilTest.java

@@ -32,11 +32,6 @@ import javax.management.ReflectionException;
 import com.acme.Derived;
 import com.acme.DerivedExtended;
 import com.acme.DerivedManaged;
-import org.eclipse.jetty.util.log.Log;
-import org.eclipse.jetty.util.log.Logger;
-import org.eclipse.jetty.util.log.StdErrLog;
-import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -47,56 +42,20 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class ObjectMBeanUtilTest
 {
-
     private ObjectMBean objectMBean;
-
     private DerivedExtended derivedExtended;
-
     private MBeanContainer container;
-
     private MBeanInfo objectMBeanInfo;
-
-    private Object mBean;
-
-    private String value;
-
     private Attribute attribute;
-
-    private AttributeList attributes;
-
     private ObjectMBean mBeanDerivedManaged;
-
-    private Derived[] derivedes;
-
-    private ArrayList<Derived> aliasNames;
-
     private DerivedManaged derivedManaged;
 
-    private static final int EMPTY = 0;
-
-    @BeforeAll
-    public static void beforeClass()
-    {
-        Logger ombLog = Log.getLogger(ObjectMBean.class);
-        if (ombLog instanceof StdErrLog && !ombLog.isDebugEnabled())
-            ((StdErrLog)ombLog).setHideStacks(true);
-    }
-
-    @AfterAll
-    public static void afterClass()
-    {
-        Logger ombLog = Log.getLogger(ObjectMBean.class);
-        if (ombLog instanceof StdErrLog)
-            ((StdErrLog)ombLog).setHideStacks(false);
-    }
-
     @BeforeEach
     public void setUp()
     {
-        derivedExtended = new DerivedExtended();
-        objectMBean = new ObjectMBean(derivedExtended);
         container = new MBeanContainer(ManagementFactory.getPlatformMBeanServer());
-        objectMBean.setMBeanContainer(container);
+        derivedExtended = new DerivedExtended();
+        objectMBean = (ObjectMBean)container.mbeanFor(derivedExtended);
         objectMBeanInfo = objectMBean.getMBeanInfo();
     }
 
@@ -114,131 +73,93 @@ public class ObjectMBeanUtilTest
     @Test
     public void testMbeanForNullCheck()
     {
-        // when
+        Object mBean = container.mbeanFor(null);
-        mBean = ObjectMBean.mbeanFor(null);
-
-        // then
         assertNull(mBean, "As we are passing null value the output should be null");
     }
 
     @Test
-    public void testGetAttributeReflectionException() throws Exception
+    public void testGetAttributeMBeanException() throws Exception
     {
-        // given
+        Attribute attribute = new Attribute("doodle4", "charu");
-        setUpGetAttribute("doodle4","charu");
+        objectMBean.setAttribute(attribute);
 
-        // when
+        MBeanException e = assertThrows(MBeanException.class, () -> objectMBean.getAttribute("doodle4"));
-        ReflectionException e = assertThrows(ReflectionException.class, ()-> {
-            objectMBean.getAttribute("doodle4");
-        });
 
-        // then
         assertNotNull(e, "An InvocationTargetException must have occurred by now as doodle4() internally throwing exception");
     }
 
-    private void setUpGetAttribute(String property, String value) throws Exception
-    {
-        Attribute attribute = new Attribute(property,value);
-        objectMBean.setAttribute(attribute);
-    }
-
     @Test
-    public void testGetAttributeAttributeNotFoundException() throws Exception
+    public void testGetAttributeAttributeNotFoundException()
     {
-        // when
+        AttributeNotFoundException e = assertThrows(AttributeNotFoundException.class, () -> objectMBean.getAttribute("ffname"));
-        AttributeNotFoundException e = assertThrows(AttributeNotFoundException.class, ()->{
-            objectMBean.getAttribute("ffname");
-        });
 
-        // then
+        assertNotNull(e, "An AttributeNotFoundException must have occurred by now as there is no attribute with the name ffname in bean");
-        assertNotNull(e, "An AttributeNotFoundException must have occurred by now as there is no " + "attribute with the name ffname in bean");
     }
 
     @Test
     public void testSetAttributeWithCorrectAttrName() throws Exception
     {
-        // given
+        Attribute attribute = new Attribute("fname", "charu");
-        setUpGetAttribute("fname","charu");
+        objectMBean.setAttribute(attribute);
 
-        // when
+        String value = (String)objectMBean.getAttribute("fname");
-        value = (String)objectMBean.getAttribute("fname");
 
-        // then
+        assertEquals("charu", value, "Attribute(fname) value must be equal to charu");
-        assertEquals("charu", value, "Attribute(fname) value must be equl to charu");
     }
 
     @Test
     public void testSetAttributeNullCheck() throws Exception
     {
-        // given
         objectMBean.setAttribute(null);
 
-        // when
+        AttributeNotFoundException e = assertThrows(AttributeNotFoundException.class, () -> objectMBean.getAttribute(null));
-        AttributeNotFoundException e = assertThrows(AttributeNotFoundException.class, ()->{
-            objectMBean.getAttribute(null);
-        });
 
-        // then
+        assertNotNull(e, "An AttributeNotFoundException must have occurred by now as there is no attribute with the name null");
-        assertNotNull(e,"An AttributeNotFoundException must have occurred by now as there is no attribute with the name null");
     }
 
     @Test
-    public void testSetAttributeAttributeWithWrongAttrName() throws Exception
+    public void testSetAttributeAttributeWithWrongAttrName()
     {
-        // given
+        attribute = new Attribute("fnameee", "charu");
-        attribute = new Attribute("fnameee","charu");
 
-        // when
+        AttributeNotFoundException e = assertThrows(AttributeNotFoundException.class, () -> objectMBean.setAttribute(attribute));
-        AttributeNotFoundException e = assertThrows(AttributeNotFoundException.class, ()->{
-            objectMBean.setAttribute(attribute);
-        });
 
-        // then
         assertNotNull(e, "An AttributeNotFoundException must have occurred by now as there is no attribute " + "with the name ffname in bean");
     }
 
     @Test
-    public void testSetAttributesWithCorrectValues() throws Exception
+    public void testSetAttributesWithCorrectValues()
     {
-        // given
+        AttributeList attributes = getAttributes("fname", "vijay");
-        attributes = getAttributes("fname","vijay");
+        objectMBean.setAttributes(attributes);
-        attributes = objectMBean.setAttributes(attributes);
 
-        // when
+        attributes = objectMBean.getAttributes(new String[]{"fname"});
-        attributes = objectMBean.getAttributes(new String[]
-        { "fname" });
 
-        // then
+        assertEquals(1, attributes.size());
         assertEquals("vijay", ((Attribute)(attributes.get(0))).getValue(), "Fname value must be equal to vijay");
     }
 
     @Test
-    public void testSetAttributesForArrayTypeAttribue() throws Exception
+    public void testSetAttributesForArrayTypeAttribute() throws Exception
     {
-        // given
+        Derived[] deriveds = getArrayTypeAttribute();
-        derivedes = getArrayTypeAttribute();
 
-        // when
+        derivedManaged.setAddresses(deriveds);
-        derivedManaged.setAddresses(derivedes);
         mBeanDerivedManaged.getMBeanInfo();
 
-        // then
         assertNotNull(mBeanDerivedManaged.getAttribute("addresses"), "Address object shouldn't be null");
     }
 
     @Test
     public void testSetAttributesForCollectionTypeAttribue() throws Exception
     {
-        // given
+        ArrayList<Derived> aliasNames = new ArrayList<>(Arrays.asList(getArrayTypeAttribute()));
-        aliasNames = getCollectionTypeAttribute();
 
-        // when
         derivedManaged.setAliasNames(aliasNames);
         mBeanDerivedManaged.getMBeanInfo();
 
-        // then
         assertNotNull(mBeanDerivedManaged.getAttribute("aliasNames"), "Address object shouldn't be null");
-        assertNull(mBeanDerivedManaged.getAttribute("derived"), "Derived object shouldn't registerd with container so its value will be null");
+        assertNull(mBeanDerivedManaged.getAttribute("derived"), "Derived object shouldn't registered with container so its value will be null");
     }
 
     private Derived[] getArrayTypeAttribute()
@@ -248,119 +169,74 @@ public class ObjectMBeanUtilTest
         MBeanContainer mBeanDerivedManagedContainer = new MBeanContainer(ManagementFactory.getPlatformMBeanServer());
         mBeanDerivedManaged.setMBeanContainer(mBeanDerivedManagedContainer);
         Derived derived0 = new Derived();
-        mBeanDerivedManagedContainer.beanAdded(null,derived0);
+        mBeanDerivedManagedContainer.beanAdded(null, derived0);
-        Derived[] derivedes = new Derived[3];
+        Derived[] deriveds = new Derived[3];
         for (int i = 0; i < 3; i++)
-        {
+            deriveds[i] = new Derived();
-            derivedes[i] = new Derived();
+        derivedManaged.setAddresses(deriveds);
-        }
-        derivedManaged.setAddresses(derivedes);
         mBeanDerivedManaged.getMBeanInfo();
-        ArrayList<Derived> aliasNames = new ArrayList<Derived>(Arrays.asList(derivedes));
+        ArrayList<Derived> aliasNames = new ArrayList<>(Arrays.asList(deriveds));
         derivedManaged.setAliasNames(aliasNames);
-        return derivedes;
+        return deriveds;
-    }
-
-    private ArrayList<Derived> getCollectionTypeAttribute()
-    {
-        ArrayList<Derived> aliasNames = new ArrayList<Derived>(Arrays.asList(getArrayTypeAttribute()));
-        return aliasNames;
     }
 
     @Test
     public void testSetAttributesException()
     {
-        // given
+        AttributeList attributes = getAttributes("fnameee", "charu");
-        attributes = getAttributes("fnameee","charu");
 
-        // when
         attributes = objectMBean.setAttributes(attributes);
 
-        // then
         // Original code eating the exception and returning zero size list
-        assertEquals(EMPTY,attributes.size(),"As there is no attribute with the name fnameee, this should return empty");
+        assertEquals(0, attributes.size(), "As there is no attribute with the name fnameee, this should return empty");
     }
 
     private AttributeList getAttributes(String name, String value)
     {
-        Attribute attribute = new Attribute(name,value);
+        Attribute attribute = new Attribute(name, value);
         AttributeList attributes = new AttributeList();
         attributes.add(attribute);
         return attributes;
     }
 
     @Test
-    public void testInvokeMBeanException() throws Exception
+    public void testInvokeMBeanException()
     {
-        // given
+        ReflectionException e = assertThrows(ReflectionException.class, () -> objectMBean.invoke("doodle2", new Object[0], new String[0]));
-        setMBeanInfoForInvoke();
 
-        // when
-        ReflectionException e = assertThrows(ReflectionException.class, ()->{
-            objectMBean.invoke("doodle2",new Object[] {},new String[] {});
-        });
-
-        // then
         assertNotNull(e, "An ReflectionException must have occurred by now as doodle2() in Derived bean is private");
     }
 
     @Test
-    public void testInvokeReflectionException() throws Exception
+    public void testInvokeReflectionException()
     {
-        // given
+        MBeanException e = assertThrows(MBeanException.class, () -> objectMBean.invoke("doodle1", new Object[0], new String[0]));
-        setMBeanInfoForInvoke();
 
-        // when
-        MBeanException e = assertThrows(MBeanException.class, ()->{
-            objectMBean.invoke("doodle1",new Object[] {},new String[] {});
-        });
-
-        // then
         assertNotNull(e, "MBeanException is null");
     }
 
     @Test
     public void testInvoke() throws Exception
     {
-        // given
+        String value = (String)objectMBean.invoke("good", new Object[0], new String[0]);
-        setMBeanInfoForInvoke();
 
-        // when
-        value = (String)objectMBean.invoke("good",new Object[] {},new String[] {});
-
-        // then
         assertEquals("not bad", value, "Method(good) invocation on objectMBean must return not bad");
     }
 
     @Test
-    public void testInvokeNoSuchMethodException() throws Exception
+    public void testInvokeNoSuchMethodException()
     {
-        // given
+        // DerivedMBean contains a managed method with the name good,
-        setMBeanInfoForInvoke();
+        // we must call this method without any arguments.
+        ReflectionException e = assertThrows(ReflectionException.class, () ->
+                objectMBean.invoke("good", new Object[0], new String[]{"int aone"}));
 
-        // when
-        // DerivedMBean contains a managed method with the name good,we must
-        // call this method without any arguments
-        ReflectionException e = assertThrows(ReflectionException.class, ()->{
-            objectMBean.invoke("good",new Object[] {},new String[]
-                    { "int aone" });
-        });
-
-        // then
         assertNotNull(e, "An ReflectionException must have occurred by now as we cannot call a methow with wrong signature");
-
-    }
-
-    private void setMBeanInfoForInvoke()
-    {
-        objectMBean = (ObjectMBean)ObjectMBean.mbeanFor(derivedExtended);
-        container.beanAdded(null,derivedExtended);
-        objectMBean.getMBeanInfo();
     }
 
     @Test
-    public void testToVariableName()
+    public void testToAttributeName()
     {
-        assertEquals("fullName",MetaData.toAttributeName("isfullName"));
+        assertEquals("fullName", MetaData.toAttributeName("isfullName"));
     }
 }

jetty-jmx/src/test/java/org/eclipse/jetty/jmx/PojoTest.java

@@ -20,27 +20,25 @@ package org.eclipse.jetty.jmx;
 
 import java.util.Arrays;
 import java.util.List;
-import org.eclipse.jetty.util.log.jmx.LogMBean;
+
-import org.junit.jupiter.api.Test;
 import com.openpojo.reflection.impl.PojoClassFactory;
 import com.openpojo.validation.Validator;
 import com.openpojo.validation.ValidatorBuilder;
 import com.openpojo.validation.test.impl.GetterTester;
 import com.openpojo.validation.test.impl.SetterTester;
+import org.eclipse.jetty.util.log.jmx.LogMBean;
+import org.junit.jupiter.api.Test;
 
 /*
  * This class tests all the getters and setters for a given list of classes.
  */
 public class PojoTest
 {
-
-    private Validator validator;
-
     @Test
     public void testOpenPojo()
     {
-        validator = ValidatorBuilder.create().with(new SetterTester()).with(new GetterTester()).build();
+        Validator validator = ValidatorBuilder.create().with(new SetterTester()).with(new GetterTester()).build();
-        List<Class> classes = Arrays.asList(MBeanContainer.class,ObjectMBean.class,LogMBean.class);
+        List<Class> classes = Arrays.asList(MBeanContainer.class, ObjectMBean.class, LogMBean.class);
         for (Class clazz : classes)
         {
             validator.validate(PojoClassFactory.getPojoClass(clazz));

jetty-jmx/src/test/resources/jetty-logging.properties

@@ -1,2 +1,2 @@
 org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog
-org.eclipse.jetty.jmx.LEVEL=INFO
+#org.eclipse.jetty.jmx.LEVEL=DEBUG