Changed Subscription equals to use also cleanSession flag in equality check

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

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;
     }
 
     /**

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<Token>(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<Subscription> subsToRemove = new ArrayList<Subscription>();
+        List<Subscription> 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<Subscription> findAllByClientID(String clientID) {
-        Set<Subscription> subs = new HashSet<Subscription>();
+        Set<Subscription> subs = new HashSet<>();
         for (Subscription s : m_subscriptions) {
             if (s.clientId.equals(clientID)) {
                 subs.add(s);

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<Subscription> 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());
+    }
+}