QPID-4873 Commiting patch by Helen Kwong.

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

4 files changed, 108 insertions, 15 deletions

diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java b/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java
index 72fc74e19c..99154e820f 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java
@@ -123,10 +123,10 @@ public class AddressHelper
     @SuppressWarnings("unchecked")
     public List<Binding> getBindings(Map props)
     {
-        List<Binding> bindings = new ArrayList<Binding>();
         List<Map> bindingList = (props == null) ? Collections.EMPTY_LIST : (List<Map>) props.get(X_BINDINGS);
-        if (bindingList != null)
+        if (bindingList != null && !bindingList.isEmpty())
         {
+            List<Binding> bindings = new ArrayList<Binding>(bindingList.size());
             for (Map bindingMap : bindingList)
             {
                 Binding binding = new Binding(
@@ -138,8 +138,12 @@ public class AddressHelper
                                         .get(ARGUMENTS));
                 bindings.add(binding);
             }
+            return bindings;
+        }
+        else
+        {
+            return Collections.emptyList();
         }
-        return bindings;
     }
 
     public Map getDeclareArgs(Map props)
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/Link.java b/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/Link.java
index 40a84ebd02..a614690f83 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/Link.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/Link.java
@@ -20,7 +20,6 @@
  */
 package org.apache.qpid.client.messaging.address;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -43,7 +42,7 @@ public class Link
     private int _producerCapacity = 0;
     private Subscription subscription;
     private Reliability reliability = Reliability.AT_LEAST_ONCE;
-    private List<Binding> _bindings = new ArrayList<Binding>();
+    private List<Binding> _bindings = Collections.emptyList();
     private SubscriptionQueue _subscriptionQueue;
 
     public Reliability getReliability()
