Merge pull request #1728 from harawata/jdbc3keygenerator-npe

Avoid invoking equals/hashCode when assigning generated keys

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

@@ -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;
@@ -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.
    *
@@ -171,7 +175,10 @@ 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,
+    // it must be referenced with param name e.g. 'param2.x'.
+    boolean singleParam = !keySet.contains(SECOND_GENERIC_PARAM_NAME);
     int firstDot = keyProperty.indexOf('.');
     if (firstDot == -1) {
       if (singleParam) {
@@ -180,10 +187,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));
@@ -193,7 +200,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);
     }
   }
 

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

@@ -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.
@@ -30,7 +30,7 @@
 
 public class ParamNameResolver {
 
-  private static final String GENERIC_NAME_PREFIX = "param";
+  public static final String GENERIC_NAME_PREFIX = "param";
 
   /**
    * <p>

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

@@ -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 {
 
@@ -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);
 }

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

@@ -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.
@@ -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>

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

@@ -556,4 +556,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();
+      }
+    }
+  }
 }

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();
+  }
+}