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-7233-8b9d017cdb26e7153d2784602fac971fc6470601 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 18a277c3009e71c94c12033bb9af2a9e41089684 Author: Kary Zheng <[email protected]> AuthorDate: Thu Aug 6 16:31:58 2026 -0700 feat(visualization): declare the numeric-only chart settings as numbers (#7233) ### What changes were proposed in this PR? Seven settings across two chart operators accept only numbers — each is used solely as `float(...)` — but were declared as plain strings, so the form let any text through and each operator then mishandled it differently: - Gauge Chart `delta`, `threshold`; Bullet Chart `thresholdValue` — discarded silently, leaving a finished-looking chart with the indicator missing - Bullet Chart `deltaReference`, step `start`/`end` — error page This PR declares them as `Option[Double]`: - the form rejects a non-number before the run, and both operators behave the same way - the generated code no longer parses: values are spliced as numbers, so the conversions, the `try/except ValueError` blocks and Gauge Chart's `json.loads` of its steps are gone, along with an `import json` that became dead - a step whose bounds are not both filled in is dropped while the list is built, which is what Bullet Chart's "Invalid step values" note existed for - `@JsonDeserialize(contentAs = ...)` names the boxed class: Scala erases `Option`'s element type, so without it Jackson leaves the raw JSON value inside the Option and the first use throws `ClassCastException`, and the primitive class would read a blank as 0 - Gauge Chart's step bounds are included beyond the six the issue lists — same declaration, same consumer, and a non-numeric bound was swallowed by a bare `except`, plotting the gauge with no steps at all Compatibility: a numeric string saved before this change still loads as a number and a blank one as unset; a workflow that stored a value that is not a number now fails to load rather than silently dropping it — those charts were already rendering without the setting. ### Any related issues, documentation, discussions? Fixes #7213. Contour Plot's Grid Size started here and moved to #7343: it is the one field that aborts with nothing entered, so it stands on its own rather than travelling with validation the operators only need when a wrong value is typed. ### How was this PR tested? - existing specs for both operators updated to the new types - per field: deserialization tests for a JSON number, a numeric string, blank, null and absent — plus one that uses the value as a number, the case a round trip cannot catch, since a round trip writes a number back and a missing `contentAs` survives it - generated-code tests pin the assignments (`delta_ref = 40.0`, `threshold_val = None`) and the dropped half-filled step - ran the generated Python against a pandas DataFrame for both operators, configured and left unset: each plots a figure, and the inverted-step case still reports `start ≥ end` - whole workflow-operator module: 2033 tests passing, `scalafmtCheck` clean ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-5[1m]) --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]> --- .../bulletChart/BulletChartOpDesc.scala | 84 +++++++++++----------- .../bulletChart/BulletChartStepDefinition.scala | 11 ++- .../gaugeChart/GaugeChartOpDesc.scala | 66 ++++++++--------- .../visualization/gaugeChart/GaugeChartSteps.scala | 12 +++- .../bulletChart/BulletChartOpDescSpec.scala | 59 +++++++++------ .../BulletChartStepDefinitionSpec.scala | 69 +++++++++++++----- .../gaugeChart/GaugeChartOpDescSpec.scala | 66 ++++++++++++++--- .../gaugeChart/GaugeChartStepsSpec.scala | 70 +++++++++++++----- 8 files changed, 286 insertions(+), 151 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDesc.scala index f4a89e36b7..22bee1bf50 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDesc.scala @@ -20,10 +20,11 @@ package org.apache.texera.amber.operator.visualization.bulletChart import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} +import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle import org.apache.texera.amber.core.tuple.{AttributeType, Schema} import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext -import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString +import org.apache.texera.amber.pybuilder.PyStringTypes.{EncodableString, PythonLiteral} import org.apache.texera.amber.core.workflow.PortIdentity import org.apache.texera.amber.operator.PythonOperatorDescriptor import org.apache.texera.amber.operator.metadata.annotations.AutofillAttributeName @@ -47,16 +48,20 @@ class BulletChartOpDesc extends PythonOperatorDescriptor { @NotNull(message = "Value cannot be empty") var value: EncodableString = "" + // Numeric: both are only used as float(). contentAs names the boxed class — + // Option erases its element type, and a blank must not read as 0. @JsonProperty(value = "deltaReference", required = true) @JsonSchemaTitle("Delta Reference") @JsonPropertyDescription("The reference value for the delta indicator. e.g., 100") @NotNull(message = "Delta Reference cannot be empty") - var deltaReference: EncodableString = "" + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var deltaReference: Option[Double] = None @JsonProperty(value = "thresholdValue", required = false) @JsonSchemaTitle("Threshold Value") @JsonPropertyDescription("The performance threshold value. e.g., 100") - var thresholdValue: EncodableString = "" + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var thresholdValue: Option[Double] = None @JsonProperty(value = "steps", required = false) @JsonSchemaTitle("Steps") @@ -79,14 +84,19 @@ class BulletChartOpDesc extends PythonOperatorDescriptor { ) override def generatePythonCode(): String = { - // Convert the Scala list of steps into a list of dictionaries - val stepsStr = if (steps != null && !steps.isEmpty) { - val stepsSeq = - steps.asScala.map(step => pyb"""{"start": ${step.start}, "end": ${step.end}}""") - "[" + stepsSeq.mkString(", ") + "]" - } else { - "[]" - } + // The reference keeps the 0 the generated code used to fall back to; an unset + // threshold stays absent as None. + val deltaReferenceExpr: PythonLiteral = deltaReference.getOrElse(0.0).toString + val thresholdExpr: PythonLiteral = thresholdValue.map(_.toString).getOrElse("None") + + // The steps whose bounds are both filled in, as a list literal of numbers. + val stepsExpr: PythonLiteral = + Option(steps) + .map(_.asScala.toSeq) + .getOrElse(Seq.empty) + .flatMap(step => step.start.zip(step.end)) + .map { case (start, end) => s"""{"start": $start, "end": $end}""" } + .mkString("[", ", ", "]") val finalCode = pyb""" @@ -110,24 +120,18 @@ class BulletChartOpDesc extends PythonOperatorDescriptor { | colors.append(f"hsl(0, 0%, {lightness}%)") | return colors | - | # Validate and convert user-provided step definitions + | # Validate user-provided step definitions | def generate_valid_steps(self, steps_data): | valid_steps = [] | self.step_errors = [] | | for index, step in enumerate(steps_data): - | start = step.get('start', '') - | end = step.get('end', '') - | if start and end: - | try: - | s_val = float(start) - | e_val = float(end) - | if s_val < e_val: - | valid_steps.append({"start": s_val, "end": e_val}) - | else: - | self.step_errors.append(f"Step {index + 1}: start ≥ end ({s_val} ≥ {e_val})") - | except Exception as e: - | self.step_errors.append(f"Step {index + 1}: Invalid step values: start='{start}', end='{end}'") + | s_val = step["start"] + | e_val = step["end"] + | if s_val < e_val: + | valid_steps.append({"start": s_val, "end": e_val}) + | else: + | self.step_errors.append(f"Step {index + 1}: start ≥ end ({s_val} ≥ {e_val})") | return valid_steps | | @overrides @@ -138,7 +142,7 @@ class BulletChartOpDesc extends PythonOperatorDescriptor { | | try: | value_col = $value - | delta_ref = float($deltaReference) if $deltaReference.strip() else 0 + | delta_ref = $deltaReferenceExpr | | if value_col not in table.columns: | yield {'html-content': self.render_error(f"Column '{value_col}' not found in input table.")} @@ -149,25 +153,17 @@ class BulletChartOpDesc extends PythonOperatorDescriptor { | yield {'html-content': self.render_error("No valid data rows found after dropping nulls.")} | return | - | try: - | threshold_val = float($thresholdValue) if $thresholdValue.strip() else None - | except ValueError: - | threshold_val = None - | - | # Parse and validate steps input - | try: - | steps_data = $stepsStr - | valid_steps = self.generate_valid_steps(steps_data) - | step_colors = self.generate_gray_gradient(len(valid_steps)) - | steps_list = [] - | for index, step_data in enumerate(valid_steps): - | color = step_colors[index] - | steps_list.append({ - | "range": [step_data["start"], step_data["end"]], - | "color": color - | }) - | except Exception: - | steps_list = [] + | threshold_val = $thresholdExpr + | + | valid_steps = self.generate_valid_steps($stepsExpr) + | step_colors = self.generate_gray_gradient(len(valid_steps)) + | steps_list = [] + | for index, step_data in enumerate(valid_steps): + | color = step_colors[index] + | steps_list.append({ + | "range": [step_data["start"], step_data["end"]], + | "color": color + | }) | | # Iterate through up to 10 rows of the input table | count = 0 diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartStepDefinition.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartStepDefinition.scala index 5ff0ad8953..a584801bba 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartStepDefinition.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartStepDefinition.scala @@ -20,18 +20,23 @@ package org.apache.texera.amber.operator.visualization.bulletChart import com.fasterxml.jackson.annotation.{JsonCreator, JsonProperty} +import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle -import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString /** * Defines a step range used for qualitative segments in the Bullet Chart. + * + * Numeric bounds: only used as float(). contentAs names the boxed class — Option + * erases its element type, and a blank must not read as 0. */ class BulletChartStepDefinition @JsonCreator() ( @JsonProperty("start") @JsonSchemaTitle("Start") - var start: EncodableString, + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var start: Option[Double], @JsonProperty("end") @JsonSchemaTitle("End") - var end: EncodableString + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var end: Option[Double] ) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartOpDesc.scala index 6c5361e43e..320e2fc564 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartOpDesc.scala @@ -19,12 +19,11 @@ package org.apache.texera.amber.operator.visualization.gaugeChart import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.module.scala.DefaultScalaModule +import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle import org.apache.texera.amber.core.tuple.{AttributeType, Schema} import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext -import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString +import org.apache.texera.amber.pybuilder.PyStringTypes.{EncodableString, PythonLiteral} import org.apache.texera.amber.core.workflow.PortIdentity import org.apache.texera.amber.operator.PythonOperatorDescriptor import org.apache.texera.amber.operator.metadata.annotations.AutofillAttributeName @@ -40,15 +39,19 @@ class GaugeChartOpDesc extends PythonOperatorDescriptor { @NotNull(message = "Gauge Value cannot be empty") var value: EncodableString = "" + // Numeric: both are only used as float(). contentAs names the boxed class — + // Option erases its element type, and a blank must not read as 0. @JsonProperty(value = "delta", required = false) @JsonSchemaTitle("Delta") @JsonPropertyDescription("The baseline value used to calculate the delta from the gauge value") - var delta: EncodableString = "" + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var delta: Option[Double] = None @JsonProperty(value = "threshold", required = false) @JsonSchemaTitle("Threshold Value") @JsonPropertyDescription("Defines a boundary or target value shown on the gauge chart") - var threshold: EncodableString = "" + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var threshold: Option[Double] = None @JsonProperty(value = "steps", required = false) @JsonSchemaTitle("Steps") @@ -69,21 +72,29 @@ class GaugeChartOpDesc extends PythonOperatorDescriptor { OperatorGroupConstants.VISUALIZATION_FINANCIAL_GROUP ) - private val mapper = new ObjectMapper() - mapper.registerModule(DefaultScalaModule) + /** An unset number reaches the generated code as Python's `None`. */ + private def numberOrNone(value: Option[Double]): PythonLiteral = + value.map(_.toString).getOrElse("None") - private def serializeSteps(steps: List[GaugeChartSteps]): String = { - mapper.writeValueAsString(steps) - } + /** The steps whose bounds are both filled in, as a list literal of numbers. + * The field is optional, so an explicit null leaves it null; that is no steps. + */ + private def stepsLiteral: PythonLiteral = + Option(steps) + .getOrElse(List.empty) + .flatMap(step => step.start.zip(step.end)) + .map { case (start, end) => s"""{"start": $start, "end": $end}""" } + .mkString("[", ", ", "]") override def generatePythonCode(): String = { - val stepsStr: EncodableString = serializeSteps(steps) + val deltaExpr = numberOrNone(delta) + val thresholdExpr = numberOrNone(threshold) + val stepsExpr = stepsLiteral pyb""" |from pytexera import * |import plotly.graph_objects as go |import plotly.io as pio - |import json | |class ProcessTableOperator(UDFTableOperator): | @@ -106,32 +117,23 @@ class GaugeChartOpDesc extends PythonOperatorDescriptor { | | try: | gauge_value = $value - | try: - | delta_ref = float($delta) if $delta.strip() else None - | except ValueError: - | delta_ref = None - | try: - | threshold_val = float($threshold) if $threshold.strip() else None - | except ValueError: - | threshold_val = None + | delta_ref = $deltaExpr + | threshold_val = $thresholdExpr | | table = table.dropna(subset=[gauge_value]) | if table.empty: | yield {'html-content': self.render_error("No non-null rows found for the value column.")} | return | - | try: - | valid_steps = json.loads($stepsStr) - | step_colors = self.generate_gray_gradient(len(valid_steps)) - | steps_list = [] - | for index, step_data in enumerate(valid_steps): - | color = step_colors[index] - | steps_list.append({ - | "range": [float(step_data["start"]), float(step_data["end"])], - | "color": color - | }) - | except Exception: - | steps_list = [] + | valid_steps = $stepsExpr + | step_colors = self.generate_gray_gradient(len(valid_steps)) + | steps_list = [] + | for index, step_data in enumerate(valid_steps): + | color = step_colors[index] + | steps_list.append({ + | "range": [step_data["start"], step_data["end"]], + | "color": color + | }) | | html_chunks = [] | for _, row in table.iterrows(): diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartSteps.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartSteps.scala index 4c6235a9ad..b8ac668a31 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartSteps.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartSteps.scala @@ -19,15 +19,21 @@ package org.apache.texera.amber.operator.visualization.gaugeChart import com.fasterxml.jackson.annotation.JsonProperty +import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle -import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString +/** + * Numeric bounds: only used as float(). contentAs names the boxed class — Option + * erases its element type, and a blank must not read as 0. + */ class GaugeChartSteps { @JsonProperty("start") @JsonSchemaTitle("Start") - var start: EncodableString = "" + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var start: Option[Double] = None @JsonProperty("end") @JsonSchemaTitle("End") - var end: EncodableString = "" + @JsonDeserialize(contentAs = classOf[java.lang.Double]) + var end: Option[Double] = None } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDescSpec.scala index fff547b939..ce6d5d32a7 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDescSpec.scala @@ -32,7 +32,7 @@ class BulletChartOpDescSpec extends AnyFlatSpec with Matchers { private def configured: BulletChartOpDesc = { val op = new BulletChartOpDesc op.value = "actualValue" - op.deltaReference = "100" + op.deltaReference = Some(100) op } @@ -59,40 +59,57 @@ class BulletChartOpDescSpec extends AnyFlatSpec with Matchers { } "BulletChartOpDesc.generatePythonCode" should "render Python source with a runtime decode site for the value column" in { - // EncodableString fields are NOT emitted as literal strings — the pyb - // macro wraps them in `self.decode_python_template.decode("<base64>")` - // calls. The rendered source must reference the decoder symbol at least - // for `value` and `deltaReference`. + // The column name is an EncodableString, so pyb wraps it in a decode call. The + // numeric settings carry no user text and add no decode site. val code = configured.generatePythonCode() code should include("plotly.graph_objects") val decodeOccurrences = "decode_python_template".r.findAllIn(code).length - decodeOccurrences should be >= 2 + decodeOccurrences should be >= 1 + } + + it should "assign the delta reference as a number, falling back to 0 when unset" in { + configured.generatePythonCode() should include("delta_ref = 100.0") + val unset = new BulletChartOpDesc + unset.value = "actualValue" + unset.generatePythonCode() should include("delta_ref = 0.0") + } + + it should "assign None for a threshold that is not configured" in { + configured.generatePythonCode() should include("threshold_val = None") + val withThreshold = configured + withThreshold.thresholdValue = Some(75.5) + withThreshold.generatePythonCode() should include("threshold_val = 75.5") } it should "default to an empty steps list when none are configured" in { - // The bullet-chart template ships with several unrelated `[]` literals - // (`colors`, `valid_steps`, `step_errors`, `steps_list`, `html_chunks`), - // so a bare `code should include("[]")` is too weak. Anchor on the - // generated `steps_data = ...` literal directly so a regression that - // makes it non-empty would actually fail the assertion. + // The template ships several unrelated `[]` literals, so anchor on the argument + // passed to generate_valid_steps rather than on a bare `[]`. val code = configured.generatePythonCode() - code should include regex """steps_data\s*=\s*\[\]""" + code should include regex """generate_valid_steps\(\[\]\)""" + } + + it should "emit no steps when steps is null" in { + // Steps is optional, so an explicit null in the payload leaves the field null + // rather than an empty list; that is no steps, not a failure. + val op = configured + op.steps = null + op.generatePythonCode() should include regex """generate_valid_steps\(\[\]\)""" } - it should "include each configured step's start/end JSON keys with extra decode sites" in { + it should "emit each configured step's bounds as numbers, dropping a half-filled step" in { val op = configured val steps: JList[BulletChartStepDefinition] = new util.ArrayList[BulletChartStepDefinition]() - steps.add(new BulletChartStepDefinition("0", "50")) - steps.add(new BulletChartStepDefinition("50", "100")) + steps.add(new BulletChartStepDefinition(Some(0), Some(50))) + steps.add(new BulletChartStepDefinition(Some(50), Some(100))) + steps.add(new BulletChartStepDefinition(Some(100), None)) op.steps = steps val code = op.generatePythonCode() - code should include("\"start\":") - code should include("\"end\":") - // Two steps × 2 EncodableString fields each = 4 extra decode sites on - // top of the value/deltaReference decodes from the base configuration. + code should include( + """generate_valid_steps([{"start": 0.0, "end": 50.0}, {"start": 50.0, "end": 100.0}])""" + ) + // The bounds are numbers now, so a step adds no runtime decode site. val baseDecodes = "decode_python_template".r.findAllIn(configured.generatePythonCode()).length - val withSteps = "decode_python_template".r.findAllIn(code).length - withSteps shouldBe baseDecodes + 4 + "decode_python_template".r.findAllIn(code).length shouldBe baseDecodes } it should "currently render a code block even with the default empty configuration (no assert guard)" in { diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartStepDefinitionSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartStepDefinitionSpec.scala index 984c1fd8b0..35861db461 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartStepDefinitionSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartStepDefinitionSpec.scala @@ -30,9 +30,9 @@ class BulletChartStepDefinitionSpec extends AnyFlatSpec { // --------------------------------------------------------------------------- "BulletChartStepDefinition" should "store both constructor arguments" in { - val d = new BulletChartStepDefinition("10", "90") - assert(d.start == "10") - assert(d.end == "90") + val d = new BulletChartStepDefinition(Some(10), Some(90)) + assert(d.start.contains(10)) + assert(d.end.contains(90)) } // --------------------------------------------------------------------------- @@ -40,11 +40,11 @@ class BulletChartStepDefinitionSpec extends AnyFlatSpec { // --------------------------------------------------------------------------- it should "allow both fields to be reassigned post-construction" in { - val d = new BulletChartStepDefinition("0", "1") - d.start = "low" - d.end = "high" - assert(d.start == "low") - assert(d.end == "high") + val d = new BulletChartStepDefinition(Some(0), Some(1)) + d.start = Some(2.5) + d.end = Some(7.5) + assert(d.start.contains(2.5)) + assert(d.end.contains(7.5)) } // --------------------------------------------------------------------------- @@ -52,23 +52,54 @@ class BulletChartStepDefinitionSpec extends AnyFlatSpec { // --------------------------------------------------------------------------- "BulletChartStepDefinition JSON round-trip" should - "serialize start and end under the canonical wire keys" in { - val d = new BulletChartStepDefinition("alpha", "omega") + "serialize start and end as numbers under the canonical wire keys" in { + val d = new BulletChartStepDefinition(Some(1.5), Some(9.5)) val tree = objectMapper.readTree(objectMapper.writeValueAsString(d)) assert(tree.has("start")) - assert(tree.get("start").asText() == "alpha") + assert(tree.get("start").isNumber) + assert(tree.get("start").asDouble() == 1.5) assert(tree.has("end")) - assert(tree.get("end").asText() == "omega") + assert(tree.get("end").isNumber) + assert(tree.get("end").asDouble() == 9.5) } it should "round-trip both fields cleanly" in { - val d = new BulletChartStepDefinition("33", "66") + val d = new BulletChartStepDefinition(Some(33), Some(66)) val restored = objectMapper.readValue( objectMapper.writeValueAsString(d), classOf[BulletChartStepDefinition] ) - assert(restored.start == "33") - assert(restored.end == "66") + assert(restored.start.contains(33)) + assert(restored.end.contains(66)) + } + + /** Reads the shapes a stored workflow can hold; a round trip cannot cover them, + * since it writes a number back. See GaugeChartStepsSpec for why `contentAs` is + * what these pin. + */ + private def read(json: String): BulletChartStepDefinition = + objectMapper.readValue(json, classOf[BulletChartStepDefinition]) + + "BulletChartStepDefinition bounds" should "deserialize JSON numbers" in { + val d = read("""{"start":1.5,"end":9.5}""") + assert(d.start.contains(1.5)) + assert(d.end.contains(9.5)) + } + + it should "deserialize the numeric strings a workflow saved before the bounds were numeric" in { + val d = read("""{"start":"1.5","end":"9.5"}""") + assert(d.start.contains(1.5)) + assert(d.end.contains(9.5)) + } + + it should "read absent, null and blank bounds as unset rather than as zero" in { + assert(read("""{}""").start.isEmpty) + assert(read("""{"start":null}""").start.isEmpty) + assert(read("""{"start":""}""").start.isEmpty) + } + + it should "hold a Double, not the raw JSON value" in { + assert(read("""{"start":"1.5"}""").start.map(_ * 2).contains(3.0)) } // --------------------------------------------------------------------------- @@ -109,9 +140,9 @@ class BulletChartStepDefinitionSpec extends AnyFlatSpec { // --------------------------------------------------------------------------- it should "construct two independent instances (no static state shared)" in { - val a = new BulletChartStepDefinition("a-start", "a-end") - val b = new BulletChartStepDefinition("b-start", "b-end") - a.start = "mutated" - assert(b.start == "b-start") + val a = new BulletChartStepDefinition(Some(1), Some(2)) + val b = new BulletChartStepDefinition(Some(3), Some(4)) + a.start = Some(99) + assert(b.start.contains(3)) } } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartOpDescSpec.scala index f01947e61f..3e92339926 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartOpDescSpec.scala @@ -37,11 +37,12 @@ class GaugeChartOpDescSpec extends AnyFlatSpec with Matchers { info.outputPorts should have length 1 } - "GaugeChartOpDesc" should "default value/delta/threshold to empty and steps to an empty list" in { + "GaugeChartOpDesc" should + "default value to empty, delta/threshold to unset and steps to an empty list" in { val d = new GaugeChartOpDesc d.value shouldBe "" - d.delta shouldBe "" - d.threshold shouldBe "" + d.delta shouldBe None + d.threshold shouldBe None d.steps shouldBe empty } @@ -66,20 +67,63 @@ class GaugeChartOpDescSpec extends AnyFlatSpec with Matchers { "round-trip value/delta/threshold and steps through the polymorphic base" in { val d = new GaugeChartOpDesc d.value = "v" - d.delta = "dl" - d.threshold = "th" + d.delta = Some(40) + d.threshold = Some(80) val step = new GaugeChartSteps - step.start = "0" - step.end = "50" + step.start = Some(0) + step.end = Some(50) d.steps = List(step) val restored = objectMapper.readValue(objectMapper.writeValueAsString(d), classOf[LogicalOp]) restored shouldBe a[GaugeChartOpDesc] val g = restored.asInstanceOf[GaugeChartOpDesc] g.value shouldBe "v" - g.delta shouldBe "dl" - g.threshold shouldBe "th" + g.delta shouldBe Some(40) + g.threshold shouldBe Some(80) g.steps should have length 1 - g.steps.head.start shouldBe "0" - g.steps.head.end shouldBe "50" + g.steps.head.start shouldBe Some(0) + g.steps.head.end shouldBe Some(50) + } + + /** An unset field has to arrive as Python's `None` for the template's + * `is not None` guards to read it as "not configured". + */ + "GaugeChartOpDesc.generatePythonCode" should + "assign delta and threshold as numbers, and None when they are unset" in { + val d = new GaugeChartOpDesc + d.value = "score" + d.generatePythonCode() should include("delta_ref = None") + d.generatePythonCode() should include("threshold_val = None") + d.delta = Some(40) + d.threshold = Some(80.5) + val code = d.generatePythonCode() + code should include("delta_ref = 40.0") + code should include("threshold_val = 80.5") + } + + it should "emit only the steps whose bounds are both filled in" in { + val d = new GaugeChartOpDesc + d.value = "score" + val complete = new GaugeChartSteps + complete.start = Some(0) + complete.end = Some(50) + val halfFilled = new GaugeChartSteps + halfFilled.start = Some(50) + d.steps = List(complete, halfFilled) + val code = d.generatePythonCode() + code should include("""valid_steps = [{"start": 0.0, "end": 50.0}]""") + } + + it should "emit no steps when the payload sets steps to null" in { + // Steps is optional, so an explicit null leaves the field null rather than an empty + // list; that is no steps, not a failure. + val d = objectMapper + .readValue( + """{"operatorType": "GaugeChart", "value": "score", "steps": null}""", + classOf[LogicalOp] + ) + .asInstanceOf[GaugeChartOpDesc] + d.steps shouldBe null + + d.generatePythonCode() should include("valid_steps = []") } } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartStepsSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartStepsSpec.scala index 840bd425c2..2989e26235 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartStepsSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartStepsSpec.scala @@ -29,10 +29,10 @@ class GaugeChartStepsSpec extends AnyFlatSpec { // Defaults // --------------------------------------------------------------------------- - "GaugeChartSteps" should "default start and end to the empty string" in { + "GaugeChartSteps" should "default start and end to unset" in { val s = new GaugeChartSteps - assert(s.start == "") - assert(s.end == "") + assert(s.start.isEmpty) + assert(s.end.isEmpty) } // --------------------------------------------------------------------------- @@ -41,10 +41,10 @@ class GaugeChartStepsSpec extends AnyFlatSpec { it should "allow start and end to be assigned post-construction" in { val s = new GaugeChartSteps - s.start = "10" - s.end = "90" - assert(s.start == "10") - assert(s.end == "90") + s.start = Some(10) + s.end = Some(90) + assert(s.start.contains(10)) + assert(s.end.contains(90)) } // --------------------------------------------------------------------------- @@ -52,27 +52,61 @@ class GaugeChartStepsSpec extends AnyFlatSpec { // --------------------------------------------------------------------------- "GaugeChartSteps JSON round-trip" should - "serialize start and end under the canonical wire keys" in { + "serialize start and end as numbers under the canonical wire keys" in { val s = new GaugeChartSteps - s.start = "low" - s.end = "high" + s.start = Some(1.5) + s.end = Some(9.5) val tree = objectMapper.readTree(objectMapper.writeValueAsString(s)) assert(tree.has("start")) - assert(tree.get("start").asText() == "low") + assert(tree.get("start").isNumber) + assert(tree.get("start").asDouble() == 1.5) assert(tree.has("end")) - assert(tree.get("end").asText() == "high") + assert(tree.get("end").isNumber) + assert(tree.get("end").asDouble() == 9.5) } it should "round-trip both fields cleanly" in { val s = new GaugeChartSteps - s.start = "0" - s.end = "100" + s.start = Some(0) + s.end = Some(100) val restored = objectMapper.readValue( objectMapper.writeValueAsString(s), classOf[GaugeChartSteps] ) - assert(restored.start == "0") - assert(restored.end == "100") + assert(restored.start.contains(0)) + assert(restored.end.contains(100)) + } + + /** `Option[Double]` erases its element type, so Jackson needs + * `@JsonDeserialize(contentAs = ...)` to know what to build. Without it a JSON + * string is left inside the Option unconverted and the first arithmetic use + * throws ClassCastException — which a round trip cannot catch, since it writes a + * number back. These read the shapes a stored workflow can actually hold. + */ + private def read(json: String): GaugeChartSteps = + objectMapper.readValue(json, classOf[GaugeChartSteps]) + + "GaugeChartSteps bounds" should "deserialize JSON numbers" in { + val s = read("""{"start":1.5,"end":9.5}""") + assert(s.start.contains(1.5)) + assert(s.end.contains(9.5)) + } + + it should "deserialize the numeric strings a workflow saved before the bounds were numeric" in { + val s = read("""{"start":"1.5","end":"9.5"}""") + assert(s.start.contains(1.5)) + assert(s.end.contains(9.5)) + } + + it should "read absent, null and blank bounds as unset rather than as zero" in { + assert(read("""{}""").start.isEmpty) + assert(read("""{"start":null}""").start.isEmpty) + assert(read("""{"start":""}""").start.isEmpty) + } + + it should "hold a Double, not the raw JSON value" in { + // The ClassCastException surfaces here, at the first use, not at read time. + assert(read("""{"start":"1.5"}""").start.map(_ * 2).contains(3.0)) } // --------------------------------------------------------------------------- @@ -102,7 +136,7 @@ class GaugeChartStepsSpec extends AnyFlatSpec { it should "construct two independent instances (no static state shared)" in { val a = new GaugeChartSteps val b = new GaugeChartSteps - a.start = "1" - assert(b.start == "") + a.start = Some(1) + assert(b.start.isEmpty) } }
