diff options
author | Robert Godfrey <rgodfrey@apache.org> | 2014-03-07 17:18:27 +0000 |
---|---|---|
committer | Robert Godfrey <rgodfrey@apache.org> | 2014-03-07 17:18:27 +0000 |
commit | 713dbcd70b5d2da2488c09676f18f1752683654e (patch) | |
tree | a7d25504b8854f2e03c6380bfd7979dee993ee17 /qpid | |
parent | 9e9d87a76f12567c2e0f65485bbf85b39fc6e437 (diff) | |
download | qpid-python-713dbcd70b5d2da2488c09676f18f1752683654e.tar.gz |
QPID-5601 : Address review comments from Robbie Gemmell
git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1575335 13f79535-47bb-0310-9956-ffa450edef68
Diffstat (limited to 'qpid')
9 files changed, 59 insertions, 27 deletions
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java index c5eb70c1f0..2eb2790382 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java @@ -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 AbstractVirtualHost implements VirtualHost, IConnectionReg 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(); diff --git a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java index d21da824e9..e258a27255 100644 --- a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java +++ b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java @@ -685,8 +685,9 @@ public class ServerSessionDelegate extends SessionDelegate 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 extends SessionDelegate + " 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 extends SessionDelegate 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 extends SessionDelegate 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 extends SessionDelegate if(isDefaultExchange) { + // fake the existence of the "default" exchange for 0-10 if(method.hasQueue()) { queue = getQueue(session, method.getQueue()); diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java index cb4f758fb7..37e531bd2a 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/BasicConsumeMethodHandler.java @@ -129,7 +129,7 @@ public class BasicConsumeMethodHandler implements StateAwareMethodListener<Basic } 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 implements StateAwareMethodListener<Basic 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)); diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java index fce1260155..d30961b13c 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeBoundHandler.java @@ -83,7 +83,8 @@ public class ExchangeBoundHandler implements StateAwareMethodListener<ExchangeBo 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 implements StateAwareMethodListener<ExchangeBo { 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 implements StateAwareMethodListener<ExchangeBo { 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 implements StateAwareMethodListener<ExchangeBo 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 implements StateAwareMethodListener<ExchangeBo { 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 implements StateAwareMethodListener<ExchangeBo { 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 implements StateAwareMethodListener<ExchangeBo { 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 implements StateAwareMethodListener<ExchangeBo 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 implements StateAwareMethodListener<ExchangeBo { 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); + } + } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java index 78d47aaa52..fd7d9c10ff 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java @@ -79,7 +79,7 @@ public class ExchangeDeclareHandler implements StateAwareMethodListener<Exchange 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 implements StateAwareMethodListener<Exchange session.writeFrame(responseBody.generateFrame(channelId)); } } + + protected boolean isDefaultExchange(final AMQShortString exchangeName) + { + return exchangeName == null || exchangeName.equals(AMQShortString.EMPTY_STRING); + } } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java index bc723fc3dd..9af383c3ca 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeleteHandler.java @@ -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 implements StateAwareMethodListener<ExchangeD 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 implements StateAwareMethodListener<ExchangeD throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, e.getMessage()); } } + + + protected boolean isDefaultExchange(final AMQShortString exchangeName) + { + return exchangeName == null || exchangeName.equals(AMQShortString.EMPTY_STRING); + } } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java index 7dc76d13d0..c14ac72818 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueBindHandler.java @@ -101,13 +101,14 @@ public class QueueBindHandler implements StateAwareMethodListener<QueueBindBody> { 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 StateAwareMethodListener<QueueBindBody> } } + + protected boolean isDefaultExchange(final AMQShortString exchangeName) + { + return exchangeName == null || exchangeName.equals(AMQShortString.EMPTY_STRING); + } } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java index abc9c8541c..f1bcce2ade 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/QueueUnbindHandler.java @@ -94,7 +94,7 @@ public class QueueUnbindHandler implements StateAwareMethodListener<QueueUnbindB 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 implements StateAwareMethodListener<QueueUnbindB channel.sync(); session.writeFrame(responseBody.generateFrame(channelId)); } + + protected boolean isDefaultExchange(final AMQShortString exchangeName) + { + return exchangeName == null || exchangeName.equals(AMQShortString.EMPTY_STRING); + } + } diff --git a/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 b/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 index d385f684ea..bde7d1446b 100644 --- a/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 +++ b/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 @@ -137,7 +137,7 @@ public class MessageConverter_0_10_to_0_8 implements MessageConverter<MessageTra { try { - ft.put(new AMQShortString(entry.getKey()), entry.getValue()); + ft.put(AMQShortString.validValueOf(entry.getKey()), entry.getValue()); } catch (AMQPInvalidClassException e) { |