ai-yang opened a new issue, #731:
URL: https://github.com/apache/rocketmq-dashboard/issues/731
## Description
`InstanceService.updateInstance` applies a partial update directly to the
mutable
`InstanceVO` returned by `InstanceRepository.findById` before it calls
`InstanceRepository.save`.
`InMemoryInstanceRepository` stores `InstanceVO` objects by reference in its
`ConcurrentHashMap` and returns the same references from `findById`.
Therefore,
if `save` throws, the update request fails but the repository-owned object
has
already been changed. The same ordering also lets concurrent readers observe
fields from an update before the repository save boundary is reached.
## Reproduction
This is reproducible on `rocketmq-studio` commit
`bbf1b7e0cf25a5065ba049b5450cc8155569f710` with a deterministic unit test:
1. Make `findById("inst-1")` return a stored instance with fixed original
fields
and timestamps.
2. Make `save` throw `IllegalStateException("storage unavailable")`.
3. Call `updateInstance` with new name, remark, type, and endpoint values.
4. After the exception, inspect the same repository-owned instance.
The test failed consistently in 5 out of 5 Java 21 runs:
```text
expected: "old-name"
but was: "new-name"
```
## Expected behavior
- A failed repository save leaves the stored instance unchanged.
- Readers do not observe an update before the repository save boundary.
- Successful partial updates continue to preserve omitted fields, identity,
creation time, topic count, and consumer-group count.
## Actual behavior
`updateInstance` changes `name`, `type`, `endpoint`, `remark`, and
`updatedAt` on
the repository-owned object before calling `save`. A save failure propagates
to
the caller, but those changes remain visible in stored state.
## Root cause
`InstanceService.updateInstance` treats the value returned by `findById` as a
detached value, while `InMemoryInstanceRepository` exposes its live mutable
map
entry.
## Proposed fix
Use copy-on-write in `InstanceService`:
1. Create a detached copy of the existing instance, including all instance
fields and `BaseEntity` metadata.
2. Apply the partial update and new `updatedAt` to that copy.
3. Pass only the copy to `instanceRepository.save`.
4. Add regression coverage for repository save failure and for successful
updates preserving omitted fields and metadata.
The expected implementation scope is limited to:
-
`server/src/main/java/org/apache/rocketmq/studio/instance/InstanceService.java`
-
`server/src/test/java/org/apache/rocketmq/studio/instance/InstanceServiceTest.java`
This follows the copy-on-write persistence boundary already accepted for K8s
certificates in #578 / #579. It is distinct from #613 / #615, which validate
blank instance fields before mutation, and from #607 / #631, which only
protect
frontend mock data from returned-reference mutation.
--
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]
