Copilot commented on code in PR #6243:
URL: https://github.com/apache/shenyu/pull/6243#discussion_r2576515881
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SelectorServiceImpl.java:
##########
@@ -153,6 +157,21 @@ public void doConditionPreProcessing(final
SelectorQueryCondition condition) {
}
}
+ @Override
+ public PageInfo<SelectorVO> searchByPage(final
PageCondition<SelectorQueryCondition> pageCondition) {
+ doConditionPreProcessing(pageCondition.getCondition());
+ PageHelper.startPage(pageCondition.getPageNum(),
pageCondition.getPageSize());
+ SelectorQueryCondition condition = pageCondition.getCondition();
Review Comment:
The condition is retrieved twice from pageCondition: once on line 162 for
preprocessing and again on line 164. This can be simplified by extracting the
condition once at the beginning of the method, which would improve readability
and avoid redundant method calls.
```suggestion
SelectorQueryCondition condition = pageCondition.getCondition();
doConditionPreProcessing(condition);
PageHelper.startPage(pageCondition.getPageNum(),
pageCondition.getPageSize());
```
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java:
##########
@@ -107,6 +111,20 @@ public void doConditionPreProcessing(final
RuleQueryCondition condition) {
}
}
+ @Override
+ public PageInfo<RuleVO> searchByPage(final
PageCondition<RuleQueryCondition> pageCondition) {
+ doConditionPreProcessing(pageCondition.getCondition());
+ PageHelper.startPage(pageCondition.getPageNum(),
pageCondition.getPageSize());
+ RuleQueryCondition condition = pageCondition.getCondition();
Review Comment:
The condition is retrieved twice from pageCondition: once on line 116 for
preprocessing and again on line 118. This can be simplified by extracting the
condition once at the beginning of the method, which would improve readability
and avoid redundant method calls.
```suggestion
RuleQueryCondition condition = pageCondition.getCondition();
doConditionPreProcessing(condition);
PageHelper.startPage(pageCondition.getPageNum(),
pageCondition.getPageSize());
```
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java:
##########
@@ -107,6 +111,20 @@ public void doConditionPreProcessing(final
RuleQueryCondition condition) {
}
}
+ @Override
+ public PageInfo<RuleVO> searchByPage(final
PageCondition<RuleQueryCondition> pageCondition) {
Review Comment:
The new searchByPage method lacks test coverage. The existing
RuleServiceTest.java file does not include tests for this new pagination
method. Consider adding tests to verify the pagination behavior, including
proper PageHelper usage and VO enrichment.
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SelectorServiceImpl.java:
##########
@@ -153,6 +157,21 @@ public void doConditionPreProcessing(final
SelectorQueryCondition condition) {
}
}
+ @Override
+ public PageInfo<SelectorVO> searchByPage(final
PageCondition<SelectorQueryCondition> pageCondition) {
+ doConditionPreProcessing(pageCondition.getCondition());
+ PageHelper.startPage(pageCondition.getPageNum(),
pageCondition.getPageSize());
+ SelectorQueryCondition condition = pageCondition.getCondition();
+ condition.init();
+ final Page<SelectorDO> page = (Page<SelectorDO>)
selectorMapper.selectByCondition(condition);
+ PageInfo<SelectorVO> pageInfo =
page.toPageInfo(SelectorVO::buildSelectorVO);
+ for (SelectorVO selector : pageInfo.getList()) {
+
selector.setMatchModeName(MatchModeEnum.getMatchModeByCode(selector.getMatchMode()));
+
selector.setTypeName(SelectorTypeEnum.getSelectorTypeByCode(selector.getType()));
+ }
Review Comment:
[List<SelectorDO>](1) is cast to the concrete type [Page<SelectorDO>](2),
losing abstraction.
```suggestion
final List<SelectorDO> doList =
selectorMapper.selectByCondition(condition);
PageInfo<SelectorDO> doPageInfo = new PageInfo<>(doList);
List<SelectorVO> voList = doPageInfo.getList().stream()
.map(SelectorVO::buildSelectorVO)
.collect(Collectors.toList());
for (SelectorVO selector : voList) {
selector.setMatchModeName(MatchModeEnum.getMatchModeByCode(selector.getMatchMode()));
selector.setTypeName(SelectorTypeEnum.getSelectorTypeByCode(selector.getType()));
}
PageInfo<SelectorVO> pageInfo = new PageInfo<>();
pageInfo.setPageNum(doPageInfo.getPageNum());
pageInfo.setPageSize(doPageInfo.getPageSize());
pageInfo.setTotal(doPageInfo.getTotal());
pageInfo.setPages(doPageInfo.getPages());
pageInfo.setList(voList);
```
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SelectorServiceImpl.java:
##########
@@ -153,6 +157,21 @@ public void doConditionPreProcessing(final
SelectorQueryCondition condition) {
}
}
+ @Override
+ public PageInfo<SelectorVO> searchByPage(final
PageCondition<SelectorQueryCondition> pageCondition) {
Review Comment:
The new searchByPage method lacks test coverage. The existing
SelectorServiceTest.java file does not include tests for this new pagination
method. Consider adding tests to verify the pagination behavior, including
proper PageHelper usage and VO enrichment.
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java:
##########
@@ -107,6 +111,20 @@ public void doConditionPreProcessing(final
RuleQueryCondition condition) {
}
}
+ @Override
+ public PageInfo<RuleVO> searchByPage(final
PageCondition<RuleQueryCondition> pageCondition) {
+ doConditionPreProcessing(pageCondition.getCondition());
+ PageHelper.startPage(pageCondition.getPageNum(),
pageCondition.getPageSize());
+ RuleQueryCondition condition = pageCondition.getCondition();
+ condition.init();
+ final Page<RuleDO> ruleDOList = (Page<RuleDO>)
ruleMapper.selectByCondition(condition);
+ PageInfo<RuleVO> rules = ruleDOList.toPageInfo(RuleVO::buildRuleVO);
+ for (RuleVO rule : rules.getList()) {
+
rule.setMatchModeName(MatchModeEnum.getMatchModeByCode(rule.getMatchMode()));
+ }
Review Comment:
[List<RuleDO>](1) is cast to the concrete type [Page<RuleDO>](2), losing
abstraction.
```suggestion
final List<RuleDO> ruleDOList =
ruleMapper.selectByCondition(condition);
List<RuleVO> ruleVOList =
ruleDOList.stream().map(RuleVO::buildRuleVO).collect(Collectors.toList());
for (RuleVO rule : ruleVOList) {
rule.setMatchModeName(MatchModeEnum.getMatchModeByCode(rule.getMatchMode()));
}
PageInfo<RuleVO> rules = new PageInfo<>(ruleVOList);
```
--
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]
