Improve DeliveryReceiptManager

Use anonymous inner classes for packet listeners so that the
processPacket() method is not exposed as part of the Managers public
API.

And some small fixes.

smack-extensions/src/main/java/org/jivesoftware/smackx/receipts/DeliveryReceiptManager.java

@@ -29,7 +29,10 @@ import org.jivesoftware.smack.Manager;
 import org.jivesoftware.smack.PacketListener;
 import org.jivesoftware.smack.XMPPConnectionRegistry;
 import org.jivesoftware.smack.XMPPException;
+import org.jivesoftware.smack.filter.AndFilter;
 import org.jivesoftware.smack.filter.PacketExtensionFilter;
+import org.jivesoftware.smack.filter.PacketFilter;
+import org.jivesoftware.smack.filter.PacketTypeFilter;
 import org.jivesoftware.smack.packet.Message;
 import org.jivesoftware.smack.packet.Packet;
 import org.jivesoftware.smackx.disco.ServiceDiscoveryManager;
@@ -40,8 +43,14 @@ import org.jivesoftware.smackx.disco.ServiceDiscoveryManager;
  * automatic DeliveryReceipt transmission.
  *
  * @author Georg Lukas
+ * @see <a href="http://xmpp.org/extensions/xep-0184.html">XEP-0184: Message Delivery Receipts</a>
  */
