Skip to content

Commit

Permalink
Implemented workaround for Sql2o ID type misdetection bug. Closes #8
Browse files Browse the repository at this point in the history
  • Loading branch information
mvysny committed Jan 18, 2019
1 parent 01a8a4d commit fb9fb71
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 20 deletions.
44 changes: 43 additions & 1 deletion src/main/kotlin/com/github/vokorm/DB.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import org.sql2o.converters.ConvertersProvider
import org.sql2o.quirks.NoQuirks
import org.sql2o.quirks.Quirks
import org.sql2o.quirks.QuirksProvider
import org.sql2o.reflection.PojoMetadata
import java.io.Closeable
import java.sql.ResultSet
import java.sql.Timestamp
import java.time.Instant
import java.time.LocalDate
Expand Down Expand Up @@ -166,7 +168,47 @@ private class MysqlUuidConverter : Converter<UUID> {
* Works around MySQL drivers not able to convert [UUID] to [ByteArray].
* See https://github.com/mvysny/vok-orm/issues/8 for more details.
*/
class MysqlQuirks : NoQuirks(mapOf(UUID::class.java to MysqlUuidConverter()))
class MysqlQuirks : NoQuirks(mapOf(UUID::class.java to MysqlUuidConverter())) {

override fun <E : Any?> converterOf(ofClass: Class<E>): Converter<E>? {
if (Entity::class.java.isAssignableFrom(ofClass)) {
currentEntity.set(ofClass)
}
return super.converterOf(ofClass)
}

override fun getRSVal(rs: ResultSet, idx: Int): Any? {
val rsval: Any? = super.getRSVal(rs, idx)
// here the issue is that Sql2o may misdetect the type of the ID column as Object
// that is because there are two setId() methods on every Entity:
// 1. setId(Object); and
// 2. setId(T);
// depending on the order of methods in Java reflection, Sql2o may pick the incorrect one
// and may store it into its PojoMetadata.
// I failed to hook into PojoMetadata and fix the type there.
//
// This is just a dumb workaround: I'll simply run the converter myself.
if (rsval != null) {
val e = currentEntity.get()!!
val isIdColumn = e.entityMeta.idProperty.dbColumnName == getColumnName(rs.metaData, idx)
if (isIdColumn) {
val metadata = PojoMetadata(e, false, false, mapOf(), true)!!
val isIdTypeMisdetected = metadata.getPropertySetter(e.entityMeta.idProperty.name).type == Object::class.java
if (isIdTypeMisdetected) {
val converter = converterOf(e.entityMeta.idProperty.valueType)
if (converter != null) {
return converter.convert(rsval)
}
}
}
}
return rsval
}

companion object {
private val currentEntity = ThreadLocal<Class<*>>()
}
}

/**
* Provides specialized quirks for MySQL. See [MysqlQuirks] for more details.
Expand Down
23 changes: 13 additions & 10 deletions src/test/kotlin/com/github/vokorm/Databases.kt
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,17 @@ data class EntityWithAliasedId(
/**
* A table demoing natural person with government-issued ID (birth number, social security number, etc).
*/
data class NaturalPerson(override var id: String? = null, var name: String) : Entity<String> {
data class NaturalPerson(override var id: String? = null, var name: String, var bytes: ByteArray) : Entity<String> {
companion object : Dao<NaturalPerson>
}

/**
* Demoes app-generated UUID ids.
*
* Warning: do NOT add any additional fields in here, since that would mysteriously make the compiler generate
* `void setId(UUID)` instead of `void setId(Object)` and we wouldn't test the metadata hook that fixes this issue.
*/
data class LogRecord(override var id: UUID?, var text: String, var bytes: ByteArray) : Entity<UUID> {
data class LogRecord(override var id: UUID?, var text: String) : Entity<UUID> {
companion object : Dao<LogRecord>
}

Expand Down Expand Up @@ -104,8 +107,8 @@ private fun DynaNodeGroup.usingDockerizedPosgresql() {
maritalStatus varchar(200)
)""")
ddl("""create table if not exists EntityWithAliasedId(myid bigserial primary key, name varchar(400) not null)""")
ddl("""create table if not exists NaturalPerson(id varchar(10) primary key, name varchar(400) not null)""")
ddl("""create table if not exists LogRecord(id UUID primary key, text varchar(400) not null, bytes bytea not null)""")
ddl("""create table if not exists NaturalPerson(id varchar(10) primary key, name varchar(400) not null, bytes bytea not null)""")
ddl("""create table if not exists LogRecord(id UUID primary key, text varchar(400) not null)""")
}
}

Expand Down Expand Up @@ -146,8 +149,8 @@ fun DynaNodeGroup.usingDockerizedMysql() {
maritalStatus varchar(200)
)""")
ddl("""create table if not exists EntityWithAliasedId(myid bigint primary key auto_increment, name varchar(400) not null)""")
ddl("""create table if not exists NaturalPerson(id varchar(10) primary key, name varchar(400) not null)""")
ddl("""create table if not exists LogRecord(id binary(16) primary key, text varchar(400) not null, bytes binary(16) not null)""")
ddl("""create table if not exists NaturalPerson(id varchar(10) primary key, name varchar(400) not null, bytes binary(16) not null)""")
ddl("""create table if not exists LogRecord(id binary(16) primary key, text varchar(400) not null)""")
}
}

