From f94f7590a65da381779617930638c6c5a2503cf0 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 26 Oct 2014 19:32:19 +0100 Subject: [PATCH] Store packet extensions in a HashMap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit allowing O(1) lookups for PacketExtensions The one EntityCapsManagerTest becomes obsolete with this change, as duplicate extension elements (RFC 6120 ยง 8.4) are now no longer possible after the stanza has been parsed (they still may be received on the wire, but only the last duplicate will be added). --- .../smack/filter/PacketExtensionFilter.java | 6 +- .../org/jivesoftware/smack/packet/Packet.java | 109 +++++++++++++++--- .../smackx/caps/EntityCapsManagerTest.java | 6 - 3 files changed, 97 insertions(+), 24 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/filter/PacketExtensionFilter.java b/smack-core/src/main/java/org/jivesoftware/smack/filter/PacketExtensionFilter.java index 1d4fe220c..e1452a91b 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/filter/PacketExtensionFilter.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/filter/PacketExtensionFilter.java @@ -19,6 +19,7 @@ package org.jivesoftware.smack.filter; import org.jivesoftware.smack.packet.Packet; import org.jivesoftware.smack.packet.PacketExtension; +import org.jivesoftware.smack.util.StringUtils; /** * Filters for packets with a particular type of packet extension. @@ -39,6 +40,9 @@ public class PacketExtensionFilter implements PacketFilter { * @param namespace the XML namespace of the packet extension. */ public PacketExtensionFilter(String elementName, String namespace) { + if (StringUtils.isNullOrEmpty(namespace)) { + throw new IllegalArgumentException("namespace must not be null or empty"); + } this.elementName = elementName; this.namespace = namespace; } @@ -62,6 +66,6 @@ public class PacketExtensionFilter implements PacketFilter { } public boolean accept(Packet packet) { - return packet.getExtension(elementName, namespace) != null; + return packet.hasExtension(elementName, namespace); } } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/Packet.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/Packet.java index 3a88e0a16..c14b3527c 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/Packet.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/Packet.java @@ -20,13 +20,15 @@ package org.jivesoftware.smack.packet; import org.jivesoftware.smack.util.PacketUtil; import org.jivesoftware.smack.util.StringUtils; import org.jivesoftware.smack.util.XmlStringBuilder; +import org.jxmpp.util.XmppStringUtils; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; -import java.util.concurrent.CopyOnWriteArrayList; +import java.util.Map; import java.util.concurrent.atomic.AtomicLong; /** @@ -54,12 +56,11 @@ public abstract class Packet extends TopLevelStreamElement { */ private static AtomicLong id = new AtomicLong(); + private final Map packetExtensions = new HashMap(12); + private String packetID = null; private String to = null; private String from = null; - private final List packetExtensions - = new CopyOnWriteArrayList(); - private XMPPError error = null; /** @@ -197,15 +198,17 @@ public abstract class Packet extends TopLevelStreamElement { } /** - * Returns an unmodifiable collection of the packet extensions attached to the packet. + * Returns a copy of the packet extensions attached to the packet. * * @return the packet extensions. */ - public synchronized Collection getExtensions() { - if (packetExtensions == null) { - return Collections.emptyList(); + public List getExtensions() { + synchronized (packetExtensions) { + if (packetExtensions.isEmpty()) { + return Collections.emptyList(); + } + return new ArrayList(packetExtensions.values()); } - return Collections.unmodifiableList(new ArrayList(packetExtensions)); } /** @@ -213,9 +216,11 @@ public abstract class Packet extends TopLevelStreamElement { * * @param namespace the namespace of the extension that is desired. * @return the packet extension with the given namespace. + * @deprecated use {@link #getExtension(String,String)} instead. */ + @Deprecated public PacketExtension getExtension(String namespace) { - return getExtension(null, namespace); + return PacketUtil.packetExtensionfromCollection(getExtensions(), null, namespace); } /** @@ -233,11 +238,20 @@ public abstract class Packet extends TopLevelStreamElement { * @param namespace the XML element namespace of the packet extension. * @return the extension, or null if it doesn't exist. */ + @SuppressWarnings("unchecked") public PE getExtension(String elementName, String namespace) { if (namespace == null) { return null; } - return PacketUtil.packetExtensionfromCollection(packetExtensions, elementName, namespace); + String key = XmppStringUtils.generateKey(elementName, namespace); + PacketExtension packetExtension; + synchronized (packetExtensions) { + packetExtension = packetExtensions.get(key); + } + if (packetExtension == null) { + return null; + } + return (PE) packetExtension; } /** @@ -247,7 +261,10 @@ public abstract class Packet extends TopLevelStreamElement { */ public void addExtension(PacketExtension extension) { if (extension == null) return; - packetExtensions.add(extension); + String key = XmppStringUtils.generateKey(extension.getElementName(), extension.getNamespace()); + synchronized (packetExtensions) { + packetExtensions.put(key, extension); + } } /** @@ -257,16 +274,70 @@ public abstract class Packet extends TopLevelStreamElement { */ public void addExtensions(Collection extensions) { if (extensions == null) return; - packetExtensions.addAll(extensions); + for (PacketExtension packetExtension : extensions) { + addExtension(packetExtension); + } + } + + /** + * Check if a packet extension with the given element and namespace exists. + *

+ * The argument elementName may be null. + *

+ * + * @param elementName + * @param namespace + * @return true if a packet extension exists, false otherwise. + */ + public boolean hasExtension(String elementName, String namespace) { + if (elementName == null) { + return hasExtension(namespace); + } + String key = XmppStringUtils.generateKey(elementName, namespace); + synchronized (packetExtensions) { + return packetExtensions.containsKey(key); + } + } + + /** + * Check if a packet extension with the given namespace exists. + * + * @param namespace + * @return true if a packet extension exists, false otherwise. + */ + public boolean hasExtension(String namespace) { + synchronized (packetExtensions) { + for (PacketExtension packetExtension : packetExtensions.values()) { + if (packetExtension.getNamespace().equals(namespace)) { + return true; + } + } + } + return false; + } + + /** + * Remove the packet extension with the given elementName and namespace. + * + * @param elementName + * @param namespace + * @return the removed packet extension or null. + */ + public PacketExtension removeExtension(String elementName, String namespace) { + String key = XmppStringUtils.generateKey(elementName, namespace); + synchronized (packetExtensions) { + return packetExtensions.remove(key); + } } /** * Removes a packet extension from the packet. * * @param extension the packet extension to remove. + * @return the removed packet extension or null. */ - public void removeExtension(PacketExtension extension) { - packetExtensions.remove(extension); + public PacketExtension removeExtension(PacketExtension extension) { + return removeExtension(extension.getElementName(), extension.getNamespace()); } /** @@ -303,7 +374,9 @@ public abstract class Packet extends TopLevelStreamElement { if (error != null ? !error.equals(packet.error) : packet.error != null) { return false; } if (from != null ? !from.equals(packet.from) : packet.from != null) { return false; } - if (!packetExtensions.equals(packet.packetExtensions)) { return false; } + synchronized (packetExtensions) { + if (!packetExtensions.equals(packet.packetExtensions)) { return false; } + } if (packetID != null ? !packetID.equals(packet.packetID) : packet.packetID != null) { return false; } @@ -317,7 +390,9 @@ public abstract class Packet extends TopLevelStreamElement { result = 31 * result + (packetID != null ? packetID.hashCode() : 0); result = 31 * result + (to != null ? to.hashCode() : 0); result = 31 * result + (from != null ? from.hashCode() : 0); - result = 31 * result + packetExtensions.hashCode(); + synchronized (packetExtensions) { + result = 31 * result + packetExtensions.hashCode(); + } result = 31 * result + (error != null ? error.hashCode() : 0); return result; } diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/caps/EntityCapsManagerTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/caps/EntityCapsManagerTest.java index 084138867..5af24324e 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/caps/EntityCapsManagerTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/caps/EntityCapsManagerTest.java @@ -69,12 +69,6 @@ public class EntityCapsManagerTest extends InitExtensions { assertTrue(di.containsDuplicateIdentities()); } - @Test - public void testVerificationDuplicateDataForm() { - DiscoverInfo di = createMalformedDiscoverInfo(); - assertTrue(EntityCapsManager.verifyPacketExtensions(di)); - } - private void testSimpleDirectoryCache(StringEncoder stringEncoder) throws IOException { EntityCapsPersistentCache cache = new SimpleDirectoryPersistentCache(createTempDirectory());