QPID-4390: Change port defaults to provide a default port name based on port value, binding address and protocol, remove name field from a port adapter

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/java-broker-config-qpid-4390@1435276 13f79535-47bb-0310-9956-ffa450edef68

11 files changed, 49 insertions, 55 deletions

diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
index 9d1eb12f7e..7860c6ee05 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
@@ -196,6 +196,8 @@ public class HttpManagement extends AbstractPluginAdapter
         {
             final Collection<Protocol> protocols = port.getProtocols();
             Connector connector = null;
+
+            //TODO: what to do if protocol HTTP and transport SSL?
             if (protocols.contains(Protocol.HTTP))
             {
                 connector = new SelectChannelConnector();
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/PortConfigurationHelper.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/PortConfigurationHelper.java
index c86f2e6ff1..d6fae20a68 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/PortConfigurationHelper.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/PortConfigurationHelper.java
@@ -201,6 +201,7 @@ public class PortConfigurationHelper
                     serverConfig);
 
             Map<String, Object> attributes = new HashMap<String, Object>();
+            attributes.put(Port.NAME, port + "-" + Protocol.ProtocolType.AMQP);
             attributes.put(Port.PROTOCOLS, supported);
             attributes.put(Port.TRANSPORTS, Collections.singleton(transport));
             attributes.put(Port.PORT, port);
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/XMLConfigurationEntryStore.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/XMLConfigurationEntryStore.java
index a377ff2c32..cb31228c71 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/XMLConfigurationEntryStore.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/XMLConfigurationEntryStore.java
@@ -312,6 +312,7 @@ public class XMLConfigurationEntryStore implements ConfigurationEntryStore
         attributes.put(Port.TRANSPORTS, Collections.singleton(transport));
         attributes.put(Port.PORT, port);
         attributes.put(Port.BINDING_ADDRESS, null);
+        attributes.put(Port.NAME, port + "-" + protocol);
         return new ConfigurationEntry(UUID.randomUUID(), Port.class.getSimpleName(), attributes, null, this);
     }
 
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
index c83d776bd1..090bb59835 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
@@ -45,7 +45,6 @@ import org.apache.qpid.server.model.VirtualHostAlias;
 public class PortAdapter extends AbstractAdapter implements Port
 {
 
-    private final String _name;
     private final Broker _broker;
     private AuthenticationProvider _authenticationProvider;
 
@@ -61,14 +60,6 @@ public class PortAdapter extends AbstractAdapter implements Port
 
         addParent(Broker.class, broker);
 
-        String name = (String)getAttribute(NAME);
-        if (name == null)
-        {
-            Integer port = getPort();
-            String bindingAddress = getBindingAddress();
-            name = bindingAddress == null? port + "" : bindingAddress + ":" + port;
-        }
-        _name = name;
     }
 
     @Override
@@ -151,7 +142,7 @@ public class PortAdapter extends AbstractAdapter implements Port
     @Override
     public String getName()
     {
-        return _name;
+        return (String)getAttribute(NAME);
     }
 
     @Override
@@ -237,10 +228,6 @@ public class PortAdapter extends AbstractAdapter implements Port
         {
             return getId();
         }
