feat: implement interfaces and dtos user task search #19625
Conversation
nathansandi
changed the title
## Description This is the second of three PRs for the implementation of the Search User Task API. The goal of this PR is to set up the mappers and the controller for the User Task Context. The DTOs have been updated according to the requirements identified in the previous PR. The overall implementation is divided into three PRs to facilitate the review process: 1. [PR 1] - #19625 - Prepare the interfaces and implementation, and set up the preparation for the API Mapper and Controller for the User Task Context. 2. **PR 2 (this PR)** - Prepare the Mappers and User Task Controller. 3. [PR 3] - #19691 - Integrate the Variables on the User Task Search Service and Controller. From PR 2, it will be possible to conduct end-to-end tests using builders or the API call. From this PR is already possible to test the User Task Search only over the API For example: ``` { "filter": { "taskState":"CREATED", "processInstanceKey": 2251799813685379, }, "sort":[ { "field":"creationTime", "order":"desc" } ], "page": { "limit": 20 } } ``` ## Related issues closes [#19211](#19211)
github-actions
bot
added
component/operate
.../gateway-rest/src/main/java/io/camunda/zeebe/gateway/rest/controller/UserTaskController.java
Outdated
Show resolved
Hide resolved
| consumes = MediaType.APPLICATION_JSON_VALUE) | ||
| public ResponseEntity<UserTaskSearchQueryResponse> searchUserTasks( | ||
| @RequestBody(required = false) final UserTaskSearchQueryRequest query) { | ||
| return SearchQueryRequestMapper.toUserTaskQuery(query == null? new UserTaskSearchQueryRequest() : query) |
There was a problem hiding this comment.
🔧 You could also allow to pass a null into the toUserTaskQuery method and handle it there gracefully. Just as an alternative.
zeebe/gateway-rest/src/main/java/io/camunda/zeebe/gateway/rest/SearchQueryRequestMapper.java
Outdated
Show resolved
Hide resolved
zeebe/gateway-rest/src/main/java/io/camunda/zeebe/gateway/rest/SearchQueryRequestMapper.java
Outdated
Show resolved
Hide resolved
zeebe/gateway-rest/src/main/java/io/camunda/zeebe/gateway/rest/SearchQueryRequestMapper.java
Outdated
Show resolved
Hide resolved
zeebe/gateway-rest/src/main/java/io/camunda/zeebe/gateway/rest/SearchQueryRequestMapper.java
Outdated
Show resolved
Hide resolved
zeebe/gateway-rest/src/main/java/io/camunda/zeebe/gateway/rest/SearchQueryResponseMapper.java
Outdated
Show resolved
Hide resolved
| @RequestBody(required = false) final UserTaskSearchQueryRequest query) { | ||
| return SearchQueryRequestMapper.toUserTaskQuery(query == null? new UserTaskSearchQueryRequest() : query) | ||
| .fold( | ||
| (Function<? super UserTaskQuery, ? extends ResponseEntity<UserTaskSearchQueryResponse>>) |
There was a problem hiding this comment.
❓ Why is this necessary? Does the compiler fail otherwise? Just wondering as it looks a bit odd. I think the controller method should rather return ResponseEntity<Object> because it can return a UserTaskSearchQueryResponse or a ProblemDetail inside the ResponseEntity.
There was a problem hiding this comment.
The casting of the lambda in the fold method is necessary due to type inference limitations in Java generics, especially when dealing with Either types and lambdas. The fold method is designed to take two functions: one to handle the Left value and another to handle the Right value. Without the explicit cast, the compiler may not be able to infer the correct types for these functions.
However, the casting makes the code harder to read and understand. To avoid this, we can adjust the method signature to return ResponseEntity, which can encapsulate both UserTaskSearchQueryResponse and ProblemDetail.
I have no stronger opinion over the strategy here. :)
.../gateway-rest/src/main/java/io/camunda/zeebe/gateway/rest/controller/UserTaskController.java
Outdated
Show resolved
Hide resolved
| } catch (final Throwable e) { | ||
| final var problemDetail = | ||
| RestErrorMapper.createProblemDetail( | ||
| HttpStatus.BAD_REQUEST, e.getMessage(), "Failed to execute UserTask Search Query"); |
There was a problem hiding this comment.
❓ Is this really going to be a 400 response each time? At this point, can you still differentiate based on the error what went wrong? Will it rather be non-retriable errors, so 500 responses?
There was a problem hiding this comment.
This is true, I believe 400 can be filtered by Validation Exception, and other ones can be handled by 500. I applied the suggestion over here 960e604
| // TODO move this to a shared utility module | ||
|
|
||
| public static <T> List<T> addValuesToList(final List<T> list, final List<T> values) { | ||
| final List<T> result; | ||
| if (list == null) { | ||
| result = Objects.requireNonNull(values); | ||
| } else { | ||
| result = new ArrayList<>(list); | ||
| result.addAll(values); | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| public static <T> List<T> collectValues(final T value, final T... values) { | ||
| final List<T> collectedValues = new ArrayList<>(); | ||
| collectedValues.add(value); | ||
| if (values != null && values.length > 0) { | ||
| collectedValues.addAll(Arrays.asList(values)); | ||
| } | ||
| return collectedValues; | ||
| } |
There was a problem hiding this comment.
🔧 There already is a CollectionUtil in the Java client that you should be able to use instead.
| // Return an Either.Left with a ProblemDetail for unknown sortBy field | ||
| final ProblemDetail problemDetail = | ||
| ProblemDetail.forStatusAndDetail( | ||
| HttpStatus.BAD_REQUEST, "Unknown sortBy field: " + field); | ||
| return Either.left(problemDetail); |
There was a problem hiding this comment.
🔧 How about we collect input validation errors like we do in RequestMapper.toUserTaskUpdateRequest? If we return on the first error directly like you do now, users will only receive this input as invalid. If the order is also invalid, they will only learn that in a follow-up request after correcting the field name.
|
|
||
| public static Either<ProblemDetail, List<UserTaskItem>> toUserTasks( | ||
| final List<UserTaskEntity> tasks) { | ||
| return Either.right( |
There was a problem hiding this comment.
🔧 This never returns a Either.Left so you can simply return the success type and simplify the code.
| tasks.stream().map(SearchQueryResponseMapper::toUserTask).map(Either::get).toList()); | ||
| } | ||
|
|
||
| public static Either<ProblemDetail, UserTaskItem> toUserTask(final UserTaskEntity t) { |
There was a problem hiding this comment.
🔧 This never returns a Either.Left so you can simply return the success type and simplify the code.
| return ResponseEntity.status(HttpStatus.BAD_REQUEST) | ||
| .headers(httpHeaders -> httpHeaders.setContentType(MediaType.APPLICATION_PROBLEM_JSON)) | ||
| .body(problemDetail); |
There was a problem hiding this comment.
🔧 You can use RestErrorMapper.mapProblemToResponse for this.
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) | ||
| .headers(httpHeaders -> httpHeaders.setContentType(MediaType.APPLICATION_PROBLEM_JSON)) | ||
| .body(problemDetail); |
There was a problem hiding this comment.
🔧 You can use RestErrorMapper.mapProblemToResponse for this.
Description
This is the first of 2 PRs of the implementation of Search User Task API. The goal of this PR is prepare the interfaces and implementation, plus the controller for User Task Search
The continuation of the implementation:
The PRs were divided in order to make it easier the review process.
Test example:
Related issues
closes ##19211