Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement fn:transform #4386

Merged
merged 72 commits into from Aug 14, 2022
Merged

Conversation

adamretter
Copy link
Member

@adamretter adamretter commented May 12, 2022

Implements the fn:transform function specification

The transform function is complex, so we have chosen to structure it as a number of top level classes in their own
package, org.exist.xquery.functions.fn.transform which are invoked by the main org.exist.xquery.functions.fn.FnTransform class.

The implementation relies on the Saxon XSLT processing library to compile referenced stylesheet(s).

In highly simplified terms, the implementation:

  • Reads and checks all the parameters/options to the function
  • Invokes Saxon to compile the identified stylesheet
  • Retrieves a transformer from the compilation
  • Figures out which form of application of the stylesheet is required
  • Constructs (a) delivery object(s) to build output according to the requested delivery format
  • Calls Saxon again (the transformer this time) applying the stylesheet to the input
  • Builds the result map using the contents of the delivery objects, and returns it

Unit Tests

A number of unit tests have been added, driven by the an attempt to test the same functionality as some of the individual XQTS tests efficiently during development. These tests do not provide complete coverage of the features
of fn:transform, and the XQTS tests should be considered as the truth.

XQTS Tests

The goal of the development of fn:transform has been to pass all the tests in the fn-transform test set of XQTS tests for transformation.

The tests are run with the eXist XQTS Runner

  • The test fn-transform-68 currently fails. We are satisfied that the behaviour exhibited by our
    code is correct, and that the test is deficient Issue

  • All other tests from the fn:transform test set pass, except for 4 tests which are not run because they require features that eXist-db does not support (e.g. custom schema types in XQuery) .


This open source contribution to the eXist-db project was commissioned by the Office of the Historian, U.S. Department of State, https://history.state.gov/.

@adamretter adamretter added this to the eXist-6.1.0 milestone May 12, 2022
@brihaye
Copy link
Contributor

brihaye commented May 12, 2022

Be careful, you copied the W3C's "FOXT0001" error code too many times, I'm afraid ;-)

@adamretter
Copy link
Member Author

Thanks @brihaye I have fixed that now.

@alanpaxton alanpaxton force-pushed the feature/fn-transform branch 2 times, most recently from a53c578 to 13d3ce4 Compare May 24, 2022 07:56
@joewiz joewiz added enhancement new features, suggestions, etc. xquery issue is related to xquery implementation labels May 25, 2022
@sonarcloud
Copy link

sonarcloud bot commented Jun 22, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug E 3 Bugs
Vulnerability E 1 Vulnerability
Security Hotspot A 0 Security Hotspots
Code Smell A 55 Code Smells

39.0% 39.0% Coverage
0.0% 0.0% Duplication

Set up some unit tests from equivalent XQTS tests.
Wrap document returned in a map, per spec.
Serialize output when requested, borrowing eXist serialization code.
Use of stylesheet-params option working
Rename unit tests to .xqm
Improve transform option type error checking
Make badly typed options throw XPTY0004 (type error XPath exception) instead of throwing directly from Java.
Handle some of fn:transform “initial-template” param

This is part of what is termed call-template invocation, and the transformer’s callTemplate() method is used.

Reject call on non-existent source-node
Add getStringValue() and toString() to XQueryContext for help in debugging
Try harder to resolve stylesheet URIs (in retrospect probably not right yet).
and fix up the primary output key in the case where it should not be “output”
XQTS test fn-transform
<test-case name="fn-transform-7b">

Uses a stylesheet of the form:

<out xmlns:xsl='http://www.w3.org/1999/XSL/Transform' xsl:version='2.0'>
                          <xsl:value-of select='.' />
                         </out>

and we were not picking the stylesheet version from that. Burrow through the element and check for the appropriate NamespaceNode and xsl:version.
To set the base URI, we need to set the system id for the stylesheet. Then the stylesheet can resolve-uri successfully, and write secondary output documents.

Add a result document handler to collect the output for secondary output documents.

Result documents now returned to us and we can build tha map with URI and value.

Test runner passing document URI where that exists, and we set it up as the system id to the Saxon transformer if it does exist.
Separate Options class within FnTransform where fully resolved values of options are fetched from the options map once. Called as the first step of eval(), and leaves doing evaluation work based on the options as a cleanly separated task.

Raw output request is now handled, and rejected (pre XSLT 3.0) or produced (3.0+) per the spec.

Handling raw output and checking for the raw option mutliple times was the indication that we needed to factor out the options processing.
When the cause chain for an error contains an XPathException in native eXist, or the Saxon type for an XPathException, work harder to get the correct error code out of that cause, and reflect it in the XPathException ultimately thrown out of FnTransform.

This has an effect on the error thrown by XQTS test transform-68, which becomes consistent with the proposed test suite fix here w3c/qt3tests#44
The fn:transform implementation is large, and has accumulated many static and dynamic subclasses of FnTransform. This change creates its own package, and creates several top level classes in this package.

The FnTransform module itself becomes a much smaller stub around a call into a Transform object which is part of the new package.
Unit tests for fn:transform are mostly composed of convenient UT re-interpretations of the XQTS test suite. There are a lot, so collect them in a subdirectory.
Broken in the refactor. The refactored code was fetching secondary result documents from the wrong delivery object, so the secondary results were non-existent.

Fetch from the correct delivery object.
This seems to break a number of tests outside of fn:transform. Instead, adapt the option handling for fn:transform to do an explicit check on the case of requiring a string, and supply the StringValue of an atomic value if the value is not of string subtype.
fix flagged vulnerability
All XQTS tests work within the raw Saxon to eXist conversion code we have, so remove the fallback conversion-via-serialization, which incidentally helps coverage checking.
XQTS tests were intermittently getting the wrong error message becase we were pullling it as the last error from a shared global error listener (error logger). Make an individual listener for each compilation/request which also side-effects the write to the shared logger.
Cleaned up the ones which make sense.

Not cleaned up - functions with high complexity where the complexity seems essential to the function.

Not cleaned up - old code we have just brushed past lightly, and which sonarcloud has consequently highlighted.
@adamretter adamretter marked this pull request as ready for review August 9, 2022 17:49
@joewiz
Copy link
Member

joewiz commented Aug 9, 2022

@alanpaxton Thank you for this PR and the truly Herculean effort that went into implementing this complex function!

Copy link
Member

@dizzzz dizzzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very impressed by the work, and the needed complexity and wrapping.

@dizzzz
Copy link
Member

dizzzz commented Aug 9, 2022

it would be nice if we could get of the old unmaintained xalan lib.....

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
@sonarcloud
Copy link

sonarcloud bot commented Aug 10, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 21 Code Smells

75.7% 75.7% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new features, suggestions, etc. xquery issue is related to xquery implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants