QPID-366 : Reference counting not being decremented correctly and other persistence issues

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

3 files changed, 41 insertions, 36 deletions

diff --git a/java/common/src/main/java/org/apache/qpid/url/AMQBindingURL.java b/java/common/src/main/java/org/apache/qpid/url/AMQBindingURL.java
index 04d152acf5..2ee4ce21cb 100644
--- a/java/common/src/main/java/org/apache/qpid/url/AMQBindingURL.java
+++ b/java/common/src/main/java/org/apache/qpid/url/AMQBindingURL.java
@@ -26,9 +26,12 @@ import java.util.HashMap;
 
 import org.apache.qpid.exchange.ExchangeDefaults;
 import org.apache.qpid.framing.AMQShortString;
+import org.apache.log4j.Logger;
 
 public class AMQBindingURL implements BindingURL
 {
+    private static final Logger _logger = Logger.getLogger(AMQBindingURL.class);
+
     String _url;
     AMQShortString _exchangeClass;
     AMQShortString _exchangeName;
@@ -41,7 +44,7 @@ public class AMQBindingURL implements BindingURL
     {
         //format:
         // <exch_class>://<exch_name>/[<destination>]/[<queue>]?<option>='<value>'[,<option>='<value>']*
-
+        _logger.debug("Parsing URL: " + url);
         _url = url;
         _options = new HashMap<String, String>();
 
@@ -73,17 +76,19 @@ public class AMQBindingURL implements BindingURL
 
             if (exchangeName == null)
             {
-                URLHelper.parseError(-1, "Exchange Name not specified.", _url);
+                throw URLHelper.parseError(-1, "Exchange Name not specified.", _url);
             }
             else
             {
                 setExchangeName(exchangeName);
             }
 
+            String queueName;
+
             if (connection.getPath() == null ||
                     connection.getPath().equals(""))
             {
-                URLHelper.parseError(_url.indexOf(_exchangeName.toString()) + _exchangeName.length(),
+                throw URLHelper.parseError(_url.indexOf(_exchangeName.toString()) + _exchangeName.length(),
                         "Destination or Queue requried", _url);
             }
             else
@@ -91,7 +96,7 @@ public class AMQBindingURL implements BindingURL
                 int slash = connection.getPath().indexOf("/", 1);
                 if (slash == -1)
                 {
-                    URLHelper.parseError(_url.indexOf(_exchangeName.toString()) + _exchangeName.length(),
+                    throw URLHelper.parseError(_url.indexOf(_exchangeName.toString()) + _exchangeName.length(),
                             "Destination requried", _url);
                 }
                 else
@@ -99,7 +104,10 @@ public class AMQBindingURL implements BindingURL
                     String path = connection.getPath();
                     setDestinationName(path.substring(1, slash));
 
-                    setQueueName(path.substring(slash + 1));
+                    // We don't set queueName yet as the actual value we use depends on options set
+                    // when we are dealing with durable subscriptions
+
+                    queueName = path.substring(slash + 1);
 
                 }
             }
@@ -108,14 +116,19 @@ public class AMQBindingURL implements BindingURL
 
             processOptions();
 
+            // We can now call setQueueName as the URL is full parsed.
+
+            setQueueName(queueName);
+
             //Fragment is #string (not used)
             //System.out.println(connection.getFragment());
+            _logger.debug("URL Parsed: " + this);            
 
         }
         catch (URISyntaxException uris)
         {
 
-            URLHelper.parseError(uris.getIndex(), uris.getReason(), uris.getInput());
+            throw URLHelper.parseError(uris.getIndex(), uris.getReason(), uris.getInput());
 
         }
     }
@@ -125,7 +138,7 @@ public class AMQBindingURL implements BindingURL
         setExchangeClass(new AMQShortString(exchangeClass));
     }
 
-    private void setQueueName(String name)
+    private void setQueueName(String name) throws URLSyntaxException
     {
         setQueueName(new AMQShortString(name));
     }
@@ -155,8 +168,9 @@ public class AMQBindingURL implements BindingURL
         return _exchangeClass;
     }
 
-    public void setExchangeClass(AMQShortString exchangeClass)
+    private void setExchangeClass(AMQShortString exchangeClass)
     {
+
         _exchangeClass = exchangeClass;
     }
 
@@ -165,7 +179,7 @@ public class AMQBindingURL implements BindingURL
         return _exchangeName;
     }
 
