btlqql opened a new pull request, #5323:
URL: https://github.com/apache/rocketmq-dashboard/pull/5323
### Which Issue(s) This PR Fixes
- Fixes #5055
### Brief Description
`CloudCredentialService.update`
(server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialService.java:132-145)
loaded the whole credential with `findById`, applied the supplied fields to
that snapshot and persisted the snapshot with
`credentialRepository.replace(existing)`. The repository writes the whole
non-null row, so two overlapping requests wrote each other's stale values: a
name or remark edit that had read the row before a secret rotation wrote the
old secret back after the rotation was persisted, and a rotation wrote back the
name/remark it had read earlier. Both requests answered success, and the lost
rotation only surfaced later as an invalid-credential failure — a later
client-cache invalidation cannot recover a value that is no longer in storage.
The update now builds a field-scoped patch that carries only the fields the
request supplied (plus the id and `gmt_modified`) and persists that. Fields the
request did not supply stay `null` and are skipped by the update, so a
concurrent edit of another field survives; an update against a credential that
disappeared still reports not found, because the write is still reported as not
applied.
The existing `updateShouldNotRecreateConcurrentlyDeletedCredentialTest` stub
is adjusted from the identity of the loaded snapshot to
`any(CloudCredentialVO.class)`, since the repository is intentionally no longer
handed that snapshot.
### How Did You Test This Change?
Two new regressions in `CloudCredentialServiceTest`:
- `nameOnlyUpdateShouldNotRestoreAConcurrentlyRotatedSecretTest` — the edit
reads a snapshot whose secret is `rotated-away-secret` (a rotation already
stored a new one) and sends only a name; the captured write must carry the new
name and no secret.
- `secretOnlyRotationShouldNotRewriteMetadataFromTheStaleSnapshotTest` — the
mirror image: a secret-only rotation must not write back the
`old-name`/`old-remark` of its snapshot.
Red run on the unmodified tree:
```
$ cd server && mvn -B -ntp test -Dtest=CloudCredentialServiceTest
[ERROR] Tests run: 19, Failures: 2, Errors: 0, Skipped: 0, Time elapsed:
2.417 s <<< FAILURE! -- in
org.apache.rocketmq.studio.provider.credential.CloudCredentialServiceTest
[ERROR]
CloudCredentialServiceTest.nameOnlyUpdateShouldNotRestoreAConcurrentlyRotatedSecretTest:246
expected: null
but was: "rotated-away-secret"
[ERROR]
CloudCredentialServiceTest.secretOnlyRotationShouldNotRewriteMetadataFromTheStaleSnapshotTest:271
expected: null
but was: "old-name"
[ERROR] Tests run: 19, Failures: 2, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE
```
Green run after the fix, extended to the whole credential package slice (the
two other classes cover the repository mapping and the controller and were
unaffected):
```
$ cd server && mvn -B -ntp test
-Dtest=CloudCredentialServiceTest,MybatisPlusCloudCredentialRepositoryTest,CloudCredentialControllerTest
[INFO] You have 0 Checkstyle violations.
[INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed:
0.676 s -- in
org.apache.rocketmq.studio.provider.credential.CloudCredentialServiceTest
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.277
s -- in
org.apache.rocketmq.studio.provider.credential.MybatisPlusCloudCredentialRepositoryTest
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 8.742
s -- in
org.apache.rocketmq.studio.provider.credential.CloudCredentialControllerTest
[INFO] Tests run: 27, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits
- [x] Tests added or updated for non-trivial changes
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
— not applicable, no UI text changed
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks) — the change stays inside the existing service; no new type, endpoint
or dependency, and the ArchUnit rules run in the same reactor
- [x] New source files carry the ASF license header — no new source file,
the change is inside an existing method
- [x] Documentation touched where behaviour changed (README / `docs/` /
in-app help) — not applicable: the update API request/response contract is
unchanged; only which columns are written by an update changed
--
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]
