Author: rgodfrey
Date: Fri Mar  7 17:18:27 2014
New Revision: 1575335

URL: http://svn.apache.org/r1575335
Log:
QPID-5601 : Address review comments from Robbie Gemmell

Modified:
    
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
    
qpid/trunk/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java
    
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java
    
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java
    
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java
    
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java
    
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java
    
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java
    
qpid/trunk/qpid/java/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java

Modified: 
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
 Fri Mar  7 17:18:27 2014
@@ -34,6 +34,7 @@ import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.log4j.Logger;
+import org.apache.qpid.exchange.ExchangeDefaults;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.exchange.AMQUnknownExchangeType;
 import org.apache.qpid.server.exchange.ExchangeImpl;
@@ -382,7 +383,7 @@ public abstract class AbstractVirtualHos
         String exchangeName = queueConfiguration.getExchange();
 
 
-        if("".equals(exchangeName))
+        if(ExchangeDefaults.DEFAULT_EXCHANGE_NAME.equals(exchangeName))
         {
             //get routing keys in configuration (returns empty list if none 
are defined)
             List<?> routingKeys = queueConfiguration.getRoutingKeys();

Modified: 
qpid/trunk/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java
 Fri Mar  7 17:18:27 2014
@@ -685,8 +685,9 @@ public class ServerSessionDelegate exten
                 return;
             }
         }
