Skip to content

Commit

Permalink
Fix how unprefixed QNames are resolved
Browse files Browse the repository at this point in the history
Commit 2590f54 change it so that if no default namespace is defined
then an unprefixed QName reference is resolved using the targetNamespace
as its namespace. However, this logic was not correct.

Instead, we need different logic depending on if the schema containing
such a reference is included vs imported. In the former case, we should
use the targetNamespace of the including schema. In the latter, we
should just use no-namespace.

This modifies the logic to add a new 'noPrefixNamespace' member which
keeps track of what namespace to use when resolving a unprefixed QName
reference using include/import information. This member replaces
previous changes of targetNamespace.

This found a number of tests where references to defined formats were
not actually correct but happened to work due to previously flawed
logic. This corrects these tests, and adds additional tests to make sure
include/import logic works as expected.

Also Found and removed dead code:

- Grammar classes do not need to resolve QNames--the ResolvesQNames
  trait is removed from BasicCompoent which Grammar mixes in.
  ResolvesQNames is now only mixed into DSOM-level components. This
  allows removing members related to resolving QNames
- Nothing uses resolveExtendedSyntaxRef so it is removed. Instead one
  should use QName.refQNameFromExtendedSyntax

Deprecation/Compatibility:

A bug was fixed so Daffodil now correctly handles resolving unprefixed
QName references. However, this means that some unprefixed references
that relied on the previous incorrect behavior may not resolve anymore.
Although there are a number of ways to fix this, defining a namespace
prefix and adding that prefix to broken reference should work in all
cases.

DAFFODIL-2916
  • Loading branch information
