[PR] fix(studio): persist cleared optional instance and registry fields [rocketmq-dashboard]

["btlqql (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(studio): persist cleared optional instance and registry fields [rocketmq-dashboard]

["lizhimins (via GitHub)" <gi...@apache.org>]

lizhimins closed pull request #4369: fix(studio): persist cleared optional instance and registry fields
URL: https://github.com/apache/rocketmq-dashboard/pull/4369


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org