diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index 939dde047c..1b56008783 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -67,6 +67,7 @@ * @author Milan Milanov * @author Myeonghyeon Lee * @author Yunyoung LEE + * @author Radim Tlusty * @since 1.1 */ public class DefaultDataAccessStrategy implements DataAccessStrategy { @@ -121,24 +122,33 @@ public Object insert(T instance, Class domainType, Identifier identifier) addConvertedPropertyValue(parameterSource, idProperty, idValue, idProperty.getColumnName()); } - KeyHolder holder = new GeneratedKeyHolder(); - - IdGeneration idGeneration = sqlGeneratorSource.getDialect().getIdGeneration(); String insertSql = sqlGenerator.getInsert(new HashSet<>(parameterSource.getIdentifiers())); - if (idGeneration.driverRequiresKeyColumnNames()) { + if (idValue == null) { - String[] keyColumnNames = getKeyColumnNames(domainType); - if (keyColumnNames.length == 0) { - operations.update(insertSql, parameterSource, holder); + KeyHolder holder = new GeneratedKeyHolder(); + + IdGeneration idGeneration = sqlGeneratorSource.getDialect().getIdGeneration(); + + if (idGeneration.driverRequiresKeyColumnNames()) { + + String[] keyColumnNames = getKeyColumnNames(domainType); + if (keyColumnNames.length == 0) { + operations.update(insertSql, parameterSource, holder); + } else { + operations.update(insertSql, parameterSource, holder, keyColumnNames); + } } else { - operations.update(insertSql, parameterSource, holder, keyColumnNames); + operations.update(insertSql, parameterSource, holder); } - } else { - operations.update(insertSql, parameterSource, holder); + + return getIdFromHolder(holder, persistentEntity); } + else { - return getIdFromHolder(holder, persistentEntity); + operations.update(insertSql, parameterSource); + return null; + } } /* diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java index e2ee1eb85e..51e3043bc4 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java @@ -58,11 +58,13 @@ * @author Mark Paluch * @author Myeonghyeon Lee * @author Myat Min + * @author Radim Tlusty */ public class DefaultDataAccessStrategyUnitTests { public static final long ID_FROM_ADDITIONAL_VALUES = 23L; public static final long ORIGINAL_ID = 4711L; + public static final long GENERATED_ID = 17; NamedParameterJdbcOperations namedJdbcOperations = mock(NamedParameterJdbcOperations.class); JdbcOperations jdbcOperations = mock(JdbcOperations.class); @@ -99,7 +101,7 @@ public void additionalParameterForIdDoesNotLeadToDuplicateParameters() { accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, Identifier.from(additionalParameters)); verify(namedJdbcOperations).update(eq("INSERT INTO \"DUMMY_ENTITY\" (\"ID\") VALUES (:ID)"), - paramSourceCaptor.capture(), any(KeyHolder.class)); + paramSourceCaptor.capture()); } @Test // DATAJDBC-146 @@ -111,7 +113,7 @@ public void additionalParametersGetAddedToStatement() { accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, Identifier.from(additionalParameters)); - verify(namedJdbcOperations).update(sqlCaptor.capture(), paramSourceCaptor.capture(), any(KeyHolder.class)); + verify(namedJdbcOperations).update(sqlCaptor.capture(), paramSourceCaptor.capture()); assertThat(sqlCaptor.getValue()) // .containsSubsequence("INSERT INTO \"DUMMY_ENTITY\" (", "\"ID\"", ") VALUES (", ":id", ")") // @@ -131,7 +133,7 @@ public void considersConfiguredWriteConverter() { accessStrategy.insert(entity, EntityWithBoolean.class, Identifier.empty()); - verify(namedJdbcOperations).update(sqlCaptor.capture(), paramSourceCaptor.capture(), any(KeyHolder.class)); + verify(namedJdbcOperations).update(sqlCaptor.capture(), paramSourceCaptor.capture()); assertThat(paramSourceCaptor.getValue().getValue("id")).isEqualTo(ORIGINAL_ID); assertThat(paramSourceCaptor.getValue().getValue("flag")).isEqualTo("T"); @@ -150,7 +152,7 @@ public void considersConfiguredWriteConverterForIdValueObjects() { accessStrategy.insert(entity, WithValueObjectId.class, Identifier.empty()); - verify(namedJdbcOperations).update(anyString(), paramSourceCaptor.capture(), any(KeyHolder.class)); + verify(namedJdbcOperations).update(anyString(), paramSourceCaptor.capture()); assertThat(paramSourceCaptor.getValue().getValue("id")).isEqualTo(rawId); assertThat(paramSourceCaptor.getValue().getValue("value")).isEqualTo("vs. superman"); @@ -177,7 +179,7 @@ public void considersConfiguredWriteConverterForIdValueObjectsWhichReferencedInO additionalParameters.put(SqlIdentifier.quoted("DUMMYENTITYROOT"), rootIdValue); accessStrategy.insert(root, DummyEntityRoot.class, Identifier.from(additionalParameters)); - verify(namedJdbcOperations).update(anyString(), paramSourceCaptor.capture(), any(KeyHolder.class)); + verify(namedJdbcOperations).update(anyString(), paramSourceCaptor.capture()); assertThat(paramSourceCaptor.getValue().getValue("id")).isEqualTo(rawId); @@ -191,6 +193,36 @@ public void considersConfiguredWriteConverterForIdValueObjectsWhichReferencedInO assertThat(paramSourceCaptor.getValue().getValue("DUMMYENTITYROOT")).isEqualTo(rawId); } + @Test // gh-933 + public void insertWithDefinedIdDoesNotRetrieveGeneratedKeys() { + + Object generatedId = accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, Identifier.from(additionalParameters)); + + assertThat(generatedId).isNull(); + + verify(namedJdbcOperations).update(eq("INSERT INTO \"DUMMY_ENTITY\" (\"ID\") VALUES (:id)"), + paramSourceCaptor.capture()); + } + + @Test // gh-933 + public void insertWithUndefinedIdRetrievesGeneratedKeys() { + + when(namedJdbcOperations.update(any(), any(), any())) + .then(invocation -> { + + KeyHolder keyHolder = invocation.getArgument(2); + keyHolder.getKeyList().add(singletonMap("ID", GENERATED_ID)); + return 1; + }); + + Object generatedId = accessStrategy.insert(new DummyEntity(null), DummyEntity.class, Identifier.from(additionalParameters)); + + assertThat(generatedId).isEqualTo(GENERATED_ID); + + verify(namedJdbcOperations).update(eq("INSERT INTO \"DUMMY_ENTITY\" VALUES ()"), + paramSourceCaptor.capture(), any(KeyHolder.class)); + } + private DefaultDataAccessStrategy createAccessStrategyWithConverter(List converters) { DelegatingDataAccessStrategy relationResolver = new DelegatingDataAccessStrategy();