Broken reference caching for keepLoadedReferencesOutOfTransaction

[hyeonseop-lee]

Reference caching is broken when preloading references from parent to child model.

Exposed: 0.35.3

Suppose we have two models in parent-child relation and preloading childrens from parent like below:

parent.load(Parent::children)

1. Reference caching from parent entity to child entities is missing. It seems that caching should be performed in preloadRelations() by calling storeReferenceCache() like other reference types.

   References.kt#L169

   is OptionalReference<*, *, *> -> {
       (refObject as OptionalReference<Comparable<Comparable<*>>, *, Entity<*>>).reference.let { refColumn ->
           this.mapNotNull { it.run { refColumn.lookup() } }.takeIf { it.isNotEmpty() }?.let { refIds ->
               refObject.factory.find { refColumn.referee<Comparable<Comparable<*>>>()!! inList refIds.distinct() }.toList()
           }.orEmpty()
           storeReferenceCache(refColumn, prop)
       }
   }
   is Referrers<*, *, *, *, *> -> {
       (refObject as Referrers<ID, Entity<ID>, *, Entity<*>, Any>).reference.let { refColumn ->
           val refIds = this.map { it.run { refColumn.referee<Any>()!!.lookup() } }
           refObject.factory.warmUpReferences(refIds, refColumn)
           // storeReferenceCache(refColumn, prop) is missing
       }
   }
   is OptionalReferrers<*, *, *, *, *> -> {
       (refObject as OptionalReferrers<ID, Entity<ID>, *, Entity<*>, Any>).reference.let { refColumn ->
           val refIds = this.mapNotNull { it.run { refColumn.referee<Any?>()!!.lookup() } }
           refObject.factory.warmUpOptReferences(refIds, refColumn)
           storeReferenceCache(refColumn, prop)
       }
   }
   

2. Reference caching from child entity to parent entity is broken. warmUpReferences() is caching SizedIterable of children in child entity which has parent id as its id.

   EntityClass.kt#L364

   if (refColumn.columnType is EntityIDColumnType<*>) {
       refColumn as Column<EntityID<*>>
       distinctRefIds as List<EntityID<ID>>
       // distinctRefIds is actually a list of parent entity ids, which is EntityID<SID>, not EntityID<ID>
       val toLoad = distinctRefIds.filter {
           cache.referrers[refColumn]?.containsKey(it)?.not() ?: true
       }
       if (toLoad.isNotEmpty()) {
           val findQuery = find { refColumn inList toLoad }
           val entities = when (forUpdate) {
               true -> findQuery.forUpdate()
               false -> findQuery.notForUpdate()
               else -> findQuery
           }.toList()
   
           val result = entities.groupBy { it.readValues[refColumn] }
   
           distinctRefIds.forEach { id ->
               cache.getOrPutReferrers(id, refColumn) { result[id]?.let { SizedCollection(it) } ?: emptySized() }.also {
                   if (keepLoadedReferenceOutOfTransaction) {
                       findById(id)?.storeReferenceInCache(refColumn, it)
                       // findById() belongs to child model, id belongs to parent model, it is a list of children.
                       // Proper child-to-parent caching would be something like
                       // it.forEach { entity -> entity.storeReferenceInCache(refColumn, parentEntity) }
                   }
               }
           }
       }
   
       return distinctRefIds.flatMap { cache.getReferrers<T>(it, refColumn)?.toList().orEmpty() }
   } else {
   

   By broken caching above warmUpReferences() is invoking numbers of select queries finding child entities having some random id one by one.

   Fix to this doesn't seem trivial since we don't have any information abouot parent entity here except type of id (which is SID). My first thought was to provide entity class of parent as parameter of warmUpReferences() but it might break API compatibility if warmUpReferences() is considered as a public API.

Please give me some opinions on fix and I'd be glad to make a PR. Thank you!

[hyeonseop-lee]

@Tapac Can you take a look into this please?

[fthouraud]

Hi there 👋

I encountered the first case while testing Exposed before using it in a new project. Could you give us your thought on this so maybe we could help with a PR?

It's possible to get around the Referrers caching problem by using the OptionalReferrers but it hides the real representation which isn't ideal.

Another method is to make use of the referrers field inside the transaction. For example:

transaction {
    Author.find { Authors.firstName eq "Ricard" }.first().load(Author::books).also { it.books }
}

Regards.

[bog-walk]

Looking through the git history, it seems a fix was merged using the suggestions above in this commit. The CI build failed so part of the fix was reverted and the other part introduced in this commit.

Running tests for preloading references and referrers are all passing with current version 0.43.0.

If you believe that there is still an issue with reference caching in spite of these commits, please consider reopening this issue on YouTrack, so we can attempt to investigate further. If you do so, please also consider providing a reproducible example that shows what is the actual vs expected behavior, so that we can also evaluate the coverage of our current test cases.