Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Registering KotlinMapper with a prefix does not seems to be returning correct results #1944

Closed
mahesh-chotrani opened this issue Oct 28, 2021 · 5 comments

Comments

@mahesh-chotrani
Copy link

mahesh-chotrani commented Oct 28, 2021

While using KotlinMapper, when data is being fetched from more than one tables, and few column names are same from both the tables (like, id), the result is not populated correctly in the data classes.

Here is how a sample query looks like (id column is same in both the tables)

SELECT 
  t.id, t.name, 
  p.id pid, p.tagId ptagId, p.primaryName pprimaryName 
FROM Tag t JOIN Product P ON t.id = p.TagId

Tag and Product data classes

data class Tag(
    val id: Int?,
    val name: String?
)
data class Product(
    val id: Int?,
    val primaryName: String?,
    val tagId: Int?,
)

And the code snippet

val rows = JdbiManager.jdbi.withHandleSync { handle ->
    handle.registerRowMapper(KotlinMapper(Tag::class.java))
    handle.registerRowMapper(KotlinMapper(Product::class.java, "p"))
    handle.registerRowMapper(JoinRowMapper.forTypes(Tag::class.java, Product::class.java))
    handle.createQuery(sql).mapTo<JoinRow>().list()
}

Tag instance has proper id populated where as the id in Product instances is that of Tag instances.

NOTE If FieldMapper is used in place of KotlinMapper (and including no-arg constructors in both the data classes), the above code works properly and correct ids are populated in Product instances.

@hgschmie
Copy link
Contributor

Hi @mahesh-chotrani! Thank you for filing an issue. I tried the following test code:

package org.jdbi.v3.kotlin

import org.jdbi.v3.core.HandleCallback
import org.jdbi.v3.core.Jdbi
import org.jdbi.v3.core.kotlin.KotlinMapper
import org.jdbi.v3.core.kotlin.mapTo
import org.jdbi.v3.core.mapper.JoinRow
import org.jdbi.v3.core.mapper.JoinRowMapper
import org.jdbi.v3.sqlobject.SqlObjectPlugin
import org.jdbi.v3.sqlobject.kotlin.KotlinSqlObjectPlugin
import org.jdbi.v3.testing.junit5.JdbiExtension
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNotNull
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.RegisterExtension

class Issue1944Test {
    @JvmField
    @RegisterExtension
    var h2Extension = JdbiExtension.h2()
        .withPlugins(SqlObjectPlugin(), KotlinSqlObjectPlugin())
        .withInitializer { _, handle ->
            handle.inTransaction(HandleCallback<Any?, RuntimeException> { h ->
                h.execute("CREATE TABLE Tag (id integer primary key, name VARCHAR(50))")
                h.execute("CREATE TABLE Product (id integer primary key, primaryName varchar(50), tagId integer)")

                h.execute("INSERT INTO Tag (id, name) VALUES (1, 'foo')")
                h.execute("INSERT INTO Product (id, primaryName, tagId) VALUES (2, 'stuff', 1)")
            })
        }

    data class Tag(
        val id: Int?,
        val name: String?
    )

    data class Product(
        val id: Int?,
        val primaryName: String?,
        val tagId: Int?,
    )

    @Test
    fun test1944Issue(jdbi: Jdbi) {
        val sql = """
            SELECT
              t.id, t.name,
              p.id pid, p.tagId ptagId, p.primaryName pprimaryName
            FROM Tag t JOIN Product P ON t.id = p.TagId
        """.trimIndent()

        val rows = jdbi.withHandle(HandleCallback<List<JoinRow>, RuntimeException> { handle ->
            handle.registerRowMapper(Tag::class.java, KotlinMapper(Tag::class.java))
            handle.registerRowMapper(Product::class.java, KotlinMapper(Product::class.java, "p"))
            handle.registerRowMapper(JoinRowMapper.forTypes(Tag::class.java, Product::class.java))
            handle.createQuery(sql).mapTo<JoinRow>().list()
        })

        assertNotNull(rows)
        assertEquals(1, rows.size)

        val joinRow = rows[0]
        assertEquals(1, joinRow[Tag::class.java].id)
        assertEquals("foo", joinRow[Tag::class.java].name)

        assertEquals(2, joinRow[Product::class.java].id)
        assertEquals("stuff", joinRow[Product::class.java].primaryName)
        assertEquals(1, joinRow[Product::class.java].tagId)
    }
}

and this test passes for me, implying that all the fields (product.id and tag.id) are filled in correctly (this is using jdbi 3.23.0). Do you have a concrete unit test that demonstrates this failure for you?

(As a side note: I was not able to use the handle.registerRowMapper(KotlinMapper(Tag::class.java)) as in your example because the InferredRowMapperFactory was not able to find the generic superclass (RowMapper<Tag> from the passed in KotlinMapper. I had to register explicitly with the java type. But then it worked fine for me.

@hgschmie
Copy link
Contributor

I managed to reproduce the problem with loading the KotlinPlugin and removing the explicit types.

@mahesh-chotrani
Copy link
Author

Thanks for swift response and the unit test, @hgschmie !

Using your sample, I changed my code to use KotlinSqlObjectPlugin in place of KotlinPlugin. After registering types explicitly, the result was as expected (proper ids were populated in Tag and Product object).

Not sure, if this is an issue with KotlinPlugin or something I missed while using it.

hgschmie added a commit to hgschmie/jdbi that referenced this issue Oct 31, 2021
Register a custom interceptor with the inference interception
framework for the Row mappers. This ensures that KotlinMappers are
correctly handled and data objects that have been registered with a
prefix are handled correctly.

This change fixes jdbi#1944.
hgschmie added a commit to hgschmie/jdbi that referenced this issue Oct 31, 2021
@hgschmie
Copy link
Contributor

This turned out to be a really interesting problem. See #1945 for the full fix. Your previous code should now work unchanged.

Fun fact: You did not have to do handle.registerRowMapper(KotlinMapper(Tag::class.java)) as the kotlin plugin registers a factory to do that for you. That was the root of the problem (that and the fact the java inferring mapper could not detect the types for the kotlin mapper).

Fix will be in the next release. Until then, you can use the workaround as decribed above (register with explicit types).

@mahesh-chotrani
Copy link
Author

Thanks @hgschmie for the fix. And the detailed explanation of the issue.

I'll use the workaround for now.

hgschmie added a commit to hgschmie/jdbi that referenced this issue Nov 1, 2021
Register a custom interceptor with the inference interception
framework for the Row mappers. This ensures that KotlinMappers are
correctly handled and data objects that have been registered with a
prefix are handled correctly.

This change fixes jdbi#1944.
hgschmie added a commit to hgschmie/jdbi that referenced this issue Nov 1, 2021
hgschmie added a commit to hgschmie/jdbi that referenced this issue Nov 1, 2021
hgschmie added a commit that referenced this issue Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants