QPID-3401 Based on review comments, made the following changes.

Moved the destination syntax stuff into DestinationStringParser.
Each implementation now provides a getType() method to denote if it's a
Queue or Topic rather than carrying it as a member variable.
Made corrections to formatting and leading/trailling spaces.

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/address-refactor2@1336443 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 256 insertions, 236 deletions

diff --git a/qpid/java/client/src/main/java/org/apache/qpid/jms/DestinationStringParser.java b/qpid/java/client/src/main/java/org/apache/qpid/jms/DestinationStringParser.java
index 6a1637739d..1005aa83b8 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/jms/DestinationStringParser.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/jms/DestinationStringParser.java
@@ -27,6 +27,9 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.jms.JMSException;
+
+import org.apache.qpid.configuration.ClientProperties;
 import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.jms.QpidDestination.DestinationType;
 import org.apache.qpid.messaging.Address;
@@ -38,110 +41,206 @@ import org.apache.qpid.messaging.address.Node.AddressPolicy;
 import org.apache.qpid.messaging.address.Node.NodeType;
 import org.apache.qpid.messaging.util.AddressHelper;
 import org.apache.qpid.url.AMQBindingURL;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class DestinationStringParser
 {
-   public static Address parseAddressString(String str, DestinationType type) throws AddressException
-   {
-       Address addr = Address.parse(str);
-       AddressHelper helper = new AddressHelper(addr);
-
-       Node node = new Node();
-       node.setName(addr.getName());
-       node.setAssertPolicy(AddressPolicy.getAddressPolicy(helper.getAssert()));
-       node.setCreatePolicy(AddressPolicy.getAddressPolicy(helper.getCreate()));
-       node.setDeletePolicy(AddressPolicy.getAddressPolicy(helper.getDelete()));
-       node.setDurable(helper.isNodeDurable());
-
-       if (DestinationType.TOPIC == type)
-       {
-           if (helper.getNodeType() == NodeType.QUEUE)
-           {
-               throw new AddressException("Destination is marked as a Topic, but address is defined as a Queue");
-           }
-           node.setType(NodeType.TOPIC);
-       }
-       else
-       {
-           if (helper.getNodeType() == NodeType.TOPIC)
-           {
-               throw new AddressException("Destination is marked as a Queue, but address is defined as a Topic");
-           }
-           node.setType(NodeType.QUEUE);
-       }
-
-       node.setDeclareProps(helper.getNodeDeclareArgs());
-       node.setBindingProps(helper.getNodeBindings());
-       addr.setNode(node);
-
-       Link link =  new Link();
-       link.setName(helper.getLinkName());
-       link.setDurable(helper.isLinkDurable());
-       link.setReliability(Reliability.getReliability(helper.getLinkReliability()));
-       link.setProducerCapacity(helper.getProducerCapacity());
-       link.setConsumerCapacity(helper.getConsumeCapacity());
-       link.setDeclareProps(helper.getLinkDeclareArgs());
-       link.setBindingProps(helper.getLinkBindings());
-       link.setSubscribeProps(helper.getLinkSubscribeArgs());
-       addr.setLink(link);
-
-       return addr;
-   }
-
-   public static Address parseBURLString(String str, DestinationType type) throws AddressException
-   {
-	   AMQBindingURL burl;
-	   try
-	   {
-           burl = new AMQBindingURL(str);
-	   }
-	   catch(URISyntaxException e)
-	   {
-	       AddressException ex = new AddressException("Error parsing BURL : " + e.getMessage());
-	       ex.initCause(e);
-	       throw ex;
-	   }
-
-	   Address addr;
-	   Node node = new Node();
-	   Link link = new Link();
-
-	   if (type == DestinationType.TOPIC)
-	   {
-	       addr = new Address(burl.getExchangeName().asString(),
-                              burl.getRoutingKey().asString(),
-                              Collections.emptyMap());
-
-	      link.setName(burl.getQueueName().asString());
-	      node.setBindingProps(Collections.emptyList());
-	   }
-	   else
-	   {
-		   addr = new Address(burl.getQueueName().asString(),
-                              burl.getRoutingKey().asString(),
-                              Collections.emptyMap());
-
-		   List<Object> bindings = new ArrayList<Object>();
-		   Map<String,Object> binding = new HashMap<String,Object>();
-		   binding.put(AddressHelper.EXCHANGE, burl.getExchangeName().asString());
-		   binding.put(AddressHelper.KEY, burl.getRoutingKey());
-		   bindings.add(binding);
-		   node.setBindingProps(bindings);
-	   }
-
-	   List<Object> bindings = node.getBindingProperties();
-	   for (AMQShortString key: burl.getBindingKeys())
-	   {
-	       Map<String,Object> binding = new HashMap<String,Object>();
-           binding.put(AddressHelper.EXCHANGE, burl.getExchangeName().asString());
-           binding.put(AddressHelper.KEY, key.asString());
-           bindings.add(binding);
-	   }
-
-	   node.setAssertPolicy(AddressPolicy.NEVER);
-       node.setCreatePolicy(AddressPolicy.RECEIVER);
-       node.setDeletePolicy(AddressPolicy.NEVER);
-
-	   return addr;
-   }
+    private static final Logger _logger = LoggerFactory.getLogger(DestinationStringParser.class);
+
+    private static final String BURL_STR = "BURL";
+    private static final String ADDR_STR = "ADDR";
+
+    public enum DestSyntax
+    {
+        BURL,ADDR;
+
+        public static DestSyntax getSyntaxType(String s)
+        {
+            if ((BURL_STR).equals(s))
+            {
+                return BURL;
+            }
+            else if ((ADDR_STR).equals(s))
+            {
+                return ADDR;
+            }
+            else
+            {
+                throw new IllegalArgumentException("Invalid Destination Syntax Type" +
+                " should be one of {BURL|ADDR}");
+            }
+        }
+    }
+
+    private static final DestSyntax _defaultDestSyntax;
+    static
+    {
+        _defaultDestSyntax = DestSyntax.getSyntaxType(
+                System.getProperty(ClientProperties.DEST_SYNTAX,
+                        DestSyntax.ADDR.toString()));
+    }
+
+    public static DestSyntax getDestType(String str)
+    {
+        if (str.startsWith(ADDR_STR))
+        {
+            return DestSyntax.ADDR;
+        }
+        else if (str.startsWith(BURL_STR))
+        {
+            return DestSyntax.BURL;
+        }
+        else
+        {
+            return _defaultDestSyntax;
+        }
+    }
+
+    public static String stripSyntaxPrefix(String str)
+    {
+        if (str.startsWith(BURL_STR) || str.startsWith(ADDR_STR))
+        {
+            return str.substring(5,str.length());
+        }
+        else
+        {
+            return str;
+        }
+    }
+
+    public static Address parseDestinationString(String str, DestinationType type) throws JMSException
+    {
+        DestSyntax destSyntax = getDestType(str);
+        str = stripSyntaxPrefix(str);
+
+        if (_logger.isDebugEnabled())
+        {
+            _logger.debug("Based on " + str + " the selected destination syntax is " + destSyntax);
+        }
+
+        try
+        {
+            if (destSyntax == DestSyntax.BURL)
+            {
+                return DestinationStringParser.parseAddressString(str,type);
+            }
+            else
+            {
+                return DestinationStringParser.parseBURLString(str,type);
+            }
+        }
+        catch (AddressException e)
+        {
+            JMSException ex = new JMSException("Error parsing destination string, due to : " + e.getMessage());
+            ex.initCause(e);
+            ex.setLinkedException(e);
+            throw ex;
+        }
+    }
+
+
+    public static Address parseAddressString(String str, DestinationType type) throws AddressException
+    {
+        Address addr = Address.parse(str);
+        AddressHelper helper = new AddressHelper(addr);
+
+        Node node = new Node();
+        node.setName(addr.getName());
+        node.setAssertPolicy(AddressPolicy.getAddressPolicy(helper.getAssert()));
+        node.setCreatePolicy(AddressPolicy.getAddressPolicy(helper.getCreate()));
+        node.setDeletePolicy(AddressPolicy.getAddressPolicy(helper.getDelete()));
+        node.setDurable(helper.isNodeDurable());
+
+        if (DestinationType.TOPIC == type)
+        {
+            if (helper.getNodeType() == NodeType.QUEUE)
+            {
+                throw new AddressException("Destination is marked as a Topic, but address is defined as a Queue");
+            }
+            node.setType(NodeType.TOPIC);
+        }
+        else
+        {
+            if (helper.getNodeType() == NodeType.TOPIC)
+            {
+                throw new AddressException("Destination is marked as a Queue, but address is defined as a Topic");
+            }
+            node.setType(NodeType.QUEUE);
+        }
+
+        node.setDeclareProps(helper.getNodeDeclareArgs());
+        node.setBindingProps(helper.getNodeBindings());
+        addr.setNode(node);
+
+        Link link =  new Link();
+        link.setName(helper.getLinkName());
+        link.setDurable(helper.isLinkDurable());
+        link.setReliability(Reliability.getReliability(helper.getLinkReliability()));
+        link.setProducerCapacity(helper.getProducerCapacity());
+        link.setConsumerCapacity(helper.getConsumeCapacity());
+        link.setDeclareProps(helper.getLinkDeclareArgs());
+        link.setBindingProps(helper.getLinkBindings());
+        link.setSubscribeProps(helper.getLinkSubscribeArgs());
+        addr.setLink(link);
+
+        return addr;
+    }
+
+    public static Address parseBURLString(String str, DestinationType type) throws AddressException
+    {
+        AMQBindingURL burl;
+        try
+        {
+            burl = new AMQBindingURL(str);
+        }
+        catch(URISyntaxException e)
+        {
+            AddressException ex = new AddressException("Error parsing BURL : " + e.getMessage());
+            ex.initCause(e);
+            throw ex;
+        }
+
+        Address addr;
+        Node node = new Node();
+        Link link = new Link();
+
+        if (type == DestinationType.TOPIC)
+        {
+            addr = new Address(burl.getExchangeName().asString(),
+                    burl.getRoutingKey().asString(),
+                    Collections.emptyMap());
+
+            link.setName(burl.getQueueName().asString());
+            node.setBindingProps(Collections.emptyList());
+        }
+        else
+        {
+            addr = new Address(burl.getQueueName().asString(),
+                    burl.getRoutingKey().asString(),
+                    Collections.emptyMap());
+
+            List<Object> bindings = new ArrayList<Object>();
+            Map<String,Object> binding = new HashMap<String,Object>();
+            binding.put(AddressHelper.EXCHANGE, burl.getExchangeName().asString());
+            binding.put(AddressHelper.KEY, burl.getRoutingKey());
+            bindings.add(binding);
+            node.setBindingProps(bindings);
+        }
+
+        List<Object> bindings = node.getBindingProperties();
+        for (AMQShortString key: burl.getBindingKeys())
+        {
+            Map<String,Object> binding = new HashMap<String,Object>();
+            binding.put(AddressHelper.EXCHANGE, burl.getExchangeName().asString());
+            binding.put(AddressHelper.KEY, key.asString());
+            bindings.add(binding);
+        }
+
+        node.setAssertPolicy(AddressPolicy.NEVER);
+        node.setCreatePolicy(AddressPolicy.RECEIVER);
+        node.setDeletePolicy(AddressPolicy.NEVER);
+
+        return addr;
+    }
 }
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidDestination.java b/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidDestination.java
index 912a92d7ec..d05ca34e31 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidDestination.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidDestination.java
@@ -36,69 +36,42 @@ import org.slf4j.LoggerFactory;
 
 public abstract class QpidDestination implements Destination, Referenceable
 {
-    public enum DestinationType {QUEUE, TOPIC};
-
     private static final Logger _logger = LoggerFactory.getLogger(QpidDestination.class);
-    private static final DestSyntax defaultDestSyntax;
-    private DestSyntax _destSyntax = DestSyntax.ADDR;
 
-    protected final DestinationType type;
-    protected String destinationString;
-    protected Address address;
+    public enum DestinationType {QUEUE, TOPIC};
 
-    protected QpidDestination(DestinationType type)
+    protected String _destinationString;
+    protected Address _address;
+
+    protected QpidDestination()
     {
-        this.type = type;
     }
 
     public String getDestinationString()
     {
-        return destinationString;
+        return _destinationString;
     }
 
     public void setDestinationString(String str) throws JMSException
     {
-    	if (destinationString != null)
-    	{
+        if (_destinationString != null)
+        {
             throw new javax.jms.IllegalStateException("Once a destination string is set, it cannot be changed");
-    	}
-        destinationString = str;
+        }
+        _destinationString = str;
         parseDestinationString(str);
     }
 
+    public abstract DestinationType getType();
+
     protected void parseDestinationString(String str) throws JMSException
     {
-        _destSyntax = getDestType(str);
-        str = stripSyntaxPrefix(str);
-
-        if (_logger.isDebugEnabled())
-        {
-        	_logger.debug("Based on " + str + " the selected destination syntax is " + _destSyntax);
-        }
-
-        try 
-        {
-			if (_destSyntax == DestSyntax.BURL)
-			{    
-				address = DestinationStringParser.parseAddressString(str,type);
-			}
-			else
-			{
-				address = DestinationStringParser.parseBURLString(str,type);
-			}
-		}
-        catch (AddressException e)
-        {
-			JMSException ex = new JMSException("Error parsing destination string, due to : " + e.getMessage());
-			ex.initCause(e);
-			ex.setLinkedException(e);
-			throw ex;
-		}
+        _address = DestinationStringParser.parseDestinationString(str, getType());
     }
 
     protected Address getAddress()
     {
-    	return address;
+        return _address;
     }
 
     @Override
@@ -111,69 +84,9 @@ public abstract class QpidDestination implements Destination, Referenceable
                 null);          // factory location
     }
 
