From cd275341ad989709b807cda0585ae25e048581b7 Mon Sep 17 00:00:00 2001 From: bog-walk <82039410+bog-walk@users.noreply.github.com> Date: Tue, 2 Jul 2024 15:24:52 -0400 Subject: [PATCH] fix: EXPOSED-407 compositeMoney() nullability definition issues (#2137) * fix: EXPOSED-407 compositeMoney() nullability definition issues Defining a composite money column as nullable returns a different type, as each component column is made nullable, but not the actual expression itself. This fix adds an overload that returns the expected type and ensures that the expression is also made to be nullable overall. Tests for using the component columns individually have also been included. --- exposed-core/api/exposed-core.api | 3 +- .../jetbrains/exposed/sql/CompositeColumn.kt | 6 +- .../kotlin/org/jetbrains/exposed/sql/Table.kt | 1 + exposed-money/api/exposed-money.api | 4 +- .../exposed/sql/money/CompositeMoneyColumn.kt | 24 +++- .../sql/money/CompositeMoneyColumnType.kt | 14 +- .../jetbrains/exposed/sql/money/MoneyTests.kt | 120 ++++++++++++++---- 7 files changed, 137 insertions(+), 35 deletions(-) diff --git a/exposed-core/api/exposed-core.api b/exposed-core/api/exposed-core.api index 5371ee0415..9b9b8ddc8f 100644 --- a/exposed-core/api/exposed-core.api +++ b/exposed-core/api/exposed-core.api @@ -241,7 +241,8 @@ public final class org/jetbrains/exposed/sql/Between : org/jetbrains/exposed/sql } public abstract class org/jetbrains/exposed/sql/BiCompositeColumn : org/jetbrains/exposed/sql/CompositeColumn { - public fun (Lorg/jetbrains/exposed/sql/Column;Lorg/jetbrains/exposed/sql/Column;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;)V + public fun (Lorg/jetbrains/exposed/sql/Column;Lorg/jetbrains/exposed/sql/Column;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;Z)V + public synthetic fun (Lorg/jetbrains/exposed/sql/Column;Lorg/jetbrains/exposed/sql/Column;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V protected final fun getColumn1 ()Lorg/jetbrains/exposed/sql/Column; protected final fun getColumn2 ()Lorg/jetbrains/exposed/sql/Column; public fun getRealColumns ()Ljava/util/List; diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/CompositeColumn.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/CompositeColumn.kt index 99f5af821c..94b78c24ea 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/CompositeColumn.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/CompositeColumn.kt @@ -39,8 +39,12 @@ abstract class BiCompositeColumn( /** Transformation that receives the column's composite value and returns the parsed values of the underlying columns. */ val transformFromValue: (T) -> Pair, /** Transformation that receives the retrieved values of [column1] and [column2] and returns a composite value. */ - val transformToValue: (Any?, Any?) -> T + val transformToValue: (Any?, Any?) -> T, + nullable: Boolean = false ) : CompositeColumn() { + init { + this.nullable = nullable + } override fun getRealColumns(): List> = listOf(column1, column2) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt index 17b6885447..d8ce613a11 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt @@ -1176,6 +1176,7 @@ open class Table(name: String = "") : ColumnSet(), DdlAware { /** Marks this [CompositeColumn] as nullable. */ @Suppress("UNCHECKED_CAST") + @LowPriorityInOverloadResolution fun > C.nullable(): CompositeColumn = apply { nullable = true getRealColumns().filter { !it.columnType.nullable }.forEach { (it as Column).nullable() } diff --git a/exposed-money/api/exposed-money.api b/exposed-money/api/exposed-money.api index c0e7fe1c98..5966245823 100644 --- a/exposed-money/api/exposed-money.api +++ b/exposed-money/api/exposed-money.api @@ -1,11 +1,13 @@ public final class org/jetbrains/exposed/sql/money/CompositeMoneyColumn : org/jetbrains/exposed/sql/BiCompositeColumn { - public fun (Lorg/jetbrains/exposed/sql/Column;Lorg/jetbrains/exposed/sql/Column;)V + public fun (Lorg/jetbrains/exposed/sql/Column;Lorg/jetbrains/exposed/sql/Column;Z)V + public synthetic fun (Lorg/jetbrains/exposed/sql/Column;Lorg/jetbrains/exposed/sql/Column;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun getAmount ()Lorg/jetbrains/exposed/sql/Column; public final fun getCurrency ()Lorg/jetbrains/exposed/sql/Column; } public final class org/jetbrains/exposed/sql/money/CompositeMoneyColumnKt { public static final fun CompositeMoneyColumn (Lorg/jetbrains/exposed/sql/Table;IILjava/lang/String;Ljava/lang/String;)Lorg/jetbrains/exposed/sql/money/CompositeMoneyColumn; + public static final fun nullable (Lorg/jetbrains/exposed/sql/money/CompositeMoneyColumn;)Lorg/jetbrains/exposed/sql/money/CompositeMoneyColumn; } public final class org/jetbrains/exposed/sql/money/CompositeMoneyColumnTypeKt { diff --git a/exposed-money/src/main/kotlin/org/jetbrains/exposed/sql/money/CompositeMoneyColumn.kt b/exposed-money/src/main/kotlin/org/jetbrains/exposed/sql/money/CompositeMoneyColumn.kt index 633fdc069f..b32e26c9cf 100644 --- a/exposed-money/src/main/kotlin/org/jetbrains/exposed/sql/money/CompositeMoneyColumn.kt +++ b/exposed-money/src/main/kotlin/org/jetbrains/exposed/sql/money/CompositeMoneyColumn.kt @@ -10,11 +10,15 @@ import javax.money.Monetary import javax.money.MonetaryAmount /** - * Represents amount of money and currency using Java Money API. Data are stored using two composite columns. + * Represents amount of money and currency using Java Money API. Data is stored using two composite columns. * * @author Vladislav Kisel */ -class CompositeMoneyColumn(val amount: Column, val currency: Column) : +class CompositeMoneyColumn( + val amount: Column, + val currency: Column, + nullable: Boolean = false +) : BiCompositeColumn( column1 = amount, column2 = currency, @@ -36,12 +40,26 @@ class CompositeMoneyColumn( amount = Column(table, amountName, DecimalColumnType(precision, scale)), currency = Column(table, currencyName, CurrencyColumnType()) ) + +/** Marks this [CompositeMoneyColumn] as nullable. */ +@Suppress("UNCHECKED_CAST") +fun CompositeMoneyColumn.nullable(): CompositeMoneyColumn { + return with(amount.table) { + (this@nullable as BiCompositeColumn).nullable() as CompositeMoneyColumn + } +} diff --git a/exposed-money/src/main/kotlin/org/jetbrains/exposed/sql/money/CompositeMoneyColumnType.kt b/exposed-money/src/main/kotlin/org/jetbrains/exposed/sql/money/CompositeMoneyColumnType.kt index e3901eb455..728870f287 100644 --- a/exposed-money/src/main/kotlin/org/jetbrains/exposed/sql/money/CompositeMoneyColumnType.kt +++ b/exposed-money/src/main/kotlin/org/jetbrains/exposed/sql/money/CompositeMoneyColumnType.kt @@ -16,7 +16,11 @@ import javax.money.MonetaryAmount fun Table.compositeMoney(precision: Int, scale: Int, amountName: String, currencyName: String = amountName + "_C") = registerCompositeColumn(CompositeMoneyColumn(this, precision, scale, amountName, currencyName)) -/** Creates a composite column made up of a decimal column and a currency column. */ +/** + * Creates a composite column made up of a decimal column and a currency column. + * + * @sample org.jetbrains.exposed.sql.money.MoneyBaseTest.testUsingManualCompositeMoneyColumns + */ fun Table.compositeMoney( amountColumn: Column, currencyColumn: Column @@ -28,13 +32,17 @@ fun Table.compositeMoney( } } -/** Creates a composite column made up of a nullable decimal column and a nullable currency column. */ +/** + * Creates a composite column made up of a nullable decimal column and a nullable currency column. + * + * @sample org.jetbrains.exposed.sql.money.MoneyBaseTest.testUsingManualCompositeMoneyColumns + */ @JvmName("compositeMoneyNullable") fun Table.compositeMoney( amountColumn: Column, currencyColumn: Column ): CompositeMoneyColumn { - return CompositeMoneyColumn(amountColumn, currencyColumn).also { + return CompositeMoneyColumn(amountColumn, currencyColumn, true).also { if (amountColumn !in columns && currencyColumn !in columns) { registerCompositeColumn(it) } diff --git a/exposed-money/src/test/kotlin/org/jetbrains/exposed/sql/money/MoneyTests.kt b/exposed-money/src/test/kotlin/org/jetbrains/exposed/sql/money/MoneyTests.kt index cf5bfd7cf2..3d533cbe9f 100644 --- a/exposed-money/src/test/kotlin/org/jetbrains/exposed/sql/money/MoneyTests.kt +++ b/exposed-money/src/test/kotlin/org/jetbrains/exposed/sql/money/MoneyTests.kt @@ -12,10 +12,10 @@ import org.jetbrains.exposed.sql.tests.DatabaseTestsBase import org.jetbrains.exposed.sql.tests.TestDB import org.jetbrains.exposed.sql.tests.shared.assertEquals import org.jetbrains.exposed.sql.tests.shared.expectException -import org.junit.Ignore import org.junit.Test import java.math.BigDecimal import javax.money.CurrencyUnit +import javax.money.Monetary import javax.money.MonetaryAmount private const val AMOUNT_SCALE = 5 @@ -24,29 +24,48 @@ open class MoneyBaseTest : DatabaseTestsBase() { @Test fun testInsertSelectMoney() { - testInsertedAndSelect(Money.of(BigDecimal.TEN, "USD")) + withTables(Account) { + assertInsertOfCompositeValueReturnsEquivalentOnSelect(Money.of(BigDecimal.TEN, "USD")) + Account.deleteAll() + assertInsertOfComponentValuesReturnsEquivalentOnSelect(Money.of(BigDecimal.TEN, "USD")) + } } @Test fun testInsertSelectFloatingMoney() { - testInsertedAndSelect(Money.of(BigDecimal("0.12345"), "USD")) + withTables(Account) { + assertInsertOfCompositeValueReturnsEquivalentOnSelect(Money.of(BigDecimal("0.12345"), "USD")) + Account.deleteAll() + assertInsertOfComponentValuesReturnsEquivalentOnSelect(Money.of(BigDecimal("0.12345"), "USD")) + } } @Test - @Ignore // TODO not supported yet fun testInsertSelectNull() { - testInsertedAndSelect(null) + withTables(Account) { + assertInsertOfCompositeValueReturnsEquivalentOnSelect(null) + Account.deleteAll() + assertInsertOfComponentValuesReturnsEquivalentOnSelect(null) + } } @Test fun testInsertSelectOutOfLength() { - val toInsert = Money.of(BigDecimal.valueOf(12345678901), "CZK") + val amount = BigDecimal.valueOf(12345678901) + val toInsert = Money.of(amount, "CZK") withTables(excludeSettings = listOf(TestDB.SQLITE), Account) { expectException { - val accountID = Account.insertAndGetId { + Account.insertAndGetId { it[composite_money] = toInsert } } + + expectException { + Account.insertAndGetId { + it[composite_money.amount] = amount + it[composite_money.currency] = toInsert.currency + } + } } } @@ -78,40 +97,89 @@ open class MoneyBaseTest : DatabaseTestsBase() { } @Test - fun testNullableCompositeColumnInsertAndSelect() { - val table = object : IntIdTable("CompositeTable") { - val composite_money = compositeMoney(8, AMOUNT_SCALE, "composite_money").nullable() + fun testUsingManualCompositeMoneyColumns() { + val tester = object : Table("tester") { + val money = compositeMoney( + decimal("amount", 8, AMOUNT_SCALE), + currency("currency") + ) + val nullableMoney = compositeMoney( + decimal("nullable_amount", 8, AMOUNT_SCALE).nullable(), + currency("nullable_currency").nullable() + ) } - withTables(table) { - val id = table.insertAndGetId { - it[composite_money] = null + withTables(tester) { + val amount = BigDecimal(99).setScale(AMOUNT_SCALE) + val currencyUnit = Monetary.getCurrency("EUR") + tester.insert { + it[money.amount] = amount + it[money.currency] = currencyUnit + it[nullableMoney.amount] = null + it[nullableMoney.currency] = null } - val resultRow = table.selectAll().where { table.id.eq(id) }.single() - val result = resultRow[table.composite_money] + val result1 = tester + .selectAll() + .where { tester.nullableMoney.amount.isNull() and tester.nullableMoney.currency.isNull() } + .single() + assertEquals(amount, result1[tester.money.amount]) - assertEquals(null, result) + tester.update { + it[tester.nullableMoney.amount] = amount + it[tester.nullableMoney.currency] = currencyUnit + } + + val result2 = tester + .select(tester.money.currency, tester.nullableMoney.currency) + .where { tester.money.amount.isNotNull() and tester.nullableMoney.amount.isNotNull() } + .single() + assertEquals(currencyUnit, result2[tester.money.currency]) + assertEquals(currencyUnit, result2[tester.nullableMoney.currency]) + + // manual composite columns should still accept composite values + val compositeMoney = Money.of(BigDecimal(10), "CAD") + tester.insert { + it[money] = compositeMoney + it[nullableMoney] = null + } + tester.insert { + it[money] = compositeMoney + } + + assertEquals(2, tester.selectAll().where { tester.nullableMoney eq null }.count()) } } - private fun testInsertedAndSelect(toInsert: Money?) { - withTables(Account) { - val accountID = Account.insertAndGetId { - it[composite_money] = toInsert!! - } + private fun Transaction.assertInsertOfCompositeValueReturnsEquivalentOnSelect(toInsert: Money?) { + val accountID = Account.insertAndGetId { + it[composite_money] = toInsert + } + + val single = Account.select(Account.composite_money).where { Account.id.eq(accountID) }.single() + val inserted = single[Account.composite_money] - val single = Account.select(Account.composite_money).where { Account.id.eq(accountID) }.single() - val inserted = single[Account.composite_money] + assertEquals(toInsert, inserted) + } - assertEquals(toInsert, inserted) + private fun Transaction.assertInsertOfComponentValuesReturnsEquivalentOnSelect(toInsert: Money?) { + val amount: BigDecimal? = toInsert?.numberStripped?.setScale(AMOUNT_SCALE) + val currencyUnit: CurrencyUnit? = toInsert?.currency + val accountID = Account.insertAndGetId { + it[composite_money.amount] = amount + it[composite_money.currency] = currencyUnit } + + val single = Account.select(Account.composite_money).where { Account.id eq accountID }.single() + + assertEquals(amount, single[Account.composite_money.amount]) + assertEquals(currencyUnit, single[Account.composite_money.currency]) } } class AccountDao(id: EntityID) : IntEntity(id) { - val money: MonetaryAmount by Account.composite_money + val money: MonetaryAmount? by Account.composite_money val currency: CurrencyUnit? by Account.composite_money.currency @@ -122,5 +190,5 @@ class AccountDao(id: EntityID) : IntEntity(id) { object Account : IntIdTable("AccountTable") { - val composite_money = compositeMoney(8, AMOUNT_SCALE, "composite_money") + val composite_money = compositeMoney(8, AMOUNT_SCALE, "composite_money").nullable() }