stevedlawrence commented on a change in pull request #373:
URL: https://github.com/apache/incubator-daffodil/pull/373#discussion_r416541781
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -1510,6 +1510,14 @@ case class FunctionCallExpression(functionQNameString:
String, expressions: List
FNOneArgExpr(functionQNameString, functionQName, args,
NodeInfo.String, NodeInfo.AnyAtomic, FNLocalName1(_, _))
+ case (RefQName(_, "namespace-uri", FUNC), args) if args.length == 0 =>
+ FNZeroArgExpr(functionQNameString, functionQName,
+ NodeInfo.AnyURI, NodeInfo.AnyAtomic, FNNamespaceUri0(_, _))
Review comment:
Not your fault since this already existed, but I found this confusing
that we pass in a argument type (NodeInfo.AnyAtomic) for FNZeroArgExpr--an
arguement type should't be needed if there are zero args. Should we create an
issue to remove this paramater? I Imagine this argument is never really used.
Looks like it's only used in targetTypeForSubexpression, which should never be
called in FNZeroArgExpr since there are no args.
It also feels wrong for the FNNamespaceURI0 constructor to take any
arguments. Again, this is somethign that FNZeroArgExpr requires, but those
arguments are never used. Something we should look into fixing, probably as
another issue.
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -1510,6 +1510,14 @@ case class FunctionCallExpression(functionQNameString:
String, expressions: List
FNOneArgExpr(functionQNameString, functionQName, args,
NodeInfo.String, NodeInfo.AnyAtomic, FNLocalName1(_, _))
+ case (RefQName(_, "namespace-uri", FUNC), args) if args.length == 0 =>
+ FNZeroArgExpr(functionQNameString, functionQName,
+ NodeInfo.AnyURI, NodeInfo.AnyAtomic, FNNamespaceUri0(_, _))
+
+ case (RefQName(_, "namespace-uri", FUNC), args) if args.length == 1 =>
+ FNOneArgExpr(functionQNameString, functionQName, args,
+ NodeInfo.AnyURI, NodeInfo.AnyAtomic, FNNamespaceUri1(_, _))
Review comment:
Section 23.5.2.6 of the [DFDL
spec](https://daffodil.apache.org/docs/dfdl/) says that fn:namespace-uri should
return an xs:string rather than an xs:anyURI. But according tot the [XPath
spec](https://www.w3.org/2005/xpath-functions/#namespace-uri) xs:anyURI is
correct.
Mike, do you know if this difference is intentional? DFDL doesn't support
xs:anyURI (except for the new experimental blob feature) so perhaps this was
intentional? Should the return type arg be a NodeInfo.String instead of
NodeInfo.AnyURI?
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala
##########
@@ -706,6 +706,92 @@ case class FNLocalName1(recipe: CompiledDPath, argType:
NodeInfo.Kind)
}
}
+/**
+ * Returns the namespace URI of the name of \$arg as an xs:anyURI
+ * value.
+ *
+ * If the argument is omitted, it defaults to the context item (.).
+ * The behavior of the function if the argument is omitted is
+ * exactly the same as if the context item had been passed as
+ * the argument.
+ *
+ * If the node identified by \$arg is neither an element nor an
+ * attribute node, or it is an element or attribute node whose
+ * expanded-QName is in no namespace, then the function returns
+ * the zero-length xs:anyURI value.
+ *
+ * Otherwise, the result will be the namespace URI of the
+ * expanded-QName of the node identified by \$arg returned as an
+ * xs:anyURI value.
+ *
+ * The following errors may be raised when \$arg is omitted:
+ * - If the context item is absent, dynamic error [err:XPDY002]
+ * - If the context item is not a node, type error [err:XPTY004]
+ *
+ * This function is called when 0 arguments are provided. We
+ * treat this as if the argument passed was "." to denote self.
+ */
+case class FNNamespaceUri0(recipe: CompiledDPath, argType: NodeInfo.Kind)
+ extends RecipeOpWithSubRecipes(recipe) {
+ override def run(dstate: DState) {
+ // Hook so we can insist this is non-constant at compile time
+ dstate.fnExists()
Review comment:
Comparing this with FNLocalName0, we don't have this call. Any idea why
this is needed here but not FNLocalName0? I'm wondering if passing the argType
as NodeInfo.Exists will make this so this isn't even needed?
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala
##########
@@ -706,6 +706,92 @@ case class FNLocalName1(recipe: CompiledDPath, argType:
NodeInfo.Kind)
}
}
+/**
+ * Returns the namespace URI of the name of \$arg as an xs:anyURI
+ * value.
+ *
+ * If the argument is omitted, it defaults to the context item (.).
+ * The behavior of the function if the argument is omitted is
+ * exactly the same as if the context item had been passed as
+ * the argument.
+ *
+ * If the node identified by \$arg is neither an element nor an
+ * attribute node, or it is an element or attribute node whose
+ * expanded-QName is in no namespace, then the function returns
+ * the zero-length xs:anyURI value.
+ *
+ * Otherwise, the result will be the namespace URI of the
+ * expanded-QName of the node identified by \$arg returned as an
+ * xs:anyURI value.
+ *
+ * The following errors may be raised when \$arg is omitted:
+ * - If the context item is absent, dynamic error [err:XPDY002]
+ * - If the context item is not a node, type error [err:XPTY004]
+ *
+ * This function is called when 0 arguments are provided. We
+ * treat this as if the argument passed was "." to denote self.
+ */
+case class FNNamespaceUri0(recipe: CompiledDPath, argType: NodeInfo.Kind)
+ extends RecipeOpWithSubRecipes(recipe) {
+ override def run(dstate: DState) {
+ // Hook so we can insist this is non-constant at compile time
+ dstate.fnExists()
+ // Check that we have a current node, otherwise return empty string(URI)
+ if (dstate.currentNode eq null) {
+ dstate.setCurrentValue("")
+ }
+ else {
+ // Same as using "." to denote self.
+ val value: DataValueURI =
dstate.currentNode.namedQName.namespace.optURI.get
Review comment:
I think optURI is an optional because if the namespace of an element is
NoNamspace then optURI is None. According to the DFDL spec, if the namespace is
NoNamespace, then this should return empty string. So we probably need a
match/case here, somethign like:
```scala
val value = dstate.currentNode.namedQName.namespace.optURI match {
case Some(uri) => uri
case None => new URI("")
}
```
Would be good to add a test case for a no namespace schema.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala
##########
@@ -706,6 +706,92 @@ case class FNLocalName1(recipe: CompiledDPath, argType:
NodeInfo.Kind)
}
}
+/**
+ * Returns the namespace URI of the name of \$arg as an xs:anyURI
+ * value.
+ *
+ * If the argument is omitted, it defaults to the context item (.).
+ * The behavior of the function if the argument is omitted is
+ * exactly the same as if the context item had been passed as
+ * the argument.
+ *
+ * If the node identified by \$arg is neither an element nor an
+ * attribute node, or it is an element or attribute node whose
+ * expanded-QName is in no namespace, then the function returns
+ * the zero-length xs:anyURI value.
+ *
+ * Otherwise, the result will be the namespace URI of the
+ * expanded-QName of the node identified by \$arg returned as an
+ * xs:anyURI value.
+ *
+ * The following errors may be raised when \$arg is omitted:
+ * - If the context item is absent, dynamic error [err:XPDY002]
+ * - If the context item is not a node, type error [err:XPTY004]
+ *
+ * This function is called when 0 arguments are provided. We
+ * treat this as if the argument passed was "." to denote self.
+ */
+case class FNNamespaceUri0(recipe: CompiledDPath, argType: NodeInfo.Kind)
+ extends RecipeOpWithSubRecipes(recipe) {
+ override def run(dstate: DState) {
+ // Hook so we can insist this is non-constant at compile time
+ dstate.fnExists()
+ // Check that we have a current node, otherwise return empty string(URI)
+ if (dstate.currentNode eq null) {
+ dstate.setCurrentValue("")
Review comment:
Is this possible? I think we should always have a currentNode?
Note that this is setting the current value to a string and not a URI. If
this is needed, should this be ``new URI("")``?
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala
##########
@@ -706,6 +706,92 @@ case class FNLocalName1(recipe: CompiledDPath, argType:
NodeInfo.Kind)
}
}
+/**
+ * Returns the namespace URI of the name of \$arg as an xs:anyURI
+ * value.
+ *
+ * If the argument is omitted, it defaults to the context item (.).
+ * The behavior of the function if the argument is omitted is
+ * exactly the same as if the context item had been passed as
+ * the argument.
+ *
+ * If the node identified by \$arg is neither an element nor an
+ * attribute node, or it is an element or attribute node whose
+ * expanded-QName is in no namespace, then the function returns
+ * the zero-length xs:anyURI value.
+ *
+ * Otherwise, the result will be the namespace URI of the
+ * expanded-QName of the node identified by \$arg returned as an
+ * xs:anyURI value.
+ *
+ * The following errors may be raised when \$arg is omitted:
+ * - If the context item is absent, dynamic error [err:XPDY002]
+ * - If the context item is not a node, type error [err:XPTY004]
+ *
+ * This function is called when 0 arguments are provided. We
+ * treat this as if the argument passed was "." to denote self.
+ */
+case class FNNamespaceUri0(recipe: CompiledDPath, argType: NodeInfo.Kind)
+ extends RecipeOpWithSubRecipes(recipe) {
+ override def run(dstate: DState) {
+ // Hook so we can insist this is non-constant at compile time
+ dstate.fnExists()
+ // Check that we have a current node, otherwise return empty string(URI)
+ if (dstate.currentNode eq null) {
+ dstate.setCurrentValue("")
+ }
+ else {
+ // Same as using "." to denote self.
+ val value: DataValueURI =
dstate.currentNode.namedQName.namespace.optURI.get
+ dstate.setCurrentValue(value)
+ }
+ }
+}
+
+/**
+ * Returns the namespace URI of the name of \$arg as an xs:anyURI
+ * value.
+ *
+ * If the node identified by \$arg is neither an element nor an
+ * attribute node, or it is an element or attribute node whose
+ * expanded-QName is in no namespace, then the function returns
+ * the zero-length xs:anyURI value.
+ *
+ * Otherwise, the result will be the namespace URI of the
+ * expanded-QName of the node identified by \$arg returned as an
+ * xs:anyURI value.
+ */
+case class FNNamespaceUri1(recipe: CompiledDPath, argType: NodeInfo.Kind)
+ extends FNOneArg(recipe, argType) {
+ override def computeValue(value: DataValuePrimitive, dstate: DState) = {
+ Assert.usageError("not to be called. DPath compiler should be answering
this without runtime calls.")
+ }
+
+ override def run(dstate: DState) {
+ // Hook so we can insist this is non-constant at compile time
+ dstate.fnExists()
+ // Save off original state, which is the original
+ // element/node that calls inputValueCalc with fn:namespace-uri.
+ val origState = dstate
+
+ // Execute the recipe/expression which should
+ // return a node/element whose namespace-uri we want.
+ recipe.run(dstate)
+
+ // Check that we have a current node, otherwise return empty string(URI)
+ if (dstate.currentNode eq null) {
+ origState.setCurrentValue("")
+ }
+ else {
+ // The original state contains the node/element upon which
+ // fn:namespace-uri was called. This is where we should set
+ // the value.
+ val value : DataValueURI =
dstate.currentNode.namedQName.namespace.optURI.get
+ origState.setCurrentValue(value)
+ }
+ }
Review comment:
Same comments about this as for the zero arg one.
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -1510,6 +1510,14 @@ case class FunctionCallExpression(functionQNameString:
String, expressions: List
FNOneArgExpr(functionQNameString, functionQName, args,
NodeInfo.String, NodeInfo.AnyAtomic, FNLocalName1(_, _))
+ case (RefQName(_, "namespace-uri", FUNC), args) if args.length == 0 =>
+ FNZeroArgExpr(functionQNameString, functionQName,
+ NodeInfo.AnyURI, NodeInfo.AnyAtomic, FNNamespaceUri0(_, _))
+
+ case (RefQName(_, "namespace-uri", FUNC), args) if args.length == 1 =>
+ FNOneArgExpr(functionQNameString, functionQName, args,
+ NodeInfo.AnyURI, NodeInfo.AnyAtomic, FNNamespaceUri1(_, _))
Review comment:
Also the argument type of NodeInfo.AnyAtomic feels wrong to me. This is
what local-name uses, and I think that's wrong too. I think this should just be
NodeInfo.Exists. That should ensure that a path to a node is passed in (and not
a string/int/whatever) and that the node exist when run.
##########
File path:
daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_functions/home_schema.dfdl.xsd
##########
@@ -63,4 +63,6 @@
</xs:complexType>
</xs:element>
+ <xs:element name="e5" type="xs:string" dfdl:inputValueCalc="{
fn:namespace-uri(0) }"/>
Review comment:
I think if you pass the argType as NodeInfo.Exists instead of
NodeInfo.AnyAtomic this won't even compile, which I think is the behavior we
want. namespace-uri should really only work when passed a node.
##########
File path:
daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions.scala
##########
@@ -516,10 +516,11 @@ class TestDFDLExpressions {
//@Test def test_namespace_uri_01() { runner2.runOneTest("namespace_uri_01")
}
//@Test def test_namespace_uri_02() { runner2.runOneTest("namespace_uri_02")
}
//DFDL-1114
- //@Test def test_namespace_uri_03() { runner2.runOneTest("namespace_uri_03")
}
Review comment:
Are changes still needed to get 01 and 02 to pass?
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/FNFunctions.scala
##########
@@ -706,6 +706,92 @@ case class FNLocalName1(recipe: CompiledDPath, argType:
NodeInfo.Kind)
}
}
+/**
+ * Returns the namespace URI of the name of \$arg as an xs:anyURI
+ * value.
+ *
+ * If the argument is omitted, it defaults to the context item (.).
+ * The behavior of the function if the argument is omitted is
+ * exactly the same as if the context item had been passed as
+ * the argument.
+ *
+ * If the node identified by \$arg is neither an element nor an
+ * attribute node, or it is an element or attribute node whose
+ * expanded-QName is in no namespace, then the function returns
+ * the zero-length xs:anyURI value.
+ *
+ * Otherwise, the result will be the namespace URI of the
+ * expanded-QName of the node identified by \$arg returned as an
+ * xs:anyURI value.
+ *
+ * The following errors may be raised when \$arg is omitted:
+ * - If the context item is absent, dynamic error [err:XPDY002]
+ * - If the context item is not a node, type error [err:XPTY004]
+ *
+ * This function is called when 0 arguments are provided. We
+ * treat this as if the argument passed was "." to denote self.
+ */
+case class FNNamespaceUri0(recipe: CompiledDPath, argType: NodeInfo.Kind)
+ extends RecipeOpWithSubRecipes(recipe) {
+ override def run(dstate: DState) {
+ // Hook so we can insist this is non-constant at compile time
+ dstate.fnExists()
+ // Check that we have a current node, otherwise return empty string(URI)
+ if (dstate.currentNode eq null) {
+ dstate.setCurrentValue("")
+ }
+ else {
+ // Same as using "." to denote self.
+ val value: DataValueURI =
dstate.currentNode.namedQName.namespace.optURI.get
+ dstate.setCurrentValue(value)
+ }
+ }
+}
+
+/**
+ * Returns the namespace URI of the name of \$arg as an xs:anyURI
+ * value.
+ *
+ * If the node identified by \$arg is neither an element nor an
+ * attribute node, or it is an element or attribute node whose
+ * expanded-QName is in no namespace, then the function returns
+ * the zero-length xs:anyURI value.
+ *
+ * Otherwise, the result will be the namespace URI of the
+ * expanded-QName of the node identified by \$arg returned as an
+ * xs:anyURI value.
+ */
+case class FNNamespaceUri1(recipe: CompiledDPath, argType: NodeInfo.Kind)
+ extends FNOneArg(recipe, argType) {
+ override def computeValue(value: DataValuePrimitive, dstate: DState) = {
+ Assert.usageError("not to be called. DPath compiler should be answering
this without runtime calls.")
+ }
+
+ override def run(dstate: DState) {
+ // Hook so we can insist this is non-constant at compile time
+ dstate.fnExists()
+ // Save off original state, which is the original
+ // element/node that calls inputValueCalc with fn:namespace-uri.
Review comment:
Again, not your fault since this is what was in FNLocalName, but
inputValueCalc probably shouldn't be mentioned in this comment. fn:namespace-ui
can be called from many places other than inputValueCalc.
We also shouldn't be saving dstate, but intead just save dstate.currentNode.
You'll see this pattern alot:
```scala
val savedNode = dstate.currentNode
recipe.run(dstate)
val value = dstate.getWhateverStateWeNeed
dstate.setCurrentNode(savedNode)
// do somethign with value
```
I think FNLocaleName was just buggy, or at least is old and isn't following
the common patterns we've been using with more recently added functions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
