From b16f34f61e8aae62a2d5f4f2ac3730633a3b79ef Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 26 Oct 2013 11:17:16 +0000 Subject: [PATCH] SMACK-458 Managers should be kept on disconnects Smack's Managers should not remove itself when the connection is closed or should re-add themselves if the connection get reconnected. This should also fix some NPE's. We are currently going with two different designs of Manager: 1. The one with WeakReferences/WeakHashMaps (SDM, EntityCapsManager) and 2. the one where the managers remove their listeners on connectionClosed() *and* connectionClosedOnError(), and later add their listeners on reconnectionSuccessful(). The first design has the Connection instance only weak referenced. The other design does reference Connection strongly (e.g. the 'managers' map in IBBManager/S5BManager), but removes this references when connectionClosed(onError)() is called. git-svn-id: http://svn.igniterealtime.org/svn/repos/smack/branches/smack_3_3_2@13788 b35dd754-fafc-0310-a699-88a17e54d16e --- .../smack/PrivacyListManager.java | 64 +--- .../smackx/ServiceDiscoveryManager.java | 230 ++++++------- .../ibb/InBandBytestreamManager.java | 20 +- .../socks5/Socks5BytestreamManager.java | 18 +- .../smackx/commands/AdHocCommandManager.java | 316 ++++++++---------- .../smackx/entitycaps/EntityCapsManager.java | 2 - .../jivesoftware/smackx/ping/PingManager.java | 3 +- .../socks5/InitiationListenerTest.java | 2 +- .../socks5/Socks5ByteStreamManagerTest.java | 4 +- .../FileTransferNegotiatorTest.java | 2 +- .../smackx/receipts/DeliveryReceiptTest.java | 4 +- .../jivesoftware/util/ConnectionUtils.java | 2 +- 12 files changed, 298 insertions(+), 369 deletions(-) diff --git a/source/org/jivesoftware/smack/PrivacyListManager.java b/source/org/jivesoftware/smack/PrivacyListManager.java index eb1d231d2..88d9aed83 100644 --- a/source/org/jivesoftware/smack/PrivacyListManager.java +++ b/source/org/jivesoftware/smack/PrivacyListManager.java @@ -25,6 +25,7 @@ import org.jivesoftware.smack.packet.Packet; import org.jivesoftware.smack.packet.Privacy; import org.jivesoftware.smack.packet.PrivacyItem; +import java.lang.ref.WeakReference; import java.util.*; /** @@ -43,9 +44,10 @@ import java.util.*; public class PrivacyListManager { // Keep the list of instances of this class. - private static Map instances = new Hashtable(); + private static Map instances = Collections + .synchronizedMap(new WeakHashMap()); - private Connection connection; + private WeakReference connection; private final List listeners = new ArrayList(); PacketFilter packetFilter = new AndFilter(new IQTypeFilter(IQ.Type.SET), new PacketExtensionFilter("query", "jabber:iq:privacy")); @@ -67,49 +69,10 @@ public class PrivacyListManager { * * @param connection the XMPP connection. */ - private PrivacyListManager(Connection connection) { - this.connection = connection; - this.init(); - } - - /** Answer the connection userJID that owns the privacy. - * @return the userJID that owns the privacy - */ - private String getUser() { - return connection.getUser(); - } - - /** - * Initializes the packet listeners of the connection that will notify for any set privacy - * package. - */ - private void init() { + private PrivacyListManager(final Connection connection) { + this.connection = new WeakReference(connection); // Register the new instance and associate it with the connection instances.put(connection, this); - // Add a listener to the connection that removes the registered instance when - // the connection is closed - connection.addConnectionListener(new ConnectionListener() { - public void connectionClosed() { - // Unregister this instance since the connection has been closed - instances.remove(connection); - } - - public void connectionClosedOnError(Exception e) { - // ignore - } - - public void reconnectionFailed(Exception e) { - // ignore - } - - public void reconnectingIn(int seconds) { - // ignore - } - - public void reconnectionSuccessful() { - // ignore - } - }); connection.addPacketListener(new PacketListener() { public void processPacket(Packet packet) { @@ -151,8 +114,14 @@ public class PrivacyListManager { // Send create & join packet. connection.sendPacket(iq); } - }, packetFilter); - } + }, packetFilter); } + + /** Answer the connection userJID that owns the privacy. + * @return the userJID that owns the privacy + */ + private String getUser() { + return connection.get().getUser(); + } /** * Returns the PrivacyListManager instance associated with a given Connection. @@ -174,6 +143,8 @@ public class PrivacyListManager { * @exception XMPPException if the request or the answer failed, it raises an exception. */ private Privacy getRequest(Privacy requestPrivacy) throws XMPPException { + Connection connection = PrivacyListManager.this.connection.get(); + if (connection == null) throw new XMPPException("Connection instance already gc'ed"); // The request is a get iq type requestPrivacy.setType(Privacy.Type.GET); requestPrivacy.setFrom(this.getUser()); @@ -212,7 +183,8 @@ public class PrivacyListManager { * @exception XMPPException if the request or the answer failed, it raises an exception. */ private Packet setRequest(Privacy requestPrivacy) throws XMPPException { - + Connection connection = PrivacyListManager.this.connection.get(); + if (connection == null) throw new XMPPException("Connection instance already gc'ed"); // The request is a get iq type requestPrivacy.setType(Privacy.Type.SET); requestPrivacy.setFrom(this.getUser()); diff --git a/source/org/jivesoftware/smackx/ServiceDiscoveryManager.java b/source/org/jivesoftware/smackx/ServiceDiscoveryManager.java index 9e31f6777..29090fb87 100644 --- a/source/org/jivesoftware/smackx/ServiceDiscoveryManager.java +++ b/source/org/jivesoftware/smackx/ServiceDiscoveryManager.java @@ -34,6 +34,7 @@ import org.jivesoftware.smackx.packet.DiscoverInfo.Identity; import org.jivesoftware.smackx.packet.DiscoverItems; import org.jivesoftware.smackx.packet.DataForm; +import java.lang.ref.WeakReference; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -59,9 +60,9 @@ public class ServiceDiscoveryManager { private EntityCapsManager capsManager; private static Map instances = - new ConcurrentHashMap(); + Collections.synchronizedMap(new WeakHashMap()); - private Connection connection; + private WeakReference connection; private final Set features = new HashSet(); private DataForm extendedInfo = null; private Map nodeInformationProviders = @@ -82,12 +83,100 @@ public class ServiceDiscoveryManager { * service manager will respond to any service discovery request that the connection may * receive. * + * @deprecated use {@link #getInstanceFor(connection)} instead * @param connection the connection to which a ServiceDiscoveryManager is going to be created. */ + @Deprecated public ServiceDiscoveryManager(Connection connection) { - this.connection = connection; + this.connection = new WeakReference(connection); + // Register the new instance and associate it with the connection + instances.put(connection, this); - init(); + addFeature(DiscoverInfo.NAMESPACE); + addFeature(DiscoverItems.NAMESPACE); + + // Listen for disco#items requests and answer with an empty result + PacketFilter packetFilter = new PacketTypeFilter(DiscoverItems.class); + PacketListener packetListener = new PacketListener() { + public void processPacket(Packet packet) { + Connection connection = ServiceDiscoveryManager.this.connection.get(); + if (connection == null) return; + DiscoverItems discoverItems = (DiscoverItems) packet; + // Send back the items defined in the client if the request is of type GET + if (discoverItems != null && discoverItems.getType() == IQ.Type.GET) { + DiscoverItems response = new DiscoverItems(); + response.setType(IQ.Type.RESULT); + response.setTo(discoverItems.getFrom()); + response.setPacketID(discoverItems.getPacketID()); + response.setNode(discoverItems.getNode()); + + // Add the defined items related to the requested node. Look for + // the NodeInformationProvider associated with the requested node. + NodeInformationProvider nodeInformationProvider = + getNodeInformationProvider(discoverItems.getNode()); + if (nodeInformationProvider != null) { + // Specified node was found, add node items + response.addItems(nodeInformationProvider.getNodeItems()); + // Add packet extensions + response.addExtensions(nodeInformationProvider.getNodePacketExtensions()); + } else if(discoverItems.getNode() != null) { + // Return error since client doesn't contain + // the specified node + response.setType(IQ.Type.ERROR); + response.setError(new XMPPError(XMPPError.Condition.item_not_found)); + } + connection.sendPacket(response); + } + } + }; + connection.addPacketListener(packetListener, packetFilter); + + // Listen for disco#info requests and answer the client's supported features + // To add a new feature as supported use the #addFeature message + packetFilter = new PacketTypeFilter(DiscoverInfo.class); + packetListener = new PacketListener() { + public void processPacket(Packet packet) { + Connection connection = ServiceDiscoveryManager.this.connection.get(); + if (connection == null) return; + DiscoverInfo discoverInfo = (DiscoverInfo) packet; + // Answer the client's supported features if the request is of the GET type + if (discoverInfo != null && discoverInfo.getType() == IQ.Type.GET) { + DiscoverInfo response = new DiscoverInfo(); + response.setType(IQ.Type.RESULT); + response.setTo(discoverInfo.getFrom()); + response.setPacketID(discoverInfo.getPacketID()); + response.setNode(discoverInfo.getNode()); + // Add the client's identity and features only if "node" is null + // and if the request was not send to a node. If Entity Caps are + // enabled the client's identity and features are may also added + // if the right node is chosen + if (discoverInfo.getNode() == null) { + addDiscoverInfoTo(response); + } + else { + // Disco#info was sent to a node. Check if we have information of the + // specified node + NodeInformationProvider nodeInformationProvider = + getNodeInformationProvider(discoverInfo.getNode()); + if (nodeInformationProvider != null) { + // Node was found. Add node features + response.addFeatures(nodeInformationProvider.getNodeFeatures()); + // Add node identities + response.addIdentities(nodeInformationProvider.getNodeIdentities()); + // Add packet extensions + response.addExtensions(nodeInformationProvider.getNodePacketExtensions()); + } + else { + // Return error since specified node was not found + response.setType(IQ.Type.ERROR); + response.setError(new XMPPError(XMPPError.Condition.item_not_found)); + } + } + connection.sendPacket(response); + } + } + }; + connection.addPacketListener(packetListener, packetFilter); } /** @@ -96,8 +185,12 @@ public class ServiceDiscoveryManager { * @param connection the connection used to look for the proper ServiceDiscoveryManager. * @return the ServiceDiscoveryManager associated with a given Connection. */ - public static ServiceDiscoveryManager getInstanceFor(Connection connection) { - return instances.get(connection); + public static synchronized ServiceDiscoveryManager getInstanceFor(Connection connection) { + ServiceDiscoveryManager sdm = instances.get(connection); + if (sdm == null) { + sdm = new ServiceDiscoveryManager(connection); + } + return sdm; } /** @@ -173,122 +266,6 @@ public class ServiceDiscoveryManager { return Collections.unmodifiableList(identities); } - /** - * Initializes the packet listeners of the connection that will answer to any - * service discovery request. - */ - private void init() { - // Register the new instance and associate it with the connection - instances.put(connection, this); - - addFeature(DiscoverInfo.NAMESPACE); - addFeature(DiscoverItems.NAMESPACE); - - // Add a listener to the connection that removes the registered instance when - // the connection is closed - connection.addConnectionListener(new ConnectionListener() { - public void connectionClosed() { - // Unregister this instance since the connection has been closed - instances.remove(connection); - } - - public void connectionClosedOnError(Exception e) { - // ignore - } - - public void reconnectionFailed(Exception e) { - // ignore - } - - public void reconnectingIn(int seconds) { - // ignore - } - - public void reconnectionSuccessful() { - // ignore - } - }); - - // Listen for disco#items requests and answer with an empty result - PacketFilter packetFilter = new PacketTypeFilter(DiscoverItems.class); - PacketListener packetListener = new PacketListener() { - public void processPacket(Packet packet) { - DiscoverItems discoverItems = (DiscoverItems) packet; - // Send back the items defined in the client if the request is of type GET - if (discoverItems != null && discoverItems.getType() == IQ.Type.GET) { - DiscoverItems response = new DiscoverItems(); - response.setType(IQ.Type.RESULT); - response.setTo(discoverItems.getFrom()); - response.setPacketID(discoverItems.getPacketID()); - response.setNode(discoverItems.getNode()); - - // Add the defined items related to the requested node. Look for - // the NodeInformationProvider associated with the requested node. - NodeInformationProvider nodeInformationProvider = - getNodeInformationProvider(discoverItems.getNode()); - if (nodeInformationProvider != null) { - // Specified node was found, add node items - response.addItems(nodeInformationProvider.getNodeItems()); - // Add packet extensions - response.addExtensions(nodeInformationProvider.getNodePacketExtensions()); - } else if(discoverItems.getNode() != null) { - // Return error since client doesn't contain - // the specified node - response.setType(IQ.Type.ERROR); - response.setError(new XMPPError(XMPPError.Condition.item_not_found)); - } - connection.sendPacket(response); - } - } - }; - connection.addPacketListener(packetListener, packetFilter); - - // Listen for disco#info requests and answer the client's supported features - // To add a new feature as supported use the #addFeature message - packetFilter = new PacketTypeFilter(DiscoverInfo.class); - packetListener = new PacketListener() { - public void processPacket(Packet packet) { - DiscoverInfo discoverInfo = (DiscoverInfo) packet; - // Answer the client's supported features if the request is of the GET type - if (discoverInfo != null && discoverInfo.getType() == IQ.Type.GET) { - DiscoverInfo response = new DiscoverInfo(); - response.setType(IQ.Type.RESULT); - response.setTo(discoverInfo.getFrom()); - response.setPacketID(discoverInfo.getPacketID()); - response.setNode(discoverInfo.getNode()); - // Add the client's identity and features only if "node" is null - // and if the request was not send to a node. If Entity Caps are - // enabled the client's identity and features are may also added - // if the right node is chosen - if (discoverInfo.getNode() == null) { - addDiscoverInfoTo(response); - } - else { - // Disco#info was sent to a node. Check if we have information of the - // specified node - NodeInformationProvider nodeInformationProvider = - getNodeInformationProvider(discoverInfo.getNode()); - if (nodeInformationProvider != null) { - // Node was found. Add node features - response.addFeatures(nodeInformationProvider.getNodeFeatures()); - // Add node identities - response.addIdentities(nodeInformationProvider.getNodeIdentities()); - // Add packet extensions - response.addExtensions(nodeInformationProvider.getNodePacketExtensions()); - } - else { - // Return error since specified node was not found - response.setType(IQ.Type.ERROR); - response.setError(new XMPPError(XMPPError.Condition.item_not_found)); - } - } - connection.sendPacket(response); - } - } - }; - connection.addPacketListener(packetListener, packetFilter); - } - /** * Add discover info response data. * @@ -534,6 +511,9 @@ public class ServiceDiscoveryManager { * @throws XMPPException if the operation failed for some reason. */ public DiscoverInfo discoverInfo(String entityID, String node) throws XMPPException { + Connection connection = ServiceDiscoveryManager.this.connection.get(); + if (connection == null) throw new XMPPException("Connection instance already gc'ed"); + // Discover the entity's info DiscoverInfo disco = new DiscoverInfo(); disco.setType(IQ.Type.GET); @@ -581,6 +561,9 @@ public class ServiceDiscoveryManager { * @throws XMPPException if the operation failed for some reason. */ public DiscoverItems discoverItems(String entityID, String node) throws XMPPException { + Connection connection = ServiceDiscoveryManager.this.connection.get(); + if (connection == null) throw new XMPPException("Connection instance already gc'ed"); + // Discover the entity's items DiscoverItems disco = new DiscoverItems(); disco.setType(IQ.Type.GET); @@ -662,6 +645,9 @@ public class ServiceDiscoveryManager { */ public void publishItems(String entityID, String node, DiscoverItems discoverItems) throws XMPPException { + Connection connection = ServiceDiscoveryManager.this.connection.get(); + if (connection == null) throw new XMPPException("Connection instance already gc'ed"); + discoverItems.setType(IQ.Type.SET); discoverItems.setTo(entityID); discoverItems.setNode(node); diff --git a/source/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java b/source/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java index 158d6785d..158756fdd 100644 --- a/source/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java +++ b/source/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java @@ -95,21 +95,27 @@ public class InBandBytestreamManager implements BytestreamManager { */ static { Connection.addConnectionCreationListener(new ConnectionCreationListener() { - public void connectionCreated(Connection connection) { - final InBandBytestreamManager manager; - manager = InBandBytestreamManager.getByteStreamManager(connection); + public void connectionCreated(final Connection connection) { + // create the manager for this connection + InBandBytestreamManager.getByteStreamManager(connection); // register shutdown listener connection.addConnectionListener(new AbstractConnectionListener() { @Override public void connectionClosed() { - manager.disableService(); + InBandBytestreamManager.getByteStreamManager(connection).disableService(); } @Override - public void connectionClosedOnError(Exception e) { - manager.disableService(); + public void connectionClosedOnError(Exception e) { + InBandBytestreamManager.getByteStreamManager(connection).disableService(); + } + + @Override + public void reconnectionSuccessful() { + // re-create the manager for this connection + InBandBytestreamManager.getByteStreamManager(connection); } }); @@ -526,7 +532,7 @@ public class InBandBytestreamManager implements BytestreamManager { /** * Disables the InBandBytestreamManager by removing its packet listeners and resetting its - * internal status. + * internal status, which includes removing this instance from the managers map. */ private void disableService() { diff --git a/source/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java b/source/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java index e477222b9..a15d86ef9 100644 --- a/source/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java +++ b/source/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java @@ -89,21 +89,27 @@ public final class Socks5BytestreamManager implements BytestreamManager { static { Connection.addConnectionCreationListener(new ConnectionCreationListener() { - public void connectionCreated(Connection connection) { - final Socks5BytestreamManager manager; - manager = Socks5BytestreamManager.getBytestreamManager(connection); + public void connectionCreated(final Connection connection) { + // create the manager for this connection + Socks5BytestreamManager.getBytestreamManager(connection); // register shutdown listener connection.addConnectionListener(new AbstractConnectionListener() { @Override public void connectionClosed() { - manager.disableService(); + Socks5BytestreamManager.getBytestreamManager(connection).disableService(); } @Override public void connectionClosedOnError(Exception e) { - manager.disableService(); + Socks5BytestreamManager.getBytestreamManager(connection).disableService(); + } + + @Override + public void reconnectionSuccessful() { + // re-create the manager for this connection + Socks5BytestreamManager.getBytestreamManager(connection); } }); @@ -274,7 +280,7 @@ public final class Socks5BytestreamManager implements BytestreamManager { /** * Disables the SOCKS5 Bytestream manager by removing the SOCKS5 Bytestream feature from the * service discovery, disabling the listener for SOCKS5 Bytestream initiation requests and - * resetting its internal state. + * resetting its internal state, which includes removing this instance from the managers map. *

