When javaType has not (or partially) been specified, determine the best matching constructor

[epochcoder]

• falls back to current behaviour if any ambiguity is detected
• remove setters from immutable ResultMapping

[coveralls]

coverage: 87.324% (+0.05%) from 87.27%
when pulling be848e4 on epochcoder:feat/2618-auto-determine-type-when-missing
into 48242f5 on mybatis:master.

[harawata]

@epochcoder ,

The logic looks good, thank you!

Is it possible to perform this after the search&sort by arg name step?

mybatis-3/src/main/java/org/apache/ibatis/mapping/ResultMap.java

Lines 136 to 146 in 1c21b8d

	final List<String> actualArgNames = argNamesOfMatchingConstructor(constructorArgNames);
	if (actualArgNames == null) {
	throw new BuilderException("Error in result map '" + resultMap.id + "'. Failed to find a constructor in '"
	+ resultMap.getType().getName() + "' with arg names " + constructorArgNames
	+ ". Note that 'javaType' is required when there is no writable property with the same name ('name' is optional, BTW). There might be more info in debug log.");
	}
	resultMap.constructorResultMappings.sort((o1, o2) -> {
	int paramIdx1 = actualArgNames.indexOf(o1.getProperty());
	int paramIdx2 = actualArgNames.indexOf(o2.getProperty());
	return paramIdx1 - paramIdx2;
	});

The reason we introduced the name attribute was to allow users to write the mappings in any order ( #721 ), so for example, users may expect the following mapping to work.

import java.time.LocalDate;

public record Account4(long accountId, String accountName, LocalDate accountDob) {
}

<resultMap id="account4RM" type="org.apache.ibatis.submitted.auto_type_from_non_ambiguous_constructor.Account4">
  <constructor>
    <idArg name="accountId" column="id"/>
    <!-- these two are defined in wrong order -->
    <arg name="accountDob" column="dob" />
    <arg name="accountName" column="name"/>
  </constructor>
</resultMap>

<select id="getAccount4" resultMap="account4RM">
  select id, name, type, date '2025-01-02' dob from account where id = #{id}
</select>

I haven't given much thought, but you might have to move the search/sort logic out of ResultMap to achieve this.
Anyway, let me know what you think.

[harawata]

As a related topic, I would like to provide a clear message when the name attribute is partially specified in the constructor mapping (which is not allowed) (see #2932 ).
It does not have to be included in this PR, though.

[epochcoder]

@epochcoder ,

The logic looks good, thank you!

Is it possible to perform this after the search&sort by arg name step?

mybatis-3/src/main/java/org/apache/ibatis/mapping/ResultMap.java

Lines 136 to 146 in 1c21b8d

	final List<String> actualArgNames = argNamesOfMatchingConstructor(constructorArgNames);
	if (actualArgNames == null) {
	throw new BuilderException("Error in result map '" + resultMap.id + "'. Failed to find a constructor in '"
	+ resultMap.getType().getName() + "' with arg names " + constructorArgNames
	+ ". Note that 'javaType' is required when there is no writable property with the same name ('name' is optional, BTW). There might be more info in debug log.");
	}
	resultMap.constructorResultMappings.sort((o1, o2) -> {
	int paramIdx1 = actualArgNames.indexOf(o1.getProperty());
	int paramIdx2 = actualArgNames.indexOf(o2.getProperty());
	return paramIdx1 - paramIdx2;
	});

The reason we introduced the name attribute was to allow users to write the mappings in any order ( #721 ), so for example, users may expect the following mapping to work.

import java.time.LocalDate;

public record Account4(long accountId, String accountName, LocalDate accountDob) {
}

<resultMap id="account4RM" type="org.apache.ibatis.submitted.auto_type_from_non_ambiguous_constructor.Account4">
  <constructor>
    <idArg name="accountId" column="id"/>
    <!-- these two are defined in wrong order -->
    <arg name="accountDob" column="dob" />
    <arg name="accountName" column="name"/>
  </constructor>
</resultMap>

<select id="getAccount4" resultMap="account4RM">
  select id, name, type, date '2025-01-02' dob from account where id = #{id}
</select>

I haven't given much thought, but you might have to move the search/sort logic out of ResultMap to achieve this.
Anyway, let me know what you think.

That makes a lot of sense! I'll look into it, and add the suggested test case as well :)

[epochcoder]

@harawata This ended up being way more involved than I thought. I discovered quite a lot of functionality, and a lot of interplay between them. The only way was to unify this for XML and annotations.

We have multiple use-cases:

1. arguments out of order
2. arguments missing type information
3. arguments with an optional name
4. arguments with custom type handlers
5. arguments not provided at all (auto-mapping during runtime)

And I also tried to keep backward compatibility in mind. The resulting change is quite big; I will update shortly

[harawata]

Yeah, it got complicated after #721 .
And people (understandably) misunderstand the purpose of name.

The initial part of the determination process would look something like this (I only mention @Arg, but the same goes for <idArg> and <arg> in XML).

1. If @Arg exists and name is partially specified, throw an exception.
2. if names are specified, search the constructor by the combination of these names. If no matching constructor found, throw an exception. If a match found, sort the mappings so that they match the picked constructor's arguments order.
3. At this point, we can assume the count and order of the mappings are accurate. So, it may be safe to call autoTypeResultMappingsForUnknownJavaTypes().

It's been a while, so I could well be missing something obvious.

[epochcoder]

@harawata I added all the test cases I could think of to ResultMappingConstructorResolverTest, all constructor resolving logic is isolated in there now. :)

[harawata]

@epochcoder ,

I'm really sorry, I accidentally rebased your branch on to master and force-pushed. 😰
I am sure that none of your changes were deleted, but it seems to be difficult to undo my mistake from my side.

Assuming you have the local branch 2618-auto-determine-type-when-missing in the state before my force-push, could you force push the branch?
Please let me know if you encounter any difficulty.