mbeckerle commented on code in PR #1176:
URL: https://github.com/apache/daffodil/pull/1176#discussion_r1517906801


##########
daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/runtime1/BoundaryMarkLayer.scala:
##########
@@ -45,21 +45,29 @@ import org.apache.daffodil.runtime1.layers.api.LayerRuntime
  * disrupt the search for the boundary mark string.
  *
  * This layer does not populate any DFDL variables with results.
+ *
  * This layer defines two DFDL variables which provide required parameters.
- * @param boundaryMark a string which is the boundary marker. Searching for 
this string is
- *                     done without any notion of escaping. When parsing the 
data in the
- *                     layer is up to but not including that of the boundary 
mark string,
- *                     which is removed from the data-stream on parsing, and 
inserted into
- *                     the data stream on unparsing.
- * @param layerEncoding a string which is the name of the character set 
encoding used to
- *                      decode characters during the search for the boundary 
mark, and used
- *                      to encode characters when inserting the boundary mark 
when unparsing.
  */
-final class BoundaryMarkLayer(boundaryMark: String, layerEncoding: String)
+final class BoundaryMarkLayer
   extends Layer("boundaryMark", "urn:org.apache.daffodil.layers.boundaryMark") 
{
 
-  /** no arg contructor required for SPI class loading */
-  def this() = this("dummy", "ascii")
+  private var boundaryMark: String = _
+  private var layerEncoding: String = _
+
+  /**
+   * @param boundaryMark  a string which is the boundary marker. Searching for 
this string is
+   *                      done without any notion of escaping. When parsing 
the data in the
+   *                      layer is up to but not including that of the 
boundary mark string,
+   *                      which is removed from the data-stream on parsing, 
and inserted into
+   *                      the data stream on unparsing.
+   * @param layerEncoding a string which is the name of the character set 
encoding used to
+   *                      decode characters during the search for the boundary 
mark, and used
+   *                      to encode characters when inserting the boundary 
mark when unparsing.
+   */
+  def setLayerVariableParameters(boundaryMark: String, layerEncoding: String): 
Unit = {
+    this.boundaryMark = boundaryMark
+    this.layerEncoding = layerEncoding

Review Comment:
   I think we can say any exception thrown from this becomes a 
SchemaDefinitionError. I added that as the defined way to do this. I didn't 
want to muddy the waters with an arg that isn't about a variable. 



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/layers/ChecksumLayerBase.scala:
##########
@@ -38,11 +38,35 @@ import org.apache.commons.io.IOUtils
 abstract class ChecksumLayerBase(
   localName: String,
   namespace: String,
-  val length: Int,
 ) extends Layer(localName, namespace) {
 
   private var checksum: Int = -1
 
+  private var length: Int = -1
+  private var isInitialized: Boolean = false
+
+  final def getLength: Int = length
+  final protected def setLength(len: Int): Unit = {
+    this.length = len
+  }
+
+  /**
+   * Initializes the layer.
+   *
+   * This method is called to ensure that `setLength` has been called before 
the streams are created.
+   * It can be overridden in subclasses, but those must run the 
`super.init(lr)`
+   *
+   * @param lr The LayerRuntime instance.
+   */
+  protected def init(lr: LayerRuntime): Unit = {

Review Comment:
   not calling setLength is an SDE now, and check is gone. 



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/layers/api/Layer.java:
##########
@@ -63,11 +63,11 @@
  * <p/>

Review Comment:
   updated in next commits



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/layers/api/Layer.java:
##########
@@ -63,11 +63,11 @@
  * <p/>
  * A layer that wants to return a value after the layer algorithm completes 
defines a special recognizable
  * getter method. The name of the getter is formed from prefixing the DFDL 
variable name with the string
- * 'getDFDLResultVariable_'. The return type of the getter must match the type 
of the variable.
+ * 'getLayerVariableResult_'. The return type of the getter must match the 
type of the variable.
  * <p/>
  * For example, a result value getter for a DFDL variable named 'checksum' of 
type xs:unsignedShort would be:
  * <pre>
- *      Int getDFDLResultVariable_checksum() {
+ *      Int getLayerVariableResult_checksum() {

Review Comment:
   package private works also. I've changed all ours, since those will be used 
as examples. 



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/layers/api/ChecksumLayer.java:
##########
@@ -23,29 +23,29 @@
 /**
  * A checksum layer computes some sort of integer value from a region of the 
data stream. The term checksum is
  * used generically here to subsume all sorts of CRCs, check digits, data 
hash, and digest calculations.
- *
+ * <p/>
  * This base is suitable only for checksums computed over small sections of 
data, not large data streams or whole files.
- *
+ * <p/>
  * The entire region of data the checksum is being computed over, will be 
pulled into a byte buffer in memory.
- *
+ * <p/>
  * The resulting checksum is the return value of the compute method.
- *
+ * <p/>
  * This is delivered into a DFDL variable for use by the DFDL schema. This 
variable can have any name
  * such as 'crc', 'digest', or 'dataHash'.
- *
+ * <p/>
  * The derived implementation class must also define a getter method based on 
the name of the DFDL variable which
  * will be assigned with the checksum value. For example if the checksum is 
actually a specific digest/hash calculation
  * and the DFDL variable is named 'digest', then this getter must be defined:
- *
- *     Int getDFDLResultVariable_digest() { return getChecksum() }
- *
+ * <p/>
+ *     Int getLayerVariableResult_digest() { return getChecksum() }
+ * <p/>
  * This will be called automatically to retrieve the integer value that was 
returned from the `compute` method, and
  * the DFDL variable 'digest' will be assigned that value.
  */
 public abstract class ChecksumLayer extends ChecksumLayerBase {

Review Comment:
   fixed.



##########
daffodil-runtime1-layers/src/main/resources/org/apache/daffodil/layers/xsd/boundaryMarkLayer.dfdl.xsd:
##########
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+<schema
+  xmlns:xs="http://www.w3.org/2001/XMLSchema";
+  xmlns="http://www.w3.org/2001/XMLSchema";
+  xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/";
+  xmlns:dfdlx="http://www.ogf.org/dfdl/dfdl-1.0/extensions";
+  xmlns:bm="urn:org.apache.daffodil.layers.boundaryMark"
+  targetNamespace="urn:org.apache.daffodil.layers.boundaryMark">
+
+  <annotation>
+    <appinfo source="http://www.ogf.org/dfdl/";>
+
+      <dfdl:defineVariable name="boundaryMark" type="xs:string"/>
+      <dfdl:defineVariable name="layerEncoding" type="xs:string"/>

Review Comment:
   I have NOT addressed this yet. 



##########
daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/runtime1/FixedLengthLayer.scala:
##########
@@ -42,33 +40,35 @@ import org.apache.commons.io.IOUtils
  *   This length is enforced on both parsing and unparsing the layer.
  * There are no output/result DFDL variables from this layer.
  */
-final class FixedLengthLayer(fixedLength: JLong)
+final class FixedLengthLayer
   extends Layer("fixedLength", "urn:org.apache.daffodil.layers.fixedLength") {
 
-  Assert.invariant(fixedLength > 0)
+  private var fixedLength: Long = 0
 
-  /** Required for SPI class loading */
-  def this() = this(1)
-
-  private def maxFixedLength = Short.MaxValue
-
-  override def wrapLayerInput(jis: InputStream, lr: LayerRuntime): InputStream 
= {
+  def setLayerVariableParameters(fixedLength: Long): Unit = {
+    this.fixedLength = fixedLength
+  }
 
+  private def check(lr: LayerRuntime): Unit = {
+    if (fixedLength < 1)

Review Comment:
   Yes, this is gone. In the IPV4Checksum example, the 
setLayerVariableParameters has no args and just serves to initialize. 



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/layers/api/ChecksumLayer.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.runtime1.layers.api;
+
+import org.apache.daffodil.runtime1.layers.ChecksumLayerBase;
+
+import java.nio.ByteBuffer;
+
+/**
+ * Suitable only for checksums computed over small sections of data, not large 
data streams or whole files.
+ *
+ * The entire region of data the checksum is being computed over, will be 
pulled into a byte buffer in memory.
+ */
+public abstract class ChecksumLayer extends ChecksumLayerBase {
+
+  public ChecksumLayer(String layerName, String layerNamespace, int 
checksumLayerLength) {
+    super(layerName, layerNamespace, checksumLayerLength);
+  }
+
+  /**
+   * Override to compute the checksum of a buffer of data. The value computed 
is assigned to the first
+   * DFDL variable defined by the layer in the LayerInfo object.
+   *
+   * @param layerRuntime layer context for the computation
+   * @param isUnparse true if the direction is unparsing. Used because in some 
cases the computed checksum must
+   *                  be written into the byte buffer in a specific location.
+   * @param byteBuffer the bytes over which the checksum is to be computed. 
This can be modified, (for example so as
+   *                   to embed the computed checksum in the middle of the 
data somewhere) and the resulting
+   *                   bytes become the data that is written when unparsing.
+   *                   If the bytes in this buffer are modified by the compute 
method, those modified bytes are what
+   *                   the parsing will parse from, and the unparsing will 
output.
+   * @return the checksum value as an Int (32-bit signed integer)
+   */
+  public abstract int compute(
+    LayerRuntime layerRuntime,
+    boolean isUnparse,
+    ByteBuffer byteBuffer
+  );
+}

Review Comment:
   Turns out the code is tolerant of the unboxed types. I just had to make our 
own type checking tolerate it. 
   
   Javadoc for Layer has been updated to document the getter. 



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/layers/LayerDriver.scala:
##########
@@ -0,0 +1,227 @@
+/*
+ * 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.runtime1.layers
+
+import java.io.FilterInputStream
+import java.io.FilterOutputStream
+import java.io.InputStream
+import java.io.OutputStream
+
+import org.apache.daffodil.io.DataInputStream.Mark
+import org.apache.daffodil.io.DataOutputStream
+import org.apache.daffodil.io.DirectOrBufferedDataOutputStream
+import org.apache.daffodil.io.FormatInfo
+import org.apache.daffodil.io.InputSourceDataInputStream
+import org.apache.daffodil.lib.exceptions.Assert
+import org.apache.daffodil.lib.schema.annotation.props.gen.BitOrder
+import org.apache.daffodil.lib.util.Maybe
+import org.apache.daffodil.lib.util.Maybe.Nope
+import org.apache.daffodil.lib.util.Maybe.One
+import org.apache.daffodil.lib.util.Misc
+import org.apache.daffodil.runtime1.layers.api.Layer
+import org.apache.daffodil.runtime1.processors.unparsers.UState
+
+import passera.unsigned.ULong
+
+/**
+ * Driving mechanism that incorporates a layer at runtime to transform the 
data stream.
+ *
+ * A layer driver is created at runtime as part of a single parse/unparse call.
+ * Hence, they can be stateful without causing thread-safety issues.
+ */
+class LayerDriver(
+  layerRuntimeData: LayerRuntimeData,
+  layer: Layer,
+  layerVarsRuntime: LayerVarsRuntime,
+) {
+
+  private def wrapJavaInputStream(
+    s: InputSourceDataInputStream,
+    layerRuntimeImpl: LayerRuntimeImpl,
+  ): InputStream =
+    new JavaIOInputStream(s, layerRuntimeImpl.finfo)
+
+  private def wrapJavaOutputStream(
+    s: DataOutputStream,
+    layerRuntimeImpl: LayerRuntimeImpl,
+  ): OutputStream =
+    new JavaIOOutputStream(s, layer, layerRuntimeImpl, layerVarsRuntime)
+
+  /**
+   *  Override these if we ever have transformers that don't have these
+   *  requirements.
+   */
+  val mandatoryLayerAlignmentInBits: Int = 8
+
+  final def addInputLayer(
+    s: InputSourceDataInputStream,
+    layerRuntimeImpl: LayerRuntimeImpl,
+  ): InputSourceDataInputStream = {
+    val jis = wrapJavaInputStream(s, layerRuntimeImpl)
+    val decodedInputStream =
+      new AssuredCloseInputStream(layer.wrapLayerInput(jis, layerRuntimeImpl), 
jis)
+    val newDIS = InputSourceDataInputStream(decodedInputStream)
+    newDIS.cst.setPriorBitOrder(
+      BitOrder.MostSignificantBitFirst,
+    ) // must initialize priorBitOrder
+    newDIS.setDebugging(s.areDebugging)
+    newDIS
+  }
+
+  /**
+   * Parsing works as a tree traversal, so when the parser unwinds from
+   * parsing the layer we can invoke this to handle cleanups, and
+   * finalization issues like assigning the result variables
+   */
+  final def removeInputLayer(
+    s: InputSourceDataInputStream,
+    layerRuntimeImpl: LayerRuntimeImpl,
+  ): Unit = {
+    layerVarsRuntime.callGettersToPopulateResultVars(layer, layerRuntimeImpl)

Review Comment:
   This is cleaned up next push of changes.  The LayerDriver class now does all 
the invocation of the setter for vars and getter for results. The LayerFactory 
just invokes the LayerRuntimeCompiler and creates the Layer object instance, 
and LayerDriver that encapsulates it. 



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

Reply via email to