Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions documentation-website/Writerside/topics/Breaking-Changes.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Breaking Changes

## 1.0.0-rc-2

* The method `references()` with `EntityID` ref parameter changed the signature from
`fun <T : Any, S : T, C : Column<S>> C.references(ref: Column<EntityID<T>>, ...): C` to
`fun <T : Any, C : Column<T>> C.references(ref: Column<EntityID<T>>, ...): Column<EntityID<T>>`. It's done to align signature and behaviour of `references()` method
with `reference()` method.

Comment on lines +3 to +9

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, the current setup is intentional. reference() and Column.references() are not meant to be 100% equivalent or to be aligned exactly.

Column.references() exists for users that want to keep the column they have defined/registered, but only additionally create a foreign key on table DDL.
There could be DSL users who are choosing to use IntIdTable for convenience but don't want to deal with EntityID any more than necessary and who purposefully want to strip the entityID wrapper from the referenced column.

I think there could be a lot of people who want the original distinction to remain:

// registers a new integer column + create FK
val city: Column<Int> = integer("city_id").references(Cities.id)

// registers an entityID column that matches reference + create FK
val city: Column<EntityID<Int>> = reference("city_id", Cities.id)

My question is:

If the user is relying on DAO, and therefore expects the cache to be utilised to minimise db queries, why are they using .references() instead of reference()?
If there is no reason, maybe we should be making it clear in documentation that .references() does not involve the entity cache?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chantal Loncle (@bog-walk) Thank you, it makes sense,

in this case it could be better update documentation,

but in this case I see the problem that Exposed allows to create reference for entity on such a column, and allows to fetch entities using with(), but makes extra queries in this case, and there is no way for the user that it would happen without checking the logs, and it's easy to mix reference() and references() within code.

I will check, probably it could be easy to prevent usage of referrersOn() on columns that have no EntityID<T> in type.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, thank you for taking care of this issue as an issue reporter.

I didn't expect the references() method to affect the cache behaviour either and had to look at the code to analyse the cause. As mentioned above, If the references() method is not intended to wrap EntityID, It would be helpful to have clear documentation stating that cached entities will not be used for reference columns with references().

However, as obabichevjb said, even with documentation, the behavior of references() appears fine on the surface, and the issue is only apparent in the logs. Therefore, I think it's better to be more defensive to prevent user mistakes.

## 1.0.0-rc-1

