fix Use constructor element and discriminator case element is resultMap property NPE

[JiHongYuan]

Use constructor element and discriminator case element is resultMap property

<resultMap id="joinedPost" type="org.apache.ibatis.domain.blog.Post">
  <constructor>
    <idArg column="post_id" name="id"/>
  </constructor>
  <id property="id" column="post_id"/>
  <discriminator javaType="int" column="draft">
    <case value="1" resultMap="draftPost"/>
  </discriminator>
</resultMap>

At create resultMap，not resultMap.type

// org.apache.ibatis.mapping.ResultMap 125 
if (!constructorArgNames.isEmpty()) {
  // this 126
  final List<String> actualArgNames = argNamesOfMatchingConstructor(constructorArgNames);
  // ......
}    

// org.apache.ibatis.mapping.ResultMap 148
private List<String> argNamesOfMatchingConstructor(List<String> constructorArgNames) {
  // resultMap.type NPE
  Constructor<?>[] constructors = resultMap.type.getDeclaredConstructors();
  for (Constructor<?> constructor : constructors) {
    Class<?>[] paramTypes = constructor.getParameterTypes();
    if (constructorArgNames.size() == paramTypes.length) {
      List<String> paramNames = getArgNames(constructor);
      if (constructorArgNames.containsAll(paramNames)
          && argTypesMatch(constructorArgNames, paramTypes, paramNames)) {
        return paramNames;
      }
    }
  }
  return null;
}

Error Log：

### The error occurred while processing mapper_resultMap[map]_discriminator_case[1]
### Cause: org.apache.ibatis.builder.BuilderException: Error parsing SQL Mapper Configuration. Cause: org.apache.ibatis.builder.BuilderException: Error parsing Mapper XML. The XML location is 'StudentMapper.xml'. Cause: java.lang.NullPointerException
	at org.apache.ibatis.exceptions.ExceptionFactory.wrapException(ExceptionFactory.java:30)
	at org.apache.ibatis.session.SqlSessionFactoryBuilder.build(SqlSessionFactoryBuilder.java:80)
	at org.apache.ibatis.session.SqlSessionFactoryBuilder.build(SqlSessionFactoryBuilder.java:64)
	at com.example.App.main(App.java:33)
Caused by: org.apache.ibatis.builder.BuilderException: Error parsing SQL Mapper Configuration. Cause: org.apache.ibatis.builder.BuilderException: Error parsing Mapper XML. The XML location is 'StudentMapper.xml'. Cause: java.lang.NullPointerException
	at org.apache.ibatis.builder.xml.XMLConfigBuilder.parseConfiguration(XMLConfigBuilder.java:122)
	at org.apache.ibatis.builder.xml.XMLConfigBuilder.parse(XMLConfigBuilder.java:99)
	at org.apache.ibatis.session.SqlSessionFactoryBuilder.build(SqlSessionFactoryBuilder.java:78)
	... 2 more
Caused by: org.apache.ibatis.builder.BuilderException: Error parsing Mapper XML. The XML location is 'StudentMapper.xml'. Cause: java.lang.NullPointerException
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.configurationElement(XMLMapperBuilder.java:123)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.parse(XMLMapperBuilder.java:95)
	at org.apache.ibatis.builder.xml.XMLConfigBuilder.mapperElement(XMLConfigBuilder.java:378)
	at org.apache.ibatis.builder.xml.XMLConfigBuilder.parseConfiguration(XMLConfigBuilder.java:120)
	... 4 more
Caused by: java.lang.NullPointerException
	at org.apache.ibatis.mapping.ResultMap$Builder.argNamesOfMatchingConstructor(ResultMap.java:149)
	at org.apache.ibatis.mapping.ResultMap$Builder.build(ResultMap.java:126)
	at org.apache.ibatis.builder.MapperBuilderAssistant.addResultMap(MapperBuilderAssistant.java:208)
	at org.apache.ibatis.builder.ResultMapResolver.resolve(ResultMapResolver.java:47)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.resultMapElement(XMLMapperBuilder.java:289)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.processNestedResultMappings(XMLMapperBuilder.java:400)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.processDiscriminatorElement(XMLMapperBuilder.java:332)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.resultMapElement(XMLMapperBuilder.java:274)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.resultMapElement(XMLMapperBuilder.java:254)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.resultMapElements(XMLMapperBuilder.java:246)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.configurationElement(XMLMapperBuilder.java:119)
	... 7 more

[harawata]

Hello @JiHongYuan ,

All tests pass even if I revert the change you made to ResultMap which means the change you applied to the test code does not reproduce the issue you are trying to fix.
Please add a new independent test case instead of modifying an existing one (and verify the issue reproduces before fixing it).

[JiHongYuan]

Hello @harawata,

Thank you for your reply!

I Revert Commit modifying test case and add a new independent test case.
Ensure ResultMap throw NPE.

