diff --git a/amber/src/main/scala/org/apache/texera/workflow/LogicalLink.scala b/amber/src/main/scala/org/apache/texera/workflow/LogicalLink.scala index e6553e3cdf1..dcb2189ed72 100644 --- a/amber/src/main/scala/org/apache/texera/workflow/LogicalLink.scala +++ b/amber/src/main/scala/org/apache/texera/workflow/LogicalLink.scala @@ -20,9 +20,41 @@ package org.apache.texera.workflow import com.fasterxml.jackson.annotation.{JsonCreator, JsonProperty} +import com.fasterxml.jackson.databind.JsonNode import org.apache.texera.amber.core.virtualidentity.OperatorIdentity import org.apache.texera.amber.core.workflow.PortIdentity +object LogicalLink { + + // Reads an OperatorIdentity from either the plain-string shape the + // frontend emits (`"op-A"`) or the object shape Jackson emits for an + // OperatorIdentity (`{"id":"op-A"}`). Keep in sync with the + // workflow-compiling-service LogicalLink.readOperatorIdentity, which is + // intentionally identical. + private def readOperatorIdentity(node: JsonNode, fieldName: String): OperatorIdentity = { + if (node == null || node.isNull) { + OperatorIdentity(null) + } else if (node.isTextual) { + OperatorIdentity(node.asText()) + } else if (node.isObject) { + val idNode = node.get("id") + if (idNode == null || idNode.isNull) { + OperatorIdentity(null) + } else if (idNode.isTextual) { + OperatorIdentity(idNode.asText()) + } else { + throw new IllegalArgumentException( + s"LogicalLink $fieldName.id must be a string or null, but was ${idNode.getNodeType}" + ) + } + } else { + throw new IllegalArgumentException( + s"LogicalLink $fieldName must be a string or an object, but was ${node.getNodeType}" + ) + } + } +} + case class LogicalLink( @JsonProperty("fromOpId") fromOpId: OperatorIdentity, fromPortId: PortIdentity, @@ -42,13 +74,27 @@ case class LogicalLink( s"LogicalLink self-loop not allowed: fromOpId == toOpId == ${fromOpId.id}" ) - @JsonCreator def this( - @JsonProperty("fromOpId") fromOpId: String, + fromOpId: String, fromPortId: PortIdentity, - @JsonProperty("toOpId") toOpId: String, + toOpId: String, toPortId: PortIdentity ) = { this(OperatorIdentity(fromOpId), fromPortId, OperatorIdentity(toOpId), toPortId) } + + @JsonCreator + def this( + @JsonProperty("fromOpId") fromOpId: JsonNode, + fromPortId: PortIdentity, + @JsonProperty("toOpId") toOpId: JsonNode, + toPortId: PortIdentity + ) = { + this( + LogicalLink.readOperatorIdentity(fromOpId, "fromOpId"), + fromPortId, + LogicalLink.readOperatorIdentity(toOpId, "toOpId"), + toPortId + ) + } } diff --git a/amber/src/test/scala/org/apache/texera/workflow/LogicalLinkSpec.scala b/amber/src/test/scala/org/apache/texera/workflow/LogicalLinkSpec.scala index bd56aa7d5f6..2ddbb1fc704 100644 --- a/amber/src/test/scala/org/apache/texera/workflow/LogicalLinkSpec.scala +++ b/amber/src/test/scala/org/apache/texera/workflow/LogicalLinkSpec.scala @@ -20,7 +20,7 @@ package org.apache.texera.workflow import com.fasterxml.jackson.databind.JsonNode -import com.fasterxml.jackson.databind.exc.{MismatchedInputException, ValueInstantiationException} +import com.fasterxml.jackson.databind.exc.ValueInstantiationException import org.apache.texera.amber.core.virtualidentity.OperatorIdentity import org.apache.texera.amber.core.workflow.PortIdentity import org.apache.texera.amber.util.JSONUtils.objectMapper @@ -131,10 +131,10 @@ class LogicalLinkSpec extends AnyFlatSpec { } // --------------------------------------------------------------------------- - // Secondary @JsonCreator constructor (string opId variant) + // Secondary string opId constructor // --------------------------------------------------------------------------- - "LogicalLink secondary @JsonCreator constructor" should "wrap raw String op ids in OperatorIdentity" in { + "LogicalLink secondary String constructor" should "wrap raw String op ids in OperatorIdentity" in { val link = new LogicalLink( fromOpId = "op-A", fromPortId = PortIdentity(0), @@ -160,7 +160,7 @@ class LogicalLinkSpec extends AnyFlatSpec { assert(link.toOpId == OperatorIdentity("my.op-2")) } - it should "reject the empty string as an op id via the @JsonCreator constructor" in { + it should "reject the empty string as an op id via the secondary String constructor" in { intercept[IllegalArgumentException] { new LogicalLink("", PortIdentity(0), "op-B", PortIdentity(1)) } @@ -169,7 +169,7 @@ class LogicalLinkSpec extends AnyFlatSpec { } } - it should "reject a null string op id via the @JsonCreator constructor" in { + it should "reject a null string op id via the secondary String constructor" in { intercept[IllegalArgumentException] { new LogicalLink(null: String, PortIdentity(0), "op-B", PortIdentity(1)) } @@ -178,7 +178,7 @@ class LogicalLinkSpec extends AnyFlatSpec { } } - it should "reject a self-loop via the @JsonCreator constructor (same string op id)" in { + it should "reject a self-loop via the secondary String constructor (same string op id)" in { val ex = intercept[IllegalArgumentException] { new LogicalLink("op-A", PortIdentity(0), "op-A", PortIdentity(1)) } @@ -194,12 +194,11 @@ class LogicalLinkSpec extends AnyFlatSpec { // wiring (annotations, default-Scala-module config) surfaces here. "LogicalLink Jackson deserialization" should - "deserialize fromOpId / toOpId from raw String values via the secondary @JsonCreator constructor" in { + "deserialize fromOpId / toOpId from raw String values via the Jackson creator" in { // Build the JSON by hand to mimic a user-saved workflow file where // `fromOpId` and `toOpId` are written as plain strings (the only shape // production actually receives, since the frontend emits them as - // strings). Jackson dispatches to the @JsonCreator string-overload - // constructor. + // strings). Jackson dispatches to the @JsonCreator constructor. val node = objectMapper.createObjectNode() node.put("fromOpId", "op-A") node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) @@ -245,17 +244,7 @@ class LogicalLinkSpec extends AnyFlatSpec { assert(tree.has("toPortId")) } - it should "NOT round-trip through writeValueAsString (the @JsonCreator string overload is incompatible with the object-shape OperatorIdentity that writeValueAsString emits)" in { - // Characterization of a real asymmetry tracked by - // https://github.com/apache/texera/issues/5042. Production reads - // user-saved workflow JSON where `fromOpId`/`toOpId` are plain - // strings, but `objectMapper.writeValueAsString` writes - // OperatorIdentity as `{"id":"op-A"}` (the case-class object form). - // Re-reading the emitted JSON fails because Jackson dispatches on the - // @JsonCreator string overload, which can't accept an object for - // fromOpId. When the issue is fixed (additional @JsonCreator object - // overload or a custom @JsonDeserialize), this test must flip to a - // passing round-trip assertion alongside the fix. + it should "round-trip through writeValueAsString when OperatorIdentity fields use object shape" in { val original = LogicalLink( OperatorIdentity("op-A"), PortIdentity(0), @@ -269,16 +258,14 @@ class LogicalLinkSpec extends AnyFlatSpec { val tree = objectMapper.readTree(json) assert(tree.path("fromOpId").isObject, s"expected fromOpId to be an object: $json") assert(tree.path("fromOpId").path("id").asText() == "op-A") - // Re-reading the just-emitted JSON fails because the @JsonCreator - // String overload can't accept the object-shape fromOpId. - intercept[MismatchedInputException] { - objectMapper.readValue(json, classOf[LogicalLink]) - } + + val roundTripped = objectMapper.readValue(json, classOf[LogicalLink]) + assert(roundTripped == original) } - it should "reject missing string op-id fields when deserializing via Jackson" in { + it should "reject missing op-id fields when deserializing via Jackson" in { // When `fromOpId` / `toOpId` are omitted, Jackson invokes the - // @JsonCreator with `null` for the missing String args. The primary + // @JsonCreator with `null` for the missing args. The primary // constructor's `require` on non-null/non-empty ids then throws, and // Jackson wraps it in `ValueInstantiationException` with the original // `IllegalArgumentException` as the cause. @@ -288,4 +275,84 @@ class LogicalLinkSpec extends AnyFlatSpec { } assert(ex.getCause.isInstanceOf[IllegalArgumentException]) } + + it should "reject an object-shape op id with no `id` field (null id fails the require guard)" in { + // In amber (strict), an object-shape fromOpId lacking an `id` field + // resolves to OperatorIdentity(null), which the primary constructor's + // `require` rejects — surfaced as ValueInstantiationException. + val node = objectMapper.createObjectNode() + node.set("fromOpId", objectMapper.createObjectNode()) // {} — no "id" + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val ex = intercept[ValueInstantiationException] { + objectMapper.treeToValue(node, classOf[LogicalLink]) + } + assert(ex.getCause.isInstanceOf[IllegalArgumentException]) + assert(ex.getCause.getMessage.contains("fromOpId must be non-null")) + } + + it should "reject an object-shape op id whose `id` field is non-textual" in { + // `{"id": 123}` is malformed: `id` must be a string (or null). The + // helper throws rather than silently coercing 123 -> "123"; Jackson + // wraps the IllegalArgumentException in a ValueInstantiationException. + val node = objectMapper.createObjectNode() + val badOpId = objectMapper.createObjectNode() + badOpId.put("id", 123) + node.set("fromOpId", badOpId) + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val ex = intercept[ValueInstantiationException] { + objectMapper.treeToValue(node, classOf[LogicalLink]) + } + assert(ex.getCause.isInstanceOf[IllegalArgumentException]) + assert(ex.getCause.getMessage.contains("fromOpId.id must be a string")) + } + + it should "reject an op id that is neither a string nor an object (e.g. a number)" in { + // A top-level numeric fromOpId hits the final else branch of + // readOperatorIdentity, which throws; Jackson wraps it. + val node = objectMapper.createObjectNode() + node.put("fromOpId", 12345) + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val ex = intercept[ValueInstantiationException] { + objectMapper.treeToValue(node, classOf[LogicalLink]) + } + assert(ex.getCause.isInstanceOf[IllegalArgumentException]) + assert(ex.getCause.getMessage.contains("fromOpId must be a string or an object")) + } + + it should "reject an explicit JSON null op id (exercises the node.isNull branch)" in { + // An explicit `"fromOpId": null` arrives as a NullNode (an absent field + // arrives as Java null), exercising the `node.isNull` branch; require + // then rejects the null id. + val node = objectMapper.createObjectNode() + node.set("fromOpId", objectMapper.nullNode()) + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val ex = intercept[ValueInstantiationException] { + objectMapper.treeToValue(node, classOf[LogicalLink]) + } + assert(ex.getCause.isInstanceOf[IllegalArgumentException]) + } + + it should "reject an object-shape op id with an explicit null `id` (exercises the idNode.isNull branch)" in { + // `{"id": null}` makes idNode a NullNode, exercising the `idNode.isNull` + // branch; require then rejects the resulting OperatorIdentity(null). + val opId = objectMapper.createObjectNode() + opId.set("id", objectMapper.nullNode()) + val node = objectMapper.createObjectNode() + node.set("fromOpId", opId) + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val ex = intercept[ValueInstantiationException] { + objectMapper.treeToValue(node, classOf[LogicalLink]) + } + assert(ex.getCause.isInstanceOf[IllegalArgumentException]) + } } diff --git a/workflow-compiling-service/src/main/scala/org/apache/texera/amber/compiler/model/LogicalLink.scala b/workflow-compiling-service/src/main/scala/org/apache/texera/amber/compiler/model/LogicalLink.scala index 5c7662f9668..8e07bdfd383 100644 --- a/workflow-compiling-service/src/main/scala/org/apache/texera/amber/compiler/model/LogicalLink.scala +++ b/workflow-compiling-service/src/main/scala/org/apache/texera/amber/compiler/model/LogicalLink.scala @@ -20,22 +20,68 @@ package org.apache.texera.amber.compiler.model import com.fasterxml.jackson.annotation.{JsonCreator, JsonProperty} +import com.fasterxml.jackson.databind.JsonNode import org.apache.texera.amber.core.virtualidentity.OperatorIdentity import org.apache.texera.amber.core.workflow.PortIdentity +object LogicalLink { + + // Reads an OperatorIdentity from either the plain-string shape the + // frontend emits (`"op-A"`) or the object shape Jackson emits for an + // OperatorIdentity (`{"id":"op-A"}`). Keep in sync with the amber + // workflow LogicalLink.readOperatorIdentity, which is intentionally + // identical. + private def readOperatorIdentity(node: JsonNode, fieldName: String): OperatorIdentity = { + if (node == null || node.isNull) { + OperatorIdentity(null) + } else if (node.isTextual) { + OperatorIdentity(node.asText()) + } else if (node.isObject) { + val idNode = node.get("id") + if (idNode == null || idNode.isNull) { + OperatorIdentity(null) + } else if (idNode.isTextual) { + OperatorIdentity(idNode.asText()) + } else { + throw new IllegalArgumentException( + s"LogicalLink $fieldName.id must be a string or null, but was ${idNode.getNodeType}" + ) + } + } else { + throw new IllegalArgumentException( + s"LogicalLink $fieldName must be a string or an object, but was ${node.getNodeType}" + ) + } + } +} + case class LogicalLink( @JsonProperty("fromOpId") fromOpId: OperatorIdentity, fromPortId: PortIdentity, @JsonProperty("toOpId") toOpId: OperatorIdentity, toPortId: PortIdentity ) { - @JsonCreator def this( - @JsonProperty("fromOpId") fromOpId: String, + fromOpId: String, fromPortId: PortIdentity, - @JsonProperty("toOpId") toOpId: String, + toOpId: String, toPortId: PortIdentity ) = { this(OperatorIdentity(fromOpId), fromPortId, OperatorIdentity(toOpId), toPortId) } + + @JsonCreator + def this( + @JsonProperty("fromOpId") fromOpId: JsonNode, + fromPortId: PortIdentity, + @JsonProperty("toOpId") toOpId: JsonNode, + toPortId: PortIdentity + ) = { + this( + LogicalLink.readOperatorIdentity(fromOpId, "fromOpId"), + fromPortId, + LogicalLink.readOperatorIdentity(toOpId, "toOpId"), + toPortId + ) + } } diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/compiler/model/LogicalLinkSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/compiler/model/LogicalLinkSpec.scala new file mode 100644 index 00000000000..738ba211020 --- /dev/null +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/compiler/model/LogicalLinkSpec.scala @@ -0,0 +1,298 @@ +/* + * 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.compiler.model + +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.exc.ValueInstantiationException +import org.apache.texera.amber.core.virtualidentity.OperatorIdentity +import org.apache.texera.amber.core.workflow.PortIdentity +import org.apache.texera.amber.util.JSONUtils.objectMapper +import org.scalatest.flatspec.AnyFlatSpec + +/** + * Unit tests for the workflow-compiling-service [[LogicalLink]]. + * + * Unlike the amber engine's LogicalLink, this version is intentionally + * lenient: it carries no `require` guards so that the compiler can + * represent partially-built or invalid workflows during editing without + * throwing. Tests here pin that contract and verify the Jackson wiring + * that lets the service round-trip saved workflow JSON. + */ +class LogicalLinkSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // Primary constructor + case-class semantics + // --------------------------------------------------------------------------- + + "LogicalLink primary constructor" should "expose the four fields it was constructed with" in { + val link = LogicalLink( + fromOpId = OperatorIdentity("op-A"), + fromPortId = PortIdentity(0), + toOpId = OperatorIdentity("op-B"), + toPortId = PortIdentity(1, internal = true) + ) + assert(link.fromOpId == OperatorIdentity("op-A")) + assert(link.fromPortId == PortIdentity(0)) + assert(link.toOpId == OperatorIdentity("op-B")) + assert(link.toPortId == PortIdentity(1, internal = true)) + } + + "LogicalLink case-class equality" should "use structural equality across all four fields" in { + val a = + LogicalLink(OperatorIdentity("x"), PortIdentity(0), OperatorIdentity("y"), PortIdentity(1)) + val b = + LogicalLink(OperatorIdentity("x"), PortIdentity(0), OperatorIdentity("y"), PortIdentity(1)) + assert(a == b) + assert(a.hashCode == b.hashCode) + } + + it should "distinguish links that differ only in fromOpId" in { + val a = + LogicalLink(OperatorIdentity("x"), PortIdentity(0), OperatorIdentity("y"), PortIdentity(1)) + val b = + LogicalLink(OperatorIdentity("z"), PortIdentity(0), OperatorIdentity("y"), PortIdentity(1)) + assert(a != b) + } + + it should "distinguish links that differ only in toPortId.internal" in { + val a = LogicalLink( + OperatorIdentity("x"), + PortIdentity(0), + OperatorIdentity("y"), + PortIdentity(1, internal = false) + ) + val b = LogicalLink( + OperatorIdentity("x"), + PortIdentity(0), + OperatorIdentity("y"), + PortIdentity(1, internal = true) + ) + assert(a != b) + } + + // --------------------------------------------------------------------------- + // Secondary String constructor + // --------------------------------------------------------------------------- + + "LogicalLink secondary String constructor" should "wrap raw String op ids in OperatorIdentity" in { + val link = new LogicalLink( + fromOpId = "op-A", + fromPortId = PortIdentity(0), + toOpId = "op-B", + toPortId = PortIdentity(1) + ) + assert(link.fromOpId == OperatorIdentity("op-A")) + assert(link.toOpId == OperatorIdentity("op-B")) + assert( + link == LogicalLink( + OperatorIdentity("op-A"), + PortIdentity(0), + OperatorIdentity("op-B"), + PortIdentity(1) + ) + ) + } + + it should "accept identifiers containing dashes, dots, and digits" in { + val link = new LogicalLink("my.op-1", PortIdentity(0), "my.op-2", PortIdentity(1)) + assert(link.fromOpId == OperatorIdentity("my.op-1")) + assert(link.toOpId == OperatorIdentity("my.op-2")) + } + + // --------------------------------------------------------------------------- + // Leniency contract: no require guards in the compiler-service variant + // --------------------------------------------------------------------------- + // + // The compiler-service LogicalLink is intentionally lenient so that a + // mid-edit, partially-built workflow (e.g. one where an operator id has + // not yet been assigned) can be represented without throwing. The amber + // engine's LogicalLink enforces strict validation; tests for that live in + // amber/src/test. + + "LogicalLink (compiler-service)" should "accept a null OperatorIdentity id without throwing" in { + val link = LogicalLink( + OperatorIdentity(null), + PortIdentity(0), + OperatorIdentity("op-B"), + PortIdentity(1) + ) + assert(link.fromOpId == OperatorIdentity(null)) + } + + it should "accept a self-loop link (fromOpId == toOpId) without throwing" in { + val link = LogicalLink( + OperatorIdentity("op-A"), + PortIdentity(0), + OperatorIdentity("op-A"), + PortIdentity(1) + ) + assert(link.fromOpId == link.toOpId) + } + + // --------------------------------------------------------------------------- + // Jackson round-trip (production objectMapper) + // --------------------------------------------------------------------------- + // + // These tests use the same `JSONUtils.objectMapper` that production uses + // to read user-saved workflow JSON, so a regression in the Jackson wiring + // (annotations, default-Scala-module config) surfaces here. + + "LogicalLink Jackson deserialization" should + "deserialize fromOpId / toOpId from raw String values via the Jackson creator" in { + val node = objectMapper.createObjectNode() + node.put("fromOpId", "op-A") + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val link = objectMapper.treeToValue(node, classOf[LogicalLink]) + assert(link.fromOpId == OperatorIdentity("op-A")) + assert(link.toOpId == OperatorIdentity("op-B")) + assert(link.fromPortId == PortIdentity(0)) + assert(link.toPortId == PortIdentity(1)) + } + + it should "round-trip through writeValueAsString when OperatorIdentity fields use object shape" in { + val original = LogicalLink( + OperatorIdentity("op-A"), + PortIdentity(0), + OperatorIdentity("op-B"), + PortIdentity(1) + ) + val json = objectMapper.writeValueAsString(original) + val tree = objectMapper.readTree(json) + assert(tree.path("fromOpId").isObject, s"expected fromOpId to be an object: $json") + assert(tree.path("fromOpId").path("id").asText() == "op-A") + + val roundTripped = objectMapper.readValue(json, classOf[LogicalLink]) + assert(roundTripped == original) + } + + it should "emit `fromOpId` / `toOpId` JSON keys pinned by @JsonProperty annotations" in { + // @JsonProperty pins the key name — a Scala parameter rename keeps the + // JSON key stable, which is required for saved-workflow compatibility. + val link = LogicalLink( + OperatorIdentity("op-A"), + PortIdentity(0), + OperatorIdentity("op-B"), + PortIdentity(1) + ) + val tree = objectMapper.valueToTree[JsonNode](link) + assert(tree.has("fromOpId")) + assert(tree.has("toOpId")) + } + + it should "emit `fromPortId` / `toPortId` JSON keys derived from Scala parameter names (no @JsonProperty)" in { + // No @JsonProperty on these fields — the JSON key comes from the Scala + // parameter name. A future rename WOULD silently break saved-workflow + // compatibility; this test exists so that rename breaks here on purpose. + val link = LogicalLink( + OperatorIdentity("op-A"), + PortIdentity(0), + OperatorIdentity("op-B"), + PortIdentity(1) + ) + val tree = objectMapper.valueToTree[JsonNode](link) + assert(tree.has("fromPortId")) + assert(tree.has("toPortId")) + } + + it should "produce OperatorIdentity(null) when fromOpId is absent from JSON entirely (lenient)" in { + // When fromOpId / toOpId are omitted entirely, Jackson passes null to the + // @JsonCreator parameter. readOperatorIdentity treats null as OperatorIdentity(null) + // rather than throwing — lenient by design for partial workflows mid-edit. + val empty = objectMapper.createObjectNode() + val link = objectMapper.treeToValue(empty, classOf[LogicalLink]) + assert(link.fromOpId == OperatorIdentity(null)) + assert(link.toOpId == OperatorIdentity(null)) + } + + it should "produce OperatorIdentity(null) for an object-shape id with no id field (lenient)" in { + // WCS LogicalLink is lenient: an object-shape fromOpId with no "id" field + // produces OperatorIdentity(null) rather than throwing. This lets the + // compiler represent partially-built workflows mid-edit. + val node = objectMapper.createObjectNode() + node.set("fromOpId", objectMapper.createObjectNode()) // {} — no "id" field + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + + val link = objectMapper.treeToValue(node, classOf[LogicalLink]) + assert(link.fromOpId == OperatorIdentity(null)) + } + + it should "wrap IllegalArgumentException in ValueInstantiationException when an opId is a numeric value instead of text or object" in { + // A number is not a valid shape for fromOpId regardless of leniency — + // readOperatorIdentity explicitly throws for non-text, non-object nodes. + val node = objectMapper.createObjectNode() + node.put("fromOpId", 12345) + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + + val ex = intercept[ValueInstantiationException] { + objectMapper.treeToValue(node, classOf[LogicalLink]) + } + assert(ex.getCause.isInstanceOf[IllegalArgumentException]) + assert(ex.getCause.getMessage.contains("fromOpId must be a string or an object")) + } + + it should "throw when an object-shape opId has a non-textual `id` field" in { + // Leniency covers null/empty/self-loop semantics, not malformed JSON + // types: `{"id": 123}` is rejected rather than coerced to "123". + val node = objectMapper.createObjectNode() + val badOpId = objectMapper.createObjectNode() + badOpId.put("id", 123) + node.set("fromOpId", badOpId) + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val ex = intercept[ValueInstantiationException] { + objectMapper.treeToValue(node, classOf[LogicalLink]) + } + assert(ex.getCause.isInstanceOf[IllegalArgumentException]) + assert(ex.getCause.getMessage.contains("fromOpId.id must be a string")) + } + + it should "treat an explicit JSON null op id as OperatorIdentity(null) (exercises the node.isNull branch)" in { + // An explicit `"fromOpId": null` arrives as a NullNode (an absent field + // arrives as Java null); WCS leniency maps it to OperatorIdentity(null). + val node = objectMapper.createObjectNode() + node.set("fromOpId", objectMapper.nullNode()) + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val link = objectMapper.treeToValue(node, classOf[LogicalLink]) + assert(link.fromOpId == OperatorIdentity(null)) + } + + it should "treat an object-shape op id with explicit null `id` as OperatorIdentity(null) (exercises the idNode.isNull branch)" in { + // `{"id": null}` — `id` is present but JSON null, so idNode is a NullNode. + // WCS leniency maps this to OperatorIdentity(null). + val opId = objectMapper.createObjectNode() + opId.set("id", objectMapper.nullNode()) + val node = objectMapper.createObjectNode() + node.set("fromOpId", opId) + node.set("fromPortId", objectMapper.valueToTree[JsonNode](PortIdentity(0))) + node.put("toOpId", "op-B") + node.set("toPortId", objectMapper.valueToTree[JsonNode](PortIdentity(1))) + val link = objectMapper.treeToValue(node, classOf[LogicalLink]) + assert(link.fromOpId == OperatorIdentity(null)) + } +}