search

eXist-db / templating

HTML Templating Library for eXist-db
GNU Lesser General Public License v2.1
5 stars 8 forks source link

Fix boolean parameter casting #24

Closed joewiz closed 1 year ago

joewiz commented 1 year ago

Before this PR, a function that declared a parameter as xs:boolean with a templating default value for this parameter was not cast to xs:boolean.

In eXist 7.0.0-SNAPSHOT, this led to an error in the function-documentation app. Specifically, app:module() function declares that $details parameter be xs:boolean, and the templating annotation supplies a default value for this parameter, but when the function passes this default value to the app:print-module function, an error is raised because the function's signature requires this parameter to be of type xs:boolean. Loading the landing page of the function documentation app raises the error shown below.

This PR fixes the templates:cast function to ensure that templating defaults are correctly cast to match their function's signature when the signature requires them to be xs:boolean.

The hint that this problem was present was https://github.com/eXist-db/function-documentation/pull/64/files#r1187748791.

err:XPTY0004 xs:string(false) is not a sub-type of xs:boolean [at line 111, column 53, source: /db/apps/fundocs/modules/app.xql]
In function:
    app:print-module(element(xqdoc:xqdoc), element(xqdoc:function)*, xs:boolean) [111:9:/db/apps/fundocs/modules/app.xql]
    app:module(node(), map(*), xs:boolean) [-1:-1:/db/apps/fundocs/modules/app.xql]
    templates:process-output(element(), map(*), item()*, element(function)) [211:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:call-by-introspection(element(), map(*), map(*), function(*)) [189:28:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:call(item(), element(), map(*)) [145:37:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [237:13:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [148:81:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [148:81:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [148:81:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [148:81:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [148:81:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [-1:-1:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    lib:parse-params(node(), map(*), xs:string?, xs:string?) [-1:-1:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/lib.xqm]
    templates:process-output(element(), map(*), item()*, element(function)) [211:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:call-by-introspection(element(), map(*), map(*), function(*)) [189:28:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:call(item(), element(), map(*)) [137:36:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [237:13:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process-output(element(), map(*), item()*, element(function)) [211:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:call-by-introspection(element(), map(*), map(*), function(*)) [189:28:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:call(item(), element(), map(*)) [137:36:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [435:17:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process-output(element(), map(*), item()*) [230:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process-output(element(), map(*), item()*, element(function)) [211:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:call-by-introspection(element(), map(*), map(*), function(*)) [189:28:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:call(item(), element(), map(*)) [137:36:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [133:51:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:process(node()*, map(*)) [90:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
    templates:apply(node()+, function(*), map(*)?, map(*)?) [24:5:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.1.0/content/templates.xqm]
joewiz commented 1 year ago

While the CI passes on the tests of eXist 5.2.0, 5.4.1, and (in my local tests) release, the failure on latest is due to the change in error codes in eXist 7.0.0-SNAPSHOT (HEAD), which affects 3 tests here in the templating module.

Here's the patch that enables the tests to pass under 7.0.0-SNAPSHOT:

0001-eXist-7.0.0-SNAPSHOT-compatibility.patch

I do not know how to adjust the tests to be able to detect which matrix environment they're running under and to pass if the error code is 400 under eXist <7 and 500 under eXist >=7.

duncdrum commented 1 year ago

@joewiz i think you would need to adjust the test function to execute system:get-version make the comparison inside the function and use assertTrue as Xqs annotation. Something along these lines.

joewiz commented 1 year ago

@duncdrum Thanks for the pointer! I did a little more research and settled on a solution that hopefully makes sense. I changed the error code check from to.equal(400) to to.be.oneOf([400, 500]), because, as I noted in the commit message just now:

eXist 6.2.0 and earlier issues an error 400 when the fn:error function is called. With 7.0.0-SNAPSHOT (https://github.com/eXist-db/exist/pull/4649), eXist issues an error 500. The precise error code is outside the scope of this package - it’s an issue decided by the eXist API - so we can accept either here.

All tests now pass.

duncdrum commented 1 year ago

This works too,