(texera) 01/01: chore(workflow-operator): remove the unreferenced PropertyNameConstants (#8333)

[gi...@apache.org]

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-8333-662387be647006356ed45e45ab988140bf178b94
in repository https://gitbox.apache.org/repos/asf/texera.git

commit 5bc7f49a75bcaee257fe24bf7dfd91a83224f9d0
Author: Xinyuan Lin <xi...@uci.edu>
AuthorDate: Sun Sep 27 22:08:33 2026 +0000

    chore(workflow-operator): remove the unreferenced PropertyNameConstants (#8333)
    
    ### What changes were proposed in this PR?
    
    `PropertyNameConstants` declares 21 constants; only two are referenced
    anywhere outside its own spec. This removes the other 19. Pure deletion,
    no behaviour change: **−95 lines**.
    
    `LogicalOp` uses `OPERATOR_ID` and `OPERATOR_VERSION` as `@JsonProperty`
    annotation arguments — those are live and stay. Each of the other 19 has
    exactly three references, all inside `PropertyNameConstantsSpec`.
    
    ### History
    
    | | |
    | --- | --- |
    | **Introduced by** | the initial amber import, commit `41a8a92017`
    (2020-08-20) — predates the PR workflow |
    | **Usage removed by** | **never adopted** — operator descriptors always
    spelled their JSON keys as raw string literals, and the wire format is
    anchored elsewhere (`@JsonTypeInfo(property = "operatorType")` in
    `LogicalOp`, plus literals in `OperatorMetadataGenerator` and
    `WorkflowResource`) |
    
    These constants were never the single source of truth they were meant to
    be, which is why deleting them cannot change the serialized format.
    
    > Reviewer note: `TABLE_NAME` **is** included. Its only qualified
    references are in the spec; the bare `TABLE_NAME` hits elsewhere are an
    unrelated SQL placeholder in `sql/misc/tweets.sql`. The spec keeps its
    three tests, narrowed to the two surviving constants — including the
    distinctness and whitespace checks, which still hold.
    
    ### Any related issues, documentation, discussions?
    
    Closes #8330
    
    ### How was this PR tested?
    
    Existing tests only — this PR adds none; it narrows the assertions to
    the constants that remain.
    
    Locally, from the repo root with Java 17:
    
    - `sbt "WorkflowExecutionService/Test/compile"` — success.
    - `sbt "WorkflowOperator/testOnly *PropertyNameConstantsSpec"` — 3
    tests, all pass.
    - `sbt scalafmtCheckAll "scalafixAll --check"` — clean.
    
    Verification, re-runnable by a reviewer:
    
    ```
    git grep -oh "PropertyNameConstants\.[A-Z_]*" -- . ':!*PropertyNameConstants.scala' | sort | uniq -c
    ```
    
    Before this change every constant shows exactly 3 hits (its spec
    assertions) except `OPERATOR_ID` and `OPERATOR_VERSION`, which show 4 —
    the extra one being `LogicalOp`.
    
    ### Was this PR authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Code (Claude Opus 5)
---
 .../operator/metadata/PropertyNameConstants.scala  | 24 -------
 .../metadata/PropertyNameConstantsSpec.scala       | 75 +---------------------
 2 files changed, 2 insertions(+), 97 deletions(-)

diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala
index 52bb041469..01c02a6e3e 100644
--- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala
+++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala
@@ -27,29 +27,5 @@ package org.apache.texera.amber.operator.metadata
   */
 object PropertyNameConstants { // logical plan property names
   final val OPERATOR_ID = "operatorID"
-  final val OPERATOR_TYPE = "operatorType"
-  final val ORIGIN_OPERATOR_ID = "origin"
-  final val DESTINATION_OPERATOR_ID = "destination"
-  final val OPERATOR_LIST = "operators"
-  final val OPERATOR_LINK_LIST = "links"
   final val OPERATOR_VERSION = "operatorVersion"
-  // common operator property names
-  final val ATTRIBUTE_NAMES = "attributes"
-  final val ATTRIBUTE_NAME = "attribute"
-  final val RESULT_ATTRIBUTE_NAME = "resultAttribute"
-  final val SPAN_LIST_NAME = "spanListName"
-  final val TABLE_NAME = "tableName"
-
-  // physical plan property names
-  final val WORKFLOW_ID = "workflowID"
-  final val EXECUTION_ID = "executionID"
-  final val PARALLELIZABLE = "parallelizable"
-  final val LOCATION_PREFERENCE = "locationPreference"
-  final val PARTITION_REQUIREMENT = "partitionRequirement"
-  // derivePartition is a function type that cannot be serialized
-  final val INPUT_PORTS = "inputPorts"
-  final val OUTPUT_PORTS = "outputPorts"
-  // propagateSchema is a function type that cannot be serialized
-  final val IS_ONE_TO_MANY_OP = "isOneToManyOp"
-  final val SUGGESTED_WORKER_NUM = "suggestedWorkerNum"
 }
diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstantsSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstantsSpec.scala
index 9afd3b7d0b..455837efe2 100644
--- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstantsSpec.scala
+++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstantsSpec.scala
@@ -30,42 +30,9 @@ class PropertyNameConstantsSpec extends AnyFlatSpec {
 
   "PropertyNameConstants logical-plan keys" should "have their canonical String values" in {
     assert(PropertyNameConstants.OPERATOR_ID == "operatorID")
-    assert(PropertyNameConstants.OPERATOR_TYPE == "operatorType")
-    assert(PropertyNameConstants.ORIGIN_OPERATOR_ID == "origin")
-    assert(PropertyNameConstants.DESTINATION_OPERATOR_ID == "destination")
-    assert(PropertyNameConstants.OPERATOR_LIST == "operators")
-    assert(PropertyNameConstants.OPERATOR_LINK_LIST == "links")
     assert(PropertyNameConstants.OPERATOR_VERSION == "operatorVersion")
   }
 
-  // ---------------------------------------------------------------------------
-  // Common operator-property keys
-  // ---------------------------------------------------------------------------
-
-  "PropertyNameConstants common-property keys" should "have their canonical String values" in {
-    assert(PropertyNameConstants.ATTRIBUTE_NAMES == "attributes")
-    assert(PropertyNameConstants.ATTRIBUTE_NAME == "attribute")
-    assert(PropertyNameConstants.RESULT_ATTRIBUTE_NAME == "resultAttribute")
-    assert(PropertyNameConstants.SPAN_LIST_NAME == "spanListName")
-    assert(PropertyNameConstants.TABLE_NAME == "tableName")
-  }
-
-  // ---------------------------------------------------------------------------
-  // Physical-plan keys
-  // ---------------------------------------------------------------------------
-
-  "PropertyNameConstants physical-plan keys" should "have their canonical String values" in {
-    assert(PropertyNameConstants.WORKFLOW_ID == "workflowID")
-    assert(PropertyNameConstants.EXECUTION_ID == "executionID")
-    assert(PropertyNameConstants.PARALLELIZABLE == "parallelizable")
-    assert(PropertyNameConstants.LOCATION_PREFERENCE == "locationPreference")
-    assert(PropertyNameConstants.PARTITION_REQUIREMENT == "partitionRequirement")
-    assert(PropertyNameConstants.INPUT_PORTS == "inputPorts")
-    assert(PropertyNameConstants.OUTPUT_PORTS == "outputPorts")
-    assert(PropertyNameConstants.IS_ONE_TO_MANY_OP == "isOneToManyOp")
-    assert(PropertyNameConstants.SUGGESTED_WORKER_NUM == "suggestedWorkerNum")
-  }
-
   // ---------------------------------------------------------------------------
   // Distinctness — no two constants alias to the same string
   // ---------------------------------------------------------------------------
@@ -73,26 +40,7 @@ class PropertyNameConstantsSpec extends AnyFlatSpec {
   "PropertyNameConstants" should "have all constants distinct (no accidental aliases)" in {
     val all = List(
       PropertyNameConstants.OPERATOR_ID,
-      PropertyNameConstants.OPERATOR_TYPE,
-      PropertyNameConstants.ORIGIN_OPERATOR_ID,
-      PropertyNameConstants.DESTINATION_OPERATOR_ID,
-      PropertyNameConstants.OPERATOR_LIST,
-      PropertyNameConstants.OPERATOR_LINK_LIST,
-      PropertyNameConstants.OPERATOR_VERSION,
-      PropertyNameConstants.ATTRIBUTE_NAMES,
-      PropertyNameConstants.ATTRIBUTE_NAME,
-      PropertyNameConstants.RESULT_ATTRIBUTE_NAME,
-      PropertyNameConstants.SPAN_LIST_NAME,
-      PropertyNameConstants.TABLE_NAME,
-      PropertyNameConstants.WORKFLOW_ID,
-      PropertyNameConstants.EXECUTION_ID,
-      PropertyNameConstants.PARALLELIZABLE,
-      PropertyNameConstants.LOCATION_PREFERENCE,
-      PropertyNameConstants.PARTITION_REQUIREMENT,
-      PropertyNameConstants.INPUT_PORTS,
-      PropertyNameConstants.OUTPUT_PORTS,
-      PropertyNameConstants.IS_ONE_TO_MANY_OP,
-      PropertyNameConstants.SUGGESTED_WORKER_NUM
+      PropertyNameConstants.OPERATOR_VERSION
     )
     assert(all.distinct.size == all.size, s"duplicate constant value(s) in: $all")
   }
@@ -100,26 +48,7 @@ class PropertyNameConstantsSpec extends AnyFlatSpec {
   it should "carry no leading/trailing whitespace on any constant" in {
     val all = List(
       PropertyNameConstants.OPERATOR_ID,
-      PropertyNameConstants.OPERATOR_TYPE,
-      PropertyNameConstants.ORIGIN_OPERATOR_ID,
-      PropertyNameConstants.DESTINATION_OPERATOR_ID,
-      PropertyNameConstants.OPERATOR_LIST,
-      PropertyNameConstants.OPERATOR_LINK_LIST,
-      PropertyNameConstants.OPERATOR_VERSION,
-      PropertyNameConstants.ATTRIBUTE_NAMES,
-      PropertyNameConstants.ATTRIBUTE_NAME,
-      PropertyNameConstants.RESULT_ATTRIBUTE_NAME,
-      PropertyNameConstants.SPAN_LIST_NAME,
-      PropertyNameConstants.TABLE_NAME,
-      PropertyNameConstants.WORKFLOW_ID,
-      PropertyNameConstants.EXECUTION_ID,
-      PropertyNameConstants.PARALLELIZABLE,
-      PropertyNameConstants.LOCATION_PREFERENCE,
-      PropertyNameConstants.PARTITION_REQUIREMENT,
-      PropertyNameConstants.INPUT_PORTS,
-      PropertyNameConstants.OUTPUT_PORTS,
-      PropertyNameConstants.IS_ONE_TO_MANY_OP,
-      PropertyNameConstants.SUGGESTED_WORKER_NUM
+      PropertyNameConstants.OPERATOR_VERSION
     )
     all.foreach(c => assert(c == c.trim, s"constant has surrounding whitespace: '$c'"))
   }