Skip to content

Commit 2289195

Browse files
schaudermp911de
authored andcommitted
DATAJDBC-400 - Remove Identifier from callbacks.
Retrieval of the Identifier has been shown to cost significant performance. Original pull request: #164.
1 parent 4ab69a1 commit 2289195

File tree

16 files changed

+90
-57
lines changed

16 files changed

+90
-57
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -380,14 +380,14 @@ private <T> T triggerAfterLoad(Object id, T entity) {
380380

381381
publisher.publishEvent(new AfterLoadEvent(identifier, entity));
382382

383-
return entityCallbacks.callback(AfterLoadCallback.class, entity, identifier);
383+
return entityCallbacks.callback(AfterLoadCallback.class, entity);
384384
}
385385

386386
private <T> T triggerBeforeConvert(T aggregateRoot, @Nullable Object id) {
387387

388388
Identifier identifier = Identifier.ofNullable(id);
389389

390-
return entityCallbacks.callback(BeforeConvertCallback.class, aggregateRoot, identifier);
390+
return entityCallbacks.callback(BeforeConvertCallback.class, aggregateRoot);
391391
}
392392

393393
private <T> T triggerBeforeSave(T aggregateRoot, @Nullable Object id, AggregateChange<T> change) {
@@ -400,7 +400,7 @@ private <T> T triggerBeforeSave(T aggregateRoot, @Nullable Object id, AggregateC
400400
change //
401401
));
402402

403-
return entityCallbacks.callback(BeforeSaveCallback.class, aggregateRoot, identifier, change);
403+
return entityCallbacks.callback(BeforeSaveCallback.class, aggregateRoot, change);
404404
}
405405

406406
private <T> T triggerAfterSave(T aggregateRoot, Object id, AggregateChange<T> change) {
@@ -413,7 +413,7 @@ private <T> T triggerAfterSave(T aggregateRoot, Object id, AggregateChange<T> ch
413413
change //
414414
));
415415

416-
return entityCallbacks.callback(AfterSaveCallback.class, aggregateRoot, identifier);
416+
return entityCallbacks.callback(AfterSaveCallback.class, aggregateRoot);
417417
}
418418

