btlqql opened a new pull request, #4682:
URL: https://github.com/apache/rocketmq-dashboard/pull/4682
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Which Issue(s) This PR Fixes
- No open issue; small self-contained defect found while auditing the
audit-log search path (same defect class as #911 / #3307 for the topic and
consumer-group searches).
### Brief Description
`AuditService` forwarded the raw `search` term to the repository. Both paged
paths (`queryLogs` through the private `findPage`, and therefore `exportLogs`)
and `summarize` passed it straight into `AuditRepository`, which binds it into
the `operator` / `resource_name` / `detail` `LIKE` filters. A padded term -
easy to produce by pasting into the audit-log search box - therefore reached
SQL with its whitespace included. `" ops "` can never match `ops`, so the
record list, the summary cards (matching records, unique operators, both
hotspots) and the CSV export that shares the same helper all reported nothing
for a term that has matches.
The term is now normalized once, on the way into the repository, so every
path filters on the same value: trimmed when it has content, and treated as no
filter at all when it is only whitespace. That is the pattern
`AclService.pageUsers` already uses for its keyword
(`StringUtils.hasText(keyword) ? keyword.trim() : null`), applied here where
`search` enters the service.
Same-file note: `ops/audit/AuditService.java` and `AuditServiceTest.java`
are also touched by open PR #4482 (`feat(audit): add operator filter`), which
adds an `operator` parameter to the same methods. The two changes are
semantically independent, but whichever of them merges second needs a trivial
rebase. The repository is not touched here: the LIKE-escaping of this same term
lives in the separate #4668, which changes the bound pattern rather than the
term.
### How Did You Test This Change?
Four `AuditServiceTest` cases written first and run against the unmodified
tree (`red`, base commit `7ce9a682`):
```
$ cd server && mvn -B -ntp test -Dtest=AuditServiceTest
[ERROR] Tests run: 19, Failures: 4, Errors: 0, Skipped: 0
[ERROR] AuditServiceTest.exportLogsShouldTrimSearchTermBeforeDelegating:303
expected: "50% off"
but was: " 50% off "
[ERROR]
AuditServiceTest.queryLogsShouldTreatWhitespaceOnlySearchAsAbsent:290
expected: null
but was: " "
[ERROR] AuditServiceTest.queryLogsShouldTrimSearchTermBeforeDelegating:279
expected: "ops"
but was: " ops "
[ERROR] AuditServiceTest.summarizeShouldTrimSearchTermBeforeDelegating:313
expected: "ops"
but was: " ops "
[INFO] BUILD FAILURE
```
After the fix (`green`), together with the audit controller and repository
tests of the same package:
```
$ cd server && mvn -B -ntp test
-Dtest=AuditServiceTest,AuditControllerTest,MybatisPlusAuditRepositoryTest
[INFO] You have 0 Checkstyle violations.
[INFO] Tests run: 43, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
Test coverage:
- `queryLogsShouldTrimSearchTermBeforeDelegating` - `queryLogs(..., " ops
", ...)` must reach `AuditRepository.findPage` as `"ops"`.
- `queryLogsShouldTreatWhitespaceOnlySearchAsAbsent` - a term made only of
spaces must reach the repository as `null`, i.e. as no filter, so the list is
not emptied by a stray space.
- `summarizeShouldTrimSearchTermBeforeDelegating` - the summary aggregates
must see the same trimmed term as the list, so the cards cannot disagree with
the table.
- `exportLogsShouldTrimSearchTermBeforeDelegating` - the CSV export goes
through the same private helper, so it must export the trimmed result set as
well.
- The tests capture the arguments passed to the mocked `AuditRepository`, so
they assert the value that would be bound into SQL without needing a database.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
- not applicable, no UI text added
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header - no new source files
- [x] Documentation touched where behaviour changed - not applicable, a term
without surrounding whitespace behaves exactly as before
--
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]