-public class DeliveryReceiptManager extends Manager implements PacketListener {
+public class DeliveryReceiptManager extends Manager {
+
+    private static final PacketFilter MESSAGES_WITH_DEVLIERY_RECEIPT_REQUEST = new AndFilter(PacketTypeFilter.MESSAGE,
+                    new PacketExtensionFilter(new DeliveryReceiptRequest()));
+    private static final PacketFilter MESSAGES_WITH_DELIVERY_RECEIPT = new AndFilter(PacketTypeFilter.MESSAGE,
+                    new PacketExtensionFilter(DeliveryReceipt.ELEMENT, DeliveryReceipt.NAMESPACE));
 
     private static Map<XMPPConnection, DeliveryReceiptManager> instances = new WeakHashMap<XMPPConnection, DeliveryReceiptManager>();
 
@@ -61,8 +70,31 @@ public class DeliveryReceiptManager extends Manager implements PacketListener {
         ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(connection);
         sdm.addFeature(DeliveryReceipt.NAMESPACE);
 
-        // register listener for delivery receipts and requests
+        // Add the packet listener to handling incoming delivery receipts
-        connection.addAsyncPacketListener(this, new PacketExtensionFilter(DeliveryReceipt.NAMESPACE));
+        connection.addAsyncPacketListener(new PacketListener() {
+            @Override
+            public void processPacket(Packet packet) throws NotConnectedException {
+                DeliveryReceipt dr = DeliveryReceipt.from(packet);
+                // notify listeners of incoming receipt
+                for (ReceiptReceivedListener l : receiptReceivedListeners) {
+                    l.onReceiptReceived(packet.getFrom(), packet.getTo(), dr.getId(), packet);
+                }
+            }
+        }, MESSAGES_WITH_DELIVERY_RECEIPT);
+
+        // Add the packet listener to handle incoming delivery receipt requests
+        connection.addAsyncPacketListener(new PacketListener() {
+            @Override
+            public void processPacket(Packet packet) throws NotConnectedException {
+                // if enabled, automatically send a receipt
+                if (!auto_receipts_enabled) {
+                    return;
+                }
+                Message ack = new Message(packet.getFrom(), Message.Type.normal);
+                ack.addExtension(new DeliveryReceipt(packet.getPacketID()));
+                connection().sendPacket(ack);
+            }
+        }, MESSAGES_WITH_DEVLIERY_RECEIPT_REQUEST);
     }
 
     /**
@@ -96,29 +128,6 @@ public class DeliveryReceiptManager extends Manager implements PacketListener {
                         DeliveryReceipt.NAMESPACE);
     }
 
-    // handle incoming receipts and receipt requests
-    @Override
-    public void processPacket(Packet packet) throws NotConnectedException {
-        DeliveryReceipt dr = DeliveryReceipt.from(packet);
-        if (dr != null) {
-            // notify listeners of incoming receipt
-            for (ReceiptReceivedListener l : receiptReceivedListeners) {
-                l.onReceiptReceived(packet.getFrom(), packet.getTo(), dr.getId(), packet);
-            }
-        }
-
-        // if enabled, automatically send a receipt
-        if (auto_receipts_enabled) {
-            DeliveryReceiptRequest drr = DeliveryReceiptRequest.from(packet);
-            if (drr != null) {
-                XMPPConnection connection = connection();
-                Message ack = new Message(packet.getFrom(), Message.Type.normal);
-                ack.addExtension(new DeliveryReceipt(packet.getPacketID()));
-                connection.sendPacket(ack);
-            }
-        }
-    }
-
     /**
      * Configure whether the {@link DeliveryReceiptManager} should automatically
      * reply to incoming {@link DeliveryReceipt}s. By default, this feature is off.
@@ -170,14 +179,14 @@ public class DeliveryReceiptManager extends Manager implements PacketListener {
     }
 
     /**
-     * Test if a packet requires a delivery receipt.
+     * Test if a message requires a delivery receipt.
      *
-     * @param p Packet object to check for a DeliveryReceiptRequest
+     * @param message Packet object to check for a DeliveryReceiptRequest
      *
      * @return true if a delivery receipt was requested
      */
-    public static boolean hasDeliveryReceiptRequest(Packet p) {
+    public static boolean hasDeliveryReceiptRequest(Message message) {
-        return (DeliveryReceiptRequest.from(p) != null);
+        return (DeliveryReceiptRequest.from(message) != null);
     }
 
     /**

smack-extensions/src/test/java/org/jivesoftware/smackx/receipts/DeliveryReceiptTest.java

@@ -26,9 +26,9 @@ import java.util.Properties;
 import org.jivesoftware.smack.DummyConnection;
 import org.jivesoftware.smack.packet.Message;
 import org.jivesoftware.smack.packet.Packet;
+import org.jivesoftware.smack.test.util.WaitForPacketListener;
 import org.jivesoftware.smack.util.PacketParserUtils;
 import org.jivesoftware.smackx.InitExtensions;
-import org.jivesoftware.smackx.disco.ServiceDiscoveryManager;
 import org.junit.Test;
 import org.xmlpull.v1.XmlPullParser;
 
@@ -53,7 +53,7 @@ public class DeliveryReceiptTest extends InitExtensions {
             .asString(outputProperties);
         
         parser = PacketParserUtils.getParserFor(control);
-        Packet p = PacketParserUtils.parseMessage(parser);
+        Message p = PacketParserUtils.parseMessage(parser);
 
         DeliveryReceiptRequest drr = (DeliveryReceiptRequest)p.getExtension(
                         DeliveryReceiptRequest.ELEMENT, DeliveryReceipt.NAMESPACE);
@@ -71,8 +71,6 @@ public class DeliveryReceiptTest extends InitExtensions {
     public void receiptManagerListenerTest() throws Exception {
         DummyConnection c = new DummyConnection();
         c.connect();
-        // Ensure SDM is created for this connection
-        ServiceDiscoveryManager.getInstanceFor(c);
         DeliveryReceiptManager drm = DeliveryReceiptManager.getInstanceFor(c);
 
         TestReceiptReceivedListener rrl = new TestReceiptReceivedListener();
@@ -82,20 +80,18 @@ public class DeliveryReceiptTest extends InitExtensions {
         m.setFrom("julia@capulet.com");
         m.setPacketID("reply-id");
         m.addExtension(new DeliveryReceipt("original-test-id"));
-        drm.processPacket(m);
+        c.processPacket(m);
 
-        // ensure the listener got called
+        rrl.waitUntilInvocationOrTimeout();
-        assertEquals("original-test-id", rrl.receiptId);
     }
 
-    private static class TestReceiptReceivedListener implements ReceiptReceivedListener {
+    private static class TestReceiptReceivedListener extends WaitForPacketListener implements ReceiptReceivedListener {
-        public String receiptId = null;
         @Override
         public void onReceiptReceived(String fromJid, String toJid, String receiptId, Packet receipt) {
             assertEquals("julia@capulet.com", fromJid);
             assertEquals("romeo@montague.com", toJid);
             assertEquals("original-test-id", receiptId);
-            this.receiptId = receiptId;
+            reportInvoked();
         }
     }
 
@@ -103,8 +99,6 @@ public class DeliveryReceiptTest extends InitExtensions {
     public void receiptManagerAutoReplyTest() throws Exception {
         DummyConnection c = new DummyConnection();
         c.connect();
-        // Ensure SDM is created for this connection
-        ServiceDiscoveryManager.getInstanceFor(c);
         DeliveryReceiptManager drm = DeliveryReceiptManager.getInstanceFor(c);
 
         drm.enableAutoReceipts();
@@ -117,12 +111,10 @@ public class DeliveryReceiptTest extends InitExtensions {
         DeliveryReceiptRequest.addTo(m);
 
         // the DRM will send a reply-packet
-        assertEquals(0, c.getNumberOfSentPackets());
+        c.processPacket(m);
-        drm.processPacket(m);
-        assertEquals(1, c.getNumberOfSentPackets());
 
         Packet reply = c.getSentPacket();
-        DeliveryReceipt r = (DeliveryReceipt)reply.getExtension("received", "urn:xmpp:receipts");
+        DeliveryReceipt r = DeliveryReceipt.from(reply);
         assertEquals("romeo@montague.com", reply.getTo());
         assertEquals("test-receipt-request", r.getId());
     }