diff --git a/ChangeLog.txt b/ChangeLog.txt index 976f8cda..e30c92a3 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -19,7 +19,8 @@ Version 0.7: Fixed duplicated subscription with different QoS same client and topic (fix issue #49) Fixed UNSUBSCRIBE that missed to remove the subscription from the persistent store (fix issue #50) Fixed issue on multiple publish for overlapping subscription by the same client (fix issue #52). - Moved the authorizaion check before dropping an already connected client with same clientID (fix issue #44). + Moved the authorization check before dropping an already connected client with same clientID (fix issue #44). + Fixed subscription tree that was unable to update clean session flag on subs with same client, topic and qos (fix issue #55) Version 0.6: Fix issue #42, bad password file path composition diff --git a/broker/src/main/java/org/eclipse/moquette/spi/impl/subscriptions/Subscription.java b/broker/src/main/java/org/eclipse/moquette/spi/impl/subscriptions/Subscription.java index cc015180..a4b41bc1 100644 --- a/broker/src/main/java/org/eclipse/moquette/spi/impl/subscriptions/Subscription.java +++ b/broker/src/main/java/org/eclipse/moquette/spi/impl/subscriptions/Subscription.java @@ -72,33 +72,27 @@ public class Subscription implements Serializable { } @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - final Subscription other = (Subscription) obj; - if (this.requestedQos != other.requestedQos) { - return false; - } - if ((this.clientId == null) ? (other.clientId != null) : !this.clientId.equals(other.clientId)) { - return false; - } - if ((this.topicFilter == null) ? (other.topicFilter != null) : !this.topicFilter.equals(other.topicFilter)) { - return false; - } + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Subscription that = (Subscription) o; + + if (cleanSession != that.cleanSession) return false; + if (clientId != null ? !clientId.equals(that.clientId) : that.clientId != null) return false; + if (requestedQos != that.requestedQos) return false; + if (topicFilter != null ? !topicFilter.equals(that.topicFilter) : that.topicFilter != null) return false; + return true; } @Override public int hashCode() { - int hash = 3; - hash = 37 * hash + (this.requestedQos != null ? this.requestedQos.hashCode() : 0); - hash = 37 * hash + (this.clientId != null ? this.clientId.hashCode() : 0); - hash = 37 * hash + (this.topicFilter != null ? this.topicFilter.hashCode() : 0); - return hash; + int result = requestedQos.hashCode(); + result = 31 * result + (clientId != null ? clientId.hashCode() : 0); + result = 31 * result + (topicFilter != null ? topicFilter.hashCode() : 0); + result = 31 * result + (cleanSession ? 1 : 0); + return result; } /** diff --git a/broker/src/main/java/org/eclipse/moquette/spi/impl/subscriptions/TreeNode.java b/broker/src/main/java/org/eclipse/moquette/spi/impl/subscriptions/TreeNode.java index 29b90446..2a165ff0 100644 --- a/broker/src/main/java/org/eclipse/moquette/spi/impl/subscriptions/TreeNode.java +++ b/broker/src/main/java/org/eclipse/moquette/spi/impl/subscriptions/TreeNode.java @@ -113,7 +113,7 @@ class TreeNode { if (n.getToken().match(t)) { //Create a copy of token, else if navigate 2 sibling it //consumes 2 elements on the queue instead of one - n.matches(new LinkedBlockingQueue(tokens), matchingSubs); + n.matches(new LinkedBlockingQueue<>(tokens), matchingSubs); //TODO don't create a copy n.matches(tokens, matchingSubs); } } @@ -132,7 +132,7 @@ class TreeNode { void removeClientSubscriptions(String clientID) { //collect what to delete and then delete to avoid ConcurrentModification - List subsToRemove = new ArrayList(); + List subsToRemove = new ArrayList<>(); for (Subscription s : m_subscriptions) { if (s.clientId.equals(clientID)) { subsToRemove.add(s); @@ -185,7 +185,7 @@ class TreeNode { * @return the set of subscriptions for the given client. * */ Set findAllByClientID(String clientID) { - Set subs = new HashSet(); + Set subs = new HashSet<>(); for (Subscription s : m_subscriptions) { if (s.clientId.equals(clientID)) { subs.add(s); diff --git a/broker/src/test/java/org/eclipse/moquette/spi/impl/subscriptions/TreeNodeTest.java b/broker/src/test/java/org/eclipse/moquette/spi/impl/subscriptions/TreeNodeTest.java new file mode 100644 index 00000000..3520d2c2 --- /dev/null +++ b/broker/src/test/java/org/eclipse/moquette/spi/impl/subscriptions/TreeNodeTest.java @@ -0,0 +1,33 @@ +package org.eclipse.moquette.spi.impl.subscriptions; + + +import static junit.framework.Assert.assertEquals; +import static org.eclipse.moquette.proto.messages.AbstractMessage.QOSType.*; +import org.junit.Test; + +import java.util.Set; + +public class TreeNodeTest { + + public static final String CLIENT_ID = "FAKE_CLIENT_ID"; + + @Test + public void testAddSubscriptionOverwriteAnExistingWithDifferentCleanSessionFlag() throws Exception { + TreeNode root = new TreeNode(null); + Subscription existingSub = new Subscription(CLIENT_ID, "/topic", LEAST_ONE, false); + root.addSubscription(existingSub); + Set subs = root.findAllByClientID(CLIENT_ID); + assertEquals(1, subs.size()); + + //Exercise + Subscription overwritingSub = new Subscription(CLIENT_ID, "/topic", LEAST_ONE, true); + root.addSubscription(overwritingSub); + + //Verify + //the second subscription has overwritten the first one + subs = root.findAllByClientID(CLIENT_ID); + assertEquals(1, subs.size()); + Subscription storedSub = subs.iterator().next(); + assertEquals(overwritingSub.isCleanSession(), storedSub.isCleanSession()); + } +} \ No newline at end of file