Merge pull request #160 from ibauersachs/parse-error-texts

Get descriptive text from error without lang

smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java

@@ -23,6 +23,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
+import org.jivesoftware.smack.util.Objects;
 import org.jivesoftware.smack.util.PacketUtil;
 import org.jivesoftware.smack.util.XmlStringBuilder;
 
@@ -85,6 +86,7 @@ public class AbstractError {
      * @return the descriptive text or null.
      */
     public String getDescriptiveText(String xmllang) {
+        Objects.requireNonNull(xmllang, "xmllang must not be null");
         return descriptiveTexts.get(xmllang);
     }
 
@@ -105,7 +107,8 @@ public class AbstractError {
             String xmllang = entry.getKey();
             String text = entry.getValue();
             xml.halfOpenElement("text").xmlnsAttribute(textNamespace)
-                    .xmllangAttribute(xmllang).rightAngleBracket();
+                    .optXmlLangAttribute(xmllang)
+                    .rightAngleBracket();
             xml.escape(text);
             xml.closeElement("text");
         }
@@ -120,6 +123,15 @@ public class AbstractError {
         protected List<ExtensionElement> extensions;
 
         public B setDescriptiveTexts(Map<String, String> descriptiveTexts) {
+            if (descriptiveTexts == null) {
+                this.descriptiveTexts = null;
+                return getThis();
+            }
+            for (String key : descriptiveTexts.keySet()) {
+                if (key == null) {
+                    throw new IllegalArgumentException("descriptiveTexts cannot contain null key");
+                }
+            }
             if (this.descriptiveTexts == null) {
                 this.descriptiveTexts = descriptiveTexts;
             }

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

@@ -749,6 +749,12 @@ public class PacketParserUtils {
             descriptiveTexts = new HashMap<String, String>();
         }
         String xmllang = getLanguageAttribute(parser);
+        if (xmllang == null) {
+            // XMPPError assumes the default locale, 'en', or the empty string.
+            // Establish the invariant that there is never null as a key.
+            xmllang = "";
+        }
+
         String text = parser.nextText();
         String previousValue = descriptiveTexts.put(xmllang, text);
         assert (previousValue == null);

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

@@ -361,7 +361,7 @@ public class XmlStringBuilder implements Appendable, CharSequence {
     }
 
     public XmlStringBuilder optXmlLangAttribute(String lang) {
-        if (lang != null) {
+        if (!StringUtils.isNullOrEmpty(lang)) {
             xmllangAttribute(lang);
         }
         return this;

smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java

@@ -25,7 +25,9 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Properties;
 
 import javax.xml.parsers.FactoryConfigurationError;
@@ -35,6 +37,7 @@ import javax.xml.transform.TransformerException;
 import org.jivesoftware.smack.packet.Message;
 import org.jivesoftware.smack.packet.Presence;
 import org.jivesoftware.smack.packet.Stanza;
+import org.jivesoftware.smack.packet.XMPPError;
 import org.jivesoftware.smack.sasl.SASLError;
 import org.jivesoftware.smack.sasl.packet.SaslStreamElements;
 import org.jivesoftware.smack.sasl.packet.SaslStreamElements.SASLFailure;
@@ -867,4 +870,44 @@ public class PacketParserUtilsTest {
         return otherLanguage;
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void descriptiveTextNullLangPassedMap() throws Exception {
+        final String text = "Dummy descriptive text";
+        Map<String, String> texts = new HashMap<>();
+        texts.put(null, text);
+        XMPPError
+            .getBuilder(XMPPError.Condition.internal_server_error)
+            .setDescriptiveTexts(texts)
+            .build();
+    }
+
+    @Test
+    public void ensureNoEmptyLangInDescriptiveText() throws Exception {
+        final String text = "Dummy descriptive text";
+        Map<String, String> texts = new HashMap<>();
+        texts.put("", text);
+        XMPPError error = XMPPError
+                .getBuilder(XMPPError.Condition.internal_server_error)
+                .setDescriptiveTexts(texts)
+                .build();
+        final String errorXml = XMLBuilder
+                .create(XMPPError.ERROR).a("type", "cancel").up()
+                .element("internal-server-error", XMPPError.NAMESPACE).up()
+                .element("text", XMPPError.NAMESPACE).t(text).up()
+                .asString();
+        XmlUnitUtils.assertSimilar(errorXml, error.toXML());
+    }
+
+    @Test
+    public void ensureNoNullLangInParsedDescriptiveTexts() throws Exception {
+        final String text = "Dummy descriptive text";
+        final String errorXml = XMLBuilder
+            .create(XMPPError.ERROR).a("type", "cancel").up()
+            .element("internal-server-error", XMPPError.NAMESPACE).up()
+            .element("text", XMPPError.NAMESPACE).t(text).up()
+            .asString();
+        XmlPullParser parser = TestUtils.getParser(errorXml);
+        XMPPError error = PacketParserUtils.parseError(parser).build();
+        assertEquals(text, error.getDescriptiveText());
+    }
 }