You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2020/09/04 15:55:34 UTC
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #414: Throw SDE in the case of inputValueCalc containing checkConstraints(.)
mbeckerle commented on a change in pull request #414:
URL: https://github.com/apache/incubator-daffodil/pull/414#discussion_r483685777
##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/ElementBase.scala
##########
@@ -1114,4 +1115,10 @@ trait ElementBase
case (_, _) => this.subset((floating eq YesNo.No), "Property value floating='yes' is not supported.")
}
+ private lazy val checkInputValueCalcExpression: Boolean = {
+ if (this.inputValueCalcOption.toString.contains("checkConstraints(.)")) {
Review comment:
So you are depending on the fact that toString of inputValueCalcOption displays the expression.
The type of inputValueCalcOption is PropertyLookupResult, and that is either a Found or NotFound object.
(annoying that inputValueCalcOption isn't an Option type, but all the properties work this way.)
I think you should not depend quite so much on the toString of a Found displaying the text of the property. If the property value was say, really big then the toString might elide that with "..." in the textual rep, and that would disable your check.
Suggest
```val optIVC = inputValueCalcOption.toOption ```
then use either
```if (optIVC.isDefined) SDEWhen(optIVC.get.value.contains(...), .... )```
or
```optIVC.map{ ivc => SDEWhen(ivc.value.contains(...) , ...) }```
Instead of contains, you should compare using a regex match so that "checkConstraints( . )" will still match (has spaces around the "."). You never know what sort of whitespace convention someone will use for their DFDL schema, particularly for these XPath-like expressions.
##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions.tdml
##########
@@ -2440,38 +2440,47 @@
Purpose: This test demonstrates that checkConstraints cannot reference an inputValueCalc-ed element because the value is being computed by inputValueCalc
-->
- <tdml:parserTestCase name="dfdlCheckConstraints"
- root="e1" model="checkConstraints" description="DFDL-23-135R">
+ <tdml:parserTestCase name="dfdlCheckConstraintsFails"
+ root="e1" model="checkConstraints" description="DFDL-23-135R">
<tdml:document>6</tdml:document>
<tdml:errors>
<tdml:error>Schema Definition Error</tdml:error>
- <tdml:error>Placeholder</tdml:error>
+ <tdml:error>you cannot use checkConstraints(.)</tdml:error>
</tdml:errors>
</tdml:parserTestCase>
- <tdml:parserTestCase name="checkConstraintsComplexTypeFails"
- root="e7" model="checkConstraints" description="DFDL-23-135R">
- <tdml:document>s1s2</tdml:document>
- <tdml:errors>
- <tdml:error>Schema Definition Error</tdml:error>
- <tdml:error>checkConstraints</tdml:error>
- <tdml:error>AnyAtomic</tdml:error>
- </tdml:errors>
- </tdml:parserTestCase>
+<!-- <tdml:parserTestCase name="dfdlCheckConstraints"-->
Review comment:
Take out these commented-out tests now.
##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions.tdml
##########
@@ -2408,8 +2408,8 @@
<!-- This test is incorrect. An element of simple type cannot use "."
in any inputValueCalc, as "." means "the value of this element", so
we're chasing our tail here. -->
- <xs:element name="e8" type="xs:int"
- dfdl:inputValueCalc="{ dfdl:checkConstraints(.) }" />
+<!-- <xs:element name="e8" type="xs:int"-->
Review comment:
This one doesn't have the problematic IVC. So the comment above "This test is incorrect" doesn't match does it?
Fix comment(moves up to before the prior element?), remove unused element decl.
##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions.tdml
##########
@@ -2440,38 +2440,47 @@
Purpose: This test demonstrates that checkConstraints cannot reference an inputValueCalc-ed element because the value is being computed by inputValueCalc
-->
- <tdml:parserTestCase name="dfdlCheckConstraints"
- root="e1" model="checkConstraints" description="DFDL-23-135R">
+ <tdml:parserTestCase name="dfdlCheckConstraintsFails"
+ root="e1" model="checkConstraints" description="DFDL-23-135R">
<tdml:document>6</tdml:document>
<tdml:errors>
<tdml:error>Schema Definition Error</tdml:error>
- <tdml:error>Placeholder</tdml:error>
+ <tdml:error>you cannot use checkConstraints(.)</tdml:error>
</tdml:errors>
</tdml:parserTestCase>
- <tdml:parserTestCase name="checkConstraintsComplexTypeFails"
- root="e7" model="checkConstraints" description="DFDL-23-135R">
- <tdml:document>s1s2</tdml:document>
- <tdml:errors>
- <tdml:error>Schema Definition Error</tdml:error>
- <tdml:error>checkConstraints</tdml:error>
- <tdml:error>AnyAtomic</tdml:error>
- </tdml:errors>
- </tdml:parserTestCase>
+<!-- <tdml:parserTestCase name="dfdlCheckConstraints"-->
+<!-- root="e1" model="checkConstraints" description="DFDL-23-135R">-->
+<!-- <tdml:document>6</tdml:document>-->
+<!-- <tdml:errors>-->
+<!-- <tdml:error>Schema Definition Error</tdml:error>-->
+<!-- <tdml:error>Placeholder</tdml:error>-->
+<!-- </tdml:errors>-->
+<!-- </tdml:parserTestCase>-->
+
+<!-- <tdml:parserTestCase name="checkConstraintsComplexTypeFails"-->
+<!-- root="e7" model="checkConstraints" description="DFDL-23-135R">-->
+<!-- <tdml:document>s1s2</tdml:document>-->
+<!-- <tdml:errors>-->
+<!-- <tdml:error>Schema Definition Error</tdml:error>-->
+<!-- <tdml:error>checkConstraints</tdml:error>-->
+<!-- <tdml:error>AnyAtomic</tdml:error>-->
+<!-- </tdml:errors>-->
+<!-- </tdml:parserTestCase>-->
<!--
Test Name: dfdlCheckConstraints2
Schema: checkConstraints
Purpose: This test demonstrates that checkConstraints cannot reference an inputValueCalc-ed element because the value is being computed by inputValueCalc
-->
- <tdml:parserTestCase name="dfdlCheckConstraints2" validation="on"
- root="e8" model="checkConstraints" description="DFDL-23-135R">
- <tdml:document></tdml:document>
- <tdml:errors>
- <tdml:error>Schema Definition Error</tdml:error>
- <tdml:error>Placeholder</tdml:error>
- </tdml:errors>
- </tdml:parserTestCase>
+<!-- <tdml:parserTestCase name="dfdlCheckConstraints2" validation="on"-->
Review comment:
and here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org