QPID-5820: [Java QMF2 Plugin] changes to plugin owing to the Java Broker model updates made during 0.29

* VHNs (virtualhostnodes) may exist within a VH (virtualhost)
* Use Port#availableProtocols rather than Port#protocols when trying to find the AMQP port
* Like the CPP Broker, Binding#arguments will be null if the binding was created with none.

Note that the QMF plugin still does not support virtualhosts created at runtime.  They'll be ignored until
the next restart.

Work done by Andrew MacBean <macbean@apache.org> and me.

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

4 files changed, 111 insertions, 64 deletions

diff --git a/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/QmfManagementAgent.java b/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/QmfManagementAgent.java
index 4246bd7336..9983bc2233 100644
--- a/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/QmfManagementAgent.java
+++ b/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/QmfManagementAgent.java
@@ -124,7 +124,7 @@ public class QmfManagementAgent implements ConfigurationChangeListener, QmfEvent
     public QmfManagementAgent(final String url, final Broker broker)
     {
         _broker = broker;
-        _defaultVirtualHost = (String)broker.getAttribute("defaultVirtualHost");
+        _defaultVirtualHost = broker.getDefaultVirtualHost();
 
         try
         {
@@ -174,12 +174,12 @@ public class QmfManagementAgent implements ConfigurationChangeListener, QmfEvent
         }
         catch (QmfException qmfe)
         {
-            _log.info("QmfException {} caught in QmfManagementAgent Constructor", qmfe.getMessage());
+            _log.error("QmfException caught in QmfManagementAgent Constructor", qmfe);
             _agent = null; // Causes isConnected() to be false and thus prevents the "QMF2 Management Ready" message.
         }
         catch (Exception e)
         {
-            _log.info("Exception {} caught in QmfManagementAgent Constructor", e.getMessage());
+            _log.error("Exception caught in QmfManagementAgent Constructor", e);
             _agent = null; // Causes isConnected() to be false and thus prevents the "QMF2 Management Ready" message.
         }
     }
