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 71faf440de feat(visualization): retarget attributeTypeRules at the
properties they name (#7249)
71faf440de is described below
commit 71faf440de7936d07451d214dbd5777e6906b949
Author: Kary Zheng <[email protected]>
AuthorDate: Wed Aug 12 21:46:55 2026 +0000
feat(visualization): retarget attributeTypeRules at the properties they
name (#7249)
### What changes were proposed in this PR?
Four `attributeTypeRules` name keys that match no property, so the
property editor's `findAttributeType` returns `undefined` and
`checkConstraint` returns without checking anything.
`LineConfig` used the Scala field names rather than the `@JsonProperty`
names, and `ScatterMatrixChartOpDesc` named `value` where the property
is `Selected Attributes`; both are retargeted at the property they
meant. `Scatter3dChartOpDesc` and `FunnelPlotOpDesc` constrained a
`title` that neither operator declares, and did so with the bare string
`"string"` rather than an object, so even under a correct key
`constraint.enum` / `const` / `allOf` would all be undefined and the
check would still no-op; those two rules are removed rather than
invented anew.
### Why are the changes needed?
A line chart's x and y axes accept string columns today despite
declaring `["integer", "long", "double"]`, and the same holds for the
scatter matrix's dimensions. `BandConfig` extends `LineConfig` and
inherited the same dead rule. Nothing reports a key that names no
property, so the rules read as enforced while enforcing nothing.
### Any related issues, documentation, discussions?
Closes #7210
### How was this PR tested?
`WorkflowOperator/compile`, `WorkflowOperator/scalafmtCheckAll`, and the
four operators' existing descriptor specs (24 tests, all passing).
The new `AttributeTypeRuleTargetSpec` guards the class of mistake
repo-wide rather than just the four sites fixed here: it walks every
registered operator's generated schema and fails if an
`attributeTypeRules` key names no declared property, or if a rule is not
stated as an object.
### Does this PR introduce any user-facing change?
Yes. Selecting a non-numeric column for a line chart's axes or the
scatter matrix's Selected Attributes now shows the type warning the rule
always intended. Nothing changes for a numeric column, and removing the
two `title` rules changes nothing at all, since they never applied.
### 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]>
Co-authored-by: Xuan Gu <[email protected]>
---
.../ScatterMatrixChartOpDesc.scala | 5 +-
.../funnelPlot/FunnelPlotOpDesc.scala | 9 +-
.../visualization/lineChart/LineConfig.scala | 8 +-
.../scatter3DChart/Scatter3dChartOpDesc.scala | 9 +-
.../metadata/AttributeTypeRuleTargetSpec.scala | 111 +++++++++++++++++++++
5 files changed, 122 insertions(+), 20 deletions(-)
diff --git
a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/ScatterMatrixChart/ScatterMatrixChartOpDesc.scala
b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/ScatterMatrixChart/ScatterMatrixChartOpDesc.scala
index e6397d1877..f30b5cee90 100644
---
a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/ScatterMatrixChart/ScatterMatrixChartOpDesc.scala
+++
b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/ScatterMatrixChart/ScatterMatrixChartOpDesc.scala
@@ -34,10 +34,13 @@ import
org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, Operat
import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
import javax.validation.constraints.{NotEmpty, NotNull}
+// Type constraint: every axis of the matrix is plotted numerically. The key
is the
+// PROPERTY name -- the `@JsonProperty` value, which is what the property
editor looks
+// up -- and this operator's is "Selected Attributes"; keyed "value" it
matched nothing.
@JsonSchemaInject(json = """
{
"attributeTypeRules": {
- "value": {
+ "Selected Attributes": {
"enum": ["integer", "long", "double"]
}
}
diff --git
a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/funnelPlot/FunnelPlotOpDesc.scala
b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/funnelPlot/FunnelPlotOpDesc.scala
index cd9f38619d..eb83ed5000 100644
---
a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/funnelPlot/FunnelPlotOpDesc.scala
+++
b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/funnelPlot/FunnelPlotOpDesc.scala
@@ -20,7 +20,7 @@
package org.apache.texera.amber.operator.visualization.funnelPlot
import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription}
-import com.kjetland.jackson.jsonSchema.annotations.{JsonSchemaInject,
JsonSchemaTitle}
+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
@@ -31,13 +31,6 @@ import
org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, Operat
import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
import javax.validation.constraints.NotNull
-@JsonSchemaInject(json = """
-{
- "attributeTypeRules": {
- "title": "string"
- }
-}
-""")
class FunnelPlotOpDesc extends PythonOperatorDescriptor {
@JsonProperty(required = true)
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 09f682e12e..82b9210c98 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
@@ -26,14 +26,16 @@ import
org.apache.texera.amber.operator.metadata.annotations.AutofillAttributeNa
import javax.validation.constraints.NotNull
-//type constraint: value can only be numeric
+// Type constraint: both axes can only be numeric. The keys are the PROPERTY
names --
+// the `@JsonProperty` values, which is what the property editor looks up --
not the
+// Scala field names; keyed by the latter the rule matched nothing.
@JsonSchemaInject(json = """
{
"attributeTypeRules": {
- "yValue": {
+ "y": {
"enum": ["integer", "long", "double"]
},
- "xValue": {
+ "x": {
"enum": ["integer", "long", "double"]
}
}
diff --git
a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/scatter3DChart/Scatter3dChartOpDesc.scala
b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/scatter3DChart/Scatter3dChartOpDesc.scala
index e9976f56e3..c86d9e3cc6 100644
---
a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/scatter3DChart/Scatter3dChartOpDesc.scala
+++
b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/scatter3DChart/Scatter3dChartOpDesc.scala
@@ -20,7 +20,7 @@
package org.apache.texera.amber.operator.visualization.scatter3DChart
import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription}
-import com.kjetland.jackson.jsonSchema.annotations.{JsonSchemaInject,
JsonSchemaTitle}
+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
@@ -31,13 +31,6 @@ import
org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, Operat
import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
import javax.validation.constraints.NotNull
-@JsonSchemaInject(json = """
-{
- "attributeTypeRules": {
- "title": "string"
- }
-}
-""")
class Scatter3dChartOpDesc extends PythonOperatorDescriptor {
@JsonProperty(value = "x", required = true)
@JsonSchemaTitle("X Column")
diff --git
a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/AttributeTypeRuleTargetSpec.scala
b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/AttributeTypeRuleTargetSpec.scala
new file mode 100644
index 0000000000..0bd4b2b8f1
--- /dev/null
+++
b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/AttributeTypeRuleTargetSpec.scala
@@ -0,0 +1,111 @@
+/*
+ * 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.metadata
+
+import com.fasterxml.jackson.databind.JsonNode
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import scala.jdk.CollectionConverters._
+
+/**
+ * Guard for `attributeTypeRules`, over every registered operator.
+ *
+ * The property editor looks a rule's key up among the enclosing object's
properties,
+ * by the `@JsonProperty` name. A key that names no property, or a rule that
is not an
+ * object, makes the editor's constraint check return without checking
anything -- the
+ * rule reads as enforced while enforcing nothing, and no existing test
notices,
+ * because the operator still compiles and still runs.
+ *
+ * Sweeping every operator rather than the ones a fix happened to touch is
deliberate:
+ * the mistake is invisible at the declaration site, so it is as easy to make
in the
+ * next operator as it was in these.
+ */
+class AttributeTypeRuleTargetSpec extends AnyFlatSpec with Matchers {
+
+ private val RulesKeyword = "attributeTypeRules"
+
+ /** One declared rule, and the properties it could legally have named. */
+ private case class Rule(
+ schemaPath: String,
+ key: String,
+ value: JsonNode,
+ declaredProperties: Set[String]
+ ) {
+ override def toString: String =
+ s"$schemaPath.$RulesKeyword.$key " +
+ s"(declared properties at $schemaPath:
${declaredProperties.toSeq.sorted.mkString(", ")})"
+ }
+
+ /**
+ * Rules are injected per class, so they appear both at the top level and,
for a
+ * nested config class, under its own definition -- each alongside the
properties it
+ * constrains. Collect them wherever they sit.
+ */
+ private def rulesIn(node: JsonNode, schemaPath: String): Seq[Rule] = {
+ if (!node.isObject) return Seq.empty
+
+ val here = if (node.has(RulesKeyword)) {
+ val rules = node.path(RulesKeyword)
+ val properties = node.path("properties").fieldNames().asScala.toSet
+ rules.fieldNames().asScala.toSeq.map { key =>
+ Rule(schemaPath, key, rules.path(key), properties)
+ }
+ } else Seq.empty
+
+ here ++ node
+ .fields()
+ .asScala
+ .toSeq
+ .flatMap(child => rulesIn(child.getValue,
s"$schemaPath.${child.getKey}"))
+ }
+
+ private val rulesByOperator: Seq[(String, Seq[Rule])] =
+ OperatorMetadataGenerator.operatorTypeMap.keys.toSeq
+ .map(opClass =>
+ opClass.getSimpleName -> rulesIn(
+ OperatorMetadataGenerator.generateOperatorJsonSchema(opClass),
+ "$"
+ )
+ )
+ .filter { case (_, rules) => rules.nonEmpty }
+ .sortBy { case (operator, _) => operator }
+
+ // Without this the per-operator cases below would all vanish, and the suite
would
+ // pass by testing nothing.
+ "Some operator" should s"declare $RulesKeyword, so the cases below are not
vacuous" in {
+ rulesByOperator should not be empty
+ }
+
+ rulesByOperator.foreach {
+ case (operator, rules) =>
+ behavior of s"The $RulesKeyword on $operator"
+
+ it should "name only properties the generated schema declares" in {
+ val unmatched = rules.filterNot(rule =>
rule.declaredProperties.contains(rule.key))
+ withClue("rules naming no property: ")(unmatched shouldBe empty)
+ }
+
+ it should "state each rule as an object, so the constraint is readable"
in {
+ val notObjects = rules.filterNot(_.value.isObject)
+ withClue("rules that are not objects: ")(notObjects shouldBe empty)
+ }
+ }
+}
