Skip to content

Add support for validation of length facet #1192

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.

Sign up for GitHub

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

Merged
olabusayoT merged 1 commit into from
Mar 27, 2024
Merged

Add support for validation of length facet #1192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ElementBase.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ trait ElementBase
requiredEvaluationsIfActivated(isSimpleType)
requiredEvaluationsIfActivated(if (hasPattern) patternValues)
requiredEvaluationsIfActivated(if (hasEnumeration) enumerationValues)
requiredEvaluationsIfActivated(if (hasMinLength) minLength)
requiredEvaluationsIfActivated(if (hasMaxLength) maxLength)
requiredEvaluationsIfActivated(if (hasMinLength || hasLength) minLength)
requiredEvaluationsIfActivated(if (hasMaxLength || hasLength) maxLength)
requiredEvaluationsIfActivated(if (hasMinInclusive) minInclusive)
requiredEvaluationsIfActivated(if (hasMaxInclusive) maxInclusive)
requiredEvaluationsIfActivated(if (hasMinExclusive) minExclusive)
Expand Down Expand Up @@ -711,8 +711,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 a length or maxLength facet value.",
)
val ml = repElement.maxLength
ml.longValue()
Expand Down Expand Up @@ -941,8 +941,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)
Expand All @@ -963,18 +964,19 @@ trait ElementBase
/**
* 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
// TODO: why are we using java.math.BigDecimal, when scala has a much
// nicer decimal class?

private val zeroBD = new java.math.BigDecimal(0)
private val unbBD = new java.math.BigDecimal(-1) // TODO: should this be a tunable limit?

private def computeMinMaxLength: (java.math.BigDecimal, java.math.BigDecimal) = {
schemaDefinitionUnless(
isSimpleType,
"Facets minLength and maxLength are allowed only on types string and hexBinary.",
"The length facet or minLength/maxLength facets are not allowed on complex types",
)
typeDef match {
case _ if hasRepType => {
Expand All @@ -986,7 +988,7 @@ trait ElementBase
val pt = prim.primType
schemaDefinitionWhen(
(pt == PrimType.String || pt == PrimType.HexBinary) && lengthKind == LengthKind.Implicit,
"Facets minLength and maxLength must be defined for type %s with lengthKind='implicit'",
"The length facet or minLength/maxLength facets must be defined for type %s with lengthKind='implicit'",
pt.name,
)
//
Expand All @@ -1001,12 +1003,17 @@ trait ElementBase
val pt = st.primType
val typeOK = pt == PrimType.String || pt == PrimType.HexBinary
schemaDefinitionWhen(
!typeOK && (hasMinLength || hasMaxLength),
"Facets minLength and maxLength are not allowed on types derived from type %s.\nThey are allowed only on typed derived from string and hexBinary.",
!typeOK && (hasLength || hasMinLength || hasMaxLength),
"The length facet or minLength/maxLength facets are not allowed on types derived from type %s.\nThey are allowed only on types derived from string and hexBinary.",
pt.name,
)
val res = (hasMinLength, hasMaxLength, lengthKind) match {
case (true, true, LengthKind.Implicit) => {
val res = (hasLength, hasMinLength, hasMaxLength, lengthKind) match {
case (true, false, false, _) => (r.lengthValue, r.lengthValue)
case (true, _, _, _) =>
Assert.invariantFailed(
"Facet length cannot be defined with minLength and maxLength facets",
)
case (false, true, true, LengthKind.Implicit) => {
schemaDefinitionUnless(
r.minLengthValue.compareTo(r.maxLengthValue) == 0,
"The minLength and maxLength must be equal for type %s with lengthKind='implicit'. Values were minLength of %s, maxLength of %s.",
Expand All @@ -1016,7 +1023,7 @@ trait ElementBase
)
(r.minLengthValue, r.maxLengthValue)
}
case (true, true, _) => {
case (false, true, true, _) => {
schemaDefinitionWhen(
r.minLengthValue.compareTo(r.maxLengthValue) > 0,
// always true, so we don't bother to specify the type in the message.
Expand All @@ -1026,13 +1033,13 @@ trait ElementBase
)
(r.minLengthValue, r.maxLengthValue)
}
case (_, _, LengthKind.Implicit) =>
case (false, _, _, LengthKind.Implicit) =>
SDE(
"When lengthKind='implicit', both minLength and maxLength facets must be specified.",
)
case (false, true, _) => (zeroBD, r.maxLengthValue)
case (false, false, _) => (zeroBD, unbBD)
case (true, false, _) => (r.minLengthValue, unbBD)
case (false, false, true, _) => (zeroBD, r.maxLengthValue)
case (false, false, false, _) => (zeroBD, unbBD)
case (false, true, false, _) => (r.minLengthValue, unbBD)
case _ => Assert.impossible()
}
res
Expand Down
65 changes: 48 additions & 17 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/Facets.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
"Facets 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,
"Facets 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 = {
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand All @@ -247,12 +273,6 @@ trait Facets { self: Restriction =>
final lazy val fractionDigitsValue: java.math.BigDecimal =
getFacetValue(localFractionDigitsValue, Facet.fractionDigits, hasFractionDigits)

// private def errorOnLocalLessThanBaseFacet(local: Long, base: Long, theFacetType: Facet.Type) = {
// if (local < base) SDE("SimpleTypes: The local %s (%s) was less than the base %s (%s) ", theFacetType, local, theFacetType, base)
// }
// private def errorOnLocalGreaterThanBaseFacet(local: Long, base: Long, theFacetType: Facet.Type) = {
// if (local > base) SDE("SimpleTypes: The local %s (%s) was greater than the base %s (%s) ", theFacetType, local, theFacetType, base)
// }
private def errorOnLocalLessThanBaseFacet(
local: BigInteger,
base: BigInteger,
Expand Down Expand Up @@ -313,14 +333,21 @@ trait Facets { self: Restriction =>
base,
)
}

// private def getRemoteFacets(theFacetType: Facet.Type): Seq[FacetValueR] = {
// val remoteValues = remoteBaseFacets.filter { case (f, _) => f == theFacetType }
// if (remoteValues.size > 0) {
// val res: Seq[FacetValueR] = remoteValues.map { case (f, v) => (f, v.r) }
// res
// } else Seq.empty
// }
private def errorOnLocalNotEqualToBaseFacet(
local: BigInteger,
base: BigInteger,
theFacetType: Facet.Type,
) = {
val res = local.compareTo(base)
if (res != 0)
SDE(
"SimpleTypes: The local %s (%s) was not equal to the base %s (%s) ",
theFacetType,
local,
theFacetType,
base,
)
}

private def getRemoteFacetValues(theFacetType: Facet.Type): Seq[FacetValue] = {
val res = remoteBaseFacets.filter { case (f, _) => f == theFacetType }
Expand Down Expand Up @@ -387,6 +414,10 @@ trait Facets { self: Restriction =>
errorOnLocalLessThanBaseFacet(theLocalFacet, theRemoteFacet, facetType)
localFacet
}
case Facet.length => {
errorOnLocalNotEqualToBaseFacet(theLocalFacet, theRemoteFacet, facetType)
localFacet
}
case Facet.maxLength | Facet.fractionDigits => {
errorOnLocalGreaterThanBaseFacet(theLocalFacet, theRemoteFacet, facetType)
Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that a local length facet must be less than the base length facet, is that correct? Have we confirmed with the XSD spec that that is the right behavior? Feels like make a local length should always be the same as a base length, since there is no way to narrow a single value instead of a range like min/maxValue have.

Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found this in the XSD spec:

Schema Component Constraint: length valid restriction
It is an ·error· if length is among the members of {facets} of {base type definition} and {value} is not equal to the {value} of the parent length.

So I think this is wrong. This needs to error if local is not equal to base.

localFacet
Expand Down Expand Up @@ -679,7 +710,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)
}
Expand Down
7 changes: 7 additions & 0 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/RestrictionUnion.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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()))
Expand Down
2 changes: 1 addition & 1 deletion daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SimpleTypes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 12 additions & 8 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 %s='%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 != minLengthLong && len != maxLengthLong)
warn("length", minLengthLong)
else if (hasMinLength || hasMaxLength) {
if (maxLengthLong != -1 && len > maxLengthLong) warn("maxLength", maxLengthLong)
Assert.invariant(minLengthLong >= 0)
if (minLengthLong > 0 && len < minLengthLong) warn("minLength", minLengthLong)
}
}

/*
Expand Down
1 change: 1 addition & 0 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/SimpleTypeRuntime1Mixin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) },
Expand Down
1 change: 1 addition & 0 deletions daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/Facets1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading