From a14178990ba9568326c5f5fd8a282e12fe4ea829 Mon Sep 17 00:00:00 2001 From: rcollier Date: Sat, 23 Mar 2013 11:59:08 +0000 Subject: [PATCH] SMACK-412 Added the pingMyServer back in, cleaned up unneeded synchronization and removed minimum ping interval. git-svn-id: http://svn.igniterealtime.org/svn/repos/smack/branches/smack_3_3_0@13588 b35dd754-fafc-0310-a699-88a17e54d16e --- .../org/jivesoftware/smack/PacketWriter.java | 18 ++-- source/org/jivesoftware/smack/SmackError.java | 24 ++++++ .../org/jivesoftware/smack/XMPPException.java | 26 +++++- .../smack/ping/ServerPingManager.java | 13 +-- .../smack/util/SyncPacketSend.java | 3 +- .../jivesoftware/smackx/ping/PingManager.java | 21 ++++- ...ServerPingTest.java => KeepaliveTest.java} | 27 ++++-- .../ping/{PingPongTest.java => PingTest.java} | 84 +++++++++++++++---- 8 files changed, 174 insertions(+), 42 deletions(-) create mode 100644 source/org/jivesoftware/smack/SmackError.java rename test-unit/org/jivesoftware/smack/ping/{ServerPingTest.java => KeepaliveTest.java} (87%) rename test-unit/org/jivesoftware/smackx/ping/{PingPongTest.java => PingTest.java} (69%) diff --git a/source/org/jivesoftware/smack/PacketWriter.java b/source/org/jivesoftware/smack/PacketWriter.java index 559e2afd9..1e30439c8 100644 --- a/source/org/jivesoftware/smack/PacketWriter.java +++ b/source/org/jivesoftware/smack/PacketWriter.java @@ -162,12 +162,10 @@ class PacketWriter { while (!done && (writerThread == thisThread)) { Packet packet = nextPacket(); if (packet != null) { - synchronized (writer) { - writer.write(packet.toXML()); + writer.write(packet.toXML()); + writer.flush(); + if (queue.isEmpty()) { writer.flush(); - if (queue.isEmpty()) { - writer.flush(); - } } } } @@ -175,13 +173,11 @@ class PacketWriter { // we won't have time to entirely flush it before the socket is forced closed // by the shutdown process. try { - synchronized (writer) { - while (!queue.isEmpty()) { - Packet packet = queue.remove(); - writer.write(packet.toXML()); - } - writer.flush(); + while (!queue.isEmpty()) { + Packet packet = queue.remove(); + writer.write(packet.toXML()); } + writer.flush(); } catch (Exception e) { e.printStackTrace(); diff --git a/source/org/jivesoftware/smack/SmackError.java b/source/org/jivesoftware/smack/SmackError.java new file mode 100644 index 000000000..af22e7c0e --- /dev/null +++ b/source/org/jivesoftware/smack/SmackError.java @@ -0,0 +1,24 @@ +package org.jivesoftware.smack; + +public enum SmackError { + NO_RESPONSE_FROM_SERVER("No response from server."); + + private String message; + + private SmackError(String errMessage) { + message = errMessage; + } + + public String getErrorMessage() { + return message; + } + + public static SmackError getErrorCode(String message) { + for (SmackError code : values()) { + if (code.message.equals(message)) { + return code; + } + } + return null; + } +} diff --git a/source/org/jivesoftware/smack/XMPPException.java b/source/org/jivesoftware/smack/XMPPException.java index 6da24c2b7..e6da0168c 100644 --- a/source/org/jivesoftware/smack/XMPPException.java +++ b/source/org/jivesoftware/smack/XMPPException.java @@ -41,10 +41,12 @@ import java.io.PrintWriter; * @author Matt Tucker */ public class XMPPException extends Exception { - + private static final long serialVersionUID = 6881651633890968625L; + private StreamError streamError = null; private XMPPError error = null; private Throwable wrappedThrowable = null; + private SmackError smackError = null; /** * Creates a new XMPPException. @@ -62,6 +64,16 @@ public class XMPPException extends Exception { super(message); } + /** + * Creates a new XMPPException with a Smack specific error code. + * + * @param code the root cause of the exception. + */ + public XMPPException(SmackError code) { + super(code.getErrorMessage()); + smackError = code; + } + /** * Creates a new XMPPException with the Throwable that was the root cause of the * exception. @@ -74,7 +86,7 @@ public class XMPPException extends Exception { } /** - * Cretaes a new XMPPException with the stream error that was the root case of the + * Creates a new XMPPException with the stream error that was the root case of the * exception. When a stream error is received from the server then the underlying * TCP connection will be closed by the server. * @@ -144,6 +156,16 @@ public class XMPPException extends Exception { return error; } + /** + * Returns the SmackError asscociated with this exception, or null if there + * isn't one. + * + * @return the SmackError asscociated with this exception. + */ + public SmackError getSmackError() { + return smackError; + } + /** * Returns the StreamError asscociated with this exception, or null if there * isn't one. The underlying TCP connection is closed by the server after sending the diff --git a/source/org/jivesoftware/smack/ping/ServerPingManager.java b/source/org/jivesoftware/smack/ping/ServerPingManager.java index 171710bdf..b9525d7f1 100644 --- a/source/org/jivesoftware/smack/ping/ServerPingManager.java +++ b/source/org/jivesoftware/smack/ping/ServerPingManager.java @@ -57,8 +57,6 @@ import org.jivesoftware.smackx.ServiceDiscoveryManager; * @author Florian Schmaus */ public class ServerPingManager { - public static final long PING_MINIMUM = 10000; - private static Map instances = Collections .synchronizedMap(new WeakHashMap()); private static long defaultPingInterval = SmackConfiguration.getKeepAliveInterval(); @@ -173,12 +171,15 @@ public class ServerPingManager { * The new ping time interval in milliseconds. */ public void setPingInterval(long newPingInterval) { - if (newPingInterval < PING_MINIMUM) - newPingInterval = PING_MINIMUM; - if (pingInterval != newPingInterval) { pingInterval = newPingInterval; - schedulePingServerTask(); + + if (pingInterval < 0) { + stopPinging(); + } + else { + schedulePingServerTask(); + } } } diff --git a/source/org/jivesoftware/smack/util/SyncPacketSend.java b/source/org/jivesoftware/smack/util/SyncPacketSend.java index d658637f0..6506cbd88 100644 --- a/source/org/jivesoftware/smack/util/SyncPacketSend.java +++ b/source/org/jivesoftware/smack/util/SyncPacketSend.java @@ -16,6 +16,7 @@ package org.jivesoftware.smack.util; import org.jivesoftware.smack.PacketCollector; import org.jivesoftware.smack.SmackConfiguration; import org.jivesoftware.smack.Connection; +import org.jivesoftware.smack.SmackError; import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.filter.PacketFilter; import org.jivesoftware.smack.filter.PacketIDFilter; @@ -47,7 +48,7 @@ final public class SyncPacketSend response.cancel(); if (result == null) { - throw new XMPPException("No response from " + packet.getTo()); + throw new XMPPException(SmackError.NO_RESPONSE_FROM_SERVER); } else if (result.getError() != null) { throw new XMPPException(result.getError()); diff --git a/source/org/jivesoftware/smackx/ping/PingManager.java b/source/org/jivesoftware/smackx/ping/PingManager.java index 7e7e9d2cf..9ab6f026e 100644 --- a/source/org/jivesoftware/smackx/ping/PingManager.java +++ b/source/org/jivesoftware/smackx/ping/PingManager.java @@ -18,6 +18,7 @@ package org.jivesoftware.smackx.ping; import org.jivesoftware.smack.Connection; 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.ping.packet.Ping; @@ -49,7 +50,9 @@ public class PingManager { } /** - * Pings the given jid. This method will return false if an error occurs. + * Pings the given jid. This method will return false if an error occurs. The exception + * to this, is a server ping, which will always return true if the server is reachable, + * event if there is an error on the ping itself (i.e. ping not supported). *

