OF-2655: Refactor previous fix

This refactors the previous fix to an extend that it simply re-uses pre-existing API.
surevine · Sep 4, 2023 · cb76a12 · cb76a12
1 parent 1ebf404
commit cb76a12
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 40 deletions.
diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/RoutingTable.java b/xmppserver/src/main/java/org/jivesoftware/openfire/RoutingTable.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004-2008 Jive Software. All rights reserved.
+ * Copyright (C) 2004-2008 Jive Software, 2023 Ignite Realtime Foundation. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -80,9 +80,12 @@
 public interface RoutingTable {
 
     /**
-     * Adds a route to the routing table for the specified outgoing server session. When running
-     * inside of a cluster this message {@code must} be sent from the cluster node that is
-     * actually holding the physical connection to the remote server.
+     * Adds a route to the routing table for the specified outgoing server session, or replaces a pre-existing one.
+     *
+     * When running inside a cluster, this method <em>must</em> be invoked on the cluster node that is actually holding
+     * the physical connection to the remote server. Additionally, replacing a pre-existing server session can only
+     * occur on the same cluster node as the one that was holding the original session. A runtime exception is thrown
+     * when another cluster node attempts to replace the session.
      *
      * @param route the address associated to the route.
      * @param destination the outgoing server session.
@@ -357,13 +360,4 @@ public interface RoutingTable {
      * @param onlyLocal true if only client sessions connect to the local JVM will get the message.
      */
     void broadcastPacket(Message packet, boolean onlyLocal);
-
-
-    /**
-     * Replaces all previous sessions associated with the domain pair with a new session.
-     *
-     * @param domainPair the address associated to the route.
-     * @param session the outgoing server session
-     */
-    void replaceServerRoute(DomainPair domainPair, LocalOutgoingServerSession session);
 }
diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/RespondingServerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/RespondingServerStanzaHandler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2008 Jive Software, 2023 Ignite Realtime Foundation. All rights reserved.
+ * Copyright (C) 2023 Ignite Realtime Foundation. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -102,7 +102,7 @@ private void transferConnectionToNewSession(String newStreamId, ServerSession.Au
 
         // Clear routing table cache of previous sessions for domain pair and re-add it.
         RoutingTable routingTable = XMPPServer.getInstance().getRoutingTable();
-        routingTable.replaceServerRoute(domainPair, (LocalOutgoingServerSession) session);
+        routingTable.addServerRoute(domainPair, (LocalOutgoingServerSession) session); // This _replaces_ the pre-existing session in the routing table.
         SessionManager sessionManager = SessionManager.getInstance();
         sessionManager.outgoingServerSessionCreated((LocalOutgoingServerSession) session);
 

diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/spi/RoutingTableImpl.java b/xmppserver/src/main/java/org/jivesoftware/openfire/spi/RoutingTableImpl.java
@@ -227,38 +227,18 @@ public void addServerRoute(DomainPair address, LocalOutgoingServerSession destin
         Lock lock = serversCache.getLock(address);
         lock.lock();
         try {
-            checkOnlyOneClusterNodeConnectedToAddress(address);
+            final NodeID oldValue = serversCache.putIfAbsent(address, server.getNodeID());
+            if (oldValue != null && !oldValue.equals(XMPPServer.getInstance().getNodeID())) {
+                // Existing implementation assumes that only one node has an outgoing server connection for a domain. Fail if that's not the case. See: OF-2280
+                throw new IllegalStateException("The local cluster node attempts to established a new S2S connection to '"+ address +"', but such a connection already exists on cluster node '"+oldValue+"'.");
+            }
         }
         finally {
             lock.unlock();
         }
         localRoutingTable.addRoute(address, destination);
     }
 
-    private void checkOnlyOneClusterNodeConnectedToAddress(DomainPair address) {
-        final NodeID oldValue = serversCache.putIfAbsent(address, server.getNodeID());
-        if (oldValue != null && !oldValue.equals(XMPPServer.getInstance().getNodeID())) {
-            // Existing implementation assumes that only one node has an outgoing server connection for a domain. Fail if that's not the case. See: OF-2280
-            throw new IllegalStateException("The local cluster node attempts to established a new S2S connection to '"+ address +"', but such a connection already exists on cluster node '"+oldValue+"'.");
-        }
-    }
-
-
-    @Override
-    public void replaceServerRoute(DomainPair domainPair, LocalOutgoingServerSession session) {
-        Lock lock = serversCache.getLock(domainPair);
-        lock.lock();
-        try {
-            checkOnlyOneClusterNodeConnectedToAddress(domainPair);
-            serversCache.remove(domainPair);
-            localRoutingTable.removeRoute(domainPair);
-            localRoutingTable.addRoute(domainPair, session);
-        }
-        finally {
-            lock.unlock();
-        }
-    }
-
     @Override
     public void addComponentRoute(JID route, RoutableChannelHandler destination) {
         DomainPair pair = new DomainPair("", route.getDomain());