john9x commented on code in PR #5719:
URL: https://github.com/apache/activemq-artemis/pull/5719#discussion_r2122657528
##########
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java:
##########
@@ -207,16 +204,13 @@ public boolean addSubscription(MqttTopicSubscription
subscription, WildcardConfi
SubscriptionItem existingSubscription =
subscriptions.get(subscription.topicFilter());
if (existingSubscription != null) {
- boolean updated = false;
- if (subscription.qualityOfService().value() >
existingSubscription.getSubscription().qualityOfService().value()) {
- existingSubscription.setSubscription(subscription);
- updated = true;
+ if (subscription.qualityOfService().value() >
existingSubscription.getSubscription().qualityOfService().value()
+ || !Objects.equals(subscriptionIdentifier,
existingSubscription.getId())) {
+ existingSubscription.update(subscription,
subscriptionIdentifier);
+ return true;
+ } else {
+ return false;
}
- if (subscriptionIdentifier != null &&
!subscriptionIdentifier.equals(existingSubscription.getId())) {
Review Comment:
@jbertram I'm not sure but I suspect there was a bug
>[3.8.2.1.2] The Subscription Identifier is associated with any subscription
created or modified as the result of this SUBSCRIBE packet. If there is a
Subscription Identifier, it is stored with the subscription. If this property
is not specified, then the absence of a Subscription Identifier is stored with
the subscription.
```
if (subscriptionIdentifier != null
```
means that previous non-NULL ID will not to be replaced with NULL ID if new
SUBSCRIBE does not contains subscription identifier.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
