stevedlawrence commented on a change in pull request #431:
URL: https://github.com/apache/incubator-daffodil/pull/431#discussion_r512816351



##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/api/Validator.scala
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.api
+
+import com.typesafe.config.Config
+
+import scala.xml.SAXException
+
+trait Validator {
+  def validateXML(document: java.io.InputStream): ValidationResult
+}
+
+trait ValidatorFactory {
+  def name(): String
+  def make(config: Config): Validator
+}
+
+case class ValidationResult(warnings: Seq[ValidationWarning], errors: 
Seq[ValidationFailure]) {
+  def failed: Boolean = errors.nonEmpty
+}
+
+trait ValidationFailure {
+  def getMessage: String
+}
+
+trait ValidationWarning {
+  def getMessage: String
+}
+
+trait ValidationException {
+  def getCause: Throwable
+}
+
+// todo;; can validator return other metadata?
+//trait ValidationReport
+
+sealed abstract class SaxValidationResult(e: SAXException) extends 
Exception(e) with ValidationException
+case class SaxValidationError(e: SAXException) extends SaxValidationResult(e) 
with ValidationFailure with ValidationException
+case class SaxValidationWarning(e: SAXException) extends 
SaxValidationResult(e) with  ValidationWarning with ValidationException

Review comment:
       Why do we need specific SAX Validation errors in the Validation API? I 
would expect that if a Validator used SAX, it would need to catch those 
exceptions and convert them to the ValidationResult/Failure/Warning class that 
Daffodil understands. Presumably we don't want this tied to SAX so that non-SAX 
implementations could work with out issue.

##########
File path: 
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
##########
@@ -243,6 +250,18 @@ class DataProcessor private (
 
   def withValidationMode(mode:ValidationMode.Type): DataProcessor = 
copy(validationMode = mode)
 
+  def withValidator(validator: Validator): DataProcessor = copy(validationMode 
= ValidationMode.Custom(validator))
+
+  lazy val validator: Validator = {
+    validationMode match {
+      case ValidationMode.Custom(cv) => cv
+      case _ =>
+        val v = 
ConfigValueFactory.fromAnyRef(ssrd.elementRuntimeData.schemaURIStringsForFullValidation)
+        val f = Validators.default
+        f.make(ConfigFactory.parseMap(Map(f.name() -> v).asJava))
+    }

Review comment:
       This is a bit interseting. So Custom Validators won't have any access to 
the schemaURIStrings? Is it possible other custom validators will need those? I 
imaginge in something like schematron it might not, but other XML Schema 
validators that aren't our built-in xerces one might? Seems odd that only our 
internal validator gets access to this information? Or shold no Validators get 
this access, including our own, and Validators are expected to implement their 
own logic for this like URI resolution?

##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/api/Validator.scala
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.
+ */
+

Review comment:
       Is this the API that custom validators must implement? If so, we shoul 
throuoghly add documentation to all these functions. It also might be worth 
adding a new projet for this so that we can include this validator API in the 
public documentation via unidoc, similar to what we do with UDF. Might even be 
worth implementing this API in Java to ensure both Java and Scala 
implementations can be created.

##########
File path: 
daffodil-japi/src/main/scala/org/apache/daffodil/japi/packageprivate/Utils.scala
##########
@@ -73,6 +73,8 @@ private[japi] object ValidationConversions {
       case ValidationMode.Off => SValidationMode.Off
       case ValidationMode.Limited => SValidationMode.Limited
       case ValidationMode.Full => SValidationMode.Full
+      // todo;; support custom in java?
+      case _ => SValidationMode.Off

Review comment:
       Was this issue ever resolved? How does one specify custom validation via 
the SAPI/JAPI? It would probably be worth adding Java and Scala tests to make 
sure this works as expected, even if a test validator doesn't do anything. That 
way we can ensure we have a publicly workable API to provide validator 
configurations.

##########
File path: 
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
##########
@@ -419,14 +427,16 @@ class DataProcessor private (
     // events are created rather than writing the entire infoset in memory and
     // then validating at the end of the parse. See DAFFODIL-2386
     //
-    val (outputter, maybeValidationBytes) =
-    if (validationMode == ValidationMode.Full) {
-      val bos = new java.io.ByteArrayOutputStream()
-      val xmlOutputter = new XMLTextInfosetOutputter(bos, false)
-      val teeOutputter = new TeeInfosetOutputter(output, xmlOutputter)
-      (teeOutputter, One(bos))
-    } else {
-      (output, Nope)
+    val (outputter, maybeValidationBytes) = {
+      validationMode match {
+        case ValidationMode.Full | ValidationMode.Custom(_) =>
+          val bos = new java.io.ByteArrayOutputStream()
+          val xmlOutputter = new XMLTextInfosetOutputter(bos, false)
+          val teeOutputter = new TeeInfosetOutputter(output, xmlOutputter)
+          (teeOutputter, One(bos))

Review comment:
       Yep, and that will completely change the validator API. Perhaps we'll 
end up supporting two types of API's? One that gets the entire infoset as a 
string and one that gets indidual SAX events?

##########
File path: 
daffodil-lib/src/main/scala/org/apache/daffodil/util/DefaultValidator.scala
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.util
+
+import java.net.URI
+
+import com.typesafe.config.Config
+import javax.xml.XMLConstants
+import javax.xml.transform.stream.StreamSource
+import org.apache.daffodil.api.SaxValidationError
+import org.apache.daffodil.api.SaxValidationWarning
+import org.apache.daffodil.api.ValidationFailure
+import org.apache.daffodil.api.ValidationResult
+import org.apache.daffodil.api.ValidationWarning
+import org.apache.daffodil.api.Validator
+import org.apache.daffodil.api.ValidatorFactory
+import org.apache.daffodil.util.DefaultValidator.XercesValidator
+import org.apache.daffodil.xml.DFDLCatalogResolver
+import org.xml.sax.ErrorHandler
+import org.xml.sax.SAXException
+import org.xml.sax.SAXParseException
+
+import scala.collection.JavaConverters._
+
+/**
+ * Use this for extra validation passes in the TDML Runner
+ * to do a validation pass on the TDML expected Infoset w.r.t. the model and to
+ * do a validation pass on the actual result w.r.t. the model as an XML 
document.
+ */
+class DefaultValidator(schemaFileNames: Seq[String])

Review comment:
       Agreed, I dont' think anything should be called DefaultXYZ. When a 
validator is used determines if it's a default. If we decide to change the 
default this name would no longer make sense.




----------------------------------------------------------------
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]


Reply via email to