gemmellr commented on code in PR #5410:
URL: https://github.com/apache/activemq-artemis/pull/5410#discussion_r1905356843
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java:
##########
@@ -1501,6 +1501,32 @@ public void testSubscribeToTopicWithNoLocal() throws
Exception {
conn.disconnect();
}
+ @Test
+ public void testSubscribeToTopicWithNoLocalSendWithStomp() throws Exception
{
+ conn.connect(defUser, defPass);
+ subscribeTopic(conn, null, null, null, true, true);
+
+ // send a message on the same connection => it should not be received is
noLocal = true on subscribe
Review Comment:
```suggestion
// send a message on the same connection => it should not be received
as noLocal = true on subscribe
```
(and place c&p from)
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java:
##########
@@ -1501,6 +1501,32 @@ public void testSubscribeToTopicWithNoLocal() throws
Exception {
conn.disconnect();
}
+ @Test
+ public void testSubscribeToTopicWithNoLocalSendWithStomp() throws Exception
{
+ conn.connect(defUser, defPass);
+ subscribeTopic(conn, null, null, null, true, true);
+
+ // send a message on the same connection => it should not be received is
noLocal = true on subscribe
+ send(conn, getTopicPrefix() + getTopicName(), null, "Hello World");
+
+ ClientStompFrame frame = conn.receiveFrame(100);
+ assertNull(frame, "No message should have been received since
subscription was removed");
+
+ // send message on another STOMP connection => it should be received
+ StompClientConnection conn2 =
StompClientConnectionFactory.createClientConnection(uri);
+ conn2.connect(defUser, defPass);
+ // a STOMP message does *not* automatically get a property named
__AMQ_CID
Review Comment:
```suggestion
// a STOMP message does *not* automatically get a property named
__AMQ_CID if there arent any noLocal subscriptions on the connection
```
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java:
##########
@@ -1501,6 +1501,32 @@ public void testSubscribeToTopicWithNoLocal() throws
Exception {
conn.disconnect();
}
+ @Test
+ public void testSubscribeToTopicWithNoLocalSendWithStomp() throws Exception
{
+ conn.connect(defUser, defPass);
+ subscribeTopic(conn, null, null, null, true, true);
+
+ // send a message on the same connection => it should not be received is
noLocal = true on subscribe
+ send(conn, getTopicPrefix() + getTopicName(), null, "Hello World");
+
+ ClientStompFrame frame = conn.receiveFrame(100);
+ assertNull(frame, "No message should have been received since
subscription was removed");
+
+ // send message on another STOMP connection => it should be received
+ StompClientConnection conn2 =
StompClientConnectionFactory.createClientConnection(uri);
+ conn2.connect(defUser, defPass);
+ // a STOMP message does *not* automatically get a property named
__AMQ_CID
+ send(conn2, getTopicPrefix() + getTopicName(), null, getName());
+ frame = conn.receiveFrame(10000);
+ assertNotNull(frame);
+ assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
+ assertEquals(getTopicPrefix() + getTopicName(),
frame.getHeader(Stomp.Headers.Send.DESTINATION));
+ assertEquals(getName(), frame.getBody());
+
+ conn.disconnect();
+ conn2.disconnect();
Review Comment:
Though there is fallback handling for _conn_, there isnt for _conn2_, so the
conn2.disconnect() might not happen if anything fails, might be worth adding a
try-finally, or try-with-resources if simple, or maybe use runAfter (could e.g
extract a utility method from the existing _conn_ fallback cleanup and pass it
_conn2_)
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java:
##########
@@ -1501,6 +1501,32 @@ public void testSubscribeToTopicWithNoLocal() throws
Exception {
conn.disconnect();
}
+ @Test
+ public void testSubscribeToTopicWithNoLocalSendWithStomp() throws Exception
{
+ conn.connect(defUser, defPass);
+ subscribeTopic(conn, null, null, null, true, true);
+
+ // send a message on the same connection => it should not be received is
noLocal = true on subscribe
+ send(conn, getTopicPrefix() + getTopicName(), null, "Hello World");
+
+ ClientStompFrame frame = conn.receiveFrame(100);
+ assertNull(frame, "No message should have been received since
subscription was removed");
Review Comment:
```suggestion
assertNull(frame, "No message should have been received since
subscription specified noLocal");
```
(and place c&p from)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
