TSK-417 page number and size mismatch when hidden tasks are member of query resultset

This commit is contained in:
BerndBreier 2018-04-05 13:08:25 +02:00 committed by Holger Hagen
parent 1d58cb39e1
commit b62563f463
5 changed files with 81 additions and 70 deletions

View File

@ -1,29 +0,0 @@
package pro.taskana.impl;
/**
* This class keeps a count of tasks that are contained in a workbasket.
*
* @author bbr
*/
public class TaskCountPerWorkbasket {
Integer taskCount;
String workbasketId;
public Integer getTaskCount() {
return taskCount;
}
public void setTaskCount(Integer taskCount) {
this.taskCount = taskCount;
}
public String getWorkbasketId() {
return workbasketId;
}
public void setWorkbasketId(String workbasketId) {
this.workbasketId = workbasketId;
}
}

View File

@ -3,8 +3,6 @@ package pro.taskana.impl;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.ibatis.exceptions.PersistenceException;
import org.apache.ibatis.session.RowBounds;
@ -20,13 +18,13 @@ import pro.taskana.TaskanaEngine;
import pro.taskana.TaskanaRole;
import pro.taskana.TimeInterval;
import pro.taskana.WorkbasketPermission;
import pro.taskana.WorkbasketSummary;
import pro.taskana.exceptions.InvalidArgumentException;
import pro.taskana.exceptions.NotAuthorizedException;
import pro.taskana.exceptions.NotAuthorizedToQueryWorkbasketException;
import pro.taskana.exceptions.TaskanaRuntimeException;
import pro.taskana.exceptions.WorkbasketNotFoundException;
import pro.taskana.impl.util.LoggerUtils;
import pro.taskana.security.CurrentUserContext;
/**
* TaskQuery for generating dynamic sql.
@ -34,7 +32,7 @@ import pro.taskana.impl.util.LoggerUtils;
public class TaskQueryImpl implements TaskQuery {
private static final String LINK_TO_MAPPER = "pro.taskana.mappings.QueryMapper.queryTaskSummaries";
private static final String LINK_TO_TASK_COUNT_PER_WORKBASKET = "pro.taskana.mappings.QueryMapper.queryTaskCountPerWorkbasket";
private static final String LINK_TO_COUNTER = "pro.taskana.mappings.QueryMapper.countQueryTasks";
private static final String LINK_TO_VALUEMAPPER = "pro.taskana.mappings.QueryMapper.queryTaskColumnValues";
private static final String TIME_INTERVAL = "TimeInterval ";
private static final String IS_INVALID = " is invalid.";
@ -108,6 +106,7 @@ public class TaskQueryImpl implements TaskQuery {
private String[] custom15Like;
private String[] custom16In;
private String[] custom16Like;
private String[] accessIdIn;
private TimeInterval[] createdIn;
private TimeInterval[] claimedIn;
private TimeInterval[] completedIn;
@ -704,6 +703,7 @@ public class TaskQueryImpl implements TaskQuery {
taskanaEngine.openConnection();
checkOpenPermissionForSpecifiedWorkbaskets();
List<TaskSummaryImpl> tasks = new ArrayList<>();
setupAccessIds();
tasks = taskanaEngine.getSqlSession().selectList(LINK_TO_MAPPER, this);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("mapper returned {} resulting Objects: {} ", tasks.size(),
@ -723,6 +723,22 @@ public class TaskQueryImpl implements TaskQuery {
}
}
private void setupAccessIds() {
if (taskanaEngine.isUserInRole(TaskanaRole.ADMIN)) {
this.accessIdIn = null;
} else if (this.accessIdIn == null) {
String[] accessIds = new String[0];
List<String> ucAccessIds = CurrentUserContext.getAccessIds();
if (ucAccessIds != null && !ucAccessIds.isEmpty()) {
accessIds = new String[ucAccessIds.size()];
accessIds = ucAccessIds.toArray(accessIds);
}
this.accessIdIn = accessIds;
WorkbasketQueryImpl.lowercaseAccessIds(this.accessIdIn);
}
}
@Override
public List<String> listValues(String columnName, SortDirection sortDirection) {
LOGGER.debug("Entry to listValues(dbColumnName={}) this = {}", columnName, this);
@ -752,6 +768,7 @@ public class TaskQueryImpl implements TaskQuery {
taskanaEngine.openConnection();
checkOpenPermissionForSpecifiedWorkbaskets();
RowBounds rowBounds = new RowBounds(offset, limit);
setupAccessIds();
List<TaskSummaryImpl> tasks = taskanaEngine.getSqlSession().selectList(LINK_TO_MAPPER, this, rowBounds);
result = taskService.augmentTaskSummariesByContainedSummaries(tasks);
return result;
@ -782,6 +799,7 @@ public class TaskQueryImpl implements TaskQuery {
try {
taskanaEngine.openConnection();
checkOpenPermissionForSpecifiedWorkbaskets();
setupAccessIds();
TaskSummaryImpl taskSummaryImpl = taskanaEngine.getSqlSession().selectOne(LINK_TO_MAPPER, this);
if (taskSummaryImpl == null) {
return null;
@ -807,36 +825,15 @@ public class TaskQueryImpl implements TaskQuery {
try {
taskanaEngine.openConnection();
checkOpenPermissionForSpecifiedWorkbaskets();
List<TaskCountPerWorkbasket> taskCounts = taskanaEngine.getSqlSession()
.selectList(LINK_TO_TASK_COUNT_PER_WORKBASKET, this);
return countTasksInAuthorizedWorkbaskets(taskCounts);
setupAccessIds();
rowCount = taskanaEngine.getSqlSession().selectOne(LINK_TO_COUNTER, this);
return (rowCount == null) ? 0L : rowCount;
} finally {
taskanaEngine.returnConnection();
LOGGER.debug("exit from count(). Returning result {} ", rowCount);
}
}
private long countTasksInAuthorizedWorkbaskets(List<TaskCountPerWorkbasket> taskCounts) {
Set<String> retrievedWorkbasketIds = taskCounts.stream().map(TaskCountPerWorkbasket::getWorkbasketId).collect(
Collectors.toSet());
// use WorkbasketQuery to filter for authorized WorkBaskets
WorkbasketQueryImpl query = (WorkbasketQueryImpl) taskanaEngine.getWorkbasketService().createWorkbasketQuery();
query.setUsedToAugmentTasks(true);
List<WorkbasketSummary> allowedWorkbaskets = query
.idIn(retrievedWorkbasketIds.toArray(new String[0]))
.list();
Set<String> authorizedWorkbasketIds = allowedWorkbaskets.stream().map(WorkbasketSummary::getId).collect(
Collectors.toSet());
long count = 0;
for (TaskCountPerWorkbasket taskCount : taskCounts) {
if (authorizedWorkbasketIds.contains(taskCount.getWorkbasketId())) {
count += taskCount.getTaskCount().intValue();
}
}
return count;
}
private void checkOpenPermissionForSpecifiedWorkbaskets() {
if (taskanaEngine.isUserInRole(TaskanaRole.ADMIN)) {
LOGGER.debug("Skipping permissions check since user is in role ADMIN.");

View File

@ -10,7 +10,6 @@ import pro.taskana.impl.ClassificationQueryImpl;
import pro.taskana.impl.ClassificationSummaryImpl;
import pro.taskana.impl.ObjectReference;
import pro.taskana.impl.ObjectReferenceQueryImpl;
import pro.taskana.impl.TaskCountPerWorkbasket;
import pro.taskana.impl.TaskQueryImpl;
import pro.taskana.impl.TaskSummaryImpl;
import pro.taskana.impl.WorkbasketAccessItemImpl;
@ -31,6 +30,12 @@ public interface QueryMapper {
@Select("<script>SELECT t.ID, t.CREATED, t.CLAIMED, t.COMPLETED, t.MODIFIED, t.PLANNED, t.DUE, t.NAME, t.CREATOR, t.DESCRIPTION, t.NOTE, t.PRIORITY, t.STATE, t.CLASSIFICATION_KEY, t.CLASSIFICATION_CATEGORY, t.CLASSIFICATION_ID, t.WORKBASKET_ID, t.DOMAIN, t.WORKBASKET_KEY, t.BUSINESS_PROCESS_ID, t.PARENT_BUSINESS_PROCESS_ID, t.OWNER, t.POR_COMPANY, t.POR_SYSTEM, t.POR_INSTANCE, t.POR_TYPE, t.POR_VALUE, t.IS_READ, t.IS_TRANSFERRED, t.CUSTOM_1, t.CUSTOM_2, t.CUSTOM_3, t.CUSTOM_4, t.CUSTOM_5, t.CUSTOM_6, t.CUSTOM_7, t.CUSTOM_8, t.CUSTOM_9, t.CUSTOM_10 "
+ "FROM TASKANA.TASK t "
+ "<where>"
+ "<if test='accessIdIn != null'> "
+ "AND t.WORKBASKET_ID IN ( "
+ "select WID from (select WORKBASKET_ID as WID, max(PERM_READ) as MAX_READ FROM TASKANA.WORKBASKET_ACCESS_LIST where "
+ "ACCESS_ID IN (<foreach item='item' collection='accessIdIn' separator=',' >#{item}</foreach>) "
+ "group by WORKBASKET_ID ) where max_read = 1 ) "
+ "</if> "
+ "<if test='taskIds != null'>AND t.ID IN(<foreach item='item' collection='taskIds' separator=',' >#{item}</foreach>)</if> "
+ "<if test='createdIn !=null'> AND ( <foreach item='item' collection='createdIn' separator=' OR ' > ( <if test='item.begin!=null'> t.CREATED &gt;= #{item.begin} </if> <if test='item.begin!=null and item.end!=null'> AND </if><if test='item.end!=null'> t.CREATED &lt;=#{item.end} </if>)</foreach>)</if> "
+ "<if test='claimedIn !=null'> AND ( <foreach item='item' collection='claimedIn' separator=' OR ' > ( <if test='item.begin!=null'> t.CLAIMED &gt;= #{item.begin} </if> <if test='item.begin!=null and item.end!=null'> AND </if><if test='item.end!=null'> t.CLAIMED &lt;=#{item.end} </if>)</foreach>)</if> "
@ -348,8 +353,14 @@ public interface QueryMapper {
@Result(property = "permCustom12", column = "PERM_CUSTOM_12")})
List<WorkbasketAccessItemImpl> queryWorkbasketAccessItems(WorkbasketAccessItemQueryImpl accessItemQuery);
@Select("<script> SELECT COUNT(t.ID) as TASK_COUNT , t.WORKBASKET_ID FROM TASKANA.TASK t "
@Select("<script>SELECT COUNT(ID) FROM TASKANA.TASK t "
+ "<where>"
+ "<if test='accessIdIn != null'> "
+ "AND t.WORKBASKET_ID IN ( "
+ "select WID from (select WORKBASKET_ID as WID, max(PERM_READ) as MAX_READ FROM TASKANA.WORKBASKET_ACCESS_LIST where "
+ "ACCESS_ID IN (<foreach item='item' collection='accessIdIn' separator=',' >#{item}</foreach>) "
+ "group by WORKBASKET_ID ) where max_read = 1 ) "
+ "</if> "
+ "<if test='taskIds != null'>AND t.ID IN(<foreach item='item' collection='taskIds' separator=',' >#{item}</foreach>)</if> "
+ "<if test='createdIn !=null'> AND ( <foreach item='item' collection='createdIn' separator=' OR ' > ( <if test='item.begin!=null'> t.CREATED &gt;= #{item.begin} </if> <if test='item.begin!=null and item.end!=null'> AND </if><if test='item.end!=null'> t.CREATED &lt;=#{item.end} </if>)</foreach>)</if> "
+ "<if test='claimedIn !=null'> AND ( <foreach item='item' collection='claimedIn' separator=' OR ' > ( <if test='item.begin!=null'> t.CLAIMED &gt;= #{item.begin} </if> <if test='item.begin!=null and item.end!=null'> AND </if><if test='item.end!=null'> t.CLAIMED &lt;=#{item.end} </if>)</foreach>)</if> "
@ -423,13 +434,10 @@ public interface QueryMapper {
+ "<if test='custom16In != null'>AND t.CUSTOM_16 IN(<foreach item='item' collection='custom16In' separator=',' >#{item}</foreach>)</if> "
+ "<if test='custom16Like != null'>AND (<foreach item='item' collection='custom16Like' separator=' OR '>UPPER(t.CUSTOM_16) LIKE #{item}</foreach>)</if> "
+ "</where>"
+ "GROUP BY WORKBASKET_ID "
+ "<if test='!orderBy.isEmpty()'>ORDER BY <foreach item='item' collection='orderBy' separator=',' >${item}</foreach></if> "
+ "<if test=\"_databaseId == 'db2'\">with UR </if> "
+ "</script>")
@Results(value = {
@Result(property = "taskCount", column = "TASK_COUNT"),
@Result(property = "workbasketId", column = "WORKBASKET_ID")})
List<TaskCountPerWorkbasket> queryTaskCountPerWorkbasket(TaskQueryImpl taskQuery);
Long countQueryTasks(TaskQueryImpl taskQuery);
@Select("<script>SELECT COUNT(ID) FROM TASKANA.CLASSIFICATION "
+ "<where>"
@ -568,6 +576,12 @@ public interface QueryMapper {
@Select("<script>SELECT DISTINCT ${columnName} "
+ "FROM TASKANA.TASK t "
+ "<where>"
+ "<if test='accessIdIn != null'> "
+ "AND t.WORKBASKET_ID IN ( "
+ "select WID from (select WORKBASKET_ID as WID, max(PERM_READ) as MAX_READ FROM TASKANA.WORKBASKET_ACCESS_LIST where "
+ "ACCESS_ID IN (<foreach item='item' collection='accessIdIn' separator=',' >#{item}</foreach>) "
+ "group by WORKBASKET_ID ) where max_read = 1 ) "
+ "</if> "
+ "<if test='taskIds != null'>AND t.ID IN(<foreach item='item' collection='taskIds' separator=',' >#{item}</foreach>)</if> "
+ "<if test='createdIn !=null'> AND ( <foreach item='item' collection='createdIn' separator=' OR ' > ( <if test='item.begin!=null'> t.CREATED &gt;= #{item.begin} </if> <if test='item.begin!=null and item.end!=null'> AND </if><if test='item.end!=null'> t.CREATED &lt;=#{item.end} </if>)</foreach>)</if> "
+ "<if test='claimedIn !=null'> AND ( <foreach item='item' collection='claimedIn' separator=' OR ' > ( <if test='item.begin!=null'> t.CLAIMED &gt;= #{item.begin} </if> <if test='item.begin!=null and item.end!=null'> AND </if><if test='item.end!=null'> t.CLAIMED &lt;=#{item.end} </if>)</foreach>)</if> "

View File

@ -622,4 +622,26 @@ public class QueryTasksAccTest extends AbstractAccTest {
}
@WithAccessId(
userName = "teamlead_1",
groupNames = {"businessadmin"})
@Test
public void testQueryAllPaged() {
TaskService taskService = taskanaEngine.getTaskService();
TaskQuery taskQuery = taskService.createTaskQuery();
long numberOfTasks = taskQuery.count();
Assert.assertEquals(22, numberOfTasks);
List<TaskSummary> tasks = taskQuery
.orderByDue(SortDirection.DESCENDING)
.list();
List<TaskSummary> tasksp = taskQuery
.orderByDue(SortDirection.DESCENDING)
.listPage(4, 5);
Assert.assertEquals(5, tasksp.size());
tasksp = taskQuery
.orderByDue(SortDirection.DESCENDING)
.listPage(5, 5);
Assert.assertEquals(2, tasksp.size());
}
}

View File

@ -118,26 +118,34 @@ public class TaskControllerIntTest {
}
@Test
@Ignore
public void testGetLastPageSortedByDueWithHiddenTasksRemovedFromResult() {
RestTemplate template = getRestTemplate();
HttpHeaders headers = new HttpHeaders();
headers.add("Authorization", "Basic dGVhbWxlYWRfMTp0ZWFtbGVhZF8x");
HttpEntity<String> request = new HttpEntity<String>(headers);
ResponseEntity<PagedResources<TaskSummaryResource>> response = template.exchange(
"http://127.0.0.1:" + port + "/v1/tasks?sortBy=due&order=desc&page=14&pageSize=5", HttpMethod.GET,
"http://127.0.0.1:" + port + "/v1/tasks?sortBy=due&order=desc", HttpMethod.GET,
request,
new ParameterizedTypeReference<PagedResources<TaskSummaryResource>>() {
});
int size = response.getBody().getContent().size();
assertEquals(22, response.getBody().getContent().size());
response = template.exchange(
"http://127.0.0.1:" + port + "/v1/tasks?sortBy=due&order=desc&page=5&pageSize=5", HttpMethod.GET,
request,
new ParameterizedTypeReference<PagedResources<TaskSummaryResource>>() {
});
assertEquals(2, response.getBody().getContent().size());
assertTrue(response.getBody().getLink(Link.REL_LAST).getHref().contains("page=14"));
assertEquals("TKI:000000000000000000000000000000000005",
assertTrue(response.getBody().getLink(Link.REL_LAST).getHref().contains("page=5"));
assertEquals("TKI:000000000000000000000000000000000023",
response.getBody().getContent().iterator().next().getTaskId());
assertNotNull(response.getBody().getLink(Link.REL_SELF));
assertTrue(response.getBody()
.getLink(Link.REL_SELF)
.getHref()
.endsWith("/v1/tasks?sortBy=due&order=desc&page=2&pageSize=5"));
.endsWith("/v1/tasks?sortBy=due&order=desc&page=5&pageSize=5"));
assertNotNull(response.getBody().getLink("allTasks"));
assertTrue(response.getBody()
.getLink("allTasks")
@ -145,7 +153,6 @@ public class TaskControllerIntTest {
.endsWith("/v1/tasks"));
assertNotNull(response.getBody().getLink(Link.REL_FIRST));
assertNotNull(response.getBody().getLink(Link.REL_LAST));
assertNotNull(response.getBody().getLink(Link.REL_NEXT));
assertNotNull(response.getBody().getLink(Link.REL_PREVIOUS));
}