Linz1248 opened a new pull request, #11985:
URL: https://github.com/apache/seatunnel/pull/11985
## Purpose of this pull request
Part of #11007. Supersedes #11972 (closed due to an accidental force-push;
this PR carries the same changes plus the review fix below).
Migrates `connector-pulsar` imperative validation to the declarative
`OptionRule` framework (following #10976 / #10977 and the pattern in #11095),
and adds factory-level validation tests.
## Changes
- **Remove redundant presence checks** in builder classes
- `PulsarConsumerConfig.Builder#build`: the non-blank subscription-name
check was unreachable — every consumer config is built from a
`PulsarTableConfig` produced by `PulsarMultiTableConfig.of`, which validates
`subscription.name` in `validateTableConfig` before consumer construction.
- `PulsarAdminConfig.Builder#build` / `PulsarClientConfig.Builder#build`:
the non-blank URL checks were already enforced by the factories'
`required(CLIENT_SERVICE_URL, ADMIN_SERVICE_URL)`.
- **Add `Conditions.notBlank(...)` value constraints** (addresses [review
feedback](https://github.com/apache/seatunnel/pull/11972#pullrequestreview)
from @DanielLeens on #11972)
- `OptionRule.required()` only checks non-null presence, not non-blank
content — a blank value like `""` or `" "` passed validation and failed later
with a worse error.
- Added `notBlank` constraints on `client.service-url`,
`admin.service-url` (both factories) and `subscription.name` (source factory)
so blank values are caught declaratively at submission time.
- `subscription.name` stays `.optional(option, notBlank(option))` — not
promoted to `required` to avoid regressing multi-table jobs that only set
per-table subscriptions.
- **Keep runtime-only validation**: the transaction-coordinator check in
`PulsarConfigUtil` depends on client state, not configuration, and stays
imperative per the migration guide.
- **Add factory validation tests**: `ConfigValidator`-based tests covering
required, exclusive, conditional, bundled rules, **and blank-value cases** for
each affected option (5 new blank-string tests).
## Does this PR introduce _any_ user-facing change?
No. The same conditions are still enforced (now earlier and declaratively),
so valid and invalid configurations behave the same.
## How was this patch tested?
- 65 connector-pulsar tests pass (`Tests run: 65, Failures: 0, Errors: 0`),
including the new factory validation tests and blank-value tests.
- `mvn spotless:check` on the module passes.
## Check list
* [ ] If any new Jar binary package is added, add a License Notice per the
[New License Guide](../../../docs/en/developer/new-license.md)
* [ ] If necessary, update the documentation to describe the new feature
* [ ] If necessary, update `incompatible-changes.md` to describe
incompatibility caused by the PR
* [ ] If you contribute connector code, check that the following files are
updated:
1. Update `plugin-mapping.properties` and add new connector information
2. Update the pom file of `seatunnel-dist`
3. Add CI label in `label-scope-conf`
4. Add E2E testcase in `seatunnel-e2e`
5. Update connector `plugin_config`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
