From 2c18ee92c7032027f986240a8c8eb5d75d431bf7 Mon Sep 17 00:00:00 2001 From: andrea Date: Wed, 6 Nov 2013 12:35:10 +0100 Subject: [PATCH] Added tests suite used by Roger Light to check SubscriptionStore.matches --- .../subscriptions/SubscriptionsStore.java | 8 ++-- .../spi/impl/subscriptions/TreeNode.java | 3 +- .../subscriptions/SubscriptionsStoreTest.java | 48 +++++++++++++++++-- 3 files changed, 51 insertions(+), 8 deletions(-) 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 ceb34f0a..554c37aa 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 @@ -2,6 +2,7 @@ package org.dna.mqtt.moquette.messaging.spi.impl.subscriptions; import java.text.ParseException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Queue; import java.util.concurrent.LinkedBlockingDeque; @@ -181,6 +182,7 @@ public class SubscriptionsStore { } catch (ParseException ex) { //TODO handle the parse exception LOG.error(null, ex); + return Collections.EMPTY_LIST; } Queue tokenQueue = new LinkedBlockingDeque(tokens); @@ -255,9 +257,9 @@ public class SubscriptionsStore { for (int i = 0; i < splitted.length; i++) { String s = splitted[i]; if (s.isEmpty()) { - if (i != 0) { - throw new ParseException("Bad format of topic, expetec topic name between separators", i); - } +// if (i != 0) { +// throw new ParseException("Bad format of topic, expetec topic name between separators", i); +// } res.add(Token.EMPTY); } else if (s.equals("#")) { //check that multi is the last symbol diff --git a/broker/src/main/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/TreeNode.java b/broker/src/main/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/TreeNode.java index dcf1ce59..e905d9ee 100644 --- a/broker/src/main/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/TreeNode.java +++ b/broker/src/main/java/org/dna/mqtt/moquette/messaging/spi/impl/subscriptions/TreeNode.java @@ -66,7 +66,7 @@ class TreeNode { matchingSubs.addAll(m_subscriptions); //check if it has got a MULTI child and add its subscriptions for (TreeNode n : m_children) { - if (n.getToken() == Token.MULTI) { + if (n.getToken() == Token.MULTI || n.getToken() == Token.SINGLE) { matchingSubs.addAll(n.subscriptions()); } } @@ -85,6 +85,7 @@ class TreeNode { //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); + //TODO don't create a copy n.matches(tokens, matchingSubs); } } } 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 5cb4bab7..61315f49 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 @@ -62,10 +62,10 @@ public class SubscriptionsStoreTest { assertEqualsSeq(asArray(Token.EMPTY), tokens); } - @Test(expected = ParseException.class) - public void testSplitTopicTwinsSlashAvoided() throws ParseException { - store.splitTopic("/finance//stock/ibm"); - } +// @Test(expected = ParseException.class) +// public void testSplitTopicTwinsSlashAvoided() throws ParseException { +// store.splitTopic("/finance//stock/ibm"); +// } @Test public void testSplitTopicMultiValid() throws ParseException { @@ -211,6 +211,46 @@ public class SubscriptionsStoreTest { assertFalse(store.matches("finance").isEmpty()); } + @Test + public void rogerLightTopicMatches() { + assertMatch("foo/bar", "foo/bar"); + assertMatch("foo/bar", "foo/bar"); + assertMatch("foo/+", "foo/bar"); + assertMatch("foo/+/baz", "foo/bar/baz"); + assertMatch("foo/+/#", "foo/bar/baz"); + assertMatch("#", "foo/bar/baz"); + + assertNotMatch("foo/bar", "foo"); + assertNotMatch("foo/+", "foo/bar/baz"); + assertNotMatch("foo/+/baz", "foo/bar/bar"); + assertNotMatch("foo/+/#", "fo2/bar/baz"); + + assertMatch("#", "/foo/bar"); + assertMatch("/#", "/foo/bar"); + assertNotMatch("/#", "foo/bar"); + + assertMatch("foo//bar", "foo//bar"); + assertMatch("foo//+", "foo//bar"); + assertMatch("foo/+/+/baz", "foo///baz"); + assertMatch("foo/bar/+", "foo/bar/"); + } + + private void assertMatch(String subscription, String topic) { + store = new SubscriptionsStore(); + store.init(new MemoryStorageService()); + Subscription sub = new Subscription("FAKE_CLI_ID_1", subscription, AbstractMessage.QOSType.MOST_ONE, false); + store.add(sub); + assertFalse(store.matches(topic).isEmpty()); + } + + private void assertNotMatch(String subscription, String topic) { + store = new SubscriptionsStore(); + store.init(new MemoryStorageService()); + Subscription sub = new Subscription("FAKE_CLI_ID_1", subscription, AbstractMessage.QOSType.MOST_ONE, false); + store.add(sub); + assertTrue(store.matches(topic).isEmpty()); + } + @Test public void testRemoveClientSubscriptions_existingClientID() {