fixes #1523 Better error message when too many keys are generated

Author: harawata

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

@@ -54,6 +54,9 @@ public class Jdbc3KeyGenerator implements KeyGenerator {
    */
   public static final Jdbc3KeyGenerator INSTANCE = new Jdbc3KeyGenerator();
 
+  private static final String MSG_TOO_MANY_KEYS = "Too many keys are generated. There are only %d target objects. "
+      + "You either specified a wrong 'keyProperty' or encountered a driver bug like #1523.";
+
   @Override
   public void processBefore(Executor executor, MappedStatement ms, Statement stmt, Object parameter) {
     // do nothing
@@ -110,6 +113,9 @@ private void assignKeysToParam(Configuration configuration, ResultSet rs, Result
     }
     Iterator<?> iterator = params.iterator();
     while (rs.next()) {
+      if (!iterator.hasNext()) {
+        throw new ExecutorException(String.format(MSG_TOO_MANY_KEYS, params.size()));
+      }
       Object param = iterator.next();
       assignerList.forEach(x -> x.assign(rs, param));
     }
@@ -119,7 +125,11 @@ private void assignKeysToParamMapList(Configuration configuration, ResultSet rs,
       String[] keyProperties, ArrayList<ParamMap<?>> paramMapList) throws SQLException {
     Iterator<ParamMap<?>> iterator = paramMapList.iterator();
     List<KeyAssigner> assignerList = new ArrayList<>();
+    long counter = 0;
     while (rs.next()) {
+      if (!iterator.hasNext()) {
+        throw new ExecutorException(String.format(MSG_TOO_MANY_KEYS, counter));
+      }
       ParamMap<?> paramMap = iterator.next();
       if (assignerList.isEmpty()) {
         for (int i = 0; i < keyProperties.length; i++) {
@@ -129,6 +139,7 @@ private void assignKeysToParamMapList(Configuration configuration, ResultSet rs,
         }
       }
       assignerList.forEach(x -> x.assign(rs, paramMap));
+      counter++;
     }
   }
 
@@ -145,11 +156,16 @@ private void assignKeysToParamMap(Configuration configuration, ResultSet rs, Res
           k -> entry(collectionize(paramMap.get(k)).iterator(), new ArrayList<>()));
       iteratorPair.getValue().add(entry.getValue());
     }
+    long counter = 0;
     while (rs.next()) {
       for (Entry<Iterator<?>, List<KeyAssigner>> pair : assignerMap.values()) {
+        if (!pair.getKey().hasNext()) {
+          throw new ExecutorException(String.format(MSG_TOO_MANY_KEYS, counter));
+        }
         Object param = pair.getKey().next();
         pair.getValue().forEach(x -> x.assign(rs, param));
       }
+      counter++;
     }
   }
 

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

@@ -91,4 +91,13 @@ int insertMultiParams_keyPropertyWithWrongParamName(@Param("country") Country co
   @Options(useGeneratedKeys = true, keyProperty = "planet.id,map.code")
   @Insert({ "insert into planet (name) values (#{planet.name})" })
   int insertAssignKeysToTwoParams(@Param("planet") Planet planet, @Param("map") Map<String, Object> map);
+
+
+  @Options(useGeneratedKeys = true, keyProperty = "id")
+  @Insert({ "insert into country (countryname,countrycode) values ('a','A'), ('b', 'B')" })
+  int tooManyGeneratedKeys(Country country);
+
+  @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);
 }

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

@@ -512,4 +512,48 @@ void shouldErrorUndefineProperty() {
       }
     }
   }
+
+  @Test
+  void shouldFailIfTooManyGeneratedKeys() {
+    try (SqlSession sqlSession = sqlSessionFactory.openSession()) {
+      try {
+        CountryMapper mapper = sqlSession.getMapper(CountryMapper.class);
+        when(mapper).tooManyGeneratedKeys(new Country());
+        then(caughtException()).isInstanceOf(PersistenceException.class).hasMessageContaining(
+            "Too many keys are generated. There are only 1 target objects.");
+      } finally {
+        sqlSession.rollback();
+      }
+    }
+  }
+
+  @Test
+  void shouldFailIfTooManyGeneratedKeys_ParamMap() {
+    try (SqlSession sqlSession = sqlSessionFactory.openSession()) {
+      try {
+        CountryMapper mapper = sqlSession.getMapper(CountryMapper.class);
+        when(mapper).tooManyGeneratedKeysParamMap(new Country(), 1);
+        then(caughtException()).isInstanceOf(PersistenceException.class).hasMessageContaining(
+            "Too many keys are generated. There are only 1 target objects.");
+      } finally {
+        sqlSession.rollback();
+      }
+    }
+  }
+
+  @Test
+  void shouldFailIfTooManyGeneratedKeys_Batch() {
+    try (SqlSession sqlSession = sqlSessionFactory.openSession(ExecutorType.BATCH)) {
+      try {
+        CountryMapper mapper = sqlSession.getMapper(CountryMapper.class);
+        mapper.tooManyGeneratedKeysParamMap(new Country(), 1);
+        mapper.tooManyGeneratedKeysParamMap(new Country(), 1);
+        when(sqlSession).flushStatements();
+        then(caughtException()).isInstanceOf(PersistenceException.class).hasMessageContaining(
+            "Too many keys are generated. There are only 2 target objects.");
+      } finally {
+        sqlSession.rollback();
+      }
+    }
+  }
 }