From e59eee9c799bc7b125ed79b6c71bd288e9e2ba38 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Mon, 4 May 2026 00:30:05 -0700 Subject: [PATCH 1/3] default lines to empty list and assert nonEmpty to prevent NullPointerException --- .../operator/visualization/lineChart/LineChartOpDesc.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDesc.scala index 62cabb12a86..a0efe856b7d 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDesc.scala @@ -45,7 +45,7 @@ class LineChartOpDesc extends PythonOperatorDescriptor { var xLabel: EncodableString = "" @JsonProperty(value = "lines", required = true) - var lines: util.List[LineConfig] = _ + var lines: util.List[LineConfig] = new util.ArrayList[LineConfig]() override def getOutputSchemas( inputSchemas: Map[PortIdentity, Schema] @@ -64,6 +64,7 @@ class LineChartOpDesc extends PythonOperatorDescriptor { ) def createPlotlyFigure(): PythonTemplateBuilder = { + assert(lines.asScala.nonEmpty, "At least one line must be configured") val linesPart = lines.asScala .map { lineConf => val colorPart = if (lineConf.color != "") { From 34cc7594deeaddda11bfd98cb25eccabf5fbf0e8 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Mon, 4 May 2026 00:39:40 -0700 Subject: [PATCH 2/3] Added unit test for empty list and assert nonEmpty to prevent NullPointerException --- .../lineChart/LineChartOpDescSpec.scala | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala index 26084370983..a401e7c6d83 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala @@ -96,22 +96,22 @@ class LineChartOpDescSpec extends AnyFlatSpec with Matchers { code should not include "Y_LBL_SENT" } - it should "raise NullPointerException when lines is left at its null default" in { - // Pin: `var lines: util.List[LineConfig] = _` defaults to null, and - // `createPlotlyFigure` calls `lines.asScala.map(...)` without a null - // check. Calling `generatePythonCode` on a default-constructed LineChart - // therefore throws NPE rather than rendering an empty chart or raising - // an AssertionError. Documenting so a future fix that null-guards lines - // breaks this spec deliberately. + it should "raise AssertionError when lines is left at its default (empty list)" in { + // `var lines: util.List[LineConfig]` defaults to an empty ArrayList. + // `createPlotlyFigure` asserts nonEmpty on lines before iterating, so a + // default-constructed LineChartOpDesc raises AssertionError with a + // descriptive message rather than proceeding with no traces. val op = new LineChartOpDesc - assertThrows[NullPointerException](op.generatePythonCode()) + val ex = intercept[AssertionError](op.generatePythonCode()) + ex.getMessage should include("At least one line must be configured") } - it should "render code with an empty lines list (no NPE, no assertion)" in { + it should "raise AssertionError when lines is explicitly set to an empty list" in { + // `createPlotlyFigure` guards against an empty lines list with an assertion, + // matching the fail-fast pattern used by sibling visualizers + // (HeatMapOpDesc, BarChartOpDesc). val op = configured op.lines = new util.ArrayList[LineConfig]() - val code = op.generatePythonCode() - code should include("plotly") - code should include("fig = go.Figure()") + assertThrows[AssertionError](op.generatePythonCode()) } } From 383b1b9b0b52ac8aa01209bddf0b55e1bee98ac8 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Mon, 4 May 2026 17:12:16 -0700 Subject: [PATCH 3/3] null-guard lines before assertion to prevent NPE on explicit null assignment --- .../visualization/lineChart/LineChartOpDesc.scala | 5 +++-- .../lineChart/LineChartOpDescSpec.scala | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDesc.scala index a0efe856b7d..3d0452e8214 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDesc.scala @@ -64,8 +64,9 @@ class LineChartOpDesc extends PythonOperatorDescriptor { ) def createPlotlyFigure(): PythonTemplateBuilder = { - assert(lines.asScala.nonEmpty, "At least one line must be configured") - val linesPart = lines.asScala + val configuredLines = Option(lines).getOrElse(new util.ArrayList[LineConfig]()) + assert(configuredLines.asScala.nonEmpty, "At least one line must be configured") + val linesPart = configuredLines.asScala .map { lineConf => val colorPart = if (lineConf.color != "") { pyb"line={'color':${lineConf.color}}, marker={'color':${lineConf.color}}, " diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala index a401e7c6d83..dc5323538f3 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala @@ -114,4 +114,16 @@ class LineChartOpDescSpec extends AnyFlatSpec with Matchers { op.lines = new util.ArrayList[LineConfig]() assertThrows[AssertionError](op.generatePythonCode()) } + + it should "raise AssertionError rather than NullPointerException when lines is set to null" in { + // `lines` is a public mutable field; Jackson deserializing an explicit JSON + // null or a caller assigning null can set it back to null even after the + // non-null default is in place. `createPlotlyFigure` wraps `lines` in + // `Option(...).getOrElse(emptyList)` before asserting nonEmpty, so a null + // assignment produces the descriptive AssertionError rather than an NPE. + val op = configured + op.lines = null + val ex = intercept[AssertionError](op.generatePythonCode()) + ex.getMessage should include("At least one line must be configured") + } }