[jira] [Commented] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring

2023-07-18 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/ARTEMIS-4366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17744362#comment-17744362
 ] 

ASF subversion and git services commented on ARTEMIS-4366:
--

Commit e5b18b80f79dbb7f07f6b43ef424005ea4667a46 in activemq-artemis's branch 
refs/heads/main from Clebert Suconic
[ https://gitbox.apache.org/repos/asf?p=activemq-artemis.git;h=e5b18b80f7 ]

ARTEMIS-4366 Some adjustments on MirroredSubscriptionTest


> Addresses with multiple subscriptions are not working with Mirroring
> 
>
> Key: ARTEMIS-4366
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4366
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Clebert Suconic
>Priority: Major
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> The NodeStore will be reused by the two Queues on the same address. Each 
> queue should have its own.



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


[jira] [Work logged] (ARTEMIS-4355) Update Сurator to 5.5.0; Zookeeper 3.8.1

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 21:23
Start Date: 18/Jul/23 21:23
Worklog Time Spent: 10m 
  Work Description: amarkevich commented on code in PR #4545:
URL: https://github.com/apache/activemq-artemis/pull/4545#discussion_r1267318459


##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverOneWaySSLTest.java:
##
@@ -85,9 +85,9 @@ public static Collection getParameters() {
  {"SunJCE", "JCEKS", false, false},
  {"SUN", "JKS", false, false},
  {"SunJSSE", "PKCS12", false, false},
- {"JCEKS", null, true, false}, // for compatibility with old 
keyStoreProvider
- {"JKS", null, true, false},   // for compatibility with old 
keyStoreProvider
- {"PKCS12", null, true, false} // for compatibility with old 
keyStoreProvider
+ {"JCEKS", null, false, false}, // for compatibility with old 
keyStoreProvider
+ {"JKS", null, false, false},   // for compatibility with old 
keyStoreProvider
+ {"PKCS12", null, false, false} // for compatibility with old 
keyStoreProvider

Review Comment:
   To pass tests for this PR - otherwise quorum tests are not running. Expected 
changes in first commit only





Issue Time Tracking
---

Worklog Id: (was: 871643)
Time Spent: 1h 50m  (was: 1h 40m)

> Update Сurator to 5.5.0; Zookeeper 3.8.1
> 
>
> Key: ARTEMIS-4355
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4355
> Project: ActiveMQ Artemis
>  Issue Type: Dependency upgrade
>  Components: clustering
>Affects Versions: 2.29.0
>Reporter: Alexey Markevich
>Priority: Minor
>  Time Spent: 1h 50m
>  Remaining Estimate: 0h
>




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


[jira] [Created] (AMQ-9291) networkConnector does not support IPv6 address

2023-07-18 Thread Greg Rabil (Jira)
Greg Rabil created AMQ-9291:
---

 Summary: networkConnector does not support IPv6 address
 Key: AMQ-9291
 URL: https://issues.apache.org/jira/browse/AMQ-9291
 Project: ActiveMQ
  Issue Type: Bug
  Components: Connector
Affects Versions: 5.18.2, 5.17.5, 5.16.6
Reporter: Greg Rabil


In ActiveMQ 5.15.15, the following networkConnector URI works for connecting to 
an ActiveMQ broker over IPv6:



 

However, in ActiveMQ 5.16, 5.17, and 5.18, this will fail with the following 
error (warning):

2023-07-17 12:52:30,825 | WARN  | Failed to connect to 
[ssl://[fd00::15]:61617?verifyHostName=false] after: 1 attempt(s) with Contains 
non-LDH ASCII characters, continuing to retry. | 
org.apache.activemq.transport.failover.FailoverTransport | ActiveMQ Failover 
Worker: 1448780972

 

The problem is caused by this new code in the 
org.apache.activemq.transport.tcp.SslTransport.java class:

{{        // Lets try to configure the SSL SNI field.  Handy in case your 
using}}
{{        // a single proxy to route to different messaging apps.}}
{{        final SSLParameters sslParams = new SSLParameters();}}
{{        if (remoteLocation != null) {}}
{{            sslParams.setServerNames(Collections.singletonList(new 
SNIHostName(remoteLocation.getHost(;}}
{{        }}}

 

The remoteLocation.getHost() will return "[fd00::15]", which causes the 
exception in the SNIHostName constructor.  It seems that the above condition 
should be:

 

{{        if ((remoteLocation != null) && verifyHostName) {}}
{{            sslParams.setServerNames(Collections.singletonList(new 
SNIHostName(remoteLocation.getHost(;}}
{{        }}}

 

Because the SNIHostName only makes sense in the context of verifying the 
hostname of the server certificate.

 



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


[jira] [Work logged] (ARTEMIS-4355) Update Сurator to 5.5.0; Zookeeper 3.8.1

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 20:26
Start Date: 18/Jul/23 20:26
Worklog Time Spent: 10m 
  Work Description: jbertram commented on code in PR #4545:
URL: https://github.com/apache/activemq-artemis/pull/4545#discussion_r1267273788


##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverOneWaySSLTest.java:
##
@@ -85,9 +85,9 @@ public static Collection getParameters() {
  {"SunJCE", "JCEKS", false, false},
  {"SUN", "JKS", false, false},
  {"SunJSSE", "PKCS12", false, false},
- {"JCEKS", null, true, false}, // for compatibility with old 
keyStoreProvider
- {"JKS", null, true, false},   // for compatibility with old 
keyStoreProvider
- {"PKCS12", null, true, false} // for compatibility with old 
keyStoreProvider
+ {"JCEKS", null, false, false}, // for compatibility with old 
keyStoreProvider
+ {"JKS", null, false, false},   // for compatibility with old 
keyStoreProvider
+ {"PKCS12", null, false, false} // for compatibility with old 
keyStoreProvider

Review Comment:
   I don't understand why these changes were necessary. This eliminates the 
tests for the warning log message.





Issue Time Tracking
---

Worklog Id: (was: 871628)
Time Spent: 1h 40m  (was: 1.5h)

> Update Сurator to 5.5.0; Zookeeper 3.8.1
> 
>
> Key: ARTEMIS-4355
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4355
> Project: ActiveMQ Artemis
>  Issue Type: Dependency upgrade
>  Components: clustering
>Affects Versions: 2.29.0
>Reporter: Alexey Markevich
>Priority: Minor
>  Time Spent: 1h 40m
>  Remaining Estimate: 0h
>




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


[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 19:26
Start Date: 18/Jul/23 19:26
Worklog Time Spent: 10m 
  Work Description: jbertram commented on code in PR #4556:
URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1267222798


##
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java:
##
@@ -404,12 +406,15 @@ private boolean publishToClient(int messageId, 
ICoreMessage message, int deliver
   // [MQTT-3.3.1-2] The DUP flag MUST be set to 0 for all QoS 0 messages.
   boolean redelivery = qos == 0 ? false : (deliveryCount > 1);
 
-  boolean isRetain = message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY);
+  boolean isRetain = 
message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY);
   MqttProperties mqttProperties = getPublishProperties(message);
 
   if (session.getVersion() == MQTTVersion.MQTT_5) {
- if (session.getState().getSubscription(message.getAddress()) != null 
&& 
!session.getState().getSubscription(message.getAddress()).option().isRetainAsPublished())
 {
-isRetain = false;
+ if (message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY) && 
!message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY)) {

Review Comment:
   I considered doing that, but it didn't seem as clear as I wanted it to be at 
the time. Taking another look I think it's fine.





Issue Time Tracking
---

Worklog Id: (was: 871616)
Time Spent: 1.5h  (was: 1h 20m)

> MQTT retain flag not set correctly
> --
>
> Key: ARTEMIS-4365
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4365
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Justin Bertram
>Assignee: Justin Bertram
>Priority: Major
>  Time Spent: 1.5h
>  Remaining Estimate: 0h
>
> The {{retain}} flag set on MQTT messages dispatched to clients is incorrect 
> in certain circumstances. This is demonstrated by using the {{test}} command 
> from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.:
> {noformat}
> $ mqtt test --all
> MQTT 3: OK
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 4847.01ms
> - QoS 1: Received 10/10 publishes in 27413.45ms
> - QoS 2: Received 10/10 publishes in 49551.40ms
> - Retain: OK
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED
> MQTT 5: OK
> - Connect restrictions: 
> > Retain: OK
> > Wildcard subscriptions: OK
> > Shared subscriptions: OK
> > Subscription identifiers: OK
> > Maximum QoS: 2
> > Receive maximum: 65535
> > Maximum packet size: 268435455 bytes
> > Topic alias maximum: 65535
> > Session expiry interval: Client-based
> > Server keep alive: Client-based
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 706.21ms
> - QoS 1: Received 10/10 publishes in 805.38ms
> - QoS 2: Received 10/10 publishes in 972.98ms
> - Retain: TIME_OUT
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED{noformat}
> Notice the result of {{TIME_OUT}} when testing retain functionality for MQTT 
> 5.



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


[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 17:43
Start Date: 18/Jul/23 17:43
Worklog Time Spent: 10m 
  Work Description: gemmellr commented on code in PR #4556:
URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1267114894


##
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java:
##
@@ -404,12 +406,15 @@ private boolean publishToClient(int messageId, 
ICoreMessage message, int deliver
   // [MQTT-3.3.1-2] The DUP flag MUST be set to 0 for all QoS 0 messages.
   boolean redelivery = qos == 0 ? false : (deliveryCount > 1);
 
-  boolean isRetain = message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY);
+  boolean isRetain = 
message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY);
   MqttProperties mqttProperties = getPublishProperties(message);
 
   if (session.getVersion() == MQTTVersion.MQTT_5) {
- if (session.getState().getSubscription(message.getAddress()) != null 
&& 
!session.getState().getSubscription(message.getAddress()).option().isRetainAsPublished())
 {
-isRetain = false;
+ if (message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY) && 
!message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY)) {

Review Comment:
   Since we already know the presence result from the earlier check, and only 
need to do the rest if that was false, what about:
   
   ```suggestion
if (!isRetain && 
message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY)) {
   ```





Issue Time Tracking
---

Worklog Id: (was: 871602)
Time Spent: 1h 20m  (was: 1h 10m)

> MQTT retain flag not set correctly
> --
>
> Key: ARTEMIS-4365
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4365
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Justin Bertram
>Assignee: Justin Bertram
>Priority: Major
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> The {{retain}} flag set on MQTT messages dispatched to clients is incorrect 
> in certain circumstances. This is demonstrated by using the {{test}} command 
> from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.:
> {noformat}
> $ mqtt test --all
> MQTT 3: OK
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 4847.01ms
> - QoS 1: Received 10/10 publishes in 27413.45ms
> - QoS 2: Received 10/10 publishes in 49551.40ms
> - Retain: OK
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED
> MQTT 5: OK
> - Connect restrictions: 
> > Retain: OK
> > Wildcard subscriptions: OK
> > Shared subscriptions: OK
> > Subscription identifiers: OK
> > Maximum QoS: 2
> > Receive maximum: 65535
> > Maximum packet size: 268435455 bytes
> > Topic alias maximum: 65535
> > Session expiry interval: Client-based
> > Server keep alive: Client-based
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 706.21ms
> - QoS 1: Received 10/10 publishes in 805.38ms
> - QoS 2: Received 10/10 publishes in 972.98ms
> - Retain: TIME_OUT
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED{noformat}
> Notice the result of {{TIME_OUT}} when testing retain functionality for MQTT 
> 5.



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


[jira] [Commented] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring

2023-07-18 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/ARTEMIS-4366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17744303#comment-17744303
 ] 

ASF subversion and git services commented on ARTEMIS-4366:
--

Commit 29deb30c738ea765fbed738aa3e36517c3c89ca4 in activemq-artemis's branch 
refs/heads/main from Clebert Suconic
[ https://gitbox.apache.org/repos/asf?p=activemq-artemis.git;h=29deb30c73 ]

ARTEMIS-4366 Some Adjustment to class names on Mirror

This is in relation to comments from PR #4555


> Addresses with multiple subscriptions are not working with Mirroring
> 
>
> Key: ARTEMIS-4366
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4366
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Clebert Suconic
>Priority: Major
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> The NodeStore will be reused by the two Queues on the same address. Each 
> queue should have its own.



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


[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 17:11
Start Date: 18/Jul/23 17:11
Worklog Time Spent: 10m 
  Work Description: clebertsuconic merged PR #4557:
URL: https://github.com/apache/activemq-artemis/pull/4557




Issue Time Tracking
---

Worklog Id: (was: 871599)
Time Spent: 1h 20m  (was: 1h 10m)

> Addresses with multiple subscriptions are not working with Mirroring
> 
>
> Key: ARTEMIS-4366
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4366
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Clebert Suconic
>Priority: Major
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> The NodeStore will be reused by the two Queues on the same address. Each 
> queue should have its own.



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


[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 17:11
Start Date: 18/Jul/23 17:11
Worklog Time Spent: 10m 
  Work Description: jbertram commented on PR #4556:
URL: 
https://github.com/apache/activemq-artemis/pull/4556#issuecomment-1640635646

   I fixed the failure coming from 
`MQTTInterceptorPropertiesTest.checkMessageProperties`. The failure was due to 
the new (correct) semantics for the `retain` flag.




Issue Time Tracking
---

Worklog Id: (was: 871598)
Time Spent: 1h 10m  (was: 1h)

> MQTT retain flag not set correctly
> --
>
> Key: ARTEMIS-4365
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4365
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Justin Bertram
>Assignee: Justin Bertram
>Priority: Major
>  Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> The {{retain}} flag set on MQTT messages dispatched to clients is incorrect 
> in certain circumstances. This is demonstrated by using the {{test}} command 
> from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.:
> {noformat}
> $ mqtt test --all
> MQTT 3: OK
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 4847.01ms
> - QoS 1: Received 10/10 publishes in 27413.45ms
> - QoS 2: Received 10/10 publishes in 49551.40ms
> - Retain: OK
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED
> MQTT 5: OK
> - Connect restrictions: 
> > Retain: OK
> > Wildcard subscriptions: OK
> > Shared subscriptions: OK
> > Subscription identifiers: OK
> > Maximum QoS: 2
> > Receive maximum: 65535
> > Maximum packet size: 268435455 bytes
> > Topic alias maximum: 65535
> > Session expiry interval: Client-based
> > Server keep alive: Client-based
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 706.21ms
> - QoS 1: Received 10/10 publishes in 805.38ms
> - QoS 2: Received 10/10 publishes in 972.98ms
> - Retain: TIME_OUT
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED{noformat}
> Notice the result of {{TIME_OUT}} when testing retain functionality for MQTT 
> 5.



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


[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 17:10
Start Date: 18/Jul/23 17:10
Worklog Time Spent: 10m 
  Work Description: jbertram commented on code in PR #4556:
URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1267082367


##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java:
##
@@ -2219,4 +2223,71 @@ public void testAutoDeleteRetainedQueue() throws 
Exception {
   Wait.assertTrue(() -> 
server.locateQueue(RETAINED_QUEUE).getMessageCount() == 0, 3000, 50);
   Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE) == null, 3000, 
50);
}
+
+   /*
+* [MQTT-3.3.1-9] When sending a PUBLISH Packet to a Client the 
Server...MUST set the RETAIN flag to 0 when a PUBLISH
+* Packet is sent to a Client because it matches an *established* 
subscription regardless of how the flag was set in
+* the message it received.
+*/
+   @Test(timeout = 60 * 1000)
+   public void testRetainFlagOnEstablishedSubscription() throws Exception {
+  CountDownLatch latch = new CountDownLatch(1);
+  final String topic = RandomUtil.randomString();
+
+  MqttClient subscriber = createPaho3_1_1Client("subscriber");
+  subscriber.connect();
+  subscriber.subscribe(topic, 1);
+  subscriber.setCallback(new DefaultMqtt3Callback() {
+ @Override
+ public void messageArrived(String topic, MqttMessage message) throws 
Exception {
+if (!message.isRetained()) {
+   latch.countDown();
+}
+ }
+  });
+
+  MqttClient publisher = createPaho3_1_1Client("publisher");
+  publisher.connect();
+  publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, 
true);
+  publisher.disconnect();
+  publisher.close();
+
+  assertTrue(latch.await(1000, TimeUnit.MILLISECONDS));
+
+  subscriber.disconnect();
+  subscriber.close();
+   }
+
+   /*
+* [MQTT-3.3.1-8] When sending a PUBLISH Packet to a Client the Server MUST 
set the RETAIN flag to 1 if a message is
+* sent as a result of a new subscription being made by a Client.
+*/
+   @Test(timeout = 60 * 1000)
+   public void testRetainFlagOnNewSubscription() throws Exception {
+  CountDownLatch latch = new CountDownLatch(1);
+  final String topic = RandomUtil.randomString();
+
+  MqttClient publisher = createPaho3_1_1Client("publisher");
+  publisher.connect();
+  publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, 
true);
+  publisher.disconnect();
+  publisher.close();
+
+  MqttClient subscriber = createPaho3_1_1Client("subscriber");
+  subscriber.connect();
+  subscriber.subscribe(topic, 1);
+  subscriber.setCallback(new DefaultMqtt3Callback() {
+ @Override
+ public void messageArrived(String topic, MqttMessage message) throws 
Exception {
+if (message.isRetained()) {
+   latch.countDown();
+}
+ }
+  });
+
+  assertTrue(latch.await(1000, TimeUnit.MILLISECONDS));

Review Comment:
   I also added messages to the relevant assertions.





Issue Time Tracking
---

Worklog Id: (was: 871597)
Time Spent: 1h  (was: 50m)

> MQTT retain flag not set correctly
> --
>
> Key: ARTEMIS-4365
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4365
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Justin Bertram
>Assignee: Justin Bertram
>Priority: Major
>  Time Spent: 1h
>  Remaining Estimate: 0h
>
> The {{retain}} flag set on MQTT messages dispatched to clients is incorrect 
> in certain circumstances. This is demonstrated by using the {{test}} command 
> from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.:
> {noformat}
> $ mqtt test --all
> MQTT 3: OK
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 4847.01ms
> - QoS 1: Received 10/10 publishes in 27413.45ms
> - QoS 2: Received 10/10 publishes in 49551.40ms
> - Retain: OK
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED
> MQTT 5: OK
> - Connect restrictions: 
> > Retain: OK
> > Wildcard subscriptions: OK
> > Shared subscriptions: OK
> > Subscription identifiers: OK
> > Maximum QoS: 2
> > Receive maximum: 65535
> > Maximum 

[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 17:07
Start Date: 18/Jul/23 17:07
Worklog Time Spent: 10m 
  Work Description: jbertram commented on code in PR #4556:
URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1267078926


##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java:
##
@@ -2219,4 +2223,71 @@ public void testAutoDeleteRetainedQueue() throws 
Exception {
   Wait.assertTrue(() -> 
server.locateQueue(RETAINED_QUEUE).getMessageCount() == 0, 3000, 50);
   Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE) == null, 3000, 
50);
}
+
+   /*
+* [MQTT-3.3.1-9] When sending a PUBLISH Packet to a Client the 
Server...MUST set the RETAIN flag to 0 when a PUBLISH
+* Packet is sent to a Client because it matches an *established* 
subscription regardless of how the flag was set in
+* the message it received.
+*/
+   @Test(timeout = 60 * 1000)
+   public void testRetainFlagOnEstablishedSubscription() throws Exception {
+  CountDownLatch latch = new CountDownLatch(1);
+  final String topic = RandomUtil.randomString();
+
+  MqttClient subscriber = createPaho3_1_1Client("subscriber");
+  subscriber.connect();
+  subscriber.subscribe(topic, 1);
+  subscriber.setCallback(new DefaultMqtt3Callback() {
+ @Override
+ public void messageArrived(String topic, MqttMessage message) throws 
Exception {
+if (!message.isRetained()) {
+   latch.countDown();
+}
+ }
+  });
+
+  MqttClient publisher = createPaho3_1_1Client("publisher");
+  publisher.connect();
+  publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, 
true);
+  publisher.disconnect();
+  publisher.close();
+
+  assertTrue(latch.await(1000, TimeUnit.MILLISECONDS));
+
+  subscriber.disconnect();
+  subscriber.close();
+   }
+
+   /*
+* [MQTT-3.3.1-8] When sending a PUBLISH Packet to a Client the Server MUST 
set the RETAIN flag to 1 if a message is
+* sent as a result of a new subscription being made by a Client.
+*/
+   @Test(timeout = 60 * 1000)
+   public void testRetainFlagOnNewSubscription() throws Exception {
+  CountDownLatch latch = new CountDownLatch(1);
+  final String topic = RandomUtil.randomString();
+
+  MqttClient publisher = createPaho3_1_1Client("publisher");
+  publisher.connect();
+  publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, 
true);
+  publisher.disconnect();
+  publisher.close();
+
+  MqttClient subscriber = createPaho3_1_1Client("subscriber");
+  subscriber.connect();
+  subscriber.subscribe(topic, 1);
+  subscriber.setCallback(new DefaultMqtt3Callback() {
+ @Override
+ public void messageArrived(String topic, MqttMessage message) throws 
Exception {
+if (message.isRetained()) {
+   latch.countDown();
+}
+ }
+  });
+
+  assertTrue(latch.await(1000, TimeUnit.MILLISECONDS));

Review Comment:
   I fixed a race condition between the `subsribe` and `setCallback`. I can't 
get it to fail anymore.





Issue Time Tracking
---

Worklog Id: (was: 871596)
Time Spent: 50m  (was: 40m)

> MQTT retain flag not set correctly
> --
>
> Key: ARTEMIS-4365
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4365
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Justin Bertram
>Assignee: Justin Bertram
>Priority: Major
>  Time Spent: 50m
>  Remaining Estimate: 0h
>
> The {{retain}} flag set on MQTT messages dispatched to clients is incorrect 
> in certain circumstances. This is demonstrated by using the {{test}} command 
> from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.:
> {noformat}
> $ mqtt test --all
> MQTT 3: OK
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 4847.01ms
> - QoS 1: Received 10/10 publishes in 27413.45ms
> - QoS 2: Received 10/10 publishes in 49551.40ms
> - Retain: OK
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED
> MQTT 5: OK
> - Connect restrictions: 
> > Retain: OK
> > Wildcard subscriptions: OK
> > Shared subscriptions: OK
> > Subscription identifiers: OK
> > Maximum QoS: 2
>

[jira] [Commented] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger

2023-07-18 Thread Robbie Gemmell (Jira)


[ 
https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17744275#comment-17744275
 ] 

Robbie Gemmell commented on ARTEMIS-4367:
-

An approach I've seen used elsewhere is having such 'operational messages' 
given individual loggers in a hierarchy. That way they can all be configured 
'as a whole' typically (often not configured specifically at all, just using 
the parent or root level logger config)...but can also still be configured very 
granularly if needed (i.e per message) as they also had their own log category. 
In this case each existing 'IDd message' has a unique code, and currently they 
all use a specific logger category, e.g 'the client logger' or 'the server 
logger' or 'journal logger'. Since all the logging stuff for these is 
generated, they could easily use a sub-category generated based on the code. I 
guess one consideration would be, how many of them are there, and would that be 
an issue. On the other side of that though, consider that outside of these 
specific 'IDd logger classes', every single implementation class with any 
logging has its own logger.

> Split server logger codes into advice and info category and by logger
> -
>
> Key: ARTEMIS-4367
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4367
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Affects Versions: 2.29.0
>Reporter: Gary Tully
>Priority: Major
>
> We have server logger that uses AMQXXX codes for log messages. all is good.
> We have a class of log message that is advice...
>  * address X does not a DLQ registered
>  * address X does not an expiry address registered
> These advice are info or warn trying to help users but often times the config 
> is by design and this advice is just noise. Users want to disable, but not 
> all of the server logger infor or warn messages.
> log4j allows to filter, but that is expensive.
> If we split out these advice messages into a separate class and use a nested 
> logger per code, it would be simple to suppress advice that is noise for a 
> given scenario
> There was an new advice in ARTEMIS-4362 that suggests we do have a class of 
> advices/warns that would benefit from a simple way to suppress.



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


[jira] [Updated] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger

2023-07-18 Thread Gary Tully (Jira)


 [ 
https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gary Tully updated ARTEMIS-4367:

Description: 
We have server logger that uses AMQXXX codes for log messages. all is good.

We have a class of log message that is advice...
 * address X does not a DLQ registered
 * address X does not an expiry address registered

These advice are info or warn trying to help users but often times the config 
is by design and this advice is just noise. Users want to disable, but not all 
of the server logger infor or warn messages.

log4j allows to filter, but that is expensive.

If we split out these advice messages into a separate class and use a nested 
logger per code, it would be simple to suppress advice that is noise for a 
given scenario

There was an new advice in ARTEMIS-4362 that suggests we do have a class of 
advices/warns that would benefit from a simple way to suppress.

  was:
We have server logger that uses AMQXXX codes for log messages. all is good.

We have a class of log message that is advice...
 * address X does not a DLQ registered
 * address X does not an expiry address registered

These advice are info or warn trying to help users but often times the config 
is by design and this advice is just noise. Users want to disable, but not all 
of the server logger infor or warn messages.

log4j allows to filter, but that is expensive.

If we split out these advice messages into a separate class and use a nested 
logger per code, it would be simple to suppress advice that is noise for a 
given scenario

There was an new advice in AMQ-6854 that suggests we do have a class of 
advices/warns that would benefit from a simple way to suppress.


> Split server logger codes into advice and info category and by logger
> -
>
> Key: ARTEMIS-4367
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4367
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Affects Versions: 2.29.0
>Reporter: Gary Tully
>Priority: Major
>
> We have server logger that uses AMQXXX codes for log messages. all is good.
> We have a class of log message that is advice...
>  * address X does not a DLQ registered
>  * address X does not an expiry address registered
> These advice are info or warn trying to help users but often times the config 
> is by design and this advice is just noise. Users want to disable, but not 
> all of the server logger infor or warn messages.
> log4j allows to filter, but that is expensive.
> If we split out these advice messages into a separate class and use a nested 
> logger per code, it would be simple to suppress advice that is noise for a 
> given scenario
> There was an new advice in ARTEMIS-4362 that suggests we do have a class of 
> advices/warns that would benefit from a simple way to suppress.



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


[jira] [Commented] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger

2023-07-18 Thread Gary Tully (Jira)


[ 
https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17744266#comment-17744266
 ] 

Gary Tully commented on ARTEMIS-4367:
-

an example of filtering from [~jbertram] 

# Console appender
appender.console.type=Console
appender.console.name=console
appender.console.layout.type=PatternLayout
appender.console.layout.pattern=%d %-5level [%logger] %msg%n
appender.console.filter.no_dla_or_expiry_warns.type=RegexFilter
appender.console.filter.no_dla_or_expiry_warns.regex=.*AMQ222165.*|.*AMQ222166.*
appender.console.filter.no_dla_or_expiry_warns.onMatch=DENY
appender.console.filter.no_dla_or_expiry_warns.onMismatch=ACCEPT

> Split server logger codes into advice and info category and by logger
> -
>
> Key: ARTEMIS-4367
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4367
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>  Components: Broker
>Affects Versions: 2.29.0
>Reporter: Gary Tully
>Priority: Major
>
> We have server logger that uses AMQXXX codes for log messages. all is good.
> We have a class of log message that is advice...
>  * address X does not a DLQ registered
>  * address X does not an expiry address registered
> These advice are info or warn trying to help users but often times the config 
> is by design and this advice is just noise. Users want to disable, but not 
> all of the server logger infor or warn messages.
> log4j allows to filter, but that is expensive.
> If we split out these advice messages into a separate class and use a nested 
> logger per code, it would be simple to suppress advice that is noise for a 
> given scenario
> There was an new advice in AMQ-6854 that suggests we do have a class of 
> advices/warns that would benefit from a simple way to suppress.



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


[jira] [Updated] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger

2023-07-18 Thread Gary Tully (Jira)


 [ 
https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gary Tully updated ARTEMIS-4367:

Issue Type: Improvement  (was: Bug)

> Split server logger codes into advice and info category and by logger
> -
>
> Key: ARTEMIS-4367
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4367
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Affects Versions: 2.29.0
>Reporter: Gary Tully
>Priority: Major
>
> We have server logger that uses AMQXXX codes for log messages. all is good.
> We have a class of log message that is advice...
>  * address X does not a DLQ registered
>  * address X does not an expiry address registered
> These advice are info or warn trying to help users but often times the config 
> is by design and this advice is just noise. Users want to disable, but not 
> all of the server logger infor or warn messages.
> log4j allows to filter, but that is expensive.
> If we split out these advice messages into a separate class and use a nested 
> logger per code, it would be simple to suppress advice that is noise for a 
> given scenario
> There was an new advice in AMQ-6854 that suggests we do have a class of 
> advices/warns that would benefit from a simple way to suppress.



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


[jira] [Updated] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger

2023-07-18 Thread Gary Tully (Jira)


 [ 
https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gary Tully updated ARTEMIS-4367:

Summary: Split server logger codes into advice and info category and by 
logger  (was: Split server logger codes into advice and info and by logger)

> Split server logger codes into advice and info category and by logger
> -
>
> Key: ARTEMIS-4367
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4367
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>  Components: Broker
>Affects Versions: 2.29.0
>Reporter: Gary Tully
>Priority: Major
>
> We have server logger that uses AMQXXX codes for log messages. all is good.
> We have a class of log message that is advice...
>  * address X does not a DLQ registered
>  * address X does not an expiry address registered
> These advice are info or warn trying to help users but often times the config 
> is by design and this advice is just noise. Users want to disable, but not 
> all of the server logger infor or warn messages.
> log4j allows to filter, but that is expensive.
> If we split out these advice messages into a separate class and use a nested 
> logger per code, it would be simple to suppress advice that is noise for a 
> given scenario
> There was an new advice in AMQ-6854 that suggests we do have a class of 
> advices/warns that would benefit from a simple way to suppress.



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


[jira] [Created] (ARTEMIS-4367) Split server logger codes into advice and info and by logger

2023-07-18 Thread Gary Tully (Jira)
Gary Tully created ARTEMIS-4367:
---

 Summary: Split server logger codes into advice and info and by 
logger
 Key: ARTEMIS-4367
 URL: https://issues.apache.org/jira/browse/ARTEMIS-4367
 Project: ActiveMQ Artemis
  Issue Type: Bug
  Components: Broker
Affects Versions: 2.29.0
Reporter: Gary Tully


We have server logger that uses AMQXXX codes for log messages. all is good.

We have a class of log message that is advice...
 * address X does not a DLQ registered
 * address X does not an expiry address registered

These advice are info or warn trying to help users but often times the config 
is by design and this advice is just noise. Users want to disable, but not all 
of the server logger infor or warn messages.

log4j allows to filter, but that is expensive.

If we split out these advice messages into a separate class and use a nested 
logger per code, it would be simple to suppress advice that is noise for a 
given scenario

There was an new advice in AMQ-6854 that suggests we do have a class of 
advices/warns that would benefit from a simple way to suppress.



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


[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 13:51
Start Date: 18/Jul/23 13:51
Worklog Time Spent: 10m 
  Work Description: gemmellr commented on code in PR #4555:
URL: https://github.com/apache/activemq-artemis/pull/4555#discussion_r1266809419


##
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/ProtonProtocolManager.java:
##
@@ -125,11 +125,11 @@ public ProtonProtocolManager(ProtonProtocolManagerFactory 
factory, ActiveMQServe
   routingHandler = new AMQPRoutingHandler(server);
}
 
-   public synchronized ReferenceNodeStore getReferenceIDSupplier() {
+   public synchronized ReferenceNodeStoreFactory getReferenceIDSupplier() {

Review Comment:
   Think the only one that wouldnt go static is getDefaultNodeID() , which 
didnt originally exist as the caller already had the value (serverId).
   
   Renaming would be good...the current naming just doesnt fit, makes it 
confusing to read.





Issue Time Tracking
---

Worklog Id: (was: 871555)
Time Spent: 1h 10m  (was: 1h)

> Addresses with multiple subscriptions are not working with Mirroring
> 
>
> Key: ARTEMIS-4366
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4366
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Clebert Suconic
>Priority: Major
>  Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> The NodeStore will be reused by the two Queues on the same address. Each 
> queue should have its own.



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


[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 13:31
Start Date: 18/Jul/23 13:31
Worklog Time Spent: 10m 
  Work Description: clebertsuconic opened a new pull request, #4557:
URL: https://github.com/apache/activemq-artemis/pull/4557

   This is in relation to comments from PR #4555




Issue Time Tracking
---

Worklog Id: (was: 871551)
Time Spent: 1h  (was: 50m)

> Addresses with multiple subscriptions are not working with Mirroring
> 
>
> Key: ARTEMIS-4366
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4366
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Clebert Suconic
>Priority: Major
>  Time Spent: 1h
>  Remaining Estimate: 0h
>
> The NodeStore will be reused by the two Queues on the same address. Each 
> queue should have its own.



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


[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 13:02
Start Date: 18/Jul/23 13:02
Worklog Time Spent: 10m 
  Work Description: clebertsuconic commented on code in PR #4555:
URL: https://github.com/apache/activemq-artemis/pull/4555#discussion_r1266740736


##
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/ProtonProtocolManager.java:
##
@@ -125,11 +125,11 @@ public ProtonProtocolManager(ProtonProtocolManagerFactory 
factory, ActiveMQServe
   routingHandler = new AMQPRoutingHandler(server);
}
 
-   public synchronized ReferenceNodeStore getReferenceIDSupplier() {
+   public synchronized ReferenceNodeStoreFactory getReferenceIDSupplier() {

Review Comment:
   It still the ID supplier.
   That's a reason why I actually didn't make those methods static. although 
one of them can't be made static.
   
   I can rename this as IDSupplier. it would implement the NodeStoreFactory and 
return a new instance of NodeStore.





Issue Time Tracking
---

Worklog Id: (was: 871547)
Time Spent: 50m  (was: 40m)

> Addresses with multiple subscriptions are not working with Mirroring
> 
>
> Key: ARTEMIS-4366
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4366
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Clebert Suconic
>Priority: Major
>  Time Spent: 50m
>  Remaining Estimate: 0h
>
> The NodeStore will be reused by the two Queues on the same address. Each 
> queue should have its own.



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


[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 12:33
Start Date: 18/Jul/23 12:33
Worklog Time Spent: 10m 
  Work Description: gemmellr commented on PR #4556:
URL: 
https://github.com/apache/activemq-artemis/pull/4556#issuecomment-1640128430

   Also saw another test failure in a CI run, in addition to the one added in 
the code.
   
   > java.lang.AssertionError: expected:\ but was:\
at 
org.apache.activemq.artemis.tests.integration.mqtt.MQTTInterceptorPropertiesTest.checkMessageProperties(MQTTInterceptorPropertiesTest.java:46)
at 
org.apache.activemq.artemis.tests.integration.mqtt.MQTTInterceptorPropertiesTest.lambda$testCheckInterceptedMQTTMessageProperties$1(MQTTInterceptorPropertiesTest.java:84)




Issue Time Tracking
---

Worklog Id: (was: 871539)
Time Spent: 40m  (was: 0.5h)

> MQTT retain flag not set correctly
> --
>
> Key: ARTEMIS-4365
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4365
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Justin Bertram
>Assignee: Justin Bertram
>Priority: Major
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> The {{retain}} flag set on MQTT messages dispatched to clients is incorrect 
> in certain circumstances. This is demonstrated by using the {{test}} command 
> from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.:
> {noformat}
> $ mqtt test --all
> MQTT 3: OK
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 4847.01ms
> - QoS 1: Received 10/10 publishes in 27413.45ms
> - QoS 2: Received 10/10 publishes in 49551.40ms
> - Retain: OK
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED
> MQTT 5: OK
> - Connect restrictions: 
> > Retain: OK
> > Wildcard subscriptions: OK
> > Shared subscriptions: OK
> > Subscription identifiers: OK
> > Maximum QoS: 2
> > Receive maximum: 65535
> > Maximum packet size: 268435455 bytes
> > Topic alias maximum: 65535
> > Session expiry interval: Client-based
> > Server keep alive: Client-based
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 706.21ms
> - QoS 1: Received 10/10 publishes in 805.38ms
> - QoS 2: Received 10/10 publishes in 972.98ms
> - Retain: TIME_OUT
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED{noformat}
> Notice the result of {{TIME_OUT}} when testing retain functionality for MQTT 
> 5.



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


[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 12:31
Start Date: 18/Jul/23 12:31
Worklog Time Spent: 10m 
  Work Description: gemmellr commented on code in PR #4556:
URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1266701120


##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java:
##
@@ -2219,4 +2223,71 @@ public void testAutoDeleteRetainedQueue() throws 
Exception {
   Wait.assertTrue(() -> 
server.locateQueue(RETAINED_QUEUE).getMessageCount() == 0, 3000, 50);
   Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE) == null, 3000, 
50);
}
+
+   /*
+* [MQTT-3.3.1-9] When sending a PUBLISH Packet to a Client the 
Server...MUST set the RETAIN flag to 0 when a PUBLISH
+* Packet is sent to a Client because it matches an *established* 
subscription regardless of how the flag was set in
+* the message it received.
+*/
+   @Test(timeout = 60 * 1000)
+   public void testRetainFlagOnEstablishedSubscription() throws Exception {
+  CountDownLatch latch = new CountDownLatch(1);
+  final String topic = RandomUtil.randomString();
+
+  MqttClient subscriber = createPaho3_1_1Client("subscriber");
+  subscriber.connect();
+  subscriber.subscribe(topic, 1);
+  subscriber.setCallback(new DefaultMqtt3Callback() {
+ @Override
+ public void messageArrived(String topic, MqttMessage message) throws 
Exception {
+if (!message.isRetained()) {
+   latch.countDown();
+}
+ }
+  });
+
+  MqttClient publisher = createPaho3_1_1Client("publisher");
+  publisher.connect();
+  publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, 
true);
+  publisher.disconnect();
+  publisher.close();
+
+  assertTrue(latch.await(1000, TimeUnit.MILLISECONDS));
+
+  subscriber.disconnect();
+  subscriber.close();
+   }
+
+   /*
+* [MQTT-3.3.1-8] When sending a PUBLISH Packet to a Client the Server MUST 
set the RETAIN flag to 1 if a message is
+* sent as a result of a new subscription being made by a Client.
+*/
+   @Test(timeout = 60 * 1000)
+   public void testRetainFlagOnNewSubscription() throws Exception {
+  CountDownLatch latch = new CountDownLatch(1);
+  final String topic = RandomUtil.randomString();
+
+  MqttClient publisher = createPaho3_1_1Client("publisher");
+  publisher.connect();
+  publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, 
true);
+  publisher.disconnect();
+  publisher.close();
+
+  MqttClient subscriber = createPaho3_1_1Client("subscriber");
+  subscriber.connect();
+  subscriber.subscribe(topic, 1);
+  subscriber.setCallback(new DefaultMqtt3Callback() {
+ @Override
+ public void messageArrived(String topic, MqttMessage message) throws 
Exception {
+if (message.isRetained()) {
+   latch.countDown();
+}
+ }
+  });
+
+  assertTrue(latch.await(1000, TimeUnit.MILLISECONDS));

Review Comment:
   I saw this test fail here on a CI run.
   
   Adding a failure message would be good. Its annoying having to go and match 
up line numbers in remote repositories to see what the failure actually meant.





Issue Time Tracking
---

Worklog Id: (was: 871538)
Time Spent: 0.5h  (was: 20m)

> MQTT retain flag not set correctly
> --
>
> Key: ARTEMIS-4365
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4365
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Justin Bertram
>Assignee: Justin Bertram
>Priority: Major
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> The {{retain}} flag set on MQTT messages dispatched to clients is incorrect 
> in certain circumstances. This is demonstrated by using the {{test}} command 
> from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.:
> {noformat}
> $ mqtt test --all
> MQTT 3: OK
> - Maximum topic length: 65535 bytes
> - QoS 0: Received 10/10 publishes in 4847.01ms
> - QoS 1: Received 10/10 publishes in 27413.45ms
> - QoS 2: Received 10/10 publishes in 49551.40ms
> - Retain: OK
> - Wildcard subscriptions: OK
> - Shared subscriptions: OK
> - Payload size: >= 10 bytes
> - Maximum client id length: 65535 bytes
> - Unsupported Ascii Chars: ALL SUPPORTED
> MQTT 5: OK
> - Connect restrictions: 
> > Retain: OK
> > Wildcard subscriptions: OK
> 

[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 11:25
Start Date: 18/Jul/23 11:25
Worklog Time Spent: 10m 
  Work Description: gemmellr commented on code in PR #4556:
URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1266568647


##
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java:
##
@@ -404,12 +406,17 @@ private boolean publishToClient(int messageId, 
ICoreMessage message, int deliver
   // [MQTT-3.3.1-2] The DUP flag MUST be set to 0 for all QoS 0 messages.
   boolean redelivery = qos == 0 ? false : (deliveryCount > 1);
 
-  boolean isRetain = message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY);
+  boolean isRetain = false;

Review Comment:
   If it is primarily going to gate on 
MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY presence for both protocols as 
this does now, then could you not check for it once here and update isRetain 
based on it? Seems like it would be simpler than doing it 3 times below. Would 
allow dropping a couple else/else-if segments, leaving jsut the MQTT5-specific 
check on the MQTT_MESSAGE_RETAIN_KEY if it wasnt already true.



##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java:
##
@@ -512,7 +540,8 @@ public void testRetainHandlingTwo() throws Exception {
   MqttClient consumer = createPahoClient(CONSUMER_ID);
   consumer.setCallback(new DefaultMqttCallback() {
  @Override
- public void messageArrived(String topic, MqttMessage message) throws 
Exception {
+ public void messageArrived(String topic, MqttMessage message) {
+assertTrue(message.isRetained());

Review Comment:
   Seems superfluous when the test is already burning 2 seconds (ouch) to 
verify this method isn't called.
   
   Though if it wasnt, also seems odd the assertion is the opposite of the 
previous test when it expected not to get the message.



##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java:
##
@@ -481,7 +503,8 @@ public void messageArrived(String topic, MqttMessage 
message) throws Exception {
   final CountDownLatch latch2 = new CountDownLatch(1);
   consumer.setCallback(new DefaultMqttCallback() {
  @Override
- public void messageArrived(String topic, MqttMessage message) throws 
Exception {
+ public void messageArrived(String topic, MqttMessage message) {
+assertFalse(message.isRetained());

Review Comment:
   Seems superfluous when the test is already burning 2 seconds (ouch) to 
verify this method isn't called.



##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java:
##
@@ -2219,4 +2223,71 @@ public void testAutoDeleteRetainedQueue() throws 
Exception {
   Wait.assertTrue(() -> 
server.locateQueue(RETAINED_QUEUE).getMessageCount() == 0, 3000, 50);
   Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE) == null, 3000, 
50);
}
+
+   /*
+* [MQTT-3.3.1-9] When sending a PUBLISH Packet to a Client the 
Server...MUST set the RETAIN flag to 0 when a PUBLISH
+* Packet is sent to a Client because it matches an *established* 
subscription regardless of how the flag was set in
+* the message it received.
+*/
+   @Test(timeout = 60 * 1000)
+   public void testRetainFlagOnEstablishedSubscription() throws Exception {
+  CountDownLatch latch = new CountDownLatch(1);
+  final String topic = RandomUtil.randomString();

Review Comment:
   Can be nice to name things based on the test to help easily see later it was 
for etc, useful for debugging.
   
   E.g use, or add a prefix via, getTopicName() / getQueueName() / getName() 
helpers.
   
   Same for next test.





Issue Time Tracking
---

Worklog Id: (was: 871530)
Time Spent: 20m  (was: 10m)

> MQTT retain flag not set correctly
> --
>
> Key: ARTEMIS-4365
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4365
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Justin Bertram
>Assignee: Justin Bertram
>Priority: Major
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> The {{retain}} flag set on MQTT messages dispatched to clients is incorrect 
> in certain circumstances. This is demonstrated by using the {{test}} command 
> from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.:
> 

[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 09:56
Start Date: 18/Jul/23 09:56
Worklog Time Spent: 10m 
  Work Description: gemmellr commented on PR #4555:
URL: 
https://github.com/apache/activemq-artemis/pull/4555#issuecomment-1639911264

   Its odd/confusing that the ReferenceNodeStoreFactory is still named to in 
most parameters and fields as 'referenceIDSupplier' or 'nodeStore', especially 
the latter with there being an actual seperate ReferenceNodeStore. Its mixing 
roles and field/param names up in a confusing manner. Feels like the behaviours 
should be split into separate concerns, as they largely were originally.
   
   Alternatively many of the ID methods now on ReferenceNodeStoreFactory could 
perhaps just be made static helpers rather than being 'passed around on the 
side of a factory', since they only pull details from the message[reference] 
rather than the factory itself.




Issue Time Tracking
---

Worklog Id: (was: 871515)
Time Spent: 40m  (was: 0.5h)

> Addresses with multiple subscriptions are not working with Mirroring
> 
>
> Key: ARTEMIS-4366
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4366
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Clebert Suconic
>Priority: Major
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> The NodeStore will be reused by the two Queues on the same address. Each 
> queue should have its own.



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


[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring

2023-07-18 Thread ASF GitHub Bot (Jira)


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

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

Author: ASF GitHub Bot
Created on: 18/Jul/23 09:48
Start Date: 18/Jul/23 09:48
Worklog Time Spent: 10m 
  Work Description: gemmellr commented on code in PR #4555:
URL: https://github.com/apache/activemq-artemis/pull/4555#discussion_r1266421706


##
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/ProtonProtocolManager.java:
##
@@ -75,7 +75,7 @@ public static String getMirrorAddress(String connectionName) {
 
// We must use one referenceIDSupplier per server.
// protocol manager is the perfect aggregation for that.
-   private ReferenceNodeStore referenceIDSupplier;
+   private ReferenceNodeStoreFactory referenceIDSupplier;

Review Comment:
   Change makes the comment above seem inaccurate / contradictory (somewhat 
similar with the field name). There is now a factory being returned, and so the 
overall change here seems like it was to make sure there **is not** a single 
reference ID supplier per server as the comment says, but rather now one 
per-queue.



##
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/ReferenceNodeStoreFactory.java:
##
@@ -0,0 +1,82 @@
+/*
+ * 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.activemq.artemis.protocol.amqp.connect.mirror;
+
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.MessageReference;
+import org.apache.activemq.artemis.utils.collections.NodeStore;
+import org.apache.activemq.artemis.utils.collections.NodeStoreFactory;
+
+import static 
org.apache.activemq.artemis.protocol.amqp.connect.mirror.AMQPMirrorControllerSource.INTERNAL_BROKER_ID_EXTRA_PROPERTY;
+import static 
org.apache.activemq.artemis.protocol.amqp.connect.mirror.AMQPMirrorControllerSource.INTERNAL_ID_EXTRA_PROPERTY;
+
+public class ReferenceNodeStoreFactory implements 
NodeStoreFactory {
+
+   final ActiveMQServer server;
+
+   private final String serverID;
+
+   public ReferenceNodeStoreFactory(ActiveMQServer server) {
+  this.server = server;
+  this.serverID = server.getNodeID().toString();
+

Review Comment:
   Superfluous newline



##
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/ProtonProtocolManager.java:
##
@@ -125,11 +125,11 @@ public ProtonProtocolManager(ProtonProtocolManagerFactory 
factory, ActiveMQServe
   routingHandler = new AMQPRoutingHandler(server);
}
 
-   public synchronized ReferenceNodeStore getReferenceIDSupplier() {
+   public synchronized ReferenceNodeStoreFactory getReferenceIDSupplier() {

Review Comment:
   The method name also seems off now.



##
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/ReferenceNodeStoreFactory.java:
##
@@ -0,0 +1,82 @@
+/*
+ * 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.activemq.artemis.protocol.amqp.connect.mirror;
+
+import org.apache.activemq.artemis.api.core.Message;