419419
private <T> void triggerAfterDelete(@Nullable T aggregateRoot, Object id, AggregateChange<?> change) {
@@ -423,7 +423,7 @@ private <T> void triggerAfterDelete(@Nullable T aggregateRoot, Object id, Aggreg
423423
publisher.publishEvent(new AfterDeleteEvent(identifier, Optional.ofNullable(aggregateRoot), change));
424424

425425
if (aggregateRoot != null) {
426-
entityCallbacks.callback(AfterDeleteCallback.class, aggregateRoot, identifier);
426+
entityCallbacks.callback(AfterDeleteCallback.class, aggregateRoot);
427427
}
428428
}
429429

@@ -435,7 +435,7 @@ private <T> T triggerBeforeDelete(@Nullable T aggregateRoot, Object id, Aggregat
435435
publisher.publishEvent(new BeforeDeleteEvent(identifier, Optional.ofNullable(aggregateRoot), change));
436436

437437
if (aggregateRoot != null) {
438-
return entityCallbacks.callback(BeforeDeleteCallback.class, aggregateRoot, identifier, change);
438+
return entityCallbacks.callback(BeforeDeleteCallback.class, aggregateRoot, change);
439439
}
440440

441441
return null;

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.springframework.data.jdbc.core.convert.EntityRowMapper;
2424
import org.springframework.data.jdbc.core.convert.JdbcConverter;
2525
import org.springframework.data.jdbc.repository.QueryMappingConfiguration;
26+
import org.springframework.data.mapping.callback.EntityCallbacks;
2627
import org.springframework.data.projection.ProjectionFactory;
2728
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
2829
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
@@ -47,6 +48,7 @@
4748
class JdbcQueryLookupStrategy implements QueryLookupStrategy {
4849

4950
private final ApplicationEventPublisher publisher;
51+
private final EntityCallbacks callbacks;
5052
private final RelationalMappingContext context;
5153
private final JdbcConverter converter;
5254
private final QueryMappingConfiguration queryMappingConfiguration;
@@ -64,7 +66,7 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository
6466

6567
RowMapper<?> mapper = queryMethod.isModifyingQuery() ? null : createMapper(queryMethod);
6668

67-
return new JdbcRepositoryQuery(publisher, context, queryMethod, operations, mapper);
69+
return new JdbcRepositoryQuery(publisher, callbacks, context, queryMethod, operations, mapper);
6870
}
6971

7072
private RowMapper<?> createMapper(JdbcQueryMethod queryMethod) {

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ protected Optional<QueryLookupStrategy> getQueryLookupStrategy(@Nullable QueryLo
149149
if (key == null || key == QueryLookupStrategy.Key.CREATE_IF_NOT_FOUND
150150
|| key == QueryLookupStrategy.Key.USE_DECLARED_QUERY) {
151151

152-
JdbcQueryLookupStrategy strategy = new JdbcQueryLookupStrategy(publisher, context, converter,
152+
JdbcQueryLookupStrategy strategy = new JdbcQueryLookupStrategy(publisher, entityCallbacks, context, converter,
153153
queryMappingConfiguration, operations);
154154
return Optional.of(strategy);
155155
}

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryQuery.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
import org.springframework.beans.BeanUtils;
2222
import org.springframework.context.ApplicationEventPublisher;
2323
import org.springframework.dao.EmptyResultDataAccessException;
24+
import org.springframework.data.mapping.callback.EntityCallbacks;
2425
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
2526
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
27+
import org.springframework.data.relational.core.mapping.event.AfterLoadCallback;
2628
import org.springframework.data.relational.core.mapping.event.AfterLoadEvent;
2729
import org.springframework.data.relational.core.mapping.event.Identifier;
2830
import org.springframework.data.repository.query.RepositoryQuery;
@@ -49,6 +51,7 @@ class JdbcRepositoryQuery implements RepositoryQuery {
4951
private static final String PARAMETER_NEEDS_TO_BE_NAMED = "For queries with named parameters you need to provide names for method parameters. Use @Param for query method parameters, or when on Java 8+ use the javac flag -parameters.";
5052

5153
private final ApplicationEventPublisher publisher;
54+
private final EntityCallbacks callbacks;
5255
private final RelationalMappingContext context;
5356
private final JdbcQueryMethod queryMethod;
5457
private final NamedParameterJdbcOperations operations;
@@ -64,8 +67,9 @@ class JdbcRepositoryQuery implements RepositoryQuery {
6467
* @param operations must not be {@literal null}.
6568
* @param defaultRowMapper can be {@literal null} (only in case of a modifying query).
6669
*/
67-
JdbcRepositoryQuery(ApplicationEventPublisher publisher, RelationalMappingContext context,
68-
JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations, RowMapper<?> defaultRowMapper) {
70+
JdbcRepositoryQuery(ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks,
71+
RelationalMappingContext context, JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
72+
RowMapper<?> defaultRowMapper) {
6973

7074
Assert.notNull(publisher, "Publisher must not be null!");
7175
Assert.notNull(context, "Context must not be null!");
@@ -77,6 +81,7 @@ class JdbcRepositoryQuery implements RepositoryQuery {
7781
}
7882

7983
this.publisher = publisher;
84+
this.callbacks = callbacks == null ? EntityCallbacks.create() : callbacks;
8085
this.context = context;
8186
this.queryMethod = queryMethod;
8287
this.operations = operations;
@@ -214,7 +219,8 @@ private MapSqlParameterSource bindParameters(Object[] objects) {
214219
@Nullable
215220
private ResultSetExtractor determineResultSetExtractor(@Nullable RowMapper<?> rowMapper) {
216221

217-
Class<? extends ResultSetExtractor> resultSetExtractorClass = (Class<? extends ResultSetExtractor>) queryMethod.getResultSetExtractorClass();
222+
Class<? extends ResultSetExtractor> resultSetExtractorClass = (Class<? extends ResultSetExtractor>) queryMethod
223+
.getResultSetExtractorClass();
218224

219225
if (isUnconfigured(resultSetExtractorClass, ResultSetExtractor.class)) {
220226
return null;
@@ -262,6 +268,8 @@ private <T> void publishAfterLoad(@Nullable T entity) {
262268
if (identifier != null) {
263269
publisher.publishEvent(new AfterLoadEvent(Identifier.of(identifier), entity));
264270
}
271+
272+
callbacks.callback(AfterLoadCallback.class, entity);
265273
}
266274

267275
}

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@
2727
import org.junit.Test;
2828
import org.junit.runner.RunWith;
2929
import org.mockito.Mock;
30+
import org.mockito.invocation.InvocationOnMock;
3031
import org.mockito.junit.MockitoJUnitRunner;
32+
import org.mockito.stubbing.Answer;
3133
import org.springframework.context.ApplicationEventPublisher;
3234
import org.springframework.data.annotation.Id;
3335
import org.springframework.data.jdbc.core.convert.BasicJdbcConverter;
@@ -45,7 +47,6 @@
4547
import org.springframework.data.relational.core.mapping.event.BeforeConvertCallback;
4648
import org.springframework.data.relational.core.mapping.event.BeforeDeleteCallback;
4749
import org.springframework.data.relational.core.mapping.event.BeforeSaveCallback;
48-
import org.springframework.data.relational.core.mapping.event.Identifier;
4950

5051
/**
5152
* Unit tests for {@link JdbcAggregateTemplate}.
@@ -71,6 +72,7 @@ public void setUp() {
7172

7273
template = new JdbcAggregateTemplate(eventPublisher, mappingContext, converter, dataAccessStrategy);
7374
((JdbcAggregateTemplate) template).setEntityCallbacks(callbacks);
75+
7476
}
7577

7678
@Test // DATAJDBC-378
@@ -101,10 +103,9 @@ public void callbackOnSave() {
101103

102104
SampleEntity last = template.save(first);
103105

104-
verify(callbacks).callback(BeforeConvertCallback.class, first, Identifier.ofNullable(null));
105-
verify(callbacks).callback(eq(BeforeSaveCallback.class), eq(second), eq(Identifier.ofNullable(23L)),
106-
any(AggregateChange.class));
107-
verify(callbacks).callback(AfterSaveCallback.class, third, Identifier.of(23L));
106+
verify(callbacks).callback(BeforeConvertCallback.class, first);
107+
verify(callbacks).callback(eq(BeforeSaveCallback.class), eq(second), any(AggregateChange.class));
108+
verify(callbacks).callback(AfterSaveCallback.class, third);
108109
assertThat(last).isEqualTo(third);
109110
}
110111

@@ -114,13 +115,12 @@ public void callbackOnDelete() {
114115
SampleEntity first = new SampleEntity(23L, "Alfred");
115116
SampleEntity second = new SampleEntity(23L, "Alfred E.");
116117

117-
when(callbacks.callback(any(Class.class), any(), any(), any())).thenReturn(second);
118+
when(callbacks.callback(any(Class.class), any(), any())).thenReturn(second);
118119

119120
template.delete(first, SampleEntity.class);
120121

121-
verify(callbacks).callback(eq(BeforeDeleteCallback.class), eq(first), eq(Identifier.of(23L)),
122-
any(AggregateChange.class));
123-
verify(callbacks).callback(AfterDeleteCallback.class, second, Identifier.of(23L));
122+
verify(callbacks).callback(eq(BeforeDeleteCallback.class), eq(first), any(AggregateChange.class));
123+
verify(callbacks).callback(AfterDeleteCallback.class, second);
124124
}
125125

126126
@Test // DATAJDBC-393
@@ -139,8 +139,8 @@ public void callbackOnLoad() {
139139

140140
Iterable<SampleEntity> all = template.findAll(SampleEntity.class);
141141

142-
verify(callbacks).callback(AfterLoadCallback.class, alfred1, Identifier.of(23L));
143-
verify(callbacks).callback(AfterLoadCallback.class, neumann1, Identifier.of(42L));
142+
verify(callbacks).callback(AfterLoadCallback.class, alfred1);
143+
verify(callbacks).callback(AfterLoadCallback.class, neumann1);
144144

145145
assertThat(all).containsExactly(alfred2, neumann2);
146146
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public String getTableName(Class<?> type) {
173173

174174
@Bean
175175
BeforeConvertCallback<ImmutableWithManualIdEntity> idGenerator() {
176-
return (e, __) -> e.withId(lastId.incrementAndGet());
176+
return e -> e.withId(lastId.incrementAndGet());
177177
}
178178
}
179179
}

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/EnableJdbcAuditingHsqlIntegrationTests.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import org.springframework.data.relational.core.mapping.NamingStrategy;
4545
import org.springframework.data.relational.core.mapping.event.BeforeConvertCallback;
4646
import org.springframework.data.relational.core.mapping.event.BeforeSaveEvent;
47-
import org.springframework.data.relational.core.mapping.event.Identifier;
4847
import org.springframework.data.repository.CrudRepository;
4948
import org.springframework.stereotype.Component;
5049
import org.springframework.test.context.ActiveProfiles;
@@ -191,12 +190,12 @@ public void auditingListenerTriggersBeforeDefaultListener() {
191190
OrderAssertingEventListener.class, //
192191
OrderAssertingCallback.class //
193192
) //
194-
.accept(repository -> {
193+
.accept(repository -> {
195194

196-
AuditingAnnotatedDummyEntity entity = repository.save(new AuditingAnnotatedDummyEntity());
195+
AuditingAnnotatedDummyEntity entity = repository.save(new AuditingAnnotatedDummyEntity());
197196

198-
assertThat(entity.id).isNotNull();
199-
});
197+
assertThat(entity.id).isNotNull();
198+
});
200199
}
201200

202201
/**
@@ -344,7 +343,7 @@ public void onApplicationEvent(BeforeSaveEvent event) {
344343
static class OrderAssertingCallback implements BeforeConvertCallback {
345344

346345
@Override
347-
public Object onBeforeConvert(Object entity, Identifier id) {
346+
public Object onBeforeConvert(Object entity) {
348347

349348
assertThat(entity).isInstanceOf(AuditingAnnotatedDummyEntity.class);
350349
assertThat(((AuditingAnnotatedDummyEntity) entity).createdDate).isNotNull();

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategyUnitTests.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.springframework.data.jdbc.repository.QueryMappingConfiguration;
3131
import org.springframework.data.jdbc.repository.config.DefaultQueryMappingConfiguration;
3232
import org.springframework.data.jdbc.repository.query.Query;
33+
import org.springframework.data.mapping.callback.EntityCallbacks;
3334
import org.springframework.data.projection.ProjectionFactory;
3435
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
3536
import org.springframework.data.repository.core.NamedQueries;
@@ -52,6 +53,7 @@
5253
public class JdbcQueryLookupStrategyUnitTests {
5354

5455
ApplicationEventPublisher publisher = mock(ApplicationEventPublisher.class);
56+
EntityCallbacks callbacks = mock(EntityCallbacks.class);
5557
RelationalMappingContext mappingContext = mock(RelationalMappingContext.class, RETURNS_DEEP_STUBS);
5658
JdbcConverter converter = mock(JdbcConverter.class);
5759
DataAccessStrategy accessStrategy = mock(DataAccessStrategy.class);
@@ -85,7 +87,7 @@ public void typeBasedRowMapperGetsUsedForQuery() {
8587

8688
private RepositoryQuery getRepositoryQuery(String name, QueryMappingConfiguration mappingConfiguration) {
8789

88-
JdbcQueryLookupStrategy queryLookupStrategy = new JdbcQueryLookupStrategy(publisher, mappingContext, converter,
90+
JdbcQueryLookupStrategy queryLookupStrategy = new JdbcQueryLookupStrategy(publisher, callbacks, mappingContext, converter,
8991
mappingConfiguration, operations);
9092

9193
Method method = ReflectionUtils.findMethod(MyRepository.class, name);

0 commit comments

Comments
 (0)