Skip to content

Commit 5f23038

Browse files
adamretteradamretter
committed
[bugfix] External XQuery variables should be set after query compilation, and before query execution
1 parent ee15dd0 commit 5f23038

File tree

15 files changed

+175
-101
lines changed

15 files changed

+175
-101
lines changed

‎exist-core/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,8 @@
11221122
<include>src/main/java/org/exist/xquery/LocationStep.java</include>
11231123
<include>src/main/java/org/exist/xquery/Module.java</include>
11241124
<include>src/main/java/org/exist/xquery/NamedFunctionReference.java</include>
1125+
<include>src/main/java/org/exist/xquery/NameTest.java</include>
1126+
<include>src/test/java/org/exist/xquery/NodeTypeTest.java</include>
11251127
<include>src/main/java/org/exist/xquery/Optimizer.java</include>
11261128
<include>src/main/java/org/exist/xquery/Option.java</include>
11271129
<include>src/main/java/org/exist/xquery/PerformanceStatsImpl.java</include>
@@ -1336,6 +1338,7 @@
13361338
<include>src/main/java/org/exist/xupdate/Modification.java</include>
13371339
<include>src/test/java/org/exist/xupdate/RemoveAppendTest.java</include>
13381340
<include>src/main/java/org/exist/xupdate/XUpdateProcessor.java</include>
1341+
<include>src/test/java/org/exist/xupdate/XUpdateTest.java</include>
13391342
</includes>
13401343
</licenseSet>
13411344

@@ -1791,6 +1794,7 @@
17911794
<exclude>src/main/java/org/exist/xquery/Module.java</exclude>
17921795
<exclude>src/main/java/org/exist/xquery/NamedFunctionReference.java</exclude>
17931796
<exclude>src/main/java/org/exist/xquery/NameTest.java</exclude>
1797+
<exclude>src/test/java/org/exist/xquery/NodeTypeTest.java</exclude>
17941798
<exclude>src/main/java/org/exist/xquery/Optimizer.java</exclude>
17951799
<exclude>src/main/java/org/exist/xquery/Option.java</exclude>
17961800
<exclude>src/main/java/org/exist/xquery/PerformanceStatsImpl.java</exclude>
@@ -2036,6 +2040,7 @@
20362040
<exclude>src/main/java/org/exist/xupdate/Modification.java</exclude>
20372041
<exclude>src/test/java/org/exist/xupdate/RemoveAppendTest.java</exclude>
20382042
<exclude>src/main/java/org/exist/xupdate/XUpdateProcessor.java</exclude>
2043+
<exclude>src/test/java/org/exist/xupdate/XUpdateTest.java</exclude>
20392044

20402045
<!--
20412046
Derivative work licensed under dbXML 1.0 and LGPL 2.1

