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

V8 #165

Merged
merged 38 commits into from
Mar 8, 2024
Merged

V8 #165

merged 38 commits into from
Mar 8, 2024

Conversation

phax
Copy link
Owner

@phax phax commented Feb 15, 2024

bertrand-lorentz and others added 30 commits February 14, 2024 15:11
Make the evaluation of variables declared in <let> more consistent with
the Schematron ISO spec and other implementation.

During bind(), store the variables as members of the bound item they are
defined in: schema, pattern and rule. The variables declared in a phase
are stored in the schema, as it is already bound to the phase.
Each set of variables is stored as map of name to compiled XPath
expression.

During validate(), for each item we evaluate the corresponding variables
in the appropriate context, and store the result in a
LetVariableResolver instance.
This instance is set as the XPathVariableResolver, so a variable value
will be resolved when executing an XPath that references the variable.

This fixes issue #143, so enable the corresponding unit test.
This also resolves issue #88, so fix the corresponding unit test, as it
was checking for the current behaviour, which is not correct.
Make the evaluation of variables declared in <let> more consistent with
the Schematron ISO spec and other implementation.

During bind(), store the variables as members of the bound item they are
defined in: schema, pattern and rule. The variables declared in a phase
are stored in the schema, as it is already bound to the phase.
Each set of variables is stored as map of name to compiled XPath
expression.

During validate(), for each item we evaluate the corresponding variables
in the appropriate context, and store the result in a
LetVariableResolver instance.
This instance is set as the XPathVariableResolver, so a variable value
will be resolved when executing an XPath that references the variable.

This fixes issue #143, so enable the corresponding unit test.
This also resolves issue #88, so fix the corresponding unit test, as it
was checking for the current behaviour, which is not correct.
@phax
Copy link
Owner Author

phax commented Feb 15, 2024

@bertrand-lorentz I did some further changes and cleansing - all the tests are still passing so I am confident that it is okay. The most relevant change was to use the "LetVariableResolver" only inside the bound Schematron to make sure it is always there. This simplifies the code and avoid dealing with the issue of it not being present.

If you don't have any objections, I would merge this to main and start a release build.

@phax phax self-assigned this Feb 15, 2024
@bertrand-lorentz
Copy link
Contributor

The change on the resolver looks good. Thanks for cleaning up ! 😄

I did notice an exception while running the unit test (although the test is not considered as failure):

[INFO] Running com.helger.schematron.pure.bound.xpath.PSXPathBoundSchemaTest
. . .
3883 [main] ERROR com.helger.schematron.pure.errorhandler.LoggingPSErrorHandler - [error] [PSRule] @ external/test-sch/CellarBook.sch Failed to evaluate XPath expression to a boolean: '$nbBottles < 10' (net.sf.saxon.trans.XPathException: Cannot compare xs:string to xs:integer)
; SystemID: ; Line#: 1; Column#: 1
net.sf.saxon.trans.XPathException: Cannot compare xs:string to xs:integer
	at net.sf.saxon.expr.ValueComparison.compare(ValueComparison.java:397)
	at net.sf.saxon.expr.GeneralComparison.compare(GeneralComparison.java:694)
	at net.sf.saxon.expr.GeneralComparison$GeneralComparisonElaborator.evaluateManyToOne(GeneralComparison.java:903)
	at net.sf.saxon.expr.GeneralComparison$GeneralComparisonElaborator.lambda$elaborateForBoolean$1(GeneralComparison.java:839)
	at net.sf.saxon.xpath.XPathExpressionImpl.evaluate(XPathExpressionImpl.java:182)
	at com.helger.schematron.pure.xpath.XPathEvaluationHelper.evaluate(XPathEvaluationHelper.java:65)
	at com.helger.schematron.pure.xpath.XPathEvaluationHelper.evaluateAsBooleanObj(XPathEvaluationHelper.java:74)
	at com.helger.schematron.pure.xpath.XPathEvaluationHelper.evaluateAsBoolean(XPathEvaluationHelper.java:81)
	at com.helger.schematron.pure.bound.xpath.PSXPathBoundSchema._validateSerial(PSXPathBoundSchema.java:887)

I think it's because in PSXPathBoundSchema._evaluateVariables evaluateAsString is tried before evaluateAsNumber: an XPath expression that returns a number, like sum(wine/quantity), will not cause an exception when be evaluated as a string. But it then fails when the variable is used in an expression that expects a number.

I had put evaluateAsNumber before evaluateAsString to avoid this. This was just the result of trial and error, I'm not sure what the correct order is.

Note: you don't need to rush out a release for my account. It will anyways take a while before we can make use of it in our application.

@phax
Copy link
Owner Author

phax commented Mar 8, 2024

I changed the order to

  1. number
  2. boolean
  3. string
    to fix the exception - seems to be okay now. Added some more test cases to make sure no other exception occurs

@phax phax merged commit 25e0fa5 into master Mar 8, 2024
2 of 3 checks passed
@phax phax deleted the v8 branch March 9, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants