Skip to content

Commit 720a2cb

Browse files
alanpaxtonalanpaxton
committed
fn:transform - review feedback
Turn XSLT version number into an XSLTVersion class to be more explicit about precise versions, and to avoid potential float equality issues (2.99999999 != 3.0) Declaration order - member variables first Clarify a commented-out line
1 parent 339e40d commit 720a2cb

File tree

4 files changed

+159
-48
lines changed

4 files changed

+159
-48
lines changed

‎exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Options.java‎

Lines changed: 85 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
import javax.xml.transform.stream.StreamSource;
6060
import java.io.Reader;
6161
import java.io.StringReader;
62+
import java.math.BigDecimal;
63+
import java.math.RoundingMode;
6264
import java.net.URI;
6365
import java.net.URISyntaxException;
6466
import java.util.*;
@@ -84,28 +86,11 @@ class Options {
8486
private static final long XXHASH64_SEED = 0x2245a28e;
8587
private static final XXHash64 XX_HASH_64 = XXHashFactory.fastestInstance().hash64();
8688

87-
private Map<QName, XdmValue> readParamsMap(final Optional<MapType> option, final String name) throws XPathException {
88-
89-
final Map<net.sf.saxon.s9api.QName, XdmValue> result = new HashMap<>();
90-
91-
final MapType paramsMap = option.orElse(new MapType(context));
92-
for (final IEntry<AtomicValue, Sequence> entry : paramsMap) {
93-
final AtomicValue key = entry.key();
94-
if (!(key instanceof QNameValue)) {
95-
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Supplied " + name + " is not a valid xs:qname: " + entry);
96-
}
97-
if (entry.value() == null) {
98-
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Supplied " + name + " is not a valid xs:sequence: " + entry);
99-
}
100-
result.put(Convert.ToSaxon.of((QNameValue) key), toSaxon.of(entry.value()));
101-
}
102-
return result;
103-
}
10489

10590
final Tuple2<String, Source> xsltSource;
10691
final MapType stylesheetParams;
10792
final Map<net.sf.saxon.s9api.QName, XdmValue> staticParams;
108-
final float xsltVersion;
93+
final XSLTVersion xsltVersion;
10994
final Optional<AnyURIValue> resolvedStylesheetBaseURI;
11095
final Optional<QNameValue> initialFunction;
11196
final Optional<ArrayType> functionParams;
@@ -161,11 +146,11 @@ private Map<QName, XdmValue> readParamsMap(final Optional<MapType> option, final
161146
final Optional<DecimalValue> explicitXsltVersion = Options.XSLT_VERSION.get(options);
162147
if (explicitXsltVersion.isPresent()) {
163148
try {
164-
xsltVersion = explicitXsltVersion.get().getFloat();
165-
} catch (final XPathException e) {
166-
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Supplied xslt-version is not a valid xs:decimal: " + e.getMessage(), explicitXsltVersion.get(), e);
167-
}
168-
if (xsltVersion != V1_0 && xsltVersion != V2_0 && xsltVersion != V3_0) {
149+
xsltVersion = XSLTVersion.fromDecimal(explicitXsltVersion.get().getValue());
150+
if (xsltVersion.equals(V1_0) && xsltVersion.equals(V2_0) && xsltVersion.equals(V3_0)) {
151+
throw new XPathException(fnTransform, ErrorCodes.FOXT0001, "Supplied xslt-version is an unknown XSLT version: " + explicitXsltVersion.get());
152+
}
153+
} catch (final Transform.PendingException pe) {
169154
throw new XPathException(fnTransform, ErrorCodes.FOXT0001, "Supplied xslt-version is an unknown XSLT version: " + explicitXsltVersion.get());
170155
}
171156
} else {
@@ -230,7 +215,25 @@ private Map<QName, XdmValue> readParamsMap(final Optional<MapType> option, final
230215
vendorOptions = Options.VENDOR_OPTIONS.get(xsltVersion, options);
231216
}
232217

233-
private Delivery.Format getDeliveryFormat(final float xsltVersion, final MapType options) throws XPathException {
218+
private Map<QName, XdmValue> readParamsMap(final Optional<MapType> option, final String name) throws XPathException {
219+
220+
final Map<net.sf.saxon.s9api.QName, XdmValue> result = new HashMap<>();
221+
222+
final MapType paramsMap = option.orElse(new MapType(context));
223+
for (final IEntry<AtomicValue, Sequence> entry : paramsMap) {
224+
final AtomicValue key = entry.key();
225+
if (!(key instanceof QNameValue)) {
226+
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Supplied " + name + " is not a valid xs:qname: " + entry);
227+
}
228+
if (entry.value() == null) {
229+
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Supplied " + name + " is not a valid xs:sequence: " + entry);
230+
}
231+
result.put(Convert.ToSaxon.of((QNameValue) key), toSaxon.of(entry.value()));
232+
}
233+
return result;
234+
}
235+
236+
private Delivery.Format getDeliveryFormat(final XSLTVersion xsltVersion, final MapType options) throws XPathException {
234237
final String deliveryFormatString = Options.DELIVERY_FORMAT.get(xsltVersion, options).orElse(new StringValue(Delivery.Format.DOCUMENT.name())).getStringValue().toUpperCase();
235238
final Delivery.Format format;
236239
try {
@@ -352,16 +355,16 @@ private void validateRequestedProperties(final MapType requestedProperties) thro
352355
Type.DECIMAL,"xslt-version", V1_0, V2_0, V3_0);
353356

354357
abstract static class Option<T> {
355-
public static final float V1_0 = 1.0f;
356-
public static final float V2_0 = 2.0f;
357-
public static final float V3_0 = 3.0f;
358+
public static final XSLTVersion V1_0 = new XSLTVersion(1,0);
359+
public static final XSLTVersion V2_0 = new XSLTVersion(2,0);
360+
public static final XSLTVersion V3_0 = new XSLTVersion(3,0);
358361

359362
protected final StringValue name;
360363
protected final Optional<T> defaultValue;
361-
protected final float[] appliesToVersions;
364+
protected final XSLTVersion[] appliesToVersions;
362365
protected final int itemSubtype;
363366

364-
private Option(final int itemSubtype, final String name, final Optional<T> defaultValue, final float... appliesToVersions) {
367+
private Option(final int itemSubtype, final String name, final Optional<T> defaultValue, final XSLTVersion... appliesToVersions) {
365368
this.name = new StringValue(name);
366369
this.defaultValue = defaultValue;
367370
this.appliesToVersions = appliesToVersions;
@@ -370,16 +373,16 @@ private Option(final int itemSubtype, final String name, final Optional<T> defau
370373

371374
public abstract Optional<T> get(final MapType options) throws XPathException;
372375

373-
private boolean notApplicableToVersion(final float xsltVersion) {
374-
for (final float appliesToVersion : appliesToVersions) {
375-
if (xsltVersion == appliesToVersion) {
376+
private boolean notApplicableToVersion(final XSLTVersion xsltVersion) {
377+
for (final XSLTVersion appliesToVersion : appliesToVersions) {
378+
if (xsltVersion.equals(appliesToVersion)) {
376379
return false;
377380
}
378381
}
379382
return true;
380383
}
381384

382-
public Optional<T> get(final float xsltVersion, final MapType options) throws XPathException {
385+
public Optional<T> get(final XSLTVersion xsltVersion, final MapType options) throws XPathException {
383386
if (notApplicableToVersion(xsltVersion)) {
384387
return Optional.empty();
385388
}
@@ -392,12 +395,12 @@ static class SequenceOption<T extends Sequence> extends Option<T> {
392395

393396
private final int sequenceSubtype;
394397

395-
public SequenceOption(final int sequenceSubtype, final int itemSubtype, final String name, final float... appliesToVersions) {
398+
public SequenceOption(final int sequenceSubtype, final int itemSubtype, final String name, final XSLTVersion... appliesToVersions) {
396399
super(itemSubtype, name, Optional.empty(), appliesToVersions);
397400
this.sequenceSubtype = sequenceSubtype;
398401
}
399402

400-
public SequenceOption(final int sequenceSubtype, final int itemSubtype, final String name, @Nullable final T defaultValue, final float... appliesToVersions) {
403+
public SequenceOption(final int sequenceSubtype, final int itemSubtype, final String name, @Nullable final T defaultValue, final XSLTVersion... appliesToVersions) {
401404
super(itemSubtype, name, Optional.ofNullable(defaultValue), appliesToVersions);
402405
this.sequenceSubtype = sequenceSubtype;
403406
}
@@ -425,11 +428,11 @@ public Optional<T> get(final MapType options) throws XPathException {
425428

426429
static class ItemOption<T extends Item> extends Option<T> {
427430

428-
public ItemOption(final int itemSubtype, final String name, final float... appliesToVersions) {
431+
public ItemOption(final int itemSubtype, final String name, final XSLTVersion... appliesToVersions) {
429432
super(itemSubtype, name, Optional.empty(), appliesToVersions);
430433
}
431434

432-
public ItemOption(final int itemSubtype, final String name, @Nullable final T defaultValue, final float... appliesToVersions) {
435+
public ItemOption(final int itemSubtype, final String name, @Nullable final T defaultValue, final XSLTVersion... appliesToVersions) {
433436
super(itemSubtype, name, Optional.ofNullable(defaultValue), appliesToVersions);
434437
}
435438

@@ -553,7 +556,7 @@ private AnyURIValue resolveURI(final AnyURIValue relative, final AnyURIValue bas
553556
}
554557
}
555558

556-
private float getXsltVersion(final Source xsltStylesheet) throws XPathException {
559+
private XSLTVersion getXsltVersion(final Source xsltStylesheet) throws XPathException {
557560

558561
if (xsltStylesheet instanceof DOMSource) {
559562
return domExtractXsltVersion(xsltStylesheet);
@@ -564,7 +567,7 @@ private float getXsltVersion(final Source xsltStylesheet) throws XPathException
564567
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Unable to extract version from XSLT, unrecognised source");
565568
}
566569

567-
private float domExtractXsltVersion(final Source xsltStylesheet) throws XPathException {
570+
private XSLTVersion domExtractXsltVersion(final Source xsltStylesheet) throws XPathException {
568571

569572
Node node = ((DOMSource) xsltStylesheet).getNode();
570573
if (node instanceof Document) {
@@ -600,13 +603,13 @@ private float domExtractXsltVersion(final Source xsltStylesheet) throws XPathExc
600603
}
601604

602605
try {
603-
return Float.parseFloat(version);
604-
} catch (final NumberFormatException nfe) {
605-
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Unable to extract version from XSLT via DOM. Value: " + version + " : " + nfe.getMessage());
606+
return XSLTVersion.fromDecimal(new BigDecimal(version));
607+
} catch (final Transform.PendingException pe) {
608+
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Unable to extract version from XSLT via DOM. Value: " + version + " : " + pe.getMessage());
606609
}
607610
}
608611

609-
private float staxExtractXsltVersion(final Source xsltStylesheet) throws XPathException {
612+
private XSLTVersion staxExtractXsltVersion(final Source xsltStylesheet) throws XPathException {
610613
try {
611614
final XMLInputFactory factory = XMLInputFactory.newInstance();
612615
// Sonartype checker needs this https://rules.sonarsource.com/java/RSPEC-2755
@@ -621,11 +624,11 @@ private float staxExtractXsltVersion(final Source xsltStylesheet) throws XPathEx
621624
final StartElement startElement = event.asStartElement();
622625
if (Options.QN_XSL_STYLESHEET.equals(startElement.getName())) {
623626
final Attribute version = startElement.getAttributeByName(Options.QN_VERSION);
624-
return Float.parseFloat(version.getValue());
627+
return XSLTVersion.fromDecimal(new BigDecimal(version.getValue()));
625628
}
626629
}
627630
}
628-
} catch (final XMLStreamException e) {
631+
} catch (final XMLStreamException | Transform.PendingException e) {
629632
throw new XPathException(fnTransform, ErrorCodes.FOXT0002, "Unable to extract version from XSLT via STaX: " + e.getMessage(), Sequence.EMPTY_SEQUENCE, e);
630633
}
631634

@@ -658,5 +661,42 @@ static String get(org.exist.dom.QName qName) {
658661
return SystemProperty.getProperty(qName.getNamespaceURI(), qName.getLocalPart(), retainedStaticContext);
659662
}
660663
}
664+
665+
static class XSLTVersion {
666+
final int major;
667+
final int minor;
668+
669+
XSLTVersion(final int major, final int minor) {
670+
this.major = major;
671+
this.minor = minor;
672+
}
673+
674+
public static XSLTVersion fromDecimal(final BigDecimal decimal) throws Transform.PendingException {
675+
final BigDecimal major = decimal.setScale(0, RoundingMode.FLOOR);
676+
final BigDecimal minor = decimal.subtract(major).multiply(BigDecimal.TEN);
677+
try {
678+
return new XSLTVersion(major.intValueExact(), minor.intValueExact());
679+
} catch (final ArithmeticException ae) {
680+
throw new Transform.PendingException("XSLT Version is not an exact X.Y value: " + decimal, ae);
681+
}
682+
}
683+
684+
@Override
685+
public boolean equals(Object o) {
686+
if (this == o) return true;
687+
if (o == null || getClass() != o.getClass()) return false;
688+
XSLTVersion version = (XSLTVersion) o;
689+
return major == version.major && minor == version.minor;
690+
}
691+
692+
@Override
693+
public int hashCode() {
694+
return Objects.hash(major, minor);
695+
}
696+
697+
@Override public String toString() {
698+
return major + "." + minor;
699+
}
700+
}
661701
}
662702

‎exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Transform.java‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import java.util.Optional;
5858

5959
import static com.evolvedbinary.j8fu.tuple.Tuple.Tuple;
60+
import static org.exist.xquery.functions.fn.transform.Options.Option.*;
6061

6162
/**
6263
* Implementation of fn:transform.
@@ -115,7 +116,7 @@ public Sequence eval(final Sequence[] args, final Sequence contextSequence) thro
115116
//TODO(AR) Saxon recommends to use a <code>StreamSource</code> or <code>SAXSource</code> instead of DOMSource for performance
116117
final Optional<Source> sourceNode = Transform.getSourceNode(options.sourceNode, context.getBaseURI());
117118

118-
if (options.xsltVersion == 1.0f || options.xsltVersion == 2.0f || options.xsltVersion == 3.0f) {
119+
if (options.xsltVersion.equals(V1_0) || options.xsltVersion.equals(V2_0) || options.xsltVersion.equals(V3_0)) {
119120
try {
120121
final Holder<XPathException> compileException = new Holder<>();
121122
final XsltExecutable xsltExecutable;
@@ -252,7 +253,7 @@ private XsltExecutable compileExecutable(final Options options) throws XPathExce
252253
*
253254
* @param e the top of the exception stack
254255
* @param defaultErrorCode use this code and its description to fill in blanks in what we finally throw
255-
* @returns XPathException the eventual eXist exception which the caller is expected to throw
256+
* @return XPathException the eventual eXist exception which the caller is expected to throw
256257
*/
257258
private XPathException originalXPathException(final String prefix, @Nonnull final Throwable e, final ErrorCodes.ErrorCode defaultErrorCode) {
258259
Throwable cause = e;
@@ -330,6 +331,7 @@ private class TemplateInvocation {
330331
}
331332

332333
private MapType invokeCallFunction() throws XPathException, SaxonApiException {
334+
assert options.initialFunction.isPresent();
333335
final net.sf.saxon.s9api.QName qName = Convert.ToSaxon.of(options.initialFunction.get().getQName());
334336
final XdmValue[] functionParams;
335337
if (options.functionParams.isPresent()) {
@@ -343,6 +345,7 @@ private MapType invokeCallFunction() throws XPathException, SaxonApiException {
343345
}
344346

345347
private MapType invokeCallTemplate() throws XPathException, SaxonApiException {
348+
assert options.initialTemplate.isPresent();
346349
if (options.initialMode.isPresent()) {
347350
throw new XPathException(fnTransform, ErrorCodes.FOXT0002,
348351
Options.INITIAL_MODE.name + " supplied indicating apply-templates invocation, " +
@@ -482,4 +485,17 @@ public void fatalError(TransformerException exception) throws TransformerExcepti
482485
global.fatalError(exception);
483486
}
484487
}
488+
489+
/**
490+
* A convenience for throwing a checked exception within fn:transform support code,
491+
* without the {@link XQueryContext} necessary for an immediate XPathException.
492+
*
493+
* Useful in a static helper class, for instance.
494+
*/
495+
static class PendingException extends Exception {
496+
497+
public PendingException(String message, Throwable cause) {
498+
super(message, cause);
499+
}
500+
}
485501
}

‎exist-core/src/test/java/org/exist/xquery/functions/fn/transform/FunTransformTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* eXist-db Open Source Native XML Database
3+
* Copyright (C) 2001 The eXist-db Authors
4+
*
5+
* [email protected]
6+
* http://www.exist-db.org
7+
*
8+
* This library is free software; you can redistribute it and/or
9+
* modify it under the terms of the GNU Lesser General Public
10+
* License as published by the Free Software Foundation; either
11+
* version 2.1 of the License, or (at your option) any later version.
12+
*
13+
* This library is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
16+
* Lesser General Public License for more details.
17+
*
18+
* You should have received a copy of the GNU Lesser General Public
19+
* License along with this library; if not, write to the Free Software
20+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
21+
*/
22+
package org.exist.xquery.functions.fn.transform;
23+
24+
import org.junit.jupiter.api.Test;
25+
26+
import java.math.BigDecimal;
27+
28+
import static org.junit.jupiter.api.Assertions.*;
29+
30+
public class FunTransformTest {
31+
32+
@Test
33+
void versionNumbers() throws Transform.PendingException {
34+
35+
Options.XSLTVersion version1 = new Options.XSLTVersion(1, 0);
36+
Options.XSLTVersion version2 = new Options.XSLTVersion(2, 0);
37+
Options.XSLTVersion version3 = new Options.XSLTVersion(3, 0);
38+
Options.XSLTVersion version31 = new Options.XSLTVersion(3, 1);
39+
assertNotEquals(version1, version2);
40+
assertNotEquals(version1, version3);
41+
assertNotEquals(version2, version3);
42+
assertNotEquals(version3, version31);
43+
assertEquals(version3, Options.XSLTVersion.fromDecimal(new BigDecimal("3.0")));
44+
assertNotEquals(version3, Options.XSLTVersion.fromDecimal(new BigDecimal("3.1")));
45+
assertEquals(version31, Options.XSLTVersion.fromDecimal(new BigDecimal("3.1")));
46+
assertEquals(Options.XSLTVersion.fromDecimal(new BigDecimal("3.1")), Options.XSLTVersion.fromDecimal(new BigDecimal("3.1")));
47+
}
48+
49+
@Test void badVersionNumber() throws Transform.PendingException {
50+
51+
assertThrows(Transform.PendingException.class, () -> {
52+
Options.XSLTVersion version311 = Options.XSLTVersion.fromDecimal(new BigDecimal("3.11"));
53+
});
54+
}
55+
}

‎exist-core/src/test/java/xquery/xquery3/XQuery3Tests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
@XSuite.XSuiteFiles({
2929
"src/test/xquery/xquery3",
3030
"src/test/xquery/xquery3/transform",
31-
//"src/test/xquery/xquery3/transform/fnTransform13a.xqm",
31+
//Reminder - add an individual test like this - "src/test/xquery/xquery3/transform/<test-file>.xqm",
3232
})
3333
public class XQuery3Tests {
3434
}

0 commit comments

Comments
 (0)