[type:fix] page the mock request record list query in the database - #7215
Conversation
Aias00
left a comment
There was a problem hiding this comment.
Summary
MockRequestRecordServiceImpl#listByPage was missing @Pageable, so every list request read the whole mock_request_record table — including every stored TEXT request/response body — into memory. This PR adds the annotation so the page bounds reach the mapper query.
Review
Verified the annotation is effective, not decorative:
PageableAspectmatches on@annotation(...Pageable), so service impl methods are intercepted without extra wiring.MockRequestRecordQuerydeclaresprivate PageParameter pageParameter, soReflectUtils.getFieldValue(query, "pageParameter")resolves and the cast toPageParametersucceeds.- If a caller passes no page parameter the aspect returns
point.proceed()unchanged, so existing callers keep working exactly as before.
This is the same class of fix as #7216 (InstanceInfoServiceImpl) and is consistent with it.
CI
All green (pr_build, build, CodeQL, all integrated test suites).
Notes (non-blocking)
As with #7216, the added test only asserts the annotation exists via reflection. It cannot catch a regression where paging silently stops being applied. A behavioural test — stub the mapper, run through the aspect, assert the query is bounded — would protect the actual property you care about here (not loading every record body). Suggested, not required.
LGTM.
What
MockRequestRecordServiceImpl#listByPageis the only paginated list service in the admin serverwhose
listByPageis not annotated with@Pageable, so it never gets paged by the database:MockRequestRecordMapper.selectByQueryis a plainSELECT ... FROM mock_request_record WHERE ...with no limit.
Base_Column_Listincludes thebodyTEXT column.Every list request therefore reads the whole filtered table — every request body included — into
memory and ships it to the client.
@Pageableis what armsPageableAspect(PageMethod.startPage(...)+ MyBatis-PageHelper), whichappends the dialect's
LIMITto the statement and fills the real total count back into theCommonPager. With the annotation in place the query pages in the database, so a list requesttransfers at most
pageSizerows instead of the whole table — with no change to the response shape.Why
Closes #6808.
Verified
./mvnw -pl shenyu-admin -am test -Dtest='MockRequestRecordServiceTest,MockRequestRecordMapperTest,MockRequestRecordControllerTest,PageableAspectTest'→ Tests run: 23, Failures: 0, Errors: 0
MockRequestRecordServiceTest#testListByPageIsPageable(new) keeps the annotation in place:removing
@Pageablemakes it fail with(the guard only locks the annotation in; it cannot exercise the AOP/PageHelper path itself, which
is covered by
PageableAspectTest)checkstyle: 0 violations
Not in this PR
api_idindex suggested in the issue requires a schema change across all five supporteddatabases ([BUG] —
meta_data.pathExistedExclude: cross-namespace path scan +NOT IN+ unindexedpath#6809 family) — separate work.bodyis intentionally left in the list column list:MockRequestRecordVOexposes it and thedashboard lives in a separate repository, so dropping it could silently blank a field the UI
renders. Paging already bounds the transferred payload by
pageSize.