HY-love-sleep opened a new pull request, #7216:
URL: https://github.com/apache/shenyu/pull/7216
## What
`InstanceInfoServiceImpl#listByPage` is the only `listByPage` in the admin
server that is **not**
annotated with `@Pageable` — even though `InstanceController` already
requires `currentPage` and
`pageSize` (`@NotNull`) on the endpoint:
```
GET /instance?namespaceId=…¤tPage=1&pageSize=12
```
Both parameters are accepted and then ignored:
`InstanceInfoMapper.selectByQuery` is a plain
`SELECT … FROM instance_info WHERE namespace_id = ? …` with no limit, and
its `Base_Column_List`
includes the `instance_info` TEXT column holding the whole instance metadata
JSON. So a single list
request loads every row of the namespace — JSON blobs included — into memory
and ships it to the
client.
`@Pageable` is what arms `PageableAspect` (`PageMethod.startPage(...)` +
MyBatis-PageHelper), which
appends the dialect's `LIMIT` to the statement and fills the real total
count back into the
`CommonPager`. With the annotation in place the list pages in the database —
at most `pageSize`
rows per request — with no change to the response shape.
## Why
Closes #6807.
## Verified
* `./mvnw -pl shenyu-admin -am test
-Dtest='InstanceInfoServiceTest,InstanceInfoMapperTest,PageableAspectTest'`
→ **Tests run: 18, Failures: 0, Errors: 0**
* `InstanceInfoServiceTest#testListByPageIsPageable` (new) keeps the
annotation in place: removing
`@Pageable` makes it fail with
```
org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
at
...InstanceInfoServiceTest.testListByPageIsPageable(InstanceInfoServiceTest.java:140)
```
(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
* Excluding `instance_info` from the list query: `InstanceInfoVO` exposes
the field and the
dashboard lives in a separate repository, so dropping it would silently
change the API shape.
Paging already bounds the transferred payload by `pageSize`.
* The `(namespace_id, instance_type)` index suggested in the issue requires
a schema change across
all five supported databases (#6809 family) — separate work.
* The `instance_ip like CONCAT('%', #{instanceIp}, '%')` leading wildcard is
left as is: switching
to a prefix match would change the search semantics and needs a
maintainer's call.
--
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]
