Skip to content

Commit

Permalink
Avoid invoking equals/hashCode when assigning generated keys
Browse files Browse the repository at this point in the history
To detect single parameter case, it now checks parameter name instead of checking equality of parameter objects.
This should fix mybatisgh-1719.
  • Loading branch information
harawata committed Nov 9, 2019
1 parent 411a2fd commit a79f7ca
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 9 deletions.
Expand Up @@ -28,13 +28,15 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;

import org.apache.ibatis.binding.MapperMethod.ParamMap;
import org.apache.ibatis.executor.Executor;
import org.apache.ibatis.executor.ExecutorException;
import org.apache.ibatis.mapping.MappedStatement;
import org.apache.ibatis.reflection.ArrayUtil;
import org.apache.ibatis.reflection.MetaObject;
import org.apache.ibatis.reflection.ParamNameResolver;
import org.apache.ibatis.session.Configuration;
import org.apache.ibatis.session.defaults.DefaultSqlSession.StrictMap;
import org.apache.ibatis.type.JdbcType;
Expand All @@ -47,6 +49,8 @@
*/
public class Jdbc3KeyGenerator implements KeyGenerator {

private static final String SECOND_GENERIC_PARAM_NAME = ParamNameResolver.GENERIC_NAME_PREFIX + "2";

/**
* A shared instance.
*
Expand Down Expand Up @@ -171,7 +175,9 @@ private void assignKeysToParamMap(Configuration configuration, ResultSet rs, Res

private Entry<String, KeyAssigner> getAssignerForParamMap(Configuration config, ResultSetMetaData rsmd,
int columnPosition, Map<String, ?> paramMap, String keyProperty, String[] keyProperties, boolean omitParamName) {
boolean singleParam = paramMap.values().stream().distinct().count() == 1;
Set<String> keySet = paramMap.keySet();
// A caveat : if the only parameter has {@code @Param("param2")} on it,.
boolean singleParam = !keySet.contains(SECOND_GENERIC_PARAM_NAME);
int firstDot = keyProperty.indexOf('.');
if (firstDot == -1) {
if (singleParam) {
Expand All @@ -180,10 +186,10 @@ private Entry<String, KeyAssigner> getAssignerForParamMap(Configuration config,
throw new ExecutorException("Could not determine which parameter to assign generated keys to. "
+ "Note that when there are multiple parameters, 'keyProperty' must include the parameter name (e.g. 'param.id'). "
+ "Specified key properties are " + ArrayUtil.toString(keyProperties) + " and available parameters are "
+ paramMap.keySet());
+ keySet);
}
String paramName = keyProperty.substring(0, firstDot);
if (paramMap.containsKey(paramName)) {
if (keySet.contains(paramName)) {
String argParamName = omitParamName ? null : paramName;
String argKeyProperty = keyProperty.substring(firstDot + 1);
return entry(paramName, new KeyAssigner(config, rsmd, columnPosition, argParamName, argKeyProperty));
Expand All @@ -193,7 +199,7 @@ private Entry<String, KeyAssigner> getAssignerForParamMap(Configuration config,
throw new ExecutorException("Could not find parameter '" + paramName + "'. "
+ "Note that when there are multiple parameters, 'keyProperty' must include the parameter name (e.g. 'param.id'). "
+ "Specified key properties are " + ArrayUtil.toString(keyProperties) + " and available parameters are "
+ paramMap.keySet());
+ keySet);
}
}

Expand Down
@@ -1,5 +1,5 @@
/**
* Copyright 2009-2018 the original author or authors.
* Copyright 2009-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -30,7 +30,7 @@

public class ParamNameResolver {

private static final String GENERIC_NAME_PREFIX = "param";
public static final String GENERIC_NAME_PREFIX = "param";

/**
* <p>
Expand Down
Expand Up @@ -22,6 +22,7 @@
import org.apache.ibatis.annotations.Insert;
import org.apache.ibatis.annotations.Options;
import org.apache.ibatis.annotations.Param;
import org.apache.ibatis.reflection.ParamNameResolver;

public interface CountryMapper {

Expand Down Expand Up @@ -100,4 +101,11 @@ int insertMultiParams_keyPropertyWithWrongParamName(@Param("country") Country co
@Options(useGeneratedKeys = true, keyProperty = "country.id")
@Insert({ "insert into country (countryname,countrycode) values ('a','A'), ('b', 'B')" })
int tooManyGeneratedKeysParamMap(@Param("country") Country country, @Param("someId") Integer someId);

int insertWeirdCountries(List<NpeCountry> list);

// If the only parameter has a name 'param2', keyProperty must include the prefix 'param2.'.
@Options(useGeneratedKeys = true, keyProperty = ParamNameResolver.GENERIC_NAME_PREFIX + "2.id")
@Insert({ "insert into country (countryname,countrycode) values (#{param2.countryname},#{param2.countrycode})" })
int singleParamWithATrickyName(@Param(ParamNameResolver.GENERIC_NAME_PREFIX + "2") Country country);
}
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2009-2018 the original author or authors.
Copyright 2009-2019 the original author or authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -132,4 +132,11 @@
insert into planet (name) values (#{planet.name});
insert into country (countryname,countrycode) values (#{country.countryname},#{country.countrycode});
</insert>
<insert id="insertWeirdCountries" useGeneratedKeys="true" keyProperty="id">
insert into country (countryname,countrycode)
values
<foreach collection="list" separator="," item="country">
(#{country.countryname},#{country.countrycode})
</foreach>
</insert>
</mapper>
Expand Up @@ -54,7 +54,7 @@ static void setUp() throws Exception {

// populate in-memory database
BaseDataTest.runScript(sqlSessionFactory.getConfiguration().getEnvironment().getDataSource(),
"org/apache/ibatis/submitted/keygen/CreateDB.sql");
"org/apache/ibatis/submitted/keygen/CreateDB.sql");
}

@Test
Expand Down Expand Up @@ -416,6 +416,7 @@ void shouldAssignMultipleGeneratedKeysToABean_MultiParams() {
}
}
}

@Test
void shouldAssignMultipleGeneratedKeysToABean_MultiParams_batch() {
try (SqlSession sqlSession = sqlSessionFactory.openSession(ExecutorType.BATCH)) {
Expand Down Expand Up @@ -506,7 +507,7 @@ void shouldErrorUndefineProperty() {

when(mapper).insertUndefineKeyProperty(new Country("China", "CN"));
then(caughtException()).isInstanceOf(PersistenceException.class).hasMessageContaining(
"### Error updating database. Cause: org.apache.ibatis.executor.ExecutorException: Error getting generated key or setting result to parameter object. Cause: org.apache.ibatis.executor.ExecutorException: No setter found for the keyProperty 'country_id' in 'org.apache.ibatis.submitted.keygen.Country'.");
"### Error updating database. Cause: org.apache.ibatis.executor.ExecutorException: Error getting generated key or setting result to parameter object. Cause: org.apache.ibatis.executor.ExecutorException: No setter found for the keyProperty 'country_id' in 'org.apache.ibatis.submitted.keygen.Country'.");
} finally {
sqlSession.rollback();
}
Expand Down Expand Up @@ -556,4 +557,38 @@ void shouldFailIfTooManyGeneratedKeys_Batch() {
}
}
}

@Test
void shouldAssignKeysToListWithoutInvokingEqualsNorHashCode() {
// gh-1719
try (SqlSession sqlSession = sqlSessionFactory.openSession()) {
try {
CountryMapper mapper = sqlSession.getMapper(CountryMapper.class);
List<NpeCountry> countries = new ArrayList<>();
countries.add(new NpeCountry("China", "CN"));
countries.add(new NpeCountry("United Kiongdom", "GB"));
countries.add(new NpeCountry("United States of America", "US"));
mapper.insertWeirdCountries(countries);
for (NpeCountry country : countries) {
assertNotNull(country.getId());
}
} finally {
sqlSession.rollback();
}
}
}

@Test
void shouldAssignKeyToAParamWithTrickyName() {
try (SqlSession sqlSession = sqlSessionFactory.openSession()) {
try {
CountryMapper mapper = sqlSession.getMapper(CountryMapper.class);
Country country = new Country("China", "CN");
mapper.singleParamWithATrickyName(country);
assertNotNull(country.getId());
} finally {
sqlSession.rollback();
}
}
}
}
75 changes: 75 additions & 0 deletions src/test/java/org/apache/ibatis/submitted/keygen/NpeCountry.java
@@ -0,0 +1,75 @@
/**
* Copyright 2009-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.ibatis.submitted.keygen;

// See gh-1719
public class NpeCountry {
private Integer id;
private String countryname;
private String countrycode;

public NpeCountry() {
}

public NpeCountry(String countryname, String countrycode) {
this.countryname = countryname;
this.countrycode = countrycode;
}

public Integer getId() {
return id;
}

public void setId(Integer id) {
this.id = id;
}

public String getCountryname() {
return countryname;
}

public void setCountryname(String countryname) {
this.countryname = countryname;
}

public String getCountrycode() {
return countrycode;
}

public void setCountrycode(String countrycode) {
this.countrycode = countrycode;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
NpeCountry other = (NpeCountry) o;
// throws NPE when id is null
return id.equals(other.id);
}

@Override
public int hashCode() {
// throws NPE when id is null
return id.hashCode();
}
}

0 comments on commit a79f7ca

Please sign in to comment.