[Improvement-18224][API] Migrate TaskGroupService and ExecutorServiceforceStartTaskInstance to typed returns#18233
Open
ruanwenjun wants to merge 1 commit intoapache:devfrom
Conversation
….forceStartTaskInstance to typed returns Migrate all 11 TaskGroupService methods from Map<String,Object> returns to typed returns or void with ServiceException, plus the coupled ExecutorService.forceStartTaskInstance which is only invoked from TaskGroupServiceImpl.forceStartTask. TaskGroupService: - createTaskGroup -> TaskGroup - updateTaskGroup -> TaskGroup - queryAllTaskGroup / queryTaskGroupByStatus / queryTaskGroupByProjectCode -> PageInfo<TaskGroup> - queryTaskGroupById -> TaskGroup (nullable) - doQuery -> PageInfo<TaskGroup> - closeTaskGroup / startTaskGroup / forceStartTask / modifyPriority -> void ExecutorService: - forceStartTaskInstance -> void The private hasProjectPerm helper is replaced with requireProjectPerm which throws ServiceException. Admin users still bypass the project existence check (preserving the prior admin-shortcut semantics) and the non-admin-on-missing-project NPE is replaced with a clean PROJECT_NOT_FOUND error. ApiExceptionHandler now propagates the controller method's @ResponseStatus to the error response so error and success paths share the same HTTP status code (e.g. POST endpoints stay 201 Created on ServiceException). This is the byte-for-byte fix for putMsg/throw divergence the migration would otherwise introduce — confirmed against TaskGroupControllerTest's real SpringBootTest harness. Two pre-existing tests that asserted the legacy "errors always 200" behaviour are updated to the unified status: - AccessTokenControllerTest.testExceptionHandler - UsersControllerTest.testCreateUser ApiExceptionHandlerTest is updated to reflect the new ResponseEntity<Result> return type. No PythonGateway or cross-service callers exist for these methods, so the migration is contained within the api module.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| return result; | ||
| log.info("Create task group complete, taskGroupName:{}.", taskGroup.getName()); |
|
|
||
| @ExceptionHandler(Throwable.class) | ||
| public Result<Object> exceptionHandler(Throwable e, HandlerMethod hm) { | ||
| public ResponseEntity<Result<Object>> exceptionHandler(Throwable e, HandlerMethod hm) { |
| * @return | ||
| */ | ||
| Map<String, Object> forceStartTaskInstance(User loginUser, int queueId); | ||
| void forceStartTaskInstance(User loginUser, int queueId); |
| * query task group by id (returns null if not found) | ||
| */ | ||
| Map<String, Object> queryTaskGroupById(User loginUser, int id); | ||
| TaskGroup queryTaskGroupById(User loginUser, int id); |
| * paginated query of task groups | ||
| */ | ||
| Map<String, Object> doQuery(User loginUser, int pageNo, int pageSize, int userId, String name, Integer status); | ||
| PageInfo<TaskGroup> doQuery(User loginUser, int pageNo, int pageSize, int userId, String name, Integer status); |
| * paginated query of task groups | ||
| */ | ||
| Map<String, Object> doQuery(User loginUser, int pageNo, int pageSize, int userId, String name, Integer status); | ||
| PageInfo<TaskGroup> doQuery(User loginUser, int pageNo, int pageSize, int userId, String name, Integer status); |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Was this PR generated or assisted by AI?
YES, ops 4.7
Purpose of the pull request
Migrate all 11 TaskGroupService methods from Map<String,Object> returns to typed returns or void with ServiceException, plus the coupled ExecutorService.forceStartTaskInstance which is only invoked from TaskGroupServiceImpl.forceStartTask.
TaskGroupService:
ExecutorService:
The private hasProjectPerm helper is replaced with requireProjectPerm which throws ServiceException. Admin users still bypass the project existence check (preserving the prior admin-shortcut semantics) and the non-admin-on-missing-project NPE is replaced with a clean PROJECT_NOT_FOUND error.
ApiExceptionHandler now propagates the controller method's @ResponseStatus to the error response so error and success paths share the same HTTP status code (e.g. POST endpoints stay 201 Created on ServiceException). This is the byte-for-byte fix for putMsg/throw divergence the migration would otherwise introduce — confirmed against TaskGroupControllerTest's real SpringBootTest harness. Two pre-existing tests that asserted the legacy "errors always 200" behaviour are updated to the unified status:
ApiExceptionHandlerTest is updated to reflect the new ResponseEntity return type.
No PythonGateway or cross-service callers exist for these methods, so the migration is contained within the api module.
Brief change log
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
Pull Request Notice
Pull Request Notice
If your pull request contains incompatible change, you should also add it to
docs/docs/en/guide/upgrade/incompatible.md