Skip to content

Commit a4d2f1e

Browse files
schauderschauder
committed
Polishing.
This change extracts entity modifying behaviour into separate methods, so it doesn't appear as an unexpected side effect of the creation of aggregate changes. Also some formatting. Original pull request #1196 See #1137
1 parent 6500bfe commit a4d2f1e

File tree

3 files changed

+64
-31
lines changed

3 files changed

+64
-31
lines changed

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,9 @@ public <T> T save(T instance) {
146146

147147
RelationalPersistentEntity<?> persistentEntity = context.getRequiredPersistentEntity(instance.getClass());
148148

149-
Function<T, MutableAggregateChange<T>> changeCreator = persistentEntity.isNew(instance) ? this::createInsertChange
150-
: this::createUpdateChange;
149+
Function<T, MutableAggregateChange<T>> changeCreator = persistentEntity.isNew(instance)
150+
? entity -> createInsertChange(prepareVersionForInsert(entity))
151+
: entity -> createUpdateChange(prepareVersionForUpdate(entity));
151152

152153
return store(instance, changeCreator, persistentEntity);
153154
}
@@ -166,7 +167,7 @@ public <T> T insert(T instance) {
166167

167168
RelationalPersistentEntity<?> persistentEntity = context.getRequiredPersistentEntity(instance.getClass());
168169

169-
return store(instance, this::createInsertChange, persistentEntity);
170+
return store(instance, entity -> createInsertChange(prepareVersionForInsert(entity)), persistentEntity);
170171
}
171172