-
-    // ------- utility methods -------
-    
-    static
-    {
-        defaultDestSyntax = DestSyntax.getSyntaxType(
-                     System.getProperty(ClientProperties.DEST_SYNTAX,
-                                        DestSyntax.ADDR.toString()));
-    }
-    
-    public enum DestSyntax 
-    {        
-        BURL,ADDR;
-        
-        public static DestSyntax getSyntaxType(String s)
-        {
-            if (("BURL").equals(s))
-            {
-                return BURL;
-            }
-            else if (("ADDR").equals(s))
-            {
-                return ADDR;
-            }
-            else
-            {
-                throw new IllegalArgumentException("Invalid Destination Syntax Type" +
-                                                   " should be one of {BURL|ADDR}");
-            }
-        }
-    }
-    
-    public  static DestSyntax getDestType(String str)
-    {
-        if (str.startsWith("ADDR:"))                
-        {
-            return DestSyntax.ADDR;
-        }
-        else if (str.startsWith("BURL:"))
-        {
-            return DestSyntax.BURL;
-        }
-        else
-        {
-            return defaultDestSyntax;
-        }
-    }
-    
-    public static String stripSyntaxPrefix(String str)
-    {
-        if (str.startsWith("BURL:") || str.startsWith("ADDR:"))
-        {
-            return str.substring(5,str.length());
-        }
-        else
-        {
-            return str;
-        }
-    }
-
     @Override
     public String toString()
     {
-        return address == null ? "" : address.toString();
+        return _address == null ? "" : _address.toString();
     }
 }
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidQueue.java b/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidQueue.java
index d37924566a..de5014c7bc 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidQueue.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidQueue.java
@@ -23,24 +23,30 @@ package org.apache.qpid.jms;
 import javax.jms.JMSException;
 import javax.jms.Queue;
 
