Repository: qpid-broker-j
Updated Branches:
  refs/heads/master 37365f918 -> efb7ec0c1


QPID-8060: [Broker-J][AMQP 0-8..0-10] 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/efb7ec0c
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/efb7ec0c
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/efb7ec0c

Branch: refs/heads/master
Commit: efb7ec0c1d68f130cf7d5598b4eded9b499abc1d
Parents: 37365f9
Author: Alex Rudyy <[email protected]>
Authored: Wed Dec 27 15:49:24 2017 +0000
Committer: Alex Rudyy <[email protected]>
Committed: Wed Dec 27 15:49:24 2017 +0000

----------------------------------------------------------------------
 .../qpid/server/exchange/AbstractExchange.java   |  4 +---
 .../apache/qpid/server/queue/AbstractQueue.java  |  4 +---
 .../UnknownAlternateBindingException.java        | 12 ++++++++++--
 .../qpid/server/exchange/DirectExchangeTest.java |  5 +++--
 .../qpid/server/queue/AbstractQueueTestBase.java |  5 +++--
 .../protocol/v0_10/ServerSessionDelegate.java    | 19 ++++++++++---------
 .../qpid/server/protocol/v0_8/AMQChannel.java    | 10 ++++------
 7 files changed, 32 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/efb7ec0c/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 1c07178..6227384 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
@@ -1055,9 +1055,7 @@ public abstract class AbstractExchange<T extends 
AbstractExchange<T>>
                     
_virtualHost.getAttainedMessageDestination(destinationName, mayCreate);
             if (messageDestination == null)
             {
-                throw new UnknownAlternateBindingException(String.format(
-                        "Cannot create alternate binding for '%s' : Alternate 
binding destination '%s' cannot be found.",
-                        getName(), destinationName));
+                throw new UnknownAlternateBindingException(destinationName);
             }
             else if (messageDestination == this)
             {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/efb7ec0c/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 ceb9610..09ec51c 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
@@ -3535,9 +3535,7 @@ public abstract class AbstractQueue<X extends 
AbstractQueue<X>>
                     
_virtualHost.getAttainedMessageDestination(destinationName, mayCreate);
             if (messageDestination == null)
             {
-                throw new UnknownAlternateBindingException(String.format(
-                        "Cannot create alternate binding for '%s' : Alternate 
binding destination '%s' cannot be found.",
-                        getName(), destinationName));
+                throw new UnknownAlternateBindingException(destinationName);
             }
             else if (messageDestination == this)
             {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/efb7ec0c/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
index c3f88e6..0d3e581 100644
--- 
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
@@ -24,8 +24,16 @@ import 
org.apache.qpid.server.configuration.IllegalConfigurationException;
 
 public class UnknownAlternateBindingException extends 
IllegalConfigurationException
 {
-    public UnknownAlternateBindingException(final String message)
+    private final String _alternateBindingName;
+
+    public UnknownAlternateBindingException(final String alternateBindingName)
+    {
+        super(String.format("Alternate binding destination '%s' is not 
found.", alternateBindingName));
+        _alternateBindingName = alternateBindingName;
+    }
+
+    public String getAlternateBindingName()
     {
-        super(message);
+        return _alternateBindingName;
     }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/efb7ec0c/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 653a0a2..e23ed39 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
@@ -168,8 +168,9 @@ public class DirectExchangeTest extends QpidTestCase
         Map<String, Object> attributes = new HashMap<>();
         attributes.put(Exchange.NAME, getTestName());
         attributes.put(Exchange.TYPE, ExchangeDefaults.DIRECT_EXCHANGE_CLASS);
+        String alternateBinding = "nonExisting";
         attributes.put(Exchange.ALTERNATE_BINDING,
-                       Collections.singletonMap(AlternateBinding.DESTINATION, 
"nonExisting"));
+                       Collections.singletonMap(AlternateBinding.DESTINATION, 
alternateBinding));
 
         try
         {
@@ -178,7 +179,7 @@ public class DirectExchangeTest extends QpidTestCase
         }
         catch (UnknownAlternateBindingException e)
         {
-            // pass
+            assertEquals("Unexpected exception alternate binding", 
alternateBinding, e.getAlternateBindingName());
         }
     }
 

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/efb7ec0c/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 e9ffcce..727f4c0 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
@@ -975,7 +975,8 @@ abstract class AbstractQueueTestBase extends QpidTestCase
     {
         Map<String, Object> attributes = new HashMap<>(_arguments);
         attributes.put(Queue.NAME, getTestName());
-        attributes.put(Queue.ALTERNATE_BINDING, 
Collections.singletonMap(AlternateBinding.DESTINATION, "nonExisting"));
+        String alternateBinding = "nonExisting";
+        attributes.put(Queue.ALTERNATE_BINDING, 
Collections.singletonMap(AlternateBinding.DESTINATION, alternateBinding));
 
         try
         {
@@ -984,7 +985,7 @@ abstract class AbstractQueueTestBase extends QpidTestCase
         }
         catch (UnknownAlternateBindingException e)
         {
-            // pass
+            assertEquals("Unexpected exception alternate binding", 
alternateBinding, e.getAlternateBindingName());
         }
     }
 

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/efb7ec0c/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 5af1cef..1ef431c 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
@@ -952,11 +952,12 @@ public class ServerSessionDelegate extends 
MethodDelegate<ServerSession> impleme
                                                                                
    + exchangeName + " which begins with reserved name or prefix.");
                     }
                 }
