PR for #4098 - Pipeline processing errors not showing location for stroom:xxxx XSLT func calls

stroom-pipeline/src/main/java/stroom/pipeline/LocationFactory.java

@@ -19,7 +19,21 @@
 import stroom.util.shared.Location;
 
 public interface LocationFactory {
+
     Location create(int colNo, int lineNo);
 
     Location create();
+
+    /**
+     * Create a {@link Location} from the passed {@link Location}, which allows the
+     * {@link LocationFactory} implementation to add any location information in
+     * addition to the line/col. If location is null, creates a new {@link Location}.
+     */
+    default Location create(final Location location) {
+        if (location == null) {
+            return create();
+        } else {
+            return create(location.getLineNo(), location.getColNo());
+        }
+    }
 }

stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/ExtensionFunctionCallProxy.java

@@ -16,25 +16,58 @@
 
 package stroom.pipeline.xsltfunctions;
 
+import stroom.util.NullSafe;
+import stroom.util.shared.DefaultLocation;
+import stroom.util.shared.Location;
+
+import net.sf.saxon.expr.Expression;
+import net.sf.saxon.expr.StaticContext;
 import net.sf.saxon.expr.XPathContext;
 import net.sf.saxon.lib.ExtensionFunctionCall;
 import net.sf.saxon.om.Sequence;
 import net.sf.saxon.trans.XPathException;
 
 class ExtensionFunctionCallProxy extends ExtensionFunctionCall {
+
     private final String functionName;
     private transient StroomExtensionFunctionCall functionCall;
+    private transient Location location = null;
 
     ExtensionFunctionCallProxy(final String functionName) {
         this.functionName = functionName;
     }
 
     @Override
     public Sequence call(final XPathContext context, final Sequence[] arguments) throws XPathException {
-        return functionCall.call(functionName, context, arguments);
+        return functionCall.call(functionName, context, arguments, location);
     }
 
     void setFunctionCall(final StroomExtensionFunctionCall functionCall) {
         this.functionCall = functionCall;
     }
+
+    void clearFunctionCall() {
+        this.functionCall = null;
+    }
+
+    @Override
+    public void supplyStaticContext(final StaticContext context,
+                                    final int locationId,
+                                    final Expression[] arguments) {
+
+        // Called before the func executes, but after setFunctionCall(), so hold the location locally
+        // such that we can pass the location into the function when called.
+        this.location = NullSafe.get(
+                context,
+                StaticContext::getContainingLocation,
+                saxonLocation ->
+                        DefaultLocation.of(saxonLocation.getLineNumber(), saxonLocation.getColumnNumber()));
+
+//        LOGGER.trace("supplyStaticContext [{}({})] for {}({}), line: {}",
+//                functionName,
+//                System.identityHashCode(this),
+//                NullSafe.get(functionCall, Object::getClass, Class::getSimpleName),
+//                NullSafe.get(functionCall, System::identityHashCode),
+//                NullSafe.get(location, Location::getLineNo));
+    }
 }

stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionCall.java

@@ -42,6 +42,13 @@ abstract class StroomExtensionFunctionCall {
     private ErrorReceiver errorReceiver;
     private LocationFactory locationFactory;
     private List<PipelineReference> pipelineReferences;
+    private Location location = null;
+
+    Sequence call(String functionName, XPathContext context, Sequence[] arguments, Location location)
+            throws XPathException {
+        this.location = location;
+        return call(functionName, context, arguments);
+    }
 
     abstract Sequence call(String functionName, XPathContext context, Sequence[] arguments) throws XPathException;
 
@@ -149,13 +156,21 @@ void log(final XPathContext context, final Severity severity, final String messa
     }
 
     private Location getLocation(final XPathContext context) {
-        final Item item = context.getContextItem();
-        if (item instanceof NodeInfo) {
-            final NodeInfo nodeInfo = (NodeInfo) item;
-            return locationFactory.create(nodeInfo.getLineNumber(), nodeInfo.getColumnNumber());
+        if (location != null) {
+            // Favour the location passed in. Use the locationFactory to decorate it with a stream
+            // if it is stream aware
+            return locationFactory.create(location);
+        } else {
+            // Not sure that there is any point in getting the location from the context as
+            // the extension functions operate in their own fresh context, so can't see the location
+            // of the function in the XSLT.
+            final Item<?> item = context.getContextItem();
+            if (item instanceof final NodeInfo nodeInfo) {
+                return locationFactory.create(nodeInfo.getLineNumber(), nodeInfo.getColumnNumber());
+            } else {
+                return locationFactory.create();
+            }
         }
-
-        return locationFactory.create();
     }
 
     void configure(final ErrorReceiver errorReceiver,

stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionDefinition.java

@@ -20,13 +20,15 @@
 import stroom.pipeline.errorhandler.ErrorReceiver;
 import stroom.pipeline.shared.data.PipelineReference;
 import stroom.pipeline.xml.NamespaceConstants;
+import stroom.util.NullSafe;
 
 import jakarta.inject.Provider;
 import net.sf.saxon.lib.ExtensionFunctionCall;
 import net.sf.saxon.lib.ExtensionFunctionDefinition;
 import net.sf.saxon.om.StructuredQName;
 import net.sf.saxon.value.SequenceType;
 
+import java.util.ArrayList;
 import java.util.List;
 
 class StroomExtensionFunctionDefinition<T extends StroomExtensionFunctionCall> extends ExtensionFunctionDefinition {
@@ -39,7 +41,7 @@ class StroomExtensionFunctionDefinition<T extends StroomExtensionFunctionCall> e
     private final transient StructuredQName qName;
     private final Provider<T> functionCallProvider;
 
-    private ExtensionFunctionCallProxy proxy;
+    private List<ExtensionFunctionCallProxy> proxies = null;
 
     StroomExtensionFunctionDefinition(final String functionName,
                                       final int minArgs,
@@ -84,25 +86,30 @@ public SequenceType getResultType(final SequenceType[] suppliedArgumentTypes) {
 
     @Override
     public ExtensionFunctionCall makeCallExpression() {
-        if (proxy == null) {
-            proxy = new ExtensionFunctionCallProxy(functionName);
+        // Think this is called for each call to a func in an XSLT. E.g. if there are two instances
+        // of stroom:log in an XSLT, then this will be called twice (even if one or both of those
+        // instances is called >1 time, e.g. in a loop). Maybe.
+        if (proxies == null) {
+            proxies = new ArrayList<>();
         }
+        final ExtensionFunctionCallProxy proxy = new ExtensionFunctionCallProxy(functionName);
+        proxies.add(proxy);
         return proxy;
     }
 
     void configure(final ErrorReceiver errorReceiver,
                    final LocationFactory locationFactory,
                    final List<PipelineReference> pipelineReferences) {
-        if (proxy != null) {
+        NullSafe.forEach(proxies, proxy -> {
             final StroomExtensionFunctionCall functionCall = functionCallProvider.get();
             functionCall.configure(errorReceiver, locationFactory, pipelineReferences);
             proxy.setFunctionCall(functionCall);
-        }
+        });
     }
 
     void reset() {
-        if (proxy != null) {
-            proxy.setFunctionCall(null);
-        }
+        NullSafe.forEach(
+                proxies,
+                ExtensionFunctionCallProxy::clearFunctionCall);
     }
 }

stroom-util/src/main/java/stroom/util/NullSafe.java

@@ -330,6 +330,18 @@ public static <T1, T2 extends Map<K, V>, K, V> boolean hasEntries(
         }
     }
 
+    /**
+     * If iterable is non-null, performs action on each item in iterable.
+     */
+    public static <T> void forEach(
+            final Iterable<T> iterable,
+            final Consumer<? super T> action) {
+
+        if (iterable != null) {
+            iterable.forEach(action);
+        }
+    }
+
     /**
      * Returns a {@link Stream<E>} if collection is non-null else returns an empty {@link Stream<E>}
      */

stroom-util/src/test/java/stroom/util/TestNullSafe.java

@@ -506,7 +506,7 @@ Stream<DynamicTest> testIsEmptyCollection() {
     @TestFactory
     Stream<DynamicTest> testIsEmptyArray() {
         final String[] emptyArr = new String[0];
-        final String[] nonEmptyArr = new String[]{ "foo", "bar" };
+        final String[] nonEmptyArr = new String[]{"foo", "bar"};
 
         return TestUtil.buildDynamicTestStream()
                 .withWrappedInputType(new TypeLiteral<String[]>() {
@@ -542,7 +542,7 @@ Stream<DynamicTest> testHasItems_collection() {
     @TestFactory
     Stream<DynamicTest> testHasItems_array() {
         final String[] emptyArr = new String[0];
-        final String[] nonEmptyArr = new String[]{ "foo", "bar" };
+        final String[] nonEmptyArr = new String[]{"foo", "bar"};
 
         return TestUtil.buildDynamicTestStream()
                 .withWrappedInputType(new TypeLiteral<String[]>() {
@@ -557,6 +557,34 @@ Stream<DynamicTest> testHasItems_array() {
                 .build();
     }
 
+    @TestFactory
+    Stream<DynamicTest> testForEach() {
+        final List<Integer> nonEmptyList = List.of(1, 2, 3);
+        final List<Integer> emptyList = Collections.emptyList();
+        final List<Integer> nullList = null;
+
+        final List<Integer> output = new ArrayList<>();
+
+        return TestUtil.buildDynamicTestStream()
+                .withWrappedInputType(new TypeLiteral<List<Integer>>() {
+                })
+                .withWrappedOutputType(new TypeLiteral<List<Integer>>() {
+                })
+                .withTestFunction(testCase -> {
+                    NullSafe.forEach(
+                            testCase.getInput(),
+                            output::add);
+                    return output;
+                })
+                .withSimpleEqualityAssertion()
+                .withBeforeTestCaseAction(output::clear)
+                .addCase(null, emptyList)
+                .addCase(emptyList, emptyList)
+                .addCase(nonEmptyList, nonEmptyList)
+                .build();
+    }
+
+
     @TestFactory
     Stream<DynamicTest> testSize_collection() {
         final List<String> emptyList = Collections.emptyList();

unreleased_changes/20240212_104601_107__4098.md

@@ -0,0 +1,24 @@
+* Issue **#4098** : Fix missing location information for stroom:xxx function calls in XSLT.
+
+
+```sh
+# ********************************************************************************
+# Issue title: Pipeline processing errors not showing location for stroom:xxxx XSLT func calls
+# Issue link:  https://github.com/gchq/stroom/issues/4098
+# ********************************************************************************
+
+# ONLY the top line will be included as a change entry in the CHANGELOG.
+# The entry should be in GitHub flavour markdown and should be written on a SINGLE
+# line with no hard breaks. You can have multiple change files for a single GitHub issue.
+# The  entry should be written in the imperative mood, i.e. 'Fix nasty bug' rather than
+# 'Fixed nasty bug'.
+#
+# Examples of acceptable entries are:
+#
+#
+# * Issue **123** : Fix bug with an associated GitHub issue in this repository
+#
+# * Issue **namespace/other-repo#456** : Fix bug with an associated GitHub issue in another repository
+#
+# * Fix bug with no associated GitHub issue.
+```