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-7257-357296db573a0b51b3d662d17a4113fce9ed50f7 in repository https://gitbox.apache.org/repos/asf/texera.git
commit a1e64cdd18bee57dcc0f143c2bc6fe9a8d6b2af6 Author: Kary Zheng <[email protected]> AuthorDate: Fri Aug 7 17:21:44 2026 -0700 feat(visualization): constrain the colour settings to what plotly accepts (#7257) ### What changes were proposed in this PR? Three colour settings — Line Chart's Line Color, Continuous Error Bands' Fill Color, and Figure Factory Table's Font Color — were plain strings whose only statement of what is legal lived in a description. Each now declares a `pattern` mirroring `ColorValidator` in plotly's `_plotly_utils/basevalidators.py`: 3- or 6-digit hex, an `rgb`/`rgba`/`hsl`/`hsla`/`hsv`/`hsva` call, a `var(--…)` variable, or a colour name. Empty stays legal, since every one of these paths omits the colour argument when the field is blank. Two details are deliberate. Letters are matched through character classes rather than an inline `(?i)`, because the browser compiles the pattern with `new RegExp`. And `\s*` sits between every element, because plotly strips spaces before matching and really does accept `#ff ffff`; rejecting those would make the field stricter than the library it feeds. The colour-name branch is lexical, so a misspelling still reaches plotly — matching exactly would mean copying plotly's 148 CSS names into the annotation. ### Why are the changes needed? Nothing inspected the value before the run, so a mistyped colour aborted a workflow inside plotly with an error naming a plotly property rather than the field the user filled in: `ValueError: Invalid value of type 'builtins.str' received for the 'color' property of scatter.line`. ### Any related issues, documentation, discussions? Closes #7252 ### How was this PR tested? `scalafmtCheckAll`, `scalafixAll --check`, and the three operators' descriptor specs. The new `ColorPatternSpec` covers the pattern: it reads it back out of the generated schema for each of the four fields that carry it, then tests sixteen values — ten accepted, one per branch, and six rejected near-misses — as one test per case, 69 in total. The same sixteen were checked under `new RegExp` and against plotly itself, which agrees on every one except a misspelled colour name, as noted above. ### Does this PR introduce any user-facing change? Yes. The three fields now reject `1`, `#12`, `#ggg`, `#ffff`, `rgb(1,2)` and `rgb(-1,2,3)` in the form. Every value that previously produced a chart still does. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]> --- .../continuousErrorBands/BandConfig.scala | 8 +- .../FigureFactoryTableOpDesc.scala | 8 +- .../visualization/lineChart/LineConfig.scala | 10 ++ .../operator/visualization/ColorPatternSpec.scala | 127 +++++++++++++++++++++ 4 files changed, 151 insertions(+), 2 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/continuousErrorBands/BandConfig.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/continuousErrorBands/BandConfig.scala index 1589c2a8de..9d360212bb 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/continuousErrorBands/BandConfig.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/continuousErrorBands/BandConfig.scala @@ -20,7 +20,7 @@ package org.apache.texera.amber.operator.visualization.continuousErrorBands import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} -import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle +import com.kjetland.jackson.jsonSchema.annotations.{JsonSchemaInject, JsonSchemaTitle} import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString import org.apache.texera.amber.operator.metadata.annotations.AutofillAttributeName import org.apache.texera.amber.operator.visualization.lineChart.LineConfig @@ -43,8 +43,14 @@ class BandConfig extends LineConfig { @NotNull(message = "Y-Axis Lower Bound cannot be empty") var yLower: EncodableString = "" + // Same shapes plotly accepts for a colour as LineConfig's line colour -- see there. @JsonProperty(required = false) @JsonSchemaTitle("Fill Color") @JsonPropertyDescription("must be a valid CSS color or hex color string") + @JsonSchemaInject(json = """ +{ + "pattern": "^\\s*$|^\\s*#(?:\\s*[0-9a-fA-F]){3}(?:(?:\\s*[0-9a-fA-F]){3})?\\s*$|^\\s*(?:[rR]\\s*[gG]\\s*[bB]|[hH]\\s*[sS]\\s*[lL]|[hH]\\s*[sS]\\s*[vV])(?:\\s*[aA])?\\s*\\(\\s*(?:\\s*[0-9.])+(?:\\s*%)?(?:\\s*,(?:\\s*[0-9.])+(?:\\s*%)?){2,3}\\s*\\)\\s*$|^\\s*[vV]\\s*[aA]\\s*[rR]\\s*\\(\\s*-\\s*-[^)]*\\)\\s*$|^\\s*[a-zA-Z][a-zA-Z\\s]*$" +} +""") var fillColor: EncodableString = "" } diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/figureFactoryTable/FigureFactoryTableOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/figureFactoryTable/FigureFactoryTableOpDesc.scala index c1c94f7b12..f9aa6bcd2b 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/figureFactoryTable/FigureFactoryTableOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/figureFactoryTable/FigureFactoryTableOpDesc.scala @@ -20,7 +20,7 @@ package org.apache.texera.amber.operator.visualization.figureFactoryTable import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} -import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle +import com.kjetland.jackson.jsonSchema.annotations.{JsonSchemaInject, 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 @@ -38,9 +38,15 @@ class FigureFactoryTableOpDesc extends PythonOperatorDescriptor { @DecimalMin(value = "0", message = "Font size must be a non-negative number") var fontSize: Double = 12 + // Same shapes plotly accepts for a colour as LineConfig's line colour -- see there. @JsonProperty(required = false) @JsonSchemaTitle("Font Color (Hex Code)") @JsonPropertyDescription("Font color of the Figure Factory Table") + @JsonSchemaInject(json = """ +{ + "pattern": "^\\s*$|^\\s*#(?:\\s*[0-9a-fA-F]){3}(?:(?:\\s*[0-9a-fA-F]){3})?\\s*$|^\\s*(?:[rR]\\s*[gG]\\s*[bB]|[hH]\\s*[sS]\\s*[lL]|[hH]\\s*[sS]\\s*[vV])(?:\\s*[aA])?\\s*\\(\\s*(?:\\s*[0-9.])+(?:\\s*%)?(?:\\s*,(?:\\s*[0-9.])+(?:\\s*%)?){2,3}\\s*\\)\\s*$|^\\s*[vV]\\s*[aA]\\s*[rR]\\s*\\(\\s*-\\s*-[^)]*\\)\\s*$|^\\s*[a-zA-Z][a-zA-Z\\s]*$" +} +""") var fontColor: EncodableString = "#000000" @JsonProperty(required = false) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineConfig.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineConfig.scala index 1a6378be73..09f682e12e 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineConfig.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/lineChart/LineConfig.scala @@ -68,9 +68,19 @@ class LineConfig { @JsonSchemaTitle("Line Name") var name: EncodableString = "" + // Mirrors ColorValidator in plotly's _plotly_utils/basevalidators.py. Character + // classes rather than an inline `(?i)`, because the browser compiles this with + // `new RegExp`. `\s*` between every element, because plotly strips spaces first and + // so really does accept `#ff ffff`. The name branch stays lexical: matching exactly + // would mean copying plotly's 148 CSS names in here. @JsonProperty(value = "color", required = false) @JsonSchemaTitle("Line Color") @JsonPropertyDescription("must be a valid CSS color or hex color string") + @JsonSchemaInject(json = """ +{ + "pattern": "^\\s*$|^\\s*#(?:\\s*[0-9a-fA-F]){3}(?:(?:\\s*[0-9a-fA-F]){3})?\\s*$|^\\s*(?:[rR]\\s*[gG]\\s*[bB]|[hH]\\s*[sS]\\s*[lL]|[hH]\\s*[sS]\\s*[vV])(?:\\s*[aA])?\\s*\\(\\s*(?:\\s*[0-9.])+(?:\\s*%)?(?:\\s*,(?:\\s*[0-9.])+(?:\\s*%)?){2,3}\\s*\\)\\s*$|^\\s*[vV]\\s*[aA]\\s*[rR]\\s*\\(\\s*-\\s*-[^)]*\\)\\s*$|^\\s*[a-zA-Z][a-zA-Z\\s]*$" +} +""") var color: EncodableString = "" } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/ColorPatternSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/ColorPatternSpec.scala new file mode 100644 index 0000000000..4708fb58d7 --- /dev/null +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/ColorPatternSpec.scala @@ -0,0 +1,127 @@ +/* + * 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.operator.visualization + +import org.apache.texera.amber.operator.LogicalOp +import org.apache.texera.amber.operator.metadata.OperatorMetadataGenerator +import org.apache.texera.amber.operator.visualization.continuousErrorBands.ContinuousErrorBandsOpDesc +import org.apache.texera.amber.operator.visualization.figureFactoryTable.FigureFactoryTableOpDesc +import org.apache.texera.amber.operator.visualization.lineChart.LineChartOpDesc +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.util.regex.Pattern + +/** + * Covers the colour `pattern` the visualization settings inject into their schema, one + * test per field per value. + * + * The values are the shapes plotly's ColorValidator accepts, one per branch of the + * pattern, plus the near-misses it rejects. A colour name is matched lexically, so + * `red` stands for the branch rather than for the CSS list. + */ +class ColorPatternSpec extends AnyFlatSpec with Matchers { + + /** A value the field should take, and whether the pattern is meant to admit it. */ + private val colorValues: Seq[(String, Boolean)] = Seq( + "" -> true, // blank means the operator omits the colour argument + "#fff" -> true, + "#FFFFFF" -> true, + "#ff ffff" -> true, // plotly strips spaces before matching + "rgb(255, 0, 0)" -> true, + "rgba(255, 0, 0, 0.5)" -> true, + "hsl(120, 50%, 50%)" -> true, + "hsva(120, 50%, 50%, 0.5)" -> true, + "var(--my-color)" -> true, + "red" -> true, + "1" -> false, + "#12" -> false, + "#ggg" -> false, + "#ffff" -> false, + "rgb(1,2)" -> false, + "rgb(-1,2,3)" -> false + ) + + // Every field carrying the colour pattern, as (label, operator, path to the property). + // BandConfig appears twice: it declares fillColor and inherits color from LineConfig. + private val colorFields: Seq[(String, Class[_ <: LogicalOp], Seq[String])] = Seq( + ( + "LineConfig.color", + classOf[LineChartOpDesc], + Seq("definitions", "LineConfig", "properties", "color") + ), + ( + "BandConfig.fillColor", + classOf[ContinuousErrorBandsOpDesc], + Seq("definitions", "BandConfig", "properties", "fillColor") + ), + ( + "BandConfig.color", + classOf[ContinuousErrorBandsOpDesc], + Seq("definitions", "BandConfig", "properties", "color") + ), + ( + "FigureFactoryTableOpDesc.fontColor", + classOf[FigureFactoryTableOpDesc], + Seq("properties", "fontColor") + ) + ) + + private def patternOf(opDescClass: Class[_ <: LogicalOp], path: Seq[String]): Option[String] = { + val property = path.foldLeft(OperatorMetadataGenerator.generateOperatorJsonSchema(opDescClass))( + (node, segment) => node.path(segment) + ) + Option.when(property.has("pattern"))(property.path("pattern").asText()) + } + + // Read every schema once, up front, rather than once per case below. + private val colorPatterns: Seq[(String, Option[String])] = colorFields.map { + case (label, opDescClass, path) => label -> patternOf(opDescClass, path) + } + + private def describe(value: String): String = + if (value.isEmpty) "a blank value" else s"'$value'" + + colorPatterns.foreach { + case (label, pattern) => + behavior of s"The colour pattern on $label" + + it should "be present in the generated schema" in { + pattern shouldBe defined + } + + colorValues.foreach { + case (value, isValid) => + val verb = if (isValid) "accept" else "reject" + it should s"$verb ${describe(value)}" in { + val regex = Pattern.compile(pattern.getOrElse(fail(s"$label carries no pattern"))) + // find() rather than matches(), because the form validates with + // `new RegExp().test`, which searches instead of anchoring. + regex.matcher(value).find() shouldBe isValid + } + } + } + + behavior of "The colour pattern" + + it should "read identically from every schema, so the copies cannot drift" in { + colorPatterns.map { case (_, pattern) => pattern }.distinct should have size 1 + } +}
