Merge pull request #4703 from line-o/fix/4702

dizzzz · web-flow · commit 096a56ef0289 · 2023-02-02T11:10:40.000+01:00
[hotfix] fn:serialize issues
diff --git a/exist-core/src/main/java/org/exist/xquery/functions/fn/FunSerialize.java b/exist-core/src/main/java/org/exist/xquery/functions/fn/FunSerialize.java
@@ -28,6 +28,7 @@
 import org.exist.storage.serializers.EXistOutputKeys;
 import org.exist.util.serializer.XQuerySerializer;
 import org.exist.xquery.*;
+import org.exist.xquery.functions.array.ArrayType;
 import org.exist.xquery.functions.map.AbstractMapType;
 import org.exist.xquery.util.SerializerUtils;
 import org.exist.xquery.value.*;
@@ -82,13 +83,16 @@ public Sequence eval(Sequence[] args, Sequence contextSequence) throws XPathExce
         try(final StringWriter writer = new StringWriter()) {
             final XQuerySerializer xqSerializer = new XQuerySerializer(context.getBroker(), outputProperties, writer);
 
-            Sequence seq = args[0];
+            final Sequence input = args[0];
             if (xqSerializer.normalize()) {
                 final String itemSeparator = outputProperties.getProperty(EXistOutputKeys.ITEM_SEPARATOR, DEFAULT_ITEM_SEPARATOR);
-                seq = normalize(this, context, seq, itemSeparator);
+                final Sequence normalized = normalize(this, context, input, itemSeparator);
+                xqSerializer.serialize(normalized);
+            }
+            else {
+                xqSerializer.serialize(input);
             }
 
-            xqSerializer.serialize(seq);
             return new StringValue(this, writer.toString());
         } catch (final IOException | SAXException e) {
             throw new XPathException(this, FnModule.SENR0001, e.getMessage());
@@ -97,9 +101,9 @@ public Sequence eval(Sequence[] args, Sequence contextSequence) throws XPathExce
 
     public static Properties getSerializationProperties(final Expression callingExpr, final Item parametersItem) throws XPathException {
         final Properties outputProperties;
-        if(parametersItem.getType() == Type.MAP) {
+        if (parametersItem.getType() == Type.MAP) {
             outputProperties = SerializerUtils.getSerializationOptions(callingExpr, (AbstractMapType) parametersItem);
-        } else if(isSerializationParametersElement(parametersItem)) {
+        } else if (isSerializationParametersElement(parametersItem)) {
             outputProperties = new Properties();
             SerializerUtils.getSerializationOptions(callingExpr, (NodeValue) parametersItem, outputProperties);
         } else {
@@ -141,48 +145,59 @@ private static boolean isSerializationParametersElement(final Item item) {
      * @throws XPathException in case of dynamic error.
      */
     public static Sequence normalize(final Expression callingExpr, final XQueryContext context, final Sequence input, final String itemSeparator) throws XPathException {
-        if (input.isEmpty())
-            // "If the sequence that is input to serialization is empty, create a sequence S1 that consists of a zero-length string."
-            {return StringValue.EMPTY_STRING;}
-        final ValueSequence temp = new ValueSequence(input.getItemCount());
+        // "If the sequence that is input to serialization is empty, create a sequence S1 that consists of a zero-length string."
+        if (input.isEmpty()) {
+            return StringValue.EMPTY_STRING;
+        }
+        // flatten arrays
+        final ValueSequence step1 = new ValueSequence();
         for (final SequenceIterator i = input.iterate(); i.hasNext(); ) {
             final Item next = i.nextItem();
-            if (Type.subTypeOf(next.getType(), Type.NODE)) {
-                if (next.getType() == Type.ATTRIBUTE || next.getType() == Type.NAMESPACE || next.getType() == Type.FUNCTION_REFERENCE)
-                    {throw new XPathException(callingExpr, FnModule.SENR0001,
-                        "It is an error if an item in the sequence to serialize is an attribute node or a namespace node.");}
-                if (itemSeparator != null && itemSeparator.length() > 0 && !temp.isEmpty()) {
-                    temp.add(new StringValue(itemSeparator + next.getStringValue()));
-                } else {
-                    temp.add(next);
+            if (next.getType() == Type.ARRAY) {
+                final Sequence sequence = ArrayType.flatten(next);
+                for (final SequenceIterator si = sequence.iterate(); si.hasNext(); ) {
+                    step1.add(si.nextItem());
+                }
+            } else {
+                step1.add(next);
+            }
+        }
+
+        final ValueSequence step2 = new ValueSequence(step1.getItemCount());
+        for (final SequenceIterator i = step1.iterate(); i.hasNext(); ) {
+            final Item next = i.nextItem();
+            final int itemType = next.getType();
+            if (Type.subTypeOf(itemType, Type.NODE)) {
+                if (itemType == Type.ATTRIBUTE || itemType == Type.NAMESPACE || itemType == Type.FUNCTION_REFERENCE) {
+                    throw new XPathException(callingExpr, FnModule.SENR0001,
+                        "It is an error if an item in the sequence to serialize is an attribute node or a namespace node.");
                 }
+                step2.add(next);
             } else {
                 // atomic value
-                Item last = null;
-                if (!temp.isEmpty())
-                    {last = temp.itemAt(temp.getItemCount() - 1);}
-                if (last != null && last.getType() == Type.STRING)
-                    // "For each subsequence of adjacent strings in S2, copy a single string to the new sequence
-                    // equal to the values of the strings in the subsequence concatenated in order, each separated
-                    // by a single space."
-                    {((StringValue)last).append((itemSeparator == null ? " " : itemSeparator) + next.getStringValue());}
-                else
-                    // "For each item in S1, if the item is atomic, obtain the lexical representation of the item by
-                    // casting it to an xs:string and copy the string representation to the new sequence;"
-                    {temp.add(new StringValue(callingExpr, next.getStringValue()));}
+                // "For each item in S1, if the item is atomic, obtain the lexical representation of the item by
+                // casting it to an xs:string and copy the string representation to the new sequence;"
+                final StringValue stringRepresentation = new StringValue(callingExpr, next.getStringValue());
+                step2.add(stringRepresentation);
             }
         }
 
         context.pushDocumentContext();
         try {
             final MemTreeBuilder builder = context.getDocumentBuilder();
             final DocumentBuilderReceiver receiver = new DocumentBuilderReceiver(callingExpr, builder, true);
-            for (final SequenceIterator i = temp.iterate(); i.hasNext(); ) {
+            final String safeItemSeparator = itemSeparator == null ? "" : itemSeparator;
+            for (final SequenceIterator i = step2.iterate(); i.hasNext(); ) {
                 final Item next = i.nextItem();
                 if (Type.subTypeOf(next.getType(), Type.NODE)) {
                     next.copyTo(context.getBroker(), receiver);
                 } else {
-                    receiver.characters(next.getStringValue());
+                    final String stringValue = next.getStringValue();
+                    receiver.characters(stringValue);
+                }
+                // add itemSeparator if there is a next item
+                if (i.hasNext()) {
+                    receiver.characters(safeItemSeparator);
                 }
             }
             return (DocumentImpl)receiver.getDocument();
diff --git a/exist-core/src/main/java/org/exist/xquery/functions/util/Eval.java b/exist-core/src/main/java/org/exist/xquery/functions/util/Eval.java
@@ -404,6 +404,7 @@ private Sequence doEval(final XQueryContext evalContext, final Sequence contextS
 
                         final Sequence seq;
                         if (xqSerializer.normalize()) {
+                            // TODO(JL): should this not be changed to DEFAULT_ITEM_SEPARATOR
                             seq = FunSerialize.normalize(this, context, result, null);
                         } else {
                             seq = result;
diff --git a/exist-core/src/main/java/org/exist/xquery/util/SerializerUtils.java b/exist-core/src/main/java/org/exist/xquery/util/SerializerUtils.java
@@ -454,7 +454,12 @@ private static Sequence getParameterValue(final Expression parent, final Abstrac
         final Sequence providedParameterValue = entries.get(parameterConventionEntryKey);
 
         // should we use the default value
-        if (providedParameterValue == null || providedParameterValue.isEmpty() || (parameterConvention.getType() == Type.STRING && isEmptyStringValue(providedParameterValue))) {
+        if (providedParameterValue == null || providedParameterValue.isEmpty() || (
+                parameterConvention.getType() == Type.STRING && isEmptyStringValue(providedParameterValue) &&
+                        // allow empty separator #4704
+                        parameterConvention.getParameterName() != EXistOutputKeys.ITEM_SEPARATOR
+        )
+        ) {
             // use default value
 
             if (W3CParameterConvention.MEDIA_TYPE == parameterConvention) {
diff --git a/exist-core/src/test/java/xquery/xquery3/XQuery3Tests.java b/exist-core/src/test/java/xquery/xquery3/XQuery3Tests.java
@@ -28,7 +28,8 @@
 @XSuite.XSuiteFiles({
         "src/test/xquery/xquery3",
         "src/test/xquery/xquery3/transform",
-        //Reminder - add an individual test like this - "src/test/xquery/xquery3/transform/<test-file>.xqm",
+        // To add an individual test or only run a specific set of tests -
+//         "src/test/xquery/xquery3/serialize.xql",
 })
 public class XQuery3Tests {
 }
diff --git a/exist-core/src/test/xquery/xquery3/serialize.xql b/exist-core/src/test/xquery/xquery3/serialize.xql
@@ -224,6 +224,26 @@ function ser:serialize-atomic() {
         fn:serialize($nodes)
 };
 
+(: test for https://github.com/eXist-db/exist/issues/4704 :)
+declare
+    %test:assertEquals("aaabbb")
+function ser:serialize-atomic-empty-separator() {
+    fn:serialize(("aaa", "bbb"), map {"item-separator": ""})
+};
+
+(: test for https://github.com/eXist-db/exist/issues/4704 :)
+declare
+    %test:assertEquals("aaabbb")
+function ser:serialize-atomic-empty-separator-xml-options() {
+    fn:serialize(("aaa", "bbb"),
+      <output:serialization-parameters
+           xmlns:output="http://www.w3.org/2010/xslt-xquery-serialization">
+        <output:item-separator value=""/>
+      </output:serialization-parameters>
+    )
+};
+
+
 declare
     %test:assertEquals("")
 function ser:serialize-empty-sequence() {
@@ -927,3 +947,55 @@ function ser:serialize-html-5-needs-escape-elements() {
         => serialize($params)
         => normalize-space()
 };
+
+(: test for https://github.com/eXist-db/exist/issues/4702 :)
+declare
+    %test:assertEquals("<a>foo</a> <b>bar</b>")
+function ser:sequence-of-nodes() {
+    (<a>foo</a>, <b>bar</b>) => serialize()
+};
+
+declare
+    %test:assertEquals("[|]")
+function ser:sequence-skip-empty-text-node() {
+    (<a>[</a>, <a>
+        </a>, <a>]</a>)/text()
+    => serialize(map {"item-separator": "|"})
+};
+
+declare
+    %test:assertEquals("||")
+function ser:sequence-dont-skip-empty-string() {
+    serialize(("", "", ""), map {"item-separator": "|"})
+};
+
+declare
+    %test:assertEquals("foo")
+function ser:skip-empty-no-separator() {
+    (<a>foo</a>, <b></b>)/text()
+    => serialize(map{"item-separator": "!"})
+};
+
+declare
+    %test:assertEquals("")
+function ser:empty-array-serializes-to-empty-string() {
+    serialize([])
+};
+
+declare
+    %test:assertEquals("")
+function ser:array-with-members-serializes-to-empty-string() {
+    serialize(["", ()])
+};
+
+declare
+    %test:assertEquals("")
+function ser:sequence-of-empty-arrays-serializes-to-empty-string() {
+    serialize(([],[]), map{"item-separator": "|"})
+};
+
+declare
+    %test:assertEquals("1|2")
+function ser:item-separator-applies-to-array-members() {
+    serialize(([1,2]), map{"item-separator":"|"})
+};
