From 3a4b05ac00806fa819fe25e7244a1ba7ea1d6017 Mon Sep 17 00:00:00 2001 From: rcollier Date: Tue, 16 Apr 2013 01:39:17 +0000 Subject: [PATCH] SMACK-412 Abstracted the keepalive implementation and set the thread to start and stop on demand. git-svn-id: http://svn.igniterealtime.org/svn/repos/smack/branches/smack_3_3_0@13610 b35dd754-fafc-0310-a699-88a17e54d16e --- build/resources/META-INF/smack-config.xml | 4 +- .../KeepAliveManager.java} | 158 ++++++++++-------- .../jivesoftware/smackx/ping/PingManager.java | 67 +++++++- .../jivesoftware/smack/DummyConnection.java | 12 ++ .../{ping => keepalive}/KeepaliveTest.java | 36 +--- .../jivesoftware/smackx/ping/PingTest.java | 45 ++++- 6 files changed, 213 insertions(+), 109 deletions(-) rename source/org/jivesoftware/smack/{ping/ServerPingManager.java => keepalive/KeepAliveManager.java} (71%) rename test-unit/org/jivesoftware/smack/{ping => keepalive}/KeepaliveTest.java (79%) diff --git a/build/resources/META-INF/smack-config.xml b/build/resources/META-INF/smack-config.xml index 52421c703..a4df4764c 100644 --- a/build/resources/META-INF/smack-config.xml +++ b/build/resources/META-INF/smack-config.xml @@ -24,7 +24,7 @@ org.jivesoftware.smackx.ServiceDiscoveryManager org.jivesoftware.smack.PrivacyListManager - org.jivesoftware.smack.ping.ServerPingManager + org.jivesoftware.smack.keepalive.KeepAliveManager org.jivesoftware.smackx.XHTMLManager org.jivesoftware.smackx.muc.MultiUserChat org.jivesoftware.smackx.bytestreams.ibb.InBandBytestreamManager @@ -34,7 +34,7 @@ org.jivesoftware.smack.ReconnectionManager org.jivesoftware.smackx.commands.AdHocCommandManager org.jivesoftware.smack.util.dns.JavaxResolver + org.jivesoftware.smackx.ping.PingManager - diff --git a/source/org/jivesoftware/smack/ping/ServerPingManager.java b/source/org/jivesoftware/smack/keepalive/KeepAliveManager.java similarity index 71% rename from source/org/jivesoftware/smack/ping/ServerPingManager.java rename to source/org/jivesoftware/smack/keepalive/KeepAliveManager.java index b9525d7f1..99e1acfba 100644 --- a/source/org/jivesoftware/smack/ping/ServerPingManager.java +++ b/source/org/jivesoftware/smack/keepalive/KeepAliveManager.java @@ -14,13 +14,14 @@ * limitations under the License. */ -package org.jivesoftware.smack.ping; +package org.jivesoftware.smack.keepalive; import java.util.Collections; +import java.util.Date; +import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.WeakHashMap; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; @@ -33,17 +34,11 @@ import org.jivesoftware.smack.ConnectionListener; import org.jivesoftware.smack.PacketCollector; import org.jivesoftware.smack.PacketListener; import org.jivesoftware.smack.SmackConfiguration; -import org.jivesoftware.smack.XMPPException; -import org.jivesoftware.smack.filter.AndFilter; -import org.jivesoftware.smack.filter.IQTypeFilter; import org.jivesoftware.smack.filter.PacketFilter; import org.jivesoftware.smack.filter.PacketIDFilter; -import org.jivesoftware.smack.filter.PacketTypeFilter; -import org.jivesoftware.smack.packet.IQ; -import org.jivesoftware.smack.packet.IQ.Type; import org.jivesoftware.smack.packet.Packet; +import org.jivesoftware.smack.ping.PingFailedListener; import org.jivesoftware.smack.ping.packet.Ping; -import org.jivesoftware.smackx.ServiceDiscoveryManager; /** * Using an implementation of XMPP Ping (XEP-0199). This @@ -56,25 +51,15 @@ import org.jivesoftware.smackx.ServiceDiscoveryManager; * * @author Florian Schmaus */ -public class ServerPingManager { - private static Map instances = Collections - .synchronizedMap(new WeakHashMap()); - private static long defaultPingInterval = SmackConfiguration.getKeepAliveInterval(); +public class KeepAliveManager { + private static Map instances = new HashMap(); + private static volatile ScheduledExecutorService periodicPingExecutorService; - private static ScheduledExecutorService periodicPingExecutorService = new ScheduledThreadPoolExecutor(1, new ThreadFactory() { - @Override - public Thread newThread(Runnable runnable) { - Thread pingThread = new Thread(runnable, "Smack Server Ping"); - pingThread.setDaemon(true); - return pingThread; - } - }); - static { - if (defaultPingInterval > 0) { + if (SmackConfiguration.getKeepAliveInterval() > 0) { Connection.addConnectionCreationListener(new ConnectionCreationListener() { public void connectionCreated(Connection connection) { - new ServerPingManager(connection); + new KeepAliveManager(connection); } }); } @@ -87,56 +72,94 @@ public class ServerPingManager { private volatile long lastSuccessfulContact = -1; /** - * Retrieves a {@link ServerPingManager} for the specified {@link Connection}, creating one if it doesn't already + * Retrieves a {@link KeepAliveManager} for the specified {@link Connection}, creating one if it doesn't already * exist. * * @param connection * The connection the manager is attached to. * @return The new or existing manager. */ - public synchronized static ServerPingManager getInstanceFor(Connection connection) { - ServerPingManager pingManager = instances.get(connection); + public synchronized static KeepAliveManager getInstanceFor(Connection connection) { + KeepAliveManager pingManager = instances.get(connection); if (pingManager == null) { - pingManager = new ServerPingManager(connection); + pingManager = new KeepAliveManager(connection); + instances.put(connection, pingManager); } return pingManager; } - private ServerPingManager(Connection connection) { - ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(connection); - sdm.addFeature(Ping.NAMESPACE); + /* + * Start the executor service if it hasn't been started yet. + */ + private synchronized static void enableExecutorService() { + if (periodicPingExecutorService == null) { + periodicPingExecutorService = new ScheduledThreadPoolExecutor(1, new ThreadFactory() { + @Override + public Thread newThread(Runnable runnable) { + Thread pingThread = new Thread(runnable, "Smack Keepalive"); + pingThread.setDaemon(true); + return pingThread; + } + }); + } + } + + /* + * Stop the executor service if all monitored connections are disconnected. + */ + private synchronized static void handleDisconnect(Connection con) { + if (periodicPingExecutorService != null) { + instances.remove(con); + + if (instances.isEmpty()) { + periodicPingExecutorService.shutdownNow(); + periodicPingExecutorService = null; + } + } + } + + private KeepAliveManager(Connection connection) { this.connection = connection; init(); + handleConnect(); + } + + /* + * Call after every connection to add the packet listener. + */ + private void handleConnect() { + // Listen for all incoming packets and reset the scheduled ping whenever + // one arrives. + connection.addPacketListener(new PacketListener() { + + @Override + public void processPacket(Packet packet) { + // reschedule the ping based on this last server contact + lastSuccessfulContact = System.currentTimeMillis(); + schedulePingServerTask(); + } + }, null); } private void init() { - PacketFilter pingPacketFilter = new AndFilter(new PacketTypeFilter(Ping.class), new IQTypeFilter(Type.GET)); - - connection.addPacketListener(new PacketListener() { - /** - * Sends a Pong for every Ping - */ - public void processPacket(Packet packet) { - IQ pong = IQ.createResultIQ((Ping) packet); - connection.sendPacket(pong); - } - }, pingPacketFilter); - connection.addConnectionListener(new ConnectionListener() { @Override public void connectionClosed() { stopPingServerTask(); + handleDisconnect(connection); } @Override public void connectionClosedOnError(Exception arg0) { stopPingServerTask(); + handleDisconnect(connection); } @Override public void reconnectionSuccessful() { + handleConnect(); schedulePingServerTask(); } @@ -149,17 +172,6 @@ public class ServerPingManager { } }); - // Listen for all incoming packets and reset the scheduled ping whenever - // one arrives. - connection.addPacketListener(new PacketListener() { - - @Override - public void processPacket(Packet packet) { - // reschedule the ping based on this last server contact - lastSuccessfulContact = System.currentTimeMillis(); - schedulePingServerTask(); - } - }, null); instances.put(connection, this); schedulePingServerTask(); } @@ -171,15 +183,20 @@ public class ServerPingManager { * The new ping time interval in milliseconds. */ public void setPingInterval(long newPingInterval) { - if (pingInterval != newPingInterval) { - pingInterval = newPingInterval; + if (pingInterval == newPingInterval) + return; + + // Enable the executor service + if (newPingInterval > 0) + enableExecutorService(); + + pingInterval = newPingInterval; - if (pingInterval < 0) { - stopPinging(); - } - else { - schedulePingServerTask(); - } + if (pingInterval < 0) { + stopPinging(); + } + else { + schedulePingServerTask(); } } @@ -227,12 +244,20 @@ public class ServerPingManager { } /** - * Returns the time of the last successful contact with the server. (i.e. the last time any message was received). + * Returns the elapsed time (in milliseconds) since the last successful contact with the server + * (i.e. the last time any message was received). + *

+ * Note: Result is -1 if no message has been received since manager was created and + * 0 if the elapsed time is negative due to a clock reset. * - * @return Time of last message or -1 if none has been received since manager was created. + * @return Elapsed time since last message was received. */ - public long getLastSuccessfulContact() { - return lastSuccessfulContact; + public long getTimeSinceLastContact() { + if (lastSuccessfulContact == -1) + return lastSuccessfulContact; + long delta = System.currentTimeMillis() - lastSuccessfulContact; + + return (delta < 0) ? 0 : delta; } /** @@ -242,6 +267,7 @@ public class ServerPingManager { * This is designed so only one executor is used for scheduling all pings on all connections. This results in only 1 thread used for pinging. */ private synchronized void schedulePingServerTask() { + enableExecutorService(); stopPingServerTask(); if (pingInterval > 0) { diff --git a/source/org/jivesoftware/smackx/ping/PingManager.java b/source/org/jivesoftware/smackx/ping/PingManager.java index 9ab6f026e..2da9dcadc 100644 --- a/source/org/jivesoftware/smackx/ping/PingManager.java +++ b/source/org/jivesoftware/smackx/ping/PingManager.java @@ -16,11 +16,25 @@ package org.jivesoftware.smackx.ping; +import java.util.Collections; +import java.util.Map; +import java.util.WeakHashMap; +import java.util.concurrent.ScheduledExecutorService; + import org.jivesoftware.smack.Connection; +import org.jivesoftware.smack.ConnectionCreationListener; +import org.jivesoftware.smack.PacketListener; import org.jivesoftware.smack.SmackConfiguration; import org.jivesoftware.smack.SmackError; import org.jivesoftware.smack.XMPPException; -import org.jivesoftware.smack.ping.ServerPingManager; +import org.jivesoftware.smack.filter.AndFilter; +import org.jivesoftware.smack.filter.IQTypeFilter; +import org.jivesoftware.smack.filter.PacketFilter; +import org.jivesoftware.smack.filter.PacketTypeFilter; +import org.jivesoftware.smack.keepalive.KeepAliveManager; +import org.jivesoftware.smack.packet.IQ; +import org.jivesoftware.smack.packet.Packet; +import org.jivesoftware.smack.packet.IQ.Type; import org.jivesoftware.smack.ping.packet.Ping; import org.jivesoftware.smack.util.SyncPacketSend; import org.jivesoftware.smackx.ServiceDiscoveryManager; @@ -31,7 +45,7 @@ import org.jivesoftware.smackx.packet.DiscoverInfo; * allows one entity to 'ping' any other entity by simply sending a ping to * the appropriate JID. *

- * NOTE: The {@link ServerPingManager} already provides a keepalive functionality + * NOTE: The {@link KeepAliveManager} already provides a keepalive functionality * for regularly pinging the server to keep the underlying transport connection * alive. This class is specifically intended to do manual pings of other * entities. @@ -41,12 +55,57 @@ import org.jivesoftware.smackx.packet.DiscoverInfo; * Ping */ public class PingManager { + private static Map instances = Collections + .synchronizedMap(new WeakHashMap()); + + static { + Connection.addConnectionCreationListener(new ConnectionCreationListener() { + public void connectionCreated(Connection connection) { + new PingManager(connection); + } + }); + } + private Connection connection; - public PingManager(Connection connection) { + /** + * Retrieves a {@link PingManager} for the specified {@link Connection}, creating one if it doesn't already + * exist. + * + * @param connection + * The connection the manager is attached to. + * @return The new or existing manager. + */ + public synchronized static PingManager getInstanceFor(Connection connection) { + PingManager pingManager = instances.get(connection); + + if (pingManager == null) { + pingManager = new PingManager(connection); + } + return pingManager; + } + + private PingManager(Connection con) { + this.connection = con; ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(connection); + + // The ServiceDiscoveryManager was not pre-initialized + if (sdm == null) + sdm = new ServiceDiscoveryManager(connection); + sdm.addFeature(Ping.NAMESPACE); - this.connection = connection; + + PacketFilter pingPacketFilter = new AndFilter(new PacketTypeFilter(Ping.class), new IQTypeFilter(Type.GET)); + + connection.addPacketListener(new PacketListener() { + /** + * Sends a Pong for every Ping + */ + public void processPacket(Packet packet) { + IQ pong = IQ.createResultIQ((Ping) packet); + connection.sendPacket(pong); + } + }, pingPacketFilter); } /** diff --git a/test-unit/org/jivesoftware/smack/DummyConnection.java b/test-unit/org/jivesoftware/smack/DummyConnection.java index c144eff5a..57753a1ba 100644 --- a/test-unit/org/jivesoftware/smack/DummyConnection.java +++ b/test-unit/org/jivesoftware/smack/DummyConnection.java @@ -49,6 +49,7 @@ public class DummyConnection extends Connection { private boolean authenticated = false; private boolean anonymous = false; + private boolean reconnect = false; private String user; private String connectionID; @@ -71,6 +72,12 @@ public class DummyConnection extends Connection { @Override public void connect() throws XMPPException { connectionID = "dummy-" + new Random(new Date().getTime()).nextInt(); + + if (reconnect) { + for (ConnectionListener listener : getConnectionListeners()) { + listener.reconnectionSuccessful(); + } + } } @Override @@ -80,6 +87,11 @@ public class DummyConnection extends Connection { roster = null; authenticated = false; anonymous = false; + + for (ConnectionListener listener : getConnectionListeners()) { + listener.connectionClosed(); + } + reconnect = true; } @Override diff --git a/test-unit/org/jivesoftware/smack/ping/KeepaliveTest.java b/test-unit/org/jivesoftware/smack/keepalive/KeepaliveTest.java similarity index 79% rename from test-unit/org/jivesoftware/smack/ping/KeepaliveTest.java rename to test-unit/org/jivesoftware/smack/keepalive/KeepaliveTest.java index 25927d4e8..937bcebbb 100644 --- a/test-unit/org/jivesoftware/smack/ping/KeepaliveTest.java +++ b/test-unit/org/jivesoftware/smack/keepalive/KeepaliveTest.java @@ -1,4 +1,4 @@ -package org.jivesoftware.smack.ping; +package org.jivesoftware.smack.keepalive; import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual; import static org.junit.Assert.assertEquals; @@ -17,8 +17,10 @@ import org.jivesoftware.smack.TestUtils; import org.jivesoftware.smack.ThreadedDummyConnection; import org.jivesoftware.smack.filter.IQTypeFilter; import org.jivesoftware.smack.filter.PacketTypeFilter; +import org.jivesoftware.smack.keepalive.KeepAliveManager; import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.Packet; +import org.jivesoftware.smack.ping.PingFailedListener; import org.jivesoftware.smack.ping.packet.Ping; import org.jivesoftware.smack.util.PacketParserUtils; import org.junit.After; @@ -40,6 +42,7 @@ public class KeepaliveTest { @Before public void resetProperties() { + SmackConfiguration.setKeepAliveInterval(-1); originalTimeout = SmackConfiguration.getPacketReplyTimeout(); SmackConfiguration.setPacketReplyTimeout(1000); } @@ -56,7 +59,7 @@ public class KeepaliveTest { public void validatePingStanzaXML() throws Exception { // @formatter:off String control = "" - + "" + ""; + + ""; // @formatter:on Ping ping = new Ping(TO); @@ -65,29 +68,6 @@ public class KeepaliveTest { assertXMLEqual(control, ping.toXML()); } - @Test - public void checkProvider() throws Exception { - // @formatter:off - String control = "" - + "" + ""; - // @formatter:on - DummyConnection con = new DummyConnection(); - IQ pingRequest = PacketParserUtils.parseIQ(TestUtils.getIQParser(control), con); - - assertTrue(pingRequest instanceof Ping); - - con.processPacket(pingRequest); - - Packet pongPacket = con.getSentPacket(); - assertTrue(pongPacket instanceof IQ); - - IQ pong = (IQ) pongPacket; - assertEquals("juliet@capulet.lit/balcony", pong.getFrom()); - assertEquals("capulet.lit", pong.getTo()); - assertEquals("s2c1", pong.getPacketID()); - assertEquals(IQ.Type.RESULT, pong.getType()); - } - @Test public void serverPingFailSingleConnection() throws Exception { DummyConnection connection = getConnection(); @@ -138,7 +118,7 @@ public class KeepaliveTest { } private void addPingFailedListener(DummyConnection con, final CountDownLatch latch) { - ServerPingManager manager = ServerPingManager.getInstanceFor(con); + KeepAliveManager manager = KeepAliveManager.getInstanceFor(con); manager.addPingFailedListener(new PingFailedListener() { @Override public void pingFailed() { @@ -149,7 +129,7 @@ public class KeepaliveTest { private DummyConnection getConnection() { DummyConnection con = new DummyConnection(); - ServerPingManager mgr = ServerPingManager.getInstanceFor(con); + KeepAliveManager mgr = KeepAliveManager.getInstanceFor(con); mgr.setPingInterval(PING_MINIMUM); return con; @@ -157,7 +137,7 @@ public class KeepaliveTest { private ThreadedDummyConnection getThreadedConnection() { ThreadedDummyConnection con = new ThreadedDummyConnection(); - ServerPingManager mgr = ServerPingManager.getInstanceFor(con); + KeepAliveManager mgr = KeepAliveManager.getInstanceFor(con); mgr.setPingInterval(PING_MINIMUM); return con; diff --git a/test-unit/org/jivesoftware/smackx/ping/PingTest.java b/test-unit/org/jivesoftware/smackx/ping/PingTest.java index b7b6358b8..c127f18dc 100644 --- a/test-unit/org/jivesoftware/smackx/ping/PingTest.java +++ b/test-unit/org/jivesoftware/smackx/ping/PingTest.java @@ -38,10 +38,37 @@ public class PingTest { threadedCon = new ThreadedDummyConnection(); } + @Test + public void checkProvider() throws Exception { + // @formatter:off + String control = "" + + "" + + ""; + // @formatter:on + DummyConnection con = new DummyConnection(); + + // Enable ping for this connection + PingManager.getInstanceFor(con); + IQ pingRequest = PacketParserUtils.parseIQ(TestUtils.getIQParser(control), con); + + assertTrue(pingRequest instanceof Ping); + + con.processPacket(pingRequest); + + Packet pongPacket = con.getSentPacket(); + assertTrue(pongPacket instanceof IQ); + + IQ pong = (IQ) pongPacket; + assertEquals("juliet@capulet.lit/balcony", pong.getFrom()); + assertEquals("capulet.lit", pong.getTo()); + assertEquals("s2c1", pong.getPacketID()); + assertEquals(IQ.Type.RESULT, pong.getType()); + } + @Test public void checkSendingPing() throws Exception { dummyCon = new DummyConnection(); - PingManager pinger = new PingManager(dummyCon); + PingManager pinger = PingManager.getInstanceFor(dummyCon); pinger.ping("test@myserver.com"); Packet sentPacket = dummyCon.getSentPacket(); @@ -54,7 +81,7 @@ public class PingTest { public void checkSuccessfulPing() throws Exception { threadedCon = new ThreadedDummyConnection(); - PingManager pinger = new PingManager(threadedCon); + PingManager pinger = PingManager.getInstanceFor(threadedCon); boolean pingSuccess = pinger.ping("test@myserver.com"); @@ -69,7 +96,7 @@ public class PingTest { @Test public void checkFailedPingOnTimeout() throws Exception { dummyCon = new DummyConnection(); - PingManager pinger = new PingManager(dummyCon); + PingManager pinger = PingManager.getInstanceFor(dummyCon); boolean pingSuccess = pinger.ping("test@myserver.com"); @@ -96,7 +123,7 @@ public class PingTest { IQ serviceUnavailable = PacketParserUtils.parseIQ(TestUtils.getIQParser(reply), threadedCon); threadedCon.addIQReply(serviceUnavailable); - PingManager pinger = new PingManager(threadedCon); + PingManager pinger = PingManager.getInstanceFor(threadedCon); boolean pingSuccess = pinger.ping("test@myserver.com"); @@ -106,7 +133,7 @@ public class PingTest { @Test public void checkPingToServerSuccess() throws Exception { ThreadedDummyConnection con = new ThreadedDummyConnection(); - PingManager pinger = new PingManager(con); + PingManager pinger = PingManager.getInstanceFor(con); boolean pingSuccess = pinger.pingMyServer(); @@ -132,7 +159,7 @@ public class PingTest { IQ serviceUnavailable = PacketParserUtils.parseIQ(TestUtils.getIQParser(reply), con); con.addIQReply(serviceUnavailable); - PingManager pinger = new PingManager(con); + PingManager pinger = PingManager.getInstanceFor(con); boolean pingSuccess = pinger.pingMyServer(); @@ -142,7 +169,7 @@ public class PingTest { @Test public void checkPingToServerTimeout() throws Exception { DummyConnection con = new DummyConnection(); - PingManager pinger = new PingManager(con); + PingManager pinger = PingManager.getInstanceFor(con); boolean pingSuccess = pinger.pingMyServer(); @@ -165,7 +192,7 @@ public class PingTest { IQ discoReply = PacketParserUtils.parseIQ(TestUtils.getIQParser(reply), con); con.addIQReply(discoReply); - PingManager pinger = new PingManager(con); + PingManager pinger = PingManager.getInstanceFor(con); boolean pingSupported = pinger.isPingSupported("test@myserver.com"); assertTrue(pingSupported); @@ -187,7 +214,7 @@ public class PingTest { IQ discoReply = PacketParserUtils.parseIQ(TestUtils.getIQParser(reply), con); con.addIQReply(discoReply); - PingManager pinger = new PingManager(con); + PingManager pinger = PingManager.getInstanceFor(con); boolean pingSupported = pinger.isPingSupported("test@myserver.com"); assertFalse(pingSupported);