QPID-5715: Unregister children MBeans on Virtual Host Node stop

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1601322 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 66 insertions, 13 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java
index 49400825c9..d0a065438e 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java
@@ -205,8 +205,8 @@ public abstract class AbstractVirtualHostNode<X extends AbstractVirtualHostNode<
     @StateTransition( currentState = { State.ACTIVE, State.ERRORED, State.UNINITIALIZED }, desiredState = State.STOPPED )
     protected void doStop()
     {
-        closeConfigurationStore();
         closeChildren();
+        closeConfigurationStore();
         _state.set(State.STOPPED);
     }
 
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java
index 74153b8672..3eec242703 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java
@@ -25,6 +25,7 @@ import java.io.IOException;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 
@@ -269,7 +270,8 @@ public class JMXManagementPluginImpl
 
             if(LOGGER.isDebugEnabled())
             {
-                LOGGER.debug("Provider " + provider + " mBean for child " + child + " " + mBean);
+                LOGGER.debug("Provider " + provider + (mBean == null ? " did not create mBean" : " created mBean " + mBean)
+                        + " for child " + child);
             }
         }
     }
@@ -377,18 +379,37 @@ public class JMXManagementPluginImpl
         return mbeans.containsKey(mBeanProvider);
     }
 
-    private void destroyObjectMBeans(ConfiguredObject<?> child, boolean removeListener)
+    private void destroyObjectMBeans(ConfiguredObject<?> object, boolean removeListener)
     {
-        if (supportedConfiguredObject(child))
+        if (supportedConfiguredObject(object))
         {
             synchronized (_childrenLock)
             {
                 if (removeListener)
                 {
-                    child.removeChangeListener(_changeListener);
+                    object.removeChangeListener(_changeListener);
+                }
+                unregisterObjectMBeans(object);
+                _children.remove(object);
+                destroyChildrenMBeansIfVirtualHostNode(object);
+            }
+        }
+    }
+
+    private void destroyChildrenMBeansIfVirtualHostNode(ConfiguredObject<?> child)
+    {
+        if (child instanceof VirtualHostNode)
+        {
+            for (Iterator<ConfiguredObject<?>> iterator = _children.keySet().iterator(); iterator.hasNext();)
+            {
+                ConfiguredObject<?> registeredObject = iterator.next();
+                ConfiguredObject<?> parent = registeredObject.getParent(VirtualHostNode.class);
+                if (parent == child)
+                {
+                    registeredObject.removeChangeListener(_changeListener);
+                    unregisterObjectMBeans(registeredObject);
                 }
-                unregisterObjectMBeans(child);
-                _children.remove(child);
+                iterator.remove();
             }
         }
     }
@@ -429,6 +450,12 @@ public class JMXManagementPluginImpl
         {
             return "INTERNAL";
         }
+
+        @Override
+        public String toString()
+        {
+            return DEFAULT_NAME;
+        }
     }
 
     private class ChangeListener implements ConfigurationChangeListener
@@ -465,7 +492,14 @@ public class JMXManagementPluginImpl
             // for instance, on role change in BDB HA VHN a VH could is recovered/created.
             // A call to createObjectMBeans is safe as it checks the existence of MBean before its creation.
 
-            createObjectMBeans(object);
+            if (ConfiguredObject.DESIRED_STATE.equals(attributeName))
+            {
+                stateChanged(object, State.valueOf(String.valueOf(oldAttributeValue)), State.valueOf(String.valueOf(newAttributeValue)));
+            }
+            else
+            {
+                createObjectMBeans(object);
+            }
         }
     }
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/systest/management/jmx/MBeanLifeCycleTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/systest/management/jmx/MBeanLifeCycleTest.java
index b79c4bdc40..efbd68f9e0 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/systest/management/jmx/MBeanLifeCycleTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/systest/management/jmx/MBeanLifeCycleTest.java
@@ -33,6 +33,7 @@ import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.Plugin;
 import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.VirtualHost;
+import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.VirtualHostNode;
 import org.apache.qpid.server.security.auth.manager.AnonymousAuthenticationManager;
 import org.apache.qpid.server.virtualhost.ProvidedStoreVirtualHost;
@@ -43,7 +44,8 @@ import org.apache.qpid.test.utils.TestBrokerConfiguration;
 
 public class MBeanLifeCycleTest extends QpidRestTestCase
 {
-
+    private final static String TEST_VIRTUAL_HOST_MBEAN_SEARCH_QUERY = "org.apache.qpid:type=VirtualHost.VirtualHostManager,VirtualHost="
+            + ObjectName.quote(TEST2_VIRTUALHOST);
     private JMXTestUtils _jmxUtils;
 
     @Override
@@ -108,14 +110,31 @@ public class MBeanLifeCycleTest extends QpidRestTestCase
 
     public void testVirtualHostMBeanIsUnregisteredOnVirtualHostDeletion() throws Exception
     {
-        String query = "org.apache.qpid:type=VirtualHost.VirtualHostManager,VirtualHost="
-                + ObjectName.quote(TEST2_VIRTUALHOST)  + ",*";
-        boolean mBeanExists =_jmxUtils.doesManagedObjectExist(query);
+        boolean mBeanExists =_jmxUtils.doesManagedObjectExist(TEST_VIRTUAL_HOST_MBEAN_SEARCH_QUERY);
         assertTrue("Host mBean is not registered", mBeanExists);
 
         getRestTestHelper().submitRequest("virtualhostnode/" + TEST2_VIRTUALHOST, "DELETE", HttpServletResponse.SC_OK);
 
-        mBeanExists =_jmxUtils.doesManagedObjectExist(query);
+        mBeanExists =_jmxUtils.doesManagedObjectExist(TEST_VIRTUAL_HOST_MBEAN_SEARCH_QUERY);
+        assertFalse("Host mBean is not unregistered", mBeanExists);
+    }
+
+    public void testVirtualHostMBeanIsUnregisteredOnVirtualHostNodeStop() throws Exception
+    {
+        boolean mBeanExists =_jmxUtils.doesManagedObjectExist(TEST_VIRTUAL_HOST_MBEAN_SEARCH_QUERY);
+        assertTrue("Host mBean is not registered", mBeanExists);
+
+        ManagedBroker managedBroker = _jmxUtils.getManagedBroker(TEST2_VIRTUALHOST);
+        assertNotNull("Host mBean is not created", managedBroker);
+
+        Map<String, Object> nodeData = new HashMap<String, Object>();
+        nodeData.put(VirtualHostNode.NAME, TEST2_VIRTUALHOST);
+        nodeData.put(VirtualHostNode.DESIRED_STATE, State.STOPPED.name());
+
+        int status = getRestTestHelper().submitRequest("virtualhostnode/" + TEST2_VIRTUALHOST, "PUT", nodeData);
+        assertEquals("Unexpected code", 200, status);
+
+        mBeanExists =_jmxUtils.doesManagedObjectExist(TEST_VIRTUAL_HOST_MBEAN_SEARCH_QUERY);
         assertFalse("Host mBean is not unregistered", mBeanExists);
     }
 }