Skip to content

Commit

Permalink
Fix for total count of filtered task list result (#1512)
Browse files Browse the repository at this point in the history
* added Query utils to detect if query to be executed concerns a count query, if so skip providing groupBy

* switched to single result for count query

* added copyright header

* removed distinct from list query as this is not necessary

* added additional todo for performance improvement

* distinct is not necessary when also having a group by

---------

Co-authored-by: valtimo-platform[bot] <80107705+valtimo-platform[bot]@users.noreply.github.com>
  • Loading branch information
2 people authored and ThomasMinkeRitense committed Sep 20, 2024
1 parent 50002ca commit 6b8b81c
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 35 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2015-2024 Ritense BV, the Netherlands.
*
* Licensed under EUPL, Version 1.2 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
*
* 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 com.ritense.authorization.utils

import jakarta.persistence.criteria.AbstractQuery
import org.hibernate.query.sqm.function.SelfRenderingSqmAggregateFunction

object QueryUtils {

fun isCountQuery(query: AbstractQuery<*>) =
query.selection is SelfRenderingSqmAggregateFunction
&&
(query.selection as SelfRenderingSqmAggregateFunction<out Any>).functionName == "count"
}
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ public Page<TaskExtended> findTasksFiltered(
var processDefinitionKeyPath = taskRoot.get(PROCESS_DEFINITION).get(KEY);

query.multiselect(taskRoot, executionIdPath, businessKeyPath, processDefinitionIdPath, processDefinitionKeyPath);
query.distinct(true);
query.where(specification.toPredicate(taskRoot, query, cb));
query.groupBy(taskRoot, executionIdPath, businessKeyPath, processDefinitionIdPath, processDefinitionKeyPath);
query.orderBy(getOrderBy(cb, taskRoot, pageable.getSort()));
Expand Down Expand Up @@ -388,14 +387,16 @@ public Page<TaskExtended> findTasksFiltered(
})
.toList();

var cbCount = entityManager.getCriteriaBuilder();
var queryCount = cbCount.createQuery();
var taskCountRoot = queryCount.from(CamundaTask.class);
queryCount.select(cbCount.countDistinct(taskCountRoot));
queryCount.where(specification.toPredicate(taskCountRoot, queryCount, cbCount));
var results = entityManager.createQuery(queryCount).getResultList();
long total = results.isEmpty() ? 0 : (long) results.get(0);
return new PageImpl<>(tasks, pageable, total);
return new PageImpl<>(tasks, pageable, countTasksFiltered(specification));
}

private long countTasksFiltered(Specification<CamundaTask> specification) {
var cb = entityManager.getCriteriaBuilder();
var countQuery = cb.createQuery(Long.class);
var taskCountRoot = countQuery.from(CamundaTask.class);
countQuery.select(cb.countDistinct(taskCountRoot));
countQuery.where(specification.toPredicate(taskCountRoot, countQuery, cb));
return entityManager.createQuery(countQuery).getSingleResult();
}

