btlqql commented on PR #3213:
URL:
https://github.com/apache/rocketmq-dashboard/pull/3213#issuecomment-5651554167
@lizhimins thank you for the detailed review — all four points are in, and
the branch is rebased.
Rebase: the branch now sits directly on `rocketmq-studio` at `6c24d2ed` (was
behind `0a596661`). `QueryHistoryService` and its test had not moved in
between, so the rebase was clean and the diff is unchanged.
**1. The 404 code is now pinned.**
`hidesResultSnapshotOwnedByAnotherOperator` asserts the code, not just the type
and message:
```java
assertThatThrownBy(() -> service.getMessageQueryResults(9L))
.isInstanceOf(BusinessException.class)
.hasMessage("Query history record not found")
.extracting(e -> ((BusinessException) e).getCode())
.isEqualTo(404);
```
**2. Static import added.** `import static
org.assertj.core.api.Assertions.assertThatThrownBy;` is in the static block,
and both new tests call it unqualified alongside the existing `assertThat`
calls.
**3. Javadoc updated.** `getMessageQueryResults` now reads:
> Retrieves the stored result snapshot for a history record owned by the
authenticated operator.
>
> The lookup is scoped to the current authenticated operator. A record owned
by another operator is deliberately reported as not found (404) to prevent
history id enumeration.
**4. Import ordering fixed.**
`org.apache.rocketmq.studio.auth.AuthenticatedUserContext` now sorts before
`org.apache.rocketmq.studio.common.*`.
**Also done, since you flagged it as free:** the production throw site no
longer spells `org.apache.rocketmq.studio.common.exception.BusinessException`
inline — the type is imported and the throw is `throw new
BusinessException(404, "Query history record not found");`. You were right that
there is nothing else to change: I re-checked and there is no by-id delete or
update over query history.
Verification after the rebase and the tidy-up:
```
mvn -f server/pom.xml -B -DskipITs -Dtest=QueryHistoryServiceTest
-DfailIfNoTests=false test
[INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
Head is `f833e46a`. Could you take another look when you have a moment?
--
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]
