You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "mbeckerle (via GitHub)" <gi...@apache.org> on 2023/09/13 16:25:03 UTC
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1085: Improve and narrow dfdlx:repType extension implementaion
mbeckerle commented on code in PR #1085:
URL: https://github.com/apache/daffodil/pull/1085#discussion_r1324745343
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ElementBase.scala:
##########
@@ -985,6 +985,7 @@ trait ElementBase
"Facets minLength and maxLength are allowed only on types string and hexBinary.",
)
typeDef match {
+ case _ if hasRepType => (repTypeElementDecl.minLength, repTypeElementDecl.maxLength)
Review Comment:
With this change, it's clearly computing the min/max length of the representation.
Add that to the scaladoc.
But is this correct? Is this only used when we need the max/min length of the representation?
What about limited validation? Then we're dealing with the infoset value, and the max/min length are those of the logical data, not the rep.
Perhaps we need to distinguish infosetMinLength and repMinLength (similarly for max) ?
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/cookers/Cookers.scala:
##########
@@ -101,8 +101,6 @@ object ChoiceDispatchKeyCooker extends StringLiteralNonEmptyNoCharClassEntitiesN
object ChoiceBranchKeyCooker
extends ListOfStringLiteralNonEmptyNoCharClassEntitiesNoByteEntities()
-object RepValueCooker extends ListOfStringLiteralNonEmptyNoCharClassEntitiesNoByteEntities()
Review Comment:
If this is deleted, is that because repValues and range end points can only be integers?
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/RepTypeMixin.scala:
##########
@@ -0,0 +1,213 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil.core.grammar
+
+import scala.xml.Elem
+import scala.xml.Null
+import scala.xml.UnprefixedAttribute
+
+import org.apache.daffodil.core.dsom.ElementBase
+import org.apache.daffodil.core.dsom.GlobalSimpleTypeDef
+import org.apache.daffodil.core.dsom.RepTypeQuasiElementDecl
+import org.apache.daffodil.lib.exceptions.Assert
+import org.apache.daffodil.runtime1.dpath.LE_Byte
+import org.apache.daffodil.runtime1.dpath.LE_Int
+import org.apache.daffodil.runtime1.dpath.LE_Integer
+import org.apache.daffodil.runtime1.dpath.LE_Long
+import org.apache.daffodil.runtime1.dpath.LE_NonNegativeInteger
+import org.apache.daffodil.runtime1.dpath.LE_Short
+import org.apache.daffodil.runtime1.dpath.LE_UnsignedByte
+import org.apache.daffodil.runtime1.dpath.LE_UnsignedInt
+import org.apache.daffodil.runtime1.dpath.LE_UnsignedLong
+import org.apache.daffodil.runtime1.dpath.LE_UnsignedShort
+import org.apache.daffodil.runtime1.dpath.LT_Byte
+import org.apache.daffodil.runtime1.dpath.LT_Int
+import org.apache.daffodil.runtime1.dpath.LT_Integer
+import org.apache.daffodil.runtime1.dpath.LT_Long
+import org.apache.daffodil.runtime1.dpath.LT_NonNegativeInteger
+import org.apache.daffodil.runtime1.dpath.LT_Short
+import org.apache.daffodil.runtime1.dpath.LT_UnsignedByte
+import org.apache.daffodil.runtime1.dpath.LT_UnsignedInt
+import org.apache.daffodil.runtime1.dpath.LT_UnsignedLong
+import org.apache.daffodil.runtime1.dpath.LT_UnsignedShort
+import org.apache.daffodil.runtime1.dpath.NodeInfo
+import org.apache.daffodil.runtime1.dpath.NumberCompareOp
+import org.apache.daffodil.runtime1.infoset.DataValue.DataValueNumber
+import org.apache.daffodil.runtime1.infoset.DataValue.DataValueString
+
+trait RepTypeMixin { self: ElementBase =>
+
+ lazy val hasRepType: Boolean = LV('hasRepType) {
+ isSimpleType && findPropertyOption("repType").isDefined
+ }.value
+
+ private lazy val repTypeGSTD: GlobalSimpleTypeDef = LV('prefixLengthTypeGSTD) {
+ // throws an SDE if the simple type def is not found or if it is not a simple type (e.g. a
+ // primitive type)
+ val gstd = schemaSet.getGlobalSimpleTypeDefNoPrim(repType, "dfdlx:repType", this)
+ schemaDefinitionUnless(
+ gstd.primType.isInstanceOf[NodeInfo.Integer.Kind],
+ "dfdlx:repType (%s) must resolve to a global simple type definition derived from xs:integer, but was %s",
+ repType,
+ gstd.primType.globalQName,
+ )
+ gstd
+ }.value
+
+ lazy val repTypeElementDecl: RepTypeQuasiElementDecl = LV('repTypeElementDecl) {
+ val xmlElem = Elem(
+ null,
+ "QuasiElementForRepType",
+ new UnprefixedAttribute("type", repType.toString, Null),
+ namespaces,
+ true,
+ )
+ RepTypeQuasiElementDecl(xmlElem, repTypeGSTD)
+ }.value
+
+ lazy val optRepTypeElementDecl: Option[RepTypeQuasiElementDecl] = LV('optRepTypeElementDecl) {
+ if (hasRepType) Some(repTypeElementDecl) else None
+ }.value
+
+ lazy val (
+ repTypeCompareLT: NumberCompareOp,
+ repTypeCompareLE: NumberCompareOp,
+ ) = LV('repTypeComparers) {
+ repTypeElementDecl.primType match {
+ case NodeInfo.Integer => (LT_Integer, LE_Integer)
+ case NodeInfo.Long => (LT_Long, LE_Long)
+ case NodeInfo.Int => (LT_Int, LE_Int)
+ case NodeInfo.Short => (LT_Short, LE_Short)
+ case NodeInfo.Byte => (LT_Byte, LE_Byte)
+ case NodeInfo.NonNegativeInteger => (LT_NonNegativeInteger, LE_NonNegativeInteger)
+ case NodeInfo.UnsignedLong => (LT_UnsignedLong, LE_UnsignedLong)
+ case NodeInfo.UnsignedInt => (LT_UnsignedInt, LE_UnsignedInt)
+ case NodeInfo.UnsignedShort => (LT_UnsignedShort, LE_UnsignedShort)
+ case NodeInfo.UnsignedByte => (LT_UnsignedByte, LE_UnsignedByte)
+ // $COVERAGE-OFF$
+ case _ => Assert.invariantFailed("repType should only be an integer type")
+ // $COVERAGE-ON$
+ }
+ }.value
+
+ lazy val (
+ repTypeParseValuesMap: Map[DataValueNumber, DataValueString],
+ repTypeParseRangesMap: Seq[(DataValueNumber, DataValueNumber, DataValueString)],
+ repTypeUnparseMap: Map[DataValueString, DataValueNumber],
+ ) = LV('repTypeMaps) {
+
+ schemaDefinitionUnless(
+ primType.isInstanceOf[NodeInfo.String.Kind],
+ "Type must derive from xs:string when dfdlx:repType (%s) is defined: %s",
+ repType,
+ primType.globalQName,
+ )
+
+ schemaDefinitionWhen(
+ typeDef.optRestriction.isEmpty,
+ "A restriction is required when dfdlx:repType (%s) is defined",
+ repType,
+ )
+
+ val restriction = typeDef.optRestriction.get
+
+ schemaDefinitionWhen(
+ restriction.enumerations.isEmpty,
+ "There must be at least one enumeration defined when dfdlx:repType (%s) is defined",
+ repType,
+ )
+
+ val lt = repTypeCompareLT
+
+ // This is Seq of attributes extracted from each enumeration, where each item in the
+ // sequence is a 3-tuple containing the cooked repValues, repValueRanges, and enum value
+ // from the associated enum
+ val enumAttributes = restriction.enumerations.map { enum =>
+ enum.schemaDefinitionWhen(
+ enum.repValuesRaw.isEmpty && enum.repValueRangesRaw.isEmpty,
+ "All enumerations must define dfdlx:repValues and/or dfdlx:repValueRanges when dfdlx:repType (%s) is defined",
+ repType,
+ )
+
+ val repValues: Seq[DataValueNumber] =
+ enum.repValuesRaw
+ .map(repTypeElementDecl.primType.fromXMLString(_).asInstanceOf[DataValueNumber])
+ val repValueRanges: Seq[(DataValueNumber, DataValueNumber)] =
+ enum.repValueRangesRaw
+ .map(repTypeElementDecl.primType.fromXMLString(_).asInstanceOf[DataValueNumber])
+ .sliding(2, 2)
+ .map { case Seq(low, high) =>
+ enum.schemaDefinitionUnless(
+ lt.operate(low, high).getBoolean,
+ "dfdlx:repValueRanges low value (%s) must be less than high value (%s)",
+ low.value,
+ high.value,
+ )
+ (low, high)
+ }
+ .toSeq
+ val enumValue = enum.enumValueCooked.asInstanceOf[DataValueString]
+ (repValues, repValueRanges, enumValue)
+ }
+
+ // combine all rep values and ranges into a single seq of low/high tuples, sort them by
+ // their low value, and look for overlaps
+ enumAttributes
+ .flatMap { case (repValues, repRanges, _) => repValues.map(v => (v, v)) ++ repRanges }
+ .sortWith { case ((l1, _), (l2, _)) => lt.operate(l1, l2).getBoolean }
+ .reduce[(DataValueNumber, DataValueNumber)] { case ((l1, h1), (l2, h2)) =>
+ restriction.schemaDefinitionUnless(
+ lt.operate(h1, l2).getBoolean,
+ "Overlapping dfdlx:%s (%s) and dfdlx:%s (%s) found",
+ if (lt.operate(l1, h1).getBoolean) "repValueRanges" else "repValues",
+ if (lt.operate(l1, h1).getBoolean) l1.value + " " + h1.value else l1.value,
+ if (lt.operate(l2, h2).getBoolean) "repValueRanges" else "repValues",
+ if (lt.operate(l2, h2).getBoolean) l2.value + " " + h2.value else l2.value,
+ )
+ (l2, h2)
+ }
+
+ // generate the data structures for fast mapping of rep values to/from logical values
+ val parseValuesMap: Map[DataValueNumber, DataValueString] = enumAttributes
+ .filterNot(_._1.isEmpty)
+ .flatMap { case (repValues, _, enumValue) =>
+ repValues.map { repValue => (repValue, enumValue) }
+ }
+ .toMap
+ val parseRangesMap: Seq[(DataValueNumber, DataValueNumber, DataValueString)] =
+ enumAttributes
+ .filterNot(_._2.isEmpty)
+ .flatMap { case (_, repRanges, enumValue) =>
+ repRanges.map { case (low, high) => (low, high, enumValue) }
+ }
+ val unparseMap: Map[DataValueString, DataValueNumber] = enumAttributes.map {
+ case (repValues, repRanges, enumValue) =>
+ val canonicalRep = None
Review Comment:
None.orElse(x.headOption).orElse(...)
I think can be simplified to
x.headOption.orElse(...)
##########
daffodil-test/src/test/resources/org/apache/daffodil/extensions/repType/repType.tdml:
##########
@@ -0,0 +1,708 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<tdml:testSuite xmlns:ex="http://example.com" xmlns="http://example.com"
+ xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/" xmlns:xs="http://www.w3.org/2001/XMLSchema"
+ xmlns:dfdlx="http://www.ogf.org/dfdl/dfdl-1.0/extensions"
+ xmlns:fn="http://www.w3.org/2005/xpath-functions"
+ xmlns:tns="http://example.com"
+ >
+
+ <tdml:defineSchema name="repType-Embedded.dfdl.xsd">
+
+ <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <dfdl:format ref="ex:GeneralFormat" lengthKind="delimited"
+ lengthUnits="bytes" encoding="UTF-8" separator="" initiator=""
+ terminator="" occursCountKind="parsed" ignoreCase="no"
+ textNumberRep="standard" representation="binary" />
+
+ <xs:element name="keysetValue_00" type="tns:keysetValue_01"/>
+
+ <xs:element name="keysetValue_01">
+ <xs:complexType>
+ <xs:sequence>
+ <xs:element name="byte" type="tns:keysetValue_01"/>
+ </xs:sequence>
+ </xs:complexType>
+ </xs:element>
+
+ <xs:element name="keysetValue_02">
+ <xs:complexType>
+ <xs:sequence>
+ <xs:element name="byte" type="tns:keysetValue_01" maxOccurs="unbounded" dfdl:occursCountKind="parsed"/>
+ </xs:sequence>
+ </xs:complexType>
+ </xs:element>
+
+ <xs:simpleType name="uint8" dfdl:lengthKind="explicit" dfdl:length="1">
+ <xs:restriction base="xs:unsignedInt"/>
+ </xs:simpleType>
+
+ <xs:simpleType name="keysetValue_01" dfdlx:repType="tns:uint8">
+ <xs:restriction base="xs:string">
+ <xs:enumeration value="zero" dfdlx:repValues="0" />
+ <xs:enumeration value="one" dfdlx:repValues="1" />
+ <xs:enumeration value="2-100" dfdlx:repValueRanges="2 100" />
+ <xs:enumeration value="101 103-110 115 120-125" dfdlx:repValues="101 115" dfdlx:repValueRanges="103 110 120 125"/>
Review Comment:
Nice test case. I would not have thought to put both repValues and ranges on one enum.
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/cookers/Cookers.scala:
##########
@@ -101,8 +101,6 @@ object ChoiceDispatchKeyCooker extends StringLiteralNonEmptyNoCharClassEntitiesN
object ChoiceBranchKeyCooker
extends ListOfStringLiteralNonEmptyNoCharClassEntitiesNoByteEntities()
-object RepValueCooker extends ListOfStringLiteralNonEmptyNoCharClassEntitiesNoByteEntities()
Review Comment:
Follow up. I've never seen it, but are our repValues integers nonNegative only, or can things like -1 -2, etc. be enum repValues?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org