Skip to content

Commit d9c40ad

Browse files
obabichevjbobabichevjb
authored
fix: Exposed-829 Eager loading relationships with an orderBy (JetBrains#2693)
* fix: Exposed-829 Eager loading relationships with an orderBy clause does not honor order * fix: Exposed-829 Review issues
1 parent ac4a8dc commit d9c40ad

File tree

4 files changed

+57
-14
lines changed

4 files changed

+57
-14
lines changed

‎exposed-dao/api/exposed-dao.api‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,10 @@ public abstract class org/jetbrains/exposed/v1/dao/EntityClass {
171171
protected fun warmCache ()Lorg/jetbrains/exposed/v1/dao/EntityCache;
172172
public final fun warmUpLinkedReferences (Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Table;Ljava/lang/Boolean;Z)Ljava/util/List;
173173
public static synthetic fun warmUpLinkedReferences$default (Lorg/jetbrains/exposed/v1/dao/EntityClass;Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Table;Ljava/lang/Boolean;ZILjava/lang/Object;)Ljava/util/List;
174-
public final fun warmUpOptReferences (Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Column;Ljava/lang/Boolean;)Ljava/util/List;
175-
public static synthetic fun warmUpOptReferences$default (Lorg/jetbrains/exposed/v1/dao/EntityClass;Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Column;Ljava/lang/Boolean;ILjava/lang/Object;)Ljava/util/List;
176-
public final fun warmUpReferences (Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Column;Ljava/lang/Boolean;)Ljava/util/List;
177-
public static synthetic fun warmUpReferences$default (Lorg/jetbrains/exposed/v1/dao/EntityClass;Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Column;Ljava/lang/Boolean;ILjava/lang/Object;)Ljava/util/List;
174+
public final fun warmUpOptReferences (Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Column;Ljava/lang/Boolean;[Lkotlin/Pair;)Ljava/util/List;
175+
public static synthetic fun warmUpOptReferences$default (Lorg/jetbrains/exposed/v1/dao/EntityClass;Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Column;Ljava/lang/Boolean;[Lkotlin/Pair;ILjava/lang/Object;)Ljava/util/List;
176+
public final fun warmUpReferences (Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Column;Ljava/lang/Boolean;[Lkotlin/Pair;)Ljava/util/List;
177+
public static synthetic fun warmUpReferences$default (Lorg/jetbrains/exposed/v1/dao/EntityClass;Ljava/util/List;Lorg/jetbrains/exposed/v1/core/Column;Ljava/lang/Boolean;[Lkotlin/Pair;ILjava/lang/Object;)Ljava/util/List;
178178
public final fun wrap (Lorg/jetbrains/exposed/v1/core/dao/id/EntityID;Lorg/jetbrains/exposed/v1/core/ResultRow;)Lorg/jetbrains/exposed/v1/dao/Entity;
179179
public final fun wrapRow (Lorg/jetbrains/exposed/v1/core/ResultRow;)Lorg/jetbrains/exposed/v1/dao/Entity;
180180
public final fun wrapRow (Lorg/jetbrains/exposed/v1/core/ResultRow;Lorg/jetbrains/exposed/v1/core/Alias;)Lorg/jetbrains/exposed/v1/dao/Entity;

‎exposed-dao/src/main/kotlin/org/jetbrains/exposed/v1/dao/EntityClass.kt‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -825,9 +825,15 @@ abstract class EntityClass<ID : Any, out T : Entity<ID>>(
825825
*
826826
* Set [forUpdate] to `true` or `false` depending on whether a locking read should be placed or removed from the
827827
* search query used. Leave the argument as `null` to use the query without any locking option.
828+
*
829+
* Set [orderBy] to specify the order in which entities should be sorted.
828830
*/
829-
fun <SID> warmUpOptReferences(references: List<SID>, refColumn: Column<SID?>, forUpdate: Boolean? = null): List<T> =
830-
warmUpReferences(references, refColumn as Column<SID>, forUpdate)
831+
fun <SID> warmUpOptReferences(
832+
references: List<SID>,
833+
refColumn: Column<SID?>,
834+
forUpdate: Boolean? = null,
835+
orderBy: Array<Pair<Expression<*>, SortOrder>>? = null
836+
): List<T> = warmUpReferences(references, refColumn as Column<SID>, forUpdate, orderBy)
831837

832838
/**
833839
* Returns a list of retrieved [Entity] instances whose [refColumn] matches any of the id values in [references].
@@ -837,8 +843,15 @@ abstract class EntityClass<ID : Any, out T : Entity<ID>>(
837843
*
838844
* Set [forUpdate] to `true` or `false` depending on whether a locking read should be placed or removed from the
839845
* search query used. Leave the argument as `null` to use the query without any locking option.
846+
*
847+
* Set [orderBy] to specify the order in which entities should be sorted.
840848
*/
841-
fun <SID> warmUpReferences(references: List<SID>, refColumn: Column<SID>, forUpdate: Boolean? = null): List<T> {
849+
fun <SID> warmUpReferences(
850+
references: List<SID>,
851+
refColumn: Column<SID>,
852+
forUpdate: Boolean? = null,
853+
orderBy: Array<Pair<Expression<*>, SortOrder>>? = null
854+
): List<T> {
842855
val parentTable = refColumn.referee?.table as? IdTable<*>
843856
requireNotNull(parentTable) { "RefColumn should have reference to IdTable" }
844857
if (references.isEmpty()) return emptyList()
@@ -854,6 +867,7 @@ abstract class EntityClass<ID : Any, out T : Entity<ID>>(
854867
}
855868
if (toLoad.isNotEmpty()) {
856869
val findQuery = find { refColumn inList toLoad }
870+
.orderBy(order = orderBy ?: emptyArray())
857871
val entities = getEntities(forUpdate, findQuery)
858872

859873
val result = entities.groupByReference(refColumn = refColumn)
@@ -876,6 +890,7 @@ abstract class EntityClass<ID : Any, out T : Entity<ID>>(
876890
baseQuery.adjustSelect { select(fields + parentTable.id) }
877891
.adjustColumnSet { innerJoin(parentTable, { refColumn }, { refColumn.referee!! }) }
878892
}
893+
.orderBy(order = orderBy ?: emptyArray())
879894

880895
val findQuery = wrapRows(finalQuery)
881896
val entities = getEntities(forUpdate, findQuery).distinct()
@@ -902,7 +917,8 @@ abstract class EntityClass<ID : Any, out T : Entity<ID>>(
902917
references: List<CompositeID>,
903918
refColumns: Map<Column<*>, Column<*>>,
904919
delegateRefColumn: Column<*>,
905-
forUpdate: Boolean? = null
920+
forUpdate: Boolean? = null,
921+
orderBy: Array<Pair<Expression<*>, SortOrder>>? = null
906922
): List<T> {
907923
val parentTable = refColumns.values.firstOrNull()?.table as? CompositeIdTable
908924
requireNotNull(parentTable) { "RefColumns should have reference to CompositeIdTable" }
@@ -917,6 +933,7 @@ abstract class EntityClass<ID : Any, out T : Entity<ID>>(
917933
}
918934
if (toLoad.isNotEmpty()) {
919935
val findQuery = find { refColumns.keys.toList() inList toLoad.map { it.value } }
936+
.orderBy(order = orderBy ?: emptyArray())
920937
val entities = getEntities(forUpdate, findQuery)
921938
val result = entities.groupByReference<CompositeID>(refColumns = refColumns)
922939

@@ -932,6 +949,7 @@ abstract class EntityClass<ID : Any, out T : Entity<ID>>(
932949
return distinctRefIds.flatMap { cache.getReferrers<T>(it, delegateRefColumn)?.toList().orEmpty() }
933950
} else {
934951
val baseQuery = searchQuery(refColumns.keys.toList() inList distinctRefIds.map { it.value })
952+
.orderBy(order = orderBy ?: emptyArray())
935953
val findQuery = wrapRows(baseQuery)
936954
val entities = getEntities(forUpdate, findQuery).distinct()
937955
val result = entities.groupByReference<CompositeID>(refColumns = refColumns)

‎exposed-dao/src/main/kotlin/org/jetbrains/exposed/v1/dao/References.kt‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ open class Referrers<ParentID : Any, in Parent : Entity<ParentID>, ChildID : Any
113113
/** The set of columns and their [SortOrder] for ordering referred entities in one-to-many relationship. */
114114
private val orderByExpressions = linkedSetOf<Pair<Expression<*>, SortOrder>>()
115115

116+
/** Returns the order by expressions as an array. */
117+
internal fun getOrderByExpressions(): Array<Pair<Expression<*>, SortOrder>> = orderByExpressions.toTypedArray()
118+
116119
val allReferences = references ?: run {
117120
reference.referee ?: error("Column $reference is not a reference")
118121

@@ -323,13 +326,14 @@ private fun <ID : Any> List<Entity<ID>>.preloadRelations(
323326
is Referrers<*, *, *, *, *> -> {
324327
(refObject as Referrers<ID, Entity<ID>, *, Entity<*>, Any>).allReferences.let { refColumns ->
325328
val delegateRefColumn = refObject.reference
329+
val orderByExpressions = refObject.getOrderByExpressions()
326330
if (hasSingleReferenceWithReferee(refColumns)) {
327331
val castReferee = delegateRefColumn.referee<Any>()!!
328332
val refIds = this.map { entity -> entity.getRefereeId(castReferee, delegateRefColumn) }
329-
refObject.factory.warmUpReferences(refIds, delegateRefColumn)
333+
refObject.factory.warmUpReferences(refIds, delegateRefColumn, null, orderByExpressions)
330334
} else {
331335
val refIds = this.map { it.getCompositeReferrerId(refColumns) }
332-
refObject.factory.warmUpCompositeIdReferences(refIds, refColumns, delegateRefColumn)
336+
refObject.factory.warmUpCompositeIdReferences(refIds, refColumns, delegateRefColumn, orderBy = orderByExpressions)
333337
}
334338
storeReferenceCache(delegateRefColumn, prop)
335339
}
@@ -348,26 +352,28 @@ private fun <ID : Any> List<Entity<ID>>.preloadRelations(
348352
is BackReference<*, *, *, *, *> -> {
349353
(refObject.delegate as Referrers<ID, Entity<ID>, *, Entity<*>, Any>).allReferences.let { refColumns ->
350354
val delegateRefColumn = refObject.delegate.reference
355+
val orderByExpressions = refObject.delegate.getOrderByExpressions()
351356
if (hasSingleReferenceWithReferee(refColumns)) {
352357
val castReferee = delegateRefColumn.referee<Any>()!!
353358
val refIds = this.map { entity -> entity.getRefereeId(castReferee, delegateRefColumn) }
354-
refObject.delegate.factory.warmUpReferences(refIds, delegateRefColumn)
359+
refObject.delegate.factory.warmUpReferences(refIds, delegateRefColumn, null, orderByExpressions)
355360
} else {
356361
val refIds = this.map { it.getCompositeReferrerId(refColumns) }
357-
refObject.delegate.factory.warmUpCompositeIdReferences(refIds, refColumns, delegateRefColumn)
362+
refObject.delegate.factory.warmUpCompositeIdReferences(refIds, refColumns, delegateRefColumn, orderBy = orderByExpressions)
358363
}
359364
storeReferenceCache(delegateRefColumn, prop)
360365
}
361366
}
362367
is OptionalBackReference<*, *, *, *, *> -> {
363368
(refObject.delegate as Referrers<ID, Entity<ID>, *, Entity<*>, Any?>).allReferences.let { refColumns ->
364369
val delegateRefColumn = refObject.delegate.reference
370+
val orderByExpressions = refObject.delegate.getOrderByExpressions()
365371
if (hasSingleReferenceWithReferee(refColumns)) {
366372
val refIds = this.map { it.run { delegateRefColumn.referee<Any>()!!.lookup() } }
367-
refObject.delegate.factory.warmUpOptReferences(refIds, delegateRefColumn)
373+
refObject.delegate.factory.warmUpOptReferences(refIds, delegateRefColumn, orderBy = orderByExpressions)
368374
} else {
369375
val refIds = this.map { it.getCompositeReferrerId(refColumns) }
370-
refObject.delegate.factory.warmUpCompositeIdReferences(refIds, refColumns, delegateRefColumn)
376+
refObject.delegate.factory.warmUpCompositeIdReferences(refIds, refColumns, delegateRefColumn, orderBy = orderByExpressions)
371377
}
372378
storeReferenceCache(delegateRefColumn, prop)
373379
}

‎exposed-tests/src/test/kotlin/org/jetbrains/exposed/v1/tests/shared/entities/OrderedReferenceTest.kt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ import org.jetbrains.exposed.v1.core.statements.StatementInterceptor
99
import org.jetbrains.exposed.v1.dao.IntEntity
1010
import org.jetbrains.exposed.v1.dao.IntEntityClass
1111
import org.jetbrains.exposed.v1.dao.entityCache
12+
import org.jetbrains.exposed.v1.dao.load
1213
import org.jetbrains.exposed.v1.jdbc.JdbcTransaction
1314
import org.jetbrains.exposed.v1.jdbc.insert
1415
import org.jetbrains.exposed.v1.jdbc.insertAndGetId
1516
import org.jetbrains.exposed.v1.tests.DatabaseTestsBase
1617
import org.jetbrains.exposed.v1.tests.MISSING_R2DBC_TEST
1718
import org.jetbrains.exposed.v1.tests.TestDB
19+
import org.jetbrains.exposed.v1.tests.shared.assertEqualLists
1820
import org.jetbrains.exposed.v1.tests.shared.assertEquals
1921
import org.jetbrains.exposed.v1.tests.shared.assertTrue
2022
import org.junit.jupiter.api.Tag
@@ -203,6 +205,23 @@ class OrderedReferenceTest : DatabaseTestsBase() {
203205
}
204206
}
205207

208+
@Test
209+
@Tag(MISSING_R2DBC_TEST)
210+
fun testOrderByWithEagerLoad() {
211+
withOrderedReferenceTestTables {
212+
// Clearing cache is not critical, just to be sure that there are no caches from
213+
// creating entities step
214+
entityCache.clear()
215+
216+
val user = UserDefaultOrder.all().first().load(UserDefaultOrder::ratings)
217+
218+
val expected = user.ratings.map { it.value }.sorted()
219+
val actual = user.ratings.map { it.value }
220+
221+
assertEqualLists(expected, actual)
222+
}
223+
}
224+
206225
private val unsortedRatingValues = listOf(0, 3, 1, 2, 4, 4, 5, 4, 5, 6, 9, 8)
207226

208227
private fun withOrderedReferenceTestTables(statement: JdbcTransaction.(TestDB) -> Unit) {

0 commit comments

Comments
 (0)