From 225b8763e75f051a4edfb0b03e2a64b6b1122c25 Mon Sep 17 00:00:00 2001 From: olabusayoT <50379531+olabusayoT@users.noreply.github.com> Date: Mon, 25 Mar 2024 15:28:41 -0400 Subject: [PATCH] Add support for validation of length facet - add support for length, also set minLength and maxLength with the value of length, when hasLength is true, and those variables are queried - ensure length and minLength and maxLength facets are not declared together (Xerces Error, but we have asserts incase Xerces validation is turned off) - fix error message for isSimpleType check in computeMinMaxLength and length definitions - add tests for validation=limited and validation=on - added hexBinary tests including test with facet length=0 DAFFODIL-2842 --- .../daffodil/core/dsom/ElementBase.scala | 77 +++++- .../apache/daffodil/core/dsom/Facets.scala | 34 ++- .../daffodil/core/dsom/RestrictionUnion.scala | 7 + .../daffodil/core/dsom/SimpleTypes.scala | 2 +- .../grammar/ElementBaseGrammarMixin.scala | 20 +- .../runtime1/SimpleTypeRuntime1Mixin.scala | 1 + .../daffodil/runtime1/dsom/Facets1.scala | 1 + .../runtime1/processors/RuntimeData.scala | 36 ++- .../validation_errors/Validation.tdml | 242 +++++++++++++++++- .../validation_errors/TestValidationErr.scala | 28 ++ 10 files changed, 421 insertions(+), 27 deletions(-) diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ElementBase.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ElementBase.scala index 15740e4e8f..45d1566e9d 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ElementBase.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ElementBase.scala @@ -83,6 +83,7 @@ trait ElementBase requiredEvaluationsIfActivated(isSimpleType) requiredEvaluationsIfActivated(if (hasPattern) patternValues) requiredEvaluationsIfActivated(if (hasEnumeration) enumerationValues) + requiredEvaluationsIfActivated(if (hasLength) length) requiredEvaluationsIfActivated(if (hasMinLength) minLength) requiredEvaluationsIfActivated(if (hasMaxLength) maxLength) requiredEvaluationsIfActivated(if (hasMinInclusive) minInclusive) @@ -711,8 +712,8 @@ trait ElementBase Assert.invariant(repElement.lengthKind =:= LengthKind.Implicit) // it's a string with implicit length. get from facets schemaDefinitionUnless( - repElement.hasMaxLength, - "String with dfdl:lengthKind='implicit' must have an XSD maxLength facet value.", + repElement.hasMaxLength || repElement.hasLength, + "String with dfdl:lengthKind='implicit' must have an XSD maxLength or XSD Length facet value.", ) val ml = repElement.maxLength ml.longValue() @@ -941,8 +942,9 @@ trait ElementBase private lazy val hasPattern: Boolean = typeDef.optRestriction.exists(_.hasPattern) private lazy val hasEnumeration: Boolean = typeDef.optRestriction.exists(_.hasEnumeration) - protected lazy val hasMinLength = typeDef.optRestriction.exists(_.hasMinLength) - protected lazy val hasMaxLength = typeDef.optRestriction.exists(_.hasMaxLength) + protected lazy val hasLength: Boolean = typeDef.optRestriction.exists(_.hasLength) + protected lazy val hasMinLength: Boolean = typeDef.optRestriction.exists(_.hasMinLength) + protected lazy val hasMaxLength: Boolean = typeDef.optRestriction.exists(_.hasMaxLength) private lazy val hasMinInclusive = typeDef.optRestriction.exists(_.hasMinInclusive) private lazy val hasMaxInclusive = typeDef.optRestriction.exists(_.hasMaxInclusive) private lazy val hasMinExclusive = typeDef.optRestriction.exists(_.hasMinExclusive) @@ -960,12 +962,73 @@ trait ElementBase typeDef.optRestriction.flatMap { _.enumerationValues } } + final lazy val length: java.math.BigDecimal = { + Assert.invariant(hasLength) + schemaDefinitionUnless( + isSimpleType, + "Facet length is allowed only on simple types or types derived from simple types.", + ) + typeDef match { + case _ if hasRepType => { + // this length is only used for the length of the representation. The + // values used for facet restrictions during limited validation come from elsewhere + repTypeElementDecl.length + } + case prim: PrimitiveType => { + val pt = prim.primType + schemaDefinitionWhen( + (pt == PrimType.String || pt == PrimType.HexBinary) && lengthKind == LengthKind.Implicit, + "Facet length must be defined for type %s with lengthKind='implicit'", + pt.name, + ) + // + // We handle text numbers by getting a stringValue first, then + // we convert to the number type. + // + // This means we cannot check and SDE here on incorrect simple type. + zeroBD + } + case st: SimpleTypeDefBase if st.optRestriction.isDefined => { + val r = st.optRestriction.get + val pt = st.primType + val typeOK = pt == PrimType.String || pt == PrimType.HexBinary + schemaDefinitionWhen( + !typeOK && hasLength, + "Facet length is not allowed on types derived from type %s.\nIt is allowed only on types derived from string and hexBinary.", + pt.name, + ) + val res = (hasLength, lengthKind) match { + case (false, LengthKind.Implicit) => + SDE( + "When lengthKind='implicit', length facet must be specified.", + ) + case (true, _) => r.lengthValue + case (false, _) => zeroBD + case _ => Assert.impossible() + } + res + } + case st: SimpleTypeDefBase => { + Assert.invariant(st.optRestriction.isEmpty) + zeroBD + } + } + } + /** * Compute minLength and maxLength together to share error-checking * and case dispatch that would otherwise have to be repeated. + * + * Also set them to the value of length, in the case we've used the length facet */ - final lazy val (minLength: java.math.BigDecimal, maxLength: java.math.BigDecimal) = - computeMinMaxLength + final lazy val (minLength: java.math.BigDecimal, maxLength: java.math.BigDecimal) = { + // if hasLength is true, and min/maxLength is queried, set it to length + if (hasLength) { + (length, length) + } else { + computeMinMaxLength + } + } // TODO: why are we using java.math.BigDecimal, when scala has a much // nicer decimal class? private val zeroBD = new java.math.BigDecimal(0) @@ -974,7 +1037,7 @@ trait ElementBase private def computeMinMaxLength: (java.math.BigDecimal, java.math.BigDecimal) = { schemaDefinitionUnless( isSimpleType, - "Facets minLength and maxLength are allowed only on types string and hexBinary.", + "Facets minLength and maxLength are allowed only on simple types or types derived from simple types.", ) typeDef match { case _ if hasRepType => { diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/Facets.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/Facets.scala index 0c448f106d..c246f4f868 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/Facets.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/Facets.scala @@ -51,6 +51,9 @@ trait Facets { self: Restriction => private def fractionDigits(xml: Node): String = { retrieveFacetValueFromRestrictionBase(xml, Facet.fractionDigits) } + private def length(xml: Node): String = { + retrieveFacetValueFromRestrictionBase(xml, Facet.length) + } private def maxExclusive(xml: Node): String = { retrieveFacetValueFromRestrictionBase(xml, Facet.maxExclusive) } @@ -93,8 +96,27 @@ trait Facets { self: Restriction => final lazy val localMaxInclusiveValue: String = maxInclusive(xml) final lazy val localMinExclusiveValue: String = minExclusive(xml) final lazy val localMaxExclusiveValue: String = maxExclusive(xml) - final lazy val localMinLengthValue: String = minLength(xml) - final lazy val localMaxLengthValue: String = maxLength(xml) + final lazy val localLengthValue: String = length(xml) + final lazy val localMinLengthValue: String = { + val ml = minLength(xml) + // Xerces checks for the case where length and min/maxLength are used together, + // so we won't get to this code in those cases unless Xerces validation is turned off + Assert.usage( + ml.isEmpty || localLengthValue.isEmpty, + "Length and minLength cannot be specified together", + ) + ml + } + final lazy val localMaxLengthValue: String = { + val ml = maxLength(xml) + // Xerces checks for the case where length and min/maxLength are used together, + // so we won't get to this code in those cases unless Xerces validation is turned off + Assert.usage( + ml.isEmpty || localLengthValue.isEmpty, + "Length and maxLength cannot be specified together", + ) + ml + } final lazy val localTotalDigitsValue: String = totalDigits(xml) final lazy val localFractionDigitsValue: String = fractionDigits(xml) final lazy val localEnumerationValue: String = { @@ -140,6 +162,8 @@ trait Facets { self: Restriction => (localEnumerationValue.length > 0) || (getRemoteFacetValues(Facet.enumeration).size > 0) final lazy val hasPattern: Boolean = (localPatternValue.length > 0) || (getRemoteFacetValues(Facet.pattern).size > 0) + final lazy val hasLength: Boolean = + (localLengthValue != "") || (getRemoteFacetValues(Facet.length).size > 0) final lazy val hasMinLength: Boolean = (localMinLengthValue != "") || (getRemoteFacetValues(Facet.minLength).size > 0) final lazy val hasMaxLength: Boolean = @@ -230,6 +254,8 @@ trait Facets { self: Restriction => // TODO: Tidy up. Can likely replace getFacetValue with a similar call to combinedBaseFacets // as combinedBaseFacets should contain the 'narrowed' values. // + final lazy val lengthValue: java.math.BigDecimal = + getFacetValue(localLengthValue, Facet.length, hasLength) final lazy val minLengthValue: java.math.BigDecimal = getFacetValue(localMinLengthValue, Facet.minLength, hasMinLength) final lazy val maxLengthValue: java.math.BigDecimal = @@ -387,7 +413,7 @@ trait Facets { self: Restriction => errorOnLocalLessThanBaseFacet(theLocalFacet, theRemoteFacet, facetType) localFacet } - case Facet.maxLength | Facet.fractionDigits => { + case Facet.length | Facet.maxLength | Facet.fractionDigits => { errorOnLocalGreaterThanBaseFacet(theLocalFacet, theRemoteFacet, facetType) localFacet } @@ -679,7 +705,7 @@ trait Facets { self: Restriction => // a negative number, zero, or a positive number as this BigInteger is numerically less than, // equal to, or greater than o, which must be a BigInteger. facetType match { - case Facet.minLength | Facet.maxLength | Facet.fractionDigits => { + case Facet.length | Facet.minLength | Facet.maxLength | Facet.fractionDigits => { // Non-negative Integers. BigInt narrowNonNegativeFacets(localFacet, remoteFacet, facetType) } diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/RestrictionUnion.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/RestrictionUnion.scala index 2bd168fea9..72a49cdba6 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/RestrictionUnion.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/RestrictionUnion.scala @@ -118,6 +118,9 @@ final class Restriction private (xmlArg: Node, val simpleTypeDef: SimpleTypeDefB lazy val localBaseFacets: ElemFacets = { val myFacets: Queue[FacetValue] = Queue.empty // val not var - it's a mutable collection if (localPatternValue.length > 0) { myFacets.enqueue((Facet.pattern, localPatternValue)) } + if (localLengthValue.length > 0) { + myFacets.enqueue((Facet.length, localLengthValue)) + } if (localMinLengthValue.length > 0) { myFacets.enqueue((Facet.minLength, localMinLengthValue)) } @@ -163,6 +166,10 @@ final class Restriction private (xmlArg: Node, val simpleTypeDef: SimpleTypeDefB val cPattern = lPattern.union(rPattern) cPattern.foreach(x => combined.enqueue(x)) } + if (hasLength) { + val cValue = getCombinedValue(Facet.length) + combined.enqueue((Facet.length, cValue.toString())) + } if (hasMinLength) { val cValue = getCombinedValue(Facet.minLength) combined.enqueue((Facet.minLength, cValue.toString())) diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SimpleTypes.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SimpleTypes.scala index 7541ee4958..7462251ecf 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SimpleTypes.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SimpleTypes.scala @@ -147,7 +147,7 @@ abstract class SimpleTypeDefBase(xml: Node, lexicalParent: SchemaComponent) optRestriction .map { r => if ( - r.hasPattern || r.hasEnumeration || r.hasMinLength || r.hasMaxLength || + r.hasPattern || r.hasEnumeration || r.hasLength || r.hasMinLength || r.hasMaxLength || r.hasMinInclusive || r.hasMaxInclusive || r.hasMinExclusive || r.hasMaxExclusive || r.hasTotalDigits || r.hasFractionDigits ) false diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala index f94daf1536..1aa0dcb322 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala @@ -1616,29 +1616,33 @@ trait ElementBaseGrammarMixin * the type is a type that respects minLength and maxLength, and the constant length * is not in range. */ - val isTypeUsingMinMaxLengthFacets = typeDef.typeNode match { + val isTypeUsingLengthOrMinMaxLengthFacets = typeDef.typeNode match { case s: NodeInfo.String.Kind => true case s: NodeInfo.HexBinary.Kind => true case _ => false } if ( (lengthKind eq LengthKind.Explicit) && - isTypeUsingMinMaxLengthFacets && + isTypeUsingLengthOrMinMaxLengthFacets && optLengthConstant.isDefined ) { val len = optLengthConstant.get - val maxLengthLong = maxLength.longValueExact - val minLengthLong = minLength.longValueExact + lazy val maxLengthLong = maxLength.longValueExact + lazy val minLengthLong = minLength.longValueExact + lazy val lengthLong = length.longValueExact def warn(m: String, value: Long): Unit = SDW( WarnID.FacetExplicitLengthOutOfRange, - "Explicit dfdl:length of %s is out of range for facet %sLength='%s'.", + "Explicit dfdl:length of %s is out of range for facet %sength='%s'.", len, m, value, ) - if (maxLengthLong != -1 && len > maxLengthLong) warn("max", maxLengthLong) - Assert.invariant(minLengthLong >= 0) - if (minLengthLong > 0 && len < minLengthLong) warn("min", minLengthLong) + if (hasLength && len != lengthLong) warn("l", lengthLong) + else if (hasMinLength || hasMaxLength) { + if (maxLengthLong != -1 && len > maxLengthLong) warn("maxL", maxLengthLong) + Assert.invariant(minLengthLong >= 0) + if (minLengthLong > 0 && len < minLengthLong) warn("minL", minLengthLong) + } } /* diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/SimpleTypeRuntime1Mixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/SimpleTypeRuntime1Mixin.scala index e16efdda3b..ca61e68b15 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/SimpleTypeRuntime1Mixin.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/SimpleTypeRuntime1Mixin.scala @@ -34,6 +34,7 @@ trait SimpleTypeRuntime1Mixin { self: SimpleTypeDefBase => noFacetChecks, optRestriction.toSeq.flatMap { r => if (r.hasPattern) r.patternValues else Nil }, optRestriction.flatMap { r => toOpt(r.hasEnumeration, r.enumerationValues.get) }, + optRestriction.flatMap { r => toOpt(r.hasLength, r.lengthValue) }, optRestriction.flatMap { r => toOpt(r.hasMinLength, r.minLengthValue) }, optRestriction.flatMap { r => toOpt(r.hasMaxLength, r.maxLengthValue) }, optRestriction.flatMap { r => toOpt(r.hasMinInclusive, r.minInclusiveValue) }, diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/Facets1.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/Facets1.scala index 76c889430c..15bf73f7dd 100644 --- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/Facets1.scala +++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/Facets1.scala @@ -24,6 +24,7 @@ object Facet extends Enum { sealed abstract trait Type extends EnumValueType case object enumeration extends Type case object fractionDigits extends Type + case object length extends Type case object maxExclusive extends Type case object maxInclusive extends Type case object maxLength extends Type diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/RuntimeData.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/RuntimeData.scala index 08135c1f10..fb5fd8ca40 100644 --- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/RuntimeData.scala +++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/RuntimeData.scala @@ -229,6 +229,7 @@ final class SimpleTypeRuntimeData( val noFacetChecks: Boolean, val patternValues: Seq[FacetTypes.FacetValueR], val enumerationValues: Option[String], + val length: Option[java.math.BigDecimal], val minLength: Option[java.math.BigDecimal], val maxLength: Option[java.math.BigDecimal], val minInclusive: Option[java.math.BigDecimal], @@ -345,7 +346,15 @@ final class SimpleTypeRuntimeData( return Error("facet enumeration(s): %s".format(e.enumerationValues.mkString(","))) } } - + // Check length + e.length.foreach { length => + val lenAsLong = length.longValue() + val isLengthGreaterThanEqToZero = lenAsLong.compareTo(0L) >= 0 + if (isLengthGreaterThanEqToZero) { + if (!checkLength(currentElement, length, e, primType)) + return Error("facet length (%s)".format(length)) + } + } // Check minLength e.minLength.foreach { minLength => val minAsLong = minLength.longValue() @@ -407,7 +416,32 @@ final class SimpleTypeRuntimeData( // Note: dont check occurs counts // if(!checkMinMaxOccurs(e, pstate.arrayIterationPos)) { return java.lang.Boolean.FALSE } OK } + private def checkLength( + diNode: DISimple, + lenValue: java.math.BigDecimal, + e: ThrowsSDE, + primType: PrimType, + ): java.lang.Boolean = { + val lenAsLong = lenValue.longValueExact() + primType match { + case PrimType.String => { + val data = diNode.dataValue.getString + val dataLen = data.length.toLong + val isDataLengthEqual = dataLen.compareTo(lenAsLong) == 0 + if (isDataLengthEqual) java.lang.Boolean.TRUE + else java.lang.Boolean.FALSE + } + case PrimType.HexBinary => { + val data = diNode.dataValue.getByteArray + val dataLen = data.length.toLong + val isDataLengthEqual = dataLen.compareTo(lenAsLong) == 0 + if (isDataLengthEqual) java.lang.Boolean.TRUE + else java.lang.Boolean.FALSE + } + case _ => e.SDE("Length facet is only valid for string and hexBinary.") + } + } private def checkMinLength( diNode: DISimple, minValue: java.math.BigDecimal, diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section02/validation_errors/Validation.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section02/validation_errors/Validation.tdml index 25c6c677ae..65945e4189 100644 --- a/daffodil-test/src/test/resources/org/apache/daffodil/section02/validation_errors/Validation.tdml +++ b/daffodil-test/src/test/resources/org/apache/daffodil/section02/validation_errors/Validation.tdml @@ -385,7 +385,24 @@ - + + + + + + + + + + + + + + + + + + @@ -396,7 +413,7 @@ - + @@ -407,8 +424,33 @@ - - + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -643,7 +685,63 @@ - + + + + + deadbeefdafada + + + + + DEADBEEFDAFADA + + + + + + + + + deadbeefdafada + + + + + DEADBEEFDAFADA + + + + + + + + deadbeefdafada + + + + + DEADBEEFDAFADA + + + + + + e2_8 + failed facet checks + facet length (0) + + + + + Schema Definition Error + due to length-minLength-maxLength + not have a minLength facet if the current restriction has the minLength facet + and the current restriction or base has the length facet + + + + + + + + deadbeef + + + + + Schema Definition Error + due to length-minLength-maxLength + not have a minLength facet if the current restriction has the minLength facet + and the current restriction or base has the length facet + + + + + diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section02/validation_errors/TestValidationErr.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section02/validation_errors/TestValidationErr.scala index d1fef3f67a..5c8d37fdf8 100644 --- a/daffodil-test/src/test/scala/org/apache/daffodil/section02/validation_errors/TestValidationErr.scala +++ b/daffodil-test/src/test/scala/org/apache/daffodil/section02/validation_errors/TestValidationErr.scala @@ -168,6 +168,34 @@ class TestValidationErr { @Test def test_validation_inputValueCalc_03(): Unit = { runner.runOneTest("validation_inputValueCalc_03") } + @Test def test_validation_inputValueCalc_04(): Unit = { + runner.runOneTest("validation_inputValueCalc_04") + } + @Test def test_validation_inputValueCalc_05(): Unit = { + runner.runOneTest("validation_inputValueCalc_05") + } + @Test def test_validation_inputValueCalc_06(): Unit = { + runner.runOneTest("validation_inputValueCalc_06") + } + @Test def test_validation_inputValueCalc_07(): Unit = { + runner.runOneTest("validation_inputValueCalc_07") + } + + @Test def test_validation_testFacets_01(): Unit = { + runner.runOneTest("validation_testFacets_01") + } + @Test def test_validation_testFacets_02(): Unit = { + runner.runOneTest("validation_testFacets_02") + } + @Test def test_validation_testFacets_03(): Unit = { + runner.runOneTest("validation_testFacets_03") + } + @Test def test_validation_testFacets2_01(): Unit = { + runner.runOneTest("validation_testFacets2_01") + } + @Test def test_validation_testFacets2_02(): Unit = { + runner.runOneTest("validation_testFacets2_02") + } @Test def test_floatExclusiveValid(): Unit = { runner.runOneTest("floatExclusiveValid") } @Test def test_floatExclusiveInf(): Unit = { runner.runOneTest("floatExclusiveInf") }