172173
/**
@@ -183,7 +184,7 @@ public <T> T update(T instance) {
183184

184185
RelationalPersistentEntity<?> persistentEntity = context.getRequiredPersistentEntity(instance.getClass());
185186

186-
return store(instance, this::createUpdateChange, persistentEntity);
187+
return store(instance, entity -> createUpdateChange(prepareVersionForUpdate(entity)), persistentEntity);
187188
}
188189

189190
@Override
@@ -321,6 +322,21 @@ private <T> void deleteTree(Object id, @Nullable T entity, Class<T> domainType)
321322

322323
private <T> MutableAggregateChange<T> createInsertChange(T instance) {
323324

325+
MutableAggregateChange<T> aggregateChange = MutableAggregateChange.forSave(instance);
326+
jdbcEntityInsertWriter.write(instance, aggregateChange);
327+
return aggregateChange;
328+
}
329+
330+
private <T> MutableAggregateChange<T> createUpdateChange(EntityAndPreviousVersion<T> entityAndVersion) {
331+
332+
MutableAggregateChange<T> aggregateChange = MutableAggregateChange.forSave(entityAndVersion.entity,
333+
entityAndVersion.version);
334+
jdbcEntityUpdateWriter.write(entityAndVersion.entity, aggregateChange);
335+
return aggregateChange;
336+
}
337+
338+
private <T> T prepareVersionForInsert(T instance) {
339+
324340
RelationalPersistentEntity<T> persistentEntity = getRequiredPersistentEntity(instance);
325341
T preparedInstance = instance;
326342
if (persistentEntity.hasVersionProperty()) {
@@ -331,31 +347,26 @@ private <T> MutableAggregateChange<T> createInsertChange(T instance) {
331347
preparedInstance = RelationalEntityVersionUtils.setVersionNumberOnEntity( //
332348
instance, initialVersion, persistentEntity, converter);
333349
}
334-
MutableAggregateChange<T> aggregateChange = MutableAggregateChange.forSave(preparedInstance);
335-
jdbcEntityInsertWriter.write(preparedInstance, aggregateChange);
336-
return aggregateChange;
350+
return preparedInstance;
337351
}
338352

339-
private <T> MutableAggregateChange<T> createUpdateChange(T instance) {
353+
private <T> EntityAndPreviousVersion<T> prepareVersionForUpdate(T instance) {
340354

341355
RelationalPersistentEntity<T> persistentEntity = getRequiredPersistentEntity(instance);
342356
T preparedInstance = instance;
343357
Number previousVersion = null;
344358
if (persistentEntity.hasVersionProperty()) {
345359
// If the root aggregate has a version property, increment it.
346-
previousVersion = RelationalEntityVersionUtils.getVersionNumberFromEntity(instance,
347-
persistentEntity, converter);
360+
previousVersion = RelationalEntityVersionUtils.getVersionNumberFromEntity(instance, persistentEntity, converter);
348361

349362
Assert.notNull(previousVersion, "The root aggregate cannot be updated because the version property is null.");
350363

351364
long newVersion = previousVersion.longValue() + 1;
352365

353-
preparedInstance = RelationalEntityVersionUtils.setVersionNumberOnEntity(instance, newVersion,
354-
persistentEntity, converter);
366+
preparedInstance = RelationalEntityVersionUtils.setVersionNumberOnEntity(instance, newVersion, persistentEntity,
367+
converter);
355368
}
356-
MutableAggregateChange<T> aggregateChange = MutableAggregateChange.forSave(preparedInstance, previousVersion);
357-
jdbcEntityUpdateWriter.write(preparedInstance, aggregateChange);
358-
return aggregateChange;
369+
return new EntityAndPreviousVersion<>(preparedInstance, previousVersion);
359370
}
360371

361372
@SuppressWarnings("unchecked")
@@ -445,4 +456,16 @@ private <T> T triggerBeforeDelete(@Nullable T aggregateRoot, Object id, MutableA
445456

446457
return null;
447458
}
459+
460+
private static class EntityAndPreviousVersion<T> {
461+
462+
private final T entity;
463+
private final Number version;
464+
465+
EntityAndPreviousVersion(T entity, @Nullable Number version) {
466+
467+
this.entity = entity;
468+
this.version = version;
469+
}
470+
}
448471
}

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -821,9 +821,10 @@ void saveAndUpdateAggregateWithImmutableVersion() {
821821

822822
@Test // GH-1137
823823
void testUpdateEntityWithVersionDoesNotTriggerAnewConstructorInvocation() {
824+
824825
AggregateWithImmutableVersion aggregateWithImmutableVersion = new AggregateWithImmutableVersion(null, null);
825826

826-
final AggregateWithImmutableVersion savedRoot = template.save(aggregateWithImmutableVersion);
827+
AggregateWithImmutableVersion savedRoot = template.save(aggregateWithImmutableVersion);
827828

828829
assertThat(savedRoot).isNotNull();
829830
assertThat(savedRoot.version).isEqualTo(0L);
@@ -836,12 +837,12 @@ void testUpdateEntityWithVersionDoesNotTriggerAnewConstructorInvocation() {
836837

837838
AggregateWithImmutableVersion.clearConstructorInvocationData();
838839

839-
final AggregateWithImmutableVersion updatedRoot = template.save(savedRoot);
840+
AggregateWithImmutableVersion updatedRoot = template.save(savedRoot);
840841

841842
assertThat(updatedRoot).isNotNull();
842843
assertThat(updatedRoot.version).isEqualTo(1L);
843844

844-
// Expect only one assignnment of the version to AggregateWithImmutableVersion
845+
// Expect only one assignment of the version to AggregateWithImmutableVersion
845846
assertThat(AggregateWithImmutableVersion.constructorInvocations).containsOnly(new ConstructorInvocation(savedRoot.id, updatedRoot.version));
846847
}
847848

@@ -1261,6 +1262,7 @@ public static void clearConstructorInvocationData() {
12611262
}
12621263

12631264
public AggregateWithImmutableVersion(Long id, Long version) {
1265+
12641266
constructorInvocations.add(new ConstructorInvocation(id, version));
12651267
this.id = id;
12661268
this.version = version;
@@ -1270,8 +1272,9 @@ public AggregateWithImmutableVersion(Long id, Long version) {
12701272
@Value
12711273
@EqualsAndHashCode
12721274
private static class ConstructorInvocation {
1273-
private Long id;
1274-
private Long version;
1275+
1276+
Long id;
1277+
Long version;
12751278
}
12761279

12771280
@Data

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222

2323
import lombok.AllArgsConstructor;
2424
import lombok.Data;
25-
2625
import lombok.RequiredArgsConstructor;
26+
2727
import org.junit.jupiter.api.BeforeEach;
2828
import org.junit.jupiter.api.Test;
2929
import org.junit.jupiter.api.extension.ExtendWith;
@@ -115,7 +115,7 @@ public void callbackOnSave() {
115115
assertThat(last).isEqualTo(third);
116116
}
117117

118-
@Test
118+
@Test // GH-1137
119119
void savePreparesInstanceWithInitialVersion_onInsert() {
120120

121121
EntityWithVersion entity = new EntityWithVersion(1L);
@@ -125,11 +125,12 @@ void savePreparesInstanceWithInitialVersion_onInsert() {
125125

126126
ArgumentCaptor<Object> aggregateRootCaptor = ArgumentCaptor.forClass(Object.class);
127127
verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any());
128+
128129
EntityWithVersion afterConvert = (EntityWithVersion) aggregateRootCaptor.getValue();
129130
assertThat(afterConvert.getVersion()).isEqualTo(0L);
130131
}
131132

132-
@Test
133+
@Test // GH-1137
133134
void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsImmutable() {
134135

135136
EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, null);
@@ -139,11 +140,12 @@ void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsImmuta
139140

140141
ArgumentCaptor<Object> aggregateRootCaptor = ArgumentCaptor.forClass(Object.class);
141142
verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any());
143+
142144
EntityWithImmutableVersion afterConvert = (EntityWithImmutableVersion) aggregateRootCaptor.getValue();
143145
assertThat(afterConvert.getVersion()).isEqualTo(0L);
144146
}
145147

146-
@Test // DATAJDBC-507
148+
@Test // GH-1137
147149
void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsPrimitiveType() {
148150

149151
EntityWithPrimitiveVersion entity = new EntityWithPrimitiveVersion(1L);
@@ -153,11 +155,12 @@ void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsPrimit
153155

154156
ArgumentCaptor<Object> aggregateRootCaptor = ArgumentCaptor.forClass(Object.class);
155157
verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any());
158+
156159
EntityWithPrimitiveVersion afterConvert = (EntityWithPrimitiveVersion) aggregateRootCaptor.getValue();
157160
assertThat(afterConvert.getVersion()).isEqualTo(1L);
158161
}
159162

160-
@Test // DATAJDBC-507
163+
@Test // GH-1137
161164
void savePreparesInstanceWithInitialVersion_onInsert__whenVersionPropertyIsImmutableAndPrimitiveType() {
162165

163166
EntityWithImmutablePrimitiveVersion entity = new EntityWithImmutablePrimitiveVersion(1L, 0L);
@@ -167,11 +170,13 @@ void savePreparesInstanceWithInitialVersion_onInsert__whenVersionPropertyIsImmut
167170

168171
ArgumentCaptor<Object> aggregateRootCaptor = ArgumentCaptor.forClass(Object.class);
169172
verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any());
170-
EntityWithImmutablePrimitiveVersion afterConvert = (EntityWithImmutablePrimitiveVersion) aggregateRootCaptor.getValue();
173+
174+
EntityWithImmutablePrimitiveVersion afterConvert = (EntityWithImmutablePrimitiveVersion) aggregateRootCaptor
175+
.getValue();
171176
assertThat(afterConvert.getVersion()).isEqualTo(1L);
172177
}
173178

174-
@Test
179+
@Test // GH-1137
175180
void savePreparesChangeWithPreviousVersion_onUpdate() {
176181

177182
when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true);
@@ -183,11 +188,12 @@ void savePreparesChangeWithPreviousVersion_onUpdate() {
183188

184189
ArgumentCaptor<Object> aggregateChangeCaptor = ArgumentCaptor.forClass(Object.class);
185190
verify(callbacks).callback(eq(BeforeSaveCallback.class), any(), aggregateChangeCaptor.capture());
191+
186192
MutableAggregateChange<?> aggregateChange = (MutableAggregateChange<?>) aggregateChangeCaptor.getValue();
187193
assertThat(aggregateChange.getPreviousVersion()).isEqualTo(1L);
188194
}
189195

190-
@Test
196+
@Test // GH-1137
191197
void savePreparesInstanceWithNextVersion_onUpdate() {
192198

193199
when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true);
@@ -199,11 +205,12 @@ void savePreparesInstanceWithNextVersion_onUpdate() {
199205

200206
ArgumentCaptor<Object> aggregateRootCaptor = ArgumentCaptor.forClass(Object.class);
201207
verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any());
208+
202209
EntityWithVersion afterConvert = (EntityWithVersion) aggregateRootCaptor.getValue();
203210
assertThat(afterConvert.getVersion()).isEqualTo(2L);
204211
}
205212

206-
@Test
213+
@Test // GH-1137
207214
void savePreparesInstanceWithNextVersion_onUpdate_whenVersionPropertyIsImmutable() {
208215

209216
when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true);
@@ -218,7 +225,7 @@ void savePreparesInstanceWithNextVersion_onUpdate_whenVersionPropertyIsImmutable
218225
assertThat(afterConvert.getVersion()).isEqualTo(2L);
219226
}
220227

221-
@Test
228+
@Test // GH-1137
222229
void deletePreparesChangeWithPreviousVersion_onDeleteByInstance() {
223230

224231
EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, 1L);
@@ -228,11 +235,11 @@ void deletePreparesChangeWithPreviousVersion_onDeleteByInstance() {
228235

229236
ArgumentCaptor<Object> aggregateChangeCaptor = ArgumentCaptor.forClass(Object.class);
230237
verify(callbacks).callback(eq(BeforeDeleteCallback.class), any(), aggregateChangeCaptor.capture());
238+
231239
MutableAggregateChange<?> aggregateChange = (MutableAggregateChange<?>) aggregateChangeCaptor.getValue();
232240
assertThat(aggregateChange.getPreviousVersion()).isEqualTo(1L);
233241
}
234242

235-
236243
@Test // DATAJDBC-393
237244
public void callbackOnDelete() {
238245

0 commit comments

Comments
 (0)