-                catch(UnknownAlternateBindingException e)
+                catch (UnknownAlternateBindingException e)
                 {
-
-                    exception(session, method, ExecutionErrorCode.NOT_FOUND,
-                                                                "Unknown 
alternate exchange " + alternateExchangeName);
+                    exception(session,
+                              method,
+                              ExecutionErrorCode.NOT_FOUND,
+                              String.format("Unknown alternate exchange '%s'", 
e.getAlternateBindingName()));
                 }
                 catch(NoFactoryForTypeException e)
                 {
@@ -1540,13 +1541,11 @@ public class ServerSessionDelegate extends 
MethodDelegate<ServerSession> impleme
         }
         else
         {
-
-            final String alternateExchangeName = method.getAlternateExchange();
             try
             {
                 final Map<String, Object> arguments = 
QueueArgumentsConverter.convertWireArgsToModel(queueName,
                                                                                
                      method.getArguments());
-
+                final String alternateExchangeName = 
method.getAlternateExchange();
                 if (method.hasAlternateExchange() && 
!nameNullOrEmpty(alternateExchangeName))
                 {
                     validateAlternateExchangeIsNotQueue(addressSpace, 
alternateExchangeName);
@@ -1603,8 +1602,10 @@ public class ServerSessionDelegate extends 
MethodDelegate<ServerSession> impleme
             }
             catch (UnknownAlternateBindingException e)
             {
-                exception(session, method, ExecutionErrorCode.NOT_FOUND,
-                          "Unknown alternate exchange " + 
alternateExchangeName);
+                exception(session,
+                          method,
+                          ExecutionErrorCode.NOT_FOUND,
+                          String.format("Unknown alternate exchange '%s'", 
e.getAlternateBindingName()));
             }
             catch (IllegalArgumentException | IllegalConfigurationException e)
             {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/efb7ec0c/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 d942abd..a12b483 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
@@ -2642,7 +2642,6 @@ public class AMQChannel extends 
AbstractAMQPSession<AMQChannel, ConsumerTarget_0
             {
                 String name = exchangeName.toString();
                 String typeString = type == null ? null : type.toString();
-                String alternateExchangeName = null;
                 try
                 {
 
@@ -2660,7 +2659,7 @@ public class AMQChannel extends 
AbstractAMQPSession<AMQChannel, ConsumerTarget_0
                     Object alternateExchange = 
attributes.remove(ALTERNATE_EXCHANGE);
                     if (alternateExchange != null)
                     {
-                        alternateExchangeName = 
String.valueOf(alternateExchange);
+                        String alternateExchangeName = 
String.valueOf(alternateExchange);
                         validateAlternateExchangeIsNotQueue(virtualHost, 
alternateExchangeName);
                         attributes.put(Exchange.ALTERNATE_BINDING,
                                        
Collections.singletonMap(AlternateBinding.DESTINATION, alternateExchangeName));
@@ -2724,7 +2723,7 @@ public class AMQChannel extends 
AbstractAMQPSession<AMQChannel, ConsumerTarget_0
                 }
                 catch (UnknownAlternateBindingException e)
                 {
-                    final String message = String.format("Unknown alternate 
exchange '%s'", alternateExchangeName);
+                    final String message = String.format("Unknown alternate 
exchange '%s'", e.getAlternateBindingName());
                     _connection.sendConnectionClose(ErrorCodes.NOT_FOUND, 
message, getChannelId());
 
                 }
@@ -3002,7 +3001,6 @@ public class AMQChannel extends 
AbstractAMQPSession<AMQChannel, ConsumerTarget_0
         }
         else
         {
-            String alternateExchangeName = null;
             try
             {
                 final String queueNameString = 
AMQShortString.toString(queueName);
@@ -3010,7 +3008,7 @@ public class AMQChannel extends 
AbstractAMQPSession<AMQChannel, ConsumerTarget_0
                 Object alternateExchange = 
wireArguments.get(ALTERNATE_EXCHANGE);
                 if (alternateExchange != null)
                 {
-                    alternateExchangeName = String.valueOf(alternateExchange);
+                    String alternateExchangeName = 
String.valueOf(alternateExchange);
                     validateAlternateExchangeIsNotQueue(virtualHost, 
alternateExchangeName);
                 }
 
@@ -3131,7 +3129,7 @@ public class AMQChannel extends 
AbstractAMQPSession<AMQChannel, ConsumerTarget_0
             }
             catch (UnknownAlternateBindingException e)
             {
-                final String message = String.format("Unknown alternate 
exchange: '%s'", alternateExchangeName);
+                final String message = String.format("Unknown alternate 
exchange: '%s'", e.getAlternateBindingName());
                 _connection.sendConnectionClose(ErrorCodes.NOT_FOUND, message, 
getChannelId());
             }
             catch (IllegalArgumentException | IllegalConfigurationException e)


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

Reply via email to