-    public void setExchangeName(AMQShortString name)
+    private void setExchangeName(AMQShortString name)
     {
         _exchangeName = name;
 
@@ -180,40 +194,43 @@ public class AMQBindingURL implements BindingURL
         return _destinationName;
     }
 
-    public void setDestinationName(AMQShortString name)
+    private void setDestinationName(AMQShortString name)
     {
         _destinationName = name;
     }
 
     public AMQShortString getQueueName()
     {
+        return _queueName;
+    }
+
+    public void setQueueName(AMQShortString name) throws URLSyntaxException
+    {
         if (_exchangeClass.equals(ExchangeDefaults.TOPIC_EXCHANGE_CLASS))
         {
             if (Boolean.parseBoolean(getOption(OPTION_DURABLE)))
             {
                 if (containsOption(BindingURL.OPTION_CLIENTID) && containsOption(BindingURL.OPTION_SUBSCRIPTION))
                 {
-                    return new AMQShortString(getOption(BindingURL.OPTION_CLIENTID + ":" + BindingURL.OPTION_SUBSCRIPTION));
+                    _queueName = new AMQShortString(getOption(BindingURL.OPTION_CLIENTID + ":" + BindingURL.OPTION_SUBSCRIPTION));
                 }
                 else
                 {
-                    return getDestinationName();
+                    throw URLHelper.parseError(-1, "Durable subscription must have values for " + BindingURL.OPTION_CLIENTID + " and " + BindingURL.OPTION_SUBSCRIPTION + ".", _url);
+
                 }
             }
             else
             {
-                return getDestinationName();
+                _queueName = null;
             }
         }
         else
         {
-            return _queueName;
+            _queueName = name;
         }
-    }
 
-    public void setQueueName(AMQShortString name)
-    {
-        _queueName = name;
+
     }
 
     public String getOption(String key)
diff --git a/java/common/src/main/java/org/apache/qpid/url/BindingURL.java b/java/common/src/main/java/org/apache/qpid/url/BindingURL.java
index 86a8420d30..67be2db86f 100644
--- a/java/common/src/main/java/org/apache/qpid/url/BindingURL.java
+++ b/java/common/src/main/java/org/apache/qpid/url/BindingURL.java
@@ -40,29 +40,17 @@ public interface BindingURL
 
     AMQShortString getExchangeClass();
 
-    void setExchangeClass(AMQShortString name);
-
     AMQShortString getExchangeName();
 
-    void setExchangeName(AMQShortString name);
-
     AMQShortString getDestinationName();
 
-    void setDestinationName(AMQShortString name);
-
     AMQShortString getQueueName();
 
-    void setQueueName(AMQShortString name);
-
     String getOption(String key);
 
-    void setOption(String key, String value);
-
     boolean containsOption(String key);
 
     AMQShortString getRoutingKey();
 
-    void setRoutingKey(AMQShortString key);
-
     String toString();
 }
diff --git a/java/common/src/main/java/org/apache/qpid/url/URLHelper.java b/java/common/src/main/java/org/apache/qpid/url/URLHelper.java
index 2121346c02..806f879818 100644
--- a/java/common/src/main/java/org/apache/qpid/url/URLHelper.java
+++ b/java/common/src/main/java/org/apache/qpid/url/URLHelper.java
@@ -114,11 +114,11 @@ public class URLHelper
 
             if (sepIndex >= options.length() || sepIndex == 0)
             {
-                parseError(valueIndex, "Unterminated option", options);
+                throw parseError(valueIndex, "Unterminated option", options);
             }
             else
             {
-                parseError(sepIndex, "Unterminated option. Possible illegal option separator:'" +
+                throw parseError(sepIndex, "Unterminated option. Possible illegal option separator:'" +
                         options.charAt(sepIndex) + "'", options);
             }
         }
@@ -136,14 +136,14 @@ public class URLHelper
     }
 
 
-    public static void parseError(int index, String error, String url) throws URLSyntaxException
+    public static URLSyntaxException parseError(int index, String error, String url)
     {
-        parseError(index, 1, error, url);
+        return parseError(index, 1, error, url);
     }
 
-    public static void parseError(int index, int length, String error, String url) throws URLSyntaxException
+    public static URLSyntaxException parseError(int index, int length, String error, String url)
     {
-        throw new URLSyntaxException(url, error, index, length);
+        return new URLSyntaxException(url, error, index, length);
     }
 
     public static String printOptions(HashMap<String, String> options)