@@ -206,7 +205,7 @@ public class Link
     
     public static class Subscription
     {
-        private Map<String,Object> args = new HashMap<String,Object>();        
+        private Map<String,Object> args = Collections.emptyMap();
         private boolean exclusive = false;
         
         public Map<String, Object> getArgs()
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java b/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java
index 005f98f344..cc8e11b91a 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java
@@ -21,15 +21,14 @@
 
 package org.apache.qpid.client.messaging.address;
 
-import org.apache.qpid.client.AMQDestination;
-import org.apache.qpid.client.AMQDestination.Binding;
-
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.qpid.client.AMQDestination;
+import org.apache.qpid.client.AMQDestination.Binding;
+
 public class Node
 { 
     private int _nodeType = AMQDestination.UNKNOWN_TYPE;
@@ -39,7 +38,7 @@ public class Node
     private boolean _isExclusive;
     private String _alternateExchange;
     private String _exchangeType = "topic"; // used when node is an exchange instead of a queue.
-    private List<Binding> _bindings = new ArrayList<Binding>();
+    private List<Binding> _bindings = Collections.emptyList();
     private Map<String,Object> _declareArgs = new HashMap<String,Object>();
 
     protected Node(String name)
@@ -112,10 +111,6 @@ public class Node
         _bindings = bindings;
     }
     
-    public void addBinding(Binding binding) {
-        this._bindings.add(binding);
-    }
-    
     public Map<String,Object> getDeclareArgs()
     {
         return _declareArgs;
diff --git a/qpid/java/client/src/test/java/org/apache/qpid/client/AMQDestinationTest.java b/qpid/java/client/src/test/java/org/apache/qpid/client/AMQDestinationTest.java
index f46623ad3b..6433c9acb7 100644
--- a/qpid/java/client/src/test/java/org/apache/qpid/client/AMQDestinationTest.java
+++ b/qpid/java/client/src/test/java/org/apache/qpid/client/AMQDestinationTest.java
@@ -20,6 +20,10 @@
  */
 package org.apache.qpid.client;
 
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
 import junit.framework.TestCase;
 
 public class AMQDestinationTest extends TestCase
@@ -63,4 +67,95 @@ public class AMQDestinationTest extends TestCase
         assertTrue(dest7.hashCode() != dest8.hashCode());
         assertTrue(dest6.hashCode() != dest9.hashCode());
     }
+
+    /**
+     * Tests that destinations created with the same options string will share the same address options map.
+     */
+    public void testCacheAddressOptionsMaps() throws Exception
+    {
+        // Create destinations 1 and 3 with the same options string, and destinations 2 and 4 with a different one
+        String optionsStringA = "{create: always, node: {type: topic}}";
+        String optionsStringB = "{}";   // empty options
+        AMQDestination dest1 = createDestinationWithOptions("testDest1", optionsStringA);
+        AMQDestination dest2 = createDestinationWithOptions("testDest2", optionsStringB);
+        AMQDestination dest3 = createDestinationWithOptions("testDest3", optionsStringA);
+        AMQDestination dest4 = createDestinationWithOptions("testDest4", optionsStringB);
+
+        // Destinations 1 and 3 should refer to the same address options map
+        assertSame("Destinations 1 and 3 were created with the same options and should refer to the same options map.",
+                dest1.getAddress().getOptions(), dest3.getAddress().getOptions());
+        // Destinations 2 and 4 should refer to the same address options map
+        assertSame("Destinations 2 and 4 were created with the same options and should refer to the same options map.",
+                dest2.getAddress().getOptions(), dest4.getAddress().getOptions());
+        // Destinations 1 and 2 should have a different options map
+        assertNotSame("Destinations 1 and 2 should not have the same options map.",
+                dest1.getAddress().getOptions(), dest2.getAddress().getOptions());
+
+        // Verify the contents of the shared map are as expected
+        Map<String, Object> optionsA = new HashMap<String, Object>();
+        optionsA.put("create", "always");
+        optionsA.put("node", Collections.singletonMap("type", "topic"));
+        assertEquals("Contents of the shared address options map are not as expected.",
+                optionsA, dest1.getAddress().getOptions());
+        assertEquals("Contents of the empty shared address options map are not as expected.",
+                Collections.emptyMap(), dest2.getAddress().getOptions());
+
+        // Verify that address options map is immutable
+        try
+        {
+            dest1.getAddress().getOptions().put("testKey", "testValue");
+            fail("Should not be able able to modify an address's options map.");
+        }
+        catch (UnsupportedOperationException e)
+        {
+            // expected
+        }
+    }
+
+    private AMQDestination createDestinationWithOptions(String destName, String optionsString) throws Exception
+    {
+        String addr = "ADDR:" + destName + "; " + optionsString;
+        return new AMQAnyDestination(addr);
+    }
+
+    /**
+     * Tests that when a queue has no link subscription arguments and no link bindings, its Subscription
+     * arguments and its bindings list refer to constant empty collections.
+     */
+    public void testEmptyLinkBindingsAndSubscriptionArgs() throws Exception
+    {
+        // no link properties
+        assertEmptyLinkBindingsAndSubscriptionArgs(new AMQAnyDestination("ADDR:testQueue"));
+
+        // has link properties but no x-bindings; has link x-subscribes but no arguments
+        String xSubscribeAddr = "ADDR:testQueueWithXSubscribes; {link: {x-subscribes: {exclusive: true}}}";
+        assertEmptyLinkBindingsAndSubscriptionArgs(new AMQAnyDestination(xSubscribeAddr));
+    }
+
+    private void assertEmptyLinkBindingsAndSubscriptionArgs(AMQDestination dest) {
+        assertEquals("Default link subscription arguments should be the constant Collections empty map.",
+                Collections.emptyMap(), dest.getLink().getSubscription().getArgs());
+        assertSame("Defaultl link bindings should be the constant Collections empty list.",
+                Collections.emptyList(), dest.getLink().getBindings());
+    }
+
+    /**
+     * Tests that when a node has no bindings specified, its bindings list refers to a constant empty list,
+     * so that we are not consuming extra memory unnecessarily.
+     */
+    public void testEmptyNodeBindings() throws Exception
+    {
+        // no node properties
+        assertEmptyNodeBindings(new AMQAnyDestination("ADDR:testDest1"));
+        // has node properties but no x-bindings
+        assertEmptyNodeBindings(new AMQAnyDestination("ADDR:testDest2; {node: {type: queue}}"));
+        assertEmptyNodeBindings(new AMQAnyDestination("ADDR:testDest3; {node: {type: topic}}"));
+    }
+
+    private void assertEmptyNodeBindings(AMQDestination dest)
+    {
+        assertSame("Empty node bindings should refer to the constant Collections empty list.",
+                Collections.emptyList(), dest.getNode().getBindings());
+    }
+
 }