-        else if(NAME.equals(name))
-        {
-            return getName();
-        }
         else if(STATE.equals(name))
         {
             return getActualState();
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortFactory.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortFactory.java
index 8c2ac664fa..6488405d64 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortFactory.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortFactory.java
@@ -42,19 +42,11 @@ import org.apache.qpid.server.util.MapValueConverter;
 public class PortFactory
 {
     public static final int DEFAULT_AMQP_SEND_BUFFER_SIZE = 262144;
-
     public static final int DEFAULT_AMQP_RECEIVE_BUFFER_SIZE = 262144;
-
     public static final boolean DEFAULT_AMQP_NEED_CLIENT_AUTH = false;
-
     public static final boolean DEFAULT_AMQP_WANT_CLIENT_AUTH = false;
-
     public static final boolean DEFAULT_AMQP_TCP_NO_DELAY = true;
-
-    public static final int DEFAULT_AMQP_PORT = 5672;
-
     public static final String DEFAULT_AMQP_BINDING = "*";
-
     public static final Transport DEFAULT_TRANSPORT = Transport.TCP;
 
     private final Collection<Protocol> _defaultProtocols;
@@ -93,20 +85,38 @@ public class PortFactory
         final Port port;
         Map<String, Object> defaults = new HashMap<String, Object>();
         defaults.put(Port.TRANSPORTS, Collections.singleton(DEFAULT_TRANSPORT));
+        Object portValue = attributes.get(Port.PORT);
+        if (portValue == null)
+        {
+            throw new IllegalConfigurationException("Port attribute is not specified for port: " + attributes);
+        }
         if (isAmqpProtocol(attributes))
         {
+            Object binding = attributes.get(Port.BINDING_ADDRESS);
+            if (binding == null)
+            {
+                binding = DEFAULT_AMQP_BINDING;
+                defaults.put(Port.BINDING_ADDRESS, DEFAULT_AMQP_BINDING);
+            }
+            defaults.put(Port.NAME, binding + ":" + portValue);
             defaults.put(Port.PROTOCOLS, _defaultProtocols);
-            defaults.put(Port.PORT, DEFAULT_AMQP_PORT);
             defaults.put(Port.TCP_NO_DELAY, DEFAULT_AMQP_TCP_NO_DELAY);
             defaults.put(Port.WANT_CLIENT_AUTH, DEFAULT_AMQP_WANT_CLIENT_AUTH);
             defaults.put(Port.NEED_CLIENT_AUTH, DEFAULT_AMQP_NEED_CLIENT_AUTH);
             defaults.put(Port.RECEIVE_BUFFER_SIZE, DEFAULT_AMQP_RECEIVE_BUFFER_SIZE);
             defaults.put(Port.SEND_BUFFER_SIZE, DEFAULT_AMQP_SEND_BUFFER_SIZE);
-            defaults.put(Port.BINDING_ADDRESS, DEFAULT_AMQP_BINDING);
             port = new AmqpPortAdapter(id, broker, attributes, defaults);
         }
         else
         {
+            @SuppressWarnings("unchecked")
+            Collection<Protocol> protocols = (Collection<Protocol>)attributes.get(Port.PROTOCOLS);
+            if (protocols.size() > 1)
+            {
+                throw new IllegalConfigurationException("Only one protocol can be used on non AMQP port");
+            }
+            Protocol protocol = protocols.iterator().next();
+            defaults.put(Port.NAME, portValue + "-" + protocol.name());
             port = new PortAdapter(id, broker, attributes, defaults);
         }
         return port;
@@ -179,6 +189,7 @@ public class PortFactory
         Set<Protocol> protocols = (Set<Protocol>) portAttributes.get(Port.PROTOCOLS);
         if (protocols == null || protocols.isEmpty())
         {
+            // defaulting to AMQP if protocol is not specified
             return true;
         }
 
diff --git a/qpid/java/broker/src/main/resources/default.json b/qpid/java/broker/src/main/resources/default.json
index 35f6c1640f..44c6744377 100644
--- a/qpid/java/broker/src/main/resources/default.json
+++ b/qpid/java/broker/src/main/resources/default.json
@@ -29,26 +29,26 @@
     "authenticationProviderType" : "PlainPasswordFileAuthenticationProvider",
     "path" : "${QPID_HOME}/etc/passwd"
   } ],
-  "ports" : [ {
+  "ports" : [  {
     "type" : "Port",
+    "name" : "5672-AMQP",
+    "port" : 5672
+  }, {
+    "type" : "Port",
+    "name" : "8080-HTTP",
     "port" : 8080,
-    "name" : "8080",
     "protocols" : [ "HTTP" ]
   }, {
     "type" : "Port",
-    "name" : "5672",
-    "port" : 5672
+    "name" : "8999-RMI",
+    "port" : 8999,
+    "protocols" : [ "RMI" ]
   }, {
     "type" : "Port",
-    "name" : "9099",
+    "name" : "9099-JMX_RMI",
     "port" : 9099,
     "protocols" : [ "JMX_RMI" ]
-  }, {
-    "type" : "Port",
-    "name" : "8999",
-    "port" : 8999,
-    "protocols" : [ "RMI" ]
-  } ],
+  }],
   "virtualhosts" : [ {
     "type" : "VirtualHost",
     "name" : "default",
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java
index cf70e78bb5..ffed6ef6f2 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java
@@ -97,13 +97,15 @@ public class PortFactoryTest extends QpidTestCase
         assertEquals("Unexpected protocols", new HashSet<Protocol>(expected), new HashSet<Protocol>(protocols));
     }
 
-    public void testCreatePortWithEmptyAttributes()
+    public void testCreatePortWithMinimumAttributes()
     {
-        Port port = _portFactory.createPort(_portId, _broker, Collections.<String, Object> emptyMap());
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        attributes.put(Port.PORT, 1);
+        Port port = _portFactory.createPort(_portId, _broker, attributes);
 
         assertNotNull(port);
         assertTrue(port instanceof AmqpPortAdapter);
-        assertEquals("Unexpected port", PortFactory.DEFAULT_AMQP_PORT, port.getPort());
+        assertEquals("Unexpected port", 1, port.getPort());
         assertEquals("Unexpected transports", Collections.singleton(PortFactory.DEFAULT_TRANSPORT), port.getTransports());
         assertEquals("Unexpected protocols", _portFactory.getDefaultProtocols(), port.getProtocols());
         assertEquals("Unexpected send buffer size", PortFactory.DEFAULT_AMQP_SEND_BUFFER_SIZE,
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/Asserts.java b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/Asserts.java
index dd1902866b..f15a1d1e6a 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/Asserts.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/Asserts.java
@@ -212,7 +212,6 @@ public class Asserts
                     Port.BINDING_ADDRESS, Port.TCP_NO_DELAY, Port.SEND_BUFFER_SIZE, Port.RECEIVE_BUFFER_SIZE,
                     Port.NEED_CLIENT_AUTH, Port.WANT_CLIENT_AUTH);
         }
-        assertNotNull("Unexpected value of attribute " + Port.NAME, port.get(Port.NAME));
 
         @SuppressWarnings("unchecked")
         Collection<String> transports = (Collection<String>) port.get(Port.TRANSPORTS);
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/BrokerRestTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/BrokerRestTest.java
index 8759254d12..bf7da5ce83 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/BrokerRestTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/BrokerRestTest.java
@@ -68,11 +68,10 @@ public class BrokerRestTest extends QpidRestTestCase
             Asserts.assertPortAttributes(port);
         }
 
