OF-1496: Reduce complexity of service discovery data handling.

XMPPServer should not expose Service Discovery data providers that do not cover all service discovery information
that applies to the XMPP domain. It did so, as it only exposed providers that were based on its modules, but not
other components.

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

@@ -139,7 +139,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 +158,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 +251,36 @@ 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. This information will be used whenever a disco for
+     * info is made against the server.
+     *
+     * @param provider The provider of identities.
+     */
+    public void addServerIdentitiesProvider(ServerIdentitiesProvider provider) {
+        for (Iterator<Element> it = provider.getIdentities(); it.hasNext();) {
+            serverIdentities.add(it.next());
+        }
+    }
+
+    /**
+     * Adds the "discoverable" user identities provided by the provider. This information will be used whenever a disco
+     * for info is made against users of the server.
+     *
+     * @param provider The provider of user identities.
+     */
+    public void addUserIdentitiesProvider(UserIdentitiesProvider provider) {
+        for (Iterator<Element> it = provider.getIdentities(); it.hasNext();) {
+            registeredUserIdentities.add( it.next() );
+        }
+    }
+
     /**
      * Adds one specific feature to the information returned whenever a disco for information is
      * made against the server.
@@ -314,26 +338,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);

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;
+    }
 }