stevedlawrence committed Sep 24, 2024
1 parent e467498 commit e02ae7f
Show file tree
Hide file tree
Showing 47 changed files with 389 additions and 173 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class DFDLPathExpressionParser[T <: AnyRef](
qn: NamedQName,
nodeInfoKind: NodeInfo.Kind,
namespaces: NamespaceBinding,
targetNamespace: NS,
noPrefixNamespace: NS,
context: DPathCompileInfo,
isEvaluatedAbove: Boolean,
host: BasicComponent
Expand Down Expand Up @@ -231,7 +231,7 @@ class DFDLPathExpressionParser[T <: AnyRef](
def Comp = EqualityComp | NumberComp

def TopLevel: Parser[WholeExpression] = ("{" ~> Expr <~ "}") ^^ { xpr =>
WholeExpression(nodeInfoKind, xpr, namespaces, targetNamespace, context, host)
WholeExpression(nodeInfoKind, xpr, namespaces, noPrefixNamespace, context, host)
}

val SuccessAtEnd = Parser { in => Success(in, new CharSequenceReader("")) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import scala.util.{ Failure, Success }
import scala.xml.NamespaceBinding

import org.apache.daffodil.lib.api.DaffodilTunables
import org.apache.daffodil.lib.api.UnqualifiedPathStepPolicy
import org.apache.daffodil.lib.api.WarnID
import org.apache.daffodil.lib.equality._
import org.apache.daffodil.lib.exceptions._
Expand Down Expand Up @@ -52,8 +51,6 @@ import org.apache.daffodil.runtime1.udf.UserDefinedFunctionService
abstract class Expression extends OOLAGHostImpl() with BasicComponent {

override lazy val tunable: DaffodilTunables = parent.tunable
override lazy val unqualifiedPathStepPolicy: UnqualifiedPathStepPolicy =
parent.unqualifiedPathStepPolicy
override lazy val localSuppressSchemaDefinitionWarnings: Seq[WarnID] =
parent.localSuppressSchemaDefinitionWarnings

Expand Down Expand Up @@ -153,7 +150,7 @@ abstract class Expression extends OOLAGHostImpl() with BasicComponent {

lazy val namespaces: NamespaceBinding = parent.namespaces

lazy val targetNamespace: NS = parent.targetNamespace
lazy val noPrefixNamespace: NS = parent.noPrefixNamespace

def children: Seq[Expression]

Expand Down Expand Up @@ -221,7 +218,7 @@ abstract class Expression extends OOLAGHostImpl() with BasicComponent {

def resolveRef(qnameString: String): RefQName = {
QName
.resolveRef(qnameString, namespaces, targetNamespace, tunable.unqualifiedPathStepPolicy)
.resolveRef(qnameString, namespaces, noPrefixNamespace, tunable.unqualifiedPathStepPolicy)
.recover { case _: Throwable =>
SDE("The prefix of '%s' has no corresponding namespace definition.", qnameString)
}
Expand Down Expand Up @@ -460,14 +457,12 @@ case class WholeExpression(
nodeInfoKind: NodeInfo.Kind,
ifor: Expression,
nsBindingForPrefixResolution: NamespaceBinding,
targetNamespaceArg: NS,
noPrefixNamespaceArg: NS,
ci: DPathCompileInfo,
host: BasicComponent
) extends Expression {

final override lazy val tunable: DaffodilTunables = host.tunable
final override lazy val unqualifiedPathStepPolicy: UnqualifiedPathStepPolicy =
host.unqualifiedPathStepPolicy
final override lazy val localSuppressSchemaDefinitionWarnings: Seq[WarnID] =
host.localSuppressSchemaDefinitionWarnings

Expand All @@ -480,7 +475,7 @@ case class WholeExpression(

override lazy val namespaces = nsBindingForPrefixResolution

override lazy val targetNamespace = targetNamespaceArg
override lazy val noPrefixNamespace = noPrefixNamespaceArg

override def text = ifor.text

Expand Down Expand Up @@ -835,7 +830,7 @@ sealed abstract class StepExpression(val step: String, val pred: Option[Predicat
val e = QName.resolveStep(
step,
namespaces,
targetNamespace,
noPrefixNamespace,
tunable.unqualifiedPathStepPolicy
)
e match {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
nodeInfoKind: NodeInfo.Kind,
exprOrLiteral: String,
namespaces: NamespaceBinding,
targetNamespace: NS,
noPrefixNamespace: NS,
compileInfoWhereExpressionWasLocated: DPathCompileInfo,
isEvaluatedAbove: Boolean,
host: BasicComponent,
Expand All @@ -78,7 +78,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
nodeInfoKind,
exprOrLiteral,
namespaces,
targetNamespace,
noPrefixNamespace,
compileInfoWhereExpressionWasLocated,
isEvaluatedAbove,
host,
Expand All @@ -90,7 +90,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
nodeInfoKind,
exprOrLiteral,
namespaces,
targetNamespace,
noPrefixNamespace,
compileInfoWhereExpressionWasLocated,
isEvaluatedAbove
)
Expand Down Expand Up @@ -126,7 +126,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
nodeInfoKind,
property.value,
property.location.namespaces,
property.location.targetNamespace,
property.location.noPrefixNamespace,
compileInfo,
isEvaluatedAbove,
host,
Expand Down Expand Up @@ -171,7 +171,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
staticNodeInfoKind,
exprOrLiteral,
namespacesForNamespaceResolution,
property.location.targetNamespace,
property.location.noPrefixNamespace,
compileInfoWhereExpressionWasLocated,
isEvaluatedAbove,
host,
Expand All @@ -186,7 +186,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
runtimeNodeInfoKind,
exprOrLiteral,
namespacesForNamespaceResolution,
property.location.targetNamespace,
property.location.noPrefixNamespace,
compileInfoWhereExpressionWasLocated,
isEvaluatedAbove,
host,
Expand Down Expand Up @@ -219,7 +219,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
nodeInfoKind: NodeInfo.Kind,
exprOrLiteral: String,
namespaces: NamespaceBinding,
targetNamespace: NS,
noPrefixNamespace: NS,
compileInfoWhereExpressionWasLocated: DPathCompileInfo,
isEvaluatedAbove: Boolean,
host: BasicComponent,
Expand All @@ -239,7 +239,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
qn,
nodeInfoKind,
namespaces,
targetNamespace,
noPrefixNamespace,
compileInfo,
isEvaluatedAbove,
host
Expand All @@ -253,7 +253,7 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
nodeInfoKind: NodeInfo.Kind,
exprOrLiteral: String,
namespaces: NamespaceBinding,
targetNamespace: NS,
noPrefixNamespace: NS,
compileInfoWhereExpressionWasLocated: DPathCompileInfo,
isEvaluatedAbove: Boolean
): CompiledExpression[T] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class DFDLDefineVariable private (node: Node, doc: SchemaDocument)
val eQN = QName.resolveRef(
typeQNameString,
namespaces,
targetNamespace,
noPrefixNamespace,
tunable.unqualifiedPathStepPolicy
)
val res = eQN.recover { case _: Throwable =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ final class Restriction private (xmlArg: Node, val simpleTypeDef: SimpleTypeDefB
QName.resolveRef(
baseQNameString,
xml.scope,
targetNamespace,
noPrefixNamespace,
tunable.unqualifiedPathStepPolicy
)
schemaDefinitionUnless(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ trait SchemaComponent
Delay('nonElementParents, this, parents),
variableMap,
namespaces,
targetNamespace,
noPrefixNamespace,
path,
schemaFileLocation,
tunable.unqualifiedPathStepPolicy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ trait CommonContextMixin extends NestingLexicalMixin with CommonContextView {
*/
def targetNamespace: NS = xmlSchemaDocument.targetNamespace

/**
* The namespace to use to resolve references without a prefix
*/
def noPrefixNamespace: NS = xmlSchemaDocument.noPrefixNamespace

final lazy val targetNamespacePrefix = xml.scope.getPrefix(targetNamespace.toString)

}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,40 @@ trait SchemaDocIncludesAndImportsMixin { self: XMLSchemaDocument =>
resultNS
}.value

/**
* when we resolve a QName reference, if that reference does not have a prefix and there is no
* in-scope default namespace, then we use this namespace, which varies depending on things
* like targetNamespace and whether this schema was included or imported
*/
override lazy val noPrefixNamespace: NS = LV('noPrefixNamespace) {
ii match {
case Some(inc: Include) => {
// if this schema document was included in another document, then either the two
// schemas already have the same targetNamespace or this schema has no-namespace and
// is chameleoned into the targetNamespace of the including schema. Either way, the
// resulting included elements are in the targetNamespace and so any unprefixed
// references to those elements should use the including schemas targetNamespace when
// resolving
inc.targetNamespace
}
case Some(imp: Import) => {
// if this schema document was imported and we don't have a default namespace then any
// unprefixed references just resolve to NoNamespace. Note that if this schema has a
// targetNamespace, then it can only reference elements that are imported into it
// without a namespace
NoNamespace
}
case _ => {
// this is either a Some() that isn't an Include/Import, or this is the bootstrap schema
// and we shouldn't be asking for its noPrefixNamespace. Both cases should be
// impossible.
// $COVERAGE-OFF$
Assert.impossible()
// $COVERAGE-ON$
}
}
}.value

// There is one distinguished top level SchemaDocument
// that we use to start the ball rolling by importing all the
// files that the user supplies via the API/command line.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1485,14 +1485,14 @@ trait ElementBaseGrammarMixin
val exprProp = outputValueCalcOption.asInstanceOf[Found]
val exprText = exprProp.value
val exprNamespaces = exprProp.location.namespaces
val exprTargetNamespace = exprProp.location.targetNamespace
val exprNoPrefixNamespace = exprProp.location.noPrefixNamespace
val qn = GlobalQName(Some("daf"), "outputValueCalc", XMLUtils.dafintURI)
val expr = ExpressionCompilers.AnyRef.compileExpression(
qn,
primType,
exprText,
exprNamespaces,
exprTargetNamespace,
exprNoPrefixNamespace,
dpathCompileInfo,
isEvaluatedAbove = false,
self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ abstract class Gram(contextArg: SchemaComponent)

final override lazy val tunable = context.tunable

final override def namespaces = context.namespaces
final override def targetNamespace = context.targetNamespace
final override def unqualifiedPathStepPolicy = context.unqualifiedPathStepPolicy
final override def schemaFileLocation = context.schemaFileLocation
final override def localSuppressSchemaDefinitionWarnings =
context.localSuppressSchemaDefinitionWarnings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ abstract class AssertBase(
decl: AnnotatedSchemaComponent,
exprWithBraces: String,
namespacesForNamespaceResolution: scala.xml.NamespaceBinding,
targetNamespaceArg: NS,
noPrefixNamespaceArg: NS,
scWherePropertyWasLocated: AnnotatedSchemaComponent,
msgOpt: Option[String],
discrim: Boolean, // are we a discriminator or not.
Expand All @@ -76,7 +76,7 @@ abstract class AssertBase(
decl,
foundProp.value,
foundProp.location.namespaces,
foundProp.location.targetNamespace,
foundProp.location.noPrefixNamespace,
decl,
msgOpt,
discrim,
Expand All @@ -87,7 +87,7 @@ abstract class AssertBase(
override val baseName = assertKindName
override lazy val exprText = exprWithBraces
override lazy val exprNamespaces = namespacesForNamespaceResolution
override lazy val exprTargetNamespace = targetNamespaceArg
override lazy val exprNoPrefixNamespace = noPrefixNamespaceArg
override lazy val exprComponent = scWherePropertyWasLocated
override def nodeKind = NodeInfo.Boolean

Expand All @@ -100,7 +100,7 @@ abstract class AssertBase(
NodeInfo.String,
msgOpt.get,
exprNamespaces,
exprTargetNamespace,
exprNoPrefixNamespace,
exprComponent.dpathCompileInfo,
false,
this,
Expand Down Expand Up @@ -202,7 +202,7 @@ case class SetVariable(stmt: DFDLSetVariable, override val term: Term)

override lazy val exprText = stmt.value
override lazy val exprNamespaces = stmt.xml.scope
override lazy val exprTargetNamespace = stmt.annotatedSC.targetNamespace
override lazy val exprNoPrefixNamespace = stmt.annotatedSC.noPrefixNamespace
override lazy val exprComponent = stmt

override lazy val nodeKind = stmt.defv.primType
Expand Down Expand Up @@ -287,7 +287,7 @@ abstract class ExpressionEvaluatorBase(e: AnnotatedSchemaComponent) extends Term

def baseName: String
def exprNamespaces: scala.xml.NamespaceBinding
def exprTargetNamespace: NS
def exprNoPrefixNamespace: NS
def exprComponent: SchemaComponent
def exprText: String

Expand All @@ -301,7 +301,7 @@ abstract class ExpressionEvaluatorBase(e: AnnotatedSchemaComponent) extends Term
nodeKind,
exprText,
exprNamespaces,
exprTargetNamespace,
exprNoPrefixNamespace,
exprComponent.dpathCompileInfo,
false,
this,
Expand All @@ -315,7 +315,7 @@ abstract class ValueCalcBase(e: ElementBase, property: PropertyLookupResult)

override lazy val exprText = exprProp.value
override lazy val exprNamespaces = exprProp.location.namespaces
override lazy val exprTargetNamespace = exprProp.location.targetNamespace
override lazy val exprNoPrefixNamespace = exprProp.location.noPrefixNamespace
override lazy val exprComponent = exprProp.location.asInstanceOf[SchemaComponent]

lazy val pt = e.primType // .typeRuntimeData
Expand Down Expand Up @@ -344,7 +344,7 @@ abstract class AssertPatternPrimBase(decl: Term, stmt: DFDLAssertionBase, discri
override val baseName = if (discrim) "Discriminator" else "Assert"
override lazy val exprText = stmt.messageAttrib.get
override lazy val exprNamespaces = decl.namespaces
override lazy val exprTargetNamespace = decl.targetNamespace
override lazy val exprNoPrefixNamespace = decl.noPrefixNamespace
override lazy val exprComponent = decl

override def nodeKind = NodeInfo.String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ trait ElementBaseRuntime1Mixin { self: ElementBase =>
variableMap,
Delay('elementChildrenCompileInfo, this, elementChildrenCompileInfo),
namespaces,
targetNamespace,
noPrefixNamespace,
slashPath,
name,
isArray,
Expand Down Expand Up @@ -191,6 +191,7 @@ trait ElementBaseRuntime1Mixin { self: ElementBase =>
minimizedScope,
defaultBitOrder,
optPrimType,
targetNamespace,
optSimpleTypeRuntimeData,
optComplexTypeModelGroupRuntimeData,
minOccurs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ trait SchemaComponentRuntime1Mixin { self: SchemaComponent =>
diagnosticDebugName,
path,
namespaces,
targetNamespace,
noPrefixNamespace,
tunable.unqualifiedPathStepPolicy
)
}.value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ trait SimpleTypeRuntime1Mixin { self: SimpleTypeDefBase =>
diagnosticDebugName,
path,
namespaces,
targetNamespace,
noPrefixNamespace,
primType,
noFacetChecks,
optRestriction.toSeq.flatMap { r => if (r.hasPattern) r.patternValues else Nil },
Expand Down
Loading

0 comments on commit e02ae7f

Please sign in to comment.