* Use {@link #isPingSupported(String)} to determine if XMPP Ping is supported * by the entity. @@ -65,7 +68,8 @@ public class PingManager { SyncPacketSend.getReply(connection, ping); } catch (XMPPException exc) { - return false; + + return (jid.equals(connection.getServiceName()) && (exc.getSmackError() != SmackError.NO_RESPONSE_FROM_SERVER)); } return true; } @@ -92,4 +96,17 @@ public class PingManager { DiscoverInfo result = ServiceDiscoveryManager.getInstanceFor(connection).discoverInfo(jid); return result.containsFeature(Ping.NAMESPACE); } + + /** + * Pings the server. This method will return true if the server is reachable. It + * is the equivalent of calling ping with the XMPP domain. + *

+ * Unlike the {@link #ping(String)} case, this method will return true even if + * {@link #isPingSupported(String)} is false. + * + * @return true if a reply was received from the server, false otherwise. + */ + public boolean pingMyServer() { + return ping(connection.getServiceName()); + } } diff --git a/test-unit/org/jivesoftware/smack/ping/ServerPingTest.java b/test-unit/org/jivesoftware/smack/ping/KeepaliveTest.java similarity index 87% rename from test-unit/org/jivesoftware/smack/ping/ServerPingTest.java rename to test-unit/org/jivesoftware/smack/ping/KeepaliveTest.java index fa16dce03..25927d4e8 100644 --- a/test-unit/org/jivesoftware/smack/ping/ServerPingTest.java +++ b/test-unit/org/jivesoftware/smack/ping/KeepaliveTest.java @@ -21,9 +21,12 @@ import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.Packet; import org.jivesoftware.smack.ping.packet.Ping; import org.jivesoftware.smack.util.PacketParserUtils; +import org.junit.After; +import org.junit.Before; import org.junit.Test; -public class ServerPingTest { +public class KeepaliveTest { + private static final long PING_MINIMUM = 1000; private static String TO = "juliet@capulet.lit/balcony"; private static String ID = "s2c1"; @@ -31,7 +34,21 @@ public class ServerPingTest { { outputProperties.put(javax.xml.transform.OutputKeys.OMIT_XML_DECLARATION, "yes"); } - + + private int originalTimeout; + + @Before + public void resetProperties() + { + originalTimeout = SmackConfiguration.getPacketReplyTimeout(); + SmackConfiguration.setPacketReplyTimeout(1000); + } + + @After + public void restoreProperties() + { + SmackConfiguration.setPacketReplyTimeout(originalTimeout); + } /* * Stanza copied from spec */ @@ -133,7 +150,7 @@ public class ServerPingTest { private DummyConnection getConnection() { DummyConnection con = new DummyConnection(); ServerPingManager mgr = ServerPingManager.getInstanceFor(con); - mgr.setPingInterval(ServerPingManager.PING_MINIMUM); + mgr.setPingInterval(PING_MINIMUM); return con; } @@ -141,7 +158,7 @@ public class ServerPingTest { private ThreadedDummyConnection getThreadedConnection() { ThreadedDummyConnection con = new ThreadedDummyConnection(); ServerPingManager mgr = ServerPingManager.getInstanceFor(con); - mgr.setPingInterval(ServerPingManager.PING_MINIMUM); + mgr.setPingInterval(PING_MINIMUM); return con; } @@ -157,6 +174,6 @@ public class ServerPingTest { } private long getWaitTime() { - return ServerPingManager.PING_MINIMUM + SmackConfiguration.getPacketReplyTimeout() + 3000; + return PING_MINIMUM + SmackConfiguration.getPacketReplyTimeout() + 3000; } } diff --git a/test-unit/org/jivesoftware/smackx/ping/PingPongTest.java b/test-unit/org/jivesoftware/smackx/ping/PingTest.java similarity index 69% rename from test-unit/org/jivesoftware/smackx/ping/PingPongTest.java rename to test-unit/org/jivesoftware/smackx/ping/PingTest.java index 0346bb034..b7b6358b8 100644 --- a/test-unit/org/jivesoftware/smackx/ping/PingPongTest.java +++ b/test-unit/org/jivesoftware/smackx/ping/PingTest.java @@ -23,19 +23,28 @@ import org.jivesoftware.smack.packet.Packet; import org.jivesoftware.smack.ping.packet.Ping; import org.jivesoftware.smack.util.PacketParserUtils; import org.jivesoftware.smackx.packet.DiscoverInfo; +import org.junit.Before; import org.junit.Test; import static org.junit.Assert.*; -public class PingPongTest { - +public class PingTest { + private DummyConnection dummyCon; + private ThreadedDummyConnection threadedCon; + + @Before + public void setup() { + dummyCon = new DummyConnection(); + threadedCon = new ThreadedDummyConnection(); + } + @Test public void checkSendingPing() throws Exception { - DummyConnection con = new DummyConnection(); - PingManager pinger = new PingManager(con); + dummyCon = new DummyConnection(); + PingManager pinger = new PingManager(dummyCon); pinger.ping("test@myserver.com"); - Packet sentPacket = con.getSentPacket(); + Packet sentPacket = dummyCon.getSentPacket(); assertTrue(sentPacket instanceof Ping); @@ -43,9 +52,9 @@ public class PingPongTest { @Test public void checkSuccessfulPing() throws Exception { - ThreadedDummyConnection con = new ThreadedDummyConnection(); + threadedCon = new ThreadedDummyConnection(); - PingManager pinger = new PingManager(con); + PingManager pinger = new PingManager(threadedCon); boolean pingSuccess = pinger.ping("test@myserver.com"); @@ -59,8 +68,8 @@ public class PingPongTest { */ @Test public void checkFailedPingOnTimeout() throws Exception { - DummyConnection con = new DummyConnection(); - PingManager pinger = new PingManager(con); + dummyCon = new DummyConnection(); + PingManager pinger = new PingManager(dummyCon); boolean pingSuccess = pinger.ping("test@myserver.com"); @@ -69,12 +78,12 @@ public class PingPongTest { } /** - * DummyConnection will not reply so it will timeout. + * Server returns an exception for entity. * @throws Exception */ @Test - public void checkFailedPingError() throws Exception { - ThreadedDummyConnection con = new ThreadedDummyConnection(); + public void checkFailedPingToEntityError() throws Exception { + threadedCon = new ThreadedDummyConnection(); //@formatter:off String reply = "" + @@ -84,17 +93,62 @@ public class PingPongTest { "" + ""; //@formatter:on + IQ serviceUnavailable = PacketParserUtils.parseIQ(TestUtils.getIQParser(reply), threadedCon); + threadedCon.addIQReply(serviceUnavailable); + + PingManager pinger = new PingManager(threadedCon); + + boolean pingSuccess = pinger.ping("test@myserver.com"); + + assertFalse(pingSuccess); + } + + @Test + public void checkPingToServerSuccess() throws Exception { + ThreadedDummyConnection con = new ThreadedDummyConnection(); + PingManager pinger = new PingManager(con); + + boolean pingSuccess = pinger.pingMyServer(); + + assertTrue(pingSuccess); + } + + /** + * Server returns an exception. + * @throws Exception + */ + @Test + public void checkPingToServerError() throws Exception { + ThreadedDummyConnection con = new ThreadedDummyConnection(); + //@formatter:off + String reply = + "" + + "" + + "" + + "" + + "" + + ""; + //@formatter:on IQ serviceUnavailable = PacketParserUtils.parseIQ(TestUtils.getIQParser(reply), con); con.addIQReply(serviceUnavailable); PingManager pinger = new PingManager(con); - boolean pingSuccess = pinger.ping("test@myserver.com"); - - assertFalse(pingSuccess); + boolean pingSuccess = pinger.pingMyServer(); + assertTrue(pingSuccess); } + @Test + public void checkPingToServerTimeout() throws Exception { + DummyConnection con = new DummyConnection(); + PingManager pinger = new PingManager(con); + + boolean pingSuccess = pinger.pingMyServer(); + + assertFalse(pingSuccess); + } + @Test public void checkSuccessfulDiscoRequest() throws Exception { ThreadedDummyConnection con = new ThreadedDummyConnection();