From 43ce8d6372291f1197a007d2c3ababda87a3a545 Mon Sep 17 00:00:00 2001
From: andrea <andrea@samarcanda>
Date: Sat, 14 Dec 2013 14:03:49 +0100
Subject: [PATCH] Fixed a bug in store remove subscription that doesn't check
 the clientID but just the topic match

---
 ChangeLog.txt                                 |  1 +
 .../subscriptions/SubscriptionsStore.java     |  4 ++--
 .../subscriptions/SubscriptionsStoreTest.java | 21 +++++++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/ChangeLog.txt b/ChangeLog.txt
index 78b84818..3c2ced9b 100644
--- a/ChangeLog.txt
+++ b/ChangeLog.txt
@@ -10,3 +10,4 @@ Version 0.4:
     Dropped support for MINA
     Fix Issue 15 (bad re-activation of client's subscriptions after a recconection)
     Fix Issue 16 (bad re-publish of stored messages after a client reconnection)
+    Fixed a bug in unsubscribe due to missed clientID check
\ No newline at end of file
diff --git a/broker/src/main/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/SubscriptionsStore.java b/broker/src/main/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/SubscriptionsStore.java
index 9708460e..9b0e7c8e 100644
--- a/broker/src/main/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/SubscriptionsStore.java
+++ b/broker/src/main/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/SubscriptionsStore.java
@@ -124,10 +124,10 @@ public class SubscriptionsStore {
     public void removeSubscription(String topic, String clientID) {
         TreeNode matchNode = findMatchingNode(topic);
         
-        //search fr the subscription to remove
+        //search for the subscription to remove
         Subscription toBeRemoved = null;
         for (Subscription sub : matchNode.subscriptions()) {
-            if (sub.topic.equals(topic)) {
+            if (sub.topic.equals(topic) && sub.getClientId().equals(clientID)) {
                 toBeRemoved = sub;
                 break;
             }
diff --git a/broker/src/test/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/SubscriptionsStoreTest.java b/broker/src/test/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/SubscriptionsStoreTest.java
index 41039736..38ed26a2 100644
--- a/broker/src/test/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/SubscriptionsStoreTest.java
+++ b/broker/src/test/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/SubscriptionsStoreTest.java
@@ -324,6 +324,27 @@ public class SubscriptionsStoreTest {
         assertTrue(SubscriptionsStore.matchTopics("foo///baz", "foo/+/+/baz"));
         assertTrue(SubscriptionsStore.matchTopics("foo/bar/", "foo/bar/+"));
     }
+    
+    @Test
+    public void removeSubscription_withDifferentClients_subscribedSameTopic() {
+        SubscriptionsStore aStore = new SubscriptionsStore();
+        aStore.init(new MemoryStorageService());
+        //subscribe a not active clientID1 to /topic
+        Subscription slashSub = new Subscription("FAKE_CLI_ID_1", "/topic", AbstractMessage.QOSType.MOST_ONE, false);
+        aStore.add(slashSub);
+        aStore.deactivate(slashSub.getClientId());
+        
+        //subscribe an active clientID2 to /topic
+        Subscription slashSub2 = new Subscription("FAKE_CLI_ID_2", "/topic", AbstractMessage.QOSType.MOST_ONE, false);
+        aStore.add(slashSub2);
+        
+        //Exercise
+        aStore.removeSubscription("/topic", slashSub2.getClientId());
+        
+        //Verify
+        Subscription remainedSubscription = aStore.matches("/topic").get(0);
+        assertEquals(slashSub.getClientId(), remainedSubscription.getClientId());
+    }
 
     private static Token[] asArray(Object... l) {
         Token[] tokens = new Token[l.length];