-        if(method.getExchange() == null || method.getExchange().equals(""))
+        if(nameNullOrEmpty(method.getExchange()))
         {
+            // special case handling to fake the existence of the default 
exchange for 0-10
             if(!DirectExchange.TYPE.getType().equals(method.getType()))
             {
                 exception(session, method, ExecutionErrorCode.NOT_ALLOWED,
@@ -694,7 +695,7 @@ public class ServerSessionDelegate exten
                           + " of type " + DirectExchange.TYPE.getType()
                           + " to " + method.getType() +".");
             }
-            if(method.hasAlternateExchange() && 
!"".equals(method.getAlternateExchange()))
+            if(!nameNullOrEmpty(method.getAlternateExchange()))
             {
                 exception(session, method, ExecutionErrorCode.NOT_ALLOWED,
                           "Attempt to set alternate exchange of the default 
exchange "
@@ -901,8 +902,9 @@ public class ServerSessionDelegate exten
 
         final String exchangeName = method.getName();
 
-        if(exchangeName == null || exchangeName.equals(""))
+        if(nameNullOrEmpty(exchangeName))
         {
+            // Fake the existence of the "default" exchange for 0-10
             result.setDurable(true);
             result.setType(DirectExchange.TYPE.getType());
             result.setNotFound(false);
@@ -1046,7 +1048,7 @@ public class ServerSessionDelegate exten
         ExchangeImpl exchange;
         AMQQueue queue;
         boolean isDefaultExchange;
-        if(method.hasExchange() && !method.getExchange().equals(""))
+        if(!nameNullOrEmpty(method.getExchange()))
         {
             isDefaultExchange = false;
             exchange = virtualHost.getExchange(method.getExchange());
@@ -1064,6 +1066,7 @@ public class ServerSessionDelegate exten
 
         if(isDefaultExchange)
         {
+            // fake the existence of the "default" exchange for 0-10
             if(method.hasQueue())
             {
                 queue = getQueue(session, method.getQueue());

Modified: 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java
 Fri Mar  7 17:18:27 2014
@@ -129,7 +129,7 @@ public class BasicConsumeMethodHandler i
                     }
                     else
                     {
-                        AMQShortString msg = new AMQShortString("Non-unique 
consumer tag, '" + body.getConsumerTag() + "'");
+                        AMQShortString msg = 
AMQShortString.validValueOf("Non-unique consumer tag, '" + 
body.getConsumerTag() + "'");
 
                         MethodRegistry methodRegistry = 
protocolConnection.getMethodRegistry();
                         AMQMethodBody responseBody = 
methodRegistry.createConnectionCloseBody(AMQConstant.NOT_ALLOWED.getCode(),    
// replyCode
@@ -146,7 +146,7 @@ public class BasicConsumeMethodHandler i
 
                     MethodRegistry methodRegistry = 
protocolConnection.getMethodRegistry();
                     AMQMethodBody responseBody = 
methodRegistry.createChannelCloseBody(AMQConstant.ARGUMENT_INVALID.getCode(),
-                                                                               
        new AMQShortString(ise.getMessage()),
+                                                                               
        AMQShortString.validValueOf(ise.getMessage()),
                                                                                
        body.getClazz(),
                                                                                
        body.getMethod());
                     
protocolConnection.writeFrame(responseBody.generateFrame(channelId));

Modified: 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java
 Fri Mar  7 17:18:27 2014
@@ -83,7 +83,8 @@ public class ExchangeBoundHandler implem
         AMQShortString queueName = body.getQueue();
         AMQShortString routingKey = body.getRoutingKey();
         ExchangeBoundOkBody response;
-        if(exchangeName == null || 
exchangeName.equals(AMQShortString.EMPTY_STRING))
+
+        if(isDefaultExchange(exchangeName))
         {
             if(routingKey == null)
             {
@@ -98,7 +99,7 @@ public class ExchangeBoundHandler implem
                     {
 
                         response = 
methodRegistry.createExchangeBoundOkBody(QUEUE_NOT_FOUND,   // replyCode
-                                                                            
new AMQShortString("Queue '" + queueName + "' not found"));        // replyText
+                                                                            
AMQShortString.validValueOf("Queue '" + queueName + "' not found"));       // 
replyText
                     }
                     else
                     {
@@ -119,7 +120,7 @@ public class ExchangeBoundHandler implem
                     {
 
                         response = 
methodRegistry.createExchangeBoundOkBody(QUEUE_NOT_FOUND,   // replyCode
-                                                                            
new AMQShortString("Queue '" + queueName + "' not found"));        // replyText
+                                                                            
AMQShortString.validValueOf("Queue '" + queueName + "' not found"));       // 
replyText
                     }
                     else
                     {
@@ -136,7 +137,7 @@ public class ExchangeBoundHandler implem
 
 
                 response = 
methodRegistry.createExchangeBoundOkBody(EXCHANGE_NOT_FOUND,
-                                                                    new 
AMQShortString("Exchange '" + exchangeName + "' not found"));
+                                                                    
AMQShortString.validValueOf("Exchange '" + exchangeName + "' not found"));
             }
             else if (routingKey == null)
             {
@@ -161,7 +162,7 @@ public class ExchangeBoundHandler implem
                     {
 
                         response = 
methodRegistry.createExchangeBoundOkBody(QUEUE_NOT_FOUND,   // replyCode
-                            new AMQShortString("Queue '" + queueName + "' not 
found"));        // replyText
+                            AMQShortString.validValueOf("Queue '" + queueName 
+ "' not found"));       // replyText
                     }
                     else
                     {
@@ -175,7 +176,7 @@ public class ExchangeBoundHandler implem
                         {
 
                             response = 
methodRegistry.createExchangeBoundOkBody(QUEUE_NOT_BOUND,       // replyCode
-                                new AMQShortString("Queue '" + queueName + "' 
not bound to exchange '" + exchangeName + "'")); // replyText
+                                AMQShortString.validValueOf("Queue '" + 
queueName + "' not bound to exchange '" + exchangeName + "'"));        // 
replyText
                         }
                     }
                 }
@@ -187,7 +188,7 @@ public class ExchangeBoundHandler implem
                 {
 
                     response = 
methodRegistry.createExchangeBoundOkBody(QUEUE_NOT_FOUND,       // replyCode
-                        new AMQShortString("Queue '" + queueName + "' not 
found"));    // replyText
+                        AMQShortString.validValueOf("Queue '" + queueName + "' 
not found"));   // replyText
                 }
                 else
                 {
@@ -204,12 +205,8 @@ public class ExchangeBoundHandler implem
                         String message = "Queue '" + queueName + "' not bound 
with routing key '" +
                                             body.getRoutingKey() + "' to 
exchange '" + exchangeName + "'";
 
-                        if(message.length()>255)
-                        {
-                            message = message.substring(0,254);
-                        }
                         response = 
methodRegistry.createExchangeBoundOkBody(SPECIFIC_QUEUE_NOT_BOUND_WITH_RK,  // 
replyCode
-                            new AMQShortString(message));      // replyText
+                            AMQShortString.validValueOf(message));     // 
replyText
                     }
                 }
             }
@@ -225,11 +222,17 @@ public class ExchangeBoundHandler implem
                 {
 
                     response = 
methodRegistry.createExchangeBoundOkBody(NO_QUEUE_BOUND_WITH_RK,        // 
replyCode
-                        new AMQShortString("No queue bound with routing key '" 
+ body.getRoutingKey() +
+                        AMQShortString.validValueOf("No queue bound with 
routing key '" + body.getRoutingKey() +
                         "' to exchange '" + exchangeName + "'"));      // 
replyText
                 }
             }
         }
         session.writeFrame(response.generateFrame(channelId));
     }
+
+    protected boolean isDefaultExchange(final AMQShortString exchangeName)
+    {
+        return exchangeName == null || 
exchangeName.equals(AMQShortString.EMPTY_STRING);
+    }
+
 }

Modified: 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java
 Fri Mar  7 17:18:27 2014
@@ -79,7 +79,7 @@ public class ExchangeDeclareHandler impl
 
         ExchangeImpl exchange;
 
-        if(exchangeName == null || 
exchangeName.equals(AMQShortString.EMPTY_STRING))
+        if(isDefaultExchange(exchangeName))
         {
             if(!new 
AMQShortString(DirectExchange.TYPE.getType()).equals(body.getType()))
             {
@@ -171,4 +171,9 @@ public class ExchangeDeclareHandler impl
             session.writeFrame(responseBody.generateFrame(channelId));
         }
     }
+
+    protected boolean isDefaultExchange(final AMQShortString exchangeName)
+    {
+        return exchangeName == null || 
exchangeName.equals(AMQShortString.EMPTY_STRING);
+    }
 }

Modified: 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java
 Fri Mar  7 17:18:27 2014
@@ -21,6 +21,7 @@
 package org.apache.qpid.server.protocol.v0_8.handler;
 
 import org.apache.qpid.AMQException;
+import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.framing.ExchangeDeleteBody;
 import org.apache.qpid.framing.ExchangeDeleteOkBody;
 import org.apache.qpid.protocol.AMQConstant;
@@ -60,13 +61,14 @@ public class ExchangeDeleteHandler imple
         channel.sync();
         try
         {
-            final String exchangeName = body.getExchange() == null ? null : 
body.getExchange().toString();
 
-            if(exchangeName == null || "".equals(exchangeName))
+            if(isDefaultExchange(body.getExchange()))
             {
                 throw body.getConnectionException(AMQConstant.NOT_ALLOWED, 
"Default Exchange cannot be deleted");
             }
 
+            final String exchangeName = body.getExchange().toString();
+
             final ExchangeImpl exchange = 
virtualHost.getExchange(exchangeName);
             if(exchange == null)
             {
@@ -94,4 +96,10 @@ public class ExchangeDeleteHandler imple
             throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, 
e.getMessage());
         }
     }
+
+
+    protected boolean isDefaultExchange(final AMQShortString exchangeName)
+    {
+        return exchangeName == null || 
exchangeName.equals(AMQShortString.EMPTY_STRING);
+    }
 }

Modified: 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java
 Fri Mar  7 17:18:27 2014
@@ -101,13 +101,14 @@ public class QueueBindHandler implements
         {
             throw body.getChannelException(AMQConstant.NOT_FOUND, "Queue " + 
queueName + " does not exist.");
         }
-        final String exchangeName = body.getExchange() == null ? null : 
body.getExchange().toString();
 
-        if(exchangeName == null || "".equals(exchangeName))
+        if(isDefaultExchange(body.getExchange()))
         {
             throw body.getConnectionException(AMQConstant.NOT_ALLOWED, "Cannot 
bind the queue " + queueName + " to the default exchange");
         }
 
+        final String exchangeName = body.getExchange().toString();
+
         final ExchangeImpl exch = virtualHost.getExchange(exchangeName);
         if (exch == null)
         {
@@ -148,4 +149,9 @@ public class QueueBindHandler implements
 
         }
     }
+
+    protected boolean isDefaultExchange(final AMQShortString exchangeName)
+    {
+        return exchangeName == null || 
exchangeName.equals(AMQShortString.EMPTY_STRING);
+    }
 }

Modified: 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java
 Fri Mar  7 17:18:27 2014
@@ -94,7 +94,7 @@ public class QueueUnbindHandler implemen
             throw body.getChannelException(AMQConstant.NOT_FOUND, "Queue " + 
body.getQueue() + " does not exist.");
         }
 
-        if(body.getExchange() == null || 
body.getExchange().equals(AMQShortString.EMPTY_STRING))
+        if(isDefaultExchange(body.getExchange()))
         {
             throw body.getConnectionException(AMQConstant.NOT_ALLOWED, "Cannot 
unbind the queue " + queue.getName() + " from the default exchange");
         }
@@ -145,4 +145,10 @@ public class QueueUnbindHandler implemen
         channel.sync();
         session.writeFrame(responseBody.generateFrame(channelId));
     }
+
+    protected boolean isDefaultExchange(final AMQShortString exchangeName)
+    {
+        return exchangeName == null || 
exchangeName.equals(AMQShortString.EMPTY_STRING);
+    }
+
 }

Modified: 
qpid/trunk/qpid/java/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java
URL: 
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java?rev=1575335&r1=1575334&r2=1575335&view=diff
==============================================================================
--- 
qpid/trunk/qpid/java/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java
 (original)
+++ 
qpid/trunk/qpid/java/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java
 Fri Mar  7 17:18:27 2014
@@ -137,7 +137,7 @@ public class MessageConverter_0_10_to_0_
                 {
                     try
                     {
-                        ft.put(new AMQShortString(entry.getKey()), 
entry.getValue());
+                        ft.put(AMQShortString.validValueOf(entry.getKey()), 
entry.getValue());
                     }
                     catch (AMQPInvalidClassException e)
                     {



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to