diff --git a/pom.xml b/pom.xml index f0b2a42b23..fb77d8b607 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1502-no-id-exception-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 84eeb178e0..1765247bd2 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1502-no-id-exception-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 266af71b96..5003397761 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 3.4.0-SNAPSHOT + 3.4.0-1502-no-id-exception-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1502-no-id-exception-SNAPSHOT diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java index efa04a8d8b..c5c84b5c4b 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java @@ -165,6 +165,8 @@ public T save(T instance) { Assert.notNull(instance, "Aggregate instance must not be null"); + verifyIdProperty(instance); + return performSave(new EntityAndChangeCreator<>(instance, changeCreatorSelectorForSave(instance))); } @@ -179,6 +181,7 @@ public Iterable saveAll(Iterable instances) { List> entityAndChangeCreators = new ArrayList<>(); for (T instance : instances) { + verifyIdProperty(instance); entityAndChangeCreators.add(new EntityAndChangeCreator<>(instance, changeCreatorSelectorForSave(instance))); } return performSaveAll(entityAndChangeCreators); @@ -425,6 +428,12 @@ public void deleteAll(Iterable instances) { } } + private void verifyIdProperty(T instance) { + + Class type = instance.getClass(); + Assert.isTrue(context.getRequiredPersistentEntity(type).hasIdProperty(),() -> "Aggregate root must have an id property. " + type.getName() + " has none"); + } + private void doDeleteAll(Iterable instances, Class domainType) { BatchingAggregateChange> batchingAggregateChange = BatchingAggregateChange diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java index d9e6ed3368..4d38496805 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java @@ -32,10 +32,12 @@ import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; import org.springframework.data.jdbc.core.convert.DataAccessStrategy; +import org.springframework.data.jdbc.core.convert.Identifier; import org.springframework.data.jdbc.core.convert.JdbcConverter; import org.springframework.data.jdbc.core.convert.MappingJdbcConverter; import org.springframework.data.jdbc.core.convert.RelationResolver; import org.springframework.data.mapping.callback.EntityCallbacks; +import org.springframework.data.relational.core.conversion.IdValueSource; import org.springframework.data.relational.core.conversion.MutableAggregateChange; import org.springframework.data.relational.core.mapping.Column; import org.springframework.data.relational.core.mapping.RelationalMappingContext; @@ -46,6 +48,8 @@ import org.springframework.data.relational.core.mapping.event.BeforeDeleteCallback; import org.springframework.data.relational.core.mapping.event.BeforeSaveCallback; +import java.util.List; + /** * Unit tests for {@link JdbcAggregateTemplate}. * @@ -59,17 +63,13 @@ public class JdbcAggregateTemplateUnitTests { JdbcAggregateTemplate template; - @Mock - DataAccessStrategy dataAccessStrategy; - @Mock - ApplicationEventPublisher eventPublisher; - @Mock - RelationResolver relationResolver; - @Mock - EntityCallbacks callbacks; + @Mock DataAccessStrategy dataAccessStrategy; + @Mock ApplicationEventPublisher eventPublisher; + @Mock RelationResolver relationResolver; + @Mock EntityCallbacks callbacks; @BeforeEach - public void setUp() { + void setUp() { RelationalMappingContext mappingContext = new RelationalMappingContext(); JdbcConverter converter = new MappingJdbcConverter(mappingContext, relationResolver); @@ -80,24 +80,24 @@ public void setUp() { } @Test // DATAJDBC-378 - public void findAllByIdMustNotAcceptNullArgumentForType() { + void findAllByIdMustNotAcceptNullArgumentForType() { assertThatThrownBy(() -> template.findAllById(singleton(23L), null)).isInstanceOf(IllegalArgumentException.class); } @Test // DATAJDBC-378 - public void findAllByIdMustNotAcceptNullArgumentForIds() { + void findAllByIdMustNotAcceptNullArgumentForIds() { assertThatThrownBy(() -> template.findAllById(null, SampleEntity.class)) .isInstanceOf(IllegalArgumentException.class); } @Test // DATAJDBC-378 - public void findAllByIdWithEmptyListMustReturnEmptyResult() { + void findAllByIdWithEmptyListMustReturnEmptyResult() { assertThat(template.findAllById(emptyList(), SampleEntity.class)).isEmpty(); } @Test // DATAJDBC-393, GH-1291 - public void callbackOnSave() { + void callbackOnSave() { SampleEntity first = new SampleEntity(null, "Alfred"); SampleEntity second = new SampleEntity(23L, "Alfred E."); @@ -115,7 +115,7 @@ public void callbackOnSave() { } @Test // GH-1291 - public void doesNotEmitEvents() { + void doesNotEmitEvents() { SampleEntity first = new SampleEntity(null, "Alfred"); SampleEntity second = new SampleEntity(23L, "Alfred E."); @@ -129,8 +129,7 @@ public void doesNotEmitEvents() { verifyNoInteractions(eventPublisher); } - @Test - // GH-1137 + @Test // GH-1137 void savePreparesInstanceWithInitialVersion_onInsert() { EntityWithVersion entity = new EntityWithVersion(1L); @@ -145,8 +144,7 @@ void savePreparesInstanceWithInitialVersion_onInsert() { assertThat(afterConvert.getVersion()).isEqualTo(0L); } - @Test - // GH-1137 + @Test // GH-1137 void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsImmutable() { EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, null); @@ -161,8 +159,7 @@ void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsImmuta assertThat(afterConvert.getVersion()).isEqualTo(0L); } - @Test - // GH-1137 + @Test // GH-1137 void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsPrimitiveType() { EntityWithPrimitiveVersion entity = new EntityWithPrimitiveVersion(1L); @@ -177,8 +174,7 @@ void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsPrimit assertThat(afterConvert.getVersion()).isEqualTo(1L); } - @Test - // GH-1137 + @Test // GH-1137 void savePreparesInstanceWithInitialVersion_onInsert__whenVersionPropertyIsImmutableAndPrimitiveType() { EntityWithImmutablePrimitiveVersion entity = new EntityWithImmutablePrimitiveVersion(1L, 0L); @@ -194,8 +190,7 @@ void savePreparesInstanceWithInitialVersion_onInsert__whenVersionPropertyIsImmut assertThat(afterConvert.getVersion()).isEqualTo(1L); } - @Test - // GH-1137 + @Test // GH-1137 void savePreparesChangeWithPreviousVersion_onUpdate() { when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); @@ -212,8 +207,7 @@ void savePreparesChangeWithPreviousVersion_onUpdate() { assertThat(aggregateChange.getPreviousVersion()).isEqualTo(1L); } - @Test - // GH-1137 + @Test // GH-1137 void savePreparesInstanceWithNextVersion_onUpdate() { when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); @@ -230,8 +224,7 @@ void savePreparesInstanceWithNextVersion_onUpdate() { assertThat(afterConvert.getVersion()).isEqualTo(2L); } - @Test - // GH-1137 + @Test // GH-1137 void savePreparesInstanceWithNextVersion_onUpdate_whenVersionPropertyIsImmutable() { when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); @@ -246,8 +239,7 @@ void savePreparesInstanceWithNextVersion_onUpdate_whenVersionPropertyIsImmutable assertThat(afterConvert.getVersion()).isEqualTo(2L); } - @Test - // GH-1137 + @Test // GH-1137 void deletePreparesChangeWithPreviousVersion_onDeleteByInstance() { EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, 1L); @@ -263,7 +255,7 @@ void deletePreparesChangeWithPreviousVersion_onDeleteByInstance() { } @Test // DATAJDBC-393 - public void callbackOnDelete() { + void callbackOnDelete() { SampleEntity first = new SampleEntity(23L, "Alfred"); SampleEntity second = new SampleEntity(23L, "Alfred E."); @@ -277,7 +269,7 @@ public void callbackOnDelete() { } @Test // DATAJDBC-101 - public void callbackOnLoadSorted() { + void callbackOnLoadSorted() { SampleEntity alfred1 = new SampleEntity(23L, "Alfred"); SampleEntity alfred2 = new SampleEntity(23L, "Alfred E."); @@ -299,7 +291,7 @@ public void callbackOnLoadSorted() { } @Test // DATAJDBC-101 - public void callbackOnLoadPaged() { + void callbackOnLoadPaged() { SampleEntity alfred1 = new SampleEntity(23L, "Alfred"); SampleEntity alfred2 = new SampleEntity(23L, "Alfred E."); @@ -321,35 +313,66 @@ public void callbackOnLoadPaged() { } @Test // GH-1401 - public void saveAllWithEmptyListDoesNothing() { + void saveAllWithEmptyListDoesNothing() { assertThat(template.saveAll(emptyList())).isEmpty(); } @Test // GH-1401 - public void insertAllWithEmptyListDoesNothing() { + void insertAllWithEmptyListDoesNothing() { assertThat(template.insertAll(emptyList())).isEmpty(); } @Test // GH-1401 - public void updateAllWithEmptyListDoesNothing() { + void updateAllWithEmptyListDoesNothing() { assertThat(template.updateAll(emptyList())).isEmpty(); } @Test // GH-1401 - public void deleteAllWithEmptyListDoesNothing() { + void deleteAllWithEmptyListDoesNothing() { template.deleteAll(emptyList()); } @Test // GH-1401 - public void deleteAllByIdWithEmptyListDoesNothing() { + void deleteAllByIdWithEmptyListDoesNothing() { template.deleteAllById(emptyList(), SampleEntity.class); } + @Test // GH-1502 + void saveThrowsExceptionWhenIdIsNotSet() { + + SampleEntity alfred = new SampleEntity(null, "Alfred"); + when(callbacks.callback(any(), any(), any(Object[].class))).thenReturn(alfred); + + when(dataAccessStrategy.insert(eq(alfred), any(Class.class), any(Identifier.class), any(IdValueSource.class))) + .thenReturn(null); + + assertThatIllegalArgumentException().isThrownBy(() -> template.save(alfred)) + .withMessage("After saving the identifier must not be null"); + } + + @Test // GH-1502 + void saveThrowsExceptionWhenIdDoesNotExist() { + + NoIdEntity alfred = new NoIdEntity("Alfred"); + + assertThatIllegalArgumentException().isThrownBy(() -> template.save(alfred)) + .withMessageContaining("Aggregate root must have an id property. " + NoIdEntity.class.getName() + " has none"); + } + + @Test // GH-1502 + void saveThrowsExceptionWhenIdDoesNotExistOnSaveAll() { + + NoIdEntity alfred = new NoIdEntity("Alfred"); + NoIdEntity berta = new NoIdEntity("Berta"); + + assertThatIllegalArgumentException().isThrownBy(() -> template.saveAll( List.of(alfred, berta))) + .withMessageContaining("Aggregate root must have an id property. " + NoIdEntity.class.getName() + " has none"); + } + private static class SampleEntity { @Column("id1") - @Id - private Long id; + @Id private Long id; private String name; @@ -366,11 +389,11 @@ public String getName() { return this.name; } - public void setId(Long id) { + void setId(Long id) { this.id = id; } - public void setName(String name) { + void setName(String name) { this.name = name; } } @@ -378,11 +401,9 @@ public void setName(String name) { private static class EntityWithVersion { @Column("id1") - @Id - private final Long id; + @Id private final Long id; - @Version - private Long version; + @Version private Long version; public EntityWithVersion(Long id) { this.id = id; @@ -396,7 +417,7 @@ public Long getVersion() { return this.version; } - public void setVersion(Long version) { + void setVersion(Long version) { this.version = version; } } @@ -404,11 +425,9 @@ public void setVersion(Long version) { private static class EntityWithImmutableVersion { @Column("id1") - @Id - private final Long id; + @Id private final Long id; - @Version - private final Long version; + @Version private final Long version; public EntityWithImmutableVersion(Long id, Long version) { this.id = id; @@ -427,11 +446,9 @@ public Long getVersion() { private static class EntityWithPrimitiveVersion { @Column("id1") - @Id - private final Long id; + @Id private final Long id; - @Version - private long version; + @Version private long version; public EntityWithPrimitiveVersion(Long id) { this.id = id; @@ -445,7 +462,7 @@ public long getVersion() { return this.version; } - public void setVersion(long version) { + void setVersion(long version) { this.version = version; } } @@ -453,11 +470,9 @@ public void setVersion(long version) { private static class EntityWithImmutablePrimitiveVersion { @Column("id1") - @Id - private final Long id; + @Id private final Long id; - @Version - private final long version; + @Version private final long version; public EntityWithImmutablePrimitiveVersion(Long id, long version) { this.id = id; @@ -472,4 +487,7 @@ public long getVersion() { return this.version; } } + + record NoIdEntity(String name) { + } } diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index 3168f9d4f6..5261446bda 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-r2dbc - 3.4.0-SNAPSHOT + 3.4.0-1502-no-id-exception-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1502-no-id-exception-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 0287ece743..d2d8de03da 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 3.4.0-SNAPSHOT + 3.4.0-1502-no-id-exception-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-1502-no-id-exception-SNAPSHOT