[JM-1217] Couple more improvements to multiple connection support.

git-svn-id: http://svn.igniterealtime.org/svn/repos/openfire/trunk@9777 b35dd754-fafc-0310-a699-88a17e54d16e

src/java/org/jivesoftware/openfire/SessionManager.java

@@ -1150,8 +1150,12 @@ public class SessionManager extends BasicModule implements ClusterEventListener
             finally {
                 // Remove the session
                 localSessionManager.getComponentsSessions().remove(session);
+
                 // Remove track of the cluster node hosting the external component
-                componentSessionsCache.remove(session.getAddress().toString());
+                // if no more components are handling it.
+                if (!InternalComponentManager.getInstance().hasComponent(session.getAddress())) {
+                    componentSessionsCache.remove(session.getAddress().toString());
+                }
             }
         }
     }

src/java/org/jivesoftware/openfire/component/InternalComponentManager.java

@@ -47,7 +47,7 @@ import java.util.concurrent.TimeUnit;
  */
 public class InternalComponentManager extends BasicModule implements ComponentManager, RoutableChannelHandler {
 
-    private Map<String, RoutableComponent> routables = new ConcurrentHashMap<String, RoutableComponent>();
+    final private Map<String, RoutableComponents> routables = new ConcurrentHashMap<String, RoutableComponents>();
     private Map<String, IQ> componentInfo = new ConcurrentHashMap<String, IQ>();
     private Map<JID, JID> presenceMap = new ConcurrentHashMap<JID, JID>();
     /**
@@ -104,107 +104,112 @@ public class InternalComponentManager extends BasicModule implements ComponentMa
     }
 
     public void addComponent(String subdomain, Component component) throws ComponentException {
-        RoutableComponent routable = routables.get(subdomain);
-        if (routable != null && routable.hasComponent(component)) {
-            // This component has already registered with this subdomain.
-            // TODO: Is this all we should do?  Should we return an error?
-            return;
-        }
-        Log.debug("InternalComponentManager: Registering component for domain: " + subdomain);
-        JID componentJID = new JID(subdomain + "." + serverDomain);
-        if (routable != null) {
-            routable.addComponent(component);
-        }
-        else {
-            routable = new RoutableComponent(componentJID, component);
-            routables.put(subdomain, routable);
-            
-            // Add the route to the new service provided by the component
-            XMPPServer.getInstance().getRoutingTable().addComponentRoute(componentJID, routable);
-        }
-
-        // Initialize the new component
-        try {
-            component.initialize(componentJID, this);
-            component.start();
+        synchronized (routables) {
+            RoutableComponents routable = routables.get(subdomain);
+            if (routable != null && routable.hasComponent(component)) {
+                // This component has already registered with this subdomain.
+                // TODO: Is this all we should do?  Should we return an error?
+                return;
+            }
+            Log.debug("InternalComponentManager: Registering component for domain: " + subdomain);
+            JID componentJID = new JID(subdomain + "." + serverDomain);
+            if (routable != null) {
+                routable.addComponent(component);
+            }
+            else {
+                routable = new RoutableComponents(componentJID, component);
+                routables.put(subdomain, routable);
 
-            // Notify listeners that a new component has been registered
-            for (ComponentEventListener listener : listeners) {
-                listener.componentRegistered(component, componentJID);
+                // Add the route to the new service provided by the component
+                XMPPServer.getInstance().getRoutingTable().addComponentRoute(componentJID, routable);
             }
 
-            // Check for potential interested users.
-            checkPresences();
-            // Send a disco#info request to the new component. If the component provides information
-            // then it will be added to the list of discoverable server items.
-            checkDiscoSupport(component, componentJID);
-            Log.debug("InternalComponentManager: Component registered for domain: " + subdomain);
-        }
-        catch (Exception e) {
-            // Unregister the component's domain
-            routable.removeComponent(component);
-            if (e instanceof ComponentException) {
+            // Initialize the new component
+            try {
+                component.initialize(componentJID, this);
+                component.start();
+
+                // Notify listeners that a new component has been registered
+                for (ComponentEventListener listener : listeners) {
+                    listener.componentRegistered(component, componentJID);
+                }
+
+                // Check for potential interested users.
+                checkPresences();
+                // Send a disco#info request to the new component. If the component provides information
+                // then it will be added to the list of discoverable server items.
+                checkDiscoSupport(component, componentJID);
+                Log.debug("InternalComponentManager: Component registered for domain: " + subdomain);
+            }
+            catch (Exception e) {
+                // Unregister the component's domain
+                routable.removeComponent(component);
+                if (e instanceof ComponentException) {
+                    // Rethrow the exception
+                    throw (ComponentException)e;
+                }
                 // Rethrow the exception
-                throw (ComponentException)e;
+                throw new ComponentException(e);
             }
-            // Rethrow the exception
-            throw new ComponentException(e);
-        }
-        finally {
-            if (routable.numberOfComponents() == 0) {
-                // If there are no more components associated with this subdomain, remove it.
-                routables.remove(subdomain);
-                // Remove the route
-                XMPPServer.getInstance().getRoutingTable().removeComponentRoute(componentJID);
+            finally {
+                if (routable.numberOfComponents() == 0) {
+                    // If there are no more components associated with this subdomain, remove it.
+                    routables.remove(subdomain);
+                    // Remove the route
+                    XMPPServer.getInstance().getRoutingTable().removeComponentRoute(componentJID);
+                }
             }
         }
-
     }
 
     public void removeComponent(String subdomain) {
-        Log.debug("InternalComponentManager: Unregistering all components for domain: " + subdomain);
-        RoutableComponent routable = routables.get(subdomain);
-        routable.removeAllComponents();
-        routables.remove(subdomain);
+        synchronized (routables) {
+            Log.debug("InternalComponentManager: Unregistering all components for domain: " + subdomain);
+            RoutableComponents routable = routables.get(subdomain);
+            routable.removeAllComponents();
+            routables.remove(subdomain);
+        }
     }
 
     public void removeComponent(String subdomain, Component component) {
-        Log.debug("InternalComponentManager: Unregistering component for domain: " + subdomain);
-        RoutableComponent routable = routables.get(subdomain);
-        routable.removeComponent(component);
-        if (routable.numberOfComponents() == 0) {
-            routables.remove(subdomain);
+        synchronized (routables) {
+            Log.debug("InternalComponentManager: Unregistering component for domain: " + subdomain);
+            RoutableComponents routable = routables.get(subdomain);
+            routable.removeComponent(component);
+            if (routable.numberOfComponents() == 0) {
+                routables.remove(subdomain);
 
-            // Remove any info stored with the component being removed
-            componentInfo.remove(subdomain);
+                // Remove any info stored with the component being removed
+                componentInfo.remove(subdomain);
 
-            JID componentJID = new JID(subdomain + "." + serverDomain);
+                JID componentJID = new JID(subdomain + "." + serverDomain);
 
-            // Remove the route for the service provided by the component
-            RoutingTable routingTable = XMPPServer.getInstance().getRoutingTable();
-            if (routingTable != null) {
-                routingTable.removeComponentRoute(componentJID);
-            }
+                // Remove the route for the service provided by the component
+                RoutingTable routingTable = XMPPServer.getInstance().getRoutingTable();
+                if (routingTable != null) {
+                    routingTable.removeComponentRoute(componentJID);
+                }
 
-            // Remove the disco item from the server for the component that is being removed
-            IQDiscoItemsHandler iqDiscoItemsHandler = XMPPServer.getInstance().getIQDiscoItemsHandler();
-            if (iqDiscoItemsHandler != null) {
-                iqDiscoItemsHandler.removeComponentItem(componentJID.toBareJID());
-            }
+                // Remove the disco item from the server for the component that is being removed
+                IQDiscoItemsHandler iqDiscoItemsHandler = XMPPServer.getInstance().getIQDiscoItemsHandler();
+                if (iqDiscoItemsHandler != null) {
+                    iqDiscoItemsHandler.removeComponentItem(componentJID.toBareJID());
+                }
 
-            // Ask the component to shutdown
-            if (component != null) {
-                component.shutdown();
-            }
+                // Ask the component to shutdown
+                if (component != null) {
+                    component.shutdown();
+                }
 
-            // Notify listeners that an existing component has been unregistered
-            for (ComponentEventListener listener : listeners) {
-                listener.componentUnregistered(component, componentJID);
+                // Notify listeners that an existing component has been unregistered
+                for (ComponentEventListener listener : listeners) {
+                    listener.componentUnregistered(component, componentJID);
+                }
+                Log.debug("InternalComponentManager: Component unregistered for domain: " + subdomain);
+            }
+            else {
+                Log.debug("InternalComponentManager: Other components still tied to domain: " + subdomain);
             }
-            Log.debug("InternalComponentManager: Component unregistered for domain: " + subdomain);
-        }
-        else {
-            Log.debug("InternalComponentManager: Other components still tied to domain: " + subdomain);
         }
     }
 
@@ -212,7 +217,7 @@ public class InternalComponentManager extends BasicModule implements ComponentMa
         if (packet != null && packet.getFrom() == null) {
             throw new IllegalArgumentException("Packet with no FROM address was received from component.");
         }
-
+        
         PacketRouter router = XMPPServer.getInstance().getPacketRouter();
         if (router != null) {
             router.route(packet);
@@ -259,9 +264,9 @@ public class InternalComponentManager extends BasicModule implements ComponentMa
     public void addListener(ComponentEventListener listener) {
         listeners.add(listener);
         // Notify the new listener about existing components
-        for (Map.Entry<String, RoutableComponent> entry : routables.entrySet()) {
+        for (Map.Entry<String, RoutableComponents> entry : routables.entrySet()) {
             String subdomain = entry.getKey();
-            RoutableComponent routable = entry.getValue();
+            RoutableComponents routable = entry.getValue();
             for (Component component : routable.getComponents()) {
                 JID componentJID = new JID(subdomain + "." + serverDomain);
                 listener.componentRegistered(component, componentJID);
@@ -366,26 +371,28 @@ public class InternalComponentManager extends BasicModule implements ComponentMa
      * @return the component with the specified id.
      */
     private Component getComponent(JID componentJID) {
-        if (componentJID.getNode() != null) {
-            return null;
-        }
-        RoutableComponent routable = routables.get(componentJID.getDomain());
-        if (routable != null) {
-            return routable.getNextComponent();
-        }
-        else {
-            // Search again for those JIDs whose domain include the server name but this
-            // time remove the server name from the JID's domain
-            String serverName = componentJID.getDomain();
-            int index = serverName.lastIndexOf("." + serverDomain);
-            if (index > -1) {
-                routable = routables.get(serverName.substring(0, index));
-                if (routable != null) {
-                    return routable.getNextComponent();
+        synchronized (routables) {
+            if (componentJID.getNode() != null) {
+                return null;
+            }
+            RoutableComponents routable = routables.get(componentJID.getDomain());
+            if (routable != null) {
+                return routable.getNextComponent();
+            }
+            else {
+                // Search again for those JIDs whose domain include the server name but this
+                // time remove the server name from the JID's domain
+                String serverName = componentJID.getDomain();
+                int index = serverName.lastIndexOf("." + serverDomain);
+                if (index > -1) {
+                    routable = routables.get(serverName.substring(0, index));
+                    if (routable != null) {
+                        return routable.getNextComponent();
+                    }
                 }
             }
+            return null;
         }
-        return null;
     }
 
     /**
@@ -396,13 +403,15 @@ public class InternalComponentManager extends BasicModule implements ComponentMa
      * @return true if a component is associated to the specified address.
      */
     public boolean hasComponent(JID componentJID) {
-        if (componentJID.getNode() != null || componentJID.getResource() != null) {
-            return false;
+        synchronized (routables) {
+            if (componentJID.getNode() != null || componentJID.getResource() != null) {
+                return false;
+            }
+    //        if (componentJID.getDomain().lastIndexOf("." + serverDomain) == -1) {
+    //            componentJID = new JID(componentJID.getDomain() + "." + serverDomain);
+    //        }
+            return routingTable.hasComponentRoute(componentJID);
         }
-//        if (componentJID.getDomain().lastIndexOf("." + serverDomain) == -1) {
-//            componentJID = new JID(componentJID.getDomain() + "." + serverDomain);
-//        }
-        return routingTable.hasComponentRoute(componentJID);
     }
 
     /**
@@ -446,7 +455,8 @@ public class InternalComponentManager extends BasicModule implements ComponentMa
         iq.setChildElement("query", "http://jabber.org/protocol/disco#info");
         // Send the disco#info request to the component. The reply (if any) will be processed in
         // #process(Packet)
-        sendPacket(component, iq);
+//        sendPacket(component, iq);
+        component.processPacket(iq);
     }
 
     public JID getAddress() {
@@ -468,39 +478,38 @@ public class InternalComponentManager extends BasicModule implements ComponentMa
             if (packet instanceof IQ && IQ.Type.result == ((IQ) packet).getType()) {
                 IQ iq = (IQ) packet;
                 Element childElement = iq.getChildElement();
-                String namespace = null;
                 if (childElement != null) {
-                    namespace = childElement.getNamespaceURI();
-                }
-                if ("http://jabber.org/protocol/disco#info".equals(namespace)) {
-                    // Add a disco item to the server for the component that supports disco
-                    Element identity = childElement.element("identity");
-                    if (identity == null) {
-                        // Do nothing since there are no identities in the disco#info packet
-                        return;
-                    }
-                    try {
-                        XMPPServer.getInstance().getIQDiscoItemsHandler().addComponentItem(packet.getFrom()
-                                .toBareJID(),
-                                identity.attributeValue("name"));
-                        if (component instanceof ComponentSession.ExternalComponent) {
-                            ComponentSession.ExternalComponent externalComponent =
-                                    (ComponentSession.ExternalComponent) component;
-                            externalComponent.setName(identity.attributeValue("name"));
-                            externalComponent.setType(identity.attributeValue("type"));
-                            externalComponent.setCategory(identity.attributeValue("category"));
+                    String namespace = childElement.getNamespaceURI();
+                    if ("http://jabber.org/protocol/disco#info".equals(namespace)) {
+                        // Add a disco item to the server for the component that supports disco
+                        Element identity = childElement.element("identity");
+                        if (identity == null) {
+                            // Do nothing since there are no identities in the disco#info packet
+                            return;
+                        }
+                        try {
+                            XMPPServer.getInstance().getIQDiscoItemsHandler().addComponentItem(packet.getFrom()
+                                    .toBareJID(),
+                                    identity.attributeValue("name"));
+                            if (component instanceof ComponentSession.ExternalComponent) {
+                                ComponentSession.ExternalComponent externalComponent =
+                                        (ComponentSession.ExternalComponent) component;
+                                externalComponent.setName(identity.attributeValue("name"));
+                                externalComponent.setType(identity.attributeValue("type"));
+                                externalComponent.setCategory(identity.attributeValue("category"));
+                            }
+                        }
+                        catch (Exception e) {
+                            Log.error("Error processing disco packet of component: " + component +
+                                    " - " + packet.toXML(), e);
+                        }
+                        // Store the IQ disco#info returned by the component
+                        String subdomain = packet.getFrom().getDomain().replace("." + serverDomain, "");
+                        componentInfo.put(subdomain, iq);
+                        // Notify listeners that a component answered the disco#info request
+                        for (ComponentEventListener listener : listeners) {
+                            listener.componentInfoReceived(component, iq);
                         }
-                    }
-                    catch (Exception e) {
-                        Log.error("Error processing disco packet of component: " + component +
-                                " - " + packet.toXML(), e);
-                    }
-                    // Store the IQ disco#info returned by the component
-                    String subdomain = packet.getFrom().getDomain().replace("." + serverDomain, "");
-                    componentInfo.put(subdomain, iq);
-                    // Notify listeners that a component answered the disco#info request
-                    for (ComponentEventListener listener : listeners) {
-                        listener.componentInfoReceived(component, iq);
                     }
                 }
             }
@@ -510,12 +519,12 @@ public class InternalComponentManager extends BasicModule implements ComponentMa
     /**
      * Exposes a Component as a RoutableChannelHandler.
      */
-    public static class RoutableComponent implements RoutableChannelHandler {
+    private static class RoutableComponents implements RoutableChannelHandler {
 
         private JID jid;
         final private List<Component> components;
 
-        public RoutableComponent(JID jid, Component component) {
+        public RoutableComponents(JID jid, Component component) {
             this.jid = jid;
             this.components = new ArrayList<Component>();
             addComponent(component);

src/java/org/jivesoftware/openfire/net/ComponentSocketReader.java

@@ -111,11 +111,11 @@ public class ComponentSocketReader extends SocketReader {
         return false;
     }
 
-    boolean createSession(String namespace, Boolean allowMultiple) throws UnauthorizedException, XmlPullParserException,
+    boolean createSession(String namespace) throws UnauthorizedException, XmlPullParserException,
             IOException {
         if ("jabber:component:accept".equals(namespace)) {
             // The connected client is a component so create a ComponentSession
-            session = LocalComponentSession.createSession(serverName, reader, connection, allowMultiple);
+            session = LocalComponentSession.createSession(serverName, reader, connection);
             return true;
         }
         return false;

src/java/org/jivesoftware/openfire/net/ServerSocketReader.java

@@ -204,9 +204,8 @@ public class ServerSocketReader extends SocketReader {
         threadPool.shutdown();
     }
 
-    boolean createSession(String namespace, Boolean allowMultiple) throws UnauthorizedException, XmlPullParserException,
+    boolean createSession(String namespace) throws UnauthorizedException, XmlPullParserException,
             IOException {
-        // TODO: Should we ever consider allowing multiple of this?
         if ("jabber:server".equals(namespace)) {
             // The connected client is a server so create an IncomingServerSession
             session = LocalIncomingServerSession.createSession(serverName, reader, connection);

src/java/org/jivesoftware/openfire/net/SocketReader.java

@@ -231,6 +231,7 @@ public abstract class SocketReader implements Runnable {
      * another thread.
      *
      * @param packet the received packet.
+     * @throws UnauthorizedException if the connection required security but was not secured.
      */
     protected void processIQ(IQ packet) throws UnauthorizedException {
         // Ensure that connection was secured if TLS was required
@@ -253,6 +254,7 @@ public abstract class SocketReader implements Runnable {
      * another thread.
      *
      * @param packet the received packet.
+     * @throws UnauthorizedException if the connection required security but was not secured.
      */
     protected void processPresence(Presence packet) throws UnauthorizedException {
         // Ensure that connection was secured if TLS was required
@@ -275,6 +277,7 @@ public abstract class SocketReader implements Runnable {
      * another thread.
      *
      * @param packet the received packet.
+     * @throws UnauthorizedException if the connection required security but was not secured.
      */
     protected void processMessage(Message packet) throws UnauthorizedException {
         // Ensure that connection was secured if TLS was required
@@ -348,6 +351,10 @@ public abstract class SocketReader implements Runnable {
      * If the connection remains open, the XPP will be set to be ready for the
      * first packet. A call to next() should result in an START_TAG state with
      * the first packet in the stream.
+     *
+     * @throws UnauthorizedException if the connection required security but was not secured.
+     * @throws XmlPullParserException if there was an XML error while creating the session.
+     * @throws IOException if an IO error occured while creating the session.
      */
     protected void createSession()
             throws UnauthorizedException, XmlPullParserException, IOException {
@@ -360,7 +367,6 @@ public abstract class SocketReader implements Runnable {
         // subdomain. If the value of the 'to' attribute is not valid then return a host-unknown
         // error and close the underlying connection.
         String host = reader.getXPPParser().getAttributeValue("", "to");
-        String allowMultiple = reader.getXPPParser().getAttributeValue("", "allowMultiple");
         if (validateHost() && isHostUnknown(host)) {
             StringBuilder sb = new StringBuilder(250);
             sb.append("<?xml version='1.0' encoding='");
@@ -388,7 +394,7 @@ public abstract class SocketReader implements Runnable {
         // Create the correct session based on the sent namespace. At this point the server
         // may offer the client to secure the connection. If the client decides to secure
         // the connection then a <starttls> stanza should be received
-        else if (!createSession(xpp.getNamespace(null), allowMultiple != null)) {
+        else if (!createSession(xpp.getNamespace(null))) {
             // No session was created because of an invalid namespace prefix so answer a stream
             // error and close the underlying connection
             StringBuilder sb = new StringBuilder(250);
@@ -457,12 +463,11 @@ public abstract class SocketReader implements Runnable {
      * Creates the appropriate {@link org.jivesoftware.openfire.session.Session} subclass based on the specified namespace.
      *
      * @param namespace the namespace sent in the stream element. eg. jabber:client.
-     * @param allowMultiple Allow multiple bindings to the specified domain.
      * @return the created session or null.
-     * @throws UnauthorizedException
-     * @throws XmlPullParserException
-     * @throws IOException
+     * @throws UnauthorizedException if the connection required security but was not secured.
+     * @throws XmlPullParserException if there was an XML error while creating the session.
+     * @throws IOException if an IO error occured while creating the session.
      */
-    abstract boolean createSession(String namespace, Boolean allowMultiple) throws UnauthorizedException,
+    abstract boolean createSession(String namespace) throws UnauthorizedException,
             XmlPullParserException, IOException;
 }

src/java/org/jivesoftware/openfire/session/LocalComponentSession.java

@@ -55,15 +55,18 @@ public class LocalComponentSession extends LocalSession implements ComponentSess
      * @param serverName the name of the server where the session is connecting to.
      * @param reader     the reader that is reading the provided XML through the connection.
      * @param connection the connection with the component.
-     * @param allowMultiple     the specified domain is allowed to be connected to multiple times.
      * @return a newly created session between the server and a component.
+     * @throws UnauthorizedException if the connection required security but was not secured.
+     * @throws XmlPullParserException if there was an XML error while creating the session.
+     * @throws IOException if an IO error occured while creating the session.
      */
     public static LocalComponentSession createSession(String serverName, XMPPPacketReader reader,
-            SocketConnection connection, Boolean allowMultiple) throws UnauthorizedException, IOException,
+            SocketConnection connection) throws UnauthorizedException, IOException,
             XmlPullParserException
     {
         XmlPullParser xpp = reader.getXPPParser();
         String domain = xpp.getAttributeValue("", "to");
+        Boolean allowMultiple = reader.getXPPParser().getAttributeValue("", "allowMultiple") != null;
 
         Log.debug("LocalComponentSession: [ExComp] Starting registration of new external component for domain: " + domain);