From 9c4e5d0330db28920e6583f817569655b89dd696 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 16 Mar 2019 21:13:00 +0100 Subject: [PATCH] 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())); }