-        String bindingAddress = (String)ports.get(0).get(Port.BINDING_ADDRESS);
-
-        Map<String, Object> amqpPort = getRestTestHelper().find(Port.NAME, bindingAddress + ":" + getPort(), ports);
-        Map<String, Object> httpPort = getRestTestHelper().find(Port.NAME, "" + getRestTestHelper().getHttpPort(), ports);
+        Map<String, Object> amqpPort = getRestTestHelper().find(Port.NAME, getPort() +"-AMQP", ports);
+        Map<String, Object> httpPort = getRestTestHelper().find(Port.NAME, getRestTestHelper().getHttpPort() + "-HTTP", ports);
 
+        assertEquals("Unexpected binding address", "*", amqpPort.get(Port.BINDING_ADDRESS));
         assertNotNull("Cannot find AMQP port", amqpPort);
         assertNotNull("Cannot find HTTP port", httpPort);
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/PortRestTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/PortRestTest.java
index bbc034c754..4a12b475af 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/PortRestTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/PortRestTest.java
@@ -34,12 +34,12 @@ public class PortRestTest extends QpidRestTestCase
         assertNotNull("Port data cannot be null", ports);
         assertEquals("Unexpected number of ports", 2, ports.size());
 
-        String httpPortName = "" + getRestTestHelper().getHttpPort();
+        String httpPortName = getRestTestHelper().getHttpPort() + "-HTTP";
         Map<String, Object> portData = getRestTestHelper().find(Port.NAME, httpPortName, ports);
         assertNotNull("Http port " + httpPortName + " is not found", portData);
         Asserts.assertPortAttributes(portData);
 
-        String amqpPortName = "*:" + getPort();
+        String amqpPortName = getPort() + "-AMQP";
         Map<String, Object> amqpPortData = getRestTestHelper().find(Port.NAME, amqpPortName, ports);
         assertNotNull("Amqp port " + amqpPortName + " is not found", amqpPortData);
         Asserts.assertPortAttributes(amqpPortData);
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/StructureRestTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/StructureRestTest.java
index 9971a6b549..a84708060a 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/StructureRestTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/StructureRestTest.java
@@ -91,23 +91,15 @@ public class StructureRestTest extends QpidRestTestCase
                     }
                 }
             }
-
-            /*
-            @SuppressWarnings("unchecked")
-            List<Map<String, Object>> aliases = (List<Map<String, Object>>) host.get("virtualhostaliases");
-            assertNotNull("Host " + hostName + " aliases are not found ", aliases);
-            assertEquals("Unexpected aliases size", 1, aliases.size());
-            assertNode(aliases.get(0), hostName);
-            */
         }
 
 
-        String httpPortName = "" + getRestTestHelper().getHttpPort();
+        String httpPortName = getRestTestHelper().getHttpPort() + "-HTTP";
         Map<String, Object> portData = getRestTestHelper().find(Port.NAME, httpPortName, ports);
         assertNotNull("Http Port " + httpPortName + " is not found", portData);
         assertNode(portData, httpPortName);
 
-        String amqpPortName = "*:" + getPort();
+        String amqpPortName = getPort() + "-AMQP";
         Map<String, Object> amqpPortData = getRestTestHelper().find(Port.NAME, amqpPortName, ports);
         assertNotNull("Amqp port " + amqpPortName + " is not found", amqpPortData);
         assertNode(amqpPortData, amqpPortName);