Polish JDBC KeyHolder support

See gh-24655

spring-jdbc/src/main/java/org/springframework/jdbc/support/GeneratedKeyHolder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -66,25 +66,25 @@ public class GeneratedKeyHolder implements KeyHolder {
 
 	@Override
 	@Nullable
-	public <T> T getKeyAs(Class<T> keyClass) throws InvalidDataAccessApiUsageException, DataRetrievalFailureException {
+	public <T> T getKeyAs(Class<T> keyType) throws InvalidDataAccessApiUsageException, DataRetrievalFailureException {
 		if (this.keyList.isEmpty()) {
 			return null;
 		}
 		if (this.keyList.size() > 1 || this.keyList.get(0).size() > 1) {
 			throw new InvalidDataAccessApiUsageException(
-					"The getKey method should only be used when a single key is returned.  " +
+					"The getKey method should only be used when a single key is returned. " +
 					"The current key entry contains multiple keys: " + this.keyList);
 		}
 		Iterator<Object> keyIter = this.keyList.get(0).values().iterator();
 		if (keyIter.hasNext()) {
 			Object key = keyIter.next();
-			if (key == null || !(keyClass.isAssignableFrom(key.getClass()))) {
+			if (key == null || !(keyType.isAssignableFrom(key.getClass()))) {
 				throw new DataRetrievalFailureException(
-						"The generated key is not of a supported type. " +
+						"The generated key type is not supported. " +
 						"Unable to cast [" + (key != null ? key.getClass().getName() : null) +
-						"] to [" + keyClass.getName() + "]");
+						"] to [" + keyType.getName() + "].");
 			}
-			return keyClass.cast(key);
+			return keyType.cast(key);
 		}
 		else {
 			throw new DataRetrievalFailureException("Unable to retrieve the generated key. " +
@@ -100,7 +100,7 @@ public class GeneratedKeyHolder implements KeyHolder {
 		}
 		if (this.keyList.size() > 1) {
 			throw new InvalidDataAccessApiUsageException(
-					"The getKeys method should only be used when keys for a single row are returned.  " +
+					"The getKeys method should only be used when keys for a single row are returned. " +
 					"The current key list contains keys for multiple rows: " + this.keyList);
 		}
 		return this.keyList.get(0);

spring-jdbc/src/main/java/org/springframework/jdbc/support/KeyHolder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -31,9 +31,10 @@ import org.springframework.lang.Nullable;
  * for each row of keys.
  *
  * <p>Most applications only use one key per row and process only one row at a
- * time in an insert statement. In these cases, just call {@code getKey}
- * to retrieve the key. The returned value is a Number here, which is the
- * usual type for auto-generated keys.
+ * time in an insert statement. In these cases, just call {@link #getKey() getKey}
+ * or {@link #getKeyAs(Class) getKeyAs} to retrieve the key. The value returned
+ * by {@code getKey} is a {@link Number}, which is the usual type for auto-generated
+ * keys. For any other auto-generated key type, use {@code getKeyAs} instead.
  *
  * @author Thomas Risberg
  * @author Juergen Hoeller
@@ -54,25 +55,28 @@ public interface KeyHolder {
 	 * then an InvalidDataAccessApiUsageException is thrown.
 	 * @return the generated key as a number
 	 * @throws InvalidDataAccessApiUsageException if multiple keys are encountered
+	 * @see #getKeyAs(Class)
 	 */
 	@Nullable
 	Number getKey() throws InvalidDataAccessApiUsageException;
 
 	/**
 	 * Retrieve the first item from the first map, assuming that there is just
-	 * one item and just one map, and that the item is an instance of provided class.
-	 * This is the typical case: a single generated key of desired class.
+	 * one item and just one map, and that the item is an instance of specified type.
+	 * This is a common case: a single generated key of the specified type.
 	 * <p>Keys are held in a List of Maps, where each item in the list represents
 	 * the keys for each row. If there are multiple columns, then the Map will have
 	 * multiple entries as well. If this method encounters multiple entries in
 	 * either the map or the list meaning that multiple keys were returned,
 	 * then an InvalidDataAccessApiUsageException is thrown.
-	 * @param keyClass class of the requested key
-	 * @return the generated key as an instance of provided class
+	 * @param keyType the type of the auto-generated key
+	 * @return the generated key as an instance of specified type
 	 * @throws InvalidDataAccessApiUsageException if multiple keys are encountered
+	 * @since 5.3
+	 * @see #getKey()
 	 */
 	@Nullable
-	<T> T getKeyAs(Class<T> keyClass) throws InvalidDataAccessApiUsageException;
+	<T> T getKeyAs(Class<T> keyType) throws InvalidDataAccessApiUsageException;
 
 	/**
 	 * Retrieve the first map of keys.

spring-jdbc/src/test/java/org/springframework/jdbc/support/KeyHolderTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -26,7 +26,6 @@ import org.springframework.dao.InvalidDataAccessApiUsageException;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.emptyMap;
-import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -38,87 +37,86 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
  * @author Sam Brannen
  * @since July 18, 2004
  */
-@SuppressWarnings("serial")
-public class KeyHolderTests {
+class KeyHolderTests {
 
 	private final KeyHolder kh = new GeneratedKeyHolder();
 
 
 	@Test
-	public void singleKey() {
-		kh.getKeyList().addAll(singletonList(singletonMap("key", 1)));
+	void getKeyForSingleNumericKey() {
+		kh.getKeyList().add(singletonMap("key", 1));
 
-		assertThat(kh.getKey().intValue()).as("single key should be returned").isEqualTo(1);
+		assertThat(kh.getKey()).as("single key should be returned").isEqualTo(1);
 	}
 
 	@Test
-	public void singleKeyAsString() {
-		kh.getKeyList().addAll(singletonList(singletonMap("key", "1")));
+	void getKeyForSingleNonNumericKey() {
+		kh.getKeyList().add(singletonMap("key", "ABC"));
 
-		assertThat(kh.getKeyAs(String.class)).as("single key should be returned").isEqualTo("1");
+		assertThatExceptionOfType(DataRetrievalFailureException.class)
+			.isThrownBy(() -> kh.getKey())
+			.withMessage("The generated key type is not supported. Unable to cast [java.lang.String] to [java.lang.Number].");
 	}
 
 	@Test
-	public void singleKeyAsWrongClass() {
-		kh.getKeyList().addAll(singletonList(singletonMap("key", "1")));
+	void getKeyWithNoKeysInMap() {
+		kh.getKeyList().add(emptyMap());
 
-		assertThatExceptionOfType(DataRetrievalFailureException.class).isThrownBy(() ->
-				kh.getKeyAs(Integer.class))
-				.withMessageStartingWith("The generated key is not of a supported type.");
+		assertThatExceptionOfType(DataRetrievalFailureException.class)
+			.isThrownBy(() -> kh.getKey())
+			.withMessageStartingWith("Unable to retrieve the generated key.");
 	}
 
 	@Test
-	public void singleKeyWithNullValue() {
-		kh.getKeyList().addAll(singletonList(singletonMap("key", null)));
+	void getKeyWithMultipleKeysInMap() {
+		Map<String, Object> m = new HashMap<String, Object>() {{
+			put("key", 1);
+			put("seq", 2);
+		}};
+		kh.getKeyList().add(m);
 
-		assertThatExceptionOfType(DataRetrievalFailureException.class).isThrownBy(() ->
-				kh.getKeyAs(Integer.class))
-				.withMessageStartingWith("The generated key is not of a supported type.");
+		assertThat(kh.getKeys()).as("two keys should be in the map").hasSize(2);
+		assertThatExceptionOfType(InvalidDataAccessApiUsageException.class)
+			.isThrownBy(() -> kh.getKey())
+			.withMessageStartingWith("The getKey method should only be used when a single key is returned.");
 	}
 
 	@Test
-	public void singleKeyNonNumeric() {
-		kh.getKeyList().addAll(singletonList(singletonMap("key", "1")));
+	void getKeyAsStringForSingleKey() {
+		kh.getKeyList().add(singletonMap("key", "ABC"));
 
-		assertThatExceptionOfType(DataRetrievalFailureException.class).isThrownBy(() ->
-				kh.getKey().intValue())
-			.withMessageStartingWith("The generated key is not of a supported type.");
+		assertThat(kh.getKeyAs(String.class)).as("single key should be returned").isEqualTo("ABC");
 	}
 
 	@Test
-	public void noKeyReturnedInMap() {
-		kh.getKeyList().addAll(singletonList(emptyMap()));
+	void getKeyAsWrongType() {
+		kh.getKeyList().add(singletonMap("key", "ABC"));
 
-		assertThatExceptionOfType(DataRetrievalFailureException.class).isThrownBy(() ->
-				kh.getKey())
-			.withMessageStartingWith("Unable to retrieve the generated key.");
+		assertThatExceptionOfType(DataRetrievalFailureException.class)
+			.isThrownBy(() -> kh.getKeyAs(Integer.class))
+			.withMessage("The generated key type is not supported. Unable to cast [java.lang.String] to [java.lang.Integer].");
 	}
 
 	@Test
-	public void multipleKeys() {
-		Map<String, Object> m = new HashMap<String, Object>() {{
-			put("key", 1);
-			put("seq", 2);
-		}};
-		kh.getKeyList().addAll(singletonList(m));
+	void getKeyAsIntegerWithNullValue() {
+		kh.getKeyList().add(singletonMap("key", null));
 
-		assertThat(kh.getKeys().size()).as("two keys should be in the map").isEqualTo(2);
-		assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(() ->
-				kh.getKey())
-			.withMessageStartingWith("The getKey method should only be used when a single key is returned.");
+		assertThatExceptionOfType(DataRetrievalFailureException.class)
+			.isThrownBy(() -> kh.getKeyAs(Integer.class))
+			.withMessage("The generated key type is not supported. Unable to cast [null] to [java.lang.Integer].");
 	}
 
 	@Test
-	public void multipleKeyRows() {
+	void getKeysWithMultipleKeyRows() {
 		Map<String, Object> m = new HashMap<String, Object>() {{
 			put("key", 1);
 			put("seq", 2);
 		}};
 		kh.getKeyList().addAll(asList(m, m));
 
-		assertThat(kh.getKeyList().size()).as("two rows should be in the list").isEqualTo(2);
-		assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(() ->
-				kh.getKeys())
+		assertThat(kh.getKeyList()).as("two rows should be in the list").hasSize(2);
+		assertThatExceptionOfType(InvalidDataAccessApiUsageException.class)
+			.isThrownBy(() -> kh.getKeys())
 			.withMessageStartingWith("The getKeys method should only be used when keys for a single row are returned.");
 	}