diff --git a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index 3297d9d4..c16b7eeb 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -89,25 +89,25 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { MapSqlParameterSource parameterSource = getPropertyMap(instance, persistentEntity); Object idValue = getIdValueOrNull(instance, persistentEntity); - JdbcPersistentProperty idProperty = persistentEntity.getRequiredIdProperty(); - parameterSource.addValue(idProperty.getColumnName(), convert(idValue, idProperty.getColumnType()), - JdbcUtil.sqlTypeFor(idProperty.getColumnType())); + JdbcPersistentProperty idProperty = persistentEntity.getIdProperty(); + + if (idValue != null) { + additionalParameters.put(idProperty.getColumnName(), convert(idValue, idProperty.getColumnType())); + } additionalParameters.forEach(parameterSource::addValue); - boolean idValueDoesNotComeFromEntity = // - idValue == null // - || additionalParameters.containsKey(idProperty.getColumnName()); - operations.update( // - sql(domainType).getInsert(idValueDoesNotComeFromEntity, additionalParameters.keySet()), // + sql(domainType).getInsert(additionalParameters.keySet()), // parameterSource, // holder // ); setIdFromJdbc(instance, holder, persistentEntity); - if (entityInformation.isNew(instance)) { + // if there is an id property and it was null before the save + // The database should have created an id and provided it. + if (idProperty != null && idValue == null && entityInformation.isNew(instance)) { throw new IllegalStateException(String.format(ENTITY_NEW_AFTER_INSERT, persistentEntity)); } @@ -198,6 +198,8 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { @Override public Iterable findAllByProperty(Object rootId, JdbcPersistentProperty property) { + Assert.notNull(rootId, "rootId must not be null."); + Class actualType = property.getActualType(); String findAllByProperty = sql(actualType).getFindAllByProperty(property.getReverseColumnName(), property.getKeyColumn(), property.isOrdered()); diff --git a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java index 78b80852..89dae3e2 100644 --- a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java @@ -35,6 +35,8 @@ import org.springframework.data.mapping.PreferredConstructor.Parameter; import org.springframework.data.mapping.model.ConvertingPropertyAccessor; import org.springframework.data.mapping.model.ParameterValueProvider; import org.springframework.jdbc.core.RowMapper; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * Maps a ResultSet to an entity of type {@code T}, including entities referenced. @@ -62,7 +64,7 @@ public class EntityRowMapper implements RowMapper { this.context = context; this.accessStrategy = accessStrategy; - idProperty = entity.getRequiredIdProperty(); + idProperty = entity.getIdProperty(); } /* @@ -77,7 +79,7 @@ public class EntityRowMapper implements RowMapper { ConvertingPropertyAccessor propertyAccessor = new ConvertingPropertyAccessor(entity.getPropertyAccessor(result), conversions); - Object id = readFrom(resultSet, idProperty, ""); + Object id = idProperty == null ? null : readFrom(resultSet, idProperty, ""); for (JdbcPersistentProperty property : entity) { diff --git a/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java b/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java index 369fc9c1..282269bd 100644 --- a/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java +++ b/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java @@ -27,6 +27,7 @@ import org.springframework.util.Assert; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -121,8 +122,8 @@ class SqlGenerator { return findOneSql.get(); } - String getInsert(boolean excludeId, Set additionalColumns) { - return createInsertSql(excludeId, additionalColumns); + String getInsert(Set additionalColumns) { + return createInsertSql(additionalColumns); } String getUpdate() { @@ -237,11 +238,11 @@ class SqlGenerator { return String.format("select count(*) from %s", entity.getTableName()); } - private String createInsertSql(boolean excludeId, Set additionalColumns) { + private String createInsertSql(Set additionalColumns) { String insertTemplate = "insert into %s (%s) values (%s)"; - List columnNamesForInsert = new ArrayList<>(excludeId ? nonIdColumnNames : columnNames); + LinkedHashSet columnNamesForInsert = new LinkedHashSet<>(nonIdColumnNames); columnNamesForInsert.addAll(additionalColumns); String tableColumns = String.join(", ", columnNamesForInsert); diff --git a/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java index 6ffbe140..6c3ae9f2 100644 --- a/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java @@ -42,7 +42,7 @@ public class DefaultDataAccessStrategyUnitTests { NamedParameterJdbcOperations jdbcOperations = mock(NamedParameterJdbcOperations.class); JdbcMappingContext context = new JdbcMappingContext(new DefaultNamingStrategy(), jdbcOperations, __ -> {}); HashMap additionalParameters = new HashMap<>(); - ArgumentCaptor captor = ArgumentCaptor.forClass(SqlParameterSource.class); + ArgumentCaptor paramSourceCaptor = ArgumentCaptor.forClass(SqlParameterSource.class); DefaultDataAccessStrategy accessStrategy = new DefaultDataAccessStrategy( // new SqlGeneratorSource(context), // @@ -57,21 +57,25 @@ public class DefaultDataAccessStrategyUnitTests { accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, additionalParameters); - verify(jdbcOperations).update(eq("insert into DummyEntity (id) values (:id)"), captor.capture(), + verify(jdbcOperations).update(eq("insert into DummyEntity (id) values (:id)"), paramSourceCaptor.capture(), any(KeyHolder.class)); - assertThat(captor.getValue().getValue("id")).isEqualTo(ID_FROM_ADDITIONAL_VALUES); } @Test // DATAJDBC-146 public void additionalParametersGetAddedToStatement() { + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + additionalParameters.put("reference", ID_FROM_ADDITIONAL_VALUES); accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, additionalParameters); - verify(jdbcOperations).update(eq("insert into DummyEntity (id, reference) values (:id, :reference)"), - captor.capture(), any(KeyHolder.class)); - assertThat(captor.getValue().getValue("id")).isEqualTo(ORIGINAL_ID); + verify(jdbcOperations).update(sqlCaptor.capture(), paramSourceCaptor.capture(), any(KeyHolder.class)); + + assertThat(sqlCaptor.getValue()) // + .containsSequence("insert into DummyEntity (", "id", ") values (", ":id", ")") // + .containsSequence("insert into DummyEntity (", "reference", ") values (", ":reference", ")"); + assertThat(paramSourceCaptor.getValue().getValue("id")).isEqualTo(ORIGINAL_ID); } @RequiredArgsConstructor