Merge pull request #14 from bee-produced/13-pagination-wrong-behaviour

bee.persistent.jpa: Pagination wrong behaviour

Author: kurbaniec

bee.persistent/src/jpa/kotlin/com/beeproduced/bee/persistent/jpa/meta/EntityInfo.kt

@@ -1,9 +1,7 @@
 package com.beeproduced.bee.persistent.jpa.meta
 
-import jakarta.persistence.EmbeddedId
 import jakarta.persistence.IdClass
 import java.lang.reflect.Field
-import java.lang.reflect.Method
 
 
 /**
@@ -21,7 +19,7 @@ class EntityInfo(
 
     data class EntityToIdField(val entityField: Field, val idField: Field)
 
-    val compositeKeyMapping: Set<EntityToIdField>?
+    val compositeKeyMapping: List<EntityToIdField>?
 
     // https://www.baeldung.com/jpa-entity-table-names#defaultNames
     // https://stackoverflow.com/a/634629/12347616
@@ -33,7 +31,7 @@ class EntityInfo(
         if (type.isAnnotationPresent(IdClass::class.java)) {
             // Map id properties if composite key
             nonPrimitiveIdType = idType
-            compositeKeyMapping = idType.declaredFields.mapTo(HashSet()) { idField ->
+            compositeKeyMapping = idType.declaredFields.map { idField ->
                 idField.isAccessible = true
                 val entityField = type.getDeclaredField(idField.name)
                 EntityToIdField(entityField, idField)

bee.persistent/src/jpa/kotlin/com/beeproduced/bee/persistent/jpa/repository/BaseDataRepository.kt

@@ -11,9 +11,11 @@ import com.beeproduced.bee.persistent.selection.EmptySelection
 import com.linecorp.kotlinjdsl.QueryFactoryImpl
 import com.linecorp.kotlinjdsl.query.creator.CriteriaQueryCreatorImpl
 import com.linecorp.kotlinjdsl.query.creator.SubqueryCreatorImpl
+import com.linecorp.kotlinjdsl.query.spec.expression.ColumnSpec
 import com.linecorp.kotlinjdsl.query.spec.expression.EntitySpec
 import com.linecorp.kotlinjdsl.querydsl.CriteriaDeleteQueryDsl
 import com.linecorp.kotlinjdsl.querydsl.CriteriaQueryDsl
+import com.linecorp.kotlinjdsl.selectQuery
 import jakarta.persistence.EntityManager
 import org.hibernate.metamodel.model.domain.internal.MappingMetamodelImpl
 import org.slf4j.Logger
@@ -41,8 +43,9 @@ abstract class BaseDataRepository<T : DataEntity<T>, ID : Any>(
     protected val relations: Relations
     protected val generated: Generated
 
-    protected val selectIdColumns: CriteriaQueryDsl<*>.() -> Unit
-    protected val selectCount: CriteriaQueryDsl<Long>.() -> Unit
+    protected val selectDistinctIdColumns: CriteriaQueryDsl<*>.() -> Unit
+    protected val selectDistinctIdAndOrderColumns: CriteriaQueryDsl<*>.(List<ColumnSpec<*>>) -> Unit
+    protected val selectCount: CriteriaQueryDsl<Long>.(forceDistinct: Boolean) -> Unit
     protected val queryFactory = QueryFactoryImpl(
         criteriaQueryCreator = CriteriaQueryCreatorImpl(entityManager),
         subqueryCreator = SubqueryCreatorImpl()
@@ -78,8 +81,11 @@ abstract class BaseDataRepository<T : DataEntity<T>, ID : Any>(
         // Is needed, as there is a difference between selecting entity with single primary key
         // and with composite key
         @Suppress("UNCHECKED_CAST")
-        selectIdColumns = QueryBuilder.selectIdsFromEntity(this.type, ids)
-                as CriteriaQueryDsl<*>.() -> Unit
+        selectDistinctIdColumns = QueryBuilder.selectDistinctIdsFromEntity(this.type, ids)
+            as CriteriaQueryDsl<*>.() -> Unit
+        @Suppress("UNCHECKED_CAST")
+        selectDistinctIdAndOrderColumns = QueryBuilder.selectDistinctIdAndOrderColumnsFromEntity(this.type, ids)
+            as CriteriaQueryDsl<*>.(List<ColumnSpec<*>>) -> Unit
         selectCount = QueryBuilder.selectCount(this.type, ids)
     }
 
@@ -199,15 +205,15 @@ abstract class BaseDataRepository<T : DataEntity<T>, ID : Any>(
         dsl: CriteriaQueryDsl<*>.() -> Unit = {}
     ): List<T> {
         val testDsl = DummyCriteriaQueryDsl().apply(dsl)
-        val query = if (testDsl.hasLimitClause) {
+        val query = if (testDsl.hasLimitClause && !testDsl.hasOrderByClause) {
             // Query ids first, then query with where!
             // Hibernate cannot use `LIMIT` when joining tables!
             // To be precise, it returns a “limited” result but queries EVERYTHING and limits in memory!
             // https://vladmihalcea.com/fix-hibernate-hhh000104-entity-fetch-pagination-warning-message/
             // https://hibernate.atlassian.net/browse/HHH-15964?focusedCommentId=110593
             val resultIds: List<Any> = queryFactory.selectQuery(info.nonPrimitiveIdType) {
                 // select(selectIdColumns)
-                selectIdColumns(this)
+                selectDistinctIdColumns(this)
                 from(EntitySpec(type))
                 dsl(this)
             }.resultList
@@ -218,7 +224,25 @@ abstract class BaseDataRepository<T : DataEntity<T>, ID : Any>(
                 select(EntitySpec(type))
                 from(EntitySpec(type))
                 where(QueryBuilder.whereClauseFromIds(type, info.compositeKeyMapping, resultIds, ids))
+            }
+        } else if (testDsl.hasLimitClause && testDsl.hasOrderByClause) {
+            val orderByColumns = testDsl.orderByColumns()
+            val idsAndOrderBy: List<List<Any>> = queryFactory.selectQuery<List<Any>> {
+                selectDistinctIdAndOrderColumns(this, orderByColumns)
+                from(EntitySpec(type))
                 dsl(this)
+            }.resultList
+
+            if (idsAndOrderBy.isEmpty()) return emptyList()
+            val resultIds = idsAndOrderBy.map {
+                it.dropLast(orderByColumns.count())
+            }
+
+            queryFactory.selectQuery(type) {
+                select(EntitySpec(type))
+                from(EntitySpec(type))
+                where(QueryBuilder.whereClauseFromIds(type, info.compositeKeyMapping, resultIds, ids))
+                orderBy(testDsl.orders)
             }
         } else {
             queryFactory.selectQuery(type) {
@@ -242,8 +266,12 @@ abstract class BaseDataRepository<T : DataEntity<T>, ID : Any>(
     }
 
     open fun count(dsl: CriteriaQueryDsl<*>.() -> Unit = {}): Long {
+        return count(true, dsl)
+    }
+
+    open fun count(distinct: Boolean, dsl: CriteriaQueryDsl<*>.() -> Unit = {}): Long {
         val count = queryFactory.selectQuery(Long::class.javaObjectType) {
-            selectCount(this)
+            selectCount(this, distinct)
             from(EntitySpec(type))
             dsl(this)
         }
@@ -255,7 +283,7 @@ abstract class BaseDataRepository<T : DataEntity<T>, ID : Any>(
      */
     open fun exists(id: ID): Boolean {
         val resultIds: List<Any> = queryFactory.selectQuery(info.nonPrimitiveIdType) {
-            selectIdColumns(this)
+            selectDistinctIdColumns(this)
             from(EntitySpec(type))
             where(QueryBuilder.whereClauseFromId(type, info.compositeKeyMapping, id, ids))
         }.resultList
@@ -270,7 +298,7 @@ abstract class BaseDataRepository<T : DataEntity<T>, ID : Any>(
     open fun existsAll(ids: Collection<ID>): Boolean {
         val uniqueIds = ids.toSet()
         val resultIds: List<Any> = queryFactory.selectQuery(info.nonPrimitiveIdType) {
-            selectIdColumns(this)
+            selectDistinctIdColumns(this)
             from(EntitySpec(type))
             where(
                 QueryBuilder.whereClauseFromIds(

bee.persistent/src/jpa/kotlin/com/beeproduced/bee/persistent/jpa/repository/QueryBuilder.kt

@@ -53,7 +53,7 @@ object QueryBuilder {
      */
     fun whereClauseFromId(
         entityType: Class<*>,
-        compositeKeyMapping: Set<EntityInfo.EntityToIdField>?,
+        compositeKeyMapping: List<EntityInfo.EntityToIdField>?,
         id: Any,
         idInfo: Ids
     ): PredicateSpec? {
@@ -82,9 +82,10 @@ object QueryBuilder {
      */
     private fun whereClauseFromCompositeKey(
         entityType: Class<*>,
-        compositeKeyMapping: Set<EntityInfo.EntityToIdField>,
+        compositeKeyMapping: List<EntityInfo.EntityToIdField>,
         idInfo: Any,
     ): PredicateSpec? {
+        if (idInfo is List<*>) return whereClauseFromCompositeKeyList(entityType, compositeKeyMapping, idInfo)
         var whereClause: PredicateSpec? = null
         for ((entityField, idField) in compositeKeyMapping) {
             val entitySpec = EntitySpec(entityType)
@@ -101,10 +102,33 @@ object QueryBuilder {
         return whereClause
     }
 
+    private fun whereClauseFromCompositeKeyList(
+        entityType: Class<*>,
+        compositeKeyMapping: List<EntityInfo.EntityToIdField>,
+        idInfo: List<*>,
+    ): PredicateSpec? {
+        var whereClause: PredicateSpec? = null
+        var i = 0
+        for ((entityField) in compositeKeyMapping) {
+            val entitySpec = EntitySpec(entityType)
+            val columnSpec = ColumnSpec<Any?>(entitySpec, entityField.name)
+            val idValue = requireNotNull(idInfo[i])
+            val equalSpec = EqualValueSpec(columnSpec, idValue)
+
+            whereClause = if (whereClause == null) {
+                equalSpec
+            } else {
+                AndSpec(listOf(whereClause, equalSpec))
+            }
+            i++
+        }
+        return whereClause
+    }
+
 
     fun whereClauseFromIds(
         entityType: Class<*>,
-        compositeKeyMapping: Set<EntityInfo.EntityToIdField>?,
+        compositeKeyMapping: List<EntityInfo.EntityToIdField>?,
         ids: Collection<Any>,
         idInfo: Ids
     ): PredicateSpec? {
@@ -125,7 +149,7 @@ object QueryBuilder {
 
     private fun whereClauseFromCompositeKeys(
         entityType: Class<*>,
-        compositeKeyMapping: Set<EntityInfo.EntityToIdField>,
+        compositeKeyMapping: List<EntityInfo.EntityToIdField>,
         ids: Collection<Any>,
     ): PredicateSpec {
         // TODO: Optimize? Is there are way to use `IN` instead of joining `AND` statements with `OR`
@@ -164,7 +188,7 @@ object QueryBuilder {
     //
     // }
 
-    fun selectIdsFromEntity(
+    fun selectDistinctIdsFromEntity(
         entityType: Class<*>,
         ids: Ids
     ): CriteriaQueryDsl<Any>.() -> Unit {
@@ -183,7 +207,7 @@ object QueryBuilder {
             val column: ExpressionSpec<Any> = ColumnSpec(entitySpec, member.name)
             val singleSelectQuery: CriteriaQueryDsl<Any>.() -> Unit = {
                 // dsl.select(ColumnSpec<Any?>(entitySpec, member.name))
-                this.select(column)
+                this.selectDistinct(column)
             }
             singleSelectQuery
         } else {
@@ -193,23 +217,58 @@ object QueryBuilder {
                 ColumnSpec<Any>(entitySpec, member.name)
             }
             val multiSelectQuery: CriteriaQueryDsl<*>.() -> Unit = {
-                this.select(columns)
+                this.selectDistinct(columns)
             }
             multiSelectQuery
         }
     }
 
-    fun selectCount(
+    fun selectDistinctIdAndOrderColumnsFromEntity(
         entityType: Class<*>,
         ids: Ids
-    ): CriteriaQueryDsl<Long>.() -> Unit {
+    ): CriteriaQueryDsl<Any>.(orderBy: List<ColumnSpec<*>>) -> Unit {
+        // Return correct corresponding select statement
+        // If entity does not have a composite key
+        // use CriteriaQueryDsl's SingleSelectClause
+        // If entity has a composite key
+        // use CriteriaQueryDsl's MultiSelectClause
+        // Why? Using only MultiSelectClause can result in not found constructors
+        // E.g., the UUID constructor cannot be invoked with a list
+        return if (ids.values.count() == 1) {
+            val id = ids.values.first()
+            val member = id.field
+            val entitySpec = EntitySpec(entityType)
+            // SingleSelectClause(idType, false, ColumnSpec(entitySpec, member.name))
+            val column: ExpressionSpec<Any> = ColumnSpec(entitySpec, member.name)
+            val singleSelectQuery: CriteriaQueryDsl<Any>.(List<ColumnSpec<*>>) -> Unit = {
+                // dsl.select(ColumnSpec<Any?>(entitySpec, member.name))
+                this.selectDistinct(listOf(column) + it)
+            }
+            singleSelectQuery
+        } else {
+            val columns = ids.values.map { id ->
+                val member = id.field
+                val entitySpec = EntitySpec(entityType)
+                ColumnSpec<Any>(entitySpec, member.name)
+            }
+            val multiSelectQuery: CriteriaQueryDsl<*>.(List<ColumnSpec<*>>) -> Unit = {
+                this.selectDistinct(columns + it)
+            }
+            multiSelectQuery
+        }
+    }
+
+    fun selectCount(
+        entityType: Class<*>,
+        ids: Ids,
+    ): CriteriaQueryDsl<Long>.(forceDistinct: Boolean) -> Unit {
         val id = ids.values.first()
         val member = id.field
         val entitySpec = EntitySpec(entityType)
         val column: ExpressionSpec<Any> = ColumnSpec(entitySpec, member.name)
-        val singleSelectQuery: CriteriaQueryDsl<Long>.() -> Unit = {
-            // dsl.select(ColumnSpec<Any?>(entitySpec, member.name))
-            this.select(listOf(count(column)))
+        val singleSelectQuery: CriteriaQueryDsl<Long>.(Boolean) -> Unit = { forceDistinct ->
+            if (!forceDistinct) this.select(listOf(count(column)))
+            else this.select(listOf(countDistinct(column)))
         }
         return singleSelectQuery
     }

bee.persistent/src/jpa/kotlin/com/beeproduced/bee/persistent/jpa/repository/dsl/DummyCriteriaQueryDsl.kt

@@ -43,14 +43,16 @@ class DummyCriteriaQueryDsl : CriteriaQueryDsl<Any> {
         right: EntitySpec<R>,
         relation: Relation<T, R?>,
         joinType: JoinType
-    ) {}
+    ) {
+    }
 
     override fun <T, R> fetch(
         left: EntitySpec<T>,
         right: EntitySpec<R>,
         relation: Relation<T, R?>,
         joinType: JoinType
-    ) {}
+    ) {
+    }
 
     override fun from(entity: EntitySpec<*>) {}
 
@@ -60,11 +62,26 @@ class DummyCriteriaQueryDsl : CriteriaQueryDsl<Any> {
 
     override fun hints(hints: Map<String, Any>) {}
 
-    override fun <T, R> join(left: EntitySpec<T>, right: EntitySpec<R>, relation: Relation<T, R?>, joinType: JoinType) {}
+    override fun <T, R> join(
+        left: EntitySpec<T>,
+        right: EntitySpec<R>,
+        relation: Relation<T, R?>,
+        joinType: JoinType
+    ) {
+    }
 
     override fun <T> join(entity: EntitySpec<T>, predicate: PredicateSpec) {}
 
-    override fun orderBy(orders: List<OrderSpec>) {}
+    private val orderByClauses = mutableListOf<OrderSpec>()
+
+    val hasOrderByClause get() = orderByClauses.isNotEmpty()
+
+    val orders: List<OrderSpec> get() = orderByClauses
+    fun orderByColumns() = orderByClauses.map { it.getColumnSpec() }
+
+    override fun orderBy(orders: List<OrderSpec>) {
+        orderByClauses.addAll(orders)
+    }
 
     override fun select(distinct: Boolean, expression: ExpressionSpec<Any>): SingleSelectClause<Any> {
         throw IllegalAccessException("Should not be invoked")
@@ -81,7 +98,8 @@ class DummyCriteriaQueryDsl : CriteriaQueryDsl<Any> {
         parent: EntitySpec<P>,
         child: EntitySpec<C>,
         parentJoinType: JoinType
-    ) {}
+    ) {
+    }
 
     override fun where(predicate: PredicateSpec?) {}