Skip to content

Wait for a computable target length before unparsing truncating strings #1738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
olabusayoT merged 1 commit into from
Oct 5, 2026
+51 −10
Merged

Wait for a computable target length before unparsing truncating strings #1738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 48 additions & 3 deletions ...il-core/src/main/scala/org/apache/daffodil/unparsers/runtime1/StringLengthUnparsers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.apache.daffodil.runtime1.processors.CharsetEv
import org.apache.daffodil.runtime1.processors.ElementRuntimeData
import org.apache.daffodil.runtime1.processors.Evaluatable
import org.apache.daffodil.runtime1.processors.LengthEv
import org.apache.daffodil.runtime1.processors.SuspendableOperation
import org.apache.daffodil.runtime1.processors.TextTruncationType
import org.apache.daffodil.runtime1.processors.UnparseTargetLengthInBitsEv
import org.apache.daffodil.runtime1.processors.unparsers.*
Expand Down Expand Up @@ -70,10 +71,26 @@ class StringNoTruncateUnparser(erd: ElementRuntimeData)
sealed abstract class StringSpecifiedLengthUnparserTruncateBase(
stringTruncationType: TextTruncationType.Type,
erd: ElementRuntimeData
) extends StringSpecifiedLengthUnparserBase(erd) {
) extends StringSpecifiedLengthUnparserBase(erd)
with SuspendableUnparser {

Assert.usage(stringTruncationType ne TextTruncationType.None)

/**
* The target length the string is truncated to. It can depend on an
* outputValueCalc element that has not been computed yet.
*/
protected[runtime1] def targetLengthEv: Evaluatable[AnyRef]

/**
* Reads the target length without retrying, so it must only be called
* once the target length can be evaluated.
*/
protected[runtime1] def unparseString(state: UState): Unit

override protected def suspendableOperation =
new StringTruncationSuspendableOperation(erd, this)

/**
* We only truncate strings, and only if textStringJustification is left or
* right, and only if truncateSpecifiedLengthString is yes.
Expand Down Expand Up @@ -122,6 +139,8 @@ class StringMaybeTruncateBitsUnparser(

override val runtimeDependencies = Array(targetLengthInBitsEv, charsetEv)

override protected[runtime1] def targetLengthEv = targetLengthInBitsEv

private def getLengthInBits(str: String, state: UState): (Long, Long) = {
val cs = charsetEv.evaluate(state)
val mfw = cs.maybeFixedWidth
Expand Down Expand Up @@ -171,7 +190,7 @@ class StringMaybeTruncateBitsUnparser(
res
}

override def unparse(state: UState): Unit = {
override protected[runtime1] def unparseString(state: UState): Unit = {

//
// We have to stage the bits of the value just so as to be able to count them
Expand Down Expand Up @@ -259,7 +278,9 @@ class StringMaybeTruncateCharactersUnparser(

override val runtimeDependencies = Array(lengthInCharactersEv)

override def unparse(state: UState): Unit = {
override protected[runtime1] def targetLengthEv = lengthInCharactersEv

override protected[runtime1] def unparseString(state: UState): Unit = {
val dos = state.getDataOutputStream
val valueString = contentString(state)
val targetLengthInCharacters =
Expand All @@ -285,3 +306,27 @@ class StringMaybeTruncateCharactersUnparser(
Assert.invariant(nCharsWritten == valueToWrite.length)
}
}

/**
* Suspends until the target length can be evaluated, then unparses the
* string, because the truncating string unparsers read the target length
* without retrying.
*/
class StringTruncationSuspendableOperation(
override val rd: ElementRuntimeData,
unparser: StringSpecifiedLengthUnparserTruncateBase
) extends SuspendableOperation {

override def toString = "string truncation for " + rd.diagnosticDebugName

// Evaluating throws a RetryableException while the length is unavailable,
// which blocks this operation.
override protected def test(ustate: UState): Boolean = {
unparser.targetLengthEv.evaluate(ustate)
true
}

override protected def continuation(ustate: UState): Unit = {
unparser.unparseString(ustate)
}
}

@stevedlawrence stevedlawrence Sep 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern is very interesting. It's not how we usually implement suspensions but I wonder if we should move towards this? So the SuspendableOperation just contains a reference to the unparser and the unparser does the actual work, making it much more similar to how normal unparsers look.

We could make it even a bit more generic. For example, maybe something like this:

class MySuspendableUnparser extends DelegatedSuspendableUnparser {

  override protected def suspendableOperation =
     new DelegatingSuspendableOperation(this)

   override def suspensionTest(ustate: UState) = ...

   override def suspensionContinuation(ustate: UState) = ...

...

class DeletgatingSuspendableOperation(
   unparser: DelegatedSuspendableUnparser,
) extends SuspendableOperation {

  override val rd = unparser.erd

  override protected def test(ustate: UState): Boolean =
    unparser.suspensionTest(ustate)

  override protected def continuation(ustate: UState): Unit =
    unparser.suspensionContinuation(ustate)
}

So the original unparser implements the test/continuation functions and the DelegatingSuspendableOperation just calls into that. This avoids needing multiple classes for each suspendable unparser as we have now. There's just the original Unparser, with some special functions used by the reuseable DelegatingSuspendableOperation. Note, I'm not a fan of the names, but it gets the point across.

This doesn't work as well if there is additional state that needs to be passed into the SuspendableOperation. I'm not sure if other Unparsers do that. Might not be worth if it they all work differently and need very different state.

Also note something we necessarily need to do now, but might be worth considering in the future.

@olabusayoT olabusayoT Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created DAFFODIL-3102 to track

10 changes: 3 additions & 7 deletions ...il-test/src/test/resources/org/apache/daffodil/section00/general/testUnparserGeneral.tdml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,8 +1023,7 @@
</tdml:dfdlInfoset>
</tdml:infoset>
<tdml:errors>
<tdml:error>not have a value</tdml:error>
<tdml:error>circular dependency</tdml:error>
<tdml:error>circularly deadlocked</tdml:error>
</tdml:errors>
</tdml:unparserTestCase>

Expand Down Expand Up @@ -1055,10 +1054,7 @@
</ex:e1>
</tdml:dfdlInfoset>
</tdml:infoset>
<tdml:errors>
<tdml:error>not have a value</tdml:error>
<tdml:error>circular dependency</tdml:error>
</tdml:errors>
<tdml:document>5ABCDE</tdml:document>
</tdml:unparserTestCase>

<tdml:defineSchema name="truncatedString3">
Expand Down Expand Up @@ -1113,7 +1109,7 @@
</tdml:dfdlInfoset>
</tdml:infoset>
<tdml:errors>
<tdml:error>Value length unknown</tdml:error>
<tdml:error>circularly deadlocked</tdml:error>
</tdml:errors>
</tdml:unparserTestCase>

Expand Down
Loading