mybatis-3 icon indicating copy to clipboard operation
mybatis-3 copied to clipboard

DefaultParameterHandler bug

Open ImSejin opened this issue 2 years ago • 5 comments

MyBatis version

3.5.9 (mybatis-spring-boot:2.2.2)

Database vendor and version

h2, 1.4.200

Test case or example project

Example

Steps to reproduce

  1. Make an interface.
  2. Make some implementations of the interface.
  3. Create a mapper and its method that has parameter whose type is the interface.
  4. Call the method with argument (its type is one of the implementations).

Expected result

No exception

Actual result

nested exception is org.apache.ibatis.reflection.ReflectionException: There is no getter for property named '?' ...

I think there is a bug in org.apache.ibatis.scripting.defaults.DefaultParameterHandler#setParameters.

When type of parameter is interface, its implementation can be as the parameter. Problem is here. ParameterMapping resolves type handler by parameter type of mapper method, but checking if TypeHandlerRegistry has proper one does by actual parameter type.

I encounter exception though add type handler of the interface. If I annotate @MappedTypes on CodeTypeHandler, problem is solved, but I don't think it's nice solution,

I suggest that code change.

  • from: typeHandlerRegistry.hasTypeHandler(parameterObject.getClass())
  • to: typeHandlerRegistry.hasTypeHandler(parameterMapping.getJavaType())
if (boundSql.hasAdditionalParameter(propertyName)) { // issue #448 ask first for additional params
  value = boundSql.getAdditionalParameter(propertyName);
} else if (parameterObject == null) {
  value = null;
} else if (typeHandlerRegistry.hasTypeHandler(parameterObject.getClass())) { // parameterObject.class == BetaCode.class
  value = parameterObject;
} else {
  MetaObject metaObject = configuration.newMetaObject(parameterObject);
  value = metaObject.getValue(propertyName);
}
// parameterMapping.javaType == Code.class
// parameterMapping.typeHandler == CodeTypeHandler
TypeHandler typeHandler = parameterMapping.getTypeHandler();

ImSejin avatar Jul 08 '22 07:07 ImSejin

Hello @ImSejin ,

I'm really sorry for a late reply. I took a quick look at the demo, but I couldn't understand the point you are trying to make and then I forgot. 😓

Here are the things that confuse me:

  1. Something does not have a default constructor (it has Lombok's @Builder), so MyBatis won't be able to create the instance.
  2. Why there are two type handlers CodeTypeHandler and AlphaCodeTypeHandler? CodeTypeHandler seems to handle both AlphaCode and BetaCode, but then what is the purpose of AlphaTypeHandler?

I made some changes to pass the test and sent a PR to your repo. https://github.com/ImSejin/mybatis-bug-reproduction/pull/1

A few key points:

  • You cannot register a type handler against interface. If you think you need to do this, there may be something unusual with your application design. The only exception is Enum. I would suggest you to check out this test case.
  • The logic resolving type handler is complex, but I don't see any problem with DefaultParameterHandler.

Please let me know if I am missing the point.

harawata avatar Mar 08 '23 21:03 harawata

Thank you for feedback.

Something does not have a default constructor (it has Lombok's @Builder), so MyBatis won't be able to create the instance.

Okay, I create a default constructor into Something, but that's not a point.

Why there are two type handlers CodeTypeHandler and AlphaCodeTypeHandler? CodeTypeHandler seems to handle both AlphaCode and BetaCode, but then what is the purpose of AlphaTypeHandler?

Let's see the point. MyBatis adds two type handlers as you mentioned. Body of methods in AlphaCodeTypeHandler is implemented by exception code. It means if MyBatis uses the type handler, the test testAlphaCode must fail. However the test passes. A type handler is registered, but not used? It's weird. There is even more weird thing. If you remove class AlphaCodeTypeHandler, the test fail. This is the prove that MyBatis adds the type handler into registry, but not uses.

If you think you need to do this, there may be something unusual with your application design.

I don't understand. I think interface type can be as parameter type of mapper's method. Why you think it's unusal? MyBatis uses getter when it gets a value of prepared statement, so I think MyBatis supports any type which has a getter even if the type is interface. (interface Code has a getter getValue.)

ImSejin avatar Mar 09 '23 12:03 ImSejin

Hi @ImSejin ,

So, you wanted MyBatis to use AlphaCodeTypeHandler? Then why did you register CodeTypeHandler? You understand that both type handlers are registered because they are in the specified type-handlers-package, right?

I think interface type can be as parameter type of mapper's method.

It is OK to use interface as a parameter type of a mapper method. However, it is NOT OK to register a type handler for that interface type (CodeTypeHandler is registered against Code).

Why you think it's unusal?

Because it is usually impossible to instantiate an interface. In the CodeTypeHandler#resolve() method, you have to write all the classes that implement Code interface. It is a sign of bad design. You basically are using AlphaCode and BetaCode as if they are Enums. If you check the test case I linked, you would understand what I mean.

harawata avatar Mar 09 '23 14:03 harawata

you have to write all the classes that implement Code interface. It is a sign of bad design.

Absolutely right. I totally missed that type handler instantiates own type. 😭 I just focused on changing java type to JDBC type.

You understand that both type handlers are registered because they are in the specified type-handlers-package, right?

Yes, I understand.

you wanted MyBatis to use AlphaCodeTypeHandler? Then why did you register CodeTypeHandler?

My need is handling implemented classes whose handler is not registered. Class BetaCode implements interface Code. I wanted MyBatis to handle CodeTypeHandler as a default type handler for BetaCode.

Thank you for pointing my misundertanding.

ImSejin avatar Mar 10 '23 01:03 ImSejin

You have to register the CodeTypeHandler for all implementation classes. You already know how to do it with annotation i.e. @MappedTypes({BetaCode.class, AlphaCode.class})

Alternatively, you can search the implementation classes and register them programatically.

TypeHandlerRegistry registry = sqlSessionFactory.getConfiguration().getTypeHandlerRegistry();
CodeTypeHandler handler = new CodeTypeHandler();
for (Class<Code> codeClass : codeClasses) {
  registry.register(codeClass, handler);
}

MyBatis cannot help you with searching the implementation classes, but there are many third party tools for that. https://stackoverflow.com/q/347248/1261766 https://stackoverflow.com/q/435890/1261766

p.s. If you have control over the design, take a look at https://github.com/ImSejin/mybatis-bug-reproduction/pull/2

harawata avatar Mar 10 '23 03:03 harawata