* To re-enable the SOCKS5 Bytestream feature invoke {@link #getBytestreamManager(Connection)}. * Using the file transfer API will automatically re-enable the SOCKS5 Bytestream feature. diff --git a/source/org/jivesoftware/smackx/commands/AdHocCommandManager.java b/source/org/jivesoftware/smackx/commands/AdHocCommandManager.java index 8f4eb65c5..71698e311 100755 --- a/source/org/jivesoftware/smackx/commands/AdHocCommandManager.java +++ b/source/org/jivesoftware/smackx/commands/AdHocCommandManager.java @@ -38,10 +38,13 @@ import org.jivesoftware.smackx.packet.DiscoverInfo; import org.jivesoftware.smackx.packet.DiscoverInfo.Identity; import org.jivesoftware.smackx.packet.DiscoverItems; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; /** @@ -70,7 +73,7 @@ public class AdHocCommandManager { * pair for every active connection. */ private static Map instances = - new ConcurrentHashMap(); + Collections.synchronizedMap(new WeakHashMap()); /** * Register the listener for all the connection creations. When a new @@ -92,26 +95,23 @@ public class AdHocCommandManager { * @param connection the XMPP connection. * @return the AdHocCommandManager associated with the connection. */ - public static AdHocCommandManager getAddHocCommandsManager(Connection connection) { - return instances.get(connection); + public static synchronized AdHocCommandManager getAddHocCommandsManager(Connection connection) { + AdHocCommandManager ahcm = instances.get(connection); + if (ahcm == null) ahcm = new AdHocCommandManager(connection); + return ahcm; } - /** - * Thread that reaps stale sessions. - */ - private Thread sessionsSweeper; - /** * The Connection that this instances of AdHocCommandManager manages */ - private Connection connection; + private final WeakReference connection; /** * Map a command node with its AdHocCommandInfo. Note: Key=command node, * Value=command. Command node matches the node attribute sent by command * requesters. */ - private Map commands = new ConcurrentHashMap(); + private final Map commands = new ConcurrentHashMap(); /** * Map a command session ID with the instance LocalCommand. The LocalCommand @@ -119,175 +119,22 @@ public class AdHocCommandManager { * the command execution. Note: Key=session ID, Value=LocalCommand. Session * ID matches the sessionid attribute sent by command responders. */ - private Map executingCommands = new ConcurrentHashMap(); + private final Map executingCommands = new ConcurrentHashMap(); + + private final ServiceDiscoveryManager serviceDiscoveryManager; + + /** + * Thread that reaps stale sessions. + */ + private Thread sessionsSweeper; private AdHocCommandManager(Connection connection) { - super(); - this.connection = connection; - init(); - } - - /** - * Registers a new command with this command manager, which is related to a - * connection. The node is an unique identifier of that command for - * the connection related to this command manager. The name is the - * human readable name of the command. The class is the class of - * the command, which must extend {@link LocalCommand} and have a default - * constructor. - * - * @param node the unique identifier of the command. - * @param name the human readable name of the command. - * @param clazz the class of the command, which must extend {@link LocalCommand}. - */ - public void registerCommand(String node, String name, final Class clazz) { - registerCommand(node, name, new LocalCommandFactory() { - public LocalCommand getInstance() throws InstantiationException, IllegalAccessException { - return clazz.newInstance(); - } - }); - } - - /** - * Registers a new command with this command manager, which is related to a - * connection. The node is an unique identifier of that - * command for the connection related to this command manager. The name - * is the human readeale name of the command. The factory generates - * new instances of the command. - * - * @param node the unique identifier of the command. - * @param name the human readable name of the command. - * @param factory a factory to create new instances of the command. - */ - public void registerCommand(String node, final String name, LocalCommandFactory factory) { - AdHocCommandInfo commandInfo = new AdHocCommandInfo(node, name, connection.getUser(), factory); - - commands.put(node, commandInfo); - // Set the NodeInformationProvider that will provide information about - // the added command - ServiceDiscoveryManager.getInstanceFor(connection).setNodeInformationProvider(node, - new NodeInformationProvider() { - public List getNodeItems() { - return null; - } - - public List getNodeFeatures() { - List answer = new ArrayList(); - answer.add(DISCO_NAMESPACE); - // TODO: check if this service is provided by the - // TODO: current connection. - answer.add("jabber:x:data"); - return answer; - } - - public List getNodeIdentities() { - List answer = new ArrayList(); - DiscoverInfo.Identity identity = new DiscoverInfo.Identity( - "automation", name, "command-node"); - answer.add(identity); - return answer; - } - - @Override - public List getNodePacketExtensions() { - return null; - } - - }); - } - - /** - * Discover the commands of an specific JID. The jid is a - * full JID. - * - * @param jid the full JID to retrieve the commands for. - * @return the discovered items. - * @throws XMPPException if the operation failed for some reason. - */ - public DiscoverItems discoverCommands(String jid) throws XMPPException { - ServiceDiscoveryManager serviceDiscoveryManager = ServiceDiscoveryManager - .getInstanceFor(connection); - return serviceDiscoveryManager.discoverItems(jid, discoNode); - } - - /** - * Publish the commands to an specific JID. - * - * @param jid the full JID to publish the commands to. - * @throws XMPPException if the operation failed for some reason. - */ - public void publishCommands(String jid) throws XMPPException { - ServiceDiscoveryManager serviceDiscoveryManager = ServiceDiscoveryManager - .getInstanceFor(connection); - - // Collects the commands to publish as items - DiscoverItems discoverItems = new DiscoverItems(); - Collection xCommandsList = getRegisteredCommands(); - - for (AdHocCommandInfo info : xCommandsList) { - DiscoverItems.Item item = new DiscoverItems.Item(info.getOwnerJID()); - item.setName(info.getName()); - item.setNode(info.getNode()); - discoverItems.addItem(item); - } - - serviceDiscoveryManager.publishItems(jid, discoNode, discoverItems); - } - - /** - * Returns a command that represents an instance of a command in a remote - * host. It is used to execute remote commands. The concept is similar to - * RMI. Every invocation on this command is equivalent to an invocation in - * the remote command. - * - * @param jid the full JID of the host of the remote command - * @param node the identifier of the command - * @return a local instance equivalent to the remote command. - */ - public RemoteCommand getRemoteCommand(String jid, String node) { - return new RemoteCommand(connection, node, jid); - } - - /** - *

    - *
  • Adds listeners to the connection
    • - *
  • Registers the ad-hoc command feature to the ServiceDiscoveryManager
    • - *
  • Registers the items of the feature
    • - *
  • Adds packet listeners to handle execution requests
    • - *
  • Creates and start the session sweeper
    • - *
- */ - private void init() { + this.connection = new WeakReference(connection); + this.serviceDiscoveryManager = ServiceDiscoveryManager.getInstanceFor(connection); + // Register the new instance and associate it with the connection instances.put(connection, this); - // Add a listener to the connection that removes the registered instance - // when the connection is closed - connection.addConnectionListener(new ConnectionListener() { - public void connectionClosed() { - // Unregister this instance since the connection has been closed - instances.remove(connection); - } - - public void connectionClosedOnError(Exception e) { - // Unregister this instance since the connection has been closed - instances.remove(connection); - } - - public void reconnectionSuccessful() { - // Register this instance since the connection has been - // reestablished - instances.put(connection, AdHocCommandManager.this); - } - - public void reconnectingIn(int seconds) { - // Nothing to do - } - - public void reconnectionFailed(Exception e) { - // Nothing to do - } - }); - // Add the feature to the service discovery manage to show that this // connection supports the AdHoc-Commands protocol. // This information will be used when another client tries to @@ -346,6 +193,121 @@ public class AdHocCommandManager { sessionsSweeper = null; } + /** + * Registers a new command with this command manager, which is related to a + * connection. The node is an unique identifier of that command for + * the connection related to this command manager. The name is the + * human readable name of the command. The class is the class of + * the command, which must extend {@link LocalCommand} and have a default + * constructor. + * + * @param node the unique identifier of the command. + * @param name the human readable name of the command. + * @param clazz the class of the command, which must extend {@link LocalCommand}. + */ + public void registerCommand(String node, String name, final Class clazz) { + registerCommand(node, name, new LocalCommandFactory() { + public LocalCommand getInstance() throws InstantiationException, IllegalAccessException { + return clazz.newInstance(); + } + }); + } + + /** + * Registers a new command with this command manager, which is related to a + * connection. The node is an unique identifier of that + * command for the connection related to this command manager. The name + * is the human readeale name of the command. The factory generates + * new instances of the command. + * + * @param node the unique identifier of the command. + * @param name the human readable name of the command. + * @param factory a factory to create new instances of the command. + */ + public void registerCommand(String node, final String name, LocalCommandFactory factory) { + AdHocCommandInfo commandInfo = new AdHocCommandInfo(node, name, connection.get().getUser(), factory); + + commands.put(node, commandInfo); + // Set the NodeInformationProvider that will provide information about + // the added command + serviceDiscoveryManager.setNodeInformationProvider(node, + new NodeInformationProvider() { + public List getNodeItems() { + return null; + } + + public List getNodeFeatures() { + List answer = new ArrayList(); + answer.add(DISCO_NAMESPACE); + // TODO: check if this service is provided by the + // TODO: current connection. + answer.add("jabber:x:data"); + return answer; + } + + public List getNodeIdentities() { + List answer = new ArrayList(); + DiscoverInfo.Identity identity = new DiscoverInfo.Identity( + "automation", name, "command-node"); + answer.add(identity); + return answer; + } + + @Override + public List getNodePacketExtensions() { + return null; + } + + }); + } + + /** + * Discover the commands of an specific JID. The jid is a + * full JID. + * + * @param jid the full JID to retrieve the commands for. + * @return the discovered items. + * @throws XMPPException if the operation failed for some reason. + */ + public DiscoverItems discoverCommands(String jid) throws XMPPException { + return serviceDiscoveryManager.discoverItems(jid, discoNode); + } + + /** + * Publish the commands to an specific JID. + * + * @param jid the full JID to publish the commands to. + * @throws XMPPException if the operation failed for some reason. + */ + public void publishCommands(String jid) throws XMPPException { + // Collects the commands to publish as items + DiscoverItems discoverItems = new DiscoverItems(); + Collection xCommandsList = getRegisteredCommands(); + + for (AdHocCommandInfo info : xCommandsList) { + DiscoverItems.Item item = new DiscoverItems.Item(info.getOwnerJID()); + item.setName(info.getName()); + item.setNode(info.getNode()); + discoverItems.addItem(item); + } + + serviceDiscoveryManager.publishItems(jid, discoNode, discoverItems); + } + + /** + * Returns a command that represents an instance of a command in a remote + * host. It is used to execute remote commands. The concept is similar to + * RMI. Every invocation on this command is equivalent to an invocation in + * the remote command. + * + * @param jid the full JID of the host of the remote command + * @param node the identifier of the command + * @return a local instance equivalent to the remote command. + */ + public RemoteCommand getRemoteCommand(String jid, String node) { + return new RemoteCommand(connection.get(), node, jid); + } + /** * Process the AdHoc-Command packet that request the execution of some * action of a command. If this is the first request, this method checks, @@ -491,7 +453,7 @@ public class AdHocCommandManager { } // Sends the response packet - connection.sendPacket(response); + connection.get().sendPacket(response); } catch (XMPPException e) { @@ -607,7 +569,7 @@ public class AdHocCommandManager { executingCommands.remove(sessionId); } - connection.sendPacket(response); + connection.get().sendPacket(response); } catch (XMPPException e) { // If there is an exception caused by the next, complete, @@ -665,7 +627,7 @@ public class AdHocCommandManager { private void respondError(AdHocCommandData response, XMPPError error) { response.setType(IQ.Type.ERROR); response.setError(error); - connection.sendPacket(response); + connection.get().sendPacket(response); } /** diff --git a/source/org/jivesoftware/smackx/entitycaps/EntityCapsManager.java b/source/org/jivesoftware/smackx/entitycaps/EntityCapsManager.java index 1da222efd..1893288ab 100644 --- a/source/org/jivesoftware/smackx/entitycaps/EntityCapsManager.java +++ b/source/org/jivesoftware/smackx/entitycaps/EntityCapsManager.java @@ -234,9 +234,7 @@ public class EntityCapsManager { connection.addConnectionListener(new ConnectionListener() { public void connectionClosed() { - // Unregister this instance since the connection has been closed presenceSend = false; - instances.remove(weakRefConnection.get()); } public void connectionClosedOnError(Exception e) { diff --git a/source/org/jivesoftware/smackx/ping/PingManager.java b/source/org/jivesoftware/smackx/ping/PingManager.java index 2da9dcadc..6882545fd 100644 --- a/source/org/jivesoftware/smackx/ping/PingManager.java +++ b/source/org/jivesoftware/smackx/ping/PingManager.java @@ -19,7 +19,6 @@ 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; @@ -91,7 +90,7 @@ public class PingManager { // The ServiceDiscoveryManager was not pre-initialized if (sdm == null) - sdm = new ServiceDiscoveryManager(connection); + sdm = ServiceDiscoveryManager.getInstanceFor(connection); sdm.addFeature(Ping.NAMESPACE); diff --git a/test-unit/org/jivesoftware/smackx/bytestreams/socks5/InitiationListenerTest.java b/test-unit/org/jivesoftware/smackx/bytestreams/socks5/InitiationListenerTest.java index c45776c8d..df70ef5ec 100644 --- a/test-unit/org/jivesoftware/smackx/bytestreams/socks5/InitiationListenerTest.java +++ b/test-unit/org/jivesoftware/smackx/bytestreams/socks5/InitiationListenerTest.java @@ -59,7 +59,7 @@ public class InitiationListenerTest { connection = mock(Connection.class); // create service discovery manager for mocked connection - new ServiceDiscoveryManager(connection); + ServiceDiscoveryManager.getInstanceFor(connection); // initialize Socks5ByteStreamManager to get the InitiationListener byteStreamManager = Socks5BytestreamManager.getBytestreamManager(connection); diff --git a/test-unit/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java b/test-unit/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java index 5c7d9493c..1939ce54b 100644 --- a/test-unit/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java +++ b/test-unit/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java @@ -95,8 +95,8 @@ public class Socks5ByteStreamManagerTest { * create service discovery managers for the connections because the * ConnectionCreationListener is not called when creating mocked connections */ - new ServiceDiscoveryManager(connection1); - new ServiceDiscoveryManager(connection2); + ServiceDiscoveryManager.getInstanceFor(connection1); + ServiceDiscoveryManager.getInstanceFor(connection2); // get bytestream manager for the first connection twice Socks5BytestreamManager conn1ByteStreamManager1 = Socks5BytestreamManager.getBytestreamManager(connection1); diff --git a/test-unit/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java b/test-unit/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java index f9255241e..3140fd4ad 100644 --- a/test-unit/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java +++ b/test-unit/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java @@ -39,7 +39,7 @@ public class FileTransferNegotiatorTest { connection = new DummyConnection(); connection.connect(); connection.login("me", "secret"); - new ServiceDiscoveryManager(connection); + ServiceDiscoveryManager.getInstanceFor(connection); } @After diff --git a/test-unit/org/jivesoftware/smackx/receipts/DeliveryReceiptTest.java b/test-unit/org/jivesoftware/smackx/receipts/DeliveryReceiptTest.java index 6b0425a60..09d1aa320 100644 --- a/test-unit/org/jivesoftware/smackx/receipts/DeliveryReceiptTest.java +++ b/test-unit/org/jivesoftware/smackx/receipts/DeliveryReceiptTest.java @@ -70,7 +70,7 @@ public class DeliveryReceiptTest { @Test public void receiptManagerListenerTest() throws Exception { DummyConnection c = new DummyConnection(); - ServiceDiscoveryManager sdm = new ServiceDiscoveryManager(c); + ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(c); DeliveryReceiptManager drm = DeliveryReceiptManager.getInstanceFor(c); TestReceiptReceivedListener rrl = new TestReceiptReceivedListener(); @@ -100,7 +100,7 @@ public class DeliveryReceiptTest { @Test public void receiptManagerAutoReplyTest() throws Exception { DummyConnection c = new DummyConnection(); - ServiceDiscoveryManager sdm = new ServiceDiscoveryManager(c); + ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(c); DeliveryReceiptManager drm = DeliveryReceiptManager.getInstanceFor(c); drm.enableAutoReceipts(); diff --git a/test-unit/org/jivesoftware/util/ConnectionUtils.java b/test-unit/org/jivesoftware/util/ConnectionUtils.java index 3834159a4..ea986791f 100644 --- a/test-unit/org/jivesoftware/util/ConnectionUtils.java +++ b/test-unit/org/jivesoftware/util/ConnectionUtils.java @@ -86,7 +86,7 @@ public class ConnectionUtils { when(collector.nextResult()).thenAnswer(answer); // initialize service discovery manager for this connection - new ServiceDiscoveryManager(connection); + ServiceDiscoveryManager.getInstanceFor(connection); return connection; }