This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 4722a42c01 test(workflow-core): cover PortIdentity key serde (#5799)
4722a42c01 is described below
commit 4722a42c019a35790134e660d9becc6ea069205f
Author: Tanishq Gandhi <[email protected]>
AuthorDate: Fri Jun 19 23:24:42 2026 -0700
test(workflow-core): cover PortIdentity key serde (#5799)
### What changes were proposed in this PR?
Add dedicated unit specs for `PortIdentity` JSON map-key serde:
| Spec | Coverage |
| --- | --- |
| `PortIdentityKeySerializerSpec` | Pins the exact `id_internal` key
format and Jackson map-key serialization |
| `PortIdentityKeyDeserializerSpec` | Pins valid key parsing,
serializer/deserializer round-trips, Jackson map round-trip, and current
malformed-key failure behavior |
### Any related issues, documentation, discussions?
Closes #5776
### How was this PR tested?
Added unit tests and ran:
```bash
sbt "project WorkflowCore" "testOnly
org.apache.texera.amber.util.serde.PortIdentityKeySerializerSpec
org.apache.texera.amber.util.serde.PortIdentityKeyDeserializerSpec"
```
Result: all 12 tests passed.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-5)
---
.../serde/PortIdentityKeyDeserializerSpec.scala | 129 ++++++++++++++++++++
.../util/serde/PortIdentityKeySerializerSpec.scala | 63 ++++++++++
.../amber/util/serde/PortIdentitySerdeSpec.scala | 132 ---------------------
3 files changed, 192 insertions(+), 132 deletions(-)
diff --git
a/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentityKeyDeserializerSpec.scala
b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentityKeyDeserializerSpec.scala
new file mode 100644
index 0000000000..427239aa2c
--- /dev/null
+++
b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentityKeyDeserializerSpec.scala
@@ -0,0 +1,129 @@
+/*
+ * 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 org.apache.texera.amber.core.workflow.PortIdentity
+import org.apache.texera.amber.util.JSONUtils.objectMapper
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import scala.jdk.CollectionConverters._
+
+class PortIdentityKeyDeserializerSpec extends AnyFlatSpec with Matchers {
+
+ private val deserializer = new PortIdentityKeyDeserializer
+
+ "PortIdentityKeyDeserializer.deserializeKey" should "parse id_internal keys
into PortIdentity" in {
+ deserializer.deserializeKey("3_false", null) shouldBe PortIdentity(3,
internal = false)
+ deserializer.deserializeKey("0_true", null) shouldBe PortIdentity(0,
internal = true)
+ }
+
+ it should "round-trip serializer output for representative PortIdentity
values" in {
+ val cases = Seq(
+ PortIdentity(0, internal = false),
+ PortIdentity(0, internal = true),
+ PortIdentity(999999, internal = false),
+ PortIdentity(999999, internal = true),
+ PortIdentity(-1, internal = false),
+ PortIdentity(-1, internal = true)
+ )
+
+ cases.foreach { portIdentity =>
+ deserializer.deserializeKey(
+ PortIdentityKeySerializer.portIdToString(portIdentity),
+ null
+ ) shouldBe portIdentity
+ }
+ }
+
+ it should "round-trip Map[PortIdentity, String] keys through
JSONUtils.objectMapper" in {
+ val original = Map(
+ PortIdentity(0, internal = false) -> "zero-external",
+ PortIdentity(3, internal = true) -> "three-internal",
+ PortIdentity(999999, internal = false) -> "large-external",
+ PortIdentity(-1, internal = true) -> "negative-internal"
+ )
+ val json = objectMapper.writeValueAsString(original)
+ val mapType = objectMapper.getTypeFactory
+ .constructMapType(classOf[java.util.HashMap[_, _]],
classOf[PortIdentity], classOf[String])
+
+ val restored: java.util.Map[PortIdentity, String] =
objectMapper.readValue(json, mapType)
+
+ restored.asScala.toMap shouldBe original
+ }
+
+ it should "round-trip an empty Map[PortIdentity, String] through
JSONUtils.objectMapper" in {
+ val original = Map.empty[PortIdentity, String]
+ val json = objectMapper.writeValueAsString(original)
+ val mapType = objectMapper.getTypeFactory
+ .constructMapType(classOf[java.util.HashMap[_, _]],
classOf[PortIdentity], classOf[String])
+
+ val restored: java.util.Map[PortIdentity, String] =
objectMapper.readValue(json, mapType)
+
+ restored.asScala.toMap shouldBe original
+ }
+
+ it should "throw NumberFormatException for a non-integer id" in {
+ intercept[NumberFormatException] {
+ deserializer.deserializeKey("notAnInt_false", null)
+ }
+ }
+
+ it should "throw IllegalArgumentException for a non-boolean internal flag"
in {
+ intercept[IllegalArgumentException] {
+ deserializer.deserializeKey("0_notABool", null)
+ }
+ }
+
+ it should "throw NumberFormatException when a separator-less key is
non-numeric" in {
+ intercept[NumberFormatException] {
+ deserializer.deserializeKey("missingSeparator", null)
+ }
+ }
+
+ it should "throw NumberFormatException for an empty key" in {
+ intercept[NumberFormatException] {
+ deserializer.deserializeKey("", null)
+ }
+ }
+
+ it should "throw ArrayIndexOutOfBoundsException when only the id is
provided" in {
+ intercept[ArrayIndexOutOfBoundsException] {
+ deserializer.deserializeKey("5", null)
+ }
+ }
+
+ it should "ignore extra trailing underscore-separated segments" in {
+ deserializer.deserializeKey("1_true_extra", null) shouldBe 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.
+ intercept[IllegalArgumentException] {
+ deserializer.deserializeKey("1_true_extra", null)
+ }
+ }
+}
diff --git
a/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentityKeySerializerSpec.scala
b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentityKeySerializerSpec.scala
new file mode 100644
index 0000000000..31ba71c3c6
--- /dev/null
+++
b/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentityKeySerializerSpec.scala
@@ -0,0 +1,63 @@
+/*
+ * 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 org.apache.texera.amber.core.workflow.PortIdentity
+import org.apache.texera.amber.util.JSONUtils.objectMapper
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+class PortIdentityKeySerializerSpec extends AnyFlatSpec with Matchers {
+
+ "PortIdentityKeySerializer.portIdToString" should "format PortIdentity as
id_internal" in {
+ PortIdentityKeySerializer
+ .portIdToString(PortIdentity(3, internal = false)) shouldBe "3_false"
+ PortIdentityKeySerializer
+ .portIdToString(PortIdentity(0, internal = true)) shouldBe "0_true"
+ }
+
+ it should "preserve zero, large, negative, internal, and external ports in
the key string" in {
+ val cases = Seq(
+ PortIdentity(0, internal = false) -> "0_false",
+ PortIdentity(0, internal = true) -> "0_true",
+ PortIdentity(999999, internal = false) -> "999999_false",
+ PortIdentity(999999, internal = true) -> "999999_true",
+ PortIdentity(-1, internal = false) -> "-1_false",
+ PortIdentity(-1, internal = true) -> "-1_true"
+ )
+
+ cases.foreach {
+ case (portIdentity, expected) =>
+ PortIdentityKeySerializer.portIdToString(portIdentity) shouldBe
expected
+ }
+ }
+
+ "PortIdentityKeySerializer" should "write Jackson map keys with the
id_internal format" in {
+ val original = Map(
+ PortIdentity(3, internal = false) -> "external",
+ PortIdentity(0, internal = true) -> "internal"
+ )
+
+ val json = objectMapper.writeValueAsString(original)
+
+ json should include(""""3_false"""")
+ json should include(""""0_true"""")
+ }
+}
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
deleted file mode 100644
index e7f6ad037b..0000000000
---
a/common/workflow-core/src/test/scala/org/apache/texera/amber/util/serde/PortIdentitySerdeSpec.scala
+++ /dev/null
@@ -1,132 +0,0 @@
-/*
- * 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 org.apache.texera.amber.core.workflow.PortIdentity
-import org.apache.texera.amber.util.JSONUtils.objectMapper
-import org.scalatest.flatspec.AnyFlatSpec
-
-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)
- //
---------------------------------------------------------------------------
- //
- // 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 "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)
- }
- }
-
- 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)
- }
- }
-}