+import org.apache.qpid.jms.QpidDestination.DestinationType;
+
 public class QpidQueue extends QpidDestination implements Queue 
 {
-	public QpidQueue()
-	{
-		super(DestinationType.QUEUE);
-	}
+    public QpidQueue()
+    {
+    }
 
-	public QpidQueue(String str) throws JMSException
-	{
-		super(DestinationType.QUEUE);
+    public QpidQueue(String str) throws JMSException
+    {
         setDestinationString(str);
-	}
+    }
+
+    @Override
+    public DestinationType getType()
+    {
+        return DestinationType.QUEUE;
+    }
 
-	@Override
-	public String getQueueName() throws JMSException 
-	{
-       return address.getName();
-	}
+    @Override
+    public String getQueueName() throws JMSException
+    {
+        return _address.getName();
+    }
 
     @Override
     public boolean equals(Object obj)
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidTopic.java b/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidTopic.java
index 2551415a16..c0ce5077e2 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidTopic.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/jms/QpidTopic.java
@@ -23,26 +23,28 @@ package org.apache.qpid.jms;
 import javax.jms.JMSException;
 import javax.jms.Topic;
 
-import org.apache.qpid.jms.QpidDestination.DestinationType;
-
 public class QpidTopic extends QpidDestination implements Topic 
 {
-	public QpidTopic()
-	{
-		super(DestinationType.TOPIC);
-	}
+    public QpidTopic()
+    {
+    }
+
+    public QpidTopic(String str) throws JMSException
+    {
+        setDestinationString(str);
+    }
 
-	public QpidTopic(String str) throws JMSException
-	{
-		super(DestinationType.TOPIC);
-		setDestinationString(str);
-	}
+    @Override
+    public DestinationType getType()
+    {
+        return DestinationType.TOPIC;
+    }
 
-	@Override
-	public String getTopicName() throws JMSException 
-	{
-        return address.getSubject() == null ? "" : address.getSubject();
-	}
+    @Override
+    public String getTopicName() throws JMSException
+    {
+        return _address.getSubject() == null ? "" : _address.getSubject();
+    }
 
     @Override
     public boolean equals(Object obj)
@@ -61,13 +63,13 @@ public class QpidTopic extends QpidDestination implements Topic
 
         try
         {
-            if (!address.getName().equals(topic.getAddress().getName()))
+            if (!_address.getName().equals(topic.getAddress().getName()))
             {
                 return false;
             }
 
             // The subject being the topic name
-            if (!address.getSubject().equals(topic.getAddress().getSubject()))
+            if (!_address.getSubject().equals(topic.getAddress().getSubject()))
             {
                 return false;
             }