det101 commented on code in PR #18585:
URL:
https://github.com/apache/dolphinscheduler/pull/18585#discussion_r4055566934
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/TaskDefinitionController.java:
##########
@@ -88,7 +92,17 @@ public Result queryTaskDefinitionVersions(@Parameter(hidden
= true) @RequestAttr
@RequestParam(value = "pageNo")
int pageNo,
@RequestParam(value =
"pageSize") int pageSize) {
checkPageParams(pageNo, pageSize);
- return taskDefinitionService.queryTaskDefinitionVersions(loginUser,
projectCode, code, pageNo, pageSize);
+ Result result =
taskDefinitionService.queryTaskDefinitionVersions(loginUser, projectCode, code,
pageNo,
+ pageSize);
+ @SuppressWarnings("unchecked")
Review Comment:
Thanks for the comment.
`@SuppressWarnings("unchecked")` is here because the existing service method
only returns a raw `Result`. `getData()` is `Object`, so masking the page
requires `(PageInfo<TaskDefinitionLog>) result.getData()`. The same pattern is
used in `queryWorkflowDefinitionVersions` and `queryTaskListPaging`.
I see two ways to remove it. Which do you prefer?
1. Change the service interface and implementation to
`Result<PageInfo<...>>`, so the controller can use `result.getData()` without a
cast. This matches the data the method already returns, but it touches the
interface, implementation, and callers/tests.
2. Keep the raw `Result` and check with `instanceof` in the controller
before masking. This avoids a service signature change. On Java 8, `instanceof`
does not narrow the type, so a cast is still required after the check, and the
controller code is more verbose.
I will update all three paging endpoints the same way once you confirm.
--
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]
