btlqql opened a new pull request, #4369:
URL: https://github.com/apache/rocketmq-dashboard/pull/4369
## Problem
Two Studio update paths silently keep a value the user just cleared, because
MyBatis-Plus
`updateById` omits null entity fields:
- **Instance dialog:** clearing the admin credential reference looks saved,
but the previous
`admin_credential_ref` stays in the row, so the instance keeps using a
credential the operator
removed.
- **NameServer registry entry:** submitting an entry without a registry
description, k8s namespace
or k8s id leaves the previous value of that column, so an optional column
can never be cleared.
## Root cause
Both update paths replace the whole editable record through `updateById`,
and MyBatis-Plus emits a
`SET` clause only for non-null entity fields. A field the request omitted is
therefore not part of
the statement at all and the stored column keeps its old value. The request
payload cannot express
"clear this field" separately from "leave it alone" — the update is a full
replacement, so an absent
value has to mean *clear*.
## Change
Both update paths now assign the cleared columns explicitly with an
`UpdateWrapper`, and only for
the fields the request left absent, so a request that carries a value keeps
the existing
single-statement path:
- `MybatisPlusInstanceRepository`: `admin_credential_ref` is set to `null`
when the submitted
instance carries no credential reference.
- `NameserverRegistryService`: `description`, `k8s_namespace` and `k8s_id`
are set to `null` when
the submitted entry omits them.
## Verification on Linux
Ubuntu 22.04, OpenJDK 21.0.12, Maven 3.6.3, base `rocketmq-studio` =
`e5ac5244`.
**Red/green on the two touched test classes.** The PR's tests with the
*pristine* production files
(so the fix is absent):
```
mvn -B -ntp -o test
-Dtest='MybatisPlusInstanceRepositoryTest,NameserverRegistryServiceTest'
```
```
[ERROR]
NameserverRegistryServiceTest.updateShouldClearOptionalColumnsThatTheRequestOmitsTest
<<< FAILURE!
[ERROR]
MybatisPlusInstanceRepositoryTest.saveShouldClearTheAdminCredentialRefWhenTheUpdateRemovesIt
<<< FAILURE!
[ERROR] Tests run: 40, Failures: 2, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE
```
With this change applied:
```
[INFO] Tests run: 22, Failures: 0, Errors: 0, Skipped: 0 --
NameserverRegistryServiceTest
[INFO] Tests run: 18, Failures: 0, Errors: 0, Skipped: 0 --
MybatisPlusInstanceRepositoryTest
[INFO] Tests run: 40, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
**Whole-suite comparison** (`cd server && mvn -B -ntp -o test`), pristine
base vs this branch:
| | tests | failures | errors |
|---|---|---|---|
| pristine `rocketmq-studio` `e5ac5244` | 2414 | 0 | 17 |
| this branch | 2418 | 0 | 17 |
The 17 errors are identical in both runs and come from nine pre-existing
Spring integration test
classes (`StudioApplicationTest`, `HealthProbeIntegrationTest`, the four
`AuthService*IntegrationTest`
classes, `QueryHistoryServiceIntegrationTest`,
`NativeAlertEvaluationTransactionTest`,
`NotificationOutboxMapperIntegrationTest`,
`RmqAlertStateMapperIntegrationTest`) — they need a live
database, so they fail on a bare machine with or without this patch. This
change adds 4 tests and
introduces no new failure.
## Why this went unnoticed
`updateById`'s null-field omission is invisible on the happy path: creating
an instance or a
registry entry with every field set, then editing it with every field set,
never exercises a partial
update. The defect only appears when a field is *removed*, which is exactly
the path no existing test
covered.
## Consolidation note
This PR absorbs two previously closed one-defect PRs, per the review request
to regroup Studio fixes
into one PR per functional domain:
- #4276 (persist a cleared admin credential reference)
- #4277 (persist a cleared registry description and K8s columns)
Both are back-end instance/cluster persistence of cleared optional columns,
so they are grouped here;
the patches are otherwise unchanged apart from being rebased onto the
current `rocketmq-studio`
baseline and squashed into one commit.
Further domains from the same queue are being regrouped in the same way
(alert subsystem, audit
search, message property display, provider metrics semantics, AI tool input
validation) rather than
being resubmitted individually.
--
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]
