Skip to content

Commit 49e343f

Browse files
committed
Fix Callback order.
This undos the changes to `JdbcAggregateTemplate` done by 7cf81ae. Since we are in the RC phase I opt against trying to redo the refactoring Closes #1924
1 parent a351a66 commit 49e343f

File tree

2 files changed

+46
-26
lines changed

2 files changed

+46
-26
lines changed

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java

+34-26
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,13 @@
1616
package org.springframework.data.jdbc.core;
1717

1818
import java.util.ArrayList;
19-
import java.util.Collection;
2019
import java.util.Collections;
2120
import java.util.HashMap;
2221
import java.util.Iterator;
2322
import java.util.LinkedHashMap;
2423
import java.util.List;
2524
import java.util.Map;
2625
import java.util.Optional;
27-
import java.util.function.Consumer;
2826
import java.util.function.Function;
2927
import java.util.stream.Collectors;
3028
import java.util.stream.StreamSupport;
@@ -58,7 +56,6 @@
5856
import org.springframework.lang.Nullable;
5957
import org.springframework.util.Assert;
6058
import org.springframework.util.ClassUtils;
61-
import org.springframework.util.ObjectUtils;
6259

6360
/**
6461
* {@link JdbcAggregateOperations} implementation, storing aggregates in and obtaining them from a JDBC data store.
@@ -176,8 +173,19 @@ public <T> T save(T instance) {
176173

177174
@Override
178175
public <T> List<T> saveAll(Iterable<T> instances) {
179-
return doWithBatch(instances, entity -> changeCreatorSelectorForSave(entity).apply(entity), this::verifyIdProperty,
180-
this::performSaveAll);
176+
177+
Assert.notNull(instances, "Aggregate instances must not be null");
178+
179+
if (!instances.iterator().hasNext()) {
180+
return Collections.emptyList();
181+
}
182+
183+
List<EntityAndChangeCreator<T>> entityAndChangeCreators = new ArrayList<>();
184+
for (T instance : instances) {
185+
verifyIdProperty(instance);
186+
entityAndChangeCreators.add(new EntityAndChangeCreator<>(instance, changeCreatorSelectorForSave(instance)));
187+
}
188+
return performSaveAll(entityAndChangeCreators);
181189
}
182190

183191
/**
@@ -198,7 +206,21 @@ public <T> T insert(T instance) {
198206

199207
@Override
200208
public <T> List<T> insertAll(Iterable<T> instances) {
201-
return doWithBatch(instances, entity -> createInsertChange(prepareVersionForInsert(entity)), this::performSaveAll);
209+
210+
Assert.notNull(instances, "Aggregate instances must not be null");
211+
212+
if (!instances.iterator().hasNext()) {
213+
return Collections.emptyList();
214+
}
215+
216+
List<EntityAndChangeCreator<T>> entityAndChangeCreators = new ArrayList<>();
217+
for (T instance : instances) {
218+
219+
Function<T, RootAggregateChange<T>> changeCreator = entity -> createInsertChange(prepareVersionForInsert(entity));
220+
EntityAndChangeCreator<T> entityChange = new EntityAndChangeCreator<>(instance, changeCreator);
221+
entityAndChangeCreators.add(entityChange);
222+
}
223+
return performSaveAll(entityAndChangeCreators);
202224
}
203225

204226
/**
@@ -219,35 +241,21 @@ public <T> T update(T instance) {
219241

220242
@Override
221243
public <T> List<T> updateAll(Iterable<T> instances) {
222-
return doWithBatch(instances, entity -> createUpdateChange(prepareVersionForUpdate(entity)), this::performSaveAll);
223-
}
224-
225-
private <T> List<T> doWithBatch(Iterable<T> iterable, Function<T, RootAggregateChange<T>> changeCreator,
226-
Function<List<EntityAndChangeCreator<T>>, List<T>> performFunction) {
227-
return doWithBatch(iterable, changeCreator, entity -> {}, performFunction);
228-
}
229-
230-
private <T> List<T> doWithBatch(Iterable<T> iterable, Function<T, RootAggregateChange<T>> changeCreator,
231-
Consumer<T> beforeEntityChange, Function<List<EntityAndChangeCreator<T>>, List<T>> performFunction) {
232244

233-
Assert.notNull(iterable, "Aggregate instances must not be null");
245+
Assert.notNull(instances, "Aggregate instances must not be null");
234246

235-
if (ObjectUtils.isEmpty(iterable)) {
247+
if (!instances.iterator().hasNext()) {
236248
return Collections.emptyList();
237249
}
238250

239-
List<EntityAndChangeCreator<T>> entityAndChangeCreators = new ArrayList<>(
240-
iterable instanceof Collection<?> c ? c.size() : 16);
241-
242-
for (T instance : iterable) {
243-
244-
beforeEntityChange.accept(instance);
251+
List<EntityAndChangeCreator<T>> entityAndChangeCreators = new ArrayList<>();
252+
for (T instance : instances) {
245253

254+
Function<T, RootAggregateChange<T>> changeCreator = entity -> createUpdateChange(prepareVersionForUpdate(entity));
246255
EntityAndChangeCreator<T> entityChange = new EntityAndChangeCreator<>(instance, changeCreator);
247256
entityAndChangeCreators.add(entityChange);
248257
}
249-
250-
return performFunction.apply(entityAndChangeCreators);
258+
return performSaveAll(entityAndChangeCreators);
251259
}
252260

253261
@Override

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIdGenerationIntegrationTests.java

+12
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,18 @@ void manuallyGeneratedId() {
9595
assertThat(immutableWithManualIdEntityRepository.findAll()).hasSize(1);
9696
}
9797

98+
@Test // DATAJDBC-393
99+
void manuallyGeneratedIdForSaveAll() {
100+
101+
ImmutableWithManualIdEntity one = new ImmutableWithManualIdEntity(null, "one");
102+
ImmutableWithManualIdEntity two = new ImmutableWithManualIdEntity(null, "two");
103+
List<ImmutableWithManualIdEntity> saved = immutableWithManualIdEntityRepository.saveAll(List.of(one, two));
104+
105+
assertThat(saved).allSatisfy(e -> assertThat(e.id).isNotNull());
106+
107+
assertThat(immutableWithManualIdEntityRepository.findAll()).hasSize(2);
108+
}
109+
98110
private interface PrimitiveIdEntityRepository extends ListCrudRepository<PrimitiveIdEntity, Long> {}
99111

100112
private interface ReadOnlyIdEntityRepository extends ListCrudRepository<ReadOnlyIdEntity, Long> {}

0 commit comments

Comments
 (0)