unbridled-41 opened a new pull request, #4784:
URL: https://github.com/apache/rocketmq-dashboard/pull/4784
Fixes #4783.
## Problem / Evidence
An alert-rule update that **omits** an optional field cannot clear it.
`MybatisPlusAlertRepository.replaceRule` built the entity and called
`ruleMapper.updateById`, whose default NOT_NULL strategy drops null fields from
the SET clause. Because `AlertService.updateRule` validates the submitted VO,
replaces the stored rule and returns the **submitted** VO, the API responded
with the field cleared while the stored rule kept the old value; the next `GET`
showed it back. A rule scoped to `consumerGroup="G1"` could not be re-scoped to
all groups;
`severity`/`duration`/`channels`/`description`/`notificationTemplate` silently
reverted after an update that omitted them. Line references in the linked issue
are as of trunk `4c697f07`; references below are as of the patched file at this
PR's head.
Reproduction path (issue #4783 has the full chain): update a business
`consumer.lag.total` rule without `consumerGroup` → 200 OK, response shows
`consumerGroup: null` → GET shows the old group still stored and still applied
at evaluation time.
## Root cause / Fix
MyBatis-Plus `updateById` omits null entity fields. `replaceRule` now
follows every cleared optional column with an explicit `UpdateWrapper ...
set(column, null)` pass, the same approach
`NameserverRegistryService.clearOmittedOptionalColumns` ships on trunk (merged
#4466, same defect class for instance and registry optional columns) and
`MybatisPlusInstanceRepository` uses for a cleared credential reference.
`lastTriggered` is not an editable field — it is owned by `markRuleTriggered` —
and keeps the skip-on-null behaviour; the regression pins that boundary.
## Priority
PRIORITY 68 / 100 — impact 26 (silent response-vs-stored-state divergence on
a management API; scope filters keep applying after "removal"), blast radius 14
(every alert-rule update path: REST, import/transfer, bulk), reproducibility 18
(deterministic once the update body omits a field), maintenance value 10
(aligns the last table with the project's own established fix pattern; #4466
shows maintainers accept it). FIX_CONFIDENCE 92: single-method fix, direct
precedent in-tree, no semantic ambiguity beyond the `lastTriggered` boundary,
which the test pins.
## Tests
-
`MybatisPlusAlertRepositoryTest.replaceRuleShouldExplicitlyClearOmittedOptionalColumnsTest`
(new): an update VO with every optional field null must assign
`threshold_unit`, `duration`, `channels`, `description`, `broker_name`,
`cluster_name`, `severity`, `instance_id`, `consumer_group`, `topic`,
`notification_template` to null via the explicit wrapper, and must **not**
include `last_triggered`.
- Teeth, measured on this machine: with only the test change on base
`4c697f07` source, the test fails with `Wanted but not invoked:
ruleMapper.update(...)` (red). With the fix restored it passes (green). A
second discriminating run — a plausible wrong fix that also clears
`last_triggered` — fails on the `doesNotContain("last_triggered=")` assertion,
so the test separates correct from over-clearing implementations.
- `MybatisPlusAlertRepositoryTest` 17/17, plus related suites:
`AlertServiceTest` 83/83, `NativeAlertProcessorTest` 23/23,
`AlertRuleControllerTest` 22/22, `ClusterAlertRuleControllerTest` 6/6.
`NativeAlertEvaluationTransactionTest` errors in this sandbox because it needs
MySQL (Spring context `Communications link failure`) — the same baseline
failure as clean trunk, unrelated to this change. `mvn checkstyle:check` passes.
## Risk
Low. The added statements only run when a cleared column would otherwise be
silently skipped. Callers that pass fully-loaded VOs (`toggleRule`, bulk
toggle/delete, import) are unaffected: their loaded optional fields are
non-null or were already null in storage, making the explicit assignment
idempotent. Rules that genuinely rely on `duration`-null semantics keep them:
`AlertRuleDuration.parse(null)` already returns ZERO.
--
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]
