Merge pull request #1029 from guusdk/OF-1496_Reduce_discovery_complexity

OF-1496 reduce discovery complexity

src/java/org/jivesoftware/openfire/XMPPServer.java

@@ -667,6 +667,33 @@ public class XMPPServer {
                 logger.error(LocaleUtils.getLocalizedString("admin.error"), e);
             }
         }
+
+        // Register modules with service discovery provides where applicable.
+        for (Module module : modules.values() )
+        {
+            // Service discovery info
+            if (module instanceof ServerFeaturesProvider) {
+                getIQDiscoInfoHandler().addServerFeaturesProvider( (ServerFeaturesProvider) module );
+            }
+
+            if (module instanceof ServerIdentitiesProvider) {
+                getIQDiscoInfoHandler().addServerIdentitiesProvider( (ServerIdentitiesProvider) module );
+            }
+
+            if (module instanceof UserIdentitiesProvider) {
+                getIQDiscoInfoHandler().addUserIdentitiesProvider( (UserIdentitiesProvider) module );
+            }
+
+            // Service discovery items
+            if (module instanceof ServerItemsProvider) {
+                getIQDiscoItemsHandler().addServerItemsProvider( (ServerItemsProvider) module );
+            }
+
+            if (module instanceof UserItemsProvider) {
+                getIQDiscoItemsHandler().addUserItemsProvider( (UserItemsProvider) module);
+            }
+        }
+
     }
 
     /**
@@ -1330,7 +1357,9 @@ public class XMPPServer {
      * Returns a list with all the modules that provide "discoverable" features.
      *
      * @return a list with all the modules that provide "discoverable" features.
+     * @deprecated Use {@link IQDiscoInfoHandler} instead.
      */
+    @Deprecated
     public List<ServerFeaturesProvider> getServerFeaturesProviders() {
         List<ServerFeaturesProvider> answer = new ArrayList<>();
         for (Module module : modules.values()) {
@@ -1340,12 +1369,14 @@ public class XMPPServer {
         }
         return answer;
     }
- 
+
     /**
      * Returns a list with all the modules that provide "discoverable" identities.
      *
      * @return a list with all the modules that provide "discoverable" identities.
+     * @deprecated Use {@link IQDiscoInfoHandler} instead.
      */
+    @Deprecated
     public List<ServerIdentitiesProvider> getServerIdentitiesProviders() {
         List<ServerIdentitiesProvider> answer = new ArrayList<>();
         for (Module module : modules.values()) {
@@ -1362,7 +1393,9 @@ public class XMPPServer {
      *
      * @return a list with all the modules that provide "discoverable" items associated with
      *         the server.
+     * @deprecated Use {@link IQDiscoItemsHandler} instead.
      */
+    @Deprecated
     public List<ServerItemsProvider> getServerItemsProviders() {
         List<ServerItemsProvider> answer = new ArrayList<>();
         for (Module module : modules.values()) {
@@ -1372,12 +1405,14 @@ public class XMPPServer {
         }
         return answer;
     }
-    
+
     /**
      * Returns a list with all the modules that provide "discoverable" user identities.
      *
      * @return a list with all the modules that provide "discoverable" user identities.
+     * @deprecated Use {@link IQDiscoInfoHandler} instead.
      */
+    @Deprecated
     public List<UserIdentitiesProvider> getUserIdentitiesProviders() {
         List<UserIdentitiesProvider> answer = new ArrayList<>();
         for (Module module : modules.values()) {
@@ -1394,7 +1429,9 @@ public class XMPPServer {
      *
      * @return a list with all the modules that provide "discoverable" items associated with
      *         users.
+     * @deprecated Use {@link IQDiscoInfoHandler} instead.
      */
+    @Deprecated
     public List<UserItemsProvider> getUserItemsProviders() {
         List<UserItemsProvider> answer = new ArrayList<>();
         for (Module module : modules.values()) {

src/java/org/jivesoftware/openfire/disco/IQDiscoInfoHandler.java

@@ -69,26 +69,42 @@ public class IQDiscoInfoHandler extends IQHandler implements ClusterEventListene
     private Map<String, DiscoInfoProvider> entities = new HashMap<>();
     private Set<String> localServerFeatures = new CopyOnWriteArraySet<>();
     private Cache<String, Set<NodeID>> serverFeatures;
-    private List<Element> serverIdentities = new ArrayList<>();
+    private List<ServerIdentitiesProvider> serverIdentityProviders = new ArrayList<>();
     private Map<String, DiscoInfoProvider> serverNodeProviders = new ConcurrentHashMap<>();
     private IQHandlerInfo info;
 
-    private List<Element> anonymousUserIdentities = new ArrayList<>();
-    private List<Element> registeredUserIdentities = new ArrayList<>();
+    private List<UserIdentitiesProvider> anonymousUserIdentityProviders = new ArrayList<>();
+    private List<UserIdentitiesProvider> registeredUserIdentityProviders = new ArrayList<>();
 
     public IQDiscoInfoHandler() {
         super("XMPP Disco Info Handler");
         info = new IQHandlerInfo("query", NAMESPACE_DISCO_INFO);
-        // Initialize the user identity and features collections (optimization to avoid creating
-        // the same objects for each response)
-        Element userIdentity = DocumentHelper.createElement("identity");
-        userIdentity.addAttribute("category", "account");
-        userIdentity.addAttribute("type", "anonymous");
-        anonymousUserIdentities.add(userIdentity);
-        userIdentity = DocumentHelper.createElement("identity");
-        userIdentity.addAttribute("category", "account");
-        userIdentity.addAttribute("type", "registered");
-        registeredUserIdentities.add(userIdentity);
+
+        anonymousUserIdentityProviders.add( new UserIdentitiesProvider()
+        {
+            @Override
+            public Iterator<Element> getIdentities()
+            {
+                final Element userIdentity = DocumentHelper.createElement( "identity" );
+                userIdentity.addAttribute( "category", "account" );
+                userIdentity.addAttribute( "type", "anonymous" );
+
+                return Collections.singleton( userIdentity ).iterator();
+            }
+        } );
+
+        registeredUserIdentityProviders.add( new UserIdentitiesProvider()
+        {
+            @Override
+            public Iterator<Element> getIdentities()
+            {
+                final Element userIdentity = DocumentHelper.createElement( "identity" );
+                userIdentity.addAttribute( "category", "account" );
+                userIdentity.addAttribute( "type", "registered" );
+
+                return Collections.singleton( userIdentity ).iterator();
+            }
+        } );
     }
 
     @Override
@@ -139,7 +155,7 @@ public class IQDiscoInfoHandler extends IQHandler implements ClusterEventListene
                 boolean hasDiscoInfoFeature = false;
                 boolean hasDiscoItemsFeature = false;
                 boolean hasResultSetManagementFeature = false;
-                
+
                 while (features.hasNext()) {
                     final String feature = features.next();
                     queryElement.addElement("feature").addAttribute("var", feature);
@@ -158,13 +174,13 @@ public class IQDiscoInfoHandler extends IQHandler implements ClusterEventListene
                     queryElement.addElement("feature").addAttribute("var",
                             ResultSet.NAMESPACE_RESULT_SET_MANAGEMENT);
                 }
-                
+
                 if (!hasDiscoInfoFeature) {
                     // XEP-0030 requires that every entity that supports service
                     // discovery broadcasts the disco#info feature.
                     queryElement.addElement("feature").addAttribute("var", NAMESPACE_DISCO_INFO);
                 }
-                
+
                 // Add to the reply the extended info (XDataForm) provided by the DiscoInfoProvider
                 DataForm dataForm = infoProvider.getExtendedInfo(name, node, packet.getFrom());
                 if (dataForm != null) {
@@ -251,12 +267,65 @@ public class IQDiscoInfoHandler extends IQHandler implements ClusterEventListene
      *
      * @param provider the ServerFeaturesProvider that provides new server features.
      */
-    private void addServerFeaturesProvider(ServerFeaturesProvider provider) {
+    public void addServerFeaturesProvider(ServerFeaturesProvider provider) {
         for (Iterator<String> it = provider.getFeatures(); it.hasNext();) {
             addServerFeature(it.next());
         }
     }
 
+    /**
+     * Adds the "discoverable" identities provided by the provider whenever a disco for info is made against the server.
+     *
+     * @param provider The provider of identities.
+     */
+    public void addServerIdentitiesProvider(ServerIdentitiesProvider provider) {
+        if ( provider == null )
+        {
+            throw new NullPointerException( "Argument 'provider' cannot be null." );
+        }
+        serverIdentityProviders.add( provider );
+    }
+
+    /**
+     * Removes this provider of identities.
+     *
+     * @param provider The provider of identities.
+     */
+    public void removeServerIdentitiesProvider(ServerIdentitiesProvider provider) {
+        if ( provider == null )
+        {
+            throw new NullPointerException( "Argument 'provider' cannot be null." );
+        }
+        serverIdentityProviders.remove( provider );
+    }
+
+    /**
+     * Adds the "discoverable" user identities provided by the provider whenever a disco for info is made against users
+     * of the server.
+     *
+     * @param provider The provider of user identities.
+     */
+    public void addUserIdentitiesProvider(UserIdentitiesProvider provider) {
+        if ( provider == null )
+        {
+            throw new NullPointerException( "Argument 'provider' cannot be null." );
+        }
+        registeredUserIdentityProviders.add( provider );
+    }
+
+    /**
+     * Removes this provider of user identities.
+     *
+     * @param provider The provider of identities.
+     */
+    public void removeUserIdentitiesProvider(UserIdentitiesProvider provider) {
+        if ( provider == null )
+        {
+            throw new NullPointerException( "Argument 'provider' cannot be null." );
+        }
+        registeredUserIdentityProviders.remove( provider );
+    }
+
     /**
      * Adds one specific feature to the information returned whenever a disco for information is
      * made against the server.
@@ -314,26 +383,6 @@ public class IQDiscoInfoHandler extends IQHandler implements ClusterEventListene
         super.initialize(server);
         serverFeatures = CacheFactory.createCache("Disco Server Features");
         addServerFeature(NAMESPACE_DISCO_INFO);
-        // Track the implementors of ServerFeaturesProvider so that we can collect the features
-        // provided by the server
-        for (ServerFeaturesProvider provider : server.getServerFeaturesProviders()) {
-            addServerFeaturesProvider(provider);
-        }
-        // Collect the implementors of ServerIdentitiesProvider so that we can collect the identities
-        // for protocols supported by the server
-        for (ServerIdentitiesProvider provider : server.getServerIdentitiesProviders()) {
-            for (Iterator<Element> it = provider.getIdentities(); it.hasNext();) {
-                serverIdentities.add(it.next());
-            }
-        }
-        // Collect the implementors of UserIdentitiesProvider so that we can collect identities
-        // for registered users.
-        for (UserIdentitiesProvider provider : server.getUserIdentitiesProviders()) {
-            for (Iterator<Element> it = provider.getIdentities(); it.hasNext();) {
-                registeredUserIdentities.add(it.next());
-            }
-        }
-
         setProvider(server.getServerInfo().getXMPPDomain(), getServerInfoProvider());
         // Listen to cluster events
         ClusterManager.addListener(this);
@@ -415,7 +464,6 @@ public class IQDiscoInfoHandler extends IQHandler implements ClusterEventListene
      */
     private DiscoInfoProvider getServerInfoProvider() {
         return new DiscoInfoProvider() {
-            final ArrayList<Element> identities = new ArrayList<>();
 
             @Override
             public Iterator<Element> getIdentities(String name, String node, JID senderJID) {
@@ -425,20 +473,20 @@ public class IQDiscoInfoHandler extends IQHandler implements ClusterEventListene
                 }
                 if (name == null || name.equals(XMPPServer.getInstance().getServerInfo().getXMPPDomain())) {
                     // Answer identity of the server
-                    synchronized (identities) {
-                        if (identities.isEmpty()) {
-                            Element identity = DocumentHelper.createElement("identity");
-                            identity.addAttribute("category", "server");
-                            identity.addAttribute("name", JiveGlobals.getProperty(
-                                    "xmpp.server.name", "Openfire Server"));
-                            identity.addAttribute("type", "im");
-
-                            identities.add(identity);
-                            
-                            // Include identities from modules that implement ServerIdentitiesProvider
-                            for (Element identityElement : serverIdentities) {
-                                identities.add(identityElement);
-                            }
+                    final ArrayList<Element> identities = new ArrayList<>();
+                    final Element identity = DocumentHelper.createElement("identity");
+                    identity.addAttribute("category", "server");
+                    identity.addAttribute("name", JiveGlobals.getProperty("xmpp.server.name", "Openfire Server"));
+                    identity.addAttribute("type", "im");
+                    identities.add(identity);
+
+                    // Include identities from modules that implement ServerIdentitiesProvider
+                    for (ServerIdentitiesProvider provider : serverIdentityProviders )
+                    {
+                        final Iterator<Element> iterator = provider.getIdentities();
+                        while ( iterator.hasNext() )
+                        {
+                            identities.add( iterator.next() );
                         }
                     }
                     return identities.iterator();
@@ -449,12 +497,30 @@ public class IQDiscoInfoHandler extends IQHandler implements ClusterEventListene
                 else {
                     if (SessionManager.getInstance().isAnonymousRoute(name)) {
                         // Answer identity of an anonymous user.
-                        return anonymousUserIdentities.iterator();
+                        final Set<Element> result = new HashSet<>();
+                        for ( final UserIdentitiesProvider provider : anonymousUserIdentityProviders )
+                        {
+                            final Iterator<Element> identities = provider.getIdentities();
+                            while ( identities.hasNext() )
+                            {
+                                result.add( identities.next() );
+                            }
+                        }
+                        return result.iterator();
                     }
                     else {
                         // Answer identity of a registered user.
                         // Note: We know that this user exists because #hasInfo returned true
-                        return registeredUserIdentities.iterator();
+                        final Set<Element> result = new HashSet<>();
+                        for ( final UserIdentitiesProvider provider : registeredUserIdentityProviders )
+                        {
+                            final Iterator<Element> identities = provider.getIdentities();
+                            while ( identities.hasNext() )
+                            {
+                                result.add( identities.next() );
+                            }
+                        }
+                        return result.iterator();
                     }
                 }
             }

src/java/org/jivesoftware/openfire/disco/IQDiscoItemsHandler.java

@@ -82,6 +82,7 @@ public class IQDiscoItemsHandler extends IQHandler implements ServerFeaturesProv
     private Map<String, DiscoItemsProvider> serverNodeProviders = new ConcurrentHashMap<>();
     private IQHandlerInfo info;
     private IQDiscoInfoHandler infoHandler;
+    private List<UserItemsProvider> userItemsProviders = new ArrayList<>();
 
     public IQDiscoItemsHandler() {
         super("XMPP Disco Items Handler");
@@ -291,7 +292,27 @@ public class IQDiscoItemsHandler extends IQHandler implements ServerFeaturesProv
             infoHandler.removeProvider(host);
             removeProvider(host);
         }
+    }
 
+    /**
+     * Adds the items provided by the new service that implements the UserItemsProvider interface. This information will
+     * be used whenever a disco for items is made against users of the server.
+     *
+     * @param provider the UserItemsProvider that provides new user items.
+     */
+    public void addUserItemsProvider( UserItemsProvider provider )
+    {
+        this.userItemsProviders.add( provider );
+    }
+
+    /**
+     * Removes the UserItemsProvider
+     *
+     * @param provider the UserItemsProvider that provides new user items.
+     */
+    public void removeUserItemsProvider( UserItemsProvider provider )
+    {
+        this.userItemsProviders.remove( provider );
     }
 
     /**
@@ -409,9 +430,6 @@ public class IQDiscoItemsHandler extends IQHandler implements ServerFeaturesProv
     @Override
     public void start() throws IllegalStateException {
         super.start();
-        for (ServerItemsProvider provider : XMPPServer.getInstance().getServerItemsProviders()) {
-            addServerItemsProvider(provider);
-        }
     }
 
     @Override
@@ -503,22 +521,17 @@ public class IQDiscoItemsHandler extends IQHandler implements ServerFeaturesProv
                     return null;
                 }
                 if (name == null) {
-                    List<DiscoItem> answer = new ArrayList<>();
-                    for (ClusteredServerItem item : serverItems.values()) {
-                        answer.add(new DiscoItem(item.element));
-                    }
-                    return answer.iterator();
+                    return getServerItems().iterator();
                 }
                 else {
                     // If addressed to user@domain, add items from UserItemsProviders to
                     // the reply.
-                    List<UserItemsProvider> itemsProviders = XMPPServer.getInstance().getUserItemsProviders();
-                    if (itemsProviders.isEmpty()) {
+                    if ( userItemsProviders.isEmpty()) {
                         // If we didn't find any UserItemsProviders, then answer a not found error
                         return null;
                     }
                     List<DiscoItem> answer = new ArrayList<>();
-                    for (UserItemsProvider itemsProvider : itemsProviders) {
+                    for (UserItemsProvider itemsProvider : userItemsProviders ) {
                         // Check if we have items associated with the requested name
                         Iterator<Element> itemsItr = itemsProvider.getUserItems(name, senderJID);
                         if (itemsItr != null) {
@@ -582,4 +595,16 @@ public class IQDiscoItemsHandler extends IQHandler implements ServerFeaturesProv
             return answer.iterator();
         }
     }
+
+    /**
+     * Returns all server items.
+     * @return A collection of server items.
+     */
+    public List<DiscoItem> getServerItems() {
+        List<DiscoItem> answer = new ArrayList<>();
+        for (ClusteredServerItem item : serverItems.values()) {
+            answer.add(new DiscoItem(item.element));
+        }
+        return answer;
+    }
 }

src/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java

@@ -1273,7 +1273,6 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
         // Set us up to answer disco item requests
         XMPPServer.getInstance().getIQDiscoItemsHandler().addServerItemsProvider(this);
         XMPPServer.getInstance().getIQDiscoInfoHandler().setServerNodeInfoProvider(this.getServiceDomain(), this);
-        XMPPServer.getInstance().getServerItemsProviders().add(this);
 
         ArrayList<String> params = new ArrayList<>();
         params.clear();
@@ -1288,7 +1287,6 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
     private void stop() {
         XMPPServer.getInstance().getIQDiscoItemsHandler().removeServerItemsProvider(this);
         XMPPServer.getInstance().getIQDiscoInfoHandler().removeServerNodeInfoProvider(this.getServiceDomain());
-        XMPPServer.getInstance().getServerItemsProviders().remove(this);
         // Remove the route to this service
         routingTable.removeComponentRoute(getAddress());
         broadcastShutdown();