[ 
https://issues.apache.org/jira/browse/ARTEMIS-4212?focusedWorklogId=857041&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-857041
 ]

ASF GitHub Bot logged work on ARTEMIS-4212:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 14/Apr/23 10:13
            Start Date: 14/Apr/23 10:13
    Worklog Time Spent: 10m 
      Work Description: gemmellr commented on code in PR #4421:
URL: https://github.com/apache/activemq-artemis/pull/4421#discussion_r1166615229


##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java:
##########
@@ -124,7 +124,7 @@ public void testAutoCreateOnTopic() throws Exception {
       Connection connection = factory.createConnection();
       SimpleString addressName = 
UUIDGenerator.getInstance().generateSimpleStringUUID();
       logger.debug("Address is {}", addressName);
-      clientSession.createAddress(addressName, RoutingType.ANYCAST, false);
+      clientSession.createAddress(addressName, RoutingType.MULTICAST, false);

Review Comment:
   Change seems reasonable on one hand, its sending to a Topic, as per the test 
name, however...this class is "QueueAutoCreationTest" and this would then be 
using a pre-created Topic address....with no subscribers i.e presumably no 
queues at all? Seems out of place in this class at least.
   
   EDIT: Also, this is an AMQP test class, and the 2 updated tests (and the 3rd 
similar) are only using the Core client? Also seems out of place. Looks like 
the only thing the test uses AMQP for is the 'testSmallString and 
testHugeString'
   
   EDIT2: I see there is a similar test being updated in 
org/apache/activemq/artemis/tests/integration/client/AutoCreateJmsDestinationTest.java
 that has the exact same comment above it, down to referencing 
QueueAutoCreationTest. These may be effective duplicates currently since they 
both use Core?



##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpSecurityTest.java:
##########
@@ -120,7 +120,7 @@ public void inspectOpenedResource(Sender sender) {
 
       try {
          try {
-            session.createSender(getQueueName());
+            session.createSender(getPrefixedQueueName());

Review Comment:
   Perhaps similarly?



##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java:
##########
@@ -143,7 +143,7 @@ public void testAutoCreateOnTopicManySends() throws 
Exception {
       Connection connection = factory.createConnection();
       SimpleString addressName = 
UUIDGenerator.getInstance().generateSimpleStringUUID();
       logger.debug("Address is {}", addressName);
-      clientSession.createAddress(addressName, RoutingType.ANYCAST, false);
+      clientSession.createAddress(addressName, RoutingType.MULTICAST, false);

Review Comment:
   Similar to prev comment
   
   There is a 3rd similar test below with the same mismatch that you didnt 
update and it might make sense to. (EDIT: or not given edits to prev comment..)



##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpAnonymousRelayTest.java:
##########
@@ -285,7 +285,7 @@ public void 
testSendMessageOnAnonymousRelayLinkUsingMessageTo() throws Exception
          AmqpSender sender = session.createAnonymousSender();
          AmqpMessage message = new AmqpMessage();
 
-         message.setAddress(getQueueName());
+         message.setAddress(getPrefixedQueueName());

Review Comment:
   This change shouldnt be needed now right, if the test pre-created the queue 
as I think it does?





Issue Time Tracking
-------------------

    Worklog Id:     (was: 857041)
    Time Spent: 4h  (was: 3h 50m)

> Unexpected Behavior when Routing Type of Destinations Doesn't Match Clients
> ---------------------------------------------------------------------------
>
>                 Key: ARTEMIS-4212
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-4212
>             Project: ActiveMQ Artemis
>          Issue Type: Improvement
>            Reporter: Justin Bertram
>            Assignee: Justin Bertram
>            Priority: Major
>          Time Spent: 4h
>  Remaining Estimate: 0h
>
> When the routing type of an address (and associated queue) does not match the 
> routing type of a client producer, the resultant behavior is a bit unexpected.
> Expected Behavior:
> If a client sends a message to an address / queue with the same name, but a 
> different routing type, the expected behavior would be to throw some sort of 
> InvalidDestinationException (if auto-create is not enabled), or to create the 
> matching address and queue with the appropriate routing type. The routing 
> count on the existing address (with non-matching routing type) should remain 
> unchanged.
> Actual Behavior:
> When sending, for example, to a predefined anycast address and queue from a 
> multiccast (Topic) producer, the routed count on the address is incremented, 
> but the message count on the matching queue is not. No indication is given at 
> the client end that the messages failed to get routed - they are silently 
> dropped.
> This is reproducible using a qpid / proton queue producer to send to a 
> multicast address or using a topic producer to send to an anycast address, 
> e.g.:
> 1. Create a a broker, setting auto-create-queues and auto-create addresses to 
> "false" for the catch-all address-setting
> 2. Start the broker and create a an address and matching queue with a ANYCAST 
> routing type
> 3. Send 1000 messages to the broker using the same queue name but mismatched 
> routing type:
> {code}
> ./artemis producer --url amqp://localhost:61616 --user admin --password admin 
> --destination topic://{QUEUE NAME} --protocol amqp
> {code}
> No error is emitted and the routing count is incremented by 1000 at the 
> address level, but remains unchanged at the destination level.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to