tuxji commented on code in PR #886:
URL: https://github.com/apache/daffodil/pull/886#discussion_r1042394559
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala:
##########
@@ -154,27 +154,23 @@ class DataProcessor private (
* That means when we save for reloading, we must explicitly clobber
validationMode and externalVars to
* initialized values.
*
- * @throws java.io.ObjectStreamException
+ * @throws java.io.ObjectStreamException Must be part of writeReplace's API
* @return the serializable object
*/
@throws(classOf[java.io.ObjectStreamException])
private def writeReplace() : Object =
new SerializableDataProcessor(ssrd, tunables, externalVars, validationMode)
/**
- * The compilerExternalVars argument supports the deprecated feature to
assign external var bindings
- * on the compiler object.
- *
- * These are just incorporated into the initial variable map of the data
processor.
+ * This constructor reconstructs a DataProcessor from a
SerializedDataProcessor.
*/
-
def this(
ssrd: SchemaSetRuntimeData,
tunables:DaffodilTunables,
- compilerExternalVars: Queue[Binding] = Queue.empty,
+ externalVars: Queue[Binding] = Queue.empty,
validationMode: ValidationMode.Type = ValidationMode.Off) =
- this(ssrd, tunables,
ExternalVariablesLoader.loadVariables(compilerExternalVars, ssrd,
ssrd.originalVariables),
- false, None, validationMode, compilerExternalVars)
+ this(ssrd, tunables, ExternalVariablesLoader.loadVariables(externalVars,
ssrd, ssrd.originalVariables),
+ false, None, validationMode, externalVars)
Review Comment:
I had initially thought SchemaSetRuntime1Mixin.scala was the only caller of
this constructor too and removed the externalVars parameter from this
constructor, but I got a compilation error which showed that there was a second
call of this constructor from SerializableDataProcessor's constructor in
DataProcessor.scala at line 107 that I didn't know about.
SerializableDataProcessor extends DataProcessor(data, tunable, externalVars,
validationModeArg) in order to preserve the externalVars and validationMode in
case they may be needed by other serializations besides Daffodil save/reload
such as Apache Spark which serializes in order to move objects for remote
execution.
We can't remove the externalVars field from DataProcessor because we inform
DataProcessor about external variable bindings in that field. DataProcessor's
primary constructor is private (probably because of the mix of parameters some
of which should be serialized all the time, some which should be serialized
only sometimes, and some which should never be serialized). DataProcessor has
both a class and an object, but the object DataProcessor has no apply method,
only a private class SerializableDataProcessor. Therefore, this particular
(public) constructor is the only way which callers outside DataProcessor.scala
can create a DataProcessor. Our options are to keep this public constructor or
add an apply method to the object DataProcessor which calls the private
constructor.
At the very least, I need to fix this constructor's scaladoc. I had changed
the scaladoc to say it was needed by SerializableDataProcessor, but I misstated
the reason. I can make the scaladoc state that this constructor exists to
construct a SerializableDataProcessor, not the other way around. Or I can
remove this constructor and add an apply method to object DataProcessor, which
would put that apply method next to the class SerializableDataProcessor and
make the relationship between them more obvious. I would also add a second
apply method with only 2 parameters and its own scaladoc to let external
callers like SchemaSetRuntime1Mixin.scala create a DataProcessor.
Thoughts?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
