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

JDBI can't find the registered value mapper for a Map<String, MyClass<SomeOtherClass>> return type #2623

Open
grssam opened this issue Feb 4, 2024 · 4 comments

Comments

@grssam
Copy link

grssam commented Feb 4, 2024

Suppose a class

public class MyClass<T> {
  ...
  T data;
  ...
}

using this in declarative-style approach.. This works:

@SqlQuery("select * from my_table where condition=:value;")
@RegisterBeanMapper(MyClass.java)
List<MyClass> getList(String value);

but this fails with an exception "No row mapper registered for map value com.package.MyClass"

@SqlQuery("select m.name as name, m.* from my_table m where condition=:value;")
@RegisterBeanMapper(MyClass.java)
@KeyColumn("name")
MultiMap<String, MyClass> getMap(String value);

fails with Map<String, MyClass> or Map<String, MyClass<SomeOtherClass>> or MultiMap<String, MyClass<SomeOtherClass>> as well.

I tried to dig through the code. Looks like that in the fluent api approach, this can be solved by providing the actual Type of the map value, but no such way possible in declarative style as annotation arguments need to be compile time constants.

hgschmie added a commit to hgschmie/jdbi that referenced this issue Feb 16, 2024
Addresses the questions in jdbi#2623 with sample code.
@hgschmie
Copy link
Contributor

hgschmie commented Feb 16, 2024

I think you are running against the limitations of what can be expressed with annotations.
The main problem is that your "MyClass" is parameterized but you use the raw type ("MyClass") in the Sql object. In addition, the @RegisterBeanMapper annotation does registers a bean mapper for the bean in question but there is no type information on what to do with the "T" that is part of the type definition.

You basically have two options:

a)

have a concrete type for the row. Instead of MyClass<T> have an MyIntClass extends MyClass<Integer> that resolves the T to an Integer (or whatever type you want to use).

Now this works:

@SqlQuery("select m.name as name, m.* from my_table m where condition=:value;")
@KeyColumn("name")
@RegisterConstructorMapper(MyIntClass.class)
Multimap<String, MyIntClass> getIntMap(String value);

Note that Jdbi still needs to know how to construct the Multimap. If this is a Guava multimap, you need to load the Guava plugin.

b) You do not have a concrete type but want to use a generic:

@SqlQuery("select m.name as name, m.* from my_table m where condition=:value;")
@KeyColumn("name")
Multimap<String, MyClass<Integer>> getMap(int id);

Note that you still need to use a concrete type in the method. Java does not have dynamic typing, you need to provide an actual type information.

Also, you can not register the row mapper with an annotation. Same problem. You need to resolve the type (e.g. MyClass<Integer>) and assign a mapper to it.

So you need to implement the mapper:

public static class MyIntClassMapper implements RowMapper<MyClass<Integer>> {
    @Override
    public User<Integer> map(ResultSet rs, StatementContext ctx) throws SQLException {
        ... map columns onto MyClass<Integer> ...
    }
}

and register it with Jdbi:

jdbi.registerRowMapper(new GenericType<>() {}, new MyIntClassMapper());

(note that at least with reasonably modern Java (this is with 17) it can resolve the type information for the "GenericType<>" from the mapper parameter).

because all of this is a mouthful, I put a complete example together for you at https://github.com/jdbi/jdbi/pull/2636/files, this will go into the e2e testing module as TestIssue2623.

@grssam
Copy link
Author

grssam commented Feb 16, 2024

I think you are running against the limitations of what can be expressed with annotations. The main problem is that your "MyClass" is parameterized but you use the raw type ("MyClass") in the Sql object.

As I've mentioned, I've tried all combinations... quote from description:

fails with Map<String, MyClass> or Map<String, MyClass> or MultiMap<String, MyClass> as well.

....

Also, you can not register the row mapper with an annotation. Same problem. You need to resolve the type (e.g. MyClass<Integer>) and assign a mapper to it.

So you need to implement the mapper:

public static class MyIntClassMapper implements RowMapper<MyClass<Integer>> {
    @Override
    public User<Integer> map(ResultSet rs, StatementContext ctx) throws SQLException {
        ... map columns onto MyClass<Integer> ...
    }
}

We tried this - i.e. adding a custom row mapper and using the class with type provided. It works in code, but when we create a jar out of it (packaging our application), then it fails with the same exception of missing row mapper. We will raise a bug separately for this weird behavior.

and register it with Jdbi:

jdbi.registerRowMapper(new GenericType<>() {}, new MyIntClassMapper());

We did not try this, we registered the row mapper using the annotation itself.

(note that at least with reasonably modern Java (this is with 17) it can resolve the type information for the "GenericType<>" from the mapper parameter).

because all of this is a mouthful, I put a complete example together for you at https://github.com/jdbi/jdbi/pull/2636/files, this will go into the e2e testing module as TestIssue2623.

Thanks!

@hgschmie
Copy link
Contributor

I think you are running against the limitations of what can be expressed with annotations. The main problem is that your "MyClass" is parameterized but you use the raw type ("MyClass") in the Sql object.

As I've mentioned, I've tried all combinations... quote from description:

fails with Map<String, MyClass> or Map<String, MyClass> or MultiMap<String, MyClass> as well.

As it is expected. MyClass<T> is a parameterized class (the T is the parameter). There is no generic mapper for the type T, it needs to be resolved to a concrete type at some point. Java does not support dynamic typing, so you need to either have a class that extends MyClass<String> if T is a string or MyClass<Integer> if T is an integer. If you want to use the "any" type, you need to register the mapper for MyClass<Object>.

Each of your examples is using a "raw" MyClass. Jdbi can not find a row mapper for a "raw" MyClass. That is why it reports the error.

In the example, the jdbi.registerRowMapper(new GenericType<>() {}, new MyIntClassMapper()); stanza actually registers a mapper for a generic MyClass<Integer> type. Java can infer that type for the first argument from the second, which is why there is just an empty <>. But what the code actually does is jdbi.registerRowMapper(new GenericType<MyClass<Integer>>() {}, new MyIntClassMapper());

All the behavior of Jdbi is consistent with what you configured. Generics are tricky and omitting the generic type parameter and using a raw type generally does not work.

As I said, there are limitations to what you can do with the annotations. Registering mappers is limited to concrete classes and using the @RegisterBeanMapper, @RegisterConstructorMapper or @RegisterFieldMapper require the mapper type to match the row type.

@hgschmie
Copy link
Contributor

For the MultiMap, you need a collector btw. If you use the Guava multimap (https://guava.dev/releases/snapshot-jre/api/docs/com/google/common/collect/Multimap.html), we provide a collector as part of the jdbi3-guava module.

If you use another MultiMap, you need to register a collector with Jdbi, otherwise it can not construct the collection object.

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