From 6fec813ec0026deffdb1465b61d45173d8de4ef8 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 31 Oct 2014 22:22:22 +0100 Subject: [PATCH] Rework DiscoverInfo Make Feature and Identity immutable and allow fast lookup of those two in DiscoverInfo. --- .../socks5/Socks5BytestreamManager.java | 13 +- .../smackx/disco/ServiceDiscoveryManager.java | 12 -- .../smackx/disco/packet/DiscoverInfo.java | 141 ++++++++++-------- .../disco/provider/DiscoverInfoProvider.java | 7 +- .../smackx/caps/EntityCapsManagerTest.java | 22 +-- 5 files changed, 100 insertions(+), 95 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java index 2eeeb00a1..820359402 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java @@ -49,7 +49,6 @@ import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream.StreamHostUs import org.jivesoftware.smackx.disco.ServiceDiscoveryManager; import org.jivesoftware.smackx.disco.packet.DiscoverInfo; import org.jivesoftware.smackx.disco.packet.DiscoverItems; -import org.jivesoftware.smackx.disco.packet.DiscoverInfo.Identity; import org.jivesoftware.smackx.disco.packet.DiscoverItems.Item; import org.jivesoftware.smackx.filetransfer.FileTransferManager; @@ -572,20 +571,14 @@ public final class Socks5BytestreamManager implements BytestreamManager { continue; } - // item must have category "proxy" and type "bytestream" - for (Identity identity : proxyInfo.getIdentities()) { - if ("proxy".equalsIgnoreCase(identity.getCategory()) - && "bytestreams".equalsIgnoreCase(identity.getType())) { - proxies.add(item.getEntityID()); - break; - } - + if (proxyInfo.hasIdentity("proxy", "bytestreams")) { + proxies.add(item.getEntityID()); + } else { /* * server is not a SOCKS5 proxy, blacklist server to skip next time a Socks5 * bytestream should be established */ this.proxyBlacklist.add(item.getEntityID()); - } } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/ServiceDiscoveryManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/ServiceDiscoveryManager.java index 8c01dbc98..9f02f280d 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/ServiceDiscoveryManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/ServiceDiscoveryManager.java @@ -202,18 +202,6 @@ public class ServiceDiscoveryManager extends Manager { return identity.getName(); } - /** - * Sets the name of the client that will be returned when asked for the client identity - * in a disco request. The name could be any value you need to identity this client. - * - * @param name the name of the client that will be returned when asked for the client identity - * in a disco request. - */ - public void setIdentityName(String name) { - identity.setName(name); - renewEntityCapsVersion(); - } - /** * Sets the default identity the client will report. * diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/packet/DiscoverInfo.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/packet/DiscoverInfo.java index 70afbc26a..7e5d70f77 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/packet/DiscoverInfo.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/packet/DiscoverInfo.java @@ -17,13 +17,17 @@ package org.jivesoftware.smackx.disco.packet; import org.jivesoftware.smack.packet.IQ; +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.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Set; /** * A DiscoverInfo IQ packet, which is used by XMPP clients to request and receive information @@ -39,8 +43,11 @@ public class DiscoverInfo extends IQ implements Cloneable { public static final String NAMESPACE = "http://jabber.org/protocol/disco#info"; private final List features = new LinkedList(); + private final Set featuresSet = new HashSet(); private final List identities = new LinkedList(); + private final Set identitiesSet = new HashSet(); private String node; + private boolean containsDuplicateFeatures; public DiscoverInfo() { super(); @@ -72,9 +79,10 @@ public class DiscoverInfo extends IQ implements Cloneable { * Adds a new feature to the discovered information. * * @param feature the discovered feature + * @return true if the feature did not already exist. */ - public void addFeature(String feature) { - addFeature(new Feature(feature)); + public boolean addFeature(String feature) { + return addFeature(new Feature(feature)); } /** @@ -89,8 +97,13 @@ public class DiscoverInfo extends IQ implements Cloneable { } } - private void addFeature(Feature feature) { + public boolean addFeature(Feature feature) { features.add(feature); + boolean featureIsNew = featuresSet.add(feature); + if (!featureIsNew) { + containsDuplicateFeatures = true; + } + return featureIsNew; } /** @@ -109,6 +122,7 @@ public class DiscoverInfo extends IQ implements Cloneable { */ public void addIdentity(Identity identity) { identities.add(identity); + identitiesSet.add(identity.getKey()); } /** @@ -118,7 +132,9 @@ public class DiscoverInfo extends IQ implements Cloneable { */ public void addIdentities(Collection identitiesToAdd) { if (identitiesToAdd == null) return; - identities.addAll(identitiesToAdd); + for (Identity identity : identitiesToAdd) { + addIdentity(identity); + } } /** @@ -138,12 +154,8 @@ public class DiscoverInfo extends IQ implements Cloneable { * @return true if this DiscoverInfo contains a Identity of the given category and type. */ public boolean hasIdentity(String category, String type) { - for (Identity identity : identities) { - if (identity.getCategory().equals(category) && identity.getType().equals(type)) { - return true; - } - } - return false; + String key = XmppStringUtils.generateKey(category, type); + return identitiesSet.contains(key); } /** @@ -196,11 +208,7 @@ public class DiscoverInfo extends IQ implements Cloneable { * @return true if the requestes feature has been discovered */ public boolean containsFeature(String feature) { - for (Feature f : getFeatures()) { - if (feature.equals(f.getVar())) - return true; - } - return false; + return features.contains(new Feature(feature)); } @Override @@ -245,15 +253,7 @@ public class DiscoverInfo extends IQ implements Cloneable { * @return true if duplicate identities where found, otherwise false */ public boolean containsDuplicateFeatures() { - List checkedFeatures = new LinkedList(); - for (Feature f : features) { - for (Feature f2 : checkedFeatures) { - if (f.equals(f2)) - return true; - } - checkedFeatures.add(f); - } - return false; + return containsDuplicateFeatures; } @Override @@ -273,13 +273,27 @@ public class DiscoverInfo extends IQ implements Cloneable { public static class Identity implements Comparable, Cloneable { private final String category; - private String name; private final String type; - private String lang; // 'xml:lang; + private final String key; + private final String name; + private final String lang; // 'xml:lang; public Identity(Identity identity) { - this(identity.category, identity.name, identity.type); - lang = identity.lang; + this.category = identity.category; + this.type = identity.type; + this.key = identity.type; + this.name = identity.name; + this.lang = identity.lang; + } + + /** + * Creates a new identity for an XMPP entity. + * + * @param category the entity's category (required as per XEP-30). + * @param type the entity's type (required as per XEP-30). + */ + public Identity(String category, String type) { + this(category, type, null, null); } /** @@ -292,12 +306,31 @@ public class DiscoverInfo extends IQ implements Cloneable { * @param type the entity's type (required as per XEP-30). */ public Identity(String category, String name, String type) { - if ((category == null) || (type == null)) - throw new IllegalArgumentException("category and type cannot be null"); - + this(category, type, name, null); + } + + /** + * Creates a new identity for an XMPP entity. + * 'category' and 'type' are required by + * XEP-30 XML Schemas + * + * @param category the entity's category (required as per XEP-30). + * @param type the entity's type (required as per XEP-30). + * @param name the entity's name. + * @param lang the entity's lang. + */ + public Identity(String category, String type, String name, String lang) { + if (StringUtils.isNullOrEmpty(category)) { + throw new IllegalArgumentException("category cannot be null"); + } + if (StringUtils.isNullOrEmpty(type)) { + throw new IllegalArgumentException("type cannot be null"); + } this.category = category; - this.name = name; this.type = type; + this.key = XmppStringUtils.generateKey(category, type); + this.name = name; + this.lang = lang; } /** @@ -319,15 +352,6 @@ public class DiscoverInfo extends IQ implements Cloneable { return name; } - /** - * Set the identity's name. - * - * @param name - */ - public void setName(String name) { - this.name = name; - } - /** * Returns the entity's type. To get the official registry of values for the * 'type' attribute refer to Jabber::Registrar @@ -338,15 +362,6 @@ public class DiscoverInfo extends IQ implements Cloneable { return type; } - /** - * Sets the natural language (xml:lang) for this identity (optional) - * - * @param lang the xml:lang of this Identity - */ - public void setLanguage(String lang) { - this.lang = lang; - } - /** * Returns the identities natural language if one is set * @@ -356,6 +371,21 @@ public class DiscoverInfo extends IQ implements Cloneable { return lang; } + private String getKey() { + return key; + } + + /** + * Returns true if this identity is of the given category and type. + * + * @param category the category. + * @param type the type. + * @return true if this identity is of the given category and type. + */ + public boolean isOfCategoryAndType(String category, String type) { + return this.category.equals(category) && this.type.equals(type); + } + public XmlStringBuilder toXML() { XmlStringBuilder xml = new XmlStringBuilder(); xml.halfOpenElement("identity"); @@ -382,19 +412,13 @@ public class DiscoverInfo extends IQ implements Cloneable { return false; DiscoverInfo.Identity other = (DiscoverInfo.Identity) obj; - if (!this.category.equals(other.category)) + if (!this.key.equals(other.key)) return false; String otherLang = other.lang == null ? "" : other.lang; String thisLang = lang == null ? "" : lang; if (!otherLang.equals(thisLang)) return false; - - // This safeguard can be removed once the deprecated constructor is removed. - String otherType = other.type == null ? "" : other.type; - String thisType = type == null ? "" : type; - if (!otherType.equals(thisType)) - return false; String otherName = other.name == null ? "" : other.name; String thisName = name == null ? "" : other.name; @@ -407,9 +431,8 @@ public class DiscoverInfo extends IQ implements Cloneable { @Override public int hashCode() { int result = 1; - result = 37 * result + category.hashCode(); + result = 37 * result + key.hashCode(); result = 37 * result + (lang == null ? 0 : lang.hashCode()); - result = 37 * result + (type == null ? 0 : type.hashCode()); result = 37 * result + (name == null ? 0 : name.hashCode()); return result; } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/provider/DiscoverInfoProvider.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/provider/DiscoverInfoProvider.java index 463b277cb..8707485a7 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/provider/DiscoverInfoProvider.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/provider/DiscoverInfoProvider.java @@ -66,14 +66,13 @@ public class DiscoverInfoProvider extends IQProvider { } else if (eventType == XmlPullParser.END_TAG) { if (parser.getName().equals("identity")) { // Create a new identity and add it to the discovered info. - identity = new DiscoverInfo.Identity(category, name, type); - if (lang != null) - identity.setLanguage(lang); + identity = new DiscoverInfo.Identity(category, type, name, lang); discoverInfo.addIdentity(identity); } if (parser.getName().equals("feature")) { // Create a new feature and add it to the discovered info. - discoverInfo.addFeature(variable); + boolean notADuplicateFeature = discoverInfo.addFeature(variable); + assert(notADuplicateFeature); } if (parser.getName().equals("query")) { done = true; 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 5af24324e..8da42f9e1 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 @@ -26,6 +26,7 @@ import java.util.Collection; import java.util.LinkedList; import org.jivesoftware.smack.packet.IQ; +import org.jivesoftware.smack.test.util.SmackTestSuite; import org.jivesoftware.smack.util.stringencoder.Base32; import org.jivesoftware.smack.util.stringencoder.StringEncoder; import org.jivesoftware.smackx.InitExtensions; @@ -34,11 +35,17 @@ import org.jivesoftware.smackx.caps.cache.SimpleDirectoryPersistentCache; import org.jivesoftware.smackx.disco.packet.DiscoverInfo; import org.jivesoftware.smackx.xdata.FormField; import org.jivesoftware.smackx.xdata.packet.DataForm; +import org.junit.Before; import org.junit.Test; public class EntityCapsManagerTest extends InitExtensions { + @Before + public void initSmackTestSuite() { + SmackTestSuite.init(); + } + /** * XEP- * 0115 Complex Generation Example @@ -96,11 +103,9 @@ public class EntityCapsManagerTest extends InitExtensions { di.setType(IQ.Type.result); Collection identities = new LinkedList(); - DiscoverInfo.Identity i = new DiscoverInfo.Identity("client", "Psi 0.11", "pc"); - i.setLanguage("en"); + DiscoverInfo.Identity i = new DiscoverInfo.Identity("client", "pc", "Psi 0.11", "en"); identities.add(i); - i = new DiscoverInfo.Identity("client", "Ψ 0.11", "pc"); - i.setLanguage("el"); + i = new DiscoverInfo.Identity("client", "pc", "Ψ 0.11", "el"); identities.add(i); di.addIdentities(identities); @@ -149,16 +154,13 @@ public class EntityCapsManagerTest extends InitExtensions { di.setType(IQ.Type.result); Collection identities = new LinkedList(); - DiscoverInfo.Identity i = new DiscoverInfo.Identity("client", "Psi 0.11", "pc"); - i.setLanguage("en"); + DiscoverInfo.Identity i = new DiscoverInfo.Identity("client", "pc", "Psi 0.11", "en"); identities.add(i); - i = new DiscoverInfo.Identity("client", "Ψ 0.11", "pc"); - i.setLanguage("el"); + i = new DiscoverInfo.Identity("client", "pc", "Ψ 0.11", "el"); identities.add(i); di.addIdentities(identities); // Failure 1: Duplicate identities - i = new DiscoverInfo.Identity("client", "Ψ 0.11", "pc"); - i.setLanguage("el"); + i = new DiscoverInfo.Identity("client", "pc", "Ψ 0.11", "el"); identities.add(i); di.addIdentities(identities);