@Transactional(readOnly = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,25 +190,15 @@ private Page<JsonSchemaDocument> search(QueryWhereBuilder queryWhereBuilder, Pag
return new PageImpl<>(documents, pageable, count(queryWhereBuilder));
}

private Long count(
QueryWhereBuilder queryWhereBuilder
) {
private Long count(QueryWhereBuilder queryWhereBuilder) {
final CriteriaBuilder cb = entityManager.getCriteriaBuilder();
final CriteriaQuery<Long> countQuery = cb.createQuery(Long.class);
Root<JsonSchemaDocument> countRoot = countQuery.from(JsonSchemaDocument.class);
countQuery.select(cb.count(countRoot));
countQuery.select(cb.countDistinct(countRoot));
queryWhereBuilder.apply(cb, countQuery, countRoot);

// TODO: Should be turned into a subquery, and then do a count over the results from the subquery.
List<Long> countResultList = entityManager.createQuery(countQuery).getResultList();

long count = 0L;

if (!countResultList.isEmpty()) {
count = countResultList.size();
}

return count;
return entityManager.createQuery(countQuery).getSingleResult();
}

private void buildQueryWhere(SearchRequest searchRequest, CriteriaBuilder cb, CriteriaQuery<?> query, Root<JsonSchemaDocument> documentRoot) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ import com.ritense.authorization.AuthorizationContext.Companion.runWithoutAuthor
import com.ritense.authorization.permission.Permission
import com.ritense.authorization.request.AuthorizationRequest
import com.ritense.authorization.specification.AuthorizationSpecification
import com.ritense.authorization.utils.QueryUtils
import com.ritense.document.domain.impl.JsonSchemaDocument
import com.ritense.document.service.impl.JsonSchemaDocumentService
import com.ritense.valtimo.contract.database.QueryDialectHelper
import jakarta.persistence.criteria.AbstractQuery
import jakarta.persistence.criteria.CriteriaBuilder
import jakarta.persistence.criteria.Predicate
import jakarta.persistence.criteria.Root
import org.hibernate.query.sqm.function.SelfRenderingSqmAggregateFunction

class JsonSchemaDocumentSpecification(
authRequest: AuthorizationRequest<JsonSchemaDocument>,
Expand All @@ -41,7 +43,7 @@ class JsonSchemaDocumentSpecification(
): Predicate {
// Filter the permissions for the relevant ones and use those to find the filters that are required
// Turn those filters into predicates
if (query.groupList.isEmpty()) {
if (!QueryUtils.isCountQuery(query) && query.groupList.isEmpty()) {
val groupList = ArrayList(query.groupList)
groupList.add(root.get<Any>("id").get<Any>("id"))
query.groupBy(groupList)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package com.ritense.processdocument.camunda.authorization

import com.ritense.authorization.AuthorizationEntityMapper
import com.ritense.authorization.AuthorizationEntityMapperResult
import com.ritense.authorization.utils.QueryUtils
import com.ritense.document.domain.impl.JsonSchemaDocument
import com.ritense.document.domain.impl.JsonSchemaDocumentId
import com.ritense.document.repository.impl.JsonSchemaDocumentRepository
Expand All @@ -32,6 +33,7 @@ import com.ritense.valtimo.contract.database.QueryDialectHelper
import jakarta.persistence.criteria.AbstractQuery
import jakarta.persistence.criteria.CriteriaBuilder
import jakarta.persistence.criteria.Root
import org.hibernate.query.sqm.function.SelfRenderingSqmAggregateFunction

class CamundaTaskDocumentMapper(
private val processDocumentInstanceRepository: ProcessDocumentInstanceRepository,
Expand All @@ -53,7 +55,9 @@ class CamundaTaskDocumentMapper(
): AuthorizationEntityMapperResult<JsonSchemaDocument> {
val documentRoot = query.from(JsonSchemaDocument::class.java)
val processBusinessKey = root.get<CamundaExecution>(PROCESS_INSTANCE).get<String>(BUSINESS_KEY)
query.groupBy(query.groupList + root.get<String>(ID))
if (!QueryUtils.isCountQuery(query)) {
query.groupBy(query.groupList + root.get<String>(ID))
}
val documentId = queryDialectHelper.uuidToString(
criteriaBuilder,
documentRoot.get<JsonSchemaDocumentId>(ID).get(ID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,24 +183,27 @@ class CaseTaskListSearchService(
val groupList = query.groupList.toMutableList()
groupList.addAll(selectCols)
query.groupBy(groupList)
query.where(constructWhere(cb, query, taskRoot, documentRoot, caseDefinitionName, advancedSearchRequest))
query.orderBy(constructOrderBy(query, cb, taskRoot, documentRoot, pageable.sort))

val countQuery = cb.createQuery(Long::class.java)
val countTaskRoot = countQuery.from(CamundaTask::class.java)
val countDocumentRoot = countQuery.from(JsonSchemaDocument::class.java)
countQuery.select(cb.count(countDocumentRoot))
entityManager.createQuery(countQuery)
countQuery.where(constructWhere(cb, countQuery, countTaskRoot, countDocumentRoot, caseDefinitionName, advancedSearchRequest))
// TODO: look into ability to re-use where predicate in list and count query. improves performance
query.where(constructWhere(cb, query, taskRoot, documentRoot, caseDefinitionName, advancedSearchRequest))

// Can't use singleResult here due to the group by issue mentioned above.
val count = entityManager.createQuery(countQuery).resultList.sum()
query.orderBy(constructOrderBy(query, cb, taskRoot, documentRoot, pageable.sort))

val pagedQuery = entityManager.createQuery(query)
.setFirstResult(pageable.offset.toInt())
.setMaxResults(pageable.pageSize)

return PageImpl(pagedQuery.resultList, pageable, count)
return PageImpl(pagedQuery.resultList, pageable, count(caseDefinitionName, advancedSearchRequest))
}

private fun count(caseDefinitionName: String, advancedSearchRequest: AdvancedSearchRequest): Long {
val cb: CriteriaBuilder = entityManager.criteriaBuilder
val query = cb.createQuery(Long::class.java)
val taskRoot = query.from(CamundaTask::class.java)
val documentRoot = query.from(JsonSchemaDocument::class.java)
query.select(cb.countDistinct(taskRoot))
query.where(constructWhere(cb, query, taskRoot, documentRoot, caseDefinitionName, advancedSearchRequest))
return entityManager.createQuery(query).singleResult
}

private fun constructWhere(
Expand All @@ -216,6 +219,7 @@ class CaseTaskListSearchService(

val assignmentFilterPredicate: Predicate = constructAssignmentFilter(advancedSearchRequest.assigneeFilter, cb, taskRoot)

// TODO: look into options to improve performance as with complex rules this takes quite some time to finish
val searchRequestPredicate: Array<Predicate> = constructSearchCriteriaFilter(advancedSearchRequest, cb, query, taskRoot, documentRoot)

val where = cb.and(
Expand Down

0 comments on commit 6b8b81c

Please sign in to comment.