tuxji commented on a change in pull request #520:
URL: https://github.com/apache/daffodil/pull/520#discussion_r609801194



##########
File path: daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
##########
@@ -328,7 +332,7 @@ class CLIConf(arguments: Array[String]) extends 
scallop.ScallopConf(arguments)
   object parse extends scallop.Subcommand("parse") {
     banner("""|Usage: daffodil parse (-s <schema> [-r [{namespace}]<root>] [-p 
<path>] |
               |                       -P <parser>)
-              |                      [--validate [mode]]
+              |                      [--validate [mode]] [--validate_output 
file]

Review comment:
       Please use brackets around "file" for consistency with the other usage 
lines: `[--validate_output <file>]`

##########
File path: 
daffodil-schematron/src/main/scala/org/apache/daffodil/validation/schematron/SchematronResult.scala
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.validation.schematron
+
+import org.apache.daffodil.api.RawValidationResult
+import org.apache.daffodil.api.ValidationFailure
+import org.apache.daffodil.api.ValidationResult
+import org.apache.daffodil.api.ValidationWarning
+
+import java.util
+
+/**
+ * Captures the output of Schematron validation as a Daffodil ValidationResult
+ * @param warnings Schematron warnings parsed into ValidationWarning objects
+ * @param errors Schematron errors parsed into ValidationFailure objects
+ * @param svrl Full SVRL output
+ */
+case class SchematronResult(warnings: util.Collection[ValidationWarning],
+                            errors: util.Collection[ValidationFailure],
+                            svrl: String) extends ValidationResult with 
RawValidationResult {
+
+  def rawValidationData(): Array[Byte] = svrl.getBytes
+}

Review comment:
       I see what you mean - lines 906-917 in Main.scala performs a match on 
ValidationResult to get a case Some(vr: RawValidationResult) and writes out 
vr.rawValidationData() accordingly.  I also think there would be nothing to 
stop someone from doing a match on ValidationResult to get a case Some(vr: 
MyCustomValidationResult) if they want to use some custom data without 
conversion to/from raw bytes, right?  So people are free to use both ways in 
their own custom validator implementation - extend RawValidatorResult to let 
the CLI write the file and also provide custom methods in 
MyCustomValidationResult, which seems fair to me.

##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/api/Validator.scala
##########
@@ -54,20 +55,31 @@ trait ValidatorFactory {
 
 /**
  * Results of a validation execution
- * @param warnings [[org.apache.daffodil.api.ValidationWarning]] objects
- * @param errors [[org.apache.daffodil.api.ValidationFailure]] objects
  */
-final case class ValidationResult(warnings: 
java.util.Collection[ValidationWarning], errors: 
java.util.Collection[ValidationFailure])
+trait ValidationResult {
+  def warnings(): java.util.Collection[ValidationWarning]
+  def errors(): java.util.Collection[ValidationFailure]
+}
+
+/**
+ * Provider of raw validation output
+ */
+trait RawValidationResult {
+  def rawValidationData(): Array[Byte]
+}
 
 object ValidationResult {
   /**
    * an empty [[org.apache.daffodil.api.ValidationResult]]
    */
   val empty: ValidationResult = ValidationResult(Seq.empty, Seq.empty)
 
-  def apply(warnings: Seq[ValidationWarning], errors: Seq[ValidationFailure]): 
ValidationResult = {
+  def apply(w: Seq[ValidationWarning], e: Seq[ValidationFailure]): 
ValidationResult = {
     import scala.collection.JavaConverters.asJavaCollectionConverter
-    ValidationResult(warnings.asJavaCollection, errors.asJavaCollection)
+    new ValidationResult{
+      val warnings: java.util.Collection[ValidationWarning] = 
w.asJavaCollection
+      val errors: java.util.Collection[ValidationFailure] = e.asJavaCollection
+    }

Review comment:
       Here's an interesting tidbit.  No other place in the Daffodil codebase 
uses java.util.Collection or the asJavaCollection converter from Seq to 
Collection.  I'm also surprised that an API class in daffodil-lib would use 
Collection instead of Seq, although doing so lets Java callers use 
ValidationResult directly.  If all this is a new and better practice, I suspect 
we could apply it in more places.  




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