Skip to content

Commit

Permalink
JBPM-5842: Add more LoadBalanceStrategy test coverage (#851)
Browse files Browse the repository at this point in the history 
Fixes a bug where users in excluded groups wouldn't appear
in excluded user list and ended up still being assigned.
  • Loading branch information
@domhanak
domhanak authored and Tihomir Surdilovic committed May 23, 2017
1 parent 9cc0732 commit e37e20a
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 30 deletions.
17 changes: 16 additions & 1 deletion ...n/java/org/jbpm/services/task/assignment/impl/strategy/LoadBalanceAssignmentStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.jbpm.services.task.assignment.impl.strategy;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -61,8 +62,8 @@ public String getIdentifier() {

@Override
public Assignment apply(Task task, TaskContext taskContext, String excludedUser) {
List<OrganizationalEntity> excluded = ((InternalPeopleAssignments)task.getPeopleAssignments()).getExcludedOwners();
UserInfo userInfo = (UserInfo) ((org.jbpm.services.task.commands.TaskContext)taskContext).get(EnvironmentName.TASK_USER_INFO);
List<OrganizationalEntity> excluded = (getExcludedEntities(task, userInfo));

// Get the the users from the task's the potential owners, making sure that excluded users are not included
List<OrganizationalEntity> potentialOwners = task.getPeopleAssignments().getPotentialOwners().stream()
Expand All @@ -88,4 +89,18 @@ public Assignment apply(Task task, TaskContext taskContext, String excludedUser)
return lightestLoad != null ? new Assignment(lightestLoad.getUser().getId()):null;
}

private static List<OrganizationalEntity> getExcludedEntities(Task task, UserInfo userInfo) {
List<OrganizationalEntity> excluded = ((InternalPeopleAssignments) task.getPeopleAssignments()).getExcludedOwners();

List<OrganizationalEntity> excludedUsers = new ArrayList<>();
for (OrganizationalEntity entity : excluded) {
if (entity instanceof Group) {
userInfo.getMembersForGroup((Group) entity).forEachRemaining(excludedUsers::add);
}
}
excluded.addAll(excludedUsers);

return excluded;
}

}
178 changes: 149 additions & 29 deletions ...re/src/test/java/org/jbpm/services/task/assignment/LoadBalanceAssignmentStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
*/
package org.jbpm.services.task.assignment;

import java.io.StringReader;
import java.util.Collection;
import java.util.Collections;
import java.util.Optional;
import java.util.function.Consumer;

Expand All @@ -23,6 +25,7 @@

import org.jbpm.services.task.HumanTaskServiceFactory;
import org.jbpm.services.task.assignment.impl.strategy.LoadBalanceAssignmentStrategy;
import org.jbpm.services.task.impl.factories.TaskFactory;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -38,9 +41,6 @@
import bitronix.tm.resource.jdbc.PoolingDataSource;

public class LoadBalanceAssignmentStrategyTest extends AbstractAssignmentTest {
private PoolingDataSource pds;
private EntityManagerFactory emf;
private ListMultimap<String, Task> tasks;
private static final Logger logger = LoggerFactory.getLogger(LoadBalanceAssignmentStrategyTest.class);
private static final String BASE_TASK_INFO = "with (new Task()) { priority = 55, taskData = (with (new TaskData()) { } ), ";
private static final String MULTI_ACTOR_ASSIGNMENTS = ""
Expand All @@ -55,12 +55,47 @@ public class LoadBalanceAssignmentStrategyTest extends AbstractAssignmentTest {
private static final String REMOVE_ACTOR_ASSIGNMENTS = ""
+ "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new User('Bobba Fet'), new User('Luke Cage'), new User('Tony Stark')],"
+ " businessAdministrators = [new User('Administrator')], } ),";
private static final String NO_POTENTIAL_OWNER_ASSIGNMENTS = "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [],"
+ " businessAdministrators = [new User('Administrator')], } ),";
private static final String NOT_EXISTING_USER_ASSIGNMENTS = "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new User('Nobody')],"
+ " businessAdministrators = [new User('Administrator')], } ),";
private static final String NO_USERS_IN_GROUP_ASSIGNMENTS = "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new Group('Nobodies')],"
+ " businessAdministrators = [new User('Administrator')], } ),";
private static final String SINGLE_USER_ASSIGNMENTS = "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new User('Darth Vader')],"
+ " businessAdministrators = [new User('Administrator')], } ),";
private static final String SINGLE_USER_IN_GROUP_ASSIGNMENTS = "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new Group('Wrong Crusaders')],"
+ " businessAdministrators = [new User('Administrator')], } ),";
private static final String EXCLUDED_USER_FROM_USERS_ASSIGNMENTS = ""
+ "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new User('Bobba Fet'), new User('Darth Vader'), new User('Luke Cage')],"
+ " excludedOwners = [new User('Darth Vader')], businessAdministrators = [new User('Administrator')], } ),";
private static final String EXCLUDED_USER_FROM_GROUP_ASSIGNMENTS = ""
+ "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new Group('Crusaders')],"
+ " excludedOwners = [new User('Tony Stark')], businessAdministrators = [new User('Administrator')], } ),";
private static final String EXCLUDED_GROUP_FROM_USERS_ASSIGNMENTS = ""
+ "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new User('Bobba Fet'), new User('Darth Vader'), new User('Luke Cage')],"
+ " excludedOwners = [new Group('Wrong Crusaders')], businessAdministrators = [new User('Administrator')], } ),";
private static final String EXCLUDED_GROUP_FROM_GROUP_ASSIGNMENTS = ""
+ "peopleAssignments = (with (new PeopleAssignments()) { potentialOwners = [new Group('Crusaders')],"
+ " excludedOwners = [new Group('Knights Templer')], businessAdministrators = [new User('Administrator')], } ),";
private static final String DARTH_VADER = "Darth Vader";
private static final String BOBBA_FET = "Bobba Fet";
private static final String LUKE_CAGE = "Luke Cage";
private static final String TONY_STARK = "Tony Stark";
private static final String NOBODIES = "Nobodies";
private static final String CRUSADERS = "Crusaders";
private static final String WRONG_CRUSADERS = "Wrong Crusaders";
private PoolingDataSource pds;
private EntityManagerFactory emf;
private ListMultimap<String, Task> tasks;
private Consumer<Task> complete = (task) -> {
completeTask(task);
this.tasks.get(task.getTaskData().getActualOwner().getId()).remove(task);
};


private static String createTaskString(String peopleAssignments, String taskName) {
return "(" + BASE_TASK_INFO + peopleAssignments + "name = '" + taskName + "'})";
}

@Before
public void setUp() throws Exception {
System.setProperty("org.jbpm.task.assignment.enabled", "true");
Expand Down Expand Up @@ -92,11 +127,10 @@ public void clean() throws Exception {
}
}


@Test
public void testMultipleUser() {
final String taskString = "(" +
BASE_TASK_INFO +
BASE_TASK_INFO +
MULTI_ACTOR_ASSIGNMENTS +
"name = 'MultiUserLoadBalanceTask'})";
tasks.put(BOBBA_FET,createForCompletionTask(taskString, BOBBA_FET, 3, BOBBA_FET,DARTH_VADER,LUKE_CAGE));
Expand All @@ -112,21 +146,21 @@ public void testMultipleUser() {
assertNumberOfNonCompletedTasks(DARTH_VADER, 2);
assertNumberOfNonCompletedTasks(BOBBA_FET, 2);
assertNumberOfNonCompletedTasks(LUKE_CAGE, 2);

// Check that if we complete a task and then create a new one
// that it gets assigned to the proper user
getTaskToComplete(DARTH_VADER).ifPresent(complete);
assertNumberOfNonCompletedTasks(DARTH_VADER, 1);
tasks.put(DARTH_VADER,createForCompletionTask(taskString, DARTH_VADER, 3, BOBBA_FET,DARTH_VADER,LUKE_CAGE));
assertNumberOfNonCompletedTasks(DARTH_VADER, 2);

// Make sure that we aren't trying to complete a previously completed task
getTaskToComplete(DARTH_VADER).ifPresent(complete);
assertNumberOfNonCompletedTasks(DARTH_VADER, 1);

logger.info("testMultipleUser completed");
}

@Test
public void testMultipleUserWithGroup() {
final String taskString = "(" +
Expand All @@ -139,32 +173,32 @@ public void testMultipleUserWithGroup() {
assertNumberOfNonCompletedTasks(BOBBA_FET, 1);
assertNumberOfNonCompletedTasks(LUKE_CAGE, 1);
assertNumberOfNonCompletedTasks(TONY_STARK, 1);

tasks.put(BOBBA_FET, createForCompletionTask(taskString, BOBBA_FET, 3, BOBBA_FET,"Crusaders",LUKE_CAGE));
tasks.put(LUKE_CAGE, createForCompletionTask(taskString, LUKE_CAGE, 3, BOBBA_FET,"Crusaders",LUKE_CAGE));
tasks.put(TONY_STARK, createForCompletionTask(taskString, TONY_STARK, 3, BOBBA_FET,"Crusaders",LUKE_CAGE));
assertNumberOfNonCompletedTasks(BOBBA_FET, 2);
assertNumberOfNonCompletedTasks(LUKE_CAGE, 2);
assertNumberOfNonCompletedTasks(TONY_STARK, 2);

getTaskToComplete(LUKE_CAGE).ifPresent(complete);
assertNumberOfNonCompletedTasks(LUKE_CAGE, 1);
tasks.put(LUKE_CAGE, createForCompletionTask(taskString, LUKE_CAGE, 3, BOBBA_FET,"Crusaders",LUKE_CAGE));
assertNumberOfNonCompletedTasks(LUKE_CAGE, 2);
logger.info("testMultipleUserWithGroup completed");
}

@Test
public void testMultipleUserWithAdd() {
final String taskString = "(" +
BASE_TASK_INFO +
BASE_TASK_INFO +
MULTI_ACTOR_ASSIGNMENTS +
"name = 'MultiUserWithAddLoadBalanceTask'})";
final String taskString2 = "(" +
BASE_TASK_INFO +
ADD_ACTOR_ASSIGNMENTS +
ADD_ACTOR_ASSIGNMENTS +
"name = 'MultiUserWithAddLoadBalanceTask2'})";

tasks.put(BOBBA_FET,createForCompletionTask(taskString, BOBBA_FET, 3, BOBBA_FET,DARTH_VADER,LUKE_CAGE));
tasks.put(DARTH_VADER,createForCompletionTask(taskString, DARTH_VADER, 3, BOBBA_FET,DARTH_VADER,LUKE_CAGE));
tasks.put(LUKE_CAGE,createForCompletionTask(taskString, LUKE_CAGE, 3, BOBBA_FET,DARTH_VADER,LUKE_CAGE));
Expand All @@ -178,31 +212,31 @@ public void testMultipleUserWithAdd() {
assertNumberOfNonCompletedTasks(DARTH_VADER, 2);
assertNumberOfNonCompletedTasks(BOBBA_FET, 2);
assertNumberOfNonCompletedTasks(LUKE_CAGE, 2);

// Check that if we complete a task and then create a new one
// that it gets assigned to the proper user
getTaskToComplete(DARTH_VADER).ifPresent(complete);
assertNumberOfNonCompletedTasks(DARTH_VADER, 1);
tasks.put(DARTH_VADER,createForCompletionTask(taskString, DARTH_VADER, 3, BOBBA_FET,DARTH_VADER,LUKE_CAGE));
assertNumberOfNonCompletedTasks(DARTH_VADER, 2);

// Now add a user with no tasks and make sure that
// the assignment goes to the new user
tasks.put(TONY_STARK, createForCompletionTask(taskString2,TONY_STARK,4,BOBBA_FET,DARTH_VADER,LUKE_CAGE,TONY_STARK));
assertNumberOfNonCompletedTasks(TONY_STARK,1);
}

@Test
public void testMultipleUsersWithRemove() {
final String taskString = "(" +
BASE_TASK_INFO +
ADD_ACTOR_ASSIGNMENTS +
ADD_ACTOR_ASSIGNMENTS +
"name = 'MultiUserWithRemoveLoadBalanceTask'})";
final String taskString2 = "("
+ BASE_TASK_INFO
+ REMOVE_ACTOR_ASSIGNMENTS
+ "name = 'MultiUserWithRemoveLoadBalanceTask2'})";

tasks.put(BOBBA_FET,createForCompletionTask(taskString, BOBBA_FET, 4, BOBBA_FET,DARTH_VADER,LUKE_CAGE,TONY_STARK));
tasks.put(DARTH_VADER,createForCompletionTask(taskString, DARTH_VADER, 4, BOBBA_FET,DARTH_VADER,LUKE_CAGE,TONY_STARK));
tasks.put(LUKE_CAGE,createForCompletionTask(taskString, LUKE_CAGE, 4, BOBBA_FET,DARTH_VADER,LUKE_CAGE,TONY_STARK));
Expand All @@ -219,16 +253,102 @@ public void testMultipleUsersWithRemove() {
assertNumberOfNonCompletedTasks(BOBBA_FET, 2);
assertNumberOfNonCompletedTasks(LUKE_CAGE, 2);
assertNumberOfNonCompletedTasks(TONY_STARK, 2);

}

@Test
public void testNotExistingUser() {
final String taskString = createTaskString(NOT_EXISTING_USER_ASSIGNMENTS, "NotExistingUserLoadBalancingTask");

Task task = TaskFactory.evalTask(new StringReader(taskString));
assertPotentialOwners(task, 1, "Nobody");

taskService.addTask(task, Collections.emptyMap());
assertPotentialOwners(task, 0);
assertNoActualOwner(task);
}

@Test
public void testNoUsersInGroup() {
final String taskString = createTaskString(NO_USERS_IN_GROUP_ASSIGNMENTS, "NoUsersInGroupLoadBalancingTask");

Task task = TaskFactory.evalTask(new StringReader(taskString));
assertPotentialOwners(task, 1, NOBODIES);

taskService.addTask(task, Collections.emptyMap());
assertPotentialOwners(task, 0);
assertNoActualOwner(task);
}

@Test
public void testSingleUser() {
final String taskString = createTaskString(SINGLE_USER_ASSIGNMENTS, "SingleUserLoadBalancingTask");

createAndAssertTask(taskString, DARTH_VADER, 1, DARTH_VADER);
createAndAssertTask(taskString, DARTH_VADER, 1, DARTH_VADER);
createAndAssertTask(taskString, DARTH_VADER, 1, DARTH_VADER);
}

@Test
public void testSingleUserInGroup() {
final String taskString = createTaskString(SINGLE_USER_IN_GROUP_ASSIGNMENTS, "SingleUserInGroupLoadBalancingTask");

createAndAssertTask(taskString, DARTH_VADER, 1, WRONG_CRUSADERS);
createAndAssertTask(taskString, DARTH_VADER, 1, WRONG_CRUSADERS);
createAndAssertTask(taskString, DARTH_VADER, 1, WRONG_CRUSADERS);
}

@Test
public void testReleaseTask() {
final String taskString = createTaskString(MULTI_ACTOR_ASSIGNMENTS, "MultiActorLoadBalancingTask");

long bobbasTaskId = createAndAssertTask(taskString, BOBBA_FET, 3, BOBBA_FET, DARTH_VADER, LUKE_CAGE);
long vaderTaskId = createAndAssertTask(taskString, DARTH_VADER, 3, BOBBA_FET);

taskService.release(bobbasTaskId, BOBBA_FET);
Task bobbasTask = taskService.getTaskById(bobbasTaskId);
Task vaderTask = taskService.getTaskById(vaderTaskId);

assertActualOwner(bobbasTask, LUKE_CAGE);
assertActualOwner(vaderTask, DARTH_VADER);
}

@Test
public void testExcludedUserFromUsers() {
final String taskString = createTaskString(EXCLUDED_USER_FROM_USERS_ASSIGNMENTS, "ExcludedUsersFromUsersLoadBalancingTask");

createAndAssertTask(taskString, BOBBA_FET, 3, BOBBA_FET, DARTH_VADER, LUKE_CAGE);
createAndAssertTask(taskString, LUKE_CAGE, 3, BOBBA_FET, DARTH_VADER, LUKE_CAGE);
createAndAssertTask(taskString, BOBBA_FET, 3, BOBBA_FET, DARTH_VADER, LUKE_CAGE);
}

@Test
public void testExcludedUserFromGroup() {
final String taskString = createTaskString(EXCLUDED_USER_FROM_GROUP_ASSIGNMENTS, "ExcludedUsersFromGroupLoadBalancingTask");

createAndAssertTask(taskString, LUKE_CAGE, 1, CRUSADERS);
createAndAssertTask(taskString, BOBBA_FET, 1, CRUSADERS);
createAndAssertTask(taskString, LUKE_CAGE, 1, CRUSADERS);
}

private Consumer<Task> complete = (task) -> {
completeTask(task);
this.tasks.get(task.getTaskData().getActualOwner().getId()).remove(task);
};



@Test
public void testExcludedGroupFromUsers() {
final String taskString = createTaskString(EXCLUDED_GROUP_FROM_USERS_ASSIGNMENTS, "ExcludedGroupFromUsersLoadBalancingTask");

createAndAssertTask(taskString, BOBBA_FET, 3, BOBBA_FET, DARTH_VADER, LUKE_CAGE);
createAndAssertTask(taskString, LUKE_CAGE, 3, BOBBA_FET, DARTH_VADER, LUKE_CAGE);
createAndAssertTask(taskString, BOBBA_FET, 3, BOBBA_FET, DARTH_VADER, LUKE_CAGE);
}

@Test
public void testExcludedGroupFromGroup() {
final String taskString = createTaskString(EXCLUDED_GROUP_FROM_GROUP_ASSIGNMENTS, "ExcludedGroupFromGroupLoadBalancingTask");

createAndAssertTask(taskString, LUKE_CAGE, 1, CRUSADERS);
createAndAssertTask(taskString, TONY_STARK, 1, CRUSADERS);
createAndAssertTask(taskString, LUKE_CAGE, 1, CRUSADERS);
}

private Optional<Task> getTaskToComplete(String user) {
Optional<Task> task = Optional.empty();
if (tasks.containsKey(user)) {
Expand Down

0 comments on commit e37e20a

Please sign in to comment.