‎exist-core/src/main/java/org/exist/http/RESTServer.java‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,7 +1404,6 @@ protected void search(final DBBroker broker, final Txn transaction, final String
14041404
context.setBaseURI(new AnyURIValue(pathUri.toString()));
14051405

14061406
declareNamespaces(context, namespaces);
1407-
declareVariables(context, variables, request, response);
14081407

14091408
final long compilationTime;
14101409
if (compiled == null) {
@@ -1417,6 +1416,8 @@ protected void search(final DBBroker broker, final Txn transaction, final String
14171416
compilationTime = 0;
14181417
}
14191418

1419+
declareVariables(context, variables, request, response);
1420+
14201421
final long executeStart = System.currentTimeMillis();
14211422
final Sequence resultSequence = xquery.execute(broker, compiled, null, outputProperties);
14221423
final long executionTime = System.currentTimeMillis() - executeStart;
@@ -1591,10 +1592,6 @@ private void executeXQuery(final DBBroker broker, final Txn transaction, final D
15911592
context.setStaticallyKnownDocuments(
15921593
new XmldbURI[]{resource.getCollection().getURI()});
15931594

1594-
final HttpRequestWrapper reqw = declareVariables(context, null, request, response);
1595-
reqw.setServletPath(servletPath);
1596-
reqw.setPathInfo(pathInfo);
1597-
15981595
final long compilationTime;
15991596
if (compiled == null) {
16001597
try {
@@ -1610,6 +1607,10 @@ private void executeXQuery(final DBBroker broker, final Txn transaction, final D
16101607
compilationTime = 0;
16111608
}
16121609

1610+
final HttpRequestWrapper reqw = declareVariables(context, null, request, response);
1611+
reqw.setServletPath(servletPath);
1612+
reqw.setPathInfo(pathInfo);
1613+
16131614
DebuggeeFactory.checkForDebugRequest(request, context);
16141615

16151616
final boolean wrap = "yes".equals(outputProperties.getProperty("_wrap"));
@@ -1657,23 +1658,6 @@ private void executeXProc(final DBBroker broker, final Txn transaction, final Do
16571658
context.prepareForReuse();
16581659
}
16591660

1660-
context.declareVariable("pipeline", true, resource.getURI().toString());
1661-
1662-
final String stdin = request.getParameter("stdin");
1663-
context.declareVariable("stdin", true, stdin == null ? "" : stdin);
1664-
1665-
final String debug = request.getParameter("debug");
1666-
context.declareVariable("debug", true, debug == null ? "0" : "1");
1667-
1668-
final String bindings = request.getParameter("bindings");
1669-
context.declareVariable("bindings", true, bindings == null ? "<bindings/>" : bindings);
1670-
1671-
final String autobind = request.getParameter("autobind");
1672-
context.declareVariable("autobind", true, autobind == null ? "0" : "1");
1673-
1674-
final String options = request.getParameter("options");
1675-
context.declareVariable("options", true, options == null ? "<options/>" : options);
1676-
16771661
// TODO: don't hardcode this?
16781662
context.setModuleLoadPath(
16791663
XmldbURI.EMBEDDED_SERVER_URI.append(
@@ -1682,10 +1666,7 @@ private void executeXProc(final DBBroker broker, final Txn transaction, final Do
16821666
context.setStaticallyKnownDocuments(
16831667
new XmldbURI[]{resource.getCollection().getURI()});
16841668

1685-
final HttpRequestWrapper reqw = declareVariables(context, null, request, response);
1686-
reqw.setServletPath(servletPath);
1687-
reqw.setPathInfo(pathInfo);
1688-
1669+
// compile query
16891670
final long compilationTime;
16901671
if (compiled == null) {
16911672
try {
@@ -1702,6 +1683,29 @@ private void executeXProc(final DBBroker broker, final Txn transaction, final Do
17021683
compilationTime = 0;
17031684
}
17041685

1686+
// declare variables
1687+
context.declareVariable("pipeline", true, resource.getURI().toString());
1688+
1689+
final String stdin = request.getParameter("stdin");
1690+
context.declareVariable("stdin", true, stdin == null ? "" : stdin);
1691+
1692+
final String debug = request.getParameter("debug");
1693+
context.declareVariable("debug", true, debug == null ? "0" : "1");
1694+
1695+
final String bindings = request.getParameter("bindings");
1696+
context.declareVariable("bindings", true, bindings == null ? "<bindings/>" : bindings);
1697+
1698+
final String autobind = request.getParameter("autobind");
1699+
context.declareVariable("autobind", true, autobind == null ? "0" : "1");
1700+
1701+
final String options = request.getParameter("options");
1702+
context.declareVariable("options", true, options == null ? "<options/>" : options);
1703+
1704+
final HttpRequestWrapper reqw = declareVariables(context, null, request, response);
1705+
reqw.setServletPath(servletPath);
1706+
reqw.setPathInfo(pathInfo);
1707+
1708+
// execute query
17051709
final long executeStart = System.currentTimeMillis();
17061710
final Sequence result = xquery.execute(broker, compiled, null, outputProperties);
17071711
writeResults(response, broker, transaction, result, -1, 1, false, outputProperties, false, compilationTime, System.currentTimeMillis() - executeStart);

‎exist-core/src/main/java/org/exist/http/urlrewrite/XQueryURLRewrite.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,6 @@ private Sequence runQuery(final DBBroker broker, final RequestWrapper request, f
691691

692692
// Find correct module load path
693693
context.setModuleLoadPath(sourceInfo.moduleLoadPath);
694-
declareVariables(context, sourceInfo, staticRewrite, basePath, request, response);
695694

696695
if (compiled == null) {
697696
try {
@@ -704,6 +703,8 @@ private Sequence runQuery(final DBBroker broker, final RequestWrapper request, f
704703
context.getWatchDog().reset();
705704
}
706705

706+
declareVariables(context, sourceInfo, staticRewrite, basePath, request, response);
707+
707708
model.setSourceInfo(sourceInfo);
708709

709710
return xquery.execute(broker, compiled, null, outputProperties);

‎exist-core/src/main/java/org/exist/repo/Deployment.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -725,33 +725,37 @@ private Sequence runQuery(final DBBroker broker, final XmldbURI targetCollection
725725
context.prepareForReuse();
726726
}
727727

728-
context.declareVariable("dir", true, tempDir.toAbsolutePath().toString());
729-
final Optional<Path> home = broker.getConfiguration().getExistHome();
730-
if (home.isPresent()) {
731-
context.declareVariable("home", true, home.get().toAbsolutePath().toString());
732-
}
733-
734728
if (targetCollection != null) {
735-
context.declareVariable("target", true, targetCollection.toString());
736729
context.setModuleLoadPath(XmldbURI.EMBEDDED_SERVER_URI + targetCollection.toString());
737-
} else {
738-
context.declareVariable("target", true, Sequence.EMPTY_SEQUENCE);
739730
}
740731
if (QueryPurpose.PREINSTALL == purpose) {
741732
// when running pre-setup scripts, base path should point to directory
742733
// because the target collection does not yet exist
743734
context.setModuleLoadPath(tempDir.toAbsolutePath().toString());
744735
}
745736

737+
// Compile query
746738
final XQuery xqueryService = broker.getBrokerPool().getXQueryService();
747-
748739
if (compiled == null) {
749740
compiled = xqueryService.compile(context, source);
750741
} else {
751742
compiled.getContext().updateContext(context);
752743
context.getWatchDog().reset();
753744
}
754745

746+
// Set variables
747+
context.declareVariable("dir", true, tempDir.toAbsolutePath().toString());
748+
final Optional<Path> home = broker.getConfiguration().getExistHome();
749+
if (home.isPresent()) {
750+
context.declareVariable("home", true, home.get().toAbsolutePath().toString());
751+
}
752+
if (targetCollection != null) {
753+
context.declareVariable("target", true, targetCollection.toString());
754+
} else {
755+
context.declareVariable("target", true, Sequence.EMPTY_SEQUENCE);
756+
}
757+
758+
// Execute query
755759
return xqueryService.execute(broker, compiled, null);
756760

757761
} catch (final PermissionDeniedException e) {

‎exist-core/src/main/java/org/exist/storage/serializers/XIncludeFilter.java‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -463,12 +463,6 @@ protected Optional<ResourceError> processXInclude(final String href, String xpoi
463463
context.setHttpContext(serializer.httpContext);
464464
}
465465

466-
//TODO: change these to putting the XmldbURI in, but we need to warn users!
467-
if (document != null) {
468-
context.declareVariable("xinclude:current-doc", true, document.getFileURI().toString());
469-
context.declareVariable("xinclude:current-collection", true, document.getCollection().getURI().toString());
470-
}
471-
472466
if (xpointer != null) {
473467
if (doc != null) {
474468
context.setStaticallyKnownDocuments(new XmldbURI[]{doc.getURI()});
@@ -477,13 +471,6 @@ protected Optional<ResourceError> processXInclude(final String href, String xpoi
477471
}
478472
}
479473

480-
// pass parameters as variables
481-
if (params != null) {
482-
for (final Map.Entry<String, String> entry : params.entrySet()) {
483-
context.declareVariable(entry.getKey(), true, entry.getValue());
484-
}
485-
}
486-
487474
if (compiled == null) {
488475
try {
489476
compiled = xquery.compile(context, source, xpointer != null);
@@ -496,6 +483,18 @@ protected Optional<ResourceError> processXInclude(final String href, String xpoi
496483
}
497484
LOG.info("xpointer query: {}", ExpressionDumper.dump((Expression) compiled));
498485

486+
//TODO: change these to putting the XmldbURI in, but we need to warn users!
487+
if (document != null) {
488+
context.declareVariable("xinclude:current-doc", true, document.getFileURI().toString());
489+
context.declareVariable("xinclude:current-collection", true, document.getCollection().getURI().toString());
490+
}
491+
// pass parameters as variables
492+
if (params != null) {
493+
for (final Map.Entry<String, String> entry : params.entrySet()) {
494+
context.declareVariable(entry.getKey(), true, entry.getValue());
495+
}
496+
}
497+
499498
Sequence contextSeq = null;
500499
if (memtreeDoc != null) {
501500
contextSeq = memtreeDoc;

‎exist-core/src/main/java/org/exist/test/runner/AbstractTestRunner.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,6 @@ protected static Sequence executeQuery(final BrokerPool brokerPool, final Source
115115
}
116116
}
117117

118-
// declare variables for the query
119-
for(final Function<XQueryContext, Tuple2<String, Object>> externalVariableBinding : externalVariableBindings) {
120-
final Tuple2<String, Object> nameValue = externalVariableBinding.apply(context);
121-
context.declareVariable(nameValue._1, true, nameValue._2);
122-
}
123-
124118
final XQuery xqueryService = brokerPool.getXQueryService();
125119

126120
// compile or update the context
@@ -131,6 +125,12 @@ protected static Sequence executeQuery(final BrokerPool brokerPool, final Source
131125
context.getWatchDog().reset();
132126
}
133127

128+
// declare variables for the query
129+
for(final Function<XQueryContext, Tuple2<String, Object>> externalVariableBinding : externalVariableBindings) {
130+
final Tuple2<String, Object> nameValue = externalVariableBinding.apply(context);
131+
context.declareVariable(nameValue._1, true, nameValue._2);
132+
}
133+
134134
return xqueryService.execute(broker, compiledQuery, null);
135135

136136
} finally {

‎exist-core/src/main/java/org/exist/xmldb/LocalXPathQueryService.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ private ResourceSet execute(final DBBroker broker, final Txn transaction, XmldbU
224224
context.setProtectedDocs(lockedDocuments);
225225
}
226226
setupContext(null, context);
227+
declareVariables(context);
227228

228229
final XQuery xquery = brokerPool.getXQueryService();
229230
result = xquery.execute(broker, expr, contextSet, properties);
@@ -302,11 +303,6 @@ private ResourceSet execute(final LocalXmldbFunction<Source> sourceOp) throws XM
302303

303304
context.setStaticallyKnownDocuments(docs);
304305

305-
if (variableDecls.containsKey(Debuggee.PREFIX + ":session")) {
306-
context.declareVariable(Debuggee.SESSION, variableDecls.get(Debuggee.PREFIX + ":session"));
307-
variableDecls.remove(Debuggee.PREFIX + ":session");
308-
}
309-
310306
setupContext(source, context);
311307

312308
if (compiled == null) {
@@ -316,6 +312,8 @@ private ResourceSet execute(final LocalXmldbFunction<Source> sourceOp) throws XM
316312
context.getWatchDog().reset();
317313
}
318314

315+
declareVariables(context);
316+
319317
final Sequence result = xquery.execute(broker, compiled, null, properties);
320318
if (LOG.isDebugEnabled()) {
321319
LOG.debug("query took {} ms.", System.currentTimeMillis() - start);
@@ -416,6 +414,13 @@ protected void setupContext(final Source source, final XQueryContext context) th
416414
for (final Map.Entry<String, String> entry : namespaceDecls.entrySet()) {
417415
context.declareNamespace(entry.getKey(), entry.getValue());
418416
}
417+
}
418+
419+
protected void declareVariables(final XQueryContext context) throws XPathException {
420+
if (variableDecls.containsKey(Debuggee.PREFIX + ":session")) {
421+
context.declareVariable(Debuggee.SESSION, variableDecls.get(Debuggee.PREFIX + ":session"));
422+
variableDecls.remove(Debuggee.PREFIX + ":session");
423+
}
419424

420425
// declare static variables
421426
for (final Map.Entry<String, Object> entry : variableDecls.entrySet()) {

‎exist-core/src/main/java/org/exist/xmlrpc/RpcConnection.java‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,17 @@ protected QueryResult doQuery(final DBBroker broker, final CompiledXQuery compil
269269
checkPragmas(compiled.getContext(), parameters);
270270
LockedDocumentMap lockedDocuments = null;
271271
try {
272+
// declare static variables
273+
final Map<String, Object> variableDecls = (Map<String, Object>) parameters.get(RpcAPI.VARIABLES);
274+
if (variableDecls != null) {
275+
for (final Map.Entry<String, Object> entry : variableDecls.entrySet()) {
276+
if (LOG.isDebugEnabled()) {
277+
LOG.debug("declaring {} = {}", entry.getKey(), entry.getValue());
278+
}
279+
compiled.getContext().declareVariable(entry.getKey(), true, entry.getValue());
280+
}
281+
}
282+
272283
final long start = System.currentTimeMillis();
273284
lockedDocuments = beginProtected(broker, parameters);
274285
if (lockedDocuments != null) {
@@ -354,16 +365,7 @@ private CompiledXQuery compile(final DBBroker broker, final Source source, final
354365
if (namespaces != null && !namespaces.isEmpty()) {
355366
context.declareNamespaces(namespaces);
356367
}
357-
// declare static variables
358-
final Map<String, Object> variableDecls = (Map<String, Object>) parameters.get(RpcAPI.VARIABLES);
359-
if (variableDecls != null) {
360-
for (final Map.Entry<String, Object> entry : variableDecls.entrySet()) {
361-
if (LOG.isDebugEnabled()) {
362-
LOG.debug("declaring {} = {}", entry.getKey(), entry.getValue());
363-
}
364-
context.declareVariable(entry.getKey(), true, entry.getValue());
365-
}
366-
}
368+
367369
final Object[] staticDocuments = (Object[]) parameters.get(RpcAPI.STATIC_DOCUMENTS);
368370
if (staticDocuments != null) {
369371
try {

‎exist-core/src/main/java/org/exist/xupdate/Conditional.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ public long process(Txn transaction) throws PermissionDeniedException, LockExcep
113113
//context.setBackwardsCompatibility(true);
114114
context.setStaticallyKnownDocuments(docs);
115115
declareNamespaces(context);
116-
declareVariables(context);
117116

118117
if (compiled == null) {
119118
try {
@@ -126,6 +125,8 @@ public long process(Txn transaction) throws PermissionDeniedException, LockExcep
126125
context.getWatchDog().reset();
127126
}
128127

128+
declareVariables(context);
129+
129130
final Sequence seq = xquery.execute(broker, compiled, null);
130131
if (seq.effectiveBooleanValue()) {
131132
long mods = 0;

‎exist-core/src/main/java/org/exist/xupdate/Modification.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ protected NodeList select(DocumentSet docs)
190190
}
191191
context.setStaticallyKnownDocuments(docs);
192192
declareNamespaces(context);
193+
193194
declareVariables(context);
195+
194196
if (compiled == null) {
195197
try {
196198
compiled = xquery.compile(context, source);

0 commit comments

Comments
 (0)