* `exposed-migration` artifact has been replaced with `exposed-migration-core` to hold core common schema migration functionality across both available drivers.
Expand Down
25 changes: 13 additions & 12 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/v1/core/Table.kt
Original file line number Diff line number Diff line change
Expand Up @@ -664,14 +664,19 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {

/** Creates an [EntityID] column, with the specified [name], for storing the same objects as the specified [originalColumn]. */
fun <ID : Any> entityId(name: String, originalColumn: Column<ID>): Column<EntityID<ID>> {
return createEntityIdColumn(name, originalColumn)
.also {
_columns.addColumn(it)
}
}

private fun <ID : Any> createEntityIdColumn(name: String, originalColumn: Column<ID>): Column<EntityID<ID>> {
val columnTypeCopy = originalColumn.columnType.cloneAsBaseType()
val answer = Column<EntityID<ID>>(
return Column<EntityID<ID>>(
this,
name,
EntityIDColumnType(Column<ID>(originalColumn.table, name, columnTypeCopy))
)
_columns.addColumn(answer)
return answer
}

/** Creates an [EntityID] column, with the specified [name], for storing the identifier of the specified [table]. */
Expand Down Expand Up @@ -1137,19 +1142,15 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {
* @sample org.jetbrains.exposed.v1.tests.shared.ddl.CreateMissingTablesAndColumnsTests.ExplicitTable
*/
@JvmName("referencesById")
fun <T : Any, S : T, C : Column<S>> C.references(
fun <T : Any, C : Column<T>> C.references(
ref: Column<EntityID<T>>,
onDelete: ReferenceOption? = null,
onUpdate: ReferenceOption? = null,
fkName: String? = null
): C = apply {
this.foreignKey = ForeignKeyConstraint(
target = ref,
from = this,
onUpdate = onUpdate,
onDelete = onDelete,
name = fkName
)
): Column<EntityID<T>> {
val entityIdColumn = createEntityIdColumn(name, (ref.columnType as EntityIDColumnType<T>).idColumn)
replaceColumn(this, entityIdColumn)
return entityIdColumn.references(ref, onDelete, onUpdate, fkName)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import org.jetbrains.exposed.v1.core.DatabaseConfig
import org.jetbrains.exposed.v1.core.Key
import org.jetbrains.exposed.v1.core.Schema
import org.jetbrains.exposed.v1.core.Table
import org.jetbrains.exposed.v1.core.Transaction
import org.jetbrains.exposed.v1.core.statements.StatementContext
import org.jetbrains.exposed.v1.core.statements.StatementInterceptor
import org.jetbrains.exposed.v1.core.transactions.nullableTransactionScope
import org.jetbrains.exposed.v1.jdbc.JdbcTransaction
Expand Down Expand Up @@ -207,4 +209,30 @@ abstract class DatabaseTestsBase {
quota = "20M",
on = "USERS"
)

interface Counter {
var count: Int
fun inc()
fun reset()
}

protected fun JdbcTransaction.executionsCounter(): Counter {
val counter = object : Counter {
override var count = 0
override fun inc() {
count++
}

override fun reset() {
count = 0
}
}
registerInterceptor(object : StatementInterceptor {
override fun beforeExecution(transaction: Transaction, context: StatementContext) {
counter.inc()
}
})

return counter
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package org.jetbrains.exposed.v1.tests.shared.entities

import org.jetbrains.exposed.v1.core.ReferenceOption
import org.jetbrains.exposed.v1.core.dao.id.EntityID
import org.jetbrains.exposed.v1.core.dao.id.IntIdTable
import org.jetbrains.exposed.v1.core.dao.id.LongIdTable
import org.jetbrains.exposed.v1.core.eq
import org.jetbrains.exposed.v1.dao.IntEntity
import org.jetbrains.exposed.v1.dao.IntEntityClass
import org.jetbrains.exposed.v1.dao.LongEntity
import org.jetbrains.exposed.v1.dao.LongEntityClass
import org.jetbrains.exposed.v1.dao.with
import org.jetbrains.exposed.v1.jdbc.insertAndGetId
import org.jetbrains.exposed.v1.tests.DatabaseTestsBase
import kotlin.test.Test
import kotlin.test.assertEquals

class EntityReferrersTests : DatabaseTestsBase() {

object AlertItemTable : IntIdTable("alert_item") {
val isAlarm = bool("is_alarm").default(true)
}

class AlertItemEntity(id: EntityID<Int>) : IntEntity(id) {
companion object : IntEntityClass<AlertItemEntity>(AlertItemTable)

val bids by ItemBidEntity.referrersOn(ItemBidTable.alertItemId, cache = true)
}

object ItemBidTable : LongIdTable("item_bid") {
val alertItemId = integer("alert_item_id").references(AlertItemTable.id, onDelete = ReferenceOption.CASCADE)
}

class ItemBidEntity(id: EntityID<Long>) : LongEntity(id) {
companion object : LongEntityClass<ItemBidEntity>(ItemBidTable)
}

@Test
fun testCacheIsUsedWithReference() {
withTables(AlertItemTable, ItemBidTable) {
repeat(3) {
val itemId = AlertItemTable.insertAndGetId {
it[isAlarm] = true
}
repeat(5) {
ItemBidTable.insertAndGetId {
it[alertItemId] = itemId.value
}
}
}

val counter = executionsCounter()

AlertItemEntity
.find { AlertItemTable.isAlarm eq true }
.with(AlertItemEntity::bids)

assertEquals(2, counter.count, "'find()' must execute exactly 2 statements. One to fetch items, another one to fetch all the bids")
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package org.jetbrains.exposed.v1.tests.shared.entities

import org.jetbrains.exposed.v1.core.Column
import org.jetbrains.exposed.v1.core.dao.id.EntityID
import org.jetbrains.exposed.v1.core.dao.id.LongIdTable
Expand Down Expand Up @@ -36,7 +37,7 @@ object LongIdTables {
}

object Towns : LongIdTable("towns") {
val cityId: Column<Long> = long("city_id").references(Cities.id)
val cityId: Column<EntityID<Long>> = long("city_id").references(Cities.id)
}

class Town(id: EntityID<Long>) : LongEntity(id) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package org.jetbrains.exposed.v1.tests.shared.entities

import org.jetbrains.exposed.v1.core.Column
import org.jetbrains.exposed.v1.core.dao.id.EntityID
import org.jetbrains.exposed.v1.core.dao.id.UIntIdTable
Expand Down Expand Up @@ -136,7 +137,7 @@ object UIntIdTables {
}

object Towns : UIntIdTable("towns") {
val cityId: Column<UInt> = uinteger("city_id").references(Cities.id)
val cityId: Column<EntityID<UInt>> = uinteger("city_id").references(Cities.id)
}

class Town(id: EntityID<UInt>) : UIntEntity(id) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package org.jetbrains.exposed.v1.tests.shared.entities

import org.jetbrains.exposed.v1.core.Column
import org.jetbrains.exposed.v1.core.dao.id.EntityID
import org.jetbrains.exposed.v1.core.dao.id.ULongIdTable
Expand Down Expand Up @@ -136,7 +137,7 @@ object ULongIdTables {
}

object Towns : ULongIdTable("towns") {
val cityId: Column<ULong> = ulong("city_id").references(Cities.id)
val cityId: Column<EntityID<ULong>> = ulong("city_id").references(Cities.id)
}

class Town(id: EntityID<ULong>) : ULongEntity(id) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package org.jetbrains.exposed.v1.tests.shared.entities

import org.jetbrains.exposed.v1.core.Column
import org.jetbrains.exposed.v1.core.dao.id.EntityID
import org.jetbrains.exposed.v1.core.dao.id.UUIDTable
Expand Down Expand Up @@ -52,7 +53,7 @@ object UUIDTables {
}

object Towns : UUIDTable("towns") {
val cityId: Column<UUID> = uuid("city_id").references(Cities.id)
val cityId: Column<EntityID<UUID>> = uuid("city_id").references(Cities.id)
}

class Town(id: EntityID<UUID>) : UUIDEntity(id) {
Expand Down
Loading