@@ -212,84 +212,119 @@ public class QmfManagementAgent implements ConfigurationChangeListener, QmfEvent
     {
         childAdded(null, _broker);
 
+        if (_log.isDebugEnabled())
+        {
+            _log.debug("Registering model listeners for broker " + _broker);
+        }
+
         for (VirtualHostNode<?> vhostNode : _broker.getVirtualHostNodes())
         {
+
+            if (_log.isDebugEnabled())
+            {
+                _log.debug("Considering virtualhostnode " + vhostNode);
+            }
+
             VirtualHost<?,?,?> vhost = vhostNode.getVirtualHost();
 
             // We don't add QmfAgentData VirtualHost objects. Possibly TODO, but it's a bit awkward at the moment
             // because the C++ Broker doesn't *seem* to do much with them and the command line tools such
             // as qpid-config don't appear to be VirtualHost aware. A way to stay compatible is to mark queues,
             // exchanges etc with [vhost:<vhost-name>/]<object-name> (see Constructor comments).
-            vhost.addChangeListener(this);
 
-            for (Connection<?> connection : vhost.getConnections())
+            if (vhost != null)
             {
-                childAdded(vhost, connection);
- 
-                for (Session<?> session : connection.getSessions())
-                {
-                    childAdded(connection, session);
+                vhost.addChangeListener(this);
 
-                    if (session.getConsumers() != null)
-                    {
-                        for (Consumer subscription : session.getConsumers())
-                        {
-                            childAdded(session, subscription);
-                        }
-                    }
-                }
+                addListenersForConnectionsAndChildren(vhost);
+                addListenersForExchangesAndChildren(vhost);
+                addListenersForQueuesAndChildren(vhost);
             }
+        }
 
-            // The code blocks for adding Bindings (and adding Queues) contain checks to see if what is being added
-            // relates to Queues or Bindings for the QmfManagementAgent. If they are QmfManagementAgent related
-            // we avoid registering the Object as a QMF Object, in other words we "hide" QmfManagementAgent QMF Objects.
-            // This is done to be consistent with the C++ broker which also "hides" its own Connection, Queue & Binding.
-            for (Exchange<?> exchange : vhost.getExchanges())
-            {
-                childAdded(vhost, exchange);
 
-                for (Binding binding : exchange.getBindings())
+        if (_log.isDebugEnabled())
+        {
+            _log.debug("Registered model listeners");
+        }
+
+    }
+
+    private void addListenersForQueuesAndChildren(final VirtualHost<?, ?, ?> vhost)
+    {
+        for (Queue<?> queue : vhost.getQueues())
+        {
+            boolean agentQueue = false;
+            for (Binding binding : queue.getBindings())
+            {
+                String key = binding.getName();
+                if (key.equals("broker") || key.equals("console.request.agent_locate") ||
+                    key.startsWith("apache.org:qpidd:"))
                 {
-                    String key = binding.getName();
-                    if (key.equals("broker") || key.equals("console.request.agent_locate") ||
-                        key.startsWith("apache.org:qpidd:") || key.startsWith("TempQueue"))
-                    { // Don't add QMF related Bindings in registerConfigurationChangeListeners as those will relate
-                    } // to the Agent and we want to "hide" those.
-                    else
-                    {
-                        childAdded(exchange, binding);
-                    }
+                    agentQueue = true;
+                    break;
                 }
             }
 
-            for (Queue<?> queue : vhost.getQueues())
+            // Don't add QMF related bindings or Queues in registerConfigurationChangeListeners as those will
+            // relate to the Agent itself and we want to "hide" those to be consistent with the C++ Broker.
+            if (!agentQueue)
             {
-                boolean agentQueue = false;
+                childAdded(vhost, queue);
+
                 for (Binding binding : queue.getBindings())
                 {
-                    String key = binding.getName();
-                    if (key.equals("broker") || key.equals("console.request.agent_locate") ||
-                        key.startsWith("apache.org:qpidd:"))
-                    {
-                        agentQueue = true;
-                        break;
-                    }
+                    childAdded(queue, binding);
+                }
+
+                for (Consumer subscription : queue.getChildren(Consumer.class))
+                {
+                    childAdded(queue, subscription);
                 }
+            }
+        }
+    }
+
+    private void addListenersForExchangesAndChildren(final VirtualHost<?, ?, ?> vhost)
+    {
+        // The code blocks for adding Bindings (and adding Queues) contain checks to see if what is being added
+        // relates to Queues or Bindings for the QmfManagementAgent. If they are QmfManagementAgent related
+        // we avoid registering the Object as a QMF Object, in other words we "hide" QmfManagementAgent QMF Objects.
+        // This is done to be consistent with the C++ broker which also "hides" its own Connection, Queue & Binding.
+        for (Exchange<?> exchange : vhost.getExchanges())
+        {
+            childAdded(vhost, exchange);
 
-                // Don't add QMF related bindings or Queues in registerConfigurationChangeListeners as those will
-                // relate to the Agent itself and we want to "hide" those to be consistent with the C++ Broker.
-                if (!agentQueue)
+            for (Binding binding : exchange.getBindings())
+            {
+                String key = binding.getName();
+                if (key.equals("broker") || key.equals("console.request.agent_locate") ||
+                    key.startsWith("apache.org:qpidd:") || key.startsWith("TempQueue"))
+                { // Don't add QMF related Bindings in registerConfigurationChangeListeners as those will relate
+                } // to the Agent and we want to "hide" those.
+                else
                 {
-                    childAdded(vhost, queue);
+                    childAdded(exchange, binding);
+                }
+            }
+        }
+    }
 
-                    for (Binding binding : queue.getBindings())
-                    {
-                        childAdded(queue, binding);
-                    }
+    private void addListenersForConnectionsAndChildren(final VirtualHost<?, ?, ?> vhost)
+    {
+        for (Connection<?> connection : vhost.getConnections())
+        {
+            childAdded(vhost, connection);
 
-                    for (Consumer subscription : queue.getChildren(Consumer.class))
+            for (Session<?> session : connection.getSessions())
+            {
+                childAdded(connection, session);
+
+                if (session.getConsumers() != null)
+                {
+                    for (Consumer subscription : session.getConsumers())
                     {
-                        childAdded(queue, subscription);
+                        childAdded(session, subscription);
                     }
                 }
             }
@@ -341,10 +376,17 @@ public class QmfManagementAgent implements ConfigurationChangeListener, QmfEvent
     @Override
     public void childAdded(final ConfiguredObject object, final ConfiguredObject child)
     {
-//System.out.println("childAdded: " + child.getClass().getSimpleName() + "." + child.getName());
+
+        if (_log.isDebugEnabled())
+        {
+            _log.debug("childAdded: " + child.getClass().getSimpleName() + "." + child.getName());
+        }
 
         QmfAgentData data = null;
 
+        // We current don't listen for new virtualhostnodes or new virtualhosts, so any new instances
+        // of these objects wont be seen through QMF until the Broker is restarted.
+
         if (child instanceof Broker)
         {
             data = new org.apache.qpid.server.qmf2.agentdata.Broker((Broker)child);
@@ -478,7 +520,7 @@ public class QmfManagementAgent implements ConfigurationChangeListener, QmfEvent
         }
         catch (QmfException qmfe)
         {
-            _log.info("QmfException {} caught in QmfManagementAgent.addObject()", qmfe.getMessage());
+            _log.error("QmfException caught in QmfManagementAgent.addObject()", qmfe);
         }
 
         child.addChangeListener(this);
@@ -497,7 +539,12 @@ public class QmfManagementAgent implements ConfigurationChangeListener, QmfEvent
     @Override
     public void childRemoved(final ConfiguredObject object, final ConfiguredObject child)
     {
-//System.out.println("childRemoved: " + child.getClass().getSimpleName() + "." + child.getName());
+
+
+        if (_log.isDebugEnabled())
+        {
+            _log.debug("childRemoved: " + child.getClass().getSimpleName() + "." + child.getName());
+        }
 
         child.removeChangeListener(this);
 
diff --git a/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/QmfManagementPluginImpl.java b/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/QmfManagementPluginImpl.java
index 7efd1962f1..52ea0afa10 100644
--- a/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/QmfManagementPluginImpl.java
+++ b/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/QmfManagementPluginImpl.java
@@ -152,7 +152,8 @@ public class QmfManagementPluginImpl extends AbstractPluginAdapter<QmfManagement
             {
                 VirtualHost<?, ?, ?> vhost = node.getVirtualHost();
 
-                if (vhost.getName().equals(_defaultVirtualHost))
+
+                if (vhost != null && vhost.getName().equals(_defaultVirtualHost))
                 {
                     foundDefaultVirtualHost = true;
 
@@ -200,7 +201,7 @@ public class QmfManagementPluginImpl extends AbstractPluginAdapter<QmfManagement
         }
         catch (Exception e) // Catch and log any Exception so we avoid Plugin failures stopping Broker startup.
         {
-            _log.info("Exception {} caught in QmfManagementPlugin.start()", e.getMessage());
+            _log.error("Exception caught in QmfManagementPlugin.start()", e);
         }
     }
 
diff --git a/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/agentdata/Binding.java b/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/agentdata/Binding.java
index 5feca96bc2..b609e6715c 100644
--- a/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/agentdata/Binding.java
+++ b/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/agentdata/Binding.java
@@ -118,9 +118,8 @@ public class Binding extends QmfAgentData
         setValue("bindingKey", binding.getName());
 
         Map<String, Object> arguments = binding.getArguments();
-        // binding.getArguments() always returns a Map, but with the C++ broker if there are no arguments
-        // the property isn't populated, so we only add it if the _arguments.size() > 0
-        if (arguments.size() > 0)
+        // Only add arguments property if the bindings have arguments
+        if (arguments != null && arguments.size() > 0)
         {
             setValue("arguments", arguments);
         }
diff --git a/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/agentdata/Broker.java b/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/agentdata/Broker.java
index 10e92a70ad..2b8cbceadc 100644
--- a/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/agentdata/Broker.java
+++ b/tools/src/java/qpid-broker-plugins-management-qmf2/src/main/java/org/apache/qpid/server/qmf2/agentdata/Broker.java
@@ -352,7 +352,7 @@ public class Broker extends QmfAgentData
         super(getSchema());
 
         _broker = broker;
-        _defaultVirtualHost = (String)broker.getAttribute("defaultVirtualHost");
+        _defaultVirtualHost = broker.getDefaultVirtualHost();
         int amqpPort = 5672; // Default AMQP Port.
 
         // Search through the available Ports on this Broker looking for the AMQP Port using the TCP Transport
@@ -364,7 +364,7 @@ public class Broker extends QmfAgentData
         {
             boolean isAMQP = false;
             boolean isTCP  = false;
-            for (Protocol protocol : port.getProtocols())
+            for (Protocol protocol : port.getAvailableProtocols())
             {
                 isAMQP = protocol.isAMQP();
                 if (isAMQP)