From c36933b45084586097f38c2e2059beefeeb2795b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Heffner?= <56156750+gitgoodjhe@users.noreply.github.com> Date: Mon, 20 Apr 2020 15:31:04 +0200 Subject: [PATCH] Comments from Holger Hagen --- .../java/pro/taskana/task/api/TaskQuery.java | 4 +- ...chFields.java => WildcardSearchField.java} | 10 ++-- .../taskana/task/internal/TaskQueryImpl.java | 27 ++++++--- .../task/internal/TaskQueryMapper.java | 4 +- .../acceptance/task/QueryTasksAccTest.java | 2 - .../QueryTasksByWildcardSearchAccTest.java | 57 +++++++++++++------ .../java/pro/taskana/rest/TaskController.java | 18 +++--- 7 files changed, 78 insertions(+), 44 deletions(-) rename lib/taskana-core/src/main/java/pro/taskana/task/api/{WildcardSearchFields.java => WildcardSearchField.java} (78%) diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskQuery.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskQuery.java index a11b211f7..b929690af 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskQuery.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskQuery.java @@ -614,6 +614,7 @@ public interface TaskQuery extends BaseQuery { /** * Add your wildcard search value for pattern matching to your query. It will be compared in SQL * with the LIKE operator. You may use a wildcard like % to specify the pattern. + * Must be used in combination with the wilcardSearchFieldIn parameter * * @param wildcardSearchValue the wildcard search value * @return the query @@ -622,11 +623,12 @@ public interface TaskQuery extends BaseQuery { /** * Add the values of the wildcard search fields for exact matching to your query. + * Must be used in combination with the wildcardSearchValueLike parameter * * @param wildcardSearchFields the values of your wildcard search fields * @return the query */ - TaskQuery wildcardSearchFieldsIn(WildcardSearchFields... wildcardSearchFields); + TaskQuery wildcardSearchFieldsIn(WildcardSearchField... wildcardSearchFields); /** * This method sorts the query result according to the completed timestamp. diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/WildcardSearchFields.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/WildcardSearchField.java similarity index 78% rename from lib/taskana-core/src/main/java/pro/taskana/task/api/WildcardSearchFields.java rename to lib/taskana-core/src/main/java/pro/taskana/task/api/WildcardSearchField.java index 6a9ef124d..114baa570 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/WildcardSearchFields.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/WildcardSearchField.java @@ -5,7 +5,7 @@ import java.util.Map; import java.util.TreeMap; import java.util.stream.Collectors; -public enum WildcardSearchFields { +public enum WildcardSearchField { NAME("NAME"), DESCRIPTION("DESCRIPTION"), CUSTOM_1("CUSTOM_1"), @@ -25,21 +25,21 @@ public enum WildcardSearchFields { CUSTOM_15("CUSTOM_15"), CUSTOM_16("CUSTOM_16"); - WildcardSearchFields(String name) { + WildcardSearchField(String name) { this.name = name; } - private static final Map STRING_TO_ENUM = + private static final Map STRING_TO_ENUM = Arrays.stream(values()) .collect( Collectors.toMap( - WildcardSearchFields::toString, + WildcardSearchField::toString, e -> e, (first, second) -> first, () -> new TreeMap<>(String.CASE_INSENSITIVE_ORDER))); private String name; - public static WildcardSearchFields fromString(String name) { + public static WildcardSearchField fromString(String name) { return STRING_TO_ENUM.get(name); } 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 17f9afd1d..13180e976 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 @@ -24,7 +24,7 @@ import pro.taskana.task.api.ObjectReferenceQuery; import pro.taskana.task.api.TaskQuery; import pro.taskana.task.api.TaskQueryColumnName; import pro.taskana.task.api.TaskState; -import pro.taskana.task.api.WildcardSearchFields; +import pro.taskana.task.api.WildcardSearchField; import pro.taskana.task.api.models.TaskSummary; import pro.taskana.task.internal.models.TaskSummaryImpl; import pro.taskana.workbasket.api.WorkbasketPermission; @@ -148,7 +148,7 @@ public class TaskQueryImpl implements TaskQuery { private TimeInterval[] dueIn; private List orderBy; private List orderColumns; - private WildcardSearchFields[] wildcardSearchFieldsIn; + private WildcardSearchField[] wildcardSearchFieldIn; private String wildcardSearchValueLike; private boolean useDistinctKeyword = false; @@ -747,8 +747,8 @@ public class TaskQueryImpl implements TaskQuery { } @Override - public TaskQuery wildcardSearchFieldsIn(WildcardSearchFields... wildcardSearchFields) { - this.wildcardSearchFieldsIn = wildcardSearchFields; + public TaskQuery wildcardSearchFieldsIn(WildcardSearchField... wildcardSearchFields) { + this.wildcardSearchFieldIn = wildcardSearchFields; return this; } @@ -970,6 +970,7 @@ public class TaskQueryImpl implements TaskQuery { try { LOGGER.debug("entry to list(), this = {}", this); taskanaEngine.openConnection(); + checkForIllegalParamCombinations(); checkOpenAndReadPermissionForSpecifiedWorkbaskets(); setupJoinAndOrderParameters(); setupAccessIds(); @@ -1000,6 +1001,7 @@ public class TaskQueryImpl implements TaskQuery { List result = new ArrayList<>(); try { taskanaEngine.openConnection(); + checkForIllegalParamCombinations(); checkOpenAndReadPermissionForSpecifiedWorkbaskets(); setupAccessIds(); setupJoinAndOrderParameters(); @@ -1037,6 +1039,7 @@ public class TaskQueryImpl implements TaskQuery { this.columnName = columnName; this.orderBy.clear(); this.addOrderCriteria(columnName.toString(), sortDirection); + checkForIllegalParamCombinations(); checkOpenAndReadPermissionForSpecifiedWorkbaskets(); setupAccessIds(); @@ -1603,14 +1606,24 @@ public class TaskQueryImpl implements TaskQuery { addAttachmentClassificationNameToSelectClauseForOrdering; } - public WildcardSearchFields[] getWildcardSearchFieldIn() { - return wildcardSearchFieldsIn; + public WildcardSearchField[] getWildcardSearchFieldIn() { + return wildcardSearchFieldIn; } public String getWildcardSearchValueLike() { return wildcardSearchValueLike; } + private void checkForIllegalParamCombinations() { + + if ((wildcardSearchValueLike != null && wildcardSearchFieldIn == null) + || (wildcardSearchValueLike == null && wildcardSearchFieldIn != null)) { + throw new IllegalArgumentException( + "The params \"wildcardSearchFieldIn\" and \"wildcardSearchValueLike\"" + + " must be used together!"); + } + } + private String getDatabaseId() { return this.taskanaEngine.getSqlSession().getConfiguration().getDatabaseId(); } @@ -1922,7 +1935,7 @@ public class TaskQueryImpl implements TaskQuery { + ", addAttachmentClassificationNameToSelectClauseForOrdering=" + addAttachmentClassificationNameToSelectClauseForOrdering + ", wildcardSearchFieldsIn=" - + wildcardSearchFieldsIn + + wildcardSearchFieldIn + ", wildcardSearchValueLike=" + wildcardSearchValueLike + "]"; diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryMapper.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryMapper.java index 5679e8876..37931a8ec 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryMapper.java @@ -133,7 +133,7 @@ public interface TaskQueryMapper { + "AND a.REF_VALUE IN(#{item}) " + "AND (UPPER(a.REF_VALUE) LIKE #{item}) " + " AND ( ( a.RECEIVED >= #{item.begin} AND a.RECEIVED <=#{item.end} )) " - + "AND (t.${item} LIKE #{wildcardSearchValueLike}) " + + "AND (t.${item} LIKE #{wildcardSearchValueLike}) " + "" + "ORDER BY ${item} " + "") @@ -320,7 +320,7 @@ public interface TaskQueryMapper { + "AND a.REF_VALUE IN(#{item}) " + "AND (UPPER(a.REF_VALUE) LIKE #{item}) " + " AND ( ( a.RECEIVED >= #{item.begin} AND a.RECEIVED <=#{item.end} )) " - + "AND (t.${item} LIKE #{wildcardSearchValueLike}) " + + "AND (t.${item} LIKE #{wildcardSearchValueLike}) " + " " + "), Y (ID, EXTERNAL_ID, CREATED, CLAIMED, COMPLETED, MODIFIED, PLANNED, DUE, NAME, CREATOR, DESCRIPTION, NOTE, PRIORITY, STATE, TCLASSIFICATION_KEY, " + " CLASSIFICATION_CATEGORY, CLASSIFICATION_ID, WORKBASKET_ID, DOMAIN, WORKBASKET_KEY, BUSINESS_PROCESS_ID, PARENT_BUSINESS_PROCESS_ID, OWNER, " diff --git a/lib/taskana-core/src/test/java/acceptance/task/QueryTasksAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/QueryTasksAccTest.java index 49804460d..028304657 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/QueryTasksAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/QueryTasksAccTest.java @@ -642,8 +642,6 @@ class QueryTasksAccTest extends AbstractAccTest { void testQueryForOrderByWorkbasketIdDesc() { List results = taskService.createTaskQuery().orderByWorkbasketId(DESCENDING).list(); - .orderByCustomAttribute("4", DESCENDING) - assertEquals("99rty", results.get(0).getCustomAttribute("4")); assertThat(results) .hasSizeGreaterThan(2) diff --git a/lib/taskana-core/src/test/java/acceptance/task/QueryTasksByWildcardSearchAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/QueryTasksByWildcardSearchAccTest.java index 381cc3d1a..d5b62948b 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/QueryTasksByWildcardSearchAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/QueryTasksByWildcardSearchAccTest.java @@ -1,9 +1,11 @@ package acceptance.task; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import acceptance.AbstractAccTest; import java.util.List; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -11,7 +13,7 @@ import pro.taskana.common.api.BaseQuery.SortDirection; import pro.taskana.security.JaasExtension; import pro.taskana.security.WithAccessId; import pro.taskana.task.api.TaskService; -import pro.taskana.task.api.WildcardSearchFields; +import pro.taskana.task.api.WildcardSearchField; import pro.taskana.task.api.models.TaskSummary; @ExtendWith(JaasExtension.class) @@ -24,8 +26,8 @@ public class QueryTasksByWildcardSearchAccTest extends AbstractAccTest { void should_ReturnAllTasksByWildcardSearch_For_ProvidedSearchValue() { TaskService taskService = taskanaEngine.getTaskService(); - WildcardSearchFields[] wildcards = { - WildcardSearchFields.CUSTOM_3, WildcardSearchFields.CUSTOM_4, WildcardSearchFields.NAME + WildcardSearchField[] wildcards = { + WildcardSearchField.CUSTOM_3, WildcardSearchField.CUSTOM_4, WildcardSearchField.NAME }; List foundTasks = @@ -43,30 +45,53 @@ public class QueryTasksByWildcardSearchAccTest extends AbstractAccTest { userName = "teamlead_1", groupNames = {"group_1", "group_2"}) @Test - void should_ReturnAllTasks_When_ProvidingNoSearchFieldsOrValue() { - + void should_ReturnAllTasks_For_ProvidedSearchValueAndAdditionalParameters() { TaskService taskService = taskanaEngine.getTaskService(); - WildcardSearchFields[] wildcards = { - WildcardSearchFields.CUSTOM_3, WildcardSearchFields.CUSTOM_4, WildcardSearchFields.NAME + WildcardSearchField[] wildcards = { + WildcardSearchField.CUSTOM_3, WildcardSearchField.CUSTOM_4, WildcardSearchField.NAME }; List foundTasks = taskService .createTaskQuery() .wildcardSearchFieldsIn(wildcards) - .orderByName(SortDirection.ASCENDING) - .list(); - - assertThat(foundTasks).hasSize(83); - - foundTasks = - taskService - .createTaskQuery() .wildcardSearchValueLike("%99%") + .ownerIn("user_1_1") + .businessProcessIdLike("%PI2%") .orderByName(SortDirection.ASCENDING) .list(); - assertThat(foundTasks).hasSize(83); + assertThat(foundTasks).hasSize(1); + } + + @WithAccessId( + userName = "teamlead_1", + groupNames = {"group_1", "group_2"}) + @Test + void should_ThrowException_When_NotUsingSearchFieldsAndValueParamsTogether() { + + TaskService taskService = taskanaEngine.getTaskService(); + + ThrowingCallable queryAttempt = + () -> + taskService + .createTaskQuery() + .wildcardSearchValueLike("%99%") + .orderByName(SortDirection.ASCENDING) + .list(); + + assertThatThrownBy(queryAttempt).isInstanceOf(IllegalArgumentException.class); + + queryAttempt = + () -> + taskService + .createTaskQuery() + .wildcardSearchFieldsIn( + WildcardSearchField.CUSTOM_1, WildcardSearchField.DESCRIPTION) + .orderByName(SortDirection.ASCENDING) + .list(); + + assertThatThrownBy(queryAttempt).isInstanceOf(IllegalArgumentException.class); } } diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskController.java index b5e6dac53..9224c5ba6 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskController.java @@ -37,7 +37,7 @@ import pro.taskana.rest.resource.TaskSummaryResourceAssembler; import pro.taskana.task.api.TaskQuery; import pro.taskana.task.api.TaskService; import pro.taskana.task.api.TaskState; -import pro.taskana.task.api.WildcardSearchFields; +import pro.taskana.task.api.WildcardSearchField; import pro.taskana.task.api.exceptions.AttachmentPersistenceException; import pro.taskana.task.api.exceptions.InvalidOwnerException; import pro.taskana.task.api.exceptions.InvalidStateException; @@ -413,12 +413,12 @@ public class TaskController extends AbstractPagingController { return taskQuery; } - private WildcardSearchFields[] createWildcardSearchFields(String[] wildcardFields) { + private WildcardSearchField[] createWildcardSearchFields(String[] wildcardFields) { return Stream.of(wildcardFields) - .map(WildcardSearchFields::fromString) + .map(WildcardSearchField::fromString) .filter(Objects::nonNull) - .toArray(WildcardSearchFields[]::new); + .toArray(WildcardSearchField[]::new); } private void updateTaskQueryWithWorkbasketKey( @@ -474,15 +474,11 @@ public class TaskController extends AbstractPagingController { && params.containsKey(WILDCARD_SEARCH_VALUE)) { throw new IllegalArgumentException( - "It is prohibited to use the params " - + "\"" + "The params " + WILDCARD_SEARCH_FIELDS - + "\"" - + " or " - + "\"" + + " and " + WILDCARD_SEARCH_VALUE - + "\"" - + " without one another. If one is provided you must provide the other one as well"); + + " must be used together!"); } }