From 9ea5c0a2ce87040a8ba25c605ccf5b9c69f893fb Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Wed, 1 Jul 2015 13:39:04 +0200 Subject: [PATCH] "Smackify" HOXT code: Use Smack programming idioms --- .../smack/util/XmlStringBuilder.java | 9 +++- .../hoxt/packet/AbstractHttpOverXmpp.java | 36 +++++++------ .../smackx/hoxt/packet/Base64BinaryChunk.java | 32 ++++++------ .../smackx/hoxt/packet/HttpOverXmppReq.java | 50 +++++++------------ .../smackx/hoxt/packet/HttpOverXmppResp.java | 24 ++++----- .../AbstractHttpOverXmppProvider.java | 17 ++++--- .../provider/HttpOverXmppReqProvider.java | 32 +++++------- .../AbstractHttpOverXmppProviderTest.java | 4 +- .../provider/HttpOverXmppReqProviderTest.java | 2 +- 9 files changed, 102 insertions(+), 104 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java b/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java index 538fb7106..394d1bc43 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java @@ -1,6 +1,6 @@ /** * - * Copyright 2014 Florian Schmaus + * Copyright 2014-2015 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -328,6 +328,13 @@ public class XmlStringBuilder implements Appendable, CharSequence { return this; } + public XmlStringBuilder optBooleanAttributeDefaultTrue(String name, boolean bool) { + if (!bool) { + sb.append(' ').append(name).append("='false'"); + } + return this; + } + public XmlStringBuilder xmlnsAttribute(String value) { optAttribute("xmlns", value); return this; diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/AbstractHttpOverXmpp.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/AbstractHttpOverXmpp.java index 7af56e8b2..a7ac49d8c 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/AbstractHttpOverXmpp.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/AbstractHttpOverXmpp.java @@ -18,6 +18,7 @@ package org.jivesoftware.smackx.hoxt.packet; import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.NamedElement; +import org.jivesoftware.smack.util.Objects; import org.jivesoftware.smack.util.XmlStringBuilder; import org.jivesoftware.smackx.shim.packet.HeadersExtension; @@ -31,25 +32,22 @@ public abstract class AbstractHttpOverXmpp extends IQ { public static final String NAMESPACE = "urn:xmpp:http"; - private HeadersExtension headers; - private Data data; + private final HeadersExtension headers; + private final Data data; - protected String version; + private final String version; protected AbstractHttpOverXmpp(String element, Builder builder) { super(element, NAMESPACE); this.headers = builder.headers; this.data = builder.data; - this.version = builder.version; + this.version = Objects.requireNonNull(builder.version, "version must not be null"); } protected IQChildElementXmlStringBuilder getIQChildElementBuilder(IQChildElementXmlStringBuilder xml) { IQChildElementXmlStringBuilder builder = getIQHoxtChildElementBuilder(xml); builder.optAppend(headers); - /* data cannot be fed to optAppend */ - if (data != null) { - builder.optAppend(data.toXML()); - } + builder.optAppend(data); return builder; } @@ -149,7 +147,9 @@ public abstract class AbstractHttpOverXmpp extends IQ { *

* This class is immutable. */ - public static class Data { + public static class Data implements NamedElement { + + public static final String ELEMENT = "data"; private final NamedElement child; @@ -167,12 +167,13 @@ public abstract class AbstractHttpOverXmpp extends IQ { * * @return xml representation of this object */ - public String toXML() { - StringBuilder builder = new StringBuilder(); - builder.append(""); - builder.append(child.toXML()); - builder.append(""); - return builder.toString(); + @Override + public XmlStringBuilder toXML() { + XmlStringBuilder xml = new XmlStringBuilder(this); + xml.rightAngleBracket(); + xml.element(child); + xml.closeElement(this); + return xml; } /** @@ -183,6 +184,11 @@ public abstract class AbstractHttpOverXmpp extends IQ { public NamedElement getChild() { return child; } + + @Override + public String getElementName() { + return ELEMENT; + } } /** diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/Base64BinaryChunk.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/Base64BinaryChunk.java index a80955242..0d68414b6 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/Base64BinaryChunk.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/Base64BinaryChunk.java @@ -1,6 +1,6 @@ /** * - * Copyright 2014 Andriy Tsykholyas + * Copyright 2014 Andriy Tsykholyas, 2015 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.jivesoftware.smackx.hoxt.packet; import org.jivesoftware.smack.packet.ExtensionElement; +import org.jivesoftware.smack.util.Objects; +import org.jivesoftware.smack.util.XmlStringBuilder; import org.jivesoftware.smackx.hoxt.HOXTManager; /** @@ -47,8 +49,11 @@ public class Base64BinaryChunk implements ExtensionElement { * @param last value of last attribute */ public Base64BinaryChunk(String text, String streamId, int nr, boolean last) { - this.text = text; - this.streamId = streamId; + this.text = Objects.requireNonNull(text, "text must not be null"); + this.streamId = Objects.requireNonNull(streamId, "streamId must not be null"); + if (nr < 0) { + throw new IllegalArgumentException("nr must be a non negative integer"); + } this.nr = nr; this.last = last; } @@ -111,17 +116,14 @@ public class Base64BinaryChunk implements ExtensionElement { } @Override - public String toXML() { - StringBuilder builder = new StringBuilder(); - builder.append(""); - builder.append(text); - builder.append(""); - return builder.toString(); + public XmlStringBuilder toXML() { + XmlStringBuilder xml = new XmlStringBuilder(this); + xml.attribute("streamId", streamId); + xml.attribute("nr", nr); + xml.optBooleanAttribute("last", last); + xml.rightAngleBracket(); + xml.append(text); + xml.closeElement(this); + return xml; } } diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/HttpOverXmppReq.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/HttpOverXmppReq.java index 9f92615de..a01127939 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/HttpOverXmppReq.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/HttpOverXmppReq.java @@ -1,6 +1,6 @@ /** * - * Copyright 2014 Andriy Tsykholyas + * Copyright 2014 Andriy Tsykholyas, 2015 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,8 +16,6 @@ */ package org.jivesoftware.smackx.hoxt.packet; -import org.jivesoftware.smack.util.StringUtils; - /** * Represents Req IQ packet. * @@ -39,36 +37,26 @@ public final class HttpOverXmppReq extends AbstractHttpOverXmpp { setType(Type.set); } - private HttpMethod method; - private String resource; + private final HttpMethod method; + private final String resource; - // TODO: validate: xs:minInclusive value='256' xs:maxInclusive value='65536' - private int maxChunkSize; // 0 means not set + private final int maxChunkSize; - private boolean sipub; + private final boolean sipub; - private boolean ibb; - private boolean jingle; + private final boolean ibb; + private final boolean jingle; @Override protected IQChildElementXmlStringBuilder getIQHoxtChildElementBuilder(IQChildElementXmlStringBuilder builder) { - builder.append(" "); - builder.append("method='").append(method.toString()).append("'"); - builder.append(" "); - builder.append("resource='").append(StringUtils.escapeForXML(resource)).append("'"); - builder.append(" "); - builder.append("version='").append(StringUtils.escapeForXML(version)).append("'"); - if (maxChunkSize != 0) { - builder.append(" "); - builder.append("maxChunkSize='").append(Integer.toString(maxChunkSize)).append("'"); - } - builder.append(" "); - builder.append("sipub='").append(Boolean.toString(sipub)).append("'"); - builder.append(" "); - builder.append("ibb='").append(Boolean.toString(ibb)).append("'"); - builder.append(" "); - builder.append("jingle='").append(Boolean.toString(jingle)).append("'"); - builder.append(">"); + builder.attribute("method", method); + builder.attribute("resource", resource); + builder.attribute("version", getVersion()); + builder.optIntAttribute("maxChunkSize", maxChunkSize); + builder.optBooleanAttributeDefaultTrue("sipub", sipub); + builder.optBooleanAttributeDefaultTrue("ibb", ibb); + builder.optBooleanAttributeDefaultTrue("jingle", jingle); + builder.rightAngleBracket(); return builder; } @@ -139,9 +127,7 @@ public final class HttpOverXmppReq extends AbstractHttpOverXmpp { private HttpMethod method; private String resource; - // TODO: validate: xs:minInclusive value='256' xs:maxInclusive - // value='65536' - private int maxChunkSize = 0; // 0 means not set + private int maxChunkSize = -1; private boolean sipub = true; @@ -149,7 +135,6 @@ public final class HttpOverXmppReq extends AbstractHttpOverXmpp { private boolean jingle = true; private Builder() { - } /** @@ -220,6 +205,9 @@ public final class HttpOverXmppReq extends AbstractHttpOverXmpp { * @return the builder */ public Builder setMaxChunkSize(int maxChunkSize) { + if (maxChunkSize < 256 || maxChunkSize > 65536) { + throw new IllegalArgumentException("maxChunkSize must be within [256, 65536]"); + } this.maxChunkSize = maxChunkSize; return this; } diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/HttpOverXmppResp.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/HttpOverXmppResp.java index 3a2385055..97e74b390 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/HttpOverXmppResp.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/packet/HttpOverXmppResp.java @@ -16,7 +16,7 @@ */ package org.jivesoftware.smackx.hoxt.packet; -import org.jivesoftware.smack.util.StringUtils; +import org.jivesoftware.smack.util.Objects; /** * Represents Resp IQ packet. @@ -30,24 +30,19 @@ public final class HttpOverXmppResp extends AbstractHttpOverXmpp { private HttpOverXmppResp(Builder builder) { super(ELEMENT, builder); - this.statusCode = builder.statusCode; + this.statusCode = Objects.requireNonNull(builder.statusCode, "statusCode must not be null"); this.statusMessage = builder.statusMessage; } - private int statusCode; - private String statusMessage; + private final int statusCode; + private final String statusMessage; @Override protected IQChildElementXmlStringBuilder getIQHoxtChildElementBuilder(IQChildElementXmlStringBuilder builder) { - builder.append(" "); - builder.append("version='").append(StringUtils.escapeForXML(version)).append("'"); - builder.append(" "); - builder.append("statusCode='").append(Integer.toString(statusCode)).append("'"); - if (statusMessage != null) { - builder.append(" "); - builder.append("statusMessage='").append(StringUtils.escapeForXML(statusMessage)).append("'"); - } - builder.append(">"); + builder.attribute("version", getVersion()); + builder.attribute("statusCode", statusCode); + builder.optAttribute("statusMessage", statusMessage); + builder.rightAngleBracket(); return builder; } @@ -82,6 +77,9 @@ public final class HttpOverXmppResp extends AbstractHttpOverXmpp { private int statusCode = 200; private String statusMessage = null; + private Builder() { + } + /** * Sets statusCode attribute. * diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/provider/AbstractHttpOverXmppProvider.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/provider/AbstractHttpOverXmppProvider.java index 1cb2f541f..05bf8c813 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/provider/AbstractHttpOverXmppProvider.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/hoxt/provider/AbstractHttpOverXmppProvider.java @@ -1,6 +1,6 @@ /** * - * Copyright 2014 Andriy Tsykholyas + * Copyright 2014 Andriy Tsykholyas, 2015 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -124,7 +124,7 @@ public abstract class AbstractHttpOverXmppProvider expectedHeaders) { + private static void checkHeaders(HeadersExtension headers, Map expectedHeaders) { Collection

collection = headers.getHeaders(); assertEquals(collection.size(), expectedHeaders.size()); diff --git a/smack-experimental/src/test/java/org/jivesoftware/smackx/hoxt/provider/HttpOverXmppReqProviderTest.java b/smack-experimental/src/test/java/org/jivesoftware/smackx/hoxt/provider/HttpOverXmppReqProviderTest.java index e317ea026..c4cab9606 100644 --- a/smack-experimental/src/test/java/org/jivesoftware/smackx/hoxt/provider/HttpOverXmppReqProviderTest.java +++ b/smack-experimental/src/test/java/org/jivesoftware/smackx/hoxt/provider/HttpOverXmppReqProviderTest.java @@ -65,7 +65,7 @@ public class HttpOverXmppReqProviderTest { assertEquals(req.isJingle(), true); } - private HttpOverXmppReq parseReq(String string) throws Exception { + private static HttpOverXmppReq parseReq(String string) throws Exception { HttpOverXmppReqProvider provider = new HttpOverXmppReqProvider(); XmlPullParser parser = PacketParserUtils.getParserFor(string); IQ iq = provider.parse(parser);