Rework DiscoverInfo

Make Feature and Identity immutable and allow fast lookup of those two
in DiscoverInfo.

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"
+            if (proxyInfo.hasIdentity("proxy", "bytestreams")) {
-            for (Identity identity : proxyInfo.getIdentities()) {
-                if ("proxy".equalsIgnoreCase(identity.getCategory())
-                                && "bytestreams".equalsIgnoreCase(identity.getType())) {
                 proxies.add(item.getEntityID());
-                    break;
+            } else {
-                }
-
                 /*
                  * server is not a SOCKS5 proxy, blacklist server to skip next time a Socks5
                  * bytestream should be established
                  */
                 this.proxyBlacklist.add(item.getEntityID());
-
             }
         }
 

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.
      *

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<Feature> features = new LinkedList<Feature>();
+    private final Set<Feature> featuresSet = new HashSet<Feature>();
     private final List<Identity> identities = new LinkedList<Identity>();
+    private final Set<String> identitiesSet = new HashSet<String>();
     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) {
+    public boolean addFeature(String feature) {
-        addFeature(new Feature(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<Identity> 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) {
+        String key = XmppStringUtils.generateKey(category, type);
-            if (identity.getCategory().equals(category) && identity.getType().equals(type)) {
+        return identitiesSet.contains(key);
-                return true;
-            }
-        }
-        return false;
     }
 
     /**
@@ -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()) {
+        return features.contains(new Feature(feature));
-            if (feature.equals(f.getVar()))
-                return true;
-        }
-        return false;
     }
 
     @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<Feature> checkedFeatures = new LinkedList<Feature>();
+        return containsDuplicateFeatures;
-        for (Feature f : features) {
-            for (Feature f2 : checkedFeatures) {
-                if (f.equals(f2))
-                    return true;
-            }
-            checkedFeatures.add(f);
-        }
-        return false;
     }
 
     @Override
@@ -273,13 +273,27 @@ public class DiscoverInfo extends IQ implements Cloneable {
     public static class Identity implements Comparable<Identity>, 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);
+            this.category = identity.category;
-            lang = identity.lang;
+            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))
+            this(category, type, name, null);
-                throw new IllegalArgumentException("category and type cannot be null");
+        }
 
+        /**
+         * Creates a new identity for an XMPP entity.
+         * 'category' and 'type' are required by 
+         * <a href="http://xmpp.org/extensions/xep-0030.html#schemas">XEP-30 XML Schemas</a>
+         * 
+         * @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 <a href="http://www.jabber.org/registrar/disco-categories.html">Jabber::Registrar</a> 
@@ -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,7 +412,7 @@ 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;
@@ -390,12 +420,6 @@ public class DiscoverInfo extends IQ implements Cloneable {
             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;
             if (!thisName.equals(otherName))
@@ -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;
         }

smack-extensions/src/main/java/org/jivesoftware/smackx/disco/provider/DiscoverInfoProvider.java

@@ -66,14 +66,13 @@ public class DiscoverInfoProvider extends IQProvider<DiscoverInfo> {
             } 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);
+                    identity = new DiscoverInfo.Identity(category, type, name, lang);
-                    if (lang != null)
-                        identity.setLanguage(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;

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();
+    }
+
     /**
      * <a href="http://xmpp.org/extensions/xep-0115.html#ver-gen-complex">XEP-
      * 0115 Complex Generation Example</a>
@@ -96,11 +103,9 @@ public class EntityCapsManagerTest extends InitExtensions {
         di.setType(IQ.Type.result);
 
         Collection<DiscoverInfo.Identity> identities = new LinkedList<DiscoverInfo.Identity>();
-        DiscoverInfo.Identity i = new DiscoverInfo.Identity("client", "Psi 0.11", "pc");
+        DiscoverInfo.Identity i = new DiscoverInfo.Identity("client", "pc", "Psi 0.11", "en");
-        i.setLanguage("en");
         identities.add(i);
-        i = new DiscoverInfo.Identity("client", "Ψ 0.11", "pc");
+        i = new DiscoverInfo.Identity("client", "pc", "Ψ 0.11", "el");
-        i.setLanguage("el");
         identities.add(i);
         di.addIdentities(identities);
 
@@ -149,16 +154,13 @@ public class EntityCapsManagerTest extends InitExtensions {
         di.setType(IQ.Type.result);
 
         Collection<DiscoverInfo.Identity> identities = new LinkedList<DiscoverInfo.Identity>();
-        DiscoverInfo.Identity i = new DiscoverInfo.Identity("client", "Psi 0.11", "pc");
+        DiscoverInfo.Identity i = new DiscoverInfo.Identity("client", "pc", "Psi 0.11", "en");
-        i.setLanguage("en");
         identities.add(i);
-        i = new DiscoverInfo.Identity("client", "Ψ 0.11", "pc");
+        i = new DiscoverInfo.Identity("client", "pc", "Ψ 0.11", "el");
-        i.setLanguage("el");
         identities.add(i);
         di.addIdentities(identities);
         // Failure 1: Duplicate identities
-        i = new DiscoverInfo.Identity("client", "Ψ 0.11", "pc");
+        i = new DiscoverInfo.Identity("client", "pc", "Ψ 0.11", "el");
-        i.setLanguage("el");
         identities.add(i);
         di.addIdentities(identities);