From c3528d082e4f216cf853e547b972c80e91c52f40 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 10 Oct 2014 02:41:37 +0200 Subject: [PATCH] Add invokePacketCollectors, remove duplicate code Add javadoc about why we use CocurrentLinkedQueue for collectors. --- .../smack/AbstractXMPPConnection.java | 36 +++++++++++-------- .../jivesoftware/smack/DummyConnection.java | 5 +-- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java index e7b91404d..2e077f64d 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -102,8 +102,15 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { /** * A collection of PacketCollectors which collects packets for a specified filter * and perform blocking and polling operations on the result queue. + *

+ * We use a ConcurrentLinkedQueue here, because its Iterator is weakly + * consistent and we want {@link #invokePacketCollectors(Packet)} for-each + * loop to be lock free. As drawback, removing a PacketCollector is O(n). + * The alternative would be a synchronized HashSet, but this would mean a + * synchronized block around every usage of collectors. + *

*/ - protected final Collection collectors = new ConcurrentLinkedQueue(); + private final Collection collectors = new ConcurrentLinkedQueue(); /** * List of PacketListeners that will be notified when a new packet was received. @@ -658,15 +665,6 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { collectors.remove(collector); } - /** - * Get the collection of all packet collectors for this connection. - * - * @return a collection of packet collectors for this connection. - */ - protected Collection getPacketCollectors() { - return collectors; - } - @Override public void addPacketListener(PacketListener packetListener, PacketFilter packetFilter) { if (packetListener == null) { @@ -804,6 +802,19 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { packetReplyTimeout = timeout; } + /** + * Invoke {@link PacketCollector#processPacket(Packet)} for every + * PacketCollector with the given packet. + * + * @param packet the packet to notify the PacketCollectors about. + */ + protected void invokePacketCollectors(Packet packet) { + // Loop through all collectors and notify the appropriate ones. + for (PacketCollector collector: collectors) { + collector.processPacket(packet); + } + } + /** * Processes a packet after it's been fully parsed by looping through the installed * packet collectors and listeners and letting them examine the packet to see if @@ -816,10 +827,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { return; } - // Loop through all collectors and notify the appropriate ones. - for (PacketCollector collector: getPacketCollectors()) { - collector.processPacket(packet); - } + invokePacketCollectors(packet); // Deliver the incoming packet to listeners. executorService.submit(new ListenerNotification(packet)); diff --git a/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java b/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java index deb02e445..5689ead96 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java @@ -240,10 +240,7 @@ public class DummyConnection extends AbstractXMPPConnection { return; } - // Loop through all collectors and notify the appropriate ones. - for (PacketCollector collector: getPacketCollectors()) { - collector.processPacket(packet); - } + invokePacketCollectors(packet); if (SmackConfiguration.DEBUG_ENABLED) { System.out.println("[RECV]: " + packet.toXML());