Skip to content

Commit 3496397

Browse files
committed
Avoid invoking equals/hashCode when assigning generated keys
When detecting single parameter case, it now checks parameter name instead of equality of the parameter objects. This should fix gh-1719. A caveat : when there is only one parameter and its name is 'param2' (for whatever reason), `keyProperty` must include the parameter name i.e. `keyProperty="param2.xx"`.
1 parent 411a2fd commit 3496397

File tree

6 files changed

+138
-7
lines changed

6 files changed

+138
-7
lines changed

src/main/java/org/apache/ibatis/executor/keygen/Jdbc3KeyGenerator.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@
2828
import java.util.List;
2929
import java.util.Map;
3030
import java.util.Map.Entry;
31+
import java.util.Set;
3132

3233
import org.apache.ibatis.binding.MapperMethod.ParamMap;
3334
import org.apache.ibatis.executor.Executor;
3435
import org.apache.ibatis.executor.ExecutorException;
3536
import org.apache.ibatis.mapping.MappedStatement;
3637
import org.apache.ibatis.reflection.ArrayUtil;
3738
import org.apache.ibatis.reflection.MetaObject;
39+
import org.apache.ibatis.reflection.ParamNameResolver;
3840
import org.apache.ibatis.session.Configuration;
3941
import org.apache.ibatis.session.defaults.DefaultSqlSession.StrictMap;
4042
import org.apache.ibatis.type.JdbcType;
@@ -47,6 +49,8 @@
4749
*/
4850
public class Jdbc3KeyGenerator implements KeyGenerator {
4951

52+
private static final String SECOND_GENERIC_PARAM_NAME = ParamNameResolver.GENERIC_NAME_PREFIX + "2";
53+
5054
/**
5155
* A shared instance.
5256
*
@@ -171,7 +175,10 @@ private void assignKeysToParamMap(Configuration configuration, ResultSet rs, Res
171175

172176
private Entry<String, KeyAssigner> getAssignerForParamMap(Configuration config, ResultSetMetaData rsmd,
173177
int columnPosition, Map<String, ?> paramMap, String keyProperty, String[] keyProperties, boolean omitParamName) {
174-
boolean singleParam = paramMap.values().stream().distinct().count() == 1;
178+
Set<String> keySet = paramMap.keySet();
179+
// A caveat : if the only parameter has {@code @Param("param2")} on it,
180+
// it must be referenced with param name e.g. 'param2.x'.
181+
boolean singleParam = !keySet.contains(SECOND_GENERIC_PARAM_NAME);
175182
int firstDot = keyProperty.indexOf('.');
176183
if (firstDot == -1) {
177184
if (singleParam) {
@@ -180,10 +187,10 @@ private Entry<String, KeyAssigner> getAssignerForParamMap(Configuration config,
180187
throw new ExecutorException("Could not determine which parameter to assign generated keys to. "
181188
+ "Note that when there are multiple parameters, 'keyProperty' must include the parameter name (e.g. 'param.id'). "
182189
+ "Specified key properties are " + ArrayUtil.toString(keyProperties) + " and available parameters are "
183-
+ paramMap.keySet());
190+
+ keySet);
184191
}
185192
String paramName = keyProperty.substring(0, firstDot);
186-
if (paramMap.containsKey(paramName)) {
193+
if (keySet.contains(paramName)) {
187194
String argParamName = omitParamName ? null : paramName;
188195
String argKeyProperty = keyProperty.substring(firstDot + 1);
189196
return entry(paramName, new KeyAssigner(config, rsmd, columnPosition, argParamName, argKeyProperty));
@@ -193,7 +200,7 @@ private Entry<String, KeyAssigner> getAssignerForParamMap(Configuration config,
193200
throw new ExecutorException("Could not find parameter '" + paramName + "'. "
194201
+ "Note that when there are multiple parameters, 'keyProperty' must include the parameter name (e.g. 'param.id'). "
195202
+ "Specified key properties are " + ArrayUtil.toString(keyProperties) + " and available parameters are "
196-
+ paramMap.keySet());
203+
+ keySet);
197204
}
198205
}
199206

src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2009-2018 the original author or authors.
2+
* Copyright 2009-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,7 +30,7 @@
3030

3131
public class ParamNameResolver {
3232

33-
private static final String GENERIC_NAME_PREFIX = "param";
33+
public static final String GENERIC_NAME_PREFIX = "param";
3434

3535
/**
3636
* <p>

src/test/java/org/apache/ibatis/submitted/keygen/CountryMapper.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.apache.ibatis.annotations.Insert;
2323
import org.apache.ibatis.annotations.Options;
2424
import org.apache.ibatis.annotations.Param;
25+
import org.apache.ibatis.reflection.ParamNameResolver;
2526

2627
public interface CountryMapper {
2728

@@ -100,4 +101,11 @@ int insertMultiParams_keyPropertyWithWrongParamName(@Param("country") Country co
100101
@Options(useGeneratedKeys = true, keyProperty = "country.id")
101102
@Insert({ "insert into country (countryname,countrycode) values ('a','A'), ('b', 'B')" })
102103
int tooManyGeneratedKeysParamMap(@Param("country") Country country, @Param("someId") Integer someId);
104+
105+
int insertWeirdCountries(List<NpeCountry> list);
106+
107+
// If the only parameter has a name 'param2', keyProperty must include the prefix 'param2.'.
108+
@Options(useGeneratedKeys = true, keyProperty = ParamNameResolver.GENERIC_NAME_PREFIX + "2.id")
109+
@Insert({ "insert into country (countryname,countrycode) values (#{param2.countryname},#{param2.countrycode})" })
110+
int singleParamWithATrickyName(@Param(ParamNameResolver.GENERIC_NAME_PREFIX + "2") Country country);
103111
}

src/test/java/org/apache/ibatis/submitted/keygen/CountryMapper.xml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?xml version="1.0" encoding="UTF-8"?>
22
<!--
33
4-
Copyright 2009-2018 the original author or authors.
4+
Copyright 2009-2019 the original author or authors.
55
66
Licensed under the Apache License, Version 2.0 (the "License");
77
you may not use this file except in compliance with the License.
@@ -132,4 +132,11 @@
132132
insert into planet (name) values (#{planet.name});
133133
insert into country (countryname,countrycode) values (#{country.countryname},#{country.countrycode});
134134
</insert>
135+
<insert id="insertWeirdCountries" useGeneratedKeys="true" keyProperty="id">
136+
insert into country (countryname,countrycode)
137+
values
138+
<foreach collection="list" separator="," item="country">
139+
(#{country.countryname},#{country.countrycode})
140+
</foreach>
141+
</insert>
135142
</mapper>

src/test/java/org/apache/ibatis/submitted/keygen/Jdbc3KeyGeneratorTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,4 +556,38 @@ void shouldFailIfTooManyGeneratedKeys_Batch() {
556556
}
557557
}
558558
}
559+
560+
@Test
561+
void shouldAssignKeysToListWithoutInvokingEqualsNorHashCode() {
562+
// gh-1719
563+
try (SqlSession sqlSession = sqlSessionFactory.openSession()) {
564+
try {
565+
CountryMapper mapper = sqlSession.getMapper(CountryMapper.class);
566+
List<NpeCountry> countries = new ArrayList<>();
567+
countries.add(new NpeCountry("China", "CN"));
568+
countries.add(new NpeCountry("United Kiongdom", "GB"));
569+
countries.add(new NpeCountry("United States of America", "US"));
570+
mapper.insertWeirdCountries(countries);
571+
for (NpeCountry country : countries) {
572+
assertNotNull(country.getId());
573+
}
574+
} finally {
575+
sqlSession.rollback();
576+
}
577+
}
578+
}
579+
580+
@Test
581+
void shouldAssignKeyToAParamWithTrickyName() {
582+
try (SqlSession sqlSession = sqlSessionFactory.openSession()) {
583+
try {
584+
CountryMapper mapper = sqlSession.getMapper(CountryMapper.class);
585+
Country country = new Country("China", "CN");
586+
mapper.singleParamWithATrickyName(country);
587+
assertNotNull(country.getId());
588+
} finally {
589+
sqlSession.rollback();
590+
}
591+
}
592+
}
559593
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* Copyright 2009-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.apache.ibatis.submitted.keygen;
18+
19+
// See gh-1719
20+
public class NpeCountry {
21+
private Integer id;
22+
private String countryname;
23+
private String countrycode;
24+
25+
public NpeCountry() {
26+
}
27+
28+
public NpeCountry(String countryname, String countrycode) {
29+
this.countryname = countryname;
30+
this.countrycode = countrycode;
31+
}
32+
33+
public Integer getId() {
34+
return id;
35+
}
36+
37+
public void setId(Integer id) {
38+
this.id = id;
39+
}
40+
41+
public String getCountryname() {
42+
return countryname;
43+
}
44+
45+
public void setCountryname(String countryname) {
46+
this.countryname = countryname;
47+
}
48+
49+
public String getCountrycode() {
50+
return countrycode;
51+
}
52+
53+
public void setCountrycode(String countrycode) {
54+
this.countrycode = countrycode;
55+
}
56+
57+
@Override
58+
public boolean equals(Object o) {
59+
if (this == o) {
60+
return true;
61+
}
62+
if (o == null || getClass() != o.getClass()) {
63+
return false;
64+
}
65+
NpeCountry other = (NpeCountry) o;
66+
// throws NPE when id is null
67+
return id.equals(other.id);
68+
}
69+
70+
@Override
71+
public int hashCode() {
72+
// throws NPE when id is null
73+
return id.hashCode();
74+
}
75+
}

0 commit comments

Comments
 (0)