Repository: qpid-broker-j Updated Branches: refs/heads/master 7b4e3e8d5 -> 6e839776e
QPID-8060: [Broker-J] [AMQP 0-8..0-9-1] Address review comments Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/6e839776 Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/6e839776 Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/6e839776 Branch: refs/heads/master Commit: 6e839776eaaa83be78d09460c2be98f04de8abb5 Parents: 7b4e3e8 Author: Alex Rudyy <oru...@apache.org> Authored: Wed Dec 13 17:17:41 2017 +0000 Committer: Alex Rudyy <oru...@apache.org> Committed: Wed Dec 13 17:45:58 2017 +0000 ---------------------------------------------------------------------- .../qpid/server/exchange/AbstractExchange.java | 6 ++-- .../model/UnknownConfiguredObjectException.java | 13 ++------ .../apache/qpid/server/queue/AbstractQueue.java | 5 ++-- .../UnknownAlternateBindingException.java | 31 ++++++++++++++++++++ .../server/exchange/DirectExchangeTest.java | 4 +-- .../server/queue/AbstractQueueTestBase.java | 4 +-- .../protocol/v0_10/ServerSessionDelegate.java | 15 ++++++---- .../qpid/server/protocol/v0_8/AMQChannel.java | 21 +++++++------ 8 files changed, 64 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/6e839776/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java ---------------------------------------------------------------------- diff --git a/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java b/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java index b3714e9..1c07178 100644 --- a/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java +++ b/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java @@ -71,7 +71,6 @@ import org.apache.qpid.server.model.PublishingLink; import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.model.State; import org.apache.qpid.server.model.StateTransition; -import org.apache.qpid.server.model.UnknownConfiguredObjectException; import org.apache.qpid.server.protocol.LinkModel; import org.apache.qpid.server.queue.CreatingLinkInfo; import org.apache.qpid.server.security.SecurityToken; @@ -85,6 +84,7 @@ import org.apache.qpid.server.virtualhost.MessageDestinationIsAlternateException import org.apache.qpid.server.virtualhost.QueueManagingVirtualHost; import org.apache.qpid.server.virtualhost.RequiredExchangeException; import org.apache.qpid.server.virtualhost.ReservedExchangeNameException; +import org.apache.qpid.server.virtualhost.UnknownAlternateBindingException; import org.apache.qpid.server.virtualhost.VirtualHostUnavailableException; public abstract class AbstractExchange<T extends AbstractExchange<T>> @@ -1055,9 +1055,9 @@ public abstract class AbstractExchange<T extends AbstractExchange<T>> _virtualHost.getAttainedMessageDestination(destinationName, mayCreate); if (messageDestination == null) { - throw new UnknownConfiguredObjectException(String.format( + throw new UnknownAlternateBindingException(String.format( "Cannot create alternate binding for '%s' : Alternate binding destination '%s' cannot be found.", - getName(), destinationName), ConfiguredObject.class, destinationName); + getName(), destinationName)); } else if (messageDestination == this) { http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/6e839776/broker-core/src/main/java/org/apache/qpid/server/model/UnknownConfiguredObjectException.java ---------------------------------------------------------------------- diff --git a/broker-core/src/main/java/org/apache/qpid/server/model/UnknownConfiguredObjectException.java b/broker-core/src/main/java/org/apache/qpid/server/model/UnknownConfiguredObjectException.java index f460df7..f35a68a 100644 --- a/broker-core/src/main/java/org/apache/qpid/server/model/UnknownConfiguredObjectException.java +++ b/broker-core/src/main/java/org/apache/qpid/server/model/UnknownConfiguredObjectException.java @@ -28,22 +28,13 @@ public class UnknownConfiguredObjectException extends IllegalArgumentException private String _name; private UUID _id; - public UnknownConfiguredObjectException(String exceptionMessage, - final Class<? extends ConfiguredObject> category, - final String name) + public UnknownConfiguredObjectException(final Class<? extends ConfiguredObject> category, final String name) { - super(exceptionMessage); + super("Could not find object of category " + category.getSimpleName() + " with name '" + name + "'"); _category = category; _name = name; } - public UnknownConfiguredObjectException(final Class<? extends ConfiguredObject> category, final String name) - { - this("Could not find object of category " + category.getSimpleName() + " with name '" + name + "'", - category, - name); - } - public UnknownConfiguredObjectException(final Class<? extends ConfiguredObject> category, final UUID id) { super("Could not find object of category " + category.getSimpleName() + " with id " + id); http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/6e839776/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java ---------------------------------------------------------------------- diff --git a/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java b/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java index 39584d6..ceb9610 100644 --- a/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java +++ b/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java @@ -120,6 +120,7 @@ import org.apache.qpid.server.util.ServerScopedRuntimeException; import org.apache.qpid.server.virtualhost.HouseKeepingTask; import org.apache.qpid.server.virtualhost.MessageDestinationIsAlternateException; import org.apache.qpid.server.virtualhost.QueueManagingVirtualHost; +import org.apache.qpid.server.virtualhost.UnknownAlternateBindingException; import org.apache.qpid.server.virtualhost.VirtualHostUnavailableException; public abstract class AbstractQueue<X extends AbstractQueue<X>> @@ -3534,9 +3535,9 @@ public abstract class AbstractQueue<X extends AbstractQueue<X>> _virtualHost.getAttainedMessageDestination(destinationName, mayCreate); if (messageDestination == null) { - throw new UnknownConfiguredObjectException(String.format( + throw new UnknownAlternateBindingException(String.format( "Cannot create alternate binding for '%s' : Alternate binding destination '%s' cannot be found.", - getName(), destinationName), ConfiguredObject.class, destinationName); + getName(), destinationName)); } else if (messageDestination == this) { http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/6e839776/broker-core/src/main/java/org/apache/qpid/server/virtualhost/UnknownAlternateBindingException.java ---------------------------------------------------------------------- diff --git a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/UnknownAlternateBindingException.java b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/UnknownAlternateBindingException.java new file mode 100644 index 0000000..c3f88e6 --- /dev/null +++ b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/UnknownAlternateBindingException.java @@ -0,0 +1,31 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.qpid.server.virtualhost; + +import org.apache.qpid.server.configuration.IllegalConfigurationException; + +public class UnknownAlternateBindingException extends IllegalConfigurationException +{ + public UnknownAlternateBindingException(final String message) + { + super(message); + } +} http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/6e839776/broker-core/src/test/java/org/apache/qpid/server/exchange/DirectExchangeTest.java ---------------------------------------------------------------------- diff --git a/broker-core/src/test/java/org/apache/qpid/server/exchange/DirectExchangeTest.java b/broker-core/src/test/java/org/apache/qpid/server/exchange/DirectExchangeTest.java index 310876f..653a0a2 100644 --- a/broker-core/src/test/java/org/apache/qpid/server/exchange/DirectExchangeTest.java +++ b/broker-core/src/test/java/org/apache/qpid/server/exchange/DirectExchangeTest.java @@ -39,11 +39,11 @@ import org.apache.qpid.server.model.BrokerTestHelper; import org.apache.qpid.server.model.Exchange; import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.model.State; -import org.apache.qpid.server.model.UnknownConfiguredObjectException; import org.apache.qpid.server.model.VirtualHost; import org.apache.qpid.server.store.TransactionLogResource; import org.apache.qpid.server.virtualhost.MessageDestinationIsAlternateException; import org.apache.qpid.server.virtualhost.ReservedExchangeNameException; +import org.apache.qpid.server.virtualhost.UnknownAlternateBindingException; import org.apache.qpid.test.utils.QpidTestCase; public class DirectExchangeTest extends QpidTestCase @@ -176,7 +176,7 @@ public class DirectExchangeTest extends QpidTestCase _vhost.createChild(Exchange.class, attributes); fail("Expected exception is not thrown"); } - catch (UnknownConfiguredObjectException e) + catch (UnknownAlternateBindingException e) { // pass } http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/6e839776/broker-core/src/test/java/org/apache/qpid/server/queue/AbstractQueueTestBase.java ---------------------------------------------------------------------- diff --git a/broker-core/src/test/java/org/apache/qpid/server/queue/AbstractQueueTestBase.java b/broker-core/src/test/java/org/apache/qpid/server/queue/AbstractQueueTestBase.java index 45222f5..e9ffcce 100644 --- a/broker-core/src/test/java/org/apache/qpid/server/queue/AbstractQueueTestBase.java +++ b/broker-core/src/test/java/org/apache/qpid/server/queue/AbstractQueueTestBase.java @@ -67,7 +67,6 @@ import org.apache.qpid.server.model.Exchange; import org.apache.qpid.server.model.OverflowPolicy; import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.model.QueueNotificationListener; -import org.apache.qpid.server.model.UnknownConfiguredObjectException; import org.apache.qpid.server.queue.AbstractQueue.QueueEntryFilter; import org.apache.qpid.server.store.StoredMessage; import org.apache.qpid.server.store.TransactionLogResource; @@ -75,6 +74,7 @@ import org.apache.qpid.server.util.Action; import org.apache.qpid.server.util.StateChangeListener; import org.apache.qpid.server.virtualhost.MessageDestinationIsAlternateException; import org.apache.qpid.server.virtualhost.QueueManagingVirtualHost; +import org.apache.qpid.server.virtualhost.UnknownAlternateBindingException; import org.apache.qpid.test.utils.QpidTestCase; abstract class AbstractQueueTestBase extends QpidTestCase @@ -982,7 +982,7 @@ abstract class AbstractQueueTestBase extends QpidTestCase _virtualHost.createChild(Queue.class, attributes); fail("Expected exception is not thrown"); } - catch (UnknownConfiguredObjectException e) + catch (UnknownAlternateBindingException e) { // pass } http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/6e839776/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java b/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java index ccaee39..5af1cef 100644 --- a/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java +++ b/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java @@ -62,7 +62,6 @@ import org.apache.qpid.server.model.LifetimePolicy; import org.apache.qpid.server.model.NamedAddressSpace; import org.apache.qpid.server.model.NoFactoryForTypeException; import org.apache.qpid.server.model.Queue; -import org.apache.qpid.server.model.UnknownConfiguredObjectException; import org.apache.qpid.server.protocol.ErrorCodes; import org.apache.qpid.server.protocol.v0_10.transport.*; import org.apache.qpid.server.queue.QueueArgumentsConverter; @@ -86,6 +85,7 @@ import org.apache.qpid.server.util.ServerScopedRuntimeException; import org.apache.qpid.server.virtualhost.MessageDestinationIsAlternateException; import org.apache.qpid.server.virtualhost.RequiredExchangeException; import org.apache.qpid.server.virtualhost.ReservedExchangeNameException; +import org.apache.qpid.server.virtualhost.UnknownAlternateBindingException; import org.apache.qpid.server.virtualhost.VirtualHostUnavailableException; public class ServerSessionDelegate extends MethodDelegate<ServerSession> implements ProtocolDelegate<ServerSession> @@ -952,11 +952,11 @@ public class ServerSessionDelegate extends MethodDelegate<ServerSession> impleme + exchangeName + " which begins with reserved name or prefix."); } } - catch(UnknownConfiguredObjectException e) + catch(UnknownAlternateBindingException e) { exception(session, method, ExecutionErrorCode.NOT_FOUND, - "Unknown alternate exchange " + e.getName()); + "Unknown alternate exchange " + alternateExchangeName); } catch(NoFactoryForTypeException e) { @@ -1541,11 +1541,9 @@ public class ServerSessionDelegate extends MethodDelegate<ServerSession> impleme else { + final String alternateExchangeName = method.getAlternateExchange(); try { - - final String alternateExchangeName = method.getAlternateExchange(); - final Map<String, Object> arguments = QueueArgumentsConverter.convertWireArgsToModel(queueName, method.getArguments()); @@ -1603,6 +1601,11 @@ public class ServerSessionDelegate extends MethodDelegate<ServerSession> impleme { exception(session, method, ExecutionErrorCode.UNAUTHORIZED_ACCESS, e.getMessage()); } + catch (UnknownAlternateBindingException e) + { + exception(session, method, ExecutionErrorCode.NOT_FOUND, + "Unknown alternate exchange " + alternateExchangeName); + } catch (IllegalArgumentException | IllegalConfigurationException e) { exception(session, method, ExecutionErrorCode.ILLEGAL_ARGUMENT, e.getMessage()); http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/6e839776/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java index 07d0f91..d942abd 100644 --- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java +++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java @@ -83,7 +83,6 @@ import org.apache.qpid.server.model.LifetimePolicy; import org.apache.qpid.server.model.NamedAddressSpace; import org.apache.qpid.server.model.NoFactoryForTypeException; import org.apache.qpid.server.model.Queue; -import org.apache.qpid.server.model.UnknownConfiguredObjectException; import org.apache.qpid.server.protocol.ErrorCodes; import org.apache.qpid.server.protocol.ProtocolVersion; import org.apache.qpid.server.protocol.v0_8.UnacknowledgedMessageMap.Visitor; @@ -103,6 +102,7 @@ import org.apache.qpid.server.util.ServerScopedRuntimeException; import org.apache.qpid.server.virtualhost.MessageDestinationIsAlternateException; import org.apache.qpid.server.virtualhost.RequiredExchangeException; import org.apache.qpid.server.virtualhost.ReservedExchangeNameException; +import org.apache.qpid.server.virtualhost.UnknownAlternateBindingException; public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0_8> implements AsyncAutoCommitTransaction.FutureRecorder, @@ -2642,6 +2642,7 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0 { String name = exchangeName.toString(); String typeString = type == null ? null : type.toString(); + String alternateExchangeName = null; try { @@ -2659,9 +2660,10 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0 Object alternateExchange = attributes.remove(ALTERNATE_EXCHANGE); if (alternateExchange != null) { - validateAlternateExchangeIsNotQueue(virtualHost, String.valueOf(alternateExchange)); + alternateExchangeName = String.valueOf(alternateExchange); + validateAlternateExchangeIsNotQueue(virtualHost, alternateExchangeName); attributes.put(Exchange.ALTERNATE_BINDING, - Collections.singletonMap(AlternateBinding.DESTINATION, alternateExchange)); + Collections.singletonMap(AlternateBinding.DESTINATION, alternateExchangeName)); } exchange = virtualHost.createMessageDestination(Exchange.class, attributes); @@ -2720,9 +2722,9 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0 _connection.sendConnectionClose(ErrorCodes.ACCESS_REFUSED, e.getMessage(), getChannelId()); } - catch (UnknownConfiguredObjectException e) + catch (UnknownAlternateBindingException e) { - final String message = String.format("Unknown alternate exchange '%s'", e.getName()); + final String message = String.format("Unknown alternate exchange '%s'", alternateExchangeName); _connection.sendConnectionClose(ErrorCodes.NOT_FOUND, message, getChannelId()); } @@ -3000,7 +3002,7 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0 } else { - + String alternateExchangeName = null; try { final String queueNameString = AMQShortString.toString(queueName); @@ -3008,7 +3010,8 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0 Object alternateExchange = wireArguments.get(ALTERNATE_EXCHANGE); if (alternateExchange != null) { - validateAlternateExchangeIsNotQueue(virtualHost, String.valueOf(alternateExchange)); + alternateExchangeName = String.valueOf(alternateExchange); + validateAlternateExchangeIsNotQueue(virtualHost, alternateExchangeName); } Map<String, Object> attributes = @@ -3126,9 +3129,9 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0 { _connection.sendConnectionClose(ErrorCodes.ACCESS_REFUSED, e.getMessage(), getChannelId()); } - catch (UnknownConfiguredObjectException e) + catch (UnknownAlternateBindingException e) { - final String message = String.format("Unknown alternate exchange: '%s'", e.getName()); + final String message = String.format("Unknown alternate exchange: '%s'", alternateExchangeName); _connection.sendConnectionClose(ErrorCodes.NOT_FOUND, message, getChannelId()); } catch (IllegalArgumentException | IllegalConfigurationException e) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org