From a99b3f397a11678368efddd7e749706534ceedf9 Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Thu, 19 Sep 2024 22:43:13 -0400 Subject: [PATCH] feat!: EXPOSED-320 Many-to-many relation with extra columns - Add more tests (particularly for update) & rename test classes - Refactor cache to ensure no overlap with wrapped type. Each link entity is now stored by its target column, source column, and target id (stored in entity) - Move new entity classes to own file - Refactor logic for deleting cached entities --- .../Writerside/topics/Deep-Dive-into-DAO.md | 2 +- .../org/jetbrains/exposed/dao/EntityCache.kt | 26 +++-- .../org/jetbrains/exposed/dao/EntityClass.kt | 34 ++++++ .../exposed/dao/EntityLifecycleInterceptor.kt | 1 + .../jetbrains/exposed/dao/InnerTableLink.kt | 74 +++---------- .../exposed/dao/InnerTableLinkEntity.kt | 54 ++++++++++ .../sql/tests/shared/entities/ViaTest.kt | 100 ++++++++++++------ 7 files changed, 194 insertions(+), 97 deletions(-) create mode 100644 exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/InnerTableLinkEntity.kt diff --git a/documentation-website/Writerside/topics/Deep-Dive-into-DAO.md b/documentation-website/Writerside/topics/Deep-Dive-into-DAO.md index 91c4a058c1..d7d79deed6 100644 --- a/documentation-website/Writerside/topics/Deep-Dive-into-DAO.md +++ b/documentation-website/Writerside/topics/Deep-Dive-into-DAO.md @@ -420,7 +420,7 @@ StarWarsFilm.all().first.actors.map { it.roleName } ``` If only some additional columns in the intermediate table should be used during batch insert, these can be specified by using -via() with an argument provided to additionalColumns. +via() with an argument provided to additionalColumns: class StarWarsFilm(id: EntityID<Int>) : IntEntity(id) { companion object : IntEntityClass<StarWarsFilm>(StarWarsFilms) diff --git a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt index ab5871d875..9cb301c33a 100644 --- a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt +++ b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt @@ -23,7 +23,9 @@ class EntityCache(private val transaction: Transaction) { internal val inserts = LinkedHashMap, MutableSet>>() private val updates = LinkedHashMap, MutableSet>>() internal val referrers = HashMap, MutableMap, SizedIterable<*>>>() - internal val innerTableLinks by lazy { LinkedHashMap, MutableMap>>() } + internal val innerTableLinks by lazy { + HashMap, MutableMap, MutableSet>>>() + } /** * The amount of entities to store in this [EntityCache] per [Entity] class. @@ -56,11 +58,7 @@ class EntityCache(private val transaction: Transaction) { } } - private fun getMap(f: EntityClass<*, *>): MutableMap> = if (f is InnerTableLinkEntityClass<*, *>) { - innerTableLinks.getOrPut(f.table) { LimitedHashMap() } - } else { - getMap(f.table) - } + private fun getMap(f: EntityClass<*, *>): MutableMap> = getMap(f.table) private fun getMap(table: IdTable<*>): MutableMap> = data.getOrPut(table) { LimitedHashMap() @@ -104,6 +102,14 @@ class EntityCache(private val transaction: Transaction) { */ fun , T : Entity> findAll(f: EntityClass): Collection = getMap(f).values as Collection + internal fun , ID : Comparable, T : InnerTableLinkEntity> findInnerTableLink( + targetColumn: Column>, + targetId: EntityID, + sourceId: EntityID + ): T? { + return innerTableLinks[targetColumn]?.get(sourceId)?.firstOrNull { it.id == targetId } as? T + } + /** Stores the specified [Entity] in this [EntityCache] using its associated [EntityClass] as the key. */ fun , T : Entity> store(f: EntityClass, o: T) { getMap(f)[o.id.value] = o @@ -118,6 +124,14 @@ class EntityCache(private val transaction: Transaction) { getMap(o.klass.table)[o.id.value] = o } + internal fun , ID : Comparable, T : InnerTableLinkEntity> storeInnerTableLink( + targetColumn: Column>, + sourceId: EntityID, + targetEntity: T + ) { + innerTableLinks.getOrPut(targetColumn) { HashMap() }.getOrPut(sourceId) { mutableSetOf() }.add(targetEntity) + } + /** Removes the specified [Entity] from this [EntityCache] using its associated [table] as the key. */ fun , T : Entity> remove(table: IdTable, o: T) { getMap(table).remove(o.id.value) diff --git a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityClass.kt b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityClass.kt index 4a59a8e7a8..6c125b89de 100644 --- a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityClass.kt +++ b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityClass.kt @@ -163,6 +163,13 @@ abstract class EntityClass, out T : Entity>( with(entity) { col.lookup() }?.let { referrers.remove(it as EntityID<*>) } } } + cache.innerTableLinks.forEach { (_, links) -> + links.remove(entity.id) + + links.forEach { (_, targetEntities) -> + targetEntities.removeAll { it.wrapped == entity } + } + } } /** Returns a [SizedIterable] containing all entities with [EntityID] values from the provided [ids] list. */ @@ -207,6 +214,33 @@ abstract class EntityClass, out T : Entity>( wrapRow(it, alias) } + internal fun > wrapLinkRows( + rows: SizedIterable, + targetColumn: Column>, + sourceColumn: Column> + ): SizedIterable = rows mapLazy { wrapLinkRow(it, targetColumn, sourceColumn) } + + private fun > wrapLinkRow( + row: ResultRow, + targetColumn: Column>, + sourceColumn: Column> + ): T { + val targetId = row[table.id] + val sourceId = row[sourceColumn] + val transaction = TransactionManager.current() + val entity = transaction.entityCache.findInnerTableLink(targetColumn, targetId, sourceId) + ?: createInstance(targetId, row).also { new -> + new.klass = this + new.db = transaction.db + warmCache().storeInnerTableLink(targetColumn, sourceId, new as InnerTableLinkEntity) + } + if (entity._readValues == null) { + entity._readValues = row + } + + return entity + } + /** Wraps the specified [ResultRow] data into an [Entity] instance. */ @Suppress("MemberVisibilityCanBePrivate") fun wrapRow(row: ResultRow): T { diff --git a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityLifecycleInterceptor.kt b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityLifecycleInterceptor.kt index e1e6d8441a..ce66a16254 100644 --- a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityLifecycleInterceptor.kt +++ b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityLifecycleInterceptor.kt @@ -96,6 +96,7 @@ class EntityLifecycleInterceptor : GlobalStatementInterceptor { override fun beforeRollback(transaction: Transaction) { val entityCache = transaction.entityCache entityCache.clearReferrersCache() + entityCache.innerTableLinks.clear() entityCache.data.clear() entityCache.inserts.clear() } diff --git a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/InnerTableLink.kt b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/InnerTableLink.kt index 76aa8a4d93..49fd4ecd61 100644 --- a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/InnerTableLink.kt +++ b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/InnerTableLink.kt @@ -90,12 +90,13 @@ class InnerTableLink, Source : Entity, ID : Comparabl val (columns, entityTables) = columnsAndTables val query = { - target.wrapRows( - @Suppress("SpreadOperator") - entityTables.select(columns) - .where { sourceColumn eq o.id } - .orderBy(*orderByExpressions.toTypedArray()) - ) + @Suppress("SpreadOperator") + val row = entityTables.select(columns) + .where { sourceColumn eq o.id } + .orderBy(*orderByExpressions.toTypedArray()) + (target as? InnerTableLinkEntityClass) + ?.wrapLinkRows(row, targetColumn, sourceColumn) as? SizedIterable + ?: target.wrapRows(row) } return transaction.entityCache.getOrPutReferrers(o.id, sourceColumn, query).also { o.storeReferenceInCache(sourceColumn, it) @@ -120,14 +121,19 @@ class InnerTableLink, Source : Entity, ID : Comparabl val oldValue = getValue(o, unused) val existingValues = oldValue.mapIdToAdditionalValues() val existingIds = existingValues.keys - val additionalColumnsExist = additionalColumns.isNotEmpty() - if (additionalColumnsExist) { - entityCache.innerTableLinks[target.table]?.remove(o.id.value) - } entityCache.referrers[sourceColumn]?.remove(o.id) val targetValues = value.mapIdToAdditionalValues() val targetIds = targetValues.keys + val additionalColumnsExist = additionalColumns.isNotEmpty() + if (additionalColumnsExist) { + entityCache.innerTableLinks[targetColumn]?.get(o.id)?.removeAll { cached -> + targetValues[cached.id]?.any { (column, targetValue) -> + cached.getInnerTableLinkValue(column) != targetValue + } != false + } + } + executeAsPartOfEntityLifecycle { val deleteCondition = if (additionalColumnsExist) { val targetAdditionalValues = targetValues.map { it.value.values.toList() + it.key } @@ -184,51 +190,3 @@ class InnerTableLink, Source : Entity, ID : Comparabl /** Modifies this reference to sort entities by a column specified in [expression] using ascending order. **/ infix fun orderBy(expression: Expression<*>) = orderBy(listOf(expression to SortOrder.ASC)) } - -/** - * Base class for an [Entity] instance identified by a [wrapped] entity comprised of any ID value. - * - * Instances of this base class should be used when needing to represent referenced entities in a many-to-many relation - * from fields defined using `via`, which require additional columns in the intermediate table. These additional - * columns should be added as constructor properties and the property-column mapping should be defined by - * [getInnerTableLinkValue]. - * - * @param WID ID type of the [wrapped] entity instance. - * @property wrapped The referenced (parent) entity whose unique ID value identifies this [InnerTableLinkEntity] instance. - * @sample org.jetbrains.exposed.sql.tests.shared.entities.ViaTests.ProjectWithApproval - */ -abstract class InnerTableLinkEntity>(val wrapped: Entity) : Entity(wrapped.id) { - /** - * Returns the initial column-property mapping for an [InnerTableLinkEntity] instance - * before being flushed and inserted into the database. - * - * @sample org.jetbrains.exposed.sql.tests.shared.entities.ViaTests.ProjectWithApproval - */ - abstract fun getInnerTableLinkValue(column: Column<*>): Any? -} - -/** - * Base class representing the [EntityClass] that manages [InnerTableLinkEntity] instances and - * maintains their relation to the provided [table] of the wrapped entity. - * - * This should be used, as a companion object to [InnerTableLinkEntity], when needing to represent referenced entities - * in a many-to-many relation from fields defined using `via`, which require additional columns in the intermediate table. - * These additional columns will be retrieved as part of a queries [ResultRow] and the column-property mapping to create - * new instances should be defined by [createInstance]. - * - * @param WID ID type of the wrapped entity instance. - * @param E The [InnerTableLinkEntity] type that is managed by this class. - * @param [table] The [IdTable] object that stores rows mapped to the wrapped entity of this class. - * @sample org.jetbrains.exposed.sql.tests.shared.entities.ViaTests.ProjectWithApproval - */ -abstract class InnerTableLinkEntityClass, out E : InnerTableLinkEntity>( - table: IdTable -) : EntityClass(table, null, null) { - /** - * Creates a new [InnerTableLinkEntity] instance by using the provided [row] to both create the wrapped entity - * and any additional columns. - * - * @sample org.jetbrains.exposed.sql.tests.shared.entities.ViaTests.ProjectWithApproval - */ - abstract override fun createInstance(entityId: EntityID, row: ResultRow?): E -} diff --git a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/InnerTableLinkEntity.kt b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/InnerTableLinkEntity.kt new file mode 100644 index 0000000000..c847fd167e --- /dev/null +++ b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/InnerTableLinkEntity.kt @@ -0,0 +1,54 @@ +package org.jetbrains.exposed.dao + +import org.jetbrains.exposed.dao.id.EntityID +import org.jetbrains.exposed.dao.id.IdTable +import org.jetbrains.exposed.sql.Column +import org.jetbrains.exposed.sql.ResultRow + +/** + * Base class for an [Entity] instance identified by a [wrapped] entity comprised of any ID value. + * + * Instances of this base class should be used when needing to represent referenced entities in a many-to-many relation + * from fields defined using `via`, which require additional columns in the intermediate table. These additional + * columns should be added as constructor properties and the property-column mapping should be defined by + * [getInnerTableLinkValue]. + * + * @param WID ID type of the [wrapped] entity instance. + * @property wrapped The referenced (parent) entity whose unique ID value identifies this [InnerTableLinkEntity] instance. + * @sample org.jetbrains.exposed.sql.tests.shared.entities.ViaTests.ProjectWithApproval + */ +abstract class InnerTableLinkEntity>(val wrapped: Entity) : Entity(wrapped.id) { + /** + * Returns the initial column-property mapping for an [InnerTableLinkEntity] instance + * before being flushed and inserted into the database. + * + * @sample org.jetbrains.exposed.sql.tests.shared.entities.ViaTests.ProjectWithApproval + */ + abstract fun getInnerTableLinkValue(column: Column<*>): Any? +} + +/** + * Base class representing the [EntityClass] that manages [InnerTableLinkEntity] instances and + * maintains their relation to the provided [table] of the wrapped entity. + * + * This should be used, as a companion object to [InnerTableLinkEntity], when needing to represent referenced entities + * in a many-to-many relation from fields defined using `via`, which require additional columns in the intermediate table. + * These additional columns will be retrieved as part of a query's [ResultRow] and the column-property mapping to create + * new instances should be defined by [createInstance]. + * + * @param WID ID type of the wrapped entity instance. + * @param E The [InnerTableLinkEntity] type that is managed by this class. + * @param [table] The [IdTable] object that stores rows mapped to the wrapped entity of this class. + * @sample org.jetbrains.exposed.sql.tests.shared.entities.ViaTests.ProjectWithApproval + */ +abstract class InnerTableLinkEntityClass, out E : InnerTableLinkEntity>( + table: IdTable +) : EntityClass(table, null, null) { + /** + * Creates a new [InnerTableLinkEntity] instance by using the provided [row] to both create the wrapped entity + * and any additional columns. + * + * @sample org.jetbrains.exposed.sql.tests.shared.entities.ViaTests.ProjectWithApproval + */ + abstract override fun createInstance(entityId: EntityID, row: ResultRow?): E +} diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/ViaTest.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/ViaTest.kt index 44370639c0..bdb375a352 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/ViaTest.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/ViaTest.kt @@ -18,8 +18,6 @@ import org.junit.Test import java.sql.Connection import java.util.* import kotlin.reflect.jvm.isAccessible -import kotlin.test.assertFalse -import kotlin.test.assertTrue object ViaTestData { object NumbersTable : UUIDTable() { @@ -288,14 +286,17 @@ class ViaTests : DatabaseTestsBase() { } } - object Projects : IntIdTable("projects") { + // IdTable without auto-increment is used so manual ids can be inserted without excluding SQL Server + object Projects : IdTable("projects") { + override val id = integer("id").entityId() val name = varchar("name", 50) + override val primaryKey = PrimaryKey(id) } class Project(id: EntityID) : IntEntity(id) { companion object : IntEntityClass(Projects) var name by Projects.name - var tasks by TaskWithApproval via ProjectTasks + var tasks by TaskWithData via ProjectTasks } object ProjectTasks : Table("project_tasks") { @@ -307,26 +308,29 @@ class ViaTests : DatabaseTestsBase() { override val primaryKey = PrimaryKey(project, task) } - object Tasks : IntIdTable("tasks") { + // IdTable without auto-increment is used so manual ids can be inserted without excluding SQL Server + object Tasks : IdTable("tasks") { + override val id = integer("id").entityId() val title = varchar("title", 64) + override val primaryKey = PrimaryKey(id) } class Task(id: EntityID) : IntEntity(id) { companion object : IntEntityClass(Tasks) var title by Tasks.title - var projects by ProjectWithApproval via ProjectTasks + var projects by ProjectWithData via ProjectTasks } - class ProjectWithApproval( + class ProjectWithData( val project: Project, val approved: Boolean, val sprint: Int ) : InnerTableLinkEntity(project) { - companion object : InnerTableLinkEntityClass(Projects) { - override fun createInstance(entityId: EntityID, row: ResultRow?): ProjectWithApproval { + companion object : InnerTableLinkEntityClass(Projects) { + override fun createInstance(entityId: EntityID, row: ResultRow?): ProjectWithData { return row?.let { - ProjectWithApproval(Project.wrapRow(it), it[ProjectTasks.approved], it[ProjectTasks.sprint]) - } ?: ProjectWithApproval(Project(entityId), false, 0) + ProjectWithData(Project.wrapRow(it), it[ProjectTasks.approved], it[ProjectTasks.sprint]) + } ?: ProjectWithData(Project(entityId), false, 0) } } @@ -337,16 +341,16 @@ class ViaTests : DatabaseTestsBase() { } } - class TaskWithApproval( + class TaskWithData( val task: Task, val approved: Boolean, val sprint: Int ) : InnerTableLinkEntity(task) { - companion object : InnerTableLinkEntityClass(Tasks) { - override fun createInstance(entityId: EntityID, row: ResultRow?): TaskWithApproval { + companion object : InnerTableLinkEntityClass(Tasks) { + override fun createInstance(entityId: EntityID, row: ResultRow?): TaskWithData { return row?.let { - TaskWithApproval(Task.wrapRow(it), it[ProjectTasks.approved], it[ProjectTasks.sprint]) - } ?: TaskWithApproval(Task(entityId), false, 0) + TaskWithData(Task.wrapRow(it), it[ProjectTasks.approved], it[ProjectTasks.sprint]) + } ?: TaskWithData(Task(entityId), false, 0) } } @@ -358,51 +362,83 @@ class ViaTests : DatabaseTestsBase() { } @Test - fun testAdditionalLinkDataUsingInnerTableLinkEntities() { + fun testAdditionalLinkDataInsertAndUpdate() { withTables(Projects, Tasks, ProjectTasks) { - val p1 = Project.new { name = "Project 1" } - val p2 = Project.new { name = "Project 2" } - val t1 = Task.new { title = "Task 1" } - val t2 = Task.new { title = "Task 2" } - val t3 = Task.new { title = "Task 3" } + val p1 = Project.new(123) { name = "Project 1" } + val p2 = Project.new(456) { name = "Project 2" } + val t1 = Task.new(11) { title = "Task 1" } + val t2 = Task.new(22) { title = "Task 2" } + val t3 = Task.new(33) { title = "Task 3" } - p1.tasks = SizedCollection(TaskWithApproval(t1, false, 1)) - p2.tasks = SizedCollection(TaskWithApproval(t2, true, 2), TaskWithApproval(t3, false, 3)) + p1.tasks = SizedCollection(TaskWithData(t1, false, 1)) + p2.tasks = SizedCollection(TaskWithData(t2, true, 2), TaskWithData(t1, false, 3)) assertFalse(p1.tasks.single().approved) - p1.tasks = SizedCollection(TaskWithApproval(t1, true, 1)) + p1.tasks = SizedCollection(TaskWithData(t1, true, 1)) assertTrue(p1.tasks.single().approved) + assertEqualCollections(p2.tasks.map { it.task.id }, listOf(t2.id, t1.id)) + p2.tasks = SizedCollection(TaskWithData(t2, true, 2), TaskWithData(t3, false, 3)) + assertEqualCollections(p2.tasks.map { it.task.id }, listOf(t2.id, t3.id)) + } + } + + @Test + fun testAdditionalLinkDataLoadedOnParent() { + withTables(Projects, Tasks, ProjectTasks) { + val p1 = Project.new(123) { name = "Project 1" } + val p2 = Project.new(456) { name = "Project 2" } + val t1 = Task.new(11) { title = "Task 1" } + val t2 = Task.new(22) { title = "Task 2" } + val t3 = Task.new(33) { title = "Task 3" } + + p1.tasks = SizedCollection(TaskWithData(t1, false, 1)) + p2.tasks = SizedCollection(TaskWithData(t2, true, 2), TaskWithData(t3, false, 3)) + commit() - // test that all child entities set on the parent can be loaded by parent inTopLevelTransaction(Connection.TRANSACTION_SERIALIZABLE) { maxAttempts = 1 Project.all().with(Project::tasks) val cache = TransactionManager.current().entityCache - val p1Task = cache.getReferrers(p1.id, ProjectTasks.project)?.single() + val p1Task = cache.getReferrers(p1.id, ProjectTasks.project)?.single() assertEquals(t1.id, p1Task?.id) assertEquals(t1.id, p1Task?.task?.id) - assertEquals(true, p1Task?.approved) + assertEquals(false, p1Task?.approved) assertEquals(1, p1Task?.sprint) - val p2Tasks = cache.getReferrers(p2.id, ProjectTasks.project)?.toList().orEmpty() + val p2Tasks = cache.getReferrers(p2.id, ProjectTasks.project)?.toList().orEmpty() assertEqualLists(p2Tasks.map { it.id }, listOf(t2.id, t3.id)) assertEqualLists(p2Tasks.map { it.approved }, listOf(true, false)) assertEqualLists(p2Tasks.map { it.sprint }, listOf(2, 3)) } + } + } + + @Test + fun testAdditionalLinkDataLoadedOnChild() { + withTables(Projects, Tasks, ProjectTasks) { + val p1 = Project.new(123) { name = "Project 1" } + val p2 = Project.new(456) { name = "Project 2" } + val t1 = Task.new(11) { title = "Task 1" } + val t2 = Task.new(22) { title = "Task 2" } + val t3 = Task.new(33) { title = "Task 3" } + + p1.tasks = SizedCollection(TaskWithData(t1, false, 1)) + p2.tasks = SizedCollection(TaskWithData(t2, true, 2), TaskWithData(t3, false, 3)) + + commit() - // test that all parent entities can then be found by the child entity without setting again inTopLevelTransaction(Connection.TRANSACTION_SERIALIZABLE) { maxAttempts = 1 Task.all().with(Task::projects) val cache = TransactionManager.current().entityCache - val t1Project = cache.getReferrers(t1.id, ProjectTasks.task)?.single() + val t1Project = cache.getReferrers(t1.id, ProjectTasks.task)?.single() assertEquals(p1.id, t1Project?.id) assertEquals(p1.id, t1Project?.project?.id) - assertEquals(true, t1Project?.approved) + assertEquals(false, t1Project?.approved) assertEquals(1, t1Project?.sprint) } }