From 2a511d56cbf8fad78ba597d213196339ed41202b 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 | 25 +++++++++++ .../taskana/task/internal/TaskQueryImpl.java | 11 ++++- .../rest/util/QueryParamsValidator.java | 20 +++++++++ .../pro/taskana/task/rest/TaskController.java | 1 + .../task/rest/TaskQueryFilterParameter.java | 32 +++++++++++++- .../task/rest/TaskControllerIntTest.java | 44 ++++++++++++++++++- 7 files changed, 131 insertions(+), 4 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..0e933fbebb 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 @@ -24,7 +24,7 @@ public static StringBuilder whereIn(String collection, String column, StringBuil .append("") .append("0=1") .append(""); - if (column.matches("t.CUSTOM_\\d+")) { + if (column.matches("t.CUSTOM_\\d+") || 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..9d7608b612 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,29 @@ 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().workbasketIdIn(wb.getId()).ownerIn(nullArray).list(); + + assertThat(list).containsExactly(taskSummary4); + } + + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnTaskWithOwnerNullAndUser11_When_QueryingForOwnerIn() { + List list = + taskService + .createTaskQuery() + .workbasketIdIn(wb.getId()) + .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..2710517860 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 { @@ -33,4 +36,21 @@ public static void validateParams(HttpServletRequest request, Class... filter throw new IllegalArgumentException("Unknown request parameters found: " + providedParams); } } + + public static void checkExactParam(HttpServletRequest request, String queryParameter) { + String queryString = request.getQueryString(); + boolean containParam = queryString != null && queryString.contains(queryParameter); + if (containParam) { + 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 hasExactParam = matcher.find(); + if (!hasExactParam) { + 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/TaskController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskController.java index 33908e5eb1..d097fbd0be 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskController.java @@ -154,6 +154,7 @@ public ResponseEntity getTasks( filterCustomIntFields.apply(query); groupByParameter.apply(query); sortParameter.apply(query); + QueryParamsValidator.checkExactParam(request, "owner-is-null"); List taskSummaries = pagingParameter.apply(query); 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..99e1658730 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) @@ -2183,5 +2194,24 @@ private void validateFilterParameters() throws InvalidArgumentException { throw new InvalidArgumentException( "provided value of the property 'without-attachment' must be 'true'"); } + if (ownerNull != null && (ownerNull.length() > 0)) { + throw new InvalidArgumentException( + "It is prohibited to use the param owner-is-null with values."); + } + } + + 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..a04c55ba1b 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 @@ -26,6 +26,8 @@ import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.TestInstance.Lifecycle; import org.junit.jupiter.api.function.ThrowingConsumer; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.core.ParameterizedTypeReference; @@ -34,9 +36,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; @@ -68,7 +72,7 @@ class TaskControllerIntTest { private final RestHelper restHelper; private final DataSource dataSource; - + private final String schemaName; @Autowired @@ -1165,6 +1169,44 @@ void should_KeepFiltersInTheLinkOfTheResponse_When_GettingTasks() { + "&sort-by=POR_VALUE&order=DESCENDING"); } + @ParameterizedTest + @CsvSource({ + "owner=user-1-1, 10", + "owner-is-null, 65", + "owner-is-null&owner=user-1-1, 75", + "state=READY&owner-is-null&owner=user-1-1, 56", // Add more test cases as needed + }) + void should_ReturnTasksWithVariousOwnerParameters_When_GettingTasks( + String queryParams, int expectedSize) { + String url = restHelper.toUrl(RestEndpoints.URL_TASKS) + "?" + queryParams; + 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(expectedSize); + } + + @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 =