Merge pull request #6164 from joewiz/feature/fix-skipped-tests

[bugfix] Fix 3 skipped test failures: ResourceSet, util:eval, XUpdate

Author: line-o

exist-core/src/main/java/org/exist/xmldb/ResourceSetHelper.java

@@ -21,40 +21,79 @@
  */
 package org.exist.xmldb;
 
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.Map;
-import java.util.Set;
 
+import org.exist.dom.persistent.NodeProxy;
 import org.xmldb.api.base.Resource;
 import org.xmldb.api.base.ResourceSet;
 import org.xmldb.api.base.XMLDBException;
 
 /**
  * @author jmv
- *
  */
 public class ResourceSetHelper {
 
+    /**
+     * Computes the intersection of two resource sets.
+     *
+     * <p>For {@link LocalXMLResource} instances backed by persistent nodes,
+     * intersection is determined by node identity (document ID + node ID).
+     * For other resources, the resource ID ({@link Resource#getId()}) is used
+     * as the identity key.</p>
+     *
+     * @param s1 the first resource set
+     * @param s2 the second resource set
+     * @return a new resource set containing only resources present in both sets
+     * @throws XMLDBException if an error occurs accessing the resources
+     */
     public static ResourceSet intersection(final ResourceSet s1, final ResourceSet s2) throws XMLDBException {
-        final Map<String, Resource> m1 = new MapResourceSet(s1).getResourcesMap();
-        final Map<String, Resource> m2 = new MapResourceSet(s2).getResourcesMap();
-        final Set<String> set1 = m1.keySet();
-        final Set<String> set2 = m2.keySet();
-        set1.retainAll(set2);
-        final Map<String, Resource> m = new HashMap<>();
-        for (String key : set1) {
-            final Resource resource = m1.get(key);
-            m.put(key, resource);
+        // Build a map from identity key to resource for the first set
+        final Map<String, Resource> m1 = buildIdentityMap(s1);
+
+        // Build the intersection: for each resource in s2, check if it's also in s1.
+        // Use the node identity key (not res.getId()) as the MapResourceSet key
+        // to avoid collapsing multiple nodes from the same document into one entry.
+        final Map<String, Resource> result = new LinkedHashMap<>();
+        for (long i = 0; i < s2.getSize(); i++) {
+            final Resource res = s2.getResource(i);
+            final String key = getIdentityKey(res);
+            if (m1.containsKey(key) && !result.containsKey(key)) {
+                result.put(key, m1.get(key));
+            }
+        }
+
+        return new MapResourceSet(result);
+    }
+
+    /**
+     * Builds a map from identity key to resource.
+     */
+    private static Map<String, Resource> buildIdentityMap(final ResourceSet rs) throws XMLDBException {
+        final Map<String, Resource> map = new LinkedHashMap<>();
+        for (long i = 0; i < rs.getSize(); i++) {
+            final Resource res = rs.getResource(i);
+            final String key = getIdentityKey(res);
+            map.putIfAbsent(key, res);
+        }
+        return map;
+    }
+
+    /**
+     * Returns a key that uniquely identifies a resource by node identity.
+     *
+     * <p>For {@link LocalXMLResource} instances backed by a {@link NodeProxy},
+     * the key combines the document ID and the node ID to uniquely identify
+     * the node within the database. For other resources, the resource ID string
+     * is used as a fallback.</p>
+     */
+    private static String getIdentityKey(final Resource res) throws XMLDBException {
+        if (res instanceof LocalXMLResource localRes) {
+            final NodeProxy proxy = localRes.getNode();
+            if (proxy != null) {
+                return proxy.getOwnerDocument().getDocId() + "#" + proxy.getNodeId();
+            }
         }
-        final MapResourceSet res = new MapResourceSet(m);
-        /*
-         VectorResourceSet res = new VectorResourceSet(); 
-         Collection c1 = new VectorResourceSet(s1).getResources();
-         res.getResources().addAll( c1 );
-         Collection c2 = new VectorResourceSet(s2).getResources();
-         res.getResources().retainAll(c2);
-         return res;
-         */
-        return res;
+        return res.getId();
     }
 }

exist-core/src/main/java/org/exist/xquery/functions/util/Eval.java

@@ -361,6 +361,14 @@ private Sequence doEval(final XQueryContext evalContext, final Sequence contextS
                 exprContext = initContextSequence;
             }
 
+            // If no explicit context was provided and we have a context sequence
+            // from the calling expression (e.g., path expression step), use it.
+            // This allows relative expressions like <a><b/></a>/util:eval('*')
+            // to see the context item.
+            if (exprContext == null && contextSequence != null && !contextSequence.isEmpty()) {
+                exprContext = contextSequence;
+            }
+
             Sequence result = null;
             try {
                 if (!isCalledAs(FS_EVAL_AND_SERIALIZE_NAME)) {

exist-core/src/main/java/org/exist/xupdate/Append.java

@@ -27,7 +27,10 @@
 import org.exist.collections.triggers.TriggerException;
 import org.exist.dom.persistent.DocumentImpl;
 import org.exist.dom.persistent.DocumentSet;
+import org.exist.dom.persistent.ElementImpl;
+import org.exist.dom.persistent.IStoredNode;
 import org.exist.dom.persistent.StoredNode;
+import org.exist.dom.persistent.TextImpl;
 import org.exist.security.Permission;
 import org.exist.security.PermissionDeniedException;
 import org.exist.storage.DBBroker;
@@ -36,6 +39,7 @@
 import org.exist.storage.txn.Txn;
 import org.exist.util.LockException;
 import org.exist.xquery.XPathException;
+import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
 /**
@@ -85,6 +89,10 @@ public long process(Txn transaction) throws PermissionDeniedException, LockExcep
 					throw new PermissionDeniedException("User '" + broker.getCurrentSubject().getName() + "' does not have permission to write to the document '" + doc.getDocumentURI() + "'!");
 				}
 				node.appendChildren(transaction, children, child);
+				// Merge adjacent text nodes to maintain XDM compliance (issue #6160)
+				if (node instanceof ElementImpl) {
+					mergeAdjacentTextNodes(transaction, (ElementImpl) node);
+				}
 				doc.setLastModified(System.currentTimeMillis());
 				modifiedDocuments.add(doc);
 				broker.storeXMLResource(transaction, doc);
@@ -98,6 +106,37 @@ public long process(Txn transaction) throws PermissionDeniedException, LockExcep
 	    }
 	}
 
+	/**
+	 * Merges adjacent text node children of the given element.
+	 * After an append, two text nodes may end up adjacent (e.g., the existing
+	 * last text child and a newly appended text node). The XDM spec requires
+	 * that adjacent text nodes be merged into a single text node.
+	 */
+	private void mergeAdjacentTextNodes(final Txn transaction, final ElementImpl element) {
+		final NodeList childNodes = element.getChildNodes();
+		final int length = childNodes.getLength();
+		IStoredNode<?> prevTextNode = null;
+		for (int i = 0; i < length; i++) {
+			final Node child = childNodes.item(i);
+			if (child.getNodeType() == Node.TEXT_NODE && child instanceof IStoredNode) {
+				if (prevTextNode != null) {
+					// Found adjacent text nodes: merge by updating prev and removing current
+					final String mergedValue = prevTextNode.getNodeValue() + child.getNodeValue();
+					final TextImpl mergedText = new TextImpl(prevTextNode.getExpression(), mergedValue);
+					mergedText.setOwnerDocument(element.getOwnerDocument());
+					element.updateChild(transaction, prevTextNode, mergedText);
+					element.removeChild(transaction, child);
+					// prevTextNode stays as the merged node for potential further merges
+					// but we need to re-read it since updateChild replaced it
+					return; // only one merge needed per append
+				}
+				prevTextNode = (IStoredNode<?>) child;
+			} else {
+				prevTextNode = null;
+			}
+		}
+	}
+
 	@Override
 	public String getName() {
 		return "append";

exist-core/src/test/java/org/exist/xmldb/ResourceSetTest.java

@@ -23,7 +23,10 @@
 
 import org.exist.test.ExistXmldbEmbeddedServer;
 import org.exist.util.io.InputStreamUtil;
-import org.junit.*;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
 import org.xmldb.api.base.*;
 import org.xmldb.api.modules.*;
 
@@ -68,13 +71,12 @@ public void tearDown() throws XMLDBException {
 		service.removeCollection(TEST_COLLECTION);
 	}
 
-	@Ignore("ResourceSet intersection returns wrong count, see #6157")
     @Test
 	public void intersection1() throws XMLDBException {
 		final String xpathPrefix = "doc('/db/" + TEST_COLLECTION + "/shakes.xsl')/*/*";
 		final String query1 = xpathPrefix + "[position() >= 5 ]";
 		final String query2 = xpathPrefix + "[position() <= 10]";
-		final int expected = 87;
+		final int expected = 6;
 
         final XPathQueryService service = testCollection.getService(XPathQueryService.class);
 

exist-core/src/test/java/org/exist/xquery/XQueryFunctionsTest.java

@@ -393,7 +393,6 @@ public void exclusiveLock() throws XMLDBException {
         assertEquals("<root/>", r);
     }
 
-    @Ignore("Context item not propagated into util:eval(), see #6159")
     @Test
     public void utilEval1() throws XMLDBException {
         String query = "<a><b/></a>/util:eval('*')";

exist-core/src/test/java/org/exist/xquery/XQueryTest.java

@@ -1659,7 +1659,6 @@ public void largeAttributeText() throws XMLDBException {
         assertEquals(1, result.getSize());
     }
 
-    @Ignore("Adjacent text nodes not merged after XUpdate append, see #6160")
     @Test
     public void xupdateWithAdjacentTextNodes() throws XMLDBException {
         String query = "let $name := xmldb:store('/db' , 'xupdateTest.xml', <test>aaa</test>)" +