From 49d2ee00521a112870b363137c8470116b6a1b75 Mon Sep 17 00:00:00 2001 From: jamesrdi Date: Fri, 28 Jul 2023 13:13:29 +0200 Subject: [PATCH] Closes #2338 - Add query of tasks without owner --- .../common/internal/util/SqlProviderUtil.java | 2 + .../task/query/TaskQueryImplAccTest.java | 19 +++++ .../taskana/task/internal/TaskQueryImpl.java | 11 ++- .../rest/util/QueryParamsValidator.java | 21 +++++ .../task/rest/TaskQueryFilterParameter.java | 28 ++++++- .../task/rest/TaskControllerIntTest.java | 81 +++++++++++++++++-- 6 files changed, 154 insertions(+), 8 deletions(-) diff --git a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/SqlProviderUtil.java b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/SqlProviderUtil.java index 0ed7a1979c..caa9d0f522 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/SqlProviderUtil.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/SqlProviderUtil.java @@ -26,6 +26,8 @@ public static StringBuilder whereIn(String collection, String column, StringBuil .append(""); if (column.matches("t.CUSTOM_\\d+")) { sb.append(" OR " + column + " IS NULL "); + } else if (column.matches("t.OWNER")) { + sb.append(" OR " + column + " IS NULL "); } return sb.append(") "); } diff --git a/lib/taskana-core-test/src/test/java/acceptance/task/query/TaskQueryImplAccTest.java b/lib/taskana-core-test/src/test/java/acceptance/task/query/TaskQueryImplAccTest.java index 706df1847a..d8c93d8b83 100644 --- a/lib/taskana-core-test/src/test/java/acceptance/task/query/TaskQueryImplAccTest.java +++ b/lib/taskana-core-test/src/test/java/acceptance/task/query/TaskQueryImplAccTest.java @@ -1814,6 +1814,7 @@ class Owner { TaskSummary taskSummary1; TaskSummary taskSummary2; TaskSummary taskSummary3; + TaskSummary taskSummary4; @WithAccessId(user = "user-1-1") @BeforeAll @@ -1822,6 +1823,7 @@ void setup() throws Exception { taskSummary1 = taskInWorkbasket(wb).owner("user-2-1").buildAndStoreAsSummary(taskService); taskSummary2 = taskInWorkbasket(wb).owner("user-1-2").buildAndStoreAsSummary(taskService); taskSummary3 = taskInWorkbasket(wb).owner("user-1-3").buildAndStoreAsSummary(taskService); + taskSummary4 = taskInWorkbasket(wb).owner(null).buildAndStoreAsSummary(taskService); } @WithAccessId(user = "user-1-1") @@ -1859,6 +1861,23 @@ void should_ApplyFilter_When_QueryingForOwnerNotLike() { assertThat(list).containsExactly(taskSummary1); } + + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnTaskWithOwnerNull_When_QueryingForOwnerIn() { + String[] nullArray = {null}; + List list = taskService.createTaskQuery().ownerIn(nullArray).list(); + + assertThat(list).containsExactly(taskSummary4); + } + + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnTaskWithOwnerNullAndUser11_When_QueryingForOwnerIn() { + List list = taskService.createTaskQuery().ownerIn("user-1-2", null).list(); + + assertThat(list).containsExactlyInAnyOrder(taskSummary2, taskSummary4); + } } @Nested diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryImpl.java index 4b13b63897..1f27043688 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryImpl.java @@ -140,6 +140,7 @@ public class TaskQueryImpl implements TaskQuery { private String[] parentBusinessProcessIdLike; private String[] parentBusinessProcessIdNotLike; private String[] ownerIn; + private boolean ownerInContainsNull; private String[] ownerNotIn; private String[] ownerLike; private String[] ownerNotLike; @@ -865,7 +866,15 @@ public TaskQuery orderByParentBusinessProcessId(SortDirection sortDirection) { @Override public TaskQuery ownerIn(String... owners) { - this.ownerIn = owners; + List conditionList = new ArrayList<>(Arrays.asList(owners)); + boolean containsNull = conditionList.contains(null); + if (containsNull) { + conditionList.remove(null); + ownerInContainsNull = true; + this.ownerIn = conditionList.toArray(new String[owners.length - 1]); + } else { + this.ownerIn = owners; + } return this; } diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/util/QueryParamsValidator.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/util/QueryParamsValidator.java index 8fb29a0b54..cbf81c4566 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/util/QueryParamsValidator.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/util/QueryParamsValidator.java @@ -4,9 +4,12 @@ import java.util.HashSet; import java.util.Optional; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.servlet.http.HttpServletRequest; +import pro.taskana.common.api.exceptions.InvalidArgumentException; public class QueryParamsValidator { @@ -32,5 +35,23 @@ public static void validateParams(HttpServletRequest request, Class... filter if (!providedParams.isEmpty()) { throw new IllegalArgumentException("Unknown request parameters found: " + providedParams); } + checkExactParam(request, "owner-is-null"); + } + + private static void checkExactParam(HttpServletRequest request, String queryParameter) { + String queryString = request.getQueryString(); + boolean hasOwnerIsNullParam = queryString != null && queryString.contains(queryParameter); + if (hasOwnerIsNullParam) { + Pattern pattern = Pattern.compile("\\b" + queryParameter + "(&|$)"); + // Create a Matcher object to perform the match + Matcher matcher = pattern.matcher(queryString); + + // Check if the "owner-is-null" substring is present in the query string + boolean hasExactOwnerIsNullParam = matcher.find(); + if (!hasExactOwnerIsNullParam) { + throw new InvalidArgumentException( + "It is prohibited to use the param " + queryParameter + " with values."); + } + } } } diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskQueryFilterParameter.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskQueryFilterParameter.java index 843800bff3..99febcd6f3 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskQueryFilterParameter.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskQueryFilterParameter.java @@ -740,6 +740,13 @@ public class TaskQueryFilterParameter implements QueryParameter */ @JsonProperty("owner-not-like") private final String[] ownerNotLike; + + /** + * Filter by tasks that have no owner. The parameter should exactly be "owner-is-null" without + * being followed by "=..." + */ + @JsonProperty("owner-is-null") + private final String ownerNull; // endregion // region primaryObjectReference /** @@ -1259,6 +1266,7 @@ public class TaskQueryFilterParameter implements QueryParameter "owner-not", "owner-like", "owner-not-like", + "owner-is-null", "por", "por-company", "por-company-not", @@ -1415,6 +1423,7 @@ public TaskQueryFilterParameter( String[] ownerNotIn, String[] ownerLike, String[] ownerNotLike, + String ownerNull, ObjectReference[] primaryObjectReferenceIn, String[] porCompanyIn, String[] porCompanyNotIn, @@ -1570,6 +1579,7 @@ public TaskQueryFilterParameter( this.ownerNotIn = ownerNotIn; this.ownerLike = ownerLike; this.ownerNotLike = ownerNotLike; + this.ownerNull = ownerNull; this.primaryObjectReferenceIn = primaryObjectReferenceIn; this.porCompanyIn = porCompanyIn; this.porCompanyNotIn = porCompanyNotIn; @@ -1845,7 +1855,8 @@ public Void apply(TaskQuery query) { .map(this::wrapElementsInLikeStatement) .ifPresent(query::parentBusinessProcessIdNotLike); - Optional.ofNullable(ownerIn).ifPresent(query::ownerIn); + String[] ownerInAndNull = addNullToOwnerIn(); + Optional.ofNullable(ownerInAndNull).ifPresent(query::ownerIn); Optional.ofNullable(ownerNotIn).ifPresent(query::ownerNotIn); Optional.ofNullable(ownerLike) .map(this::wrapElementsInLikeStatement) @@ -2184,4 +2195,19 @@ private void validateFilterParameters() throws InvalidArgumentException { "provided value of the property 'without-attachment' must be 'true'"); } } + + private String[] addNullToOwnerIn() { + if (this.ownerNull == null) { + return this.ownerIn; + } + if (this.ownerIn == null) { + return new String[] {null}; + } + String[] newArray; + int nullPosition; + newArray = Arrays.copyOf(this.ownerIn, this.ownerIn.length + 1); + nullPosition = this.ownerIn.length; + newArray[nullPosition] = null; + return newArray; + } } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java index 5147319217..d0adaf8c3e 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java @@ -34,9 +34,11 @@ import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.HttpStatusCodeException; import pro.taskana.TaskanaConfiguration; import pro.taskana.classification.rest.models.ClassificationSummaryRepresentationModel; +import pro.taskana.common.internal.util.Pair; import pro.taskana.common.rest.RestEndpoints; import pro.taskana.rest.test.RestHelper; import pro.taskana.rest.test.TaskanaSpringBootTest; @@ -55,21 +57,16 @@ /** Test Task Controller. */ @TaskanaSpringBootTest class TaskControllerIntTest { - - @Autowired TaskanaConfiguration taskanaConfiguration; private static final ParameterizedTypeReference TASK_SUMMARY_PAGE_MODEL_TYPE = new ParameterizedTypeReference<>() {}; - private static final ParameterizedTypeReference TASK_SUMMARY_COLLECTION_MODEL_TYPE = new ParameterizedTypeReference<>() {}; - private static final ParameterizedTypeReference TASK_MODEL_TYPE = ParameterizedTypeReference.forType(TaskRepresentationModel.class); - private final RestHelper restHelper; private final DataSource dataSource; - private final String schemaName; + @Autowired TaskanaConfiguration taskanaConfiguration; @Autowired TaskControllerIntTest( @@ -1165,6 +1162,78 @@ void should_KeepFiltersInTheLinkOfTheResponse_When_GettingTasks() { + "&sort-by=POR_VALUE&order=DESCENDING"); } + @Test + void should_ReturnTasksWithOwnerUser11_When_GettingTasks() { + String url = restHelper.toUrl(RestEndpoints.URL_TASKS) + "?owner=user-1-1"; + HttpEntity auth = new HttpEntity<>(RestHelper.generateHeadersForUser("admin")); + + ResponseEntity response = + TEMPLATE.exchange(url, HttpMethod.GET, auth, TASK_SUMMARY_PAGE_MODEL_TYPE); + + assertThat(response.getBody()).isNotNull(); + assertThat((response.getBody()).getLink(IanaLinkRelations.SELF)).isNotNull(); + assertThat((response.getBody()).getContent()).hasSize(10); + } + + @Test + void should_ReturnTasksWithOwnerNull_When_GettingTasks() { + String url = restHelper.toUrl(RestEndpoints.URL_TASKS) + "?owner-is-null"; + HttpEntity auth = new HttpEntity<>(RestHelper.generateHeadersForUser("admin")); + + ResponseEntity response = + TEMPLATE.exchange(url, HttpMethod.GET, auth, TASK_SUMMARY_PAGE_MODEL_TYPE); + + assertThat(response.getBody()).isNotNull(); + assertThat((response.getBody()).getLink(IanaLinkRelations.SELF)).isNotNull(); + assertThat((response.getBody()).getContent()).hasSize(65); + } + + @Test + void should_ReturnTasksWithOwnerNullOrUser11_When_GettingTasks() { + String url = restHelper.toUrl(RestEndpoints.URL_TASKS) + "?owner-is-null&owner=user-1-1"; + HttpEntity auth = new HttpEntity<>(RestHelper.generateHeadersForUser("admin")); + + ResponseEntity response = + TEMPLATE.exchange(url, HttpMethod.GET, auth, TASK_SUMMARY_PAGE_MODEL_TYPE); + + assertThat(response.getBody()).isNotNull(); + assertThat((response.getBody()).getLink(IanaLinkRelations.SELF)).isNotNull(); + assertThat((response.getBody()).getContent()).hasSize(75); + } + + @Test + void should_ReturnTasksWithReadyStateAndOwnerNullOrUser11_When_GettingTasks() { + String url = + restHelper.toUrl(RestEndpoints.URL_TASKS) + "?state=READY&owner-is-null&owner=user-1-1"; + HttpEntity auth = new HttpEntity<>(RestHelper.generateHeadersForUser("admin")); + + ResponseEntity response = + TEMPLATE.exchange(url, HttpMethod.GET, auth, TASK_SUMMARY_PAGE_MODEL_TYPE); + + assertThat(response.getBody()).isNotNull(); + assertThat((response.getBody()).getLink(IanaLinkRelations.SELF)).isNotNull(); + assertThat((response.getBody()).getContent()).hasSize(56); + } + + @TestFactory + Stream should_ThrowException_When_OwnerIsNullParamNotStrict() { + List> list = + List.of( + Pair.of("When owner-is-null=", "?owner-is-null="), + Pair.of("When owner-is-null=anyValue", "?owner-is-null=anyValue1,anyValue2")); + ThrowingConsumer> testOwnerIsNull = + t -> { + String url = restHelper.toUrl(RestEndpoints.URL_TASKS) + t.getRight(); + HttpEntity auth = new HttpEntity<>(RestHelper.generateHeadersForUser("admin")); + + assertThatThrownBy( + () -> + TEMPLATE.exchange(url, HttpMethod.GET, auth, TASK_SUMMARY_PAGE_MODEL_TYPE)) + .isInstanceOf(HttpClientErrorException.class); + }; + return DynamicTest.stream(list.iterator(), Pair::getLeft, testOwnerIsNull); + } + @Test void should_GetAllTasks_For_GettingLastTaskSummaryPageSortedByPorValue() { String url =