From 3d3d89612f7b4cbbe2c9f0e2eb5e60acd2bb880a Mon Sep 17 00:00:00 2001 From: Heckel Date: Wed, 13 Mar 2019 23:57:15 +0100 Subject: [PATCH 01/18] PingManager: Assume server ping successful if stanzas where received We assume that the connection is good even if there was no ping result but a stanzas within the ping interval. In some case we have produced so much load, that the ping result was not delivered in time because so many other packets were delivered before. --- .../org/jivesoftware/smackx/ping/PingManager.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/ping/PingManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/ping/PingManager.java index 76e89369a..89d4b5bb3 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/ping/PingManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/ping/PingManager.java @@ -447,6 +447,19 @@ public final class PingManager extends Manager { pingFuture.onError(new ExceptionCallback() { @Override public void processException(Exception exception) { + long lastStanzaReceived = connection.getLastStanzaReceived(); + if (lastStanzaReceived > 0) { + long now = System.currentTimeMillis(); + // Delta since the last stanza was received + int deltaInSeconds = (int) ((now - lastStanzaReceived) / 1000); + // If the delta is smaller then the ping interval, we have got an valid stanza in time + // So not error notification needed + if (deltaInSeconds < pingInterval) { + maybeSchedulePingServerTask(deltaInSeconds); + return; + } + } + for (PingFailedListener l : pingFailedListeners) { l.pingFailed(); } From 29af92cbd1d691e76f5137fe2e4d7cfcf4545577 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 15 Mar 2019 09:43:09 +0100 Subject: [PATCH 02/18] Smack 4.3.4-SNAPSHOT --- version.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.gradle b/version.gradle index 85012ef33..82e39bdf3 100644 --- a/version.gradle +++ b/version.gradle @@ -1,7 +1,7 @@ allprojects { ext { - shortVersion = '4.3.3' - isSnapshot = false + shortVersion = '4.3.4' + isSnapshot = true // When using dynamic versions for those, do *not* use [1.0, // 2.0), since this will also pull in 2.0-alpha1. Instead use // [1.0, 1.0.99]. From 3ded023629e1d90365d95e2857230a8086d4e67a Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 16 Mar 2019 10:27:18 +0100 Subject: [PATCH 03/18] Remove 'synchronized' from notifyConnectionError() as it is not neccessary and causes stalls and deadlocks with a concurrent connect()/login() (which could be resolved interrupting the connect()/login() calling thread): "Smack Reader (0)" daemon prio=5 tid=21 Blocked | group="main" sCount=1 dsCount=0 obj=0x12c84670 self=0x7e072ca200 | sysTid=14965 nice=0 cgrp=default sched=0/0 handle=0x7e06155450 | state=S schedstat=( 63430626 3034269 21 ) utm=5 stm=0 core=2 HZ=100 | stack=0x7e06053000-0x7e06055000 stackSize=1037KB | held mutexes= at org.jivesoftware.smack.tcp.XMPPTCPConnection.notifyConnectionError(XMPPTCPConnection.java:-1) - waiting to lock <0x0ec6e6bb> (a org.jivesoftware.smack.tcp.XMPPTCPConnection) held by thread 22 at org.jivesoftware.smack.tcp.XMPPTCPConnection.access$3900(XMPPTCPConnection.java:154) at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.parsePackets(XMPPTCPConnection.java:1330) at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$900(XMPPTCPConnection.java:1064) at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:1081) at java.lang.Thread.run(Thread.java:761) "connect" prio=5 tid=22 Waiting | group="main" sCount=1 dsCount=0 obj=0x12c8e820 self=0x7e19bcd600 | sysTid=15047 nice=0 cgrp=default sched=0/0 handle=0x7e05ee4450 | state=S schedstat=( 14067083 7475835 14 ) utm=1 stm=0 core=0 HZ=100 | stack=0x7e05de2000-0x7e05de4000 stackSize=1037KB | held mutexes= at java.lang.Object.wait!(Native method) - waiting on <0x0c058b14> (a java.lang.Object) at java.lang.Thread.parkFor$(Thread.java:2127) - locked <0x0c058b14> (a java.lang.Object) at sun.misc.Unsafe.park(Unsafe.java:325) at java.util.concurrent.locks.LockSupport.park(LockSupport.java:161) at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:840) at java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(AbstractQueuedSynchronizer.java:994) at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1303) at java.util.concurrent.Semaphore.acquire(Semaphore.java:446) at org.jivesoftware.smack.tcp.XMPPTCPConnection.initConnection(XMPPTCPConnection.java:685) at org.jivesoftware.smack.tcp.XMPPTCPConnection.connectInternal(XMPPTCPConnection.java:942) at org.jivesoftware.smack.AbstractXMPPConnection.connect(AbstractXMPPConnection.java:417) - locked <0x0ec6e6bb> (a org.jivesoftware.smack.tcp.XMPPTCPConnection) at org.yaxim.androidclient.service.SmackableImp.connectAndLogin(SmackableImp.java:717) - locked <0x0ec6e6bb> (a org.jivesoftware.smack.tcp.XMPPTCPConnection) at org.yaxim.androidclient.service.SmackableImp.doConnect(SmackableImp.java:304) at org.yaxim.androidclient.service.SmackableImp$5.run(SmackableImp.java:391) --- .../main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 61de855d6..b0fcb8b7d 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -953,7 +953,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { * * @param e the exception that causes the connection close event. */ - private synchronized void notifyConnectionError(final Exception e) { + private void notifyConnectionError(final Exception e) { ASYNC_BUT_ORDERED.performAsyncButOrdered(this, new Runnable() { @Override public void run() { From 8e52e80399495e40321c3fb5a3c9835c5e5db1c7 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 16 Mar 2019 10:30:16 +0100 Subject: [PATCH 04/18] Synchronize later in notifyConnectionError(Exception) especially *after* the sync points have been notified so that a potential thread currently callin gconnect()/login() throws and leaves the synchronized section. This commit is more or less equivalent to 3ded023629e1d90365d95e2857230a8086d4e67a of the 4.3 branch. --- .../smack/AbstractXMPPConnection.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java index 53236e6b1..49b1e516f 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -858,9 +858,6 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { ASYNC_BUT_ORDERED.performAsyncButOrdered(this, () -> { currentConnectionException = exception; - synchronized (AbstractXMPPConnection.this) { - notifyAll(); - } for (StanzaCollector collector : collectors) { collector.notifyConnectionError(exception); @@ -873,10 +870,14 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { // XMPPTCPConnection. Create delegation method? // maybeCompressFeaturesReceived.reportGenericFailure(smackWrappedException); - // Closes the connection temporary. A if the connection supports stream management, then a reconnection is - // possible. Note that a connection listener of e.g. XMPPTCPConnection will drop the SM state in - // case the Exception is a StreamErrorException. - instantShutdown(); + synchronized (AbstractXMPPConnection.this) { + notifyAll(); + + // Closes the connection temporary. A if the connection supports stream management, then a reconnection is + // possible. Note that a connection listener of e.g. XMPPTCPConnection will drop the SM state in + // case the Exception is a StreamErrorException. + instantShutdown(); + } Async.go(() -> { // Notify connection listeners of the error. From 7059b60672628d1427776a794d396fb09b2ed42b Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 16 Mar 2019 11:11:58 +0100 Subject: [PATCH 05/18] Wait for reader/writer thread termination at the end of shutdown() This synchronizes the place with what the master branch currently does. --- .../org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index b0fcb8b7d..bf4409506 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -561,6 +561,10 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { writer = null; initState(); + + // Wait for reader and writer threads to be terminated. + readerWriterSemaphore.acquireUninterruptibly(2); + readerWriterSemaphore.release(2); } @Override @@ -978,10 +982,6 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { // Note that a connection listener of XMPPTCPConnection will drop the SM state in // case the Exception is a StreamErrorException. instantShutdown(); - - // Wait for reader and writer threads to be terminated. - readerWriterSemaphore.acquireUninterruptibly(2); - readerWriterSemaphore.release(2); } Async.go(new Runnable() { From c31e93a00ffe647367e6bf12dbea77d15ce31645 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 16 Mar 2019 21:12:05 +0100 Subject: [PATCH 06/18] Improve IntegrationTestRosterUtil.ensureSubscribedto() Improve the names of the arguments and variables. And ensure that all listeners are removed at the end. --- .../util/IntegrationTestRosterUtil.java | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/util/IntegrationTestRosterUtil.java b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/util/IntegrationTestRosterUtil.java index 11a0d7b59..c03ba1850 100644 --- a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/util/IntegrationTestRosterUtil.java +++ b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/util/IntegrationTestRosterUtil.java @@ -1,6 +1,6 @@ /** * - * Copyright 2015-2018 Florian Schmaus + * Copyright 2015-2019 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,11 +25,13 @@ import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.packet.Presence; import org.jivesoftware.smack.roster.AbstractPresenceEventListener; +import org.jivesoftware.smack.roster.PresenceEventListener; import org.jivesoftware.smack.roster.Roster; import org.jivesoftware.smack.roster.RosterEntry; import org.jivesoftware.smack.roster.SubscribeListener; import org.jxmpp.jid.BareJid; +import org.jxmpp.jid.EntityFullJid; import org.jxmpp.jid.Jid; public class IntegrationTestRosterUtil { @@ -39,41 +41,47 @@ public class IntegrationTestRosterUtil { ensureSubscribedTo(conTwo, conOne, timeout); } - public static void ensureSubscribedTo(final XMPPConnection conOne, final XMPPConnection conTwo, long timeout) throws TimeoutException, Exception { - Roster rosterOne = Roster.getInstanceFor(conOne); - Roster rosterTwo = Roster.getInstanceFor(conTwo); + public static void ensureSubscribedTo(final XMPPConnection presenceRequestReceiverConnection, final XMPPConnection presenceRequestingConnection, long timeout) throws TimeoutException, Exception { + final Roster presenceRequestReceiverRoster = Roster.getInstanceFor(presenceRequestReceiverConnection); + final Roster presenceRequestingRoster = Roster.getInstanceFor(presenceRequestingConnection); - if (rosterOne.isSubscribedToMyPresence(conTwo.getUser())) { + final EntityFullJid presenceRequestReceiverAddress = presenceRequestReceiverConnection.getUser(); + final EntityFullJid presenceRequestingAddress = presenceRequestingConnection.getUser(); + + if (presenceRequestReceiverRoster.isSubscribedToMyPresence(presenceRequestingAddress)) { return; } final SubscribeListener subscribeListener = new SubscribeListener() { @Override public SubscribeAnswer processSubscribe(Jid from, Presence subscribeRequest) { - if (from.equals(conTwo.getUser().asBareJid())) { + if (from.equals(presenceRequestingConnection.getUser().asBareJid())) { return SubscribeAnswer.Approve; } return SubscribeAnswer.Deny; } }; - rosterOne.addSubscribeListener(subscribeListener); + presenceRequestReceiverRoster.addSubscribeListener(subscribeListener); final SimpleResultSyncPoint syncPoint = new SimpleResultSyncPoint(); - rosterTwo.addPresenceEventListener(new AbstractPresenceEventListener() { + final PresenceEventListener presenceEventListener = new AbstractPresenceEventListener() { @Override public void presenceSubscribed(BareJid address, Presence subscribedPresence) { - if (!address.equals(conOne.getUser().asBareJid())) { + if (!address.equals(presenceRequestReceiverAddress.asBareJid())) { return; } syncPoint.signal(); } - }); - rosterTwo.sendSubscriptionRequest(conOne.getUser().asBareJid()); + }; + presenceRequestingRoster.addPresenceEventListener(presenceEventListener); try { + presenceRequestingRoster.sendSubscriptionRequest(presenceRequestReceiverAddress.asBareJid()); + syncPoint.waitForResult(timeout); } finally { - rosterOne.removeSubscribeListener(subscribeListener); + presenceRequestReceiverRoster.removeSubscribeListener(subscribeListener); + presenceRequestingRoster.removePresenceEventListener(presenceEventListener); } } From 9c4e5d0330db28920e6583f817569655b89dd696 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 16 Mar 2019 21:13:00 +0100 Subject: [PATCH 07/18] Add Roster.createItem() and Roster.createItemAndRequestSubscription() and deprecate createEntry(). createEntry() would also send a subscription request which may is suprising given that you can also create an roster item without having to send a subscription request out. This also fixes a bug in LowLevelRosterIntegrationTest.testPresenceEventListenersOffline() where createEntry() was used, which would also trigger a presence subscription request which in turn made the test fail if the SubscribeListener of ensureSubscribedTo() was not yet set up. So the test would fail depending on the timing. --- .../org/jivesoftware/smack/roster/Roster.java | 53 +++++++++++++++++-- .../jivesoftware/smack/roster/RosterTest.java | 4 +- .../roster/LowLevelRosterIntegrationTest.java | 5 +- .../smack/roster/RosterIntegrationTest.java | 2 +- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java b/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java index 8e241151d..e9652e652 100644 --- a/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java +++ b/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java @@ -1,6 +1,6 @@ /** * - * Copyright 2003-2007 Jive Software, 2016-2017 Florian Schmaus. + * Copyright 2003-2007 Jive Software, 2016-2019 Florian Schmaus. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -629,14 +629,40 @@ public final class Roster extends Manager { * @throws NotLoggedInException If not logged in. * @throws NotConnectedException * @throws InterruptedException + * @deprecated use {@link #createItemAndRequestSubscription(BareJid, String, String[])} instead. */ + // TODO: Remove in Smack 4.5. + @Deprecated public void createEntry(BareJid user, String name, String[] groups) throws NotLoggedInException, NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { + createItemAndRequestSubscription(user, name, groups); + } + + /** + * Creates a new roster item. The server will asynchronously update the roster with the subscription status. + *

+ * There will be no presence subscription request. Consider using + * {@link #createItemAndRequestSubscription(BareJid, String, String[])} if you also want to request a presence + * subscription from the contact. + *

+ * + * @param jid the XMPP address of the contact (e.g. johndoe@jabber.org) + * @param name the nickname of the user. + * @param groups the list of group names the entry will belong to, or null if the the roster entry won't + * belong to a group. + * @throws NoResponseException if there was no response from the server. + * @throws XMPPErrorException if an XMPP exception occurs. + * @throws NotLoggedInException If not logged in. + * @throws NotConnectedException + * @throws InterruptedException + * @since 4.4.0 + */ + public void createItem(BareJid jid, String name, String[] groups) throws NotLoggedInException, NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { final XMPPConnection connection = getAuthenticatedConnectionOrThrow(); // Create and send roster entry creation packet. RosterPacket rosterPacket = new RosterPacket(); rosterPacket.setType(IQ.Type.set); - RosterPacket.Item item = new RosterPacket.Item(user, name); + RosterPacket.Item item = new RosterPacket.Item(jid, name); if (groups != null) { for (String group : groups) { if (group != null && group.trim().length() > 0) { @@ -646,8 +672,27 @@ public final class Roster extends Manager { } rosterPacket.addRosterItem(item); connection.createStanzaCollectorAndSend(rosterPacket).nextResultOrThrow(); + } - sendSubscriptionRequest(user); + /** + * Creates a new roster entry and presence subscription. The server will asynchronously + * update the roster with the subscription status. + * + * @param jid the XMPP address of the contact (e.g. johndoe@jabber.org) + * @param name the nickname of the user. + * @param groups the list of group names the entry will belong to, or null if the + * the roster entry won't belong to a group. + * @throws NoResponseException if there was no response from the server. + * @throws XMPPErrorException if an XMPP exception occurs. + * @throws NotLoggedInException If not logged in. + * @throws NotConnectedException + * @throws InterruptedException + * @since 4.4.0 + */ + public void createItemAndRequestSubscription(BareJid jid, String name, String[] groups) throws NotLoggedInException, NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { + createItem(jid, name, groups); + + sendSubscriptionRequest(jid); } /** @@ -668,7 +713,7 @@ public final class Roster extends Manager { */ public void preApproveAndCreateEntry(BareJid user, String name, String[] groups) throws NotLoggedInException, NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException, FeatureNotSupportedException { preApprove(user); - createEntry(user, name, groups); + createItemAndRequestSubscription(user, name, groups); } /** diff --git a/smack-im/src/test/java/org/jivesoftware/smack/roster/RosterTest.java b/smack-im/src/test/java/org/jivesoftware/smack/roster/RosterTest.java index 4d26b20c6..85d0457d6 100644 --- a/smack-im/src/test/java/org/jivesoftware/smack/roster/RosterTest.java +++ b/smack-im/src/test/java/org/jivesoftware/smack/roster/RosterTest.java @@ -164,7 +164,7 @@ public class RosterTest extends InitSmackIm { } }; serverSimulator.start(); - roster.createEntry(contactJID, contactName, contactGroup); + roster.createItemAndRequestSubscription(contactJID, contactName, contactGroup); serverSimulator.join(); // Check if an error occurred within the simulator @@ -430,7 +430,7 @@ public class RosterTest extends InitSmackIm { } }; serverSimulator.start(); - roster.createEntry(contactJID, contactName, contactGroup); + roster.createItemAndRequestSubscription(contactJID, contactName, contactGroup); serverSimulator.join(); // Check if an error occurred within the simulator diff --git a/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java b/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java index fac15a6dc..43d9899ec 100644 --- a/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java +++ b/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java @@ -42,9 +42,8 @@ public class LowLevelRosterIntegrationTest extends AbstractSmackLowLevelIntegrat final Roster rosterOne = Roster.getInstanceFor(conOne); final Roster rosterTwo = Roster.getInstanceFor(conTwo); - // TODO create Roster.createEntry() with boolean flag for subscribe or not. - rosterOne.createEntry(conTwo.getUser().asBareJid(), "Con Two", null); - rosterTwo.createEntry(conOne.getUser().asBareJid(), "Con One", null); + rosterOne.createItem(conTwo.getUser().asBareJid(), "Con Two", null); + rosterTwo.createItem(conOne.getUser().asBareJid(), "Con One", null); // TODO Change timeout form '5000' to something configurable. final long timeout = 5000; diff --git a/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/RosterIntegrationTest.java b/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/RosterIntegrationTest.java index 77bd1d8bb..beff7eb1b 100644 --- a/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/RosterIntegrationTest.java +++ b/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/RosterIntegrationTest.java @@ -99,7 +99,7 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest { }); try { - rosterOne.createEntry(conTwo.getUser().asBareJid(), conTwosRosterName, null); + rosterOne.createItemAndRequestSubscription(conTwo.getUser().asBareJid(), conTwosRosterName, null); assertTrue(addedAndSubscribed.waitForResult(2 * connection.getReplyTimeout())); } From 9246ea5dcac4549fd855f64568241930f74b01cc Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 16 Mar 2019 21:24:04 +0100 Subject: [PATCH 08/18] LowLevelRosterIntegrationTest: Use timeout value from superclass --- .../smack/roster/LowLevelRosterIntegrationTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java b/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java index 43d9899ec..242491c96 100644 --- a/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java +++ b/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java @@ -45,8 +45,6 @@ public class LowLevelRosterIntegrationTest extends AbstractSmackLowLevelIntegrat rosterOne.createItem(conTwo.getUser().asBareJid(), "Con Two", null); rosterTwo.createItem(conOne.getUser().asBareJid(), "Con One", null); - // TODO Change timeout form '5000' to something configurable. - final long timeout = 5000; IntegrationTestRosterUtil.ensureBothAccountsAreSubscribedToEachOther(conOne, conTwo, timeout); final SimpleResultSyncPoint offlineTriggered = new SimpleResultSyncPoint(); From a61913596013fa6ad87874aa7ea80cde9aa35ea6 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 16 Mar 2019 22:41:01 +0100 Subject: [PATCH 09/18] MoodIntegrationTest: Ensure that listener is removed --- .../smackx/mood/MoodIntegrationTest.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/smack-integration-test/src/main/java/org/jivesoftware/smackx/mood/MoodIntegrationTest.java b/smack-integration-test/src/main/java/org/jivesoftware/smackx/mood/MoodIntegrationTest.java index e75693108..902c2b97f 100644 --- a/smack-integration-test/src/main/java/org/jivesoftware/smackx/mood/MoodIntegrationTest.java +++ b/smack-integration-test/src/main/java/org/jivesoftware/smackx/mood/MoodIntegrationTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2018 Paul Schaub. + * Copyright 2018 Paul Schaub, 2019 Florian Schmaus. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,9 +18,6 @@ package org.jivesoftware.smackx.mood; import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.XMPPException; -import org.jivesoftware.smack.packet.Message; - -import org.jivesoftware.smackx.mood.element.MoodElement; import org.igniterealtime.smack.inttest.AbstractSmackIntegrationTest; import org.igniterealtime.smack.inttest.SmackIntegrationTest; @@ -28,7 +25,6 @@ import org.igniterealtime.smack.inttest.SmackIntegrationTestEnvironment; import org.igniterealtime.smack.inttest.util.IntegrationTestRosterUtil; import org.igniterealtime.smack.inttest.util.SimpleResultSyncPoint; import org.junit.AfterClass; -import org.jxmpp.jid.BareJid; public class MoodIntegrationTest extends AbstractSmackIntegrationTest { @@ -47,18 +43,20 @@ public class MoodIntegrationTest extends AbstractSmackIntegrationTest { final SimpleResultSyncPoint moodReceived = new SimpleResultSyncPoint(); - mm2.addMoodListener(new MoodListener() { - @Override - public void onMoodUpdated(BareJid jid, Message message, MoodElement moodElement) { - if (moodElement.getMood() == Mood.satisfied) { - moodReceived.signal(); - } + final MoodListener moodListener = (jid, message, moodElement) -> { + if (moodElement.getMood() == Mood.satisfied) { + moodReceived.signal(); } - }); + }; + mm2.addMoodListener(moodListener); - mm1.setMood(Mood.satisfied); + try { + mm1.setMood(Mood.satisfied); - moodReceived.waitForResult(timeout); + moodReceived.waitForResult(timeout); + } finally { + mm2.removeMoodListener(moodListener); + } } @AfterClass From d6e25730d0cc29a64c40a8f28e3ef60a351be833 Mon Sep 17 00:00:00 2001 From: adiaholic Date: Mon, 18 Mar 2019 13:10:20 +0530 Subject: [PATCH 10/18] Saving an instance of ServiceDiscoveryManager in MultiUserChatManager. Previously, the costly method 'ServiceDiscoveryManager.getInstance()' was called at multiple instances inside MultiUserChatManager. In this commit I wish to replace this call by saving a private final instance of 'ServiceDiscoveryManager' which will is an attempt at solving SMACK-836. --- .../smackx/muc/MultiUserChatManager.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java index 98e3dfdc0..639e2f792 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java @@ -152,8 +152,11 @@ public final class MultiUserChatManager extends Manager { private AutoJoinFailedCallback autoJoinFailedCallback; + private final ServiceDiscoveryManager serviceDiscoveryManager; + private MultiUserChatManager(XMPPConnection connection) { super(connection); + serviceDiscoveryManager = ServiceDiscoveryManager.getInstanceFor(connection); // Listens for all messages that include a MUCUser extension and fire the invitation // listeners if the message includes an invitation. StanzaListener invitationPacketListener = new StanzaListener() { @@ -277,7 +280,7 @@ public final class MultiUserChatManager extends Manager { * @throws InterruptedException */ public boolean isServiceEnabled(Jid user) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { - return ServiceDiscoveryManager.getInstanceFor(connection()).supportsFeature(user, MUCInitialPresence.NAMESPACE); + return serviceDiscoveryManager.supportsFeature(user, MUCInitialPresence.NAMESPACE); } /** @@ -304,7 +307,7 @@ public final class MultiUserChatManager extends Manager { public List getJoinedRooms(EntityJid user) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { // Send the disco packet to the user - DiscoverItems result = ServiceDiscoveryManager.getInstanceFor(connection()).discoverItems(user, DISCO_NODE); + DiscoverItems result = serviceDiscoveryManager.discoverItems(user, DISCO_NODE); List items = result.getItems(); List answer = new ArrayList<>(items.size()); // Collect the entityID for each returned item @@ -331,7 +334,7 @@ public final class MultiUserChatManager extends Manager { * @throws InterruptedException */ public RoomInfo getRoomInfo(EntityBareJid room) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { - DiscoverInfo info = ServiceDiscoveryManager.getInstanceFor(connection()).discoverInfo(room); + DiscoverInfo info = serviceDiscoveryManager.discoverInfo(room); return new RoomInfo(info); } @@ -345,8 +348,7 @@ public final class MultiUserChatManager extends Manager { * @throws InterruptedException */ public List getMucServiceDomains() throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { - ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(connection()); - return sdm.findServices(MUCInitialPresence.NAMESPACE, false, false); + return serviceDiscoveryManager.findServices(MUCInitialPresence.NAMESPACE, false, false); } /** @@ -379,7 +381,7 @@ public final class MultiUserChatManager extends Manager { */ public boolean providesMucService(DomainBareJid domainBareJid) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { - return ServiceDiscoveryManager.getInstanceFor(connection()).supportsFeature(domainBareJid, + return serviceDiscoveryManager.supportsFeature(domainBareJid, MUCInitialPresence.NAMESPACE); } @@ -402,8 +404,7 @@ public final class MultiUserChatManager extends Manager { if (!providesMucService(serviceName)) { throw new NotAMucServiceException(serviceName); } - ServiceDiscoveryManager discoManager = ServiceDiscoveryManager.getInstanceFor(connection()); - DiscoverItems discoverItems = discoManager.discoverItems(serviceName); + DiscoverItems discoverItems = serviceDiscoveryManager.discoverItems(serviceName); List items = discoverItems.getItems(); Map answer = new HashMap<>(items.size()); From b1cd5066f64c7c3e68320abf217939c8c7b422d1 Mon Sep 17 00:00:00 2001 From: Matthew Wild Date: Tue, 19 Mar 2019 08:59:42 +0000 Subject: [PATCH 11/18] Minor fix to integrationtest.md formatting The preceding paragraph was being treated as part of the header. --- documentation/developer/integrationtest.md | 1 + 1 file changed, 1 insertion(+) diff --git a/documentation/developer/integrationtest.md b/documentation/developer/integrationtest.md index fc6684a3e..2bf2143db 100644 --- a/documentation/developer/integrationtest.md +++ b/documentation/developer/integrationtest.md @@ -80,6 +80,7 @@ debugger=console ### Where to place the properties file The framework will first load the properties file from `~/.config/smack-integration-test/properties` + Overview of the components -------------------------- From 3521346c906691d782cffb4abce7d7c3e28dda2c Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Wed, 20 Mar 2019 20:05:43 +0100 Subject: [PATCH 12/18] Add Github Pull Request Template --- .github/pull_request_template.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..26e468f33 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,7 @@ +Thank you for your contribution! + +Before creating a Pull Request, please make sure to +* read https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors +* run `gradle check` successfully in order to make sure that your code does not break any JUnit tests and is conform to the projects code style. +* (if applicable) mention any Jira issue codes (eg. `SMACK-XXX`) in the *body* of your commit message (not the header), so that Jira automatically links the PR to the issue. +* squash your commits if possible/sensible. From a9b88d7517e3bf20968b743949ace007f585a6a8 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Wed, 20 Mar 2019 20:16:21 +0100 Subject: [PATCH 13/18] Show all exceptions ErrorsWhileSendingOrReceivingException message --- .../smack/XmppConnectionStressTest.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/smack-integration-test/src/main/java/org/igniterealtime/smack/XmppConnectionStressTest.java b/smack-integration-test/src/main/java/org/igniterealtime/smack/XmppConnectionStressTest.java index 8cabed30c..acb62b869 100644 --- a/smack-integration-test/src/main/java/org/igniterealtime/smack/XmppConnectionStressTest.java +++ b/smack-integration-test/src/main/java/org/igniterealtime/smack/XmppConnectionStressTest.java @@ -299,10 +299,32 @@ public class XmppConnectionStressTest { private ErrorsWhileSendingOrReceivingException(Map sendExceptions, Map receiveExceptions) { - super("Exceptions while sending and/or receiving"); + super(createMessageFrom(sendExceptions, receiveExceptions)); this.sendExceptions = sendExceptions; this.receiveExceptions = receiveExceptions; } + + private static String createMessageFrom(Map sendExceptions, + Map receiveExceptions) { + StringBuilder sb = new StringBuilder(1024); + sb.append("Exceptions while sending and/or receiving."); + + if (!sendExceptions.isEmpty()) { + sb.append(" Send exxceptions: "); + for (Map.Entry entry : sendExceptions.entrySet()) { + sb.append(entry.getKey()).append(": ").append(entry.getValue()).append(';'); + } + } + + if (!receiveExceptions.isEmpty()) { + sb.append(" Receive exceptions: "); + for (Map.Entry entry : receiveExceptions.entrySet()) { + sb.append(entry.getKey()).append(": ").append(entry.getValue()); + } + } + + return sb.toString(); + } } } } From c83d717a26544eb9c355a0ca932a3cca2867c88f Mon Sep 17 00:00:00 2001 From: Mohsen Hariri Date: Wed, 27 Jun 2018 11:18:50 +0200 Subject: [PATCH 14/18] Allow adding custom HTTP headers to bosh communications --- smack-bosh/build.gradle | 2 +- .../jivesoftware/smack/bosh/BOSHConfiguration.java | 14 ++++++++++++++ .../smack/bosh/XMPPBOSHConnection.java | 4 ++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/smack-bosh/build.gradle b/smack-bosh/build.gradle index a3416325e..6a0e1c679 100644 --- a/smack-bosh/build.gradle +++ b/smack-bosh/build.gradle @@ -4,5 +4,5 @@ This API is considered beta quality.""" dependencies { compile project(':smack-core') - compile 'org.igniterealtime.jbosh:jbosh:[0.9,0.10)' + compile 'org.igniterealtime.jbosh:jbosh:[0.9.1,0.10)' } diff --git a/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/BOSHConfiguration.java b/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/BOSHConfiguration.java index a83819ad2..8cf01fe85 100644 --- a/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/BOSHConfiguration.java +++ b/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/BOSHConfiguration.java @@ -19,6 +19,8 @@ package org.jivesoftware.smack.bosh; import java.net.URI; import java.net.URISyntaxException; +import java.util.HashMap; +import java.util.Map; import org.jivesoftware.smack.ConnectionConfiguration; import org.jivesoftware.smack.proxy.ProxyInfo; @@ -34,6 +36,7 @@ public final class BOSHConfiguration extends ConnectionConfiguration { private final boolean https; private final String file; + private Map httpHeaders; private BOSHConfiguration(Builder builder) { super(builder); @@ -49,6 +52,7 @@ public final class BOSHConfiguration extends ConnectionConfiguration { } else { file = builder.file; } + httpHeaders = builder.httpHeaders; } public boolean isProxyEnabled() { @@ -76,6 +80,10 @@ public final class BOSHConfiguration extends ConnectionConfiguration { return new URI((https ? "https://" : "http://") + this.host + ":" + this.port + file); } + public Map getHttpHeaders() { + return httpHeaders; + } + public static Builder builder() { return new Builder(); } @@ -83,6 +91,7 @@ public final class BOSHConfiguration extends ConnectionConfiguration { public static final class Builder extends ConnectionConfiguration.Builder { private boolean https; private String file; + private Map httpHeaders = new HashMap<>(); private Builder() { } @@ -101,6 +110,11 @@ public final class BOSHConfiguration extends ConnectionConfiguration { return this; } + public Builder addHttpHeader(String name, String value) { + httpHeaders.put(name, value); + return this; + } + @Override public BOSHConfiguration build() { return new BOSHConfiguration(this); diff --git a/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java b/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java index 68dd48fcb..3c8995b14 100644 --- a/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java +++ b/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java @@ -22,6 +22,7 @@ import java.io.PipedReader; import java.io.PipedWriter; import java.io.StringReader; import java.io.Writer; +import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -156,6 +157,9 @@ public class XMPPBOSHConnection extends AbstractXMPPConnection { if (config.isProxyEnabled()) { cfgBuilder.setProxy(config.getProxyAddress(), config.getProxyPort()); } + for (Map.Entry h : config.getHttpHeaders().entrySet()) { + cfgBuilder.addHttpHeader(h.getKey(), h.getValue()); + } client = BOSHClient.create(cfgBuilder.build()); client.addBOSHClientConnListener(new BOSHConnectionListener()); From 653d9dbba75826968f4f5bf648760edefa43e51f Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 22 Mar 2019 21:53:53 +0100 Subject: [PATCH 15/18] smack-bosh: Limit jbosh to the 0.9 series akin to version.gradle --- smack-bosh/build.gradle | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/smack-bosh/build.gradle b/smack-bosh/build.gradle index 6a0e1c679..f686c8209 100644 --- a/smack-bosh/build.gradle +++ b/smack-bosh/build.gradle @@ -4,5 +4,7 @@ This API is considered beta quality.""" dependencies { compile project(':smack-core') - compile 'org.igniterealtime.jbosh:jbosh:[0.9.1,0.10)' + // See https://issues.igniterealtime.org/browse/SMACK-858 and + // comment in version.gradle why the specify the version this way. + compile 'org.igniterealtime.jbosh:jbosh:[0.9.1,0.9.999]' } From 3450ffad2b8ea4ae1015c81146f6efe23515852a Mon Sep 17 00:00:00 2001 From: Oliver Mihatsch Date: Mon, 18 Mar 2019 10:34:49 +0100 Subject: [PATCH 16/18] Do not use "CONNECT" in the Host header field. --- .../org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java index cd1000c15..6326cf1db 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java @@ -58,7 +58,7 @@ class HTTPProxySocketConnection implements ProxySocketConnection { proxyLine = "\r\nProxy-Authorization: Basic " + Base64.encode(username + ":" + password); } socket.getOutputStream().write((hostport + " HTTP/1.1\r\nHost: " - + hostport + proxyLine + "\r\n\r\n").getBytes("UTF-8")); + + host + ":" + port + proxyLine + "\r\n\r\n").getBytes("UTF-8")); InputStream in = socket.getInputStream(); StringBuilder got = new StringBuilder(100); From 8e88cd2e31a744bf9c5d8e599d38796b927c0413 Mon Sep 17 00:00:00 2001 From: Georg Lukas Date: Tue, 12 Mar 2019 11:33:38 +0100 Subject: [PATCH 17/18] Proxy mode: add failed address on error --- .../main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 1 + 1 file changed, 1 insertion(+) diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index bf4409506..97e0e2044 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -647,6 +647,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { proxyInfo.getProxySocketConnection().connect(socket, host, port, timeout); } catch (IOException e) { hostAddress.setException(e); + failedAddresses.add(hostAddress); continue; } LOGGER.finer("Established TCP connection to " + hostAndPort); From 007a04c4fe599bbea04fdd4bc6cf03e45ae8db36 Mon Sep 17 00:00:00 2001 From: Oliver Mihatsch Date: Mon, 18 Mar 2019 10:28:50 +0100 Subject: [PATCH 18/18] Better error messages when using a Proxy to connect to the XMPP server. --- .../jivesoftware/smack/proxy/HTTPProxySocketConnection.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java index 6326cf1db..549406423 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/proxy/HTTPProxySocketConnection.java @@ -115,7 +115,8 @@ class HTTPProxySocketConnection implements ProxySocketConnection { int code = Integer.parseInt(m.group(1)); if (code != HttpURLConnection.HTTP_OK) { - throw new ProxyException(ProxyInfo.ProxyType.HTTP); + throw new ProxyException(ProxyInfo.ProxyType.HTTP, + "Error code in proxy response: " + code); } }