This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5722-249d115bd8e1df957466fa0ccb2e3a7a133989cd in repository https://gitbox.apache.org/repos/asf/texera.git
commit 77cc0f4f92d79946b6bb8d99106bf33df4dfe4b6 Author: Eugene Gu <[email protected]> AuthorDate: Sun Jun 14 23:59:09 2026 -0700 test(workflow-core): add dedicated GlobalPortIdentitySerde spec (#5722) ### What changes were proposed in this PR? This PR adds a dedicated `GlobalPortIdentitySerdeSpec` for `GlobalPortIdentitySerde`, the helper that serializes/deserializes a `GlobalPortIdentity` to the compact, underscore-free string used in VFS URIs and file paths. `GlobalPortIdentitySerde` was previously exercised inside `PortIdentitySerdeSpec`, which mixed it with the unrelated `PortIdentity` Jackson key (de)serializer tests. Rather than duplicate that coverage, this PR: - **Moves** the `GlobalPortIdentitySerde` cases into their own file (the name issue #5717 asks for), so the two serde concerns are tested independently. - **Trims** `PortIdentitySerdeSpec` down to only the `PortIdentity` Jackson key (de)serialization tests, dropping the now-unused imports. - **Adds** seven edge cases that were previously unpinned (see below). There are **no production-code changes** and **no duplication** between the two specs. New edge cases added beyond the migrated coverage: 1. **serialize/deserialize asymmetry** — the deserializer regex accepts underscores even though the serializer rejects them; characterized so a future tightening of the regex breaks the test on purpose. 2. **unescaped comma** — a `logicalOpId` containing a `,` fails to round-trip, because the format does not escape its separator. 3. **portId boundary** — `Int.MaxValue` round-trips, and an out-of-Int-range value (`9999999999`) is rejected with `NumberFormatException` on deserialize. 4. **mixed-case booleans** — `True` / `FALSE` are accepted on deserialize, since Scala's `String.toBoolean` is case-insensitive. 5. **`=` in a value round-trips** — only `,` is a sensitive separator; an embedded `=` is captured fine, the counterpart to the comma case. 6. **empty `logicalOpId`** — serializes into `(logicalOpId=,...)` but the result can no longer be deserialized (`[^,]+` requires ≥1 char), the serialize side of the empty-field asymmetry. 7. **require messages name the field** — the serialize-side guards identify the offending field (`logicalOpId` / `layerName` / `portId`) in their message, catching a regression that wires a check to the wrong field. | File | Change | | --- | --- | | `common/workflow-core/.../util/serde/GlobalPortIdentitySerdeSpec.scala` | **new** — 23 tests (`AnyFlatSpec with Matchers`) | | `common/workflow-core/.../util/serde/PortIdentitySerdeSpec.scala` | removed the migrated `GlobalPortIdentitySerde` block + now-unused imports (−189 lines); `PortIdentity` Jackson key (de)serialization tests kept intact | ### Any related issues, documentation, discussions? Closes #5717 ### How was this PR tested? Test-only PR. All checks below were run locally from the repository root and pass: 1. **Unit tests** — the new and modified specs: ``` sbt "WorkflowCore/testOnly org.apache.texera.amber.util.serde.GlobalPortIdentitySerdeSpec org.apache.texera.amber.util.serde.PortIdentitySerdeSpec" ``` Result: **31 tests, 0 failures** (1 `pendingUntilFixed`, pre-existing in `PortIdentitySerdeSpec`). 2. **Lint (scalafix)** — `sbt "scalafixAll --check"` → success. 3. **Format (scalafmt)** — `sbt scalafmtCheckAll` → success. ### Was this PR authored or co-authored using generative AI tooling? Co-authored by: Claude Opus 4.8 --- ...pec.scala => GlobalPortIdentitySerdeSpec.scala} | 212 +++++++++------------ .../amber/util/serde/PortIdentitySerdeSpec.scala | 190 +----------------- 2 files changed, 91 insertions(+), 311 deletions(-) diff --git a/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/GlobalPortIdentitySerdeSpec.scala similarity index 56% copy from common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala copy to common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/GlobalPortIdentitySerdeSpec.scala index 89a815c39e..2dc455e32a 100644 --- a/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala +++ b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/GlobalPortIdentitySerdeSpec.scala @@ -21,15 +21,11 @@ package org.apache.texera.amber.util.serde import org.apache.texera.amber.core.virtualidentity.{OperatorIdentity, PhysicalOpIdentity} import org.apache.texera.amber.core.workflow.{GlobalPortIdentity, PortIdentity} -import org.apache.texera.amber.util.JSONUtils.objectMapper import org.apache.texera.amber.util.serde.GlobalPortIdentitySerde.SerdeOps import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers -class PortIdentitySerdeSpec extends AnyFlatSpec { - - // --------------------------------------------------------------------------- - // GlobalPortIdentitySerde - // --------------------------------------------------------------------------- +class GlobalPortIdentitySerdeSpec extends AnyFlatSpec with Matchers { private def globalPort( logical: String = "op-A", @@ -47,7 +43,7 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { "GlobalPortIdentitySerde" should "round-trip a default GlobalPortIdentity through serializeAsString → deserializeFromString" in { val original = globalPort() val restored = GlobalPortIdentitySerde.deserializeFromString(original.serializeAsString) - assert(restored == original) + restored shouldBe original } it should "preserve all five fields independently across the round-trip" in { @@ -69,7 +65,7 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { cases.foreach { p => val s = p.serializeAsString val restored = GlobalPortIdentitySerde.deserializeFromString(s) - assert(restored == p, s"round-trip mismatch for $p (serialized: $s)") + restored shouldBe p } } @@ -77,20 +73,16 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { // Pin the exact format. If this changes, callers reading existing // VFS URIs from disk will break — locking it down forces a deliberate // migration story. - assert( - globalPort().serializeAsString == - "(logicalOpId=op-A,layerName=main,portId=0,isInternal=false,isInput=true)" - ) - assert( - globalPort( - logical = "op-Z", - layer = "extra-layer", - portIdValue = 7, - internal = true, - input = false - ).serializeAsString == - "(logicalOpId=op-Z,layerName=extra-layer,portId=7,isInternal=true,isInput=false)" - ) + globalPort().serializeAsString shouldBe + "(logicalOpId=op-A,layerName=main,portId=0,isInternal=false,isInput=true)" + globalPort( + logical = "op-Z", + layer = "extra-layer", + portIdValue = 7, + internal = true, + input = false + ).serializeAsString shouldBe + "(logicalOpId=op-Z,layerName=extra-layer,portId=7,isInternal=true,isInput=false)" } it should "round-trip identifiers containing dashes and dots (regex non-comma matcher)" in { @@ -100,7 +92,7 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { // both; if the regex were ever tightened to alphanumerics only, this // would fail on purpose. val p = globalPort(logical = "my.op-with-dashes.v2", layer = "main-1") - assert(GlobalPortIdentitySerde.deserializeFromString(p.serializeAsString) == p) + GlobalPortIdentitySerde.deserializeFromString(p.serializeAsString) shouldBe p } it should "throw IllegalArgumentException when serializing a negative port id" in { @@ -147,13 +139,6 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { } } - it should "throw IllegalArgumentException on a completely malformed string" in { - val ex = intercept[IllegalArgumentException] { - GlobalPortIdentitySerde.deserializeFromString("not even close") - } - assert(ex.getMessage.contains("not even close")) - } - it should "throw IllegalArgumentException when a required field is missing" in { // Drop isInput. val malformed = "(logicalOpId=op-A,layerName=main,portId=0,isInternal=false)" @@ -173,7 +158,7 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { } it should "throw IllegalArgumentException when a boolean field is non-boolean" in { - // `String.toBoolean` is strict: only \"true\" / \"false\" (case-insensitive) + // `String.toBoolean` is strict: only "true" / "false" (case-insensitive) // pass; anything else throws IllegalArgumentException. val malformed = "(logicalOpId=op-A,layerName=main,portId=0,isInternal=maybe,isInput=true)" intercept[IllegalArgumentException] { @@ -181,18 +166,22 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { } } + it should "throw IllegalArgumentException on a completely malformed string" in { + val ex = intercept[IllegalArgumentException] { + GlobalPortIdentitySerde.deserializeFromString("not even close") + } + ex.getMessage should include("not even close") + } + it should "use no underscore in its own format characters (separators / keys)" in { // Pin the format-character invariant: the wrapping `(...)`, the field // separators `,`, the key=value separators, and the field NAMES - // themselves contain no underscore. Verify by building the format with - // empty-string-replacement values for every input field, so anything - // left in the output is purely from `serializeAsString`'s own format. - // (For the layerName field the empty-input variant is rejected by the - // deserializer regex; here we only check the SERIALIZED output, not the - // round-trip.) + // themselves contain no underscore. Verify by stripping the input + // field values, so anything left in the output is purely from + // `serializeAsString`'s own format. val s = globalPort(logical = "x", layer = "x").serializeAsString val formatChars = s.replace("x", "").replace("0", "").replace("false", "").replace("true", "") - assert(!formatChars.contains("_"), s"format characters must be underscore-free: $formatChars") + formatChars should not include "_" } it should "throw IllegalArgumentException when logicalOpId contains an underscore" in { @@ -214,107 +203,86 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { } // --------------------------------------------------------------------------- - // PortIdentityKeySerializer.portIdToString (companion, not the Jackson class) - // --------------------------------------------------------------------------- - - "PortIdentityKeySerializer.portIdToString" should "format a PortIdentity as `id_internal`" in { - assert(PortIdentityKeySerializer.portIdToString(PortIdentity(0, internal = false)) == "0_false") - assert(PortIdentityKeySerializer.portIdToString(PortIdentity(7, internal = true)) == "7_true") - } - - // --------------------------------------------------------------------------- - // PortIdentityKeySerializer + PortIdentityKeyDeserializer (Jackson wiring) + // Edge cases // --------------------------------------------------------------------------- - // - // These tests use the production `JSONUtils.objectMapper` directly so a - // regression in the singleton wiring (e.g. the module that registers the - // PortIdentity key (de)serializer being removed or reordered) surfaces - // here, not just on a freshly-constructed mapper. - - "PortIdentity Jackson key (de)serialization" should "round-trip a Map[PortIdentity, String] via JSONUtils.objectMapper" in { - val original = Map( - PortIdentity(0, internal = false) -> "a", - PortIdentity(1, internal = true) -> "b" - ) - val json = objectMapper.writeValueAsString(original) - // Verify the JSON keys match the documented `id_internal` format. - assert(json.contains("\"0_false\"")) - assert(json.contains("\"1_true\"")) - val tref = objectMapper.getTypeFactory - .constructMapType(classOf[java.util.HashMap[_, _]], classOf[PortIdentity], classOf[String]) - val restored: java.util.Map[PortIdentity, String] = objectMapper.readValue(json, tref) - import scala.jdk.CollectionConverters._ - assert(restored.asScala.toMap == original) - } - - it should "round-trip an empty Map[PortIdentity, V] without invoking the (de)serializer" in { - val original = Map.empty[PortIdentity, String] - val json = objectMapper.writeValueAsString(original) - val tref = objectMapper.getTypeFactory - .constructMapType(classOf[java.util.HashMap[_, _]], classOf[PortIdentity], classOf[String]) - val restored: java.util.Map[PortIdentity, String] = objectMapper.readValue(json, tref) - assert(restored.isEmpty) - } - "PortIdentityKeyDeserializer.deserializeKey" should "throw NumberFormatException for a non-integer id" in { - val d = new PortIdentityKeyDeserializer - intercept[NumberFormatException] { - d.deserializeKey("notAnInt_false", null) - } + it should "accept underscores on deserialize even though serialize rejects them (serialize/deserialize asymmetry)" in { + // The deserializer regex `[^,]+` does not reject underscores, so a + // hand-crafted string with underscored logicalOpId / layerName parses + // fine — unlike the serializer, which rejects them at the boundary. + // Characterize this asymmetry so a future tightening of the regex + // breaks this test deliberately. + val s = "(logicalOpId=op_A,layerName=main_layer,portId=0,isInternal=false,isInput=true)" + val restored = GlobalPortIdentitySerde.deserializeFromString(s) + restored.opId.logicalOpId.id shouldBe "op_A" + restored.opId.layerName shouldBe "main_layer" } - it should "throw IllegalArgumentException for a non-boolean internal flag" in { - val d = new PortIdentityKeyDeserializer + it should "fail to round-trip a logicalOpId containing a comma (separators are not escaped)" in { + // The format does not escape its `,` separator, so a logicalOpId with + // an embedded comma serializes into a string the deserializer can no + // longer parse back into the same value. + val s = globalPort(logical = "op,A").serializeAsString intercept[IllegalArgumentException] { - d.deserializeKey("0_notABool", null) + GlobalPortIdentitySerde.deserializeFromString(s) } } - it should "throw NumberFormatException when the underscore separator is missing and the whole string is non-numeric" in { - // `key.split("_")` on a separator-less non-numeric string yields a - // single-element array, and `parts(0).toInt` fires first → NFE. - val d = new PortIdentityKeyDeserializer + it should "round-trip Int.MaxValue but reject an out-of-Int-range portId on deserialize" in { + // Upper boundary of the portId domain round-trips intact. + val p = globalPort(portIdValue = Int.MaxValue) + GlobalPortIdentitySerde.deserializeFromString(p.serializeAsString) shouldBe p + // A value past Int range fails in `.toInt` with NumberFormatException. + val overflow = + "(logicalOpId=op-A,layerName=main,portId=9999999999,isInternal=false,isInput=true)" intercept[NumberFormatException] { - d.deserializeKey("missingSeparator", null) + GlobalPortIdentitySerde.deserializeFromString(overflow) } } - it should "throw ArrayIndexOutOfBoundsException when only the id is provided (no `_internal` suffix)" in { - // Different separator-missing path: `\"5\".split(\"_\")` yields - // [\"5\"], parts(0).toInt = 5 succeeds, then parts(1) reads past the - // end. Pin this failure mode explicitly so a future safer parser - // breaks the spec on purpose (and the safer error type is chosen - // consciously). - val d = new PortIdentityKeyDeserializer - intercept[ArrayIndexOutOfBoundsException] { - d.deserializeKey("5", null) - } + it should "accept mixed-case boolean fields on deserialize (String.toBoolean is case-insensitive)" in { + // `String.toBoolean` accepts any case variant of true/false, so a + // tampered/legacy string with `True` / `FALSE` still parses. + val s = "(logicalOpId=op-A,layerName=main,portId=0,isInternal=True,isInput=FALSE)" + val restored = GlobalPortIdentitySerde.deserializeFromString(s) + restored.portId.internal shouldBe true + restored.input shouldBe false } - it should "silently accept extra trailing underscore-separated segments (lenient parser, current behavior)" in { - // Pin the current lenient behavior: `parts(0).toInt` and - // `parts(1).toBoolean` ignore everything past `parts(1)`, so a key - // like `"1_true_garbage"` deserializes to `PortIdentity(1, true)` - // without complaint. The strict-rejection variant lives in a - // pendingUntilFixed test below; characterizing today's lenient - // path here means a future-tightening fix would need to update - // both tests deliberately. - val d = new PortIdentityKeyDeserializer - val pid = d.deserializeKey("1_true_garbage", null) - assert(pid == PortIdentity(1, internal = true)) + it should "round-trip a value containing '=' (only ',' is a sensitive separator)" in { + // Counterpart to the comma case above: the deserializer anchors on the + // literal `logicalOpId=` prefix and then captures up to the next comma + // (`[^,]+`), so an embedded `=` is harmless and round-trips intact. + // Pin this so a future regex change that special-cased `=` would fail. + val p = globalPort(logical = "a=b") + GlobalPortIdentitySerde.deserializeFromString(p.serializeAsString) shouldBe p } - it should "eventually reject keys with extra trailing segments (pendingUntilFixed)" in pendingUntilFixed { - // Documented contract: a `PortIdentityKeySerializer` output is exactly - // `id_internal` — two underscore-separated segments. Anything else is - // corrupt JSON and should be rejected, not silently truncated. The - // current implementation is lenient (see characterization test - // above); this pendingUntilFixed flips to passing once the parser - // is hardened, then `pendingUntilFixed` inverts that into a - // deliberate failure forcing the marker to be removed. - val d = new PortIdentityKeyDeserializer + it should "serialize an empty logicalOpId but fail to deserialize the result (serialize does not guard emptiness)" in { + // The serializer only rejects underscores and negative portIds, not empty + // identifiers, so an empty logicalOpId serializes into `(logicalOpId=,...)`. + // The deserializer's `[^,]+` requires at least one character, so that + // output can no longer be parsed back — the serialize side of the same + // asymmetry the "empty field body" deserialize test characterizes. + val s = globalPort(logical = "").serializeAsString intercept[IllegalArgumentException] { - d.deserializeKey("1_true_garbage", null) + GlobalPortIdentitySerde.deserializeFromString(s) } } + + it should "name the offending field in the require failure message" in { + // The three serialize-side guards throw IllegalArgumentException; assert + // the message identifies which field failed so a regression that wires a + // check to the wrong field (e.g. validating logicalOpId in layerName's + // guard) is caught instead of silently still throwing. + intercept[IllegalArgumentException] { + globalPort(logical = "__x").serializeAsString + }.getMessage should include("logicalOpId") + intercept[IllegalArgumentException] { + globalPort(layer = "a_b").serializeAsString + }.getMessage should include("layerName") + intercept[IllegalArgumentException] { + globalPort(portIdValue = -1).serializeAsString + }.getMessage should include("portId") + } } diff --git a/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala index 89a815c39e..e7f6ad037b 100644 --- a/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala +++ b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala @@ -19,200 +19,12 @@ package org.apache.texera.amber.util.serde -import org.apache.texera.amber.core.virtualidentity.{OperatorIdentity, PhysicalOpIdentity} -import org.apache.texera.amber.core.workflow.{GlobalPortIdentity, PortIdentity} +import org.apache.texera.amber.core.workflow.PortIdentity import org.apache.texera.amber.util.JSONUtils.objectMapper -import org.apache.texera.amber.util.serde.GlobalPortIdentitySerde.SerdeOps import org.scalatest.flatspec.AnyFlatSpec class PortIdentitySerdeSpec extends AnyFlatSpec { - // --------------------------------------------------------------------------- - // GlobalPortIdentitySerde - // --------------------------------------------------------------------------- - - private def globalPort( - logical: String = "op-A", - layer: String = "main", - portIdValue: Int = 0, - internal: Boolean = false, - input: Boolean = true - ): GlobalPortIdentity = - GlobalPortIdentity( - opId = PhysicalOpIdentity(OperatorIdentity(logical), layer), - portId = PortIdentity(id = portIdValue, internal = internal), - input = input - ) - - "GlobalPortIdentitySerde" should "round-trip a default GlobalPortIdentity through serializeAsString → deserializeFromString" in { - val original = globalPort() - val restored = GlobalPortIdentitySerde.deserializeFromString(original.serializeAsString) - assert(restored == original) - } - - it should "preserve all five fields independently across the round-trip" in { - // Vary each field individually so a regression that swapped two fields - // (e.g., isInput / isInternal) would surface here, not as a general - // round-trip failure. - val cases = Seq( - globalPort(logical = "op-A"), - globalPort(logical = "op-Z"), - globalPort(layer = "main"), - globalPort(layer = "extra-layer"), - globalPort(portIdValue = 0), - globalPort(portIdValue = 7), - globalPort(internal = false), - globalPort(internal = true), - globalPort(input = true), - globalPort(input = false) - ) - cases.foreach { p => - val s = p.serializeAsString - val restored = GlobalPortIdentitySerde.deserializeFromString(s) - assert(restored == p, s"round-trip mismatch for $p (serialized: $s)") - } - } - - it should "produce the documented format for default and non-default values" in { - // Pin the exact format. If this changes, callers reading existing - // VFS URIs from disk will break — locking it down forces a deliberate - // migration story. - assert( - globalPort().serializeAsString == - "(logicalOpId=op-A,layerName=main,portId=0,isInternal=false,isInput=true)" - ) - assert( - globalPort( - logical = "op-Z", - layer = "extra-layer", - portIdValue = 7, - internal = true, - input = false - ).serializeAsString == - "(logicalOpId=op-Z,layerName=extra-layer,portId=7,isInternal=true,isInput=false)" - ) - } - - it should "round-trip identifiers containing dashes and dots (regex non-comma matcher)" in { - // The deserialization regex uses `[^,]+` for the field body, so any - // non-comma character is fair game. Cover the realistic counter- - // examples (dashes, dots) since logical op ids and layer names use - // both; if the regex were ever tightened to alphanumerics only, this - // would fail on purpose. - val p = globalPort(logical = "my.op-with-dashes.v2", layer = "main-1") - assert(GlobalPortIdentitySerde.deserializeFromString(p.serializeAsString) == p) - } - - it should "throw IllegalArgumentException when serializing a negative port id" in { - // Port ids are array indices and must be non-negative; the serializer - // rejects negatives so corrupt data can't reach VFS URIs. - intercept[IllegalArgumentException] { - globalPort(portIdValue = -1).serializeAsString - } - } - - it should "throw IllegalArgumentException when deserializing a negative port id" in { - // Symmetric: a hand-crafted string with a negative portId must be - // rejected by the deserializer too (so tampered URIs don't slip - // through). - val malformed = "(logicalOpId=op-A,layerName=main,portId=-1,isInternal=false,isInput=true)" - intercept[IllegalArgumentException] { - GlobalPortIdentitySerde.deserializeFromString(malformed) - } - } - - it should "throw IllegalArgumentException when the input has the wrong field order" in { - // The regex pins the documented field order; a swapped order should - // not silently parse with confused values. - val swapped = "(layerName=main,logicalOpId=op-A,portId=0,isInternal=false,isInput=true)" - intercept[IllegalArgumentException] { - GlobalPortIdentitySerde.deserializeFromString(swapped) - } - } - - it should "throw IllegalArgumentException when the input has trailing content past the closing paren" in { - val withTrailing = - "(logicalOpId=op-A,layerName=main,portId=0,isInternal=false,isInput=true) extra" - intercept[IllegalArgumentException] { - GlobalPortIdentitySerde.deserializeFromString(withTrailing) - } - } - - it should "throw IllegalArgumentException when a field body is empty" in { - // `[^,]+` requires at least one character, so an empty layerName - // (`,layerName=,`) must fail to match. - val emptyLayer = "(logicalOpId=op-A,layerName=,portId=0,isInternal=false,isInput=true)" - intercept[IllegalArgumentException] { - GlobalPortIdentitySerde.deserializeFromString(emptyLayer) - } - } - - it should "throw IllegalArgumentException on a completely malformed string" in { - val ex = intercept[IllegalArgumentException] { - GlobalPortIdentitySerde.deserializeFromString("not even close") - } - assert(ex.getMessage.contains("not even close")) - } - - it should "throw IllegalArgumentException when a required field is missing" in { - // Drop isInput. - val malformed = "(logicalOpId=op-A,layerName=main,portId=0,isInternal=false)" - intercept[IllegalArgumentException] { - GlobalPortIdentitySerde.deserializeFromString(malformed) - } - } - - it should "throw NumberFormatException when portId is non-numeric" in { - // The regex matches (`[^,]+`) but `.toInt` fails. NumberFormatException - // extends IllegalArgumentException; assert the more specific type so a - // regression that swallowed/rewrapped it is visible. - val malformed = "(logicalOpId=op-A,layerName=main,portId=NaN,isInternal=false,isInput=true)" - intercept[NumberFormatException] { - GlobalPortIdentitySerde.deserializeFromString(malformed) - } - } - - it should "throw IllegalArgumentException when a boolean field is non-boolean" in { - // `String.toBoolean` is strict: only \"true\" / \"false\" (case-insensitive) - // pass; anything else throws IllegalArgumentException. - val malformed = "(logicalOpId=op-A,layerName=main,portId=0,isInternal=maybe,isInput=true)" - intercept[IllegalArgumentException] { - GlobalPortIdentitySerde.deserializeFromString(malformed) - } - } - - it should "use no underscore in its own format characters (separators / keys)" in { - // Pin the format-character invariant: the wrapping `(...)`, the field - // separators `,`, the key=value separators, and the field NAMES - // themselves contain no underscore. Verify by building the format with - // empty-string-replacement values for every input field, so anything - // left in the output is purely from `serializeAsString`'s own format. - // (For the layerName field the empty-input variant is rejected by the - // deserializer regex; here we only check the SERIALIZED output, not the - // round-trip.) - val s = globalPort(logical = "x", layer = "x").serializeAsString - val formatChars = s.replace("x", "").replace("0", "").replace("false", "").replace("true", "") - assert(!formatChars.contains("_"), s"format characters must be underscore-free: $formatChars") - } - - it should "throw IllegalArgumentException when logicalOpId contains an underscore" in { - // Enforces the documented VFS-compatibility contract: the serialized - // form must be underscore-free. The serializer rejects underscored - // inputs at the boundary instead of silently emitting a string that - // would interfere with VFS URI parsing downstream. - intercept[IllegalArgumentException] { - globalPort(logical = "__DummyOperator").serializeAsString - } - } - - it should "throw IllegalArgumentException when layerName contains an underscore" in { - // Both fields enforce the same invariant; cover them independently so - // a partial fix that only validates one surfaces as a test failure. - intercept[IllegalArgumentException] { - globalPort(layer = "main_source_0_op").serializeAsString - } - } - // --------------------------------------------------------------------------- // PortIdentityKeySerializer.portIdToString (companion, not the Jackson class) // ---------------------------------------------------------------------------
