From cc301011b2edd6c6a02bfe5236482038c4b6473a Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 16 Jun 2020 14:17:43 +0200 Subject: [PATCH] Polish JDBC KeyHolder support See gh-24655 --- .../jdbc/support/GeneratedKeyHolder.java | 16 ++-- .../jdbc/support/KeyHolder.java | 22 +++-- .../jdbc/support/KeyHolderTests.java | 86 +++++++++---------- 3 files changed, 63 insertions(+), 61 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/support/GeneratedKeyHolder.java b/spring-jdbc/src/main/java/org/springframework/jdbc/support/GeneratedKeyHolder.java index e27ee701cd0..c2b3293a252 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/support/GeneratedKeyHolder.java +++ b/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 getKeyAs(Class keyClass) throws InvalidDataAccessApiUsageException, DataRetrievalFailureException { + public T getKeyAs(Class 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 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); diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/support/KeyHolder.java b/spring-jdbc/src/main/java/org/springframework/jdbc/support/KeyHolder.java index f3c7d3cab26..46b6c669455 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/support/KeyHolder.java +++ b/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. * *

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

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 getKeyAs(Class keyClass) throws InvalidDataAccessApiUsageException; + T getKeyAs(Class keyType) throws InvalidDataAccessApiUsageException; /** * Retrieve the first map of keys. diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/support/KeyHolderTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/support/KeyHolderTests.java index eefb7858991..4b950c783e2 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/support/KeyHolderTests.java +++ b/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 m = new HashMap() {{ + 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 m = new HashMap() {{ - 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 m = new HashMap() {{ 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."); }