From 035e8ab31fd0562a99f51cc0280d7e8b3a020415 Mon Sep 17 00:00:00 2001 From: Martin Cimbalek Date: Thu, 13 Apr 2023 15:33:42 +0200 Subject: [PATCH] RHPAM-4442: before and after TaskAssignmentsAddedEvent show same user details in both events --- .../task/events/TaskEventSupport.java | 10 +- .../commands/AddPeopleAssignmentsCommand.java | 17 +- .../TaskAssignmentAddedListenersTest.java | 290 ++++++++++++++++++ .../functional/task/HumanTask-simple.bpmn2 | 149 +++++++++ 4 files changed, 458 insertions(+), 8 deletions(-) create mode 100644 jbpm-test-coverage/src/test/java/org/jbpm/test/functional/task/TaskAssignmentAddedListenersTest.java create mode 100644 jbpm-test-coverage/src/test/resources/org/jbpm/test/functional/task/HumanTask-simple.bpmn2 diff --git a/jbpm-human-task/jbpm-human-task-core/src/main/java/org/jbpm/services/task/events/TaskEventSupport.java b/jbpm-human-task/jbpm-human-task-core/src/main/java/org/jbpm/services/task/events/TaskEventSupport.java index 41153d461f..1e64a0d6dd 100644 --- a/jbpm-human-task/jbpm-human-task-core/src/main/java/org/jbpm/services/task/events/TaskEventSupport.java +++ b/jbpm-human-task/jbpm-human-task-core/src/main/java/org/jbpm/services/task/events/TaskEventSupport.java @@ -169,11 +169,11 @@ public void fireBeforeTaskOutputVariablesChanged(final Task task, TaskContext co notifyAllListeners( event, ( l, e ) -> l.beforeTaskOutputVariableChangedEvent( e, variables ) ); } } - - public void fireBeforeTaskAssignmentsAddedEvent(final Task task, TaskContext context, AssignmentType type, List entities) { + + public void fireBeforeTaskAssignmentsAddedEvent(final Task task, TaskContext context, AssignmentType type, List entities, List beforeChangeEntities) { if ( hasListeners() ) { final TaskEventImpl event = new TaskEventImpl(task, context); - notifyAllListeners( event, ( l, e ) -> l.beforeTaskAssignmentsAddedEvent(e, type, entities) ); + notifyAllListeners( event, ( l, e ) -> l.beforeTaskAssignmentsAddedEvent(e, type, entities, beforeChangeEntities) ); } } @@ -329,10 +329,10 @@ public void fireAfterTaskOutputVariablesChanged(final Task task, TaskContext con } } - public void fireAfterTaskAssignmentsAddedEvent(final Task task, TaskContext context, AssignmentType type, List entities) { + public void fireAfterTaskAssignmentsAddedEvent(final Task task, TaskContext context, AssignmentType type, List entities, List afterChangeEntities) { if ( hasListeners() ) { final TaskEventImpl event = new TaskEventImpl(task, context); - notifyAllListeners( event, ( l, e ) -> l.afterTaskAssignmentsAddedEvent(e, type, entities) ); + notifyAllListeners( event, ( l, e ) -> l.afterTaskAssignmentsAddedEvent(e, type, entities, afterChangeEntities) ); } } diff --git a/jbpm-services/jbpm-kie-services/src/main/java/org/jbpm/kie/services/impl/admin/commands/AddPeopleAssignmentsCommand.java b/jbpm-services/jbpm-kie-services/src/main/java/org/jbpm/kie/services/impl/admin/commands/AddPeopleAssignmentsCommand.java index 6974ba9690..11949522d4 100644 --- a/jbpm-services/jbpm-kie-services/src/main/java/org/jbpm/kie/services/impl/admin/commands/AddPeopleAssignmentsCommand.java +++ b/jbpm-services/jbpm-kie-services/src/main/java/org/jbpm/kie/services/impl/admin/commands/AddPeopleAssignmentsCommand.java @@ -16,6 +16,7 @@ package org.jbpm.kie.services.impl.admin.commands; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -63,10 +64,12 @@ public Void execute(Context cntxt) { throw new PermissionDeniedException("User " + userId + " is not business admin of task " + taskId); } + List beforeChangeEntityList = new ArrayList<>(); List entityList = Arrays.asList(entities); AssignmentType assignmentType = null; switch (type) { case POT_OWNER: + beforeChangeEntityList.addAll(task.getPeopleAssignments().getPotentialOwners()); if (removeExisting) { task.getPeopleAssignments().getPotentialOwners().clear(); } @@ -74,6 +77,7 @@ public Void execute(Context cntxt) { assignmentType = AssignmentType.POT_OWNER; break; case EXCL_OWNER: + beforeChangeEntityList.addAll(((InternalPeopleAssignments)task.getPeopleAssignments()).getExcludedOwners()); if (removeExisting) { ((InternalPeopleAssignments)task.getPeopleAssignments()).getExcludedOwners().clear(); } @@ -81,6 +85,7 @@ public Void execute(Context cntxt) { assignmentType = AssignmentType.EXCL_OWNER; break; case ADMIN: + beforeChangeEntityList.addAll(task.getPeopleAssignments().getBusinessAdministrators()); if (removeExisting) { task.getPeopleAssignments().getBusinessAdministrators().clear(); } @@ -91,10 +96,16 @@ public Void execute(Context cntxt) { default: break; } - taskEventSupport.fireBeforeTaskAssignmentsAddedEvent(task, context, assignmentType, entityList); - doCallbackOperationForPeopleAssignments(((InternalPeopleAssignments)task.getPeopleAssignments()), context); + + taskEventSupport.fireBeforeTaskAssignmentsAddedEvent(task, context, assignmentType, entityList, beforeChangeEntityList); + doCallbackOperationForPeopleAssignments(((InternalPeopleAssignments) task.getPeopleAssignments()), context); context.getPersistenceContext().updateTask(task); - taskEventSupport.fireAfterTaskAssignmentsAddedEvent(task, context, assignmentType, entityList); + + List afterChangeEntityList = new ArrayList<>(entityList); + if (!removeExisting) { + afterChangeEntityList.addAll(beforeChangeEntityList); + } + taskEventSupport.fireAfterTaskAssignmentsAddedEvent(task, context, assignmentType, entityList, afterChangeEntityList); return null; } diff --git a/jbpm-test-coverage/src/test/java/org/jbpm/test/functional/task/TaskAssignmentAddedListenersTest.java b/jbpm-test-coverage/src/test/java/org/jbpm/test/functional/task/TaskAssignmentAddedListenersTest.java new file mode 100644 index 0000000000..d07117e791 --- /dev/null +++ b/jbpm-test-coverage/src/test/java/org/jbpm/test/functional/task/TaskAssignmentAddedListenersTest.java @@ -0,0 +1,290 @@ +/* + * Copyright 2023 Red Hat, Inc. and/or its affiliates. + * + * 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. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jbpm.test.functional.task; + +import org.apache.commons.lang3.mutable.MutableInt; +import org.jbpm.kie.services.impl.admin.commands.AddPeopleAssignmentsCommand; +import org.jbpm.services.task.events.DefaultTaskEventListener; +import org.jbpm.services.task.impl.model.UserImpl; +import org.jbpm.test.JbpmTestCase; +import org.junit.After; +import org.junit.Test; +import org.kie.api.runtime.KieSession; +import org.kie.api.runtime.manager.RuntimeEngine; +import org.kie.api.runtime.process.ProcessInstance; +import org.kie.api.task.TaskEvent; +import org.kie.api.task.TaskLifeCycleEventListener; +import org.kie.api.task.TaskService; +import org.kie.api.task.model.OrganizationalEntity; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Test listeners tied to adding assignments - RHPAM-4442 + */ +public class TaskAssignmentAddedListenersTest extends JbpmTestCase { + + private KieSession ksession; + private TaskService ts; + + + List beforePotentialOwners; + List beforeEntitiesToSet; + List afterPotentialOwners; + List afterEntitiesToSet; + + + MutableInt triggeredBeforeListenerCounter; + MutableInt triggeredAfterListenerCounter; + + + + + private static final String PROCESS = "org/jbpm/test/functional/task/HumanTask-simple.bpmn2"; + private static final String PROCESS_ID = "org.jbpm.test.functional.task.HumanTask-simple"; + + + private void init() { + createRuntimeManager(PROCESS); + RuntimeEngine runtimeEngine = getRuntimeEngine(); + ksession = runtimeEngine.getKieSession(); + ts = runtimeEngine.getTaskService(); + } + + @After + public void clenaup() { + if (ksession != null) { + ksession.dispose(); + } + disposeRuntimeManager(); + } + + @Test + public void testAddedAssignmentListenersThreeParamApiRegression() { + DefaultTaskEventListener listener = new DefaultTaskEventListener() { + @Override + public void beforeTaskAssignmentsAddedEvent(TaskEvent event, AssignmentType type, List entities) { + triggeredBeforeListenerCounter.increment(); + beforePotentialOwners.addAll(event.getTask().getPeopleAssignments().getPotentialOwners()); + beforeEntitiesToSet.addAll(entities); + logger.debug("before potentialOwners: " + beforePotentialOwners); + logger.debug("before entities: " + beforeEntitiesToSet); + } + + @Override + public void afterTaskAssignmentsAddedEvent(TaskEvent event, AssignmentType type, List entities) { + triggeredAfterListenerCounter.increment(); + afterPotentialOwners.addAll(event.getTask().getPeopleAssignments().getPotentialOwners()); + afterEntitiesToSet.addAll(entities); + + logger.debug("after potentialOwners: " + afterPotentialOwners); + logger.debug("after entities: " + afterEntitiesToSet); + } + + }; + testRegression(listener); + } + + @Test + public void testAddedAssignmentListenersFourParamApiRegression() { + + TaskLifeCycleEventListener listener = new DefaultTaskEventListener() { + @Override + public void beforeTaskAssignmentsAddedEvent(TaskEvent event, AssignmentType type, List entities, List beforeChangeEntities) { + triggeredBeforeListenerCounter.increment(); + beforePotentialOwners.addAll(event.getTask().getPeopleAssignments().getPotentialOwners()); + beforeEntitiesToSet.addAll(entities); + logger.debug("before potentialOwners: " + beforePotentialOwners); + logger.debug("before entities: " + beforeEntitiesToSet); + } + + @Override + public void afterTaskAssignmentsAddedEvent(TaskEvent event, AssignmentType type, List entities, List afterChangeEntities) { + triggeredAfterListenerCounter.increment(); + afterPotentialOwners.addAll(event.getTask().getPeopleAssignments().getPotentialOwners()); + afterEntitiesToSet.addAll(entities); + + logger.debug("after potentialOwners: " + afterPotentialOwners); + logger.debug("after entities: " + afterEntitiesToSet); + } + }; + + testRegression(listener); + } + + @Test + public void testAddedAssignmentListenersFourParamApi() { + triggeredBeforeListenerCounter = new MutableInt(0); + triggeredAfterListenerCounter = new MutableInt(0); + + List beforeChangeEntitiesResult = new ArrayList<>(); + List afterChangeEntitiesResult = new ArrayList<>(); + + TaskLifeCycleEventListener listener = new DefaultTaskEventListener() { + @Override + public void beforeTaskAssignmentsAddedEvent(TaskEvent event, AssignmentType type, List entities, List beforeChangeEntities) { + triggeredBeforeListenerCounter.increment(); + beforeChangeEntitiesResult.addAll(beforeChangeEntities); + logger.debug("beforeChangeEntities: " + beforeChangeEntities); + } + + @Override + public void afterTaskAssignmentsAddedEvent(TaskEvent event, AssignmentType type, List entities, List afterChangeEntities) { + triggeredAfterListenerCounter.increment(); + afterChangeEntitiesResult.addAll(afterChangeEntities); + logger.debug("afterChangeEntities: " + afterChangeEntities); + + } + }; + + addTaskEventListener(listener); + + init(); + + ProcessInstance pi = ksession.startProcess(PROCESS_ID); + + long pid = pi.getId(); + + assertProcessInstanceActive(pi.getId(), ksession); + assertNodeTriggered(pi.getId(), "Start", "Task"); + + long taskId = ts.getTasksByProcessInstanceId(pid).get(0); + + UserImpl john = new UserImpl("john"); + UserImpl mary = new UserImpl("mary"); + UserImpl jim = new UserImpl("jim"); + + //add assignment + ts.execute(new AddPeopleAssignmentsCommand("Administrator", taskId, 0, new UserImpl[]{john}, false)); + assertThat(beforeChangeEntitiesResult).isEmpty(); + + assertThat(afterChangeEntitiesResult).hasSize(1); + assertThat(afterChangeEntitiesResult).contains(john); + + clearLists(beforeChangeEntitiesResult, afterChangeEntitiesResult); + + // Add one more without removing existing + ts.execute(new AddPeopleAssignmentsCommand("Administrator", taskId, 0, new UserImpl[]{mary}, false)); + assertThat(beforeChangeEntitiesResult).hasSize(1); + assertThat(beforeChangeEntitiesResult).contains(john).doesNotContain(mary); + + assertThat(afterChangeEntitiesResult).hasSize(2); + assertThat(afterChangeEntitiesResult).contains(john, mary); + + clearLists(beforeChangeEntitiesResult, afterChangeEntitiesResult); + + // Add one more but with removing existing ones + ts.execute(new AddPeopleAssignmentsCommand("Administrator", taskId, 0, new UserImpl[]{jim}, true)); + assertThat(beforeChangeEntitiesResult).hasSize(2); + assertThat(beforeChangeEntitiesResult).contains(john, mary).doesNotContain(jim); + + assertThat(afterChangeEntitiesResult).hasSize(1); + assertThat(afterChangeEntitiesResult).contains(jim).doesNotContain(john, mary); + + clearLists(beforeChangeEntitiesResult, afterChangeEntitiesResult); + + assertThat(triggeredBeforeListenerCounter.getValue()).isEqualTo(3); + assertThat(triggeredAfterListenerCounter.getValue()).isEqualTo(3); + } + + private void testRegression(TaskLifeCycleEventListener listener) { + triggeredBeforeListenerCounter = new MutableInt(0); + triggeredAfterListenerCounter = new MutableInt(0); + + beforePotentialOwners = new ArrayList<>(); + beforeEntitiesToSet = new ArrayList<>(); + afterPotentialOwners = new ArrayList<>(); + afterEntitiesToSet = new ArrayList<>(); + + addTaskEventListener(listener); + + init(); + + ProcessInstance pi = ksession.startProcess(PROCESS_ID); + long pid = pi.getId(); + + assertProcessInstanceActive(pi.getId(), ksession); + assertNodeTriggered(pi.getId(), "Start", "Task"); + + long taskId = ts.getTasksByProcessInstanceId(pid).get(0); + + UserImpl john = new UserImpl("john"); + UserImpl mary = new UserImpl("mary"); + UserImpl jim = new UserImpl("jim"); + + //add assignment + ts.execute(new AddPeopleAssignmentsCommand("Administrator", taskId, 0, new UserImpl[]{john}, false)); + assertThat(beforePotentialOwners).hasSize(1); + assertThat(beforePotentialOwners).contains(john); + + assertThat(beforeEntitiesToSet).hasSize(1); + assertThat(beforeEntitiesToSet).contains(john); + + assertThat(afterPotentialOwners).hasSize(1); + assertThat(afterPotentialOwners).contains(john); + + assertThat(afterEntitiesToSet).hasSize(1); + assertThat(afterEntitiesToSet).contains(john); + + clearLists(beforePotentialOwners, beforeEntitiesToSet, afterPotentialOwners, afterEntitiesToSet); + + // Add one more without removing existing + ts.execute(new AddPeopleAssignmentsCommand("Administrator", taskId, 0, new UserImpl[]{mary}, false)); + assertThat(beforePotentialOwners).hasSize(2); + assertThat(beforePotentialOwners).contains(john, mary); + + assertThat(beforeEntitiesToSet).hasSize(1); + assertThat(beforeEntitiesToSet).contains(mary); + + assertThat(afterPotentialOwners).hasSize(2); + assertThat(afterPotentialOwners).contains(john, mary); + + assertThat(afterEntitiesToSet).hasSize(1); + assertThat(afterEntitiesToSet).contains(mary); + + clearLists(beforePotentialOwners, beforeEntitiesToSet, afterPotentialOwners, afterEntitiesToSet); + + // Add one more but with removing existing ones + ts.execute(new AddPeopleAssignmentsCommand("Administrator", taskId, 0, new UserImpl[]{jim}, true)); + assertThat(beforePotentialOwners).hasSize(1); + assertThat(beforePotentialOwners).contains(jim).doesNotContain(john, mary); + + assertThat(beforeEntitiesToSet).hasSize(1); + assertThat(beforeEntitiesToSet).contains(jim).doesNotContain(john, mary); + + assertThat(afterPotentialOwners).isEmpty(); + + assertThat(afterEntitiesToSet).hasSize(1); + assertThat(afterEntitiesToSet).contains(jim).doesNotContain(john, mary); + + clearLists(beforePotentialOwners, beforeEntitiesToSet, afterPotentialOwners, afterEntitiesToSet); + + assertThat(triggeredBeforeListenerCounter.getValue()).isEqualTo(3); + assertThat(triggeredAfterListenerCounter.getValue()).isEqualTo(3); + } + + @SafeVarargs + private final void clearLists(List... listsToClear) { + for (List l : listsToClear) { + l.clear(); + assertThat(l).isEmpty(); + } + } +} diff --git a/jbpm-test-coverage/src/test/resources/org/jbpm/test/functional/task/HumanTask-simple.bpmn2 b/jbpm-test-coverage/src/test/resources/org/jbpm/test/functional/task/HumanTask-simple.bpmn2 new file mode 100644 index 0000000000..5105b52a17 --- /dev/null +++ b/jbpm-test-coverage/src/test/resources/org/jbpm/test/functional/task/HumanTask-simple.bpmn2 @@ -0,0 +1,149 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + _99027D7C-CB12-4BD7-AB46-E0BD6651AC33 + + + + + + + + _99027D7C-CB12-4BD7-AB46-E0BD6651AC33 + _99C505A7-2B8C-4A30-A845-A87BC62986B6 + + + + + + _9E72D077-F2EE-4F00-A987-6A88DD09F5EA_TaskNameInputX + _9E72D077-F2EE-4F00-A987-6A88DD09F5EA_GroupIdInputX + _9E72D077-F2EE-4F00-A987-6A88DD09F5EA_SkippableInputX + + + + + _9E72D077-F2EE-4F00-A987-6A88DD09F5EA_TaskNameInputX + + + + + + + _9E72D077-F2EE-4F00-A987-6A88DD09F5EA_SkippableInputX + + + + + + + + + + + + + _99C505A7-2B8C-4A30-A845-A87BC62986B6 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + _qawoYLrzEDuh_a8vC56DnQ + _qawoYLrzEDuh_a8vC56DnQ + + \ No newline at end of file