jdbi icon indicating copy to clipboard operation
jdbi copied to clipboard

Make sure record types work

Open stevenschlansker opened this issue 3 years ago • 8 comments

Let's make sure Jdbi has good support for the new jdk16 record types.

stevenschlansker avatar Apr 02 '21 15:04 stevenschlansker

For others finding this from Google, I found out by some trial and error that I need to use @BindMethods instead of @BindBean to make everything work when binding a record as an argument in a DAO interface.

record MyRecord(int id, int value);

public interface UserDao {
    @SqlUpdate("insert into table(id, value) values (:id, :value)")
    void save(@BindMethods final MyRecord record);
}

niklasfo avatar Apr 21 '21 19:04 niklasfo

what scenarios should be made to ensure it works with new jdk16 record types ??

GeorgeSalu avatar Apr 24 '21 16:04 GeorgeSalu

I think adding test cases and docs for binding query parameters and mapping from results is probably the first step. Essentially what niklasfo posted above, just expanded on a little bit :)

stevenschlansker avatar Apr 25 '21 04:04 stevenschlansker

In general I didn't encounter any problems using records, once I changed bindBean to bindMethods and added @JdbiConstructor (not sure if it's necessary but it doesn't hurt to add it). However I have encountered a problem with records in createUpdate with BindMethods() and StringTemplate4 not getting the attributes

 return getDb()
            .withHandle(
                handle ->
                    handle
                        .createUpdate(updateStatement)
                        .bindMethods(model)
                        .defineNamedBindings()
                        .define("TABLE", tableName)
                        .define("CODE_FIELD", idColumn)
                        .bind("code", code)
                        .bind("updatedBy", updatedBy)
                        .execute()
        final String UPDATE_THIRD_PARTY_REFRESH_TOKEN =
            """
                UPDATE third_party_refresh_token tprt SET
                    <if(refreshToken)> tprt.refresh_token = :refreshToken, <endif>
                    <if(thirdPartyCode)> tprt.third_party_code = :code, <endif>
                    <if(isRevoked)> tprt.revoked = :isRevoked, <endif>
                    tprt.updated_at = now()
                WHERE tprt.third_party_code = :code
            """;
public record RefreshToken(

    @ColumnName("third_party_code")
    String thirdPartyCode,

    @ColumnName("refresh_token")
    String refreshToken,

    @Nullable
    @ColumnName("revoked")
    boolean isRevoked
) {

    @JdbiConstructor
    public RefreshToken {
        if (Objects.isNull(isRevoked)) {
            isRevoked = Boolean.FALSE;
        }
    }

    public RefreshToken(String code, String refreshToken) {
        this(code, refreshToken, Boolean.FALSE);
    }

}

context [anonymous] 2:12 attribute refreshToken isn't defined
context [anonymous] 3:12 attribute thirdPartyCode isn't defined
context [anonymous] 4:12 attribute isRevoked isn't defined

But it works if we get rid of StringTemplate4 like this

        final String UPDATE_THIRD_PARTY_REFRESH_TOKEN =
            """
                UPDATE third_party_refresh_token tprt SET
                     tprt.refresh_token = :refreshToken,
                     tprt.third_party_code = :code,
                     tprt.revoked = :isRevoked,
                    tprt.updated_at = now()
                WHERE tprt.third_party_code = :code
            """;

I can provide a working example if needed.

soyPeter avatar May 29 '21 22:05 soyPeter

~Here again to say that version 3.20.1 fixes this for me using bindBean()~, My bad I tested against a record with custom getters on attributes, this was working in 3.20.0 too, no regressions introduced :).

Thanks!!!

