This is an automated email from the ASF dual-hosted git repository.
slawrence pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/daffodil.git
The following commit(s) were added to refs/heads/main by this push:
new 39cc88cad Change recoverable errors to validation errors instead of
schema definition warnings
39cc88cad is described below
commit 39cc88cad7e5e14815fe0234de5debd455265f1a
Author: Steve Lawrence <[email protected]>
AuthorDate: Mon Oct 24 15:13:22 2022 -0400
Change recoverable errors to validation errors instead of schema definition
warnings
A dfdl:assert with failureType="recoverableError" currently results in a
schema definition warning. However, SDW's are usually used for things
that can be safely ignored, while these asserts are generally used for
validation checking beyond the capabilities of XSD that should not be
ignored, or should at least be treated differently than warnings so that
a user can easily differentiate between the two.
This modifies these recoverable errors to become validation errors,
accessible just like those created when validation is turned on. This
also refactors the handling of errors to avoid duplication between
assert expressions and assert patterns, as well as changes to make error
messages more consistent.
Note that this is a non-backwards compatible change, but is arguably the
more correct behavior since these asserts really are predominantly used
for more complex validation checks.
DAFFODIL-2357
---
.../grammar/primitives/PrimitivesExpressions.scala | 8 +++--
.../processors/parsers/AssertPatternParsers.scala | 37 ++++++++++++----------
.../parsers/ExpressionEvaluatingParsers.scala | 15 ++-------
.../org/apache/daffodil/tdml/TDMLRunner.scala | 12 ++++---
.../org/apache/daffodil/tdml/TestTDMLRunner2.scala | 19 +++++------
.../resources/org/apache/daffodil/layers/IPv4.tdml | 6 ++--
.../daffodil/section07/assertions/assert.tdml | 6 ++--
7 files changed, 52 insertions(+), 51 deletions(-)
diff --git
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesExpressions.scala
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesExpressions.scala
index ef71a9d85..3bd73ba6d 100644
---
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesExpressions.scala
+++
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesExpressions.scala
@@ -86,7 +86,9 @@ abstract class AssertBase(
qn,
NodeInfo.String, msgOpt.get, exprNamespaces,
exprComponent.dpathCompileInfo, false, this, exprComponent.dpathCompileInfo)
} else {
- new ConstantExpression[String](qn, NodeInfo.String, exprWithBraces + "
failed")
+ val typeString = if (discrim) "Discriminator" else "Assertion"
+ val defaultMessage = s"$typeString expression failed: $exprWithBraces"
+ new ConstantExpression[String](qn, NodeInfo.String, defaultMessage)
}
}
@@ -343,7 +345,9 @@ abstract class AssertPatternPrimBase(decl: Term, stmt:
DFDLAssertionBase, discri
if (stmt.messageAttrib.isDefined) {
expr
} else {
- new ConstantExpression[String](qn, NodeInfo.String, testPattern + "
failed")
+ val typeString = if (discrim) "Discriminator" else "Assertion"
+ val defaultMessage = s"$typeString pattern failed: $testPattern"
+ new ConstantExpression[String](qn, NodeInfo.String, defaultMessage)
}
override val forWhat = ForParser
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/AssertPatternParsers.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/AssertPatternParsers.scala
index 408a2599f..e32246d9b 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/AssertPatternParsers.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/AssertPatternParsers.scala
@@ -25,11 +25,27 @@ import org.apache.daffodil.processors._
import org.apache.daffodil.util.OnStack
import org.apache.daffodil.schema.annotation.props.gen.FailureType
-trait AssertMessageEvaluationMixin {
+trait AssertParserMixin {
def messageExpr: CompiledExpression[AnyRef]
def discrim: Boolean
+ def failureType: FailureType
- def getAssertFailureMessage(state: PState): String = {
+ def handleAssertionResult(res: Boolean, state: PState, context:
RuntimeData): Unit = {
+ if (!res) {
+ val message = getAssertFailureMessage(state)
+ if (failureType == FailureType.ProcessingError) {
+ val diag = new AssertionFailed(context.schemaFileLocation, state,
message)
+ state.setFailed(diag)
+ } else
+ state.validationError("%s", message)
+ } else if (discrim) {
+ // this is a discriminator. Successful assertion resolves the in scope
+ // point of uncertainty
+ state.resolvePointOfUncertainty()
+ }
+ }
+
+ private def getAssertFailureMessage(state: PState): String = {
val message =
try {
messageExpr.evaluate(state).asInstanceOf[String]
@@ -54,9 +70,9 @@ class AssertPatternParser(
override val discrim: Boolean,
testPattern: String,
override val messageExpr: CompiledExpression[AnyRef],
- failureType: FailureType)
+ override val failureType: FailureType)
extends PrimParser
- with AssertMessageEvaluationMixin {
+ with AssertParserMixin {
override lazy val runtimeDependencies = Vector()
override def toBriefXML(depthLimit: Int = -1) = {
@@ -75,17 +91,6 @@ class AssertPatternParser(
val isMatch = withMatcher { m => dis.lookingAt(m, start) }
dis.resetPos(mark)
- if (!isMatch) {
- val message = getAssertFailureMessage(start)
- if (failureType == FailureType.ProcessingError) {
- val diag = new AssertionFailed(context.schemaFileLocation, start,
message)
- start.setFailed(diag)
- } else
- start.SDW(message)
- } else if (discrim) {
- // this is a pattern discriminator. Successful match resolves the in
- // scope point of uncertainty
- start.resolvePointOfUncertainty()
- }
+ handleAssertionResult(isMatch, start, context)
}
}
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
index 6ea2248b2..dab56a35a 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
@@ -202,9 +202,9 @@ final class AssertExpressionEvaluationParser(
override val discrim: Boolean, // are we a discriminator or not.
decl: RuntimeData,
expr: CompiledExpression[AnyRef],
- failureType: FailureType)
+ override val failureType: FailureType)
extends ExpressionEvaluationParser(expr, decl)
- with AssertMessageEvaluationMixin {
+ with AssertParserMixin {
def parse(start: PState): Unit = {
Logger.log.debug(s"This is ${toString}")
@@ -232,15 +232,6 @@ final class AssertExpressionEvaluationParser(
if (start.processorStatus ne Success) return
val testResult = res.getBoolean
- if (testResult) {
- if (discrim) start.resolvePointOfUncertainty()
- } else {
- val message = getAssertFailureMessage(start)
- if (failureType == FailureType.ProcessingError) {
- val diag = new AssertionFailed(decl.schemaFileLocation, start, message)
- start.setFailed(diag)
- } else
- start.SDW("Assertion " + message)
- }
+ handleAssertionResult(testResult, start, context)
}
}
diff --git
a/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala
b/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala
index de897236b..8b2f02498 100644
--- a/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala
+++ b/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala
@@ -1059,7 +1059,10 @@ case class ParserTestCase(ptc: NodeSeq, parentArg:
DFDLTestSuite)
VerifyTestCase.verifyParserTestData(resultXmlNode, testInfoset, implString)
(shouldValidate, expectsValidationError) match {
- case (true, true) => {
+ case (_, true) => {
+ // Note that even if shouldValidate is false, we still need to check
+ // for validation diagnostics because failed assertions with
+ // failureType="recoverableError" are treated as validation errors
VerifyTestCase.verifyAllDiagnosticsFound(actual.getDiagnostics,
optExpectedValidationErrors, implString) // verify all validation errors were
found
Assert.invariant(actual.isValidationError)
}
@@ -1067,7 +1070,6 @@ case class ParserTestCase(ptc: NodeSeq, parentArg:
DFDLTestSuite)
VerifyTestCase.verifyNoValidationErrorsFound(actual, implString) //
Verify no validation errors from parser
Assert.invariant(!actual.isValidationError)
}
- case (false, true) => throw TDMLException("Test case invalid. Validation
is off but the test expects an error.", implString)
case (false, false) => // Nothing to do here.
}
@@ -1447,7 +1449,10 @@ case class UnparserTestCase(ptc: NodeSeq, parentArg:
DFDLTestSuite)
VerifyTestCase.verifyAllDiagnosticsFound(actual.getDiagnostics,
optWarnings, implString)
(shouldValidate, expectsValidationError) match {
- case (true, true) => {
+ case (_, true) => {
+ // Note that even if shouldValidate is false, we still need to check
+ // for validation diagnostics because failed assertions with
+ // failureType="recoverableError" are treated as validation errors
VerifyTestCase.verifyAllDiagnosticsFound(actual.getDiagnostics,
optExpectedValidationErrors, implString) // verify all validation errors were
found
Assert.invariant(actual.isValidationError)
}
@@ -1455,7 +1460,6 @@ case class UnparserTestCase(ptc: NodeSeq, parentArg:
DFDLTestSuite)
VerifyTestCase.verifyNoValidationErrorsFound(actual, implString) //
Verify no validation errors from parser
Assert.invariant(!actual.isValidationError)
}
- case (false, true) => throw TDMLException("Test case invalid.
Validation is off but the test expects an error.", implString)
case (false, false) => // Nothing to do here.
}
diff --git
a/daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
b/daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
index 13de56606..c7b1ee06f 100644
---
a/daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
+++
b/daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
@@ -21,7 +21,6 @@ import org.apache.daffodil.Implicits.using
import org.apache.daffodil.xml.XMLUtils
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
-import org.junit.Assert.fail
import org.junit.Test
import org.apache.daffodil.Implicits._
import org.junit.AfterClass
@@ -52,8 +51,12 @@ class TestTDMLRunner2 {
* Exception expected? Yes
*
* Reasoning: The data parses successfully and validation is 'off'.
- * Demonstrates that when validation is off, no validation errors
- * should be expected by the testcase.
+ * Demonstrates that when validation is off, no validation errors will be
+ * generated and so the test will fail because it expects them.
+ *
+ * Note that this test case is not considered invalid because there are ways
+ * to generate validation errors with validation being off, such as asserts
+ * with failureType="recoverableError"
*/
@Test def testValidationOffValidationErrorGivenShouldError() = {
val testSuite =
@@ -88,7 +91,7 @@ class TestTDMLRunner2 {
</tdml:dfdlInfoset>
</tdml:infoset>
<tdml:validationErrors>
- <tdml:error>Specifying this should throw exception</tdml:error>
+ <tdml:error>This error will not be found with validation
disabled</tdml:error>
</tdml:validationErrors>
</tdml:parserTestCase>
</tdml:testSuite>
@@ -99,13 +102,7 @@ class TestTDMLRunner2 {
}
runner.reset
val msg = e.getMessage()
- if (!msg.contains("Test case invalid")) {
- println(msg)
- fail("message did not contain expected contents")
- }
- assertTrue(msg.contains("Test case invalid"))
- assertTrue(msg.contains("Validation is off"))
- assertTrue(msg.contains("test expects an error"))
+ assertTrue(msg.contains("expected but not found"))
}
/**
diff --git
a/daffodil-test/src/test/resources/org/apache/daffodil/layers/IPv4.tdml
b/daffodil-test/src/test/resources/org/apache/daffodil/layers/IPv4.tdml
index 0118bc2bd..3e766b2da 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/layers/IPv4.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/layers/IPv4.tdml
@@ -181,9 +181,9 @@ c0a8 0001 c0a8 00c7
</ipv4:IPv4Header>
</tdml:dfdlInfoset>
</tdml:infoset>
- <tdml:warnings>
- <tdml:warning>Assertion Incorrect checksum</tdml:warning>
- </tdml:warnings>
+ <tdml:validationErrors>
+ <tdml:error>Incorrect checksum</tdml:error>
+ </tdml:validationErrors>
</tdml:parserTestCase>
diff --git
a/daffodil-test/src/test/resources/org/apache/daffodil/section07/assertions/assert.tdml
b/daffodil-test/src/test/resources/org/apache/daffodil/section07/assertions/assert.tdml
index 0e0bce583..a44320112 100644
---
a/daffodil-test/src/test/resources/org/apache/daffodil/section07/assertions/assert.tdml
+++
b/daffodil-test/src/test/resources/org/apache/daffodil/section07/assertions/assert.tdml
@@ -397,9 +397,9 @@
</e3r>
</tdml:dfdlInfoset>
</tdml:infoset>
- <tdml:warnings>
- <tdml:warning>Assertion</tdml:warning>
- </tdml:warnings>
+ <tdml:validationErrors>
+ <tdml:error>Assertion</tdml:error>
+ </tdml:validationErrors>
</tdml:parserTestCase>
<!--
