Refactor workflow input namespaces to typed helper#29739
Conversation
yan-3005
added
the
safe to test
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
There was a problem hiding this comment.
Pull request overview
This PR refactors how Flowable workflow delegates resolve input namespaces by introducing a typed helper (WorkflowVariableHandler.InputNamespaces) and migrating existing delegates away from ad hoc Map handling. It also tightens task payload handling in CreateTask by using schema/record types for Data Access Request (DAR) and recognizer feedback payloads.
Changes:
- Add
WorkflowVariableHandler.InputNamespacesrecord and migrate delegates/listeners to use it. - Update workflow graph validation to consume namespace mappings via the helper.
- Refactor
CreateTaskpayload merging/parsing to useDataAccessRequestPayloadand a typed recognizer payload record, with updated unit tests.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-service/src/test/java/org/openmetadata/service/governance/workflows/WorkflowVariableHandlerInputNamespacesTest.java | Adds unit coverage for parsing/reading the new InputNamespaces helper. |
| openmetadata-service/src/test/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/CreateTaskTest.java | Updates assertions to validate DAR payload merging now returns DataAccessRequestPayload. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/WorkflowVariableHandler.java | Introduces the InputNamespaces typed helper for namespace lookups. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/flowable/MainWorkflow.java | Switches workflow node validation to iterate namespaces via InputNamespaces. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/flowable/BaseDelegate.java | Stores parsed namespaces as InputNamespaces instead of a raw Map. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/SetApprovalAssigneesImpl.java | Uses InputNamespaces for namespaced variable reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/CreateRecognizerFeedbackApprovalTaskImpl.java | Uses InputNamespaces for recognizer/related entity variable resolution. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/CheckFeedbackSubmitterIsReviewerImpl.java | Uses InputNamespaces for recognizer feedback variable resolution. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/AutoApproveServiceTaskImpl.java | Uses InputNamespaces for related entity variable resolution. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/CreateTask.java | Refactors payload namespace reads + DAR expiration handling + recognizer payload typing. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/sink/SinkTaskDelegate.java | Uses InputNamespaces for entity list / related entity namespace selection. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/runApp/RunAppDelegate.java | Uses InputNamespaces for related entity variable resolution. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/SetGlossaryTermStatusImpl.java | Uses InputNamespaces for related entity / updatedBy namespace reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/SetEntityCertificationImpl.java | Uses InputNamespaces for related entity / updatedBy namespace reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/SetEntityAttributeImpl.java | Uses InputNamespaces for related entity / updatedBy namespace reads; simplifies comments. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/RollbackEntityImpl.java | Uses InputNamespaces for related entity / updatedBy namespace reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/RejectRecognizerFeedbackImpl.java | Uses InputNamespaces for recognizer feedback / updatedBy namespace reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/DataCompletenessImpl.java | Uses InputNamespaces for related entity namespace reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/CheckEntityAttributesImpl.java | Uses InputNamespaces for related entity namespace reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/CheckChangeDescriptionTaskImpl.java | Uses InputNamespaces for related entity namespace reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/impl/ApplyRecognizerFeedbackImpl.java | Uses InputNamespaces for recognizer feedback / updatedBy namespace reads. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/createAndRunIngestionPipeline/RunIngestionPipelineDelegate.java | Uses InputNamespaces for workflow input namespace handling. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/createAndRunIngestionPipeline/CreateIngestionPipelineDelegate.java | Uses InputNamespaces for related entity namespace reads. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Code Review ✅ Approved 5 resolved / 5 findingsRefactors workflow input namespaces and task payloads to use typed helper records, eliminating ad-hoc maps and unchecked casts. All session findings related to NPEs, redundant copies, and schema field injection have been resolved. ✅ 5 resolved✅ Edge Case: InputNamespaces throws NPE on null namespace values
✅ Quality: Stale @SuppressWarnings("unchecked") after removing cast
✅ Bug: withGrantExpirationDate injects schema default fields into payload
✅ Edge Case: withGrantExpirationDate still injects schema defaults for non-Map payloads
✅ Quality: Redundant map copy in withGrantExpirationDate
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/flowable/MainWorkflow.java:116
- validateNode() can throw a NullPointerException when an input namespace entry has a null/blank value (namespace.equals(...)). Since InputNamespaces explicitly preserves null values, this should be handled explicitly (e.g., skip null/blank namespaces) to avoid crashing workflow validation with an NPE.
for (Map.Entry<String, String> entry : inputNamespaces.namespaces().entrySet()) {
String variable = entry.getKey();
String namespace = entry.getValue();
if (namespace.equals(GLOBAL_NAMESPACE)) {
Apply the Java simplicity rules from CLAUDE.md to the touched helpers: - single return per method - one-line comment on each instanceof explaining why the input is untyped (Flowable delivers process variables as raw Object, Jackson variable shape for Task.payload) - drop redundant 'public static' on nested records (records are implicitly static; visibility already inherited) - trim commentary in readDataAccessRequestPayload / readRecognizerFeedbackTaskPayload Behavior unchanged. 66 unit tests still pass: mvn -pl openmetadata-service test -Dtest=WorkflowVariableHandlerInputNamespacesTest,CreateTaskTest
yan-3005
force-pushed
the
refactor/input-namespaces-typed
branch
from
95640c4 to
a6e80fd
Compare
# Conflicts: # openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/CreateTask.java
Code Review ✅ Approved 8 resolved / 8 findingsCentralizes workflow input namespace resolution into a typed ✅ 8 resolved✅ Edge Case: InputNamespaces throws NPE on null namespace values
✅ Quality: Stale @SuppressWarnings("unchecked") after removing cast
✅ Bug: withGrantExpirationDate injects schema default fields into payload
✅ Edge Case: withGrantExpirationDate still injects schema defaults for non-Map payloads
✅ Quality: Redundant map copy in withGrantExpirationDate
...and 3 more resolved from earlier reviews OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Summary
WorkflowVariableHandler.InputNamespacesrecord for workflow input namespace mapsCreateTaskpayload handling to use schema/record types for DAR and recognizer feedback payloadsTests
mvn -pl openmetadata-service spotless:applymvn -pl openmetadata-service -Dtest=WorkflowVariableHandlerInputNamespacesTest,CreateTaskTest testgit diff --checkGreptile Summary
This PR refactors how workflow delegates resolve per-input variable namespaces: instead of each delegate calling
JsonUtils.readOrConvertValue(...)and casting to a rawMap<String, String>, all delegates now delegate to the newWorkflowVariableHandler.InputNamespacesrecord, which centralises the JSON/Map dispatch and null handling. TheCreateTaskpayload logic is also lifted from ad-hoc map manipulation to typed schema classes (DataAccessRequestPayload,RecognizerFeedbackTaskPayload).WorkflowVariableHandler.InputNamespacesis a new nested record that handles both String-JSON and pre-deserialized Map inputs, always returns a non-null immutable result, and exposesnamespaceFor/namespaceForOrDefaulthelpers used uniformly across ~15 delegate classes.CreateTask's DAR and recognizer-feedback payload handling is refactored from rawMapmanipulation to typed POJOs; the newreadDataAccessRequestPayload/readRecognizerFeedbackTaskPayloadhelpers now throwIllegalArgumentExceptionon invalid payloads (previously silently falling back) — this is verified by new test cases.RunIngestionPipelineDelegateremoves the now-unusedinputNamespaceMaplocal variable, leavinginputNamespaceMapExprdeclared but never evaluated.Confidence Score: 5/5
Safe to merge — the refactor is mechanical and well-tested; the only finding is a leftover unused field declaration.
All delegate classes are updated consistently to use InputNamespaces. The new null-safety guarantee (compact constructor normalises null to Map.of()) removes a class of NPE risk that existed before. The CreateTask payload changes are covered by explicit exception-path tests. The only rough edge is the orphaned inputNamespaceMapExpr field in RunIngestionPipelineDelegate, which is cosmetic and does not affect runtime behaviour.
RunIngestionPipelineDelegate.java — the inputNamespaceMapExpr field is declared but never evaluated after this change.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD subgraph Before A1[Delegate.execute] -->|readOrConvertValue| B1[Map inputNamespaceMap] B1 -->|.get| C1[namespace String] C1 --> D1[varHandler.getNamespacedVariable] end subgraph After A2[Delegate.execute] -->|InputNamespaces.from or .read| B2[InputNamespaces record] B2 -->|.namespaceFor| C2[namespace or null] B2 -->|.namespaceForOrDefault| C3[namespace with fallback] C2 --> D2[varHandler.getNamespacedVariable] C3 --> D2 end subgraph InputNamespaces.read R1{rawValue instanceof String?} -->|Yes| R2[JsonUtils.readValue] R1 -->|No| R3[JsonUtils.convertValue] R2 --> R4[new InputNamespaces] R3 --> R4 R4 -->|null map| R5[Map.of via compact ctor] R4 -->|non-null| R6[unmodifiableMap LinkedHashMap] end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD subgraph Before A1[Delegate.execute] -->|readOrConvertValue| B1[Map inputNamespaceMap] B1 -->|.get| C1[namespace String] C1 --> D1[varHandler.getNamespacedVariable] end subgraph After A2[Delegate.execute] -->|InputNamespaces.from or .read| B2[InputNamespaces record] B2 -->|.namespaceFor| C2[namespace or null] B2 -->|.namespaceForOrDefault| C3[namespace with fallback] C2 --> D2[varHandler.getNamespacedVariable] C3 --> D2 end subgraph InputNamespaces.read R1{rawValue instanceof String?} -->|Yes| R2[JsonUtils.readValue] R1 -->|No| R3[JsonUtils.convertValue] R2 --> R4[new InputNamespaces] R3 --> R4 R4 -->|null map| R5[Map.of via compact ctor] R4 -->|non-null| R6[unmodifiableMap LinkedHashMap] endReviews (9): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile