diff --git a/stroom-pipeline/src/main/java/stroom/pipeline/LocationFactory.java b/stroom-pipeline/src/main/java/stroom/pipeline/LocationFactory.java index e4c1e3d82f7..1db3842de21 100644 --- a/stroom-pipeline/src/main/java/stroom/pipeline/LocationFactory.java +++ b/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()); + } + } } diff --git a/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/ExtensionFunctionCallProxy.java b/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/ExtensionFunctionCallProxy.java index b38ba56fa8a..5bf4d54b8ec 100644 --- a/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/ExtensionFunctionCallProxy.java +++ b/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/ExtensionFunctionCallProxy.java @@ -16,14 +16,22 @@ 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; @@ -31,10 +39,35 @@ class ExtensionFunctionCallProxy extends ExtensionFunctionCall { @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)); + } } diff --git a/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionCall.java b/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionCall.java index 1bf767dc1e2..ab37ef32d34 100644 --- a/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionCall.java +++ b/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 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, diff --git a/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionDefinition.java b/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionDefinition.java index dd64feacd5b..25337f4f5f7 100644 --- a/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionDefinition.java +++ b/stroom-pipeline/src/main/java/stroom/pipeline/xsltfunctions/StroomExtensionFunctionDefinition.java @@ -20,6 +20,7 @@ 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; @@ -27,6 +28,7 @@ import net.sf.saxon.om.StructuredQName; import net.sf.saxon.value.SequenceType; +import java.util.ArrayList; import java.util.List; class StroomExtensionFunctionDefinition extends ExtensionFunctionDefinition { @@ -39,7 +41,7 @@ class StroomExtensionFunctionDefinition e private final transient StructuredQName qName; private final Provider functionCallProvider; - private ExtensionFunctionCallProxy proxy; + private List 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 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); } } diff --git a/stroom-util/src/main/java/stroom/util/NullSafe.java b/stroom-util/src/main/java/stroom/util/NullSafe.java index 0c8947c2e0f..f7767b7d737 100644 --- a/stroom-util/src/main/java/stroom/util/NullSafe.java +++ b/stroom-util/src/main/java/stroom/util/NullSafe.java @@ -330,6 +330,18 @@ public static , K, V> boolean hasEntries( } } + /** + * If iterable is non-null, performs action on each item in iterable. + */ + public static void forEach( + final Iterable iterable, + final Consumer action) { + + if (iterable != null) { + iterable.forEach(action); + } + } + /** * Returns a {@link Stream} if collection is non-null else returns an empty {@link Stream} */ diff --git a/stroom-util/src/test/java/stroom/util/TestNullSafe.java b/stroom-util/src/test/java/stroom/util/TestNullSafe.java index 4435e1bd071..eec74fd4efd 100644 --- a/stroom-util/src/test/java/stroom/util/TestNullSafe.java +++ b/stroom-util/src/test/java/stroom/util/TestNullSafe.java @@ -506,7 +506,7 @@ Stream testIsEmptyCollection() { @TestFactory Stream 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() { @@ -542,7 +542,7 @@ Stream testHasItems_collection() { @TestFactory Stream 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() { @@ -557,6 +557,34 @@ Stream testHasItems_array() { .build(); } + @TestFactory + Stream testForEach() { + final List nonEmptyList = List.of(1, 2, 3); + final List emptyList = Collections.emptyList(); + final List nullList = null; + + final List output = new ArrayList<>(); + + return TestUtil.buildDynamicTestStream() + .withWrappedInputType(new TypeLiteral>() { + }) + .withWrappedOutputType(new TypeLiteral>() { + }) + .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 testSize_collection() { final List emptyList = Collections.emptyList(); diff --git a/unreleased_changes/20240212_104601_107__4098.md b/unreleased_changes/20240212_104601_107__4098.md new file mode 100644 index 00000000000..7c732b4f14b --- /dev/null +++ b/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. +```