refactor LocalizedContent code

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/parser/MessageParser.java        |   4 
src/main/java/eu/siacs/conversations/xml/Element.java                 |   4 
src/main/java/eu/siacs/conversations/xml/LocalizedContent.java        |  25 
src/main/java/eu/siacs/conversations/xml/XmlReader.java               | 224 
src/main/java/im/conversations/android/xmpp/model/stanza/Message.java |  38 
5 files changed, 150 insertions(+), 145 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/parser/MessageParser.java 🔗

@@ -1157,9 +1157,7 @@ public class MessageParser extends AbstractParser
                         && !packet.hasChild("thread")) { // We already know it has no body per above
                     if (conversation != null && conversation.getMode() == Conversation.MODE_MULTI) {
                         conversation.setHasMessagesLeftOnServer(conversation.countMessages() > 0);
-                        final LocalizedContent subject =
-                                packet.findInternationalizedChildContentInDefaultNamespace(
-                                        "subject");
+                        final LocalizedContent subject = packet.getSubject();
                         if (subject != null
                                 && conversation.getMucOptions().setSubject(subject.content)) {
                             mXmppConnectionService.updateConversation(conversation);

src/main/java/eu/siacs/conversations/xml/Element.java 🔗

@@ -68,10 +68,6 @@ public class Element {
         return element == null ? null : element.getContent();
     }
 
-    public LocalizedContent findInternationalizedChildContentInDefaultNamespace(String name) {
-        return LocalizedContent.get(this, name);
-    }
-
     public Element findChild(String name, String xmlns) {
         for (Element child : this.children) {
             if (name.equals(child.getName()) && xmlns.equals(child.getAttribute("xmlns"))) {

src/main/java/eu/siacs/conversations/xml/LocalizedContent.java 🔗

@@ -1,7 +1,6 @@
 package eu.siacs.conversations.xml;
 
 import com.google.common.collect.Iterables;
-
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -14,29 +13,13 @@ public class LocalizedContent {
     public final String language;
     public final int count;
 
-    private LocalizedContent(String content, String language, int count) {
+    private LocalizedContent(final String content, final String language, final int count) {
         this.content = content;
         this.language = language;
         this.count = count;
     }
 
-    public static LocalizedContent get(final Element element, String name) {
-        final HashMap<String, String> contents = new HashMap<>();
-        final String parentLanguage = element.getAttribute("xml:lang");
-        for(Element child : element.children) {
-            if (name.equals(child.getName())) {
-                final String namespace = child.getNamespace();
-                final String childLanguage = child.getAttribute("xml:lang");
-                final String lang = childLanguage == null ? parentLanguage : childLanguage;
-                final String content = child.getContent();
-                if (content != null && (namespace == null || Namespace.JABBER_CLIENT.equals(namespace))) {
-                    if (contents.put(lang, content) != null) {
-                        //anything that has multiple contents for the same language is invalid
-                        return null;
-                    }
-                }
-            }
-        }
+    public static LocalizedContent get(final Map<String, String> contents) {
         if (contents.isEmpty()) {
             return null;
         }
@@ -45,10 +28,6 @@ public class LocalizedContent {
         if (localized != null) {
             return new LocalizedContent(localized, userLanguage, contents.size());
         }
-        final String defaultLanguageContent = contents.get(null);
-        if (defaultLanguageContent != null) {
-            return new LocalizedContent(defaultLanguageContent, STREAM_LANGUAGE, contents.size());
-        }
         final String streamLanguageContent = contents.get(STREAM_LANGUAGE);
         if (streamLanguageContent != null) {
             return new LocalizedContent(streamLanguageContent, STREAM_LANGUAGE, contents.size());

src/main/java/eu/siacs/conversations/xml/XmlReader.java 🔗

@@ -2,132 +2,134 @@ package eu.siacs.conversations.xml;
 
 import android.util.Log;
 import android.util.Xml;
-
 import eu.siacs.conversations.Config;
-
 import im.conversations.android.xmpp.ExtensionFactory;
-import im.conversations.android.xmpp.model.Extension;
 import im.conversations.android.xmpp.model.StreamElement;
-
-import org.xmlpull.v1.XmlPullParser;
-import org.xmlpull.v1.XmlPullParserException;
-
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import org.xmlpull.v1.XmlPullParser;
+import org.xmlpull.v1.XmlPullParserException;
 
 public class XmlReader implements Closeable {
-	private final XmlPullParser parser;
-	private InputStream is;
+    private final XmlPullParser parser;
+    private InputStream is;
 
-	public XmlReader() {
-		this.parser = Xml.newPullParser();
-		try {
-			this.parser.setFeature(XmlPullParser.FEATURE_PROCESS_NAMESPACES, true);
-		} catch (XmlPullParserException e) {
-			Log.d(Config.LOGTAG, "error setting namespace feature on parser");
-		}
-	}
+    public XmlReader() {
+        this.parser = Xml.newPullParser();
+        try {
+            this.parser.setFeature(XmlPullParser.FEATURE_PROCESS_NAMESPACES, true);
+        } catch (XmlPullParserException e) {
+            Log.d(Config.LOGTAG, "error setting namespace feature on parser");
+        }
+    }
 
-	public void setInputStream(InputStream inputStream) throws IOException {
-		if (inputStream == null) {
-			throw new IOException();
-		}
-		this.is = inputStream;
-		try {
-			parser.setInput(new InputStreamReader(this.is));
-		} catch (XmlPullParserException e) {
-			throw new IOException("error resetting parser");
-		}
-	}
+    public void setInputStream(InputStream inputStream) throws IOException {
+        if (inputStream == null) {
+            throw new IOException();
+        }
+        this.is = inputStream;
+        try {
+            parser.setInput(new InputStreamReader(this.is));
+        } catch (XmlPullParserException e) {
+            throw new IOException("error resetting parser");
+        }
+    }
 
-	public void reset() throws IOException {
-		if (this.is == null) {
-			throw new IOException();
-		}
-		try {
-			parser.setInput(new InputStreamReader(this.is));
-		} catch (XmlPullParserException e) {
-			throw new IOException("error resetting parser");
-		}
-	}
+    public void reset() throws IOException {
+        if (this.is == null) {
+            throw new IOException();
+        }
+        try {
+            parser.setInput(new InputStreamReader(this.is));
+        } catch (XmlPullParserException e) {
+            throw new IOException("error resetting parser");
+        }
+    }
 
-	@Override
-	public void close() {
-		this.is = null;
-	}
+    @Override
+    public void close() {
+        this.is = null;
+    }
 
-	public Tag readTag() throws IOException {
-		try {
-			while (this.is != null && parser.next() != XmlPullParser.END_DOCUMENT) {
-				if (parser.getEventType() == XmlPullParser.START_TAG) {
-					Tag tag = Tag.start(parser.getName());
-					final String xmlns = parser.getNamespace();
-					for (int i = 0; i < parser.getAttributeCount(); ++i) {
-						final String prefix = parser.getAttributePrefix(i);
-						String name;
-						if (prefix != null && !prefix.isEmpty()) {
-							name = prefix+":"+parser.getAttributeName(i);
-						} else {
-							name = parser.getAttributeName(i);
-						}
-						tag.setAttribute(name,parser.getAttributeValue(i));
-					}
-					if (xmlns != null) {
-						tag.setAttribute("xmlns", xmlns);
-					}
-					return tag;
-				} else if (parser.getEventType() == XmlPullParser.END_TAG) {
-					return Tag.end(parser.getName());
-				} else if (parser.getEventType() == XmlPullParser.TEXT) {
-					return Tag.no(parser.getText());
-				}
-			}
+    public Tag readTag() throws IOException {
+        try {
+            while (this.is != null && parser.next() != XmlPullParser.END_DOCUMENT) {
+                if (parser.getEventType() == XmlPullParser.START_TAG) {
+                    Tag tag = Tag.start(parser.getName());
+                    final String xmlns = parser.getNamespace();
+                    for (int i = 0; i < parser.getAttributeCount(); ++i) {
+                        // TODO we would also look at parser.getAttributeNamespace()
+                        final String prefix = parser.getAttributePrefix(i);
+                        String name;
+                        if (prefix != null && !prefix.isEmpty()) {
+                            name = prefix + ":" + parser.getAttributeName(i);
+                        } else {
+                            name = parser.getAttributeName(i);
+                        }
+                        tag.setAttribute(name, parser.getAttributeValue(i));
+                    }
+                    if (xmlns != null) {
+                        tag.setAttribute("xmlns", xmlns);
+                    }
+                    return tag;
+                } else if (parser.getEventType() == XmlPullParser.END_TAG) {
+                    return Tag.end(parser.getName());
+                } else if (parser.getEventType() == XmlPullParser.TEXT) {
+                    return Tag.no(parser.getText());
+                }
+            }
 
-		} catch (Throwable throwable) {
-			throw new IOException("xml parser mishandled "+throwable.getClass().getSimpleName()+"("+throwable.getMessage()+")", throwable);
-		}
-		return null;
-	}
+        } catch (Throwable throwable) {
+            throw new IOException(
+                    "xml parser mishandled "
+                            + throwable.getClass().getSimpleName()
+                            + "("
+                            + throwable.getMessage()
+                            + ")",
+                    throwable);
+        }
+        return null;
+    }
 
-	public <T extends StreamElement> T readElement(final Tag current, final Class<T> clazz)
-			throws IOException {
-		final Element element = readElement(current);
-		if (clazz.isInstance(element)) {
-			return clazz.cast(element);
-		}
-		throw new IOException(
-				String.format("Read unexpected {%s}%s", element.getNamespace(), element.getName()));
-	}
+    public <T extends StreamElement> T readElement(final Tag current, final Class<T> clazz)
+            throws IOException {
+        final Element element = readElement(current);
+        if (clazz.isInstance(element)) {
+            return clazz.cast(element);
+        }
+        throw new IOException(
+                String.format("Read unexpected {%s}%s", element.getNamespace(), element.getName()));
+    }
 
-	public Element readElement(final Tag currentTag) throws IOException {
-		final var attributes = currentTag.getAttributes();
-		final var namespace = attributes.get("xmlns");
-		final var name = currentTag.getName();
-		final Element element = ExtensionFactory.create(name, namespace);
-		element.setAttributes(currentTag.getAttributes());
-		Tag nextTag = this.readTag();
-		if (nextTag == null) {
-			throw new IOException("interrupted mid tag");
-		}
-		if (nextTag.isNo()) {
-			element.setContent(nextTag.getName());
-			nextTag = this.readTag();
-			if (nextTag == null) {
-				throw new IOException("interrupted mid tag");
-			}
-		}
-		while (!nextTag.isEnd(element.getName())) {
-			if (!nextTag.isNo()) {
-				Element child = this.readElement(nextTag);
-				element.addChild(child);
-			}
-			nextTag = this.readTag();
-			if (nextTag == null) {
-				throw new IOException("interrupted mid tag");
-			}
-		}
-		return element;
-	}
+    public Element readElement(final Tag currentTag) throws IOException {
+        final var attributes = currentTag.getAttributes();
+        final var namespace = attributes.get("xmlns");
+        final var name = currentTag.getName();
+        final Element element = ExtensionFactory.create(name, namespace);
+        element.setAttributes(currentTag.getAttributes());
+        Tag nextTag = this.readTag();
+        if (nextTag == null) {
+            throw new IOException("interrupted mid tag");
+        }
+        if (nextTag.isNo()) {
+            element.setContent(nextTag.getName());
+            nextTag = this.readTag();
+            if (nextTag == null) {
+                throw new IOException("interrupted mid tag");
+            }
+        }
+        while (!nextTag.isEnd(element.getName())) {
+            if (!nextTag.isNo()) {
+                Element child = this.readElement(nextTag);
+                element.addChild(child);
+            }
+            nextTag = this.readTag();
+            if (nextTag == null) {
+                throw new IOException("interrupted mid tag");
+            }
+        }
+        return element;
+    }
 }

src/main/java/im/conversations/android/xmpp/model/stanza/Message.java 🔗

@@ -1,11 +1,13 @@
 package im.conversations.android.xmpp.model.stanza;
 
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
 import eu.siacs.conversations.xml.Element;
 import eu.siacs.conversations.xml.LocalizedContent;
-
 import im.conversations.android.annotation.XmlElement;
+import im.conversations.android.xmpp.model.Extension;
 import im.conversations.android.xmpp.model.jabber.Body;
-
+import im.conversations.android.xmpp.model.jabber.Subject;
 import java.util.Locale;
 
 @XmlElement
@@ -21,9 +23,37 @@ public class Message extends Stanza {
     }
 
     public LocalizedContent getBody() {
-        return findInternationalizedChildContentInDefaultNamespace("body");
+        return getLocalizedContent(Body.class);
+    }
+
+    public LocalizedContent getSubject() {
+        return getLocalizedContent(Subject.class);
+    }
+
+    private LocalizedContent getLocalizedContent(final Class<? extends Extension> clazz) {
+        final var builder = new ImmutableMap.Builder<String, String>();
+        final var messageLanguage = this.getAttribute("xml:lang");
+        final var parentLanguage =
+                Strings.isNullOrEmpty(messageLanguage)
+                        ? LocalizedContent.STREAM_LANGUAGE
+                        : messageLanguage;
+        for (final var element : this.getExtensions(clazz)) {
+            final var elementLanguage = element.getAttribute("xml:lang");
+            final var language =
+                    Strings.isNullOrEmpty(elementLanguage) ? parentLanguage : elementLanguage;
+            final var content = element.getContent();
+            if (content == null) {
+                continue;
+            }
+            builder.put(language, content);
+        }
+        try {
+            return LocalizedContent.get(builder.buildOrThrow());
+        } catch (final IllegalArgumentException e) {
+            return null;
+        }
     }
-    
+
     public Type getType() {
         final var value = this.getAttribute("type");
         if (value == null) {