Copilot commented on code in PR #6871:
URL: https://github.com/apache/incubator-kie/pull/6871#discussion_r3714585698
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/XQueryImplUtil.java:
##########
@@ -26,11 +26,19 @@
import net.sf.saxon.s9api.XQueryExecutable;
import net.sf.saxon.s9api.SaxonApiException;
-import java.util.regex.Pattern;
-
public class XQueryImplUtil {
- private static final Pattern XML_CHARACTER_REFERENCES_PATTERN =
Pattern.compile("['\"&<>]");
+ /**
+ * Single Saxon Processor instance. Processor is thread-safe and expensive
to construct
+ * (it initialises the Saxon Configuration and performs a license check).
One instance
+ * per JVM is the Saxon-recommended pattern.
+ */
+ private static final Processor PROCESSOR = new Processor(false);
Review Comment:
Reusing a single static `XQueryCompiler` across calls can be unsafe if
`XQueryCompiler` is not thread-safe (Saxon compiler objects commonly hold
mutable static context/options). To avoid potential races under concurrent FEEL
evaluations, keep the static `Processor` but create a new `XQueryCompiler` per
invocation, or use a `ThreadLocal<XQueryCompiler>`, or synchronize around
`COMPILER.compile(...)` (tradeoff: synchronization may reduce throughput).
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/XQueryImplUtil.java:
##########
@@ -26,11 +26,19 @@
import net.sf.saxon.s9api.XQueryExecutable;
import net.sf.saxon.s9api.SaxonApiException;
-import java.util.regex.Pattern;
-
public class XQueryImplUtil {
- private static final Pattern XML_CHARACTER_REFERENCES_PATTERN =
Pattern.compile("['\"&<>]");
+ /**
+ * Single Saxon Processor instance. Processor is thread-safe and expensive
to construct
+ * (it initialises the Saxon Configuration and performs a license check).
One instance
+ * per JVM is the Saxon-recommended pattern.
+ */
+ private static final Processor PROCESSOR = new Processor(false);
+
+ /**
+ * Single XQueryCompiler instance. XQueryCompiler is thread-safe and
reusable.
+ */
+ private static final XQueryCompiler COMPILER =
PROCESSOR.newXQueryCompiler();
Review Comment:
Reusing a single static `XQueryCompiler` across calls can be unsafe if
`XQueryCompiler` is not thread-safe (Saxon compiler objects commonly hold
mutable static context/options). To avoid potential races under concurrent FEEL
evaluations, keep the static `Processor` but create a new `XQueryCompiler` per
invocation, or use a `ThreadLocal<XQueryCompiler>`, or synchronize around
`COMPILER.compile(...)` (tradeoff: synchronization may reduce throughput).
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/XQueryImplUtil.java:
##########
@@ -50,9 +58,7 @@ public static String executeReplaceFunction(String input,
String pattern, String
static <T> T evaluateXQueryExpression(String expression, Class<T>
expectedTypeResult) {
try {
- Processor processor = new Processor(false);
- XQueryCompiler compiler = processor.newXQueryCompiler();
- XQueryExecutable executable = compiler.compile(expression);
+ XQueryExecutable executable = COMPILER.compile(expression);
Review Comment:
Reusing a single static `XQueryCompiler` across calls can be unsafe if
`XQueryCompiler` is not thread-safe (Saxon compiler objects commonly hold
mutable static context/options). To avoid potential races under concurrent FEEL
evaluations, keep the static `Processor` but create a new `XQueryCompiler` per
invocation, or use a `ThreadLocal<XQueryCompiler>`, or synchronize around
`COMPILER.compile(...)` (tradeoff: synchronization may reduce throughput).
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/XQueryImplUtil.java:
##########
@@ -66,22 +72,41 @@ static <T> T evaluateXQueryExpression(String expression,
Class<T> expectedTypeRe
} catch (SaxonApiException e) {
throw new IllegalArgumentException(e);
}
- }
+ }
/**
- * It replaces all the XML Character References (&, ", ', <, >) in a given
input string with their "escaping" characters.
- * This is required to run XPath functions containing XML Character
References.
- * @param input A string input representing one of the parameter of
managed functions
- * @return A sanitized string
+ * Escapes the five XML special characters (& " ' < >)
in a single
+ * pass over the string, so they are safe to embed as XQuery string
literals.
+ * Returns {@code null} unchanged; returns the original reference when no
escaping is needed.
+ *
+ * @param input A string parameter of a managed XQuery function
+ * @return The escaped string, or the original if no special characters
were present
*/
static String escapeXmlCharactersReferencesForXPath(String input) {
Review Comment:
The updated Javadoc explicitly describes XQuery string-literal escaping, but
the method name still says `...ForXPath`, which is now misleading. Consider
renaming to something XQuery-specific (e.g.,
`escapeXmlCharactersForXQueryStringLiteral`) and keeping the old name as a
deprecated wrapper to preserve API compatibility (if this is externally
referenced).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