In general I didn't encounter any problems using records, once I changed bindBean to bindMethods and added @JdbiConstructor (not sure if it's necessary but it doesn't hurt to add it). However I have encountered a problem with records in createUpdate with BindMethods() and StringTemplate4 not getting the attributes

 return getDb()
            .withHandle(
                handle ->
                    handle
                        .createUpdate(updateStatement)
                        .bindMethods(model)
                        .defineNamedBindings()
                        .define("TABLE", tableName)
                        .define("CODE_FIELD", idColumn)
                        .bind("code", code)
                        .bind("updatedBy", updatedBy)
                        .execute()
        final String UPDATE_THIRD_PARTY_REFRESH_TOKEN =
            """
                UPDATE third_party_refresh_token tprt SET
                    <if(refreshToken)> tprt.refresh_token = :refreshToken, <endif>
                    <if(thirdPartyCode)> tprt.third_party_code = :code, <endif>
                    <if(isRevoked)> tprt.revoked = :isRevoked, <endif>
                    tprt.updated_at = now()
                WHERE tprt.third_party_code = :code
            """;
public record RefreshToken(

    @ColumnName("third_party_code")
    String thirdPartyCode,

    @ColumnName("refresh_token")
    String refreshToken,

    @Nullable
    @ColumnName("revoked")
    boolean isRevoked
) {

    @JdbiConstructor
    public RefreshToken {
        if (Objects.isNull(isRevoked)) {
            isRevoked = Boolean.FALSE;
        }
    }

    public RefreshToken(String code, String refreshToken) {
        this(code, refreshToken, Boolean.FALSE);
    }

}
context [anonymous] 2:12 attribute refreshToken isn't defined
context [anonymous] 3:12 attribute thirdPartyCode isn't defined
context [anonymous] 4:12 attribute isRevoked isn't defined

But it works if we get rid of StringTemplate4 like this

        final String UPDATE_THIRD_PARTY_REFRESH_TOKEN =
            """
                UPDATE third_party_refresh_token tprt SET
                     tprt.refresh_token = :refreshToken,
                     tprt.third_party_code = :code,
                     tprt.revoked = :isRevoked,
                    tprt.updated_at = now()
                WHERE tprt.third_party_code = :code
            """;

I can provide a working example if needed.

soyPeter avatar Jun 26 '21 17:06 soyPeter

Probably relevant if someone stumbles upon this: For mapping the result to a Record after a SELECT query, you have to register a custom RowMapper to then do a .mapTo(YourRecord.class (see https://jdbi.org/#_row_mappers). No modification of the original record is required.

Example

Write custom mapper

class YourRecordMapper implements RowMapper<YourRecord> {
    @Override
    public YourRecord map(ResultSet rs, StatementContext ctx) throws SQLException {
      return new YourRecord(
          rs.getString("nice_key"),
          rs.getInt("different_key"));
    }

Register custom mapper

jdbi = Jdbi.create(dataSource);
 // Register custom RowMapper to map to Java Record
jdbi.registerRowMapper(new YourRecordMapper());

Use mapping in DB transaction

...

return jdbi.withHandle(
        h ->
            h.select(SELECT_QUERY)
                .mapTo(YourRecord.class)
                .one());

See more information on different ways to implement this row mapping at https://jdbi.org/#_row_mappers.

felix-seifert avatar Apr 22 '22 16:04 felix-seifert

Still, it would be nice with a reflection-based solution for mapping results to records...

orende avatar May 14 '22 07:05 orende

@orende I was able to get this working yesterday.

I enabled the -parameters compiler flag and used @RegisterConstructorMapper to directly map a SELECT * statement back to a record.

Combining that with using @BindMethods, inserts and get operations appear to work nicely with records.

A requirement is that your record must be defined to exactly match the columns in the table (note that jdbi automatically converts from snake_case database columns to camelCase record property names).

Here's a pseudocode example using @BindMethods to insert and @RegisterConstructorMapper to query:

public record FooRecord(String id, String someColumn) {}
public interface FooDao {
    
    @SqlUpdate("INSERT INTO foo(id, some_column) VALUES (:id, :someColumn)")
    void insertRecord(@BindMethods FooRecord record);

    @SqlQuery("SELECT * FROM foo WHERE id = :id")
    @RegisterConstructorMapper(FooRecord.class)
    FooRecord get(String id);

}

I assume you should be able to use bindMethods(fooRecord) and handle.registerRowMapper(ConstructorMapper.factory(FooRecord.class)) if not using the annotations approach.

tehsven avatar May 19 '22 16:05 tehsven

FYI - we don't want to add any annotations to our records so we register this row mapper and it handles any records. YMMV

public class RecordAndAnnotatedConstructorMapper
        implements RowMapperFactory
{
    @Override
    public Optional<RowMapper<?>> build(Type type, ConfigRegistry config)
    {
        if ((type instanceof Class<?> clazz) && clazz.isRecord()) {
            return ConstructorMapper.factory(clazz).build(type, config);
        }
        return Optional.empty();
    }
}

Randgalt avatar Jan 29 '23 08:01 Randgalt

We shipped that and have tests as part of the jdk17 module.

hgschmie avatar Jun 15 '23 02:06 hgschmie

We shipped that and have tests as part of the jdk17 module.

Should I be able to find such a RowMapperFactory in the main distribution of JDBI by now? Asking because I am unable to locate it and Record mapping does not seem to work out of the box still. (Tested with JDBI 3.41.3). I also was unable to find a jdk17 module.

sandrinr avatar Nov 23 '23 07:11 sandrinr

Hi @sandrinr , the expectation is that it should work ok using @BindMethods and @RegisterConstructorMapper. What did you try that didn't work?

stevenschlansker avatar Nov 27 '23 18:11 stevenschlansker

@stevenschlansker My expectation from the message https://github.com/jdbi/jdbi/issues/1822#issuecomment-1592253369 was that something like in message https://github.com/jdbi/jdbi/issues/1822#issuecomment-1407596793 would be part of the JDBI distribution. But I was unable to find it. I probably would have also expected that with that record types work out of the box.

We now had to create something like the RecordAndAnnotatedConstructorMapper ourselves and add it to our JDBI instances manually. Not a big deal, but unexpected 🙂.

sandrinr avatar Dec 05 '23 20:12 sandrinr

I can confirm JDBI works fine with record types for rows using @RegisterContructorMapper and @BindMethods annotations and we're been using this for over half a year now. Due to a bug in Java 21 (see #2527 and JDK-8320575) there is an issue when using a compact constructor since then the types of the constructor arguments are lost. The workaround for now is to duplicate the canonical constructor and put the required logic in there and annotate it with @JdbConstructor.

diversit avatar Mar 01 '24 07:03 diversit

We're going to ship a workaround in the next release for the loss of generic types until the upstream bug is fixed: https://github.com/jdbi/jdbi/pull/2648

stevenschlansker avatar Mar 01 '24 16:03 stevenschlansker