apache / apache/seatunnel

[Feature][API] Simplify condition chain model by removing (Expression)

Open
#11,052 5 comments 0 reactions 1 assignee Claimed by @nzw921rx View on GitHub
design feature
Dominant language
Java
Stars
9.7k
Forks
2.4k
Avg merge
3d 9h
Merged PRs (30d)
204

Description

### Search before asking

- [x] I had searched in the [feature](https://github.com/apache/seatunnel/issues?q=is%3Aissue+label%3A%22Feature%22) and found no similar feature requirement.

### Description

## Problem statement

Today we maintain two abstractions for the same concept:

- `Condition` already supports linked condition chains via `.and()` / `.or()`
- `Expression` wraps a `Condition` and re-implements chain composition

As a result, internal code frequently does:

1. build a `Condition` chain
2. wrap it as `Expression`
3. immediately unwrap it again (`getExpression()`) before evaluation or rendering

This indirection increases API surface and maintenance cost without adding semantics.

## Goal

Use `Condition` as the single representation of condition chains.

## Non-goals

- No new precedence rule is introduced (reuse `Condition`'s existing AND/OR evaluation logic)
- No behavior change in error messages
- No behavior change in REST metadata rendering
- No change to connector-facing `OptionRule.builder()` usage patterns

## Proposed API model

| Current | Proposed |
|---|---|
| `Expression.of(Condition.of(option, value))` | `Condition.of(option, value)` |
| `Expression::or` / `Expression::and` | `Condition::or` / `Condition::and` |
| `ConditionRule.expression: Expression` | `ConditionRule.condition: Condition` |
| `ConditionalRequiredOptions.expression: Expression` | `ConditionalRequiredOptions.condition: Condition` |
| `Map` | `Map, OptionRule>` |
| `ConfigValidator.validate(Expression)` | `ConfigValidator.validate(Condition)` |

## Compatibility strategy

Two rollout options:

### Option 1: Deprecate first, remove later

- Keep `Expression` in the current release and mark it as `@Deprecated`
- Migrate all in-repo usages to `Condition`
- Remove `Expression` in the next API cleanup window

When to use: if maintainers want a transition window for potential downstream usage.

### Option 2: Remove directly in this PR

- Delete `Expression.java` directly
- Replace all in-repo usages with `Condition`
- Remove all `Expression` entrypoints in `ConfigValidator` in the same PR

When to use: if maintainers confirm external usage is minimal and want to reduce model complexity immediately.

## Impact scope (direct changes)

- `Expression.java`: delete
- `OptionRule.java`: remove `Expression` construction path, use `Condition` directly
- `ConditionRule.java`: field type changes from `Expression` to `Condition`
- `RequiredOption.java`: `ConditionalRequiredOptions` changes from `Expression` to `Condition`
- `ConfigValidator.java`: remove all `Expression` entrypoints (including constructor/overload), evaluate via `Condition` only
- `OptionRulesService.java`: replace `getExpression()` with `getCondition()`
- `MetadataExportCommand.java`: switch export path from `Expression` to `Condition`
- tests: replace all `Expression`-related assertions and builders

## Questions to confirm

1. Should we choose **Option 1 (deprecate first)** or **Option 2 (direct removal in this PR)**?
2. Is it acceptable to update `seatunnel-engine` and `seatunnel-core` call paths in this PR (`OptionRulesService` / `MetadataExportCommand`)?
3. Can we remove all `Expression`-typed constructors/overloads in `ConfigValidator` without keeping compatibility shims?
4. Should this change be treated as API cleanup and explicitly called out in PR notes/changelog for downstream users that may import `Expression

### Usage Scenario

_No response_

### Related issues

_No response_

### Are you willing to submit a PR?

- [x] Yes I am willing to submit a PR!

### Code of Conduct

- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.