[harawata]

@JiHongYuan ,

Thank you for the update. I could verify the problem.

Although this change avoids NPE, it does not fix the real problem.
I didn't think about discriminator when fixing #721 , so for now, you may have to specify javaType instead of name when using constructor mapping with discriminator.

In the test case, the constructor takes single int type argument, so the result map should look as follows.

<resultMap id="joinedPost" type="org.apache.ibatis.domain.blog.Post">
  <constructor>
    <idArg column="post_id" javaType="_int" /> 
  </constructor>
  ...

Note that when the constructor takes multiple arguments, the order of <idArg /> / <arg /> must match the order of actual constructor arguments.

[JiHongYuan]

Hi @harawata，
OK, I should change the test case to：

<idArg column="..." javaType="..." /> 
...

right？
I need add javaType and test the problem ?

This is my first PR，so dont now to do ...
Thank you!

[harawata]

To define <constructor /> in <resultMap />, there are basically two different ways.
https://mybatis.org/mybatis-3/sqlmap-xml.html#constructor

1. Specifying javaType
2. Specifying name

And when using <discriminator />, only the first method is supported, currently.

We cannot accept this PR because your fix (checking resultMap.type != null) does not fix the real problem.
In other words, even if we accept this PR, the second method still does not work properly (it might look like it is working, but it's not).
It requires a bigger change to fix the real problem.

Please let me know if you have any further question.

[JiHongYuan]

But，during the test javaType and name, all do fix problem.

1. name

example

1.1 Mapper

  <resultMap id="joinedConstructorPost" type="org.apache.ibatis.domain.blog.Post">
    <constructor>
      <idArg column="post_id" name="id"/>
      <arg column="name" name="subject"/>
    </constructor>
   //...
  </resultMap>

1.2 Model

public class Post {
  // ...
  public Post(@Param("subject") String subject, @Param("id") int id) {
    this.id = id;
    this.subject = subject;
  }
  //...
}

result

2. javaType

example

1.1 Mapper

  <resultMap id="joinedConstructorPost" type="org.apache.ibatis.domain.blog.Post">
    <constructor>
      <idArg column="post_id" javaType="_int"/>
      <arg column="post_subject" javaType="string"/>
    </constructor>
   //...
  </resultMap>

1.2 Model

public class Post {
  // ...
  public Post(int id, String subject) {
    this.id = id;
    this.subject = subject;
  }
  //...
}

result

---

So from result, javaType and name, do fix the real problem.
Maybe Add two diff test case?

[harawata]

Constructor-mapping is not used for DraftPost because of the check you added.
The result is correct just because constructor-auto-mapping kicks in. That is not the expected behavior.

[JiHongYuan]

Oh ! You right.
I found The Real problem。
Thank you！ I will try fix problem

[harawata]

Could you please create a completely new test case with a new simpler mapper?
BlogMapper is pretty convoluted and it's difficult to understand the intent sometimes.

[JiHongYuan]

OK, I will new test case with a new simpler mapper.

[JiHongYuan]

I discover DraftPost use constructor-auto-mapping reason is resultMap id= draftPost extends joinedPost
When modify resultMap id= draftPost extends joinedConstructorPost, It OK, No problem!!!

<resultMap id="joinedConstructorPost" type="org.apache.ibatis.domain.blog.Post">
  <constructor>
    <idArg column="post_id" javaType="_int"/>
    <arg column="post_subject" javaType="string"/>
  </constructor>
  <discriminator javaType="int" column="draft">
    <case value="1" resultMap="draftPost"/>
  </discriminator>
</resultMap>

<resultMap id="draftPost" type="org.apache.ibatis.domain.blog.DraftPost" extends="joinedPost"/>

<resultMap id="joinedPost" type="org.apache.ibatis.domain.blog.Post">
  <id property="id" column="post_id"/>
  <result property="subject" column="post_subject"/>
  // ...
  <discriminator javaType="int" column="draft">
    <case value="1" resultMap="draftPost"/>
  </discriminator>
</resultMap>

When review code, discover resultMap XML process:

1. process ResultMap -> discriminator -> case
   create discriminator id is org.apache.ibatis.domain.blog.mappers.BlogMapper.mapper_resultMap[joinedPost]_discriminator_case[1]
2. ..._case[1] resultMap add parent resultMap and ..._case[*] resultMap convert discriminator
3. ..._case[1] resultMap point specific type(resultMap example org.apache.ibatis.domain.blog.mappers.BlogMapper.draftPost )

But ...case[1], Will it be used elsewhere

[harawata]

@JiHongYuan ,

Could you do the following?

1. revert the modifications to the test code
2. squash all commits into a single commit
3. force-push

I want to keep the year numbers in the license headers of the test files as-is.
I will add more focused tests afterwards.

(if it's easier, close this PR and open a new one without test changes)