Expand Down Expand Up @@ -190,8 +193,8 @@ fun DynaNodeGroup.usingH2Database() {
maritalStatus varchar
)""")
ddl("""create table EntityWithAliasedId(myid bigint primary key auto_increment, name varchar not null)""")
ddl("""create table if not exists NaturalPerson(id varchar(10) primary key, name varchar(400) not null)""")
ddl("""create table if not exists LogRecord(id UUID primary key, text varchar(400) not null, bytes binary(16) not null)""")
ddl("""create table NaturalPerson(id varchar(10) primary key, name varchar(400) not null, bytes binary(16) not null)""")
ddl("""create table LogRecord(id UUID primary key, text varchar(400) not null)""")
}
}
afterEach {
Expand Down Expand Up @@ -229,8 +232,8 @@ private fun DynaNodeGroup.usingDockerizedMariaDB() {
)"""
)
ddl("""create table if not exists EntityWithAliasedId(myid bigint primary key auto_increment, name varchar(400) not null)""")
ddl("""create table if not exists NaturalPerson(id varchar(10) primary key, name varchar(400) not null)""")
ddl("""create table if not exists LogRecord(id binary(16) primary key, text varchar(400) not null, bytes binary(16) not null)""")
ddl("""create table if not exists NaturalPerson(id varchar(10) primary key, name varchar(400) not null, bytes binary(16) not null)""")
ddl("""create table if not exists LogRecord(id binary(16) primary key, text varchar(400) not null)""")
}
}

Expand Down
19 changes: 10 additions & 9 deletions src/test/kotlin/com/github/vokorm/MappingTest.kt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@file:Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN")

package com.github.vokorm

import com.github.mvysny.dynatest.DynaTest
Expand Down Expand Up @@ -122,52 +124,51 @@ class MappingTest : DynaTest({
}
group("NaturalPerson") {
test("save fails") {
val p = NaturalPerson(id = "12345678", name = "Albedo")
val p = NaturalPerson(id = "12345678", name = "Albedo", bytes = byteArrayOf(5))
expectThrows(IllegalStateException::class, message = "We expected to update only one row but we updated 0 - perhaps there is no row with id 12345678?") {
p.save()
}
}
test("Save") {
val p = NaturalPerson(id = "12345678", name = "Albedo")
val p = NaturalPerson(id = "12345678", name = "Albedo", bytes = byteArrayOf(5))
p.create()
expectList("Albedo") { NaturalPerson.findAll().map { it.name } }
p.name = "Rubedo"
p.save()
expectList("Rubedo") { NaturalPerson.findAll().map { it.name } }
NaturalPerson(id = "aaa", name = "Nigredo").create()
NaturalPerson(id = "aaa", name = "Nigredo", bytes = byteArrayOf(5)).create()
expectList("Rubedo", "Nigredo") { NaturalPerson.findAll().map { it.name } }
}
test("delete") {
val p = NaturalPerson(id = "foo", name = "Albedo")
val p = NaturalPerson(id = "foo", name = "Albedo", bytes = byteArrayOf(5))
p.create()
p.delete()
expectList() { NaturalPerson.findAll() }
}
}
group("LogRecord") {
test("save fails") {
val p = LogRecord(id = UUID.randomUUID(), text = "foo", bytes = byteArrayOf(5))
val p = LogRecord(id = UUID.randomUUID(), text = "foo")
expectThrows(IllegalStateException::class, message = "We expected to update only one row but we updated 0 - perhaps there is no row with id") {
p.save()
}
}
test("Save") {
val p = LogRecord(id = UUID.randomUUID(), text = "Albedo", bytes = byteArrayOf(5))
val p = LogRecord(id = UUID.randomUUID(), text = "Albedo")
p.create()
expectList("Albedo") { LogRecord.findAll().map { it.text } }
p.text = "Rubedo"
p.save()
expectList("Rubedo") { LogRecord.findAll().map { it.text } }
LogRecord(id = UUID.randomUUID(), text = "Nigredo", bytes = byteArrayOf(5)).create()
LogRecord(id = UUID.randomUUID(), text = "Nigredo").create()
expect(setOf("Rubedo", "Nigredo")) { LogRecord.findAll().map { it.text } .toSet() }
}
test("delete") {
val p = LogRecord(id = UUID.randomUUID(), text = "foo", bytes = byteArrayOf(5))
val p = LogRecord(id = UUID.randomUUID(), text = "foo")
p.create()
p.delete()
expectList() { LogRecord.findAll() }
}
}
}
})

0 comments on commit fb9fb71

Please sign in to comment.