Merge pull request #558 from Flowdalic/iq-error-with-child-element

[core] Fix ErrorIQ displaying potential child elements

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

@@ -49,7 +49,7 @@ public static IqData createErrorResponse(IqView request) {
     }
 
     protected static IqData createResponse(IqView request, IQ.ResponseType responseType) {
-        if (!(request.getType() == IQ.Type.get || request.getType() == IQ.Type.set)) {
+        if (!request.isRequestIQ()) {
             throw new IllegalArgumentException("IQ request must be of type 'set' or 'get'. Original IQ: " + request);
         }
 

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

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright © 2014-2019 Florian Schmaus
+ * Copyright © 2014-2023 Florian Schmaus
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,7 +24,7 @@ public class EmptyResultIQ extends IQ {
 
     // TODO: Deprecate when stanza builder and parsing logic is ready.
     public EmptyResultIQ() {
-        super(null, null);
+        super((String) null, null);
         setType(IQ.Type.result);
     }
 

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

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright © 2014 Florian Schmaus
+ * Copyright © 2014-2023 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,24 +16,82 @@
  */
 package org.jivesoftware.smack.packet;
 
-import org.jivesoftware.smack.util.Objects;
+import java.util.Objects;
 
-public class ErrorIQ extends SimpleIQ {
+import javax.xml.namespace.QName;
+
+/**
+ * An XMPP error IQ.
+ * <p>
+ * According to RFC 6120 § 8.3.1 "4. An error stanza MUST contain an &lt;error/&gt; child element.", so this class can
+ * only be constructed if a stanza error is provided.
+ */
+public final class ErrorIQ extends IQ {
 
     public static final String ELEMENT = StanzaError.ERROR;
 
-    /**
-     * Constructs a new error IQ.
-     * <p>
-     * According to RFC 6120 § 8.3.1 "4. An error stanza MUST contain an &lt;error/&gt; child element.", so the xmppError argument is mandatory.
-     * </p>
-     * @param stanzaError the stanzaError (required).
-     */
-    public ErrorIQ(StanzaError stanzaError) {
-        super(ELEMENT, null);
-        Objects.requireNonNull(stanzaError, "stanzaError must not be null");
-        setType(IQ.Type.error);
-        setError(stanzaError);
+    private final IQ request;
+
+    private ErrorIQ(Builder builder, QName childElementQName) {
+        super(builder, childElementQName);
+        Objects.requireNonNull(builder.getError(), "Must provide an stanza error when building error IQs");
+        this.request = builder.request;
+    }
+
+    public static ErrorIQ createErrorResponse(final IQ request, final StanzaError error) {
+        Builder builder = new Builder(error, request);
+        builder.setError(error);
+        return builder.build();
+    }
+
+    @Override
+    protected IQChildElementXmlStringBuilder getIQChildElementBuilder(IQChildElementXmlStringBuilder xml) {
+        if (request == null) {
+            return null;
+        }
+
+        return request.getIQChildElementBuilder(xml);
+    }
+
+    public static Builder builder(StanzaError error) {
+        return new Builder(error, IqData.EMPTY.ofType(IQ.Type.error));
+    }
+
+    public static Builder builder(StanzaError error, IqData iqData) {
+        return new Builder(error, iqData);
     }
 
+    public static final class Builder extends IqBuilder<Builder, ErrorIQ> {
+
+        private IQ request;
+
+        Builder(StanzaError error, IqData iqData) {
+            super(iqData);
+            if (iqData.getType() != IQ.Type.error) {
+                throw new IllegalArgumentException("Error IQs must be of type 'error'");
+            }
+            Objects.requireNonNull(error, "Must provide an stanza error when building error IQs");
+            setError(error);
+        }
+
+        Builder(StanzaError error, IQ request) {
+            this(error, AbstractIqBuilder.createErrorResponse(request));
+            this.request = request;
+        }
+
+        @Override
+        public Builder getThis() {
+            return this;
+        }
+
+        @Override
+        public ErrorIQ build() {
+            QName childElementQname = null;
+            if (request != null) {
+                childElementQname = request.getChildElementQName();
+            }
+            return new ErrorIQ(this, childElementQname);
+        }
+
+    }
 }

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

@@ -68,16 +68,22 @@ protected IQ(String childElementName, String childElementNamespace) {
     }
 
     protected IQ(AbstractIqBuilder<?> iqBuilder, String childElementName, String childElementNamespace) {
+        this(iqBuilder, childElementName != null ? new QName(childElementNamespace, childElementName) : null);
+    }
+
+    protected IQ(AbstractIqBuilder<?> iqBuilder, QName childElementQName) {
         super(iqBuilder);
 
         type = iqBuilder.type;
 
-        this.childElementName = childElementName;
-        this.childElementNamespace = childElementNamespace;
-        if (childElementName == null) {
-            childElementQName = null;
+        if (childElementQName != null) {
+            this.childElementQName = childElementQName;
+            this.childElementName = childElementQName.getLocalPart();
+            this.childElementNamespace = childElementQName.getNamespaceURI();
         } else {
-            childElementQName = new QName(childElementNamespace, childElementName);
+            this.childElementQName = null;
+            this.childElementName = null;
+            this.childElementNamespace = null;
         }
     }
 
@@ -100,32 +106,6 @@ public void setType(Type type) {
         this.type = Objects.requireNonNull(type, "type must not be null");
     }
 
-    /**
-     * Return true if this IQ is a request IQ, i.e. an IQ of type {@link Type#get} or {@link Type#set}.
-     *
-     * @return true if IQ type is 'get' or 'set', false otherwise.
-     * @since 4.1
-     */
-    public boolean isRequestIQ() {
-        switch (type) {
-        case get:
-        case set:
-            return true;
-        default:
-            return false;
-        }
-    }
-
-    /**
-     * Return true if this IQ is a request, i.e. an IQ of type {@link Type#result} or {@link Type#error}.
-     *
-     * @return true if IQ type is 'result' or 'error', false otherwise.
-     * @since 4.4
-     */
-    public boolean isResponseIQ() {
-        return !isRequestIQ();
-    }
-
     public final QName getChildElementQName() {
         return childElementQName;
     }
@@ -194,7 +174,6 @@ private void appendInnerXml(XmlStringBuilder xml) {
         if (type == Type.error) {
             // Add the error sub-packet, if there is one.
             appendErrorIfExists(xml);
-            return;
         }
         if (childElementName == null) {
             return;
@@ -305,16 +284,7 @@ public static IQ createResultIQ(final IQ request) {
      * @return a new {@link Type#error IQ.Type.error} IQ based on the originating IQ.
      */
     public static ErrorIQ createErrorResponse(final IQ request, final StanzaError error) {
-        if (!request.isRequestIQ()) {
-            throw new IllegalArgumentException(
-                    "IQ must be of type 'set' or 'get'. Original IQ: " + request.toXML());
-        }
-        final ErrorIQ result = new ErrorIQ(error);
-        result.setStanzaId(request.getStanzaId());
-        result.setFrom(request.getTo());
-        result.setTo(request.getFrom());
-
-        return result;
+        return ErrorIQ.createErrorResponse(request, error);
     }
 
     /**

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

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2019 Florian Schmaus
+ * Copyright 2019-2023 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,6 +16,8 @@
  */
 package org.jivesoftware.smack.packet;
 
+import org.jivesoftware.smack.packet.IQ.Type;
+
 public interface IqView extends StanzaView {
 
     /**
@@ -25,4 +27,24 @@ public interface IqView extends StanzaView {
      */
     IQ.Type getType();
 
+    /**
+     * Return true if this IQ is a request IQ, i.e. an IQ of type {@link Type#get} or {@link Type#set}.
+     *
+     * @return true if IQ type is 'get' or 'set', false otherwise.
+     * @since 4.1
+     */
+    default boolean isRequestIQ() {
+        IQ.Type type = getType();
+        return type == IQ.Type.get || type == IQ.Type.set;
+    }
+
+    /**
+     * Return true if this IQ is a request, i.e. an IQ of type {@link Type#result} or {@link Type#error}.
+     *
+     * @return true if IQ type is 'result' or 'error', false otherwise.
+     * @since 4.4
+     */
+    default boolean isResponseIQ() {
+        return !isRequestIQ();
+    }
 }

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

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2003-2007 Jive Software, 2019 Florian Schmaus.
+ * Copyright 2003-2007 Jive Software, 2019-2023 Florian Schmaus.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -579,8 +579,9 @@ public static IQ parseIQ(XmlPullParser parser, XmlEnvironment outerXmlEnvironmen
             switch (iqData.getType()) {
             case error:
                 // If an IQ packet wasn't created above, create an empty error IQ packet.
-                iqPacket = new ErrorIQ(error);
-                break;
+                iqPacket = ErrorIQ.builder(error, iqData).build();
+                // The following return is simply to avoid setting iqData again below.
+                return iqPacket;
             case result:
                 iqPacket = new EmptyResultIQ();
                 break;

smack-core/src/test/java/org/jivesoftware/smack/packet/IqTest.java

@@ -0,0 +1,38 @@
+/**
+ *
+ * Copyright © 2023 Florian Schmaus
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jivesoftware.smack.packet;
+
+import static org.jivesoftware.smack.test.util.XmlAssertUtil.assertXmlSimilar;
+
+import org.junit.jupiter.api.Test;
+
+public class IqTest {
+
+    @Test
+    public void testIqErrorWithChildElement() {
+        IQ request = new TestIQ();
+        StanzaError error = StanzaError.getBuilder().setCondition(StanzaError.Condition.bad_request).build();
+        ErrorIQ errorIq = IQ.createErrorResponse(request, error);
+
+        String expected = "<iq xmlns='jabber:client' id='42' type='error'>"
+                          + "<error type='modify'><bad-request xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/></error>"
+                          + "<test-iq xmlns='https://igniterealtime.org/projects/smack'/>"
+                          + "</iq>";
+        assertXmlSimilar(expected, errorIq.toXML());
+    }
+
+}

smack-core/src/test/java/org/jivesoftware/smack/packet/TestIQ.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright © 2014-2019 Florian Schmaus
+ * Copyright © 2014-2023 Florian Schmaus
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,11 +21,11 @@
 public class TestIQ extends SimpleIQ {
 
     public TestIQ() {
-        this(SmackConfiguration.SMACK_URL_STRING, "test-iq");
+        this("test-iq", SmackConfiguration.SMACK_URL_STRING);
     }
 
     public TestIQ(String element, String namespace) {
-        super(element, namespace);
+        super(StanzaBuilder.buildIqData("42"), element, namespace);
     }
 
     @Override