QPID-5665 : Address review comments from Alex Rudyy

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

2 files changed, 76 insertions, 6 deletions

diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/VirtualHostManagerMBean.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/VirtualHostManagerMBean.java
index a7f2ec3257..3fdee24ff8 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/VirtualHostManagerMBean.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/VirtualHostManagerMBean.java
@@ -42,14 +42,16 @@ import org.apache.qpid.management.common.mbeans.ManagedQueue;
 import org.apache.qpid.management.common.mbeans.annotations.MBeanConstructor;
 import org.apache.qpid.management.common.mbeans.annotations.MBeanDescription;
 import org.apache.qpid.management.common.mbeans.annotations.MBeanOperationParameter;
+import org.apache.qpid.server.exchange.AMQUnknownExchangeType;
 import org.apache.qpid.server.jmx.ManagedObject;
 import org.apache.qpid.server.model.Exchange;
 import org.apache.qpid.server.model.LifetimePolicy;
 import org.apache.qpid.server.model.Queue;
-import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.queue.QueueArgumentsConverter;
+import org.apache.qpid.server.virtualhost.ExchangeExistsException;
 import org.apache.qpid.server.virtualhost.RequiredExchangeException;
+import org.apache.qpid.server.virtualhost.ReservedExchangeNameException;
 
 @MBeanDescription("This MBean exposes the broker level management features")
 public class VirtualHostManagerMBean extends AbstractStatisticsGatheringMBean<VirtualHost> implements ManagedBroker
@@ -166,8 +168,29 @@ public class VirtualHostManagerMBean extends AbstractStatisticsGatheringMBean<Vi
 
         try
         {
-            getConfiguredObject().createExchange(name, State.ACTIVE, durable,
-                                            LifetimePolicy.PERMANENT, type, Collections.EMPTY_MAP);
+            Map<String,Object> attributes = new HashMap<>();
+            attributes.put(Exchange.NAME, name);
+            attributes.put(Exchange.TYPE, type);
+            attributes.put(Exchange.DURABLE, durable);
+            attributes.put(Exchange.LIFETIME_POLICY, LifetimePolicy.PERMANENT);
+
+            getConfiguredObject().createExchange(attributes);
+        }
+        catch(ExchangeExistsException e)
+        {
+            String message = "Exchange with name '" + name + "' already exists";
+            JMException jme = new JMException(message);
+            throw new MBeanException(jme, "Error in creating exchange " + name);
+
+        }
+        catch(ReservedExchangeNameException e)
+        {
+            throw new UnsupportedOperationException("'" + name + "' is a reserved exchange name");
+        }
+        catch(AMQUnknownExchangeType e)
+        {
+            JMException jme = new JMException(e.getMessage());
+            throw new MBeanException(jme, "Error in creating exchange " + name);
         }
         catch (IllegalArgumentException iae)
         {
@@ -175,6 +198,7 @@ public class VirtualHostManagerMBean extends AbstractStatisticsGatheringMBean<Vi
             throw new MBeanException(jme, "Error in creating exchange " + name);
         }
 
+
     }
 
     @Override
diff --git a/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/VirtualHostManagerMBeanTest.java b/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/VirtualHostManagerMBeanTest.java
index d63a3983f5..8f2418ddf3 100644
--- a/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/VirtualHostManagerMBeanTest.java
+++ b/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/VirtualHostManagerMBeanTest.java
@@ -19,6 +19,7 @@
  */
 package org.apache.qpid.server.jmx.mbeans;
 
+import static org.mockito.Matchers.argThat;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -32,12 +33,12 @@ import javax.management.OperationsException;
 
 import junit.framework.TestCase;
 import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatcher;
 
 import org.apache.qpid.server.jmx.ManagedObjectRegistry;
 import org.apache.qpid.server.model.Exchange;
 import org.apache.qpid.server.model.LifetimePolicy;
 import org.apache.qpid.server.model.Queue;
-import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.queue.QueueArgumentsConverter;
 
@@ -150,7 +151,8 @@ public class VirtualHostManagerMBeanTest extends TestCase
     public void testCreateNewDurableExchange() throws Exception
     {
         _virtualHostManagerMBean.createNewExchange(TEST_EXCHANGE_NAME, TEST_EXCHANGE_TYPE, true);
-        verify(_mockVirtualHost).createExchange(TEST_EXCHANGE_NAME, State.ACTIVE, true, LifetimePolicy.PERMANENT, TEST_EXCHANGE_TYPE, EMPTY_ARGUMENT_MAP);
+
+        verify(_mockVirtualHost).createExchange(matchesMap(TEST_EXCHANGE_NAME, true, LifetimePolicy.PERMANENT, TEST_EXCHANGE_TYPE));
     }
 
     public void testCreateNewExchangeWithUnknownExchangeType() throws Exception
@@ -165,7 +167,10 @@ public class VirtualHostManagerMBeanTest extends TestCase
         {
             // PASS
         }
-        verify(_mockVirtualHost, never()).createExchange(TEST_EXCHANGE_NAME, State.ACTIVE, true, LifetimePolicy.PERMANENT, exchangeType, EMPTY_ARGUMENT_MAP);
+        verify(_mockVirtualHost, never()).createExchange(matchesMap(TEST_EXCHANGE_NAME,
+                                                                    true,
+                                                                    LifetimePolicy.PERMANENT,
+                                                                    exchangeType));
     }
 
     public void testUnregisterExchange() throws Exception
@@ -200,4 +205,45 @@ public class VirtualHostManagerMBeanTest extends TestCase
 
         verify(mockExchange, never()).delete();
     }
+
+    private static Map<String,Object> matchesMap(final String name,
+                                                 final boolean durable,
+                                                 final LifetimePolicy lifetimePolicy,
+                                                 final String exchangeType)
+    {
+        return argThat(new MapMatcher(name, durable, lifetimePolicy, exchangeType));
+    }
+
+    private static class MapMatcher extends ArgumentMatcher<Map<String,Object>>
+    {
+
+        private final String _name;
+        private final boolean _durable;
+        private final LifetimePolicy _lifetimePolicy;
+        private final String _exchangeType;
+
+        public MapMatcher(final String name,
+                          final boolean durable,
+                          final LifetimePolicy lifetimePolicy,
+                          final String exchangeType)
+        {
+            _name = name;
+            _durable = durable;
+            _lifetimePolicy = lifetimePolicy;
+            _exchangeType = exchangeType;
+
+        }
+
+        @Override
+        public boolean matches(final Object o)
+        {
+            Map<String,Object> map = (Map<String,Object>)o;
+
+            return _name.equals(map.get(Exchange.NAME))
+                   && _durable == (Boolean) map.get(Exchange.DURABLE)
+                   && _lifetimePolicy == map.get(Exchange.LIFETIME_POLICY)
+                   && _exchangeType.equals(map.get(Exchange.TYPE));
+        }
+    }
+
 }