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]