Cleanup carbons, forwarded and a few others API

Adopt to common design patterns in Smack:

- getFrom(Packet) in Packetextensions
- INSTANCES.put() in getInstanceFor()
- ELEMENT instead of ELEMENT_NAME
- Use XmlStringBuilder

smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java

@@ -160,6 +160,20 @@ public class XmlStringBuilder implements Appendable, CharSequence {
         return this;
     }
 
+    /**
+     * Add the given attribute if value => 0
+     *
+     * @param name
+     * @param value
+     * @return a reference to this object
+     */
+    public XmlStringBuilder optLongAttribute(String name, Long value) {
+        if (value >= 0) {
+            attribute(name, Long.toString(value));
+        }
+        return this;
+    }
+
     public XmlStringBuilder xmlnsAttribute(String value) {
         optAttribute("xmlns", value);
         return this;

smack-experimental/src/main/java/org/jivesoftware/smackx/carbons/CarbonManager.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2013 Georg Lukas
+ * Copyright 2013-2014 Georg Lukas
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
  */
 package org.jivesoftware.smackx.carbons;
 
-import java.util.Collections;
 import java.util.Map;
 import java.util.WeakHashMap;
 
@@ -49,8 +48,7 @@ import org.jivesoftware.smackx.disco.ServiceDiscoveryManager;
  */
 public class CarbonManager extends Manager {
 
-    private static Map<XMPPConnection, CarbonManager> instances =
+    private static Map<XMPPConnection, CarbonManager> INSTANCES = new WeakHashMap<XMPPConnection, CarbonManager>();
-            Collections.synchronizedMap(new WeakHashMap<XMPPConnection, CarbonManager>());
 
     static {
         XMPPConnectionRegistry.addConnectionCreationListener(new ConnectionCreationListener() {
@@ -66,7 +64,6 @@ public class CarbonManager extends Manager {
         super(connection);
         ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(connection);
         sdm.addFeature(CarbonExtension.NAMESPACE);
-        instances.put(connection, this);
     }
 
     /**
@@ -77,16 +74,17 @@ public class CarbonManager extends Manager {
      * @return a CarbonManager instance
      */
     public static synchronized CarbonManager getInstanceFor(XMPPConnection connection) {
-        CarbonManager carbonManager = instances.get(connection);
+        CarbonManager carbonManager = INSTANCES.get(connection);
 
         if (carbonManager == null) {
             carbonManager = new CarbonManager(connection);
+            INSTANCES.put(connection, carbonManager);
         }
 
         return carbonManager;
     }
 
-    private IQ carbonsEnabledIQ(final boolean new_state) {
+    private static IQ carbonsEnabledIQ(final boolean new_state) {
         IQ setIQ = new IQ() {
             public String getChildElementXML() {
                 return "<" + (new_state? "enable" : "disable") + " xmlns='" + CarbonExtension.NAMESPACE + "'/>";
@@ -100,10 +98,11 @@ public class CarbonManager extends Manager {
      * Returns true if XMPP Carbons are supported by the server.
      * 
      * @return true if supported
-     * @throws SmackException if there was no response from the server.
+     * @throws NotConnectedException 
-     * @throws XMPPException 
+     * @throws XMPPErrorException 
+     * @throws NoResponseException 
      */
-    public boolean isSupportedByServer() throws XMPPException, SmackException {
+    public boolean isSupportedByServer() throws NoResponseException, XMPPErrorException, NotConnectedException {
         return ServiceDiscoveryManager.getInstanceFor(connection()).supportsFeature(
                         connection().getServiceName(), CarbonExtension.NAMESPACE);
     }
@@ -184,20 +183,6 @@ public class CarbonManager extends Manager {
         return this.enabled_state;
     }
 
-    /**
-     * Obtain a Carbon from a message, if available.
-     *
-     * @param msg Message object to check for carbons
-     *
-     * @return a Carbon if available, null otherwise.
-     */
-    public static CarbonExtension getCarbon(Message msg) {
-        CarbonExtension cc = (CarbonExtension)msg.getExtension("received", CarbonExtension.NAMESPACE);
-        if (cc == null)
-            cc = (CarbonExtension)msg.getExtension("sent", CarbonExtension.NAMESPACE);
-        return cc;
-    }
-
     /**
      * Mark a message as "private", so it will not be carbon-copied.
      *

smack-experimental/src/main/java/org/jivesoftware/smackx/carbons/packet/CarbonExtension.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2013 Georg Lukas
+ * Copyright 2013-2014 Georg Lukas
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,7 +16,9 @@
  */
 package org.jivesoftware.smackx.carbons.packet;
 
+import org.jivesoftware.smack.packet.Message;
 import org.jivesoftware.smack.packet.PacketExtension;
+import org.jivesoftware.smack.util.XmlStringBuilder;
 import org.jivesoftware.smackx.forward.Forwarded;
 
 /**
@@ -33,8 +35,8 @@ import org.jivesoftware.smackx.forward.Forwarded;
 public class CarbonExtension implements PacketExtension {
     public static final String NAMESPACE = "urn:xmpp:carbons:2";
 
-    private Direction dir;
+    private final Direction dir;
-    private Forwarded fwd;
+    private final Forwarded fwd;
 
     /**
      * Construct a Carbon message extension.
@@ -67,7 +69,7 @@ public class CarbonExtension implements PacketExtension {
 
     @Override
     public String getElementName() {
-        return dir.toString();
+        return dir.name();
     }
 
     @Override
@@ -76,15 +78,29 @@ public class CarbonExtension implements PacketExtension {
     }
 
     @Override
-    public String toXML() {
+    public XmlStringBuilder toXML() {
-        StringBuilder buf = new StringBuilder();
+        XmlStringBuilder xml = new XmlStringBuilder(this);
-        buf.append("<").append(getElementName()).append(" xmlns=\"")
+        xml.rightAngelBracket();
-                .append(getNamespace()).append("\">");
+        xml.append(fwd.toXML());
+        xml.closeElement(this);
+        return xml;
+    }
 
-        buf.append(fwd.toXML());
+    /**
-
+     * Obtain a Carbon from a message, if available.
-        buf.append("</").append(getElementName()).append(">");
+     * <p>
-        return buf.toString();
+     * Only {@link Message} instances can contain a Carbon extensions.
+     * </p>
+     *
+     * @param msg Message object to check for carbons
+     *
+     * @return a Carbon if available, null otherwise.
+     */
+    public static CarbonExtension getFrom(Message msg) {
+        CarbonExtension cc = msg.getExtension(Direction.received.name(), CarbonExtension.NAMESPACE);
+        if (cc == null)
+            cc = msg.getExtension(Direction.sent.name(), CarbonExtension.NAMESPACE);
+        return cc;
     }
 
     /**
@@ -102,16 +118,19 @@ public class CarbonExtension implements PacketExtension {
     public static class Private implements PacketExtension {
         public static final String ELEMENT = "private";
 
+        @Override
         public String getElementName() {
             return ELEMENT;
         }
 
+        @Override
         public String getNamespace() {
             return CarbonExtension.NAMESPACE;
         }
 
+        @Override
         public String toXML() {
-            return "<" + ELEMENT + " xmlns=\"" + CarbonExtension.NAMESPACE + "\"/>";
+            return "<" + ELEMENT + " xmlns='" + CarbonExtension.NAMESPACE + "'/>";
         }
     }
 }

smack-experimental/src/main/java/org/jivesoftware/smackx/carbons/provider/CarbonManagerProvider.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2013 Georg Lukas
+ * Copyright 2013-2014 Georg Lukas
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,7 +26,7 @@ import org.xmlpull.v1.XmlPullParser;
 
 /**
  * This class implements the {@link PacketExtensionProvider} to parse
- * cabon copied messages from a packet.  It will return a {@link CarbonExtension} packet extension.
+ * carbon copied messages from a packet.  It will return a {@link CarbonExtension} packet extension.
  * 
  * @author Georg Lukas
  *
@@ -41,7 +41,7 @@ public class CarbonManagerProvider implements PacketExtensionProvider {
         while (!done) {
             int eventType = parser.next();
             if (eventType == XmlPullParser.START_TAG && parser.getName().equals("forwarded")) {
-                fwd = (Forwarded) PacketParserUtils.parsePacketExtension(Forwarded.ELEMENT_NAME, Forwarded.NAMESPACE, parser);
+                fwd = (Forwarded) PacketParserUtils.parsePacketExtension(Forwarded.ELEMENT, Forwarded.NAMESPACE, parser);
             }
             else if (eventType == XmlPullParser.END_TAG && dir == Direction.valueOf(parser.getName()))
                 done = true;

smack-experimental/src/test/java/org/jivesoftware/smackx/carbons/CarbonTest.java

@@ -20,30 +20,23 @@ import static org.junit.Assert.assertEquals;
 
 import java.util.Properties;
 
-import org.jivesoftware.smack.provider.ProviderManager;
 import org.jivesoftware.smack.util.PacketParserUtils;
+import org.jivesoftware.smackx.ExperimentalInitializerTest;
 import org.jivesoftware.smackx.carbons.packet.CarbonExtension;
 import org.jivesoftware.smackx.carbons.provider.CarbonManagerProvider;
 import org.jivesoftware.smackx.forward.Forwarded;
-import org.jivesoftware.smackx.forward.provider.ForwardedProvider;
-import org.junit.BeforeClass;
 import org.junit.Test;
 import org.xmlpull.v1.XmlPullParser;
 
 import com.jamesmurty.utils.XMLBuilder;
 
-public class CarbonTest {
+public class CarbonTest extends ExperimentalInitializerTest {
 
     private static Properties outputProperties = new Properties();
     static {
         outputProperties.put(javax.xml.transform.OutputKeys.OMIT_XML_DECLARATION, "yes");
     }
 
-    @BeforeClass
-    public static void setup() {
-        ProviderManager.addExtensionProvider("forwarded", "urn:xmpp:forward:0", new ForwardedProvider());
-    }
-    
     @Test
     public void carbonSentTest() throws Exception {
         XmlPullParser parser;
@@ -66,7 +59,7 @@ public class CarbonTest {
         assertEquals(CarbonExtension.Direction.sent, cc.getDirection());
         
         // no delay in packet
-        assertEquals(null, fwd.getDelayInfo());
+        assertEquals(null, fwd.getDelayInformation());
         
         // check message
         assertEquals("romeo@montague.com", fwd.getForwardedPacket().getFrom());

smack-extensions/src/main/java/org/jivesoftware/smackx/chatstates/packet/ChatStateExtension.java

@@ -20,6 +20,7 @@ package org.jivesoftware.smackx.chatstates.packet;
 import org.jivesoftware.smackx.chatstates.ChatState;
 import org.jivesoftware.smack.packet.PacketExtension;
 import org.jivesoftware.smack.provider.PacketExtensionProvider;
+import org.jivesoftware.smack.util.XmlStringBuilder;
 import org.xmlpull.v1.XmlPullParser;
 
 /**
@@ -31,7 +32,9 @@ import org.xmlpull.v1.XmlPullParser;
  */
 public class ChatStateExtension implements PacketExtension {
 
-    private ChatState state;
+    public static final String NAMESPACE = "http://jabber.org/protocol/chatstates";
+
+    private final ChatState state;
 
     /**
      * Default constructor. The argument provided is the state that the extension will represent.
@@ -42,16 +45,21 @@ public class ChatStateExtension implements PacketExtension {
         this.state = state;
     }
 
+    @Override
     public String getElementName() {
         return state.name();
     }
 
+    @Override
     public String getNamespace() {
-        return "http://jabber.org/protocol/chatstates";
+        return NAMESPACE;
     }
 
-    public String toXML() {
+    @Override
-        return "<" + getElementName() + " xmlns=\"" + getNamespace() + "\" />";
+    public XmlStringBuilder toXML() {
+        XmlStringBuilder xml = new XmlStringBuilder(this);
+        xml.closeEmptyElement();
+        return xml;
     }
 
     public static class Provider implements PacketExtensionProvider {

smack-extensions/src/main/java/org/jivesoftware/smackx/delay/DelayInformationManager.java

@@ -44,7 +44,7 @@ public class DelayInformationManager {
      * @return the Delayed Delivery information or <code>null</code>
      */
     public static DelayInformation getXep203DelayInformation(Packet packet) {
-        return (DelayInformation) packet.getExtension(DelayInformation.ELEMENT, DelayInformation.NAMESPACE);
+        return DelayInformation.getFrom(packet);
     }
 
     /**
@@ -56,7 +56,7 @@ public class DelayInformationManager {
      * @return the Delayed Delivery information or <code>null</code>
      */
     public static DelayInformation getLegacyDelayInformation(Packet packet) {
-        return (DelayInformation) packet.getExtension(LEGACY_DELAYED_DELIVERY_ELEMENT, LEGACY_DELAYED_DELIVERY_NAMESPACE);
+        return packet.getExtension(LEGACY_DELAYED_DELIVERY_ELEMENT, LEGACY_DELAYED_DELIVERY_NAMESPACE);
     }
 
     /**

smack-extensions/src/main/java/org/jivesoftware/smackx/delay/packet/DelayInformation.java

@@ -18,6 +18,7 @@ package org.jivesoftware.smackx.delay.packet;
 
 import java.util.Date;
 
+import org.jivesoftware.smack.packet.Packet;
 import org.jivesoftware.smack.packet.PacketExtension;
 import org.jivesoftware.smack.util.XmlStringBuilder;
 import org.jxmpp.util.XmppDateTime;
@@ -96,16 +97,17 @@ public class DelayInformation implements PacketExtension {
     }
 
     @Override
-    public CharSequence toXML() {
+    public XmlStringBuilder toXML() {
         XmlStringBuilder xml = new XmlStringBuilder(this);
         xml.attribute("stamp", XmppDateTime.formatXEP0082Date(stamp));
         xml.optAttribute("from", from);
         xml.rightAngelBracket();
-        if (reason != null) {
+        xml.optAppend(reason);
-            xml.escape(reason);
-        }
         xml.closeElement(this);
         return xml;
     }
 
+    public static DelayInformation getFrom(Packet packet) {
+        return packet.getExtension(ELEMENT, NAMESPACE);
+    }
 }

smack-extensions/src/main/java/org/jivesoftware/smackx/forward/Forwarded.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2013 Georg Lukas
+ * Copyright 2013-2014 Georg Lukas
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@ package org.jivesoftware.smackx.forward;
 
 import org.jivesoftware.smack.packet.Packet;
 import org.jivesoftware.smack.packet.PacketExtension;
+import org.jivesoftware.smack.util.XmlStringBuilder;
 import org.jivesoftware.smackx.delay.packet.DelayInformation;
 
 /**
@@ -27,7 +28,7 @@ import org.jivesoftware.smackx.delay.packet.DelayInformation;
  */
 public class Forwarded implements PacketExtension {
     public static final String NAMESPACE = "urn:xmpp:forward:0";
-    public static final String ELEMENT_NAME = "forwarded";
+    public static final String ELEMENT = "forwarded";
 
     private DelayInformation delay;
     private Packet forwardedPacket;
@@ -54,7 +55,7 @@ public class Forwarded implements PacketExtension {
 
     @Override
     public String getElementName() {
-        return ELEMENT_NAME;
+        return ELEMENT;
     }
 
     @Override
@@ -63,17 +64,17 @@ public class Forwarded implements PacketExtension {
     }
 
     @Override
-    public String toXML() {
+    public XmlStringBuilder toXML() {
-        StringBuilder buf = new StringBuilder();
+        XmlStringBuilder xml = new XmlStringBuilder(this);
-        buf.append("<").append(getElementName()).append(" xmlns=\"")
+        xml.rightAngelBracket();
-                .append(getNamespace()).append("\">");
+        xml.optElement(getDelayInformation());
+        xml.append(forwardedPacket.toXML());
+        xml.closeElement(this);
+        return xml;
+    }
 
-        if (delay != null)
+    public static Forwarded getFrom(Packet packet) {
-            buf.append(delay.toXML());
+        return packet.getExtension(ELEMENT, NAMESPACE);
-        buf.append(forwardedPacket.toXML());
-
-        buf.append("</").append(getElementName()).append(">");
-        return buf.toString();
     }
 
     /**

smack-extensions/src/main/java/org/jivesoftware/smackx/forward/provider/ForwardedProvider.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2013 Georg Lukas
+ * Copyright 2013-2014 Georg Lukas
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -45,7 +45,7 @@ public class ForwardedProvider implements PacketExtensionProvider {
                     packet = PacketParserUtils.parseMessage(parser);
                 else throw new Exception("Unsupported forwarded packet type: " + parser.getName());
             }
-            else if (eventType == XmlPullParser.END_TAG && parser.getName().equals(Forwarded.ELEMENT_NAME))
+            else if (eventType == XmlPullParser.END_TAG && parser.getName().equals(Forwarded.ELEMENT))
                 done = true;
         }
         if (packet == null)

smack-extensions/src/main/java/org/jivesoftware/smackx/iqlast/packet/LastActivity.java

@@ -54,11 +54,10 @@ public class LastActivity extends IQ {
     @Override
     public XmlStringBuilder getChildElementXML() {
         XmlStringBuilder xml = new XmlStringBuilder();
-        xml.halfOpenElement("query");
+        xml.halfOpenElement(IQ.QUERY_ELEMENT);
         xml.xmlnsAttribute(NAMESPACE);
-        if (lastActivity != -1) {
+        xml.optLongAttribute("seconds", lastActivity);
-            xml.attribute("seconds", Long.toString(lastActivity));
+
-        }
         // We don't support adding the optional message attribute, because it is usually only added
         // by XMPP servers and not by client entities.
         xml.closeEmptyElement();
@@ -108,10 +107,6 @@ public class LastActivity extends IQ {
         }
 
         public IQ parseIQ(XmlPullParser parser) throws SmackException, XmlPullParserException {
-            if (parser.getEventType() != XmlPullParser.START_TAG) {
-                throw new SmackException("Parser not in proper position, or bad XML.");
-            }
-
             LastActivity lastActivity = new LastActivity();
             String seconds = parser.getAttributeValue("", "seconds");
             if (seconds != null) {

smack-extensions/src/main/java/org/jivesoftware/smackx/iqversion/VersionManager.java

@@ -17,7 +17,6 @@
 
 package org.jivesoftware.smackx.iqversion;
 
-import java.util.Collections;
 import java.util.Map;
 import java.util.WeakHashMap;
 
@@ -50,8 +49,7 @@ import org.jivesoftware.smackx.iqversion.packet.Version;
  * @author Georg Lukas
  */
 public class VersionManager extends Manager {
-    private static final Map<XMPPConnection, VersionManager> instances =
+    private static final Map<XMPPConnection, VersionManager> INSTANCES = new WeakHashMap<XMPPConnection, VersionManager>();
-            Collections.synchronizedMap(new WeakHashMap<XMPPConnection, VersionManager>());
 
     private static final PacketFilter PACKET_FILTER = new AndFilter(new PacketTypeFilter(Version.class), IQTypeFilter.GET);
 
@@ -59,7 +57,6 @@ public class VersionManager extends Manager {
 
     private VersionManager(final XMPPConnection connection) {
         super(connection);
-        instances.put(connection, this);
 
         ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(connection);
         sdm.addFeature(Version.NAMESPACE);
@@ -83,10 +80,11 @@ public class VersionManager extends Manager {
     }
 
     public static synchronized VersionManager getInstanceFor(XMPPConnection connection) {
-        VersionManager versionManager = instances.get(connection);
+        VersionManager versionManager = INSTANCES.get(connection);
 
         if (versionManager == null) {
             versionManager = new VersionManager(connection);
+            INSTANCES.put(connection, versionManager);
         }
 
         return versionManager;

smack-extensions/src/main/java/org/jivesoftware/smackx/time/EntityTimeManager.java

@@ -60,6 +60,7 @@ public class EntityTimeManager extends Manager {
         EntityTimeManager entityTimeManager = INSTANCES.get(connection);
         if (entityTimeManager == null) {
             entityTimeManager = new EntityTimeManager(connection);
+            INSTANCES.put(connection, entityTimeManager);
         }
         return entityTimeManager;
     }
@@ -68,7 +69,6 @@ public class EntityTimeManager extends Manager {
 
     private EntityTimeManager(XMPPConnection connection) {
         super(connection);
-        INSTANCES.put(connection, this);
         if (autoEnable)
             enable();
 

smack-extensions/src/test/java/org/jivesoftware/smackx/forward/ForwardedTest.java

@@ -51,7 +51,7 @@ public class ForwardedTest {
         fwd = (Forwarded) new ForwardedProvider().parseExtension(parser);
 
         // no delay in packet
-        assertEquals(null, fwd.getDelayInfo());
+        assertEquals(null, fwd.getDelayInformation());
         
         // check message
         assertEquals("romeo@montague.com", fwd.getForwardedPacket().getFrom());