From 01614a202fa217df555c8895138b92d66da64346 Mon Sep 17 00:00:00 2001 From: Xinyuan Lin Date: Tue, 5 May 2026 15:21:16 -0700 Subject: [PATCH 1/3] test(workflow-core): add unit test coverage for port-identity serde helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin the wiring for the three serde helpers in `org.apache.texera.amber.util.serde`: - `GlobalPortIdentitySerde.serializeAsString` / `deserializeFromString`: default + per-field round-trip, exact format pin (default and non-default values), special-character pass-through (dashes, dots), negative `portId`, no-underscore invariant for VFS-URI compatibility, and seven negative paths (malformed, missing field, wrong order, trailing content, empty body, non-numeric portId → `NumberFormatException`, non-boolean flag). - `PortIdentityKeySerializer.portIdToString` exact `id_internal` format. - The Jackson serializer + deserializer wired into a real `ObjectMapper` (mirroring `JSONUtils.objectMapper` setup) round-tripping a `Map[PortIdentity, String]` and an empty map. - `PortIdentityKeyDeserializer.deserializeKey` happy-path coverage via the Map round-trip plus four negative paths (non-integer id, non-boolean flag, missing separator with non-numeric body, missing separator with numeric-only body). Closes #4953 --- .../util/serde/PortIdentitySerdeSpec.scala | 274 ++++++++++++++++++ 1 file changed, 274 insertions(+) create mode 100644 common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala 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 new file mode 100644 index 00000000000..51a5e7ddf89 --- /dev/null +++ b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala @@ -0,0 +1,274 @@ +/* + * 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.texera.amber.util.serde + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.module.SimpleModule +import com.fasterxml.jackson.module.scala.DefaultScalaModule +import org.apache.texera.amber.core.virtualidentity.{OperatorIdentity, PhysicalOpIdentity} +import org.apache.texera.amber.core.workflow.{GlobalPortIdentity, PortIdentity} +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 "round-trip a negative port id" in { + // PortIdentity.id is a plain Int; negatives are technically permitted + // by the type. Pin the round-trip so a future tightening of the + // numeric regex (e.g. to `\\d+`) breaks this on purpose. + val p = globalPort(portIdValue = -1) + assert(GlobalPortIdentitySerde.deserializeFromString(p.serializeAsString) == p) + } + + 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 "produce a string with no underscore (compatibility with VFS URI parsing)" in { + // The custom format exists specifically to avoid the underscore separator + // used by `PortIdentityKeySerializer.portIdToString`, which would clash + // with the VFS URI parser. Pin the no-underscore invariant for every + // field combination — including a layerName containing dashes, which + // is the realistic counter-example most likely to slip in. + val s = globalPort(logical = "my-op", layer = "extra-layer", internal = true).serializeAsString + assert(!s.contains("_"), s"serialized form must be underscore-free: $s") + } + + // --------------------------------------------------------------------------- + // 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) + // --------------------------------------------------------------------------- + // + // These tests use a real ObjectMapper with the serde pair registered as a + // module — same wiring JSONUtils.objectMapper performs in production — + // so a regression in either direction surfaces here. + + private def newMapper(): ObjectMapper = { + val m = new ObjectMapper() + m.registerModule(DefaultScalaModule) + val mod = new SimpleModule() + mod.addKeySerializer(classOf[PortIdentity], new PortIdentityKeySerializer()) + mod.addKeyDeserializer(classOf[PortIdentity], new PortIdentityKeyDeserializer()) + m.registerModule(mod) + m + } + + "PortIdentity Jackson key (de)serialization" should "round-trip a Map[PortIdentity, String] via the registered module" in { + val mapper = newMapper() + val original = Map( + PortIdentity(0, internal = false) -> "a", + PortIdentity(1, internal = true) -> "b" + ) + val json = mapper.writeValueAsString(original) + // Verify the JSON keys match the documented `id_internal` format. + assert(json.contains("\"0_false\"")) + assert(json.contains("\"1_true\"")) + val tref = mapper.getTypeFactory + .constructMapType(classOf[java.util.HashMap[_, _]], classOf[PortIdentity], classOf[String]) + val restored: java.util.Map[PortIdentity, String] = mapper.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 mapper = newMapper() + val original = Map.empty[PortIdentity, String] + val json = mapper.writeValueAsString(original) + val tref = mapper.getTypeFactory + .constructMapType(classOf[java.util.HashMap[_, _]], classOf[PortIdentity], classOf[String]) + val restored: java.util.Map[PortIdentity, String] = mapper.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 "throw IllegalArgumentException for a non-boolean internal flag" in { + val d = new PortIdentityKeyDeserializer + intercept[IllegalArgumentException] { + d.deserializeKey("0_notABool", null) + } + } + + 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 + intercept[NumberFormatException] { + d.deserializeKey("missingSeparator", null) + } + } + + 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) + } + } +} From 491766bd51f8a32d21fd9eacb104d0cc2ce1f8b4 Mon Sep 17 00:00:00 2001 From: Xinyuan Lin Date: Tue, 5 May 2026 15:44:07 -0700 Subject: [PATCH 2/3] test(workflow-core): use JSONUtils.objectMapper directly + fix underscore-invariant test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot feedback on #4954: - Switch the Jackson key (de)serialization tests to use the production `JSONUtils.objectMapper` singleton instead of constructing a fresh `ObjectMapper` + `SimpleModule` locally. The fresh-mapper version would still pass if the singleton stopped registering the PortIdentity key serializer/deserializer module — exactly the kind of regression these tests are meant to catch. - Replace the misleading "no underscore" test, which only used underscore-free inputs (so it could not catch the real failure mode flagged in the Copilot comment: an op id like `OperatorIdentity("__DummyOperator")` from `VirtualIdentityUtils`). The new spec now has two tests: - A plain test pinning that the FORMAT CHARACTERS introduce no underscore (computed by stripping the input values out of the output and checking the residue). - A `pendingUntilFixed` test asserting the fuller documented contract — output is underscore-free even when the input contains underscores. Today this test is pending because the implementation interpolates inputs verbatim; a future fix that escapes/replaces underscores will flip it to passing, and `pendingUntilFixed` then inverts that into a deliberate failure forcing the marker to be removed. --- .../util/serde/PortIdentitySerdeSpec.scala | 70 ++++++++++--------- 1 file changed, 38 insertions(+), 32 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/PortIdentitySerdeSpec.scala index 51a5e7ddf89..7d3f9fc9e54 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,11 +19,9 @@ package org.apache.texera.amber.util.serde -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.databind.module.SimpleModule -import com.fasterxml.jackson.module.scala.DefaultScalaModule 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 @@ -173,13 +171,32 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { } } - it should "produce a string with no underscore (compatibility with VFS URI parsing)" in { - // The custom format exists specifically to avoid the underscore separator - // used by `PortIdentityKeySerializer.portIdToString`, which would clash - // with the VFS URI parser. Pin the no-underscore invariant for every - // field combination — including a layerName containing dashes, which - // is the realistic counter-example most likely to slip in. - val s = globalPort(logical = "my-op", layer = "extra-layer", internal = true).serializeAsString + 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 "eventually produce an underscore-free output even for inputs that contain underscores (pendingUntilFixed)" in pendingUntilFixed { + // Documented contract on `GlobalPortIdentitySerde`: "does not include + // underscore '_' so that it does not interfere with our own VFS URI + // parsing." The implementation does NOT enforce this — inputs are + // interpolated verbatim, so an op-id like + // `OperatorIdentity("__DummyOperator")` (which is a real identifier + // used in `VirtualIdentityUtils`) produces an underscore-bearing + // output. Pin the documented invariant here so that the future fix + // that escapes / replaces underscores on the serialize side flips + // this from pending to passing, and pendingUntilFixed inverts that + // into a deliberate failure forcing the marker to be deleted. + val s = globalPort(logical = "__DummyOperator").serializeAsString assert(!s.contains("_"), s"serialized form must be underscore-free: $s") } @@ -196,44 +213,33 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { // PortIdentityKeySerializer + PortIdentityKeyDeserializer (Jackson wiring) // --------------------------------------------------------------------------- // - // These tests use a real ObjectMapper with the serde pair registered as a - // module — same wiring JSONUtils.objectMapper performs in production — - // so a regression in either direction surfaces here. - - private def newMapper(): ObjectMapper = { - val m = new ObjectMapper() - m.registerModule(DefaultScalaModule) - val mod = new SimpleModule() - mod.addKeySerializer(classOf[PortIdentity], new PortIdentityKeySerializer()) - mod.addKeyDeserializer(classOf[PortIdentity], new PortIdentityKeyDeserializer()) - m.registerModule(mod) - m - } + // 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 the registered module" in { - val mapper = newMapper() + "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 = mapper.writeValueAsString(original) + 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 = mapper.getTypeFactory + val tref = objectMapper.getTypeFactory .constructMapType(classOf[java.util.HashMap[_, _]], classOf[PortIdentity], classOf[String]) - val restored: java.util.Map[PortIdentity, String] = mapper.readValue(json, tref) + 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 mapper = newMapper() val original = Map.empty[PortIdentity, String] - val json = mapper.writeValueAsString(original) - val tref = mapper.getTypeFactory + 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] = mapper.readValue(json, tref) + val restored: java.util.Map[PortIdentity, String] = objectMapper.readValue(json, tref) assert(restored.isEmpty) } From af5fb134bfe2336aeed4cf61dee6a83b1204f41c Mon Sep 17 00:00:00 2001 From: Xinyuan Lin Date: Tue, 5 May 2026 16:18:15 -0700 Subject: [PATCH 3/3] test(workflow-core): cover layerName underscore + lenient deserializeKey Address Copilot follow-up on #4954: - Underscore-invariant pendingUntilFixed test: previously only varied `logicalOpId`. Production also constructs underscore-bearing layerNames (e.g. SpecialPhysicalOpFactory builds `${layerName}_source_${portId}_${opId}`), so a partial fix that only escapes op-ids would have flipped the pending test to passing while leaving the layerName failure mode unhandled. Now exercises both fields independently in the same test. - PortIdentityKeyDeserializer extra-segments path: previously not pinned. `key.split("_")` only reads parts(0) and parts(1), so a malformed key like "1_true_garbage" silently deserializes to PortIdentity(1, true). Added two tests: - Characterization: pin the current lenient behavior. - pendingUntilFixed: assert the stricter contract (any extra trailing segment rejected). Future hardening flips the pending test to passing, then pendingUntilFixed inverts that into a deliberate failure forcing the marker to be removed. --- .../util/serde/PortIdentitySerdeSpec.scala | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 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/PortIdentitySerdeSpec.scala index 7d3f9fc9e54..3abc7962422 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 @@ -189,15 +189,24 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { // Documented contract on `GlobalPortIdentitySerde`: "does not include // underscore '_' so that it does not interfere with our own VFS URI // parsing." The implementation does NOT enforce this — inputs are - // interpolated verbatim, so an op-id like - // `OperatorIdentity("__DummyOperator")` (which is a real identifier - // used in `VirtualIdentityUtils`) produces an underscore-bearing - // output. Pin the documented invariant here so that the future fix - // that escapes / replaces underscores on the serialize side flips - // this from pending to passing, and pendingUntilFixed inverts that - // into a deliberate failure forcing the marker to be deleted. - val s = globalPort(logical = "__DummyOperator").serializeAsString - assert(!s.contains("_"), s"serialized form must be underscore-free: $s") + // interpolated verbatim. Both fields can carry underscores in real + // production data: + // - `logicalOpId`: e.g. `__DummyOperator` from `VirtualIdentityUtils` + // - `layerName`: e.g. `${layerName}_source_${portId}_...` constructed + // by `SpecialPhysicalOpFactory` + // Cover BOTH so a partial fix that escapes only one of them flips + // pendingUntilFixed into a deliberate failure with the second + // assertion still red. + val withUnderscoreOpId = globalPort(logical = "__DummyOperator").serializeAsString + assert( + !withUnderscoreOpId.contains("_"), + s"serialized form must be underscore-free for op id: $withUnderscoreOpId" + ) + val withUnderscoreLayer = globalPort(layer = "main_source_0_op").serializeAsString + assert( + !withUnderscoreLayer.contains("_"), + s"serialized form must be underscore-free for layer name: $withUnderscoreLayer" + ) } // --------------------------------------------------------------------------- @@ -277,4 +286,31 @@ class PortIdentitySerdeSpec extends AnyFlatSpec { d.deserializeKey("5", null) } } + + 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 "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 + intercept[IllegalArgumentException] { + d.deserializeKey("1_true_garbage", null) + } + } }