From 3422116678ffaed5fad89302bd741018b0fade6e Mon Sep 17 00:00:00 2001 From: tinabandalo Date: Tue, 16 Mar 2021 21:01:44 +0100 Subject: [PATCH 1/4] refactor handleAssignment --- .../behavior/UserTaskActivityBehavior.java | 167 ++++++++++-------- 1 file changed, 94 insertions(+), 73 deletions(-) diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java index 23c091ba11a..2e5efd040be 100755 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java @@ -301,23 +301,71 @@ protected void handleAssignments(TaskService taskService, String assignee, Strin List candidateGroups, TaskEntity task, ExpressionManager expressionManager, DelegateExecution execution, ProcessEngineConfigurationImpl processEngineConfiguration) { - if (StringUtils.isNotEmpty(assignee)) { - Object assigneeExpressionValue = expressionManager.createExpression(assignee).getValue(execution); - String assigneeValue = null; - if (assigneeExpressionValue != null) { - assigneeValue = assigneeExpressionValue.toString(); + handleAssignee(assignee, expressionManager, execution, processEngineConfiguration, task); + handleOwner(owner, expressionManager, execution, processEngineConfiguration, task); + handleCandidateGroups(candidateGroups, expressionManager, execution, processEngineConfiguration, task); + handleCandidateUsers(candidateUsers, expressionManager, execution, processEngineConfiguration, task); + + if (userTask.getCustomUserIdentityLinks() != null && !userTask.getCustomUserIdentityLinks().isEmpty()) { + + List customIdentityLinkEntities = new ArrayList<>(); + for (String customUserIdentityLinkType : userTask.getCustomUserIdentityLinks().keySet()) { + for (String userIdentityLink : userTask.getCustomUserIdentityLinks().get(customUserIdentityLinkType)) { + Expression idExpression = expressionManager.createExpression(userIdentityLink); + Object value = idExpression.getValue(execution); + + Collection userIds = extractCandidates(value); + for (String userId : userIds) { + IdentityLinkEntity identityLinkEntity = processEngineConfiguration.getIdentityLinkServiceConfiguration() + .getIdentityLinkService().createTaskIdentityLink(task.getId(), userId, null, customUserIdentityLinkType); + IdentityLinkUtil.handleTaskIdentityLinkAddition(task, identityLinkEntity); + customIdentityLinkEntities.add(identityLinkEntity); + } + } } - if (StringUtils.isNotEmpty(assigneeValue)) { - TaskHelper.changeTaskAssignee(task, assigneeValue); + if (!customIdentityLinkEntities.isEmpty()) { if (processEngineConfiguration.isLoggingSessionEnabled()) { - ObjectNode loggingNode = BpmnLoggingSessionUtil.fillBasicTaskLoggingData("Set task assignee value to " + assigneeValue, task, execution); - loggingNode.put("taskAssignee", assigneeValue); - LoggingSessionUtil.addLoggingData(LoggingSessionConstants.TYPE_USER_TASK_SET_ASSIGNEE, loggingNode, ScopeTypes.BPMN); + BpmnLoggingSessionUtil.addTaskIdentityLinkData(LoggingSessionConstants.TYPE_USER_TASK_SET_USER_IDENTITY_LINKS, + "Added " + customIdentityLinkEntities.size() + " custom user identity links to task", true, + customIdentityLinkEntities, task, execution); } } } + if (userTask.getCustomGroupIdentityLinks() != null && !userTask.getCustomGroupIdentityLinks().isEmpty()) { + + List customIdentityLinkEntities = new ArrayList<>(); + for (String customGroupIdentityLinkType : userTask.getCustomGroupIdentityLinks().keySet()) { + for (String groupIdentityLink : userTask.getCustomGroupIdentityLinks().get(customGroupIdentityLinkType)) { + + Expression idExpression = expressionManager.createExpression(groupIdentityLink); + Object value = idExpression.getValue(execution); + Collection groupIds = extractCandidates(value); + for (String groupId : groupIds) { + IdentityLinkEntity identityLinkEntity = processEngineConfiguration.getIdentityLinkServiceConfiguration() + .getIdentityLinkService().createTaskIdentityLink( + task.getId(), null, groupId, customGroupIdentityLinkType); + IdentityLinkUtil.handleTaskIdentityLinkAddition(task, identityLinkEntity); + customIdentityLinkEntities.add(identityLinkEntity); + } + } + } + + if (!customIdentityLinkEntities.isEmpty()) { + if (processEngineConfiguration.isLoggingSessionEnabled()) { + BpmnLoggingSessionUtil.addTaskIdentityLinkData(LoggingSessionConstants.TYPE_USER_TASK_SET_GROUP_IDENTITY_LINKS, + "Added " + customIdentityLinkEntities.size() + " custom group identity links to task", false, + customIdentityLinkEntities, task, execution); + } + } + } + + } + + protected void handleOwner(String owner, ExpressionManager expressionManager, DelegateExecution execution, + ProcessEngineConfigurationImpl processEngineConfiguration, TaskEntity task) { + if (StringUtils.isNotEmpty(owner)) { Object ownerExpressionValue = expressionManager.createExpression(owner).getValue(execution); String ownerValue = null; @@ -334,6 +382,31 @@ protected void handleAssignments(TaskService taskService, String assignee, Strin } } } + } + + protected void handleAssignee(String assignee, ExpressionManager expressionManager, DelegateExecution execution, + ProcessEngineConfigurationImpl processEngineConfiguration, TaskEntity task){ + + if (StringUtils.isNotEmpty(assignee)) { + Object assigneeExpressionValue = expressionManager.createExpression(assignee).getValue(execution); + String assigneeValue = null; + if (assigneeExpressionValue != null) { + assigneeValue = assigneeExpressionValue.toString(); + } + + if (StringUtils.isNotEmpty(assigneeValue)) { + TaskHelper.changeTaskAssignee(task, assigneeValue); + if (processEngineConfiguration.isLoggingSessionEnabled()) { + ObjectNode loggingNode = BpmnLoggingSessionUtil.fillBasicTaskLoggingData("Set task assignee value to " + assigneeValue, task, execution); + loggingNode.put("taskAssignee", assigneeValue); + LoggingSessionUtil.addLoggingData(LoggingSessionConstants.TYPE_USER_TASK_SET_ASSIGNEE, loggingNode, ScopeTypes.BPMN); + } + } + } + } + + protected void handleCandidateGroups(List candidateGroups, ExpressionManager expressionManager, DelegateExecution execution, + ProcessEngineConfigurationImpl processEngineConfiguration, TaskEntity task) { if (candidateGroups != null && !candidateGroups.isEmpty()) { List allIdentityLinkEntities = new ArrayList<>(); @@ -351,15 +424,19 @@ protected void handleAssignments(TaskService taskService, String assignee, Strin } } } - + if (!allIdentityLinkEntities.isEmpty()) { if (processEngineConfiguration.isLoggingSessionEnabled()) { - BpmnLoggingSessionUtil.addTaskIdentityLinkData(LoggingSessionConstants.TYPE_USER_TASK_SET_GROUP_IDENTITY_LINKS, + BpmnLoggingSessionUtil.addTaskIdentityLinkData(LoggingSessionConstants.TYPE_USER_TASK_SET_GROUP_IDENTITY_LINKS, "Added " + allIdentityLinkEntities.size() + " candidate group identity links to task", false, allIdentityLinkEntities, task, execution); } } } + } + + protected void handleCandidateUsers(List candidateUsers, ExpressionManager expressionManager, DelegateExecution execution, + ProcessEngineConfigurationImpl processEngineConfiguration, TaskEntity task) { if (candidateUsers != null && !candidateUsers.isEmpty()) { List allIdentityLinkEntities = new ArrayList<>(); @@ -377,71 +454,15 @@ protected void handleAssignments(TaskService taskService, String assignee, Strin } } } - - if (!allIdentityLinkEntities.isEmpty()) { - if (processEngineConfiguration.isLoggingSessionEnabled()) { - BpmnLoggingSessionUtil.addTaskIdentityLinkData(LoggingSessionConstants.TYPE_USER_TASK_SET_USER_IDENTITY_LINKS, - "Added " + allIdentityLinkEntities.size() + " candidate user identity links to task", true, - allIdentityLinkEntities, task, execution); - } - } - } - - if (userTask.getCustomUserIdentityLinks() != null && !userTask.getCustomUserIdentityLinks().isEmpty()) { - - List customIdentityLinkEntities = new ArrayList<>(); - for (String customUserIdentityLinkType : userTask.getCustomUserIdentityLinks().keySet()) { - for (String userIdentityLink : userTask.getCustomUserIdentityLinks().get(customUserIdentityLinkType)) { - Expression idExpression = expressionManager.createExpression(userIdentityLink); - Object value = idExpression.getValue(execution); - - Collection userIds = extractCandidates(value); - for (String userId : userIds) { - IdentityLinkEntity identityLinkEntity = processEngineConfiguration.getIdentityLinkServiceConfiguration() - .getIdentityLinkService().createTaskIdentityLink(task.getId(), userId, null, customUserIdentityLinkType); - IdentityLinkUtil.handleTaskIdentityLinkAddition(task, identityLinkEntity); - customIdentityLinkEntities.add(identityLinkEntity); - } - } - } - if (!customIdentityLinkEntities.isEmpty()) { - if (processEngineConfiguration.isLoggingSessionEnabled()) { - BpmnLoggingSessionUtil.addTaskIdentityLinkData(LoggingSessionConstants.TYPE_USER_TASK_SET_USER_IDENTITY_LINKS, - "Added " + customIdentityLinkEntities.size() + " custom user identity links to task", true, - customIdentityLinkEntities, task, execution); - } - } - } - - if (userTask.getCustomGroupIdentityLinks() != null && !userTask.getCustomGroupIdentityLinks().isEmpty()) { - - List customIdentityLinkEntities = new ArrayList<>(); - for (String customGroupIdentityLinkType : userTask.getCustomGroupIdentityLinks().keySet()) { - for (String groupIdentityLink : userTask.getCustomGroupIdentityLinks().get(customGroupIdentityLinkType)) { - - Expression idExpression = expressionManager.createExpression(groupIdentityLink); - Object value = idExpression.getValue(execution); - Collection groupIds = extractCandidates(value); - for (String groupId : groupIds) { - IdentityLinkEntity identityLinkEntity = processEngineConfiguration.getIdentityLinkServiceConfiguration() - .getIdentityLinkService().createTaskIdentityLink( - task.getId(), null, groupId, customGroupIdentityLinkType); - IdentityLinkUtil.handleTaskIdentityLinkAddition(task, identityLinkEntity); - customIdentityLinkEntities.add(identityLinkEntity); - } - } - } - - if (!customIdentityLinkEntities.isEmpty()) { + if (!allIdentityLinkEntities.isEmpty()) { if (processEngineConfiguration.isLoggingSessionEnabled()) { - BpmnLoggingSessionUtil.addTaskIdentityLinkData(LoggingSessionConstants.TYPE_USER_TASK_SET_GROUP_IDENTITY_LINKS, - "Added " + customIdentityLinkEntities.size() + " custom group identity links to task", false, - customIdentityLinkEntities, task, execution); + BpmnLoggingSessionUtil.addTaskIdentityLinkData(LoggingSessionConstants.TYPE_USER_TASK_SET_USER_IDENTITY_LINKS, + "Added " + allIdentityLinkEntities.size() + " candidate user identity links to task", true, + allIdentityLinkEntities, task, execution); } } } - } protected Collection extractCandidates(Object value) { @@ -465,7 +486,7 @@ protected Collection extractCandidates(Object value) { return Collections.emptyList(); } - + protected String getAssigneeValue(UserTask userTask, MigrationContext migrationContext, ObjectNode taskElementProperties) { if (migrationContext != null && migrationContext.getAssignee() != null) { return migrationContext.getAssignee(); From 03c54bbffa08656f615ce1af6e963dbfb1ee9c21 Mon Sep 17 00:00:00 2001 From: tinabandalo Date: Fri, 19 Mar 2021 17:23:17 +0100 Subject: [PATCH 2/4] added unsupported type handling for candidateGroups and candidateUsers --- .../behavior/UserTaskActivityBehavior.java | 26 ++++-- .../test/bpmn/usertask/UserTaskTest.java | 83 +++++++++++++++---- ...est.testUnsupportedPropertyType.bpmn20.xml | 21 +++++ 3 files changed, 108 insertions(+), 22 deletions(-) create mode 100644 modules/flowable-engine/src/test/resources/org/flowable/engine/test/bpmn/usertask/UserTaskTest.testUnsupportedPropertyType.bpmn20.xml diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java index 2e5efd040be..d97c23f0834 100755 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java @@ -314,7 +314,7 @@ protected void handleAssignments(TaskService taskService, String assignee, Strin Expression idExpression = expressionManager.createExpression(userIdentityLink); Object value = idExpression.getValue(execution); - Collection userIds = extractCandidates(value); + Collection userIds = extractCandidates(value, idExpression.getExpressionText()); for (String userId : userIds) { IdentityLinkEntity identityLinkEntity = processEngineConfiguration.getIdentityLinkServiceConfiguration() .getIdentityLinkService().createTaskIdentityLink(task.getId(), userId, null, customUserIdentityLinkType); @@ -341,7 +341,7 @@ protected void handleAssignments(TaskService taskService, String assignee, Strin Expression idExpression = expressionManager.createExpression(groupIdentityLink); Object value = idExpression.getValue(execution); - Collection groupIds = extractCandidates(value); + Collection groupIds = extractCandidates(value, idExpression.getExpressionText()); for (String groupId : groupIds) { IdentityLinkEntity identityLinkEntity = processEngineConfiguration.getIdentityLinkServiceConfiguration() .getIdentityLinkService().createTaskIdentityLink( @@ -414,7 +414,7 @@ protected void handleCandidateGroups(List candidateGroups, ExpressionMan Expression groupIdExpr = expressionManager.createExpression(candidateGroup); Object value = groupIdExpr.getValue(execution); if (value != null) { - Collection candidates = extractCandidates(value); + Collection candidates = extractCandidates(value, groupIdExpr.getExpressionText()); List identityLinkEntities = processEngineConfiguration.getIdentityLinkServiceConfiguration() .getIdentityLinkService().addCandidateGroups(task.getId(), candidates); @@ -444,7 +444,7 @@ protected void handleCandidateUsers(List candidateUsers, ExpressionManag Expression userIdExpr = expressionManager.createExpression(candidateUser); Object value = userIdExpr.getValue(execution); if (value != null) { - Collection candidates = extractCandidates(value); + Collection candidates = extractCandidates(value, userIdExpr.getExpressionText()); List identityLinkEntities = processEngineConfiguration.getIdentityLinkServiceConfiguration() .getIdentityLinkService().addCandidateUsers(task.getId(), candidates); @@ -465,7 +465,8 @@ protected void handleCandidateUsers(List candidateUsers, ExpressionManag } } - protected Collection extractCandidates(Object value) { + public Collection extractCandidates(Object value, String expressionText) { + handleUnsupportedType(value, expressionText); if (value instanceof Collection) { return (Collection) value; } else if (value instanceof ArrayNode) { @@ -487,6 +488,21 @@ protected Collection extractCandidates(Object value) { } + protected void handleUnsupportedType(Object value, String expressionText) { + if (value instanceof Boolean) { + throw new FlowableIllegalArgumentException("Value of type boolean is not supported for expression '" + expressionText + "'."); + } + if (value instanceof Double) { + throw new FlowableIllegalArgumentException("Value of type double is not supported for expression '" + expressionText + "'."); + } + if (value instanceof Float) { + throw new FlowableIllegalArgumentException("Value of type float is not supported for expression '" + expressionText + "'."); + } + if (value instanceof ObjectNode) { + throw new FlowableIllegalArgumentException("Value of type objectNode is not supported for expression '" + expressionText + "'."); + } + } + protected String getAssigneeValue(UserTask userTask, MigrationContext migrationContext, ObjectNode taskElementProperties) { if (migrationContext != null && migrationContext.getAssignee() != null) { return migrationContext.getAssignee(); diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java index 2aef220b3b1..cd7f8dd7275 100644 --- a/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java @@ -1,9 +1,9 @@ /* Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -14,11 +14,13 @@ package org.flowable.engine.test.bpmn.usertask; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.HashMap; import java.util.List; import java.util.Map; +import org.flowable.common.engine.api.FlowableIllegalArgumentException; import org.flowable.common.engine.api.scope.ScopeTypes; import org.flowable.common.engine.impl.history.HistoryLevel; import org.flowable.common.engine.impl.interceptor.CommandExecutor; @@ -39,6 +41,10 @@ import org.flowable.task.api.history.HistoricTaskInstance; import org.junit.jupiter.api.Test; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; + /** * @author Joram Barrez */ @@ -59,7 +65,7 @@ public void testTaskPropertiesNotNull() { assertThat(task.getProcessDefinitionId()).isNotNull(); assertThat(task.getTaskDefinitionKey()).isNotNull(); assertThat(task.getCreateTime()).isNotNull(); - + // the next test verifies that if an execution creates a task, that no events are created during creation of the task. if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processEngineConfiguration)) { assertThat(taskService.getTaskEvents(task.getId())).isEmpty(); @@ -154,7 +160,7 @@ public void testTaskCategory() { // Complete task and verify history taskService.complete(task.getId()); - + if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.AUDIT, processEngineConfiguration)) { HistoricTaskInstance historicTaskInstance = historyService.createHistoricTaskInstanceQuery().taskId(task.getId()).singleResult(); assertThat(historicTaskInstance.getCategory()).isEqualTo(newCategory); @@ -189,7 +195,7 @@ public void testTaskFormKeyWhenUsingIncludeVariables() { assertThat(task.getFormKey()).isEqualTo("test123"); } - + @Test @Deployment public void testEmptyAssignmentExpression() { @@ -199,27 +205,27 @@ public void testEmptyAssignmentExpression() { variableMap.put("candidateGroups", null); ProcessInstance processInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", variableMap); assertThat(processInstance).isNotNull(); - + Task task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult(); assertThat(task).isNotNull(); assertThat(task.getAssignee()).isNull(); List identityLinks = taskService.getIdentityLinksForTask(task.getId()); assertThat(identityLinks).isEmpty(); - + variableMap = new HashMap<>(); variableMap.put("assignee", ""); variableMap.put("candidateUsers", ""); variableMap.put("candidateGroups", ""); processInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", variableMap); assertThat(processInstance).isNotNull(); - + task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult(); assertThat(task).isNotNull(); assertThat(task.getAssignee()).isNull(); identityLinks = taskService.getIdentityLinksForTask(task.getId()); assertThat(identityLinks).isEmpty(); } - + @Test @Deployment public void testNonStringProperties() { @@ -233,7 +239,7 @@ public void testNonStringProperties() { vars.put("taskCandidateGroups", 7); vars.put("taskCandidateUsers", 8); ProcessInstance processInstance = runtimeService.startProcessInstanceByKey("nonStringProperties", vars); - + Task task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult(); assertThat(task.getName()).isEqualTo("1"); assertThat(task.getDescription()).isEqualTo("2"); @@ -241,7 +247,7 @@ public void testNonStringProperties() { assertThat(task.getFormKey()).isEqualTo("4"); assertThat(task.getAssignee()).isEqualTo("5"); assertThat(task.getOwner()).isEqualTo("6"); - + List identityLinks = taskService.getIdentityLinksForTask(task.getId()); assertThat(identityLinks).hasSize(4); int candidateIdentityLinkCount = 0; @@ -257,13 +263,46 @@ public void testNonStringProperties() { } assertThat(candidateIdentityLinkCount).isEqualTo(2); } - + + @Test + @Deployment + public void testUnsupportedPropertyType() { + ObjectMapper objectMapper = new ObjectMapper(); + ObjectNode objectNode = objectMapper.createObjectNode(); + objectNode.set("integer", objectMapper.convertValue(1, JsonNode.class)); + objectNode.set("string", objectMapper.convertValue("string", JsonNode.class)); + + assertErrorForProperty("unsupportedPropertyType", false, "user1", + "Value of type boolean is not supported for expression '${taskCandidateGroups}'."); + + assertErrorForProperty("unsupportedPropertyType", 5.21f, "user1", + "Value of type float is not supported for expression '${taskCandidateGroups}'."); + + assertErrorForProperty("unsupportedPropertyType", 0.55d, "user1", + "Value of type double is not supported for expression '${taskCandidateGroups}'."); + + assertErrorForProperty("unsupportedPropertyType", objectNode, "user1", + "Value of type objectNode is not supported for expression '${taskCandidateGroups}'."); + + assertErrorForProperty("unsupportedPropertyType", "group1", false, + "Value of type boolean is not supported for expression '${taskCandidateUsers}'."); + + assertErrorForProperty("unsupportedPropertyType", "group1", 5.21f, + "Value of type float is not supported for expression '${taskCandidateUsers}'."); + + assertErrorForProperty("unsupportedPropertyType", "group1", 0.55d, + "Value of type double is not supported for expression '${taskCandidateUsers}'."); + + assertErrorForProperty("unsupportedPropertyType", "group1", objectNode, + "Value of type objectNode is not supported for expression '${taskCandidateUsers}'."); + } + @Test @Deployment(resources="org/flowable/engine/test/bpmn/usertask/UserTaskTest.testTaskPropertiesNotNull.bpmn20.xml") public void testCreateUserTaskInterceptor() throws Exception { TestCreateUserTaskInterceptor testCreateUserTaskInterceptor = new TestCreateUserTaskInterceptor(); processEngineConfiguration.setCreateUserTaskInterceptor(testCreateUserTaskInterceptor); - + try { ProcessInstance processInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess"); @@ -272,10 +311,10 @@ public void testCreateUserTaskInterceptor() throws Exception { assertThat(task.getName()).isEqualTo("my task"); assertThat(task.getDescription()).isEqualTo("Very important"); assertThat(task.getCategory()).isEqualTo("testCategory"); - + assertThat(testCreateUserTaskInterceptor.getBeforeCreateUserTaskCounter()).isEqualTo(1); assertThat(testCreateUserTaskInterceptor.getAfterCreateUserTaskCounter()).isEqualTo(1); - + } finally { processEngineConfiguration.setCreateUserTaskInterceptor(null); } @@ -303,11 +342,21 @@ public void testUserTaskIdVariableName() throws Exception { assertThat(myExpressionTaskId).isEqualTo(actualTaskId); } + protected void assertErrorForProperty(String processDefinitionKey, Object candidateGroupsValue, Object candidateUsersValue, String expectedMessage) { + Map variableMap = new HashMap<>(); + variableMap.put("taskCandidateGroups", candidateGroupsValue); + variableMap.put("taskCandidateUsers", candidateUsersValue); + + assertThatThrownBy(() -> runtimeService.startProcessInstanceByKey(processDefinitionKey, variableMap)) + .isInstanceOf(FlowableIllegalArgumentException.class) + .hasMessageContaining(expectedMessage); + } + protected class TestCreateUserTaskInterceptor implements CreateUserTaskInterceptor { - + protected int beforeCreateUserTaskCounter = 0; protected int afterCreateUserTaskCounter = 0; - + @Override public void beforeCreateUserTask(CreateUserTaskBeforeContext context) { beforeCreateUserTaskCounter++; diff --git a/modules/flowable-engine/src/test/resources/org/flowable/engine/test/bpmn/usertask/UserTaskTest.testUnsupportedPropertyType.bpmn20.xml b/modules/flowable-engine/src/test/resources/org/flowable/engine/test/bpmn/usertask/UserTaskTest.testUnsupportedPropertyType.bpmn20.xml new file mode 100644 index 00000000000..c72162af5ff --- /dev/null +++ b/modules/flowable-engine/src/test/resources/org/flowable/engine/test/bpmn/usertask/UserTaskTest.testUnsupportedPropertyType.bpmn20.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + \ No newline at end of file From c3819da5dbdd0127fc0b75fb03b79c40a502675b Mon Sep 17 00:00:00 2001 From: tinabandalo Date: Mon, 22 Mar 2021 16:58:27 +0100 Subject: [PATCH 3/4] added limit for candidate groups and users and method to normalize values if duplicate etc. --- .../behavior/UserTaskActivityBehavior.java | 32 +++++++- .../cfg/ProcessEngineConfigurationImpl.java | 18 +++++ .../test/bpmn/usertask/UserTaskTest.java | 73 +++++++++++++++++++ 3 files changed, 120 insertions(+), 3 deletions(-) diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java index d97c23f0834..d89b5befdd4 100755 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java @@ -18,6 +18,8 @@ import java.util.Collections; import java.util.Date; import java.util.List; +import static java.util.stream.Collectors.toList; +import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; import org.flowable.bpmn.model.UserTask; @@ -415,8 +417,13 @@ protected void handleCandidateGroups(List candidateGroups, ExpressionMan Object value = groupIdExpr.getValue(execution); if (value != null) { Collection candidates = extractCandidates(value, groupIdExpr.getExpressionText()); + Collection candidatesNormalized = normalizeCandidates(candidates); + + if(isCandidateGroupsLimit(candidatesNormalized.size(), processEngineConfiguration.getUserTaskCandidateGroupsLimit())) { + throw new FlowableException("The number of groups after resolving expression exceeds the limit " + processEngineConfiguration.getUserTaskCandidateGroupsLimit() + " of allowed groups for a user task."); + } List identityLinkEntities = processEngineConfiguration.getIdentityLinkServiceConfiguration() - .getIdentityLinkService().addCandidateGroups(task.getId(), candidates); + .getIdentityLinkService().addCandidateGroups(task.getId(), candidatesNormalized); if (identityLinkEntities != null && !identityLinkEntities.isEmpty()) { IdentityLinkUtil.handleTaskIdentityLinkAdditions(task, identityLinkEntities); @@ -439,14 +446,21 @@ protected void handleCandidateUsers(List candidateUsers, ExpressionManag ProcessEngineConfigurationImpl processEngineConfiguration, TaskEntity task) { if (candidateUsers != null && !candidateUsers.isEmpty()) { + List allIdentityLinkEntities = new ArrayList<>(); for (String candidateUser : candidateUsers) { Expression userIdExpr = expressionManager.createExpression(candidateUser); Object value = userIdExpr.getValue(execution); if (value != null) { + Collection candidates = extractCandidates(value, userIdExpr.getExpressionText()); + Collection candidatesNormalized = normalizeCandidates(candidates); + + if(isCandidateUsersLimit(candidatesNormalized.size(), processEngineConfiguration.getUserTaskCandidateUsersLimit())) { + throw new FlowableException("The number of users after resolving expression exceeds the limit " + processEngineConfiguration.getUserTaskCandidateUsersLimit() + " of allowed users for a user task."); + } List identityLinkEntities = processEngineConfiguration.getIdentityLinkServiceConfiguration() - .getIdentityLinkService().addCandidateUsers(task.getId(), candidates); + .getIdentityLinkService().addCandidateUsers(task.getId(), candidatesNormalized); if (identityLinkEntities != null && !identityLinkEntities.isEmpty()) { IdentityLinkUtil.handleTaskIdentityLinkAdditions(task, identityLinkEntities); @@ -465,6 +479,19 @@ protected void handleCandidateUsers(List candidateUsers, ExpressionManag } } + public boolean isCandidateUsersLimit(int candidateUsersSize, int userTaskCandidateUsersLimit) { + return userTaskCandidateUsersLimit >= 0 && candidateUsersSize > userTaskCandidateUsersLimit; + } + + public boolean isCandidateGroupsLimit(int candidateGroupsSize, int userTaskCandidateGroupsLimit) { + return userTaskCandidateGroupsLimit >= 0 && candidateGroupsSize > userTaskCandidateGroupsLimit; + } + + public Collection normalizeCandidates(Collection candidates) { + Stream candidatesNormalized = candidates.stream().map(String::trim).filter(candidate -> !candidate.isEmpty()).distinct(); + return candidatesNormalized.collect(toList()); + } + public Collection extractCandidates(Object value, String expressionText) { handleUnsupportedType(value, expressionText); if (value instanceof Collection) { @@ -475,7 +502,6 @@ public Collection extractCandidates(Object value, String expressionText) for (JsonNode node : valueArrayNode) { candidates.add(node.asText()); } - return candidates; } else if (value != null) { String str = value.toString(); diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/cfg/ProcessEngineConfigurationImpl.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/cfg/ProcessEngineConfigurationImpl.java index 5ad0b471541..41319d4b187 100755 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/cfg/ProcessEngineConfigurationImpl.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/cfg/ProcessEngineConfigurationImpl.java @@ -883,6 +883,8 @@ public abstract class ProcessEngineConfigurationImpl extends ProcessEngineConfig protected boolean isExpressionCacheEnabled = true; protected int expressionCacheSize = 4096; protected int expressionTextLengthCacheLimit = -1; // negative value to have no max length + protected int userTaskCandidateGroupsLimit = -1; // By default, no limit + protected int userTaskCandidateUsersLimit = -1; // By default, no limit protected BusinessCalendarManager businessCalendarManager; @@ -3425,6 +3427,22 @@ public ProcessEngineConfigurationImpl setExpressionTextLengthCacheLimit(int expr return this; } + public int getUserTaskCandidateGroupsLimit() { + return userTaskCandidateGroupsLimit; + } + + public void setUserTaskCandidateGroupsLimit(int userTaskCandidateGroupsLimit) { + this.userTaskCandidateGroupsLimit = userTaskCandidateGroupsLimit; + } + + public int getUserTaskCandidateUsersLimit() { + return userTaskCandidateUsersLimit; + } + + public void setUserTaskCandidateUsersLimit(int userTaskCandidateUsersLimit) { + this.userTaskCandidateUsersLimit = userTaskCandidateUsersLimit; + } + public BusinessCalendarManager getBusinessCalendarManager() { return businessCalendarManager; } diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java index cd7f8dd7275..b82ddd51bb2 100644 --- a/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; +import org.flowable.common.engine.api.FlowableException; import org.flowable.common.engine.api.FlowableIllegalArgumentException; import org.flowable.common.engine.api.scope.ScopeTypes; import org.flowable.common.engine.impl.history.HistoryLevel; @@ -39,6 +40,7 @@ import org.flowable.identitylink.api.IdentityLinkType; import org.flowable.task.api.Task; import org.flowable.task.api.history.HistoricTaskInstance; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import com.fasterxml.jackson.databind.JsonNode; @@ -50,6 +52,13 @@ */ public class UserTaskTest extends PluggableFlowableTestCase { + @AfterEach + protected void tearDown() throws Exception { + //set back to default value + processEngineConfiguration.setUserTaskCandidateUsersLimit(-1); + processEngineConfiguration.setUserTaskCandidateGroupsLimit(-1); + } + @Test @Deployment public void testTaskPropertiesNotNull() { @@ -342,6 +351,70 @@ public void testUserTaskIdVariableName() throws Exception { assertThat(myExpressionTaskId).isEqualTo(actualTaskId); } + @Test + @Deployment(resources="org/flowable/engine/test/bpmn/usertask/UserTaskTest.testNonStringProperties.bpmn20.xml") + public void testUserTaskCandidateUsersLimit() { + + Map vars = new HashMap<>(); + vars.put("taskAssignee", "testAssignee"); + vars.put("taskOwner", "testOwner"); + vars.put("taskCandidateGroups", "group1"); + vars.put("taskCandidateUsers", "user1, user2, user3"); + processEngineConfiguration.setUserTaskCandidateUsersLimit(2); + + assertThatThrownBy(() -> runtimeService.startProcessInstanceByKey("nonStringProperties", vars)) + .isInstanceOf(FlowableException.class) + .hasMessageContaining("The number of users after resolving expression exceeds the limit " + processEngineConfiguration.getUserTaskCandidateUsersLimit() +" of allowed users for a user task."); + } + + @Test + @Deployment(resources="org/flowable/engine/test/bpmn/usertask/UserTaskTest.testNonStringProperties.bpmn20.xml") + public void testUserTaskCandidateGroupsLimit() { + + Map vars = new HashMap<>(); + vars.put("taskAssignee", "testAssignee"); + vars.put("taskOwner", "testOwner"); + vars.put("taskCandidateGroups", "group1, group2, group3"); + vars.put("taskCandidateUsers", "user1"); + processEngineConfiguration.setUserTaskCandidateGroupsLimit(2); + + assertThatThrownBy(() -> runtimeService.startProcessInstanceByKey("nonStringProperties", vars)) + .isInstanceOf(FlowableException.class) + .hasMessageContaining("The number of groups after resolving expression exceeds the limit " + processEngineConfiguration.getUserTaskCandidateGroupsLimit() +" of allowed groups for a user task."); + } + + @Test + @Deployment(resources="org/flowable/engine/test/bpmn/usertask/UserTaskTest.testNonStringProperties.bpmn20.xml") + public void testNormalizeCandidates() { + + Map vars = new HashMap<>(); + vars.put("taskAssignee", "testAssignee"); + vars.put("taskOwner", "testOwner"); + vars.put("taskCandidateGroups", "group1, group2, group3, ,group3"); + vars.put("taskCandidateUsers", "user1, , user2, user3, user4, user1"); + + ProcessInstance processInstance = runtimeService.startProcessInstanceByKey("nonStringProperties", vars); + + Task task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult(); + + List identityLinks = taskService.getIdentityLinksForTask(task.getId()); + assertThat(identityLinks).hasSize(9); + int candidateGroupIdentityLinkCount = 0; + int candidateUserIdentityLinkCount = 0; + + for (IdentityLink identityLink : identityLinks) { + if (identityLink.getType().equals(IdentityLinkType.CANDIDATE)) { + if (identityLink.getGroupId() != null) { + candidateGroupIdentityLinkCount++; + } else { + candidateUserIdentityLinkCount++; + } + } + } + assertThat(candidateGroupIdentityLinkCount).isEqualTo(3); + assertThat(candidateUserIdentityLinkCount).isEqualTo(4); + } + protected void assertErrorForProperty(String processDefinitionKey, Object candidateGroupsValue, Object candidateUsersValue, String expectedMessage) { Map variableMap = new HashMap<>(); variableMap.put("taskCandidateGroups", candidateGroupsValue); From 92535741113bd0e930698d27c3be8808e1505d2f Mon Sep 17 00:00:00 2001 From: tinabandalo Date: Fri, 26 Mar 2021 10:22:32 +0100 Subject: [PATCH 4/4] removed limit for candidateGroups and candidateUsers --- .../behavior/UserTaskActivityBehavior.java | 8 +--- .../cfg/ProcessEngineConfigurationImpl.java | 18 -------- .../test/bpmn/usertask/UserTaskTest.java | 41 ------------------- 3 files changed, 1 insertion(+), 66 deletions(-) diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java index d89b5befdd4..e98db16db4b 100755 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java @@ -12,13 +12,13 @@ */ package org.flowable.engine.impl.bpmn.behavior; +import static java.util.stream.Collectors.toList; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.List; -import static java.util.stream.Collectors.toList; import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; @@ -419,9 +419,6 @@ protected void handleCandidateGroups(List candidateGroups, ExpressionMan Collection candidates = extractCandidates(value, groupIdExpr.getExpressionText()); Collection candidatesNormalized = normalizeCandidates(candidates); - if(isCandidateGroupsLimit(candidatesNormalized.size(), processEngineConfiguration.getUserTaskCandidateGroupsLimit())) { - throw new FlowableException("The number of groups after resolving expression exceeds the limit " + processEngineConfiguration.getUserTaskCandidateGroupsLimit() + " of allowed groups for a user task."); - } List identityLinkEntities = processEngineConfiguration.getIdentityLinkServiceConfiguration() .getIdentityLinkService().addCandidateGroups(task.getId(), candidatesNormalized); @@ -456,9 +453,6 @@ protected void handleCandidateUsers(List candidateUsers, ExpressionManag Collection candidates = extractCandidates(value, userIdExpr.getExpressionText()); Collection candidatesNormalized = normalizeCandidates(candidates); - if(isCandidateUsersLimit(candidatesNormalized.size(), processEngineConfiguration.getUserTaskCandidateUsersLimit())) { - throw new FlowableException("The number of users after resolving expression exceeds the limit " + processEngineConfiguration.getUserTaskCandidateUsersLimit() + " of allowed users for a user task."); - } List identityLinkEntities = processEngineConfiguration.getIdentityLinkServiceConfiguration() .getIdentityLinkService().addCandidateUsers(task.getId(), candidatesNormalized); diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/cfg/ProcessEngineConfigurationImpl.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/cfg/ProcessEngineConfigurationImpl.java index 41319d4b187..5ad0b471541 100755 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/cfg/ProcessEngineConfigurationImpl.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/cfg/ProcessEngineConfigurationImpl.java @@ -883,8 +883,6 @@ public abstract class ProcessEngineConfigurationImpl extends ProcessEngineConfig protected boolean isExpressionCacheEnabled = true; protected int expressionCacheSize = 4096; protected int expressionTextLengthCacheLimit = -1; // negative value to have no max length - protected int userTaskCandidateGroupsLimit = -1; // By default, no limit - protected int userTaskCandidateUsersLimit = -1; // By default, no limit protected BusinessCalendarManager businessCalendarManager; @@ -3427,22 +3425,6 @@ public ProcessEngineConfigurationImpl setExpressionTextLengthCacheLimit(int expr return this; } - public int getUserTaskCandidateGroupsLimit() { - return userTaskCandidateGroupsLimit; - } - - public void setUserTaskCandidateGroupsLimit(int userTaskCandidateGroupsLimit) { - this.userTaskCandidateGroupsLimit = userTaskCandidateGroupsLimit; - } - - public int getUserTaskCandidateUsersLimit() { - return userTaskCandidateUsersLimit; - } - - public void setUserTaskCandidateUsersLimit(int userTaskCandidateUsersLimit) { - this.userTaskCandidateUsersLimit = userTaskCandidateUsersLimit; - } - public BusinessCalendarManager getBusinessCalendarManager() { return businessCalendarManager; } diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java index b82ddd51bb2..3c575831f57 100644 --- a/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/usertask/UserTaskTest.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; -import org.flowable.common.engine.api.FlowableException; import org.flowable.common.engine.api.FlowableIllegalArgumentException; import org.flowable.common.engine.api.scope.ScopeTypes; import org.flowable.common.engine.impl.history.HistoryLevel; @@ -40,7 +39,6 @@ import org.flowable.identitylink.api.IdentityLinkType; import org.flowable.task.api.Task; import org.flowable.task.api.history.HistoricTaskInstance; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import com.fasterxml.jackson.databind.JsonNode; @@ -52,13 +50,6 @@ */ public class UserTaskTest extends PluggableFlowableTestCase { - @AfterEach - protected void tearDown() throws Exception { - //set back to default value - processEngineConfiguration.setUserTaskCandidateUsersLimit(-1); - processEngineConfiguration.setUserTaskCandidateGroupsLimit(-1); - } - @Test @Deployment public void testTaskPropertiesNotNull() { @@ -351,38 +342,6 @@ public void testUserTaskIdVariableName() throws Exception { assertThat(myExpressionTaskId).isEqualTo(actualTaskId); } - @Test - @Deployment(resources="org/flowable/engine/test/bpmn/usertask/UserTaskTest.testNonStringProperties.bpmn20.xml") - public void testUserTaskCandidateUsersLimit() { - - Map vars = new HashMap<>(); - vars.put("taskAssignee", "testAssignee"); - vars.put("taskOwner", "testOwner"); - vars.put("taskCandidateGroups", "group1"); - vars.put("taskCandidateUsers", "user1, user2, user3"); - processEngineConfiguration.setUserTaskCandidateUsersLimit(2); - - assertThatThrownBy(() -> runtimeService.startProcessInstanceByKey("nonStringProperties", vars)) - .isInstanceOf(FlowableException.class) - .hasMessageContaining("The number of users after resolving expression exceeds the limit " + processEngineConfiguration.getUserTaskCandidateUsersLimit() +" of allowed users for a user task."); - } - - @Test - @Deployment(resources="org/flowable/engine/test/bpmn/usertask/UserTaskTest.testNonStringProperties.bpmn20.xml") - public void testUserTaskCandidateGroupsLimit() { - - Map vars = new HashMap<>(); - vars.put("taskAssignee", "testAssignee"); - vars.put("taskOwner", "testOwner"); - vars.put("taskCandidateGroups", "group1, group2, group3"); - vars.put("taskCandidateUsers", "user1"); - processEngineConfiguration.setUserTaskCandidateGroupsLimit(2); - - assertThatThrownBy(() -> runtimeService.startProcessInstanceByKey("nonStringProperties", vars)) - .isInstanceOf(FlowableException.class) - .hasMessageContaining("The number of groups after resolving expression exceeds the limit " + processEngineConfiguration.getUserTaskCandidateGroupsLimit() +" of allowed groups for a user task."); - } - @Test @Deployment(resources="org/flowable/engine/test/bpmn/usertask/UserTaskTest.testNonStringProperties.bpmn20.xml") public void testNormalizeCandidates() {