DATAJDBC-102 - Determine the EntityInstantiator to be used dynamically.

Replaced the direct use of EntityInstantiator with EntityInstantiators. Moved it into the MappingContext because instantiation is part of the mapping process. Original pull request: spring-projects#68.

Author: schauder

src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java

@@ -27,6 +27,7 @@
 import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.dao.NonTransientDataAccessException;
+import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
 import org.springframework.data.jdbc.core.mapping.JdbcPersistentEntity;
 import org.springframework.data.jdbc.core.mapping.JdbcPersistentProperty;
@@ -58,18 +59,20 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy {
 private final @NonNull SqlGeneratorSource sqlGeneratorSource;
 private final @NonNull JdbcMappingContext context;
 private final @NonNull NamedParameterJdbcOperations operations;
+private final @NonNull EntityInstantiators instantiators;
 private final @NonNull DataAccessStrategy accessStrategy;
 
 /**
  * Creates a {@link DefaultDataAccessStrategy} which references it self for resolution of recursive data accesses.
  * Only suitable if this is the only access strategy in use.
  */
 public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, JdbcMappingContext context,
-NamedParameterJdbcOperations operations) {
+NamedParameterJdbcOperations operations, EntityInstantiators instantiators) {
 
 this.sqlGeneratorSource = sqlGeneratorSource;
 this.operations = operations;
 this.context = context;
+this.instantiators = instantiators;
 this.accessStrategy = this;
 }
 
@@ -321,7 +324,7 @@ private <S> Optional<Object> getIdFromHolder(KeyHolder holder, JdbcPersistentEnt
 }
 
 public <T> EntityRowMapper<T> getEntityRowMapper(Class<T> domainType) {
-return new EntityRowMapper<>(getRequiredPersistentEntity(domainType), context, accessStrategy);
+return new EntityRowMapper<>(getRequiredPersistentEntity(domainType), context, instantiators, accessStrategy);
 }
 
 private RowMapper getMapEntityRowMapper(JdbcPersistentProperty property) {

src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java

@@ -24,8 +24,7 @@
 
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.converter.Converter;
-import org.springframework.data.convert.ClassGeneratingEntityInstantiator;
-import org.springframework.data.convert.EntityInstantiator;
+import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
 import org.springframework.data.jdbc.core.mapping.JdbcPersistentEntity;
 import org.springframework.data.jdbc.core.mapping.JdbcPersistentProperty;
@@ -49,18 +48,20 @@ public class EntityRowMapper<T> implements RowMapper<T> {
 private static final Converter<Iterable<?>, Map<?, ?>> ITERABLE_OF_ENTRY_TO_MAP_CONVERTER = new IterableOfEntryToMapConverter();
 
 private final JdbcPersistentEntity<T> entity;
-private final EntityInstantiator instantiator = new ClassGeneratingEntityInstantiator();
+
 private final ConversionService conversions;
 private final JdbcMappingContext context;
 private final DataAccessStrategy accessStrategy;
 private final JdbcPersistentProperty idProperty;
+private final EntityInstantiators instantiators;
 
-public EntityRowMapper(JdbcPersistentEntity<T> entity, JdbcMappingContext context,
+public EntityRowMapper(JdbcPersistentEntity<T> entity, JdbcMappingContext context, EntityInstantiators instantiators,
 DataAccessStrategy accessStrategy) {
 
 this.entity = entity;
 this.conversions = context.getConversions();
 this.context = context;
+this.instantiators = instantiators;
 this.accessStrategy = accessStrategy;
 
 idProperty = entity.getIdProperty();
@@ -97,7 +98,9 @@ public T mapRow(ResultSet resultSet, int rowNumber) throws SQLException {
 }
 
 private T createInstance(ResultSet rs) {
-return instantiator.createInstance(entity, new ResultSetParameterValueProvider(rs, entity, conversions, ""));
+
+return instantiators.getInstantiatorFor(entity) //
+.createInstance(entity, new ResultSetParameterValueProvider(rs, entity, conversions, ""));
 }
 
 /**
@@ -135,8 +138,8 @@ private <S> S readEntityFrom(ResultSet rs, PersistentProperty<?> property) {
 return null;
 }
 
-S instance = instantiator.createInstance(entity,
-new ResultSetParameterValueProvider(rs, entity, conversions, prefix));
+S instance = instantiators.getInstantiatorFor(entity) //
+.createInstance(entity, new ResultSetParameterValueProvider(rs, entity, conversions, prefix));
 
 PersistentPropertyAccessor accessor = entity.getPropertyAccessor(instance);
 ConvertingPropertyAccessor propertyAccessor = new ConvertingPropertyAccessor(accessor, conversions);

src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java

@@ -22,6 +22,7 @@
 
 import org.apache.ibatis.session.SqlSession;
 import org.mybatis.spring.SqlSessionTemplate;
+import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.jdbc.core.CascadingDataAccessStrategy;
 import org.springframework.data.jdbc.core.DataAccessStrategy;
 import org.springframework.data.jdbc.core.DefaultDataAccessStrategy;
@@ -58,15 +59,17 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy {
  */
 public static DataAccessStrategy createCombinedAccessStrategy(JdbcMappingContext context,
 NamedParameterJdbcOperations operations, SqlSession sqlSession) {
-return createCombinedAccessStrategy(context, operations, sqlSession, NamespaceStrategy.DEFAULT_INSTANCE);
+return createCombinedAccessStrategy(context, new EntityInstantiators(), operations, sqlSession,
+NamespaceStrategy.DEFAULT_INSTANCE);
 }
 
 /**
  * Create a {@link DataAccessStrategy} that first checks for queries defined by MyBatis and if it doesn't find one
  * uses a {@link DefaultDataAccessStrategy}
  */
 public static DataAccessStrategy createCombinedAccessStrategy(JdbcMappingContext context,
-NamedParameterJdbcOperations operations, SqlSession sqlSession, NamespaceStrategy namespaceStrategy) {
+EntityInstantiators instantiators, NamedParameterJdbcOperations operations, SqlSession sqlSession,
+NamespaceStrategy namespaceStrategy) {
 
 // the DefaultDataAccessStrategy needs a reference to the returned DataAccessStrategy. This creates a dependency
 // cycle. In order to create it, we need something that allows to defer closing the cycle until all the elements are
@@ -79,8 +82,13 @@ public static DataAccessStrategy createCombinedAccessStrategy(JdbcMappingContext
 asList(myBatisDataAccessStrategy, delegatingDataAccessStrategy));
 
 SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(context);
-DefaultDataAccessStrategy defaultDataAccessStrategy = new DefaultDataAccessStrategy(sqlGeneratorSource, context,
-operations, cascadingDataAccessStrategy);
+DefaultDataAccessStrategy defaultDataAccessStrategy = new DefaultDataAccessStrategy( //
+sqlGeneratorSource, //
+context, //
+operations, //
+instantiators, //
+cascadingDataAccessStrategy //
+);
 
 delegatingDataAccessStrategy.setDelegate(defaultDataAccessStrategy);
 
@@ -93,8 +101,9 @@ public static DataAccessStrategy createCombinedAccessStrategy(JdbcMappingContext
  * Use a {@link SqlSessionTemplate} for {@link SqlSession} or a similar implementation tying the session to the proper
  * transaction. Note that the resulting {@link DataAccessStrategy} only handles MyBatis. It does not include the
  * functionality of the {@link org.springframework.data.jdbc.core.DefaultDataAccessStrategy} which one normally still
- * wants. Use {@link #createCombinedAccessStrategy(JdbcMappingContext, SqlSession)} to create such a
- * {@link DataAccessStrategy}.
+ * wants. Use
+ * {@link #createCombinedAccessStrategy(JdbcMappingContext, EntityInstantiators, NamedParameterJdbcOperations, SqlSession, NamespaceStrategy)}
+ * to create such a {@link DataAccessStrategy}.
  *
  * @param sqlSession Must be non {@literal null}.
  */

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

@@ -18,6 +18,7 @@
 import java.lang.reflect.Method;
 
 import org.springframework.core.convert.ConversionService;
+import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.jdbc.core.DataAccessStrategy;
 import org.springframework.data.jdbc.core.EntityRowMapper;
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
@@ -43,27 +44,30 @@
 class JdbcQueryLookupStrategy implements QueryLookupStrategy {
 
 private final JdbcMappingContext context;
+private final EntityInstantiators instantiators;
 private final DataAccessStrategy accessStrategy;
 private final RowMapperMap rowMapperMap;
-private final ConversionService conversionService;
 private final NamedParameterJdbcOperations operations;
 
+private final ConversionService conversionService;
+
 /**
  * Creates a new {@link JdbcQueryLookupStrategy} for the given {@link JdbcMappingContext}, {@link DataAccessStrategy}
  * and {@link RowMapperMap}.
- * 
+ *
  * @param context must not be {@literal null}.
  * @param accessStrategy must not be {@literal null}.
  * @param rowMapperMap must not be {@literal null}.
  */
-JdbcQueryLookupStrategy(JdbcMappingContext context, DataAccessStrategy accessStrategy, RowMapperMap rowMapperMap,
-NamedParameterJdbcOperations operations) {
+JdbcQueryLookupStrategy(JdbcMappingContext context, EntityInstantiators instantiators,
+DataAccessStrategy accessStrategy, RowMapperMap rowMapperMap, NamedParameterJdbcOperations operations) {
 
 Assert.notNull(context, "JdbcMappingContext must not be null!");
 Assert.notNull(accessStrategy, "DataAccessStrategy must not be null!");
 Assert.notNull(rowMapperMap, "RowMapperMap must not be null!");
 
 this.context = context;
+this.instantiators = instantiators;
 this.accessStrategy = accessStrategy;
 this.rowMapperMap = rowMapperMap;
 this.conversionService = context.getConversions();
@@ -104,6 +108,7 @@ private RowMapper<?> determineDefaultRowMapper(JdbcQueryMethod queryMethod) {
 ? new EntityRowMapper<>( //
 context.getRequiredPersistentEntity(domainType), //
 context, //
+instantiators, //
 accessStrategy) //
 : typeMappedRowMapper;
 }

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

@@ -18,6 +18,7 @@
 import java.util.Optional;
 
 import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.jdbc.core.DataAccessStrategy;
 import org.springframework.data.jdbc.core.JdbcAggregateTemplate;
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
@@ -49,6 +50,7 @@ public class JdbcRepositoryFactory extends RepositoryFactorySupport {
 private final NamedParameterJdbcOperations operations;
 
 private RowMapperMap rowMapperMap = RowMapperMap.EMPTY;
+private EntityInstantiators instantiators = new EntityInstantiators();
 
 /**
  * Creates a new {@link JdbcRepositoryFactory} for the given {@link DataAccessStrategy}, {@link JdbcMappingContext}
@@ -82,6 +84,18 @@ public void setRowMapperMap(RowMapperMap rowMapperMap) {
 this.rowMapperMap = rowMapperMap;
 }
 
+/**
+ * Set the {@link EntityInstantiators} used for instantiating entity instances.
+ * 
+ * @param instantiators Must not be {@code null}.
+ */
+public void setEntityInstantiators(EntityInstantiators instantiators) {
+
+Assert.notNull(instantiators, "EntityInstantiators must not be null.");
+
+this.instantiators = instantiators;
+}
+
 @SuppressWarnings("unchecked")
 @Override
 public <T, ID> EntityInformation<T, ID> getEntityInformation(Class<T> aClass) {
@@ -127,6 +141,6 @@ protected Optional<QueryLookupStrategy> getQueryLookupStrategy(QueryLookupStrate
 throw new IllegalArgumentException(String.format("Unsupported query lookup strategy %s!", key));
 }
 
-return Optional.of(new JdbcQueryLookupStrategy(context, accessStrategy, rowMapperMap, operations));
+return Optional.of(new JdbcQueryLookupStrategy(context, instantiators, accessStrategy, rowMapperMap, operations));
 }
 }

src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java

@@ -20,6 +20,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.ApplicationEventPublisherAware;
+import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.jdbc.core.DataAccessStrategy;
 import org.springframework.data.jdbc.core.DefaultDataAccessStrategy;
 import org.springframework.data.jdbc.core.SqlGeneratorSource;
@@ -49,6 +50,7 @@ public class JdbcRepositoryFactoryBean<T extends Repository<S, ID>, S, ID extend
 private DataAccessStrategy dataAccessStrategy;
 private RowMapperMap rowMapperMap = RowMapperMap.EMPTY;
 private NamedParameterJdbcOperations operations;
+private EntityInstantiators instantiators = new EntityInstantiators();
 
 /**
  * Creates a new {@link JdbcRepositoryFactoryBean} for the given repository interface.
@@ -115,6 +117,11 @@ public void setJdbcOperations(NamedParameterJdbcOperations operations) {
 this.operations = operations;
 }
 
+@Autowired(required = false)
+public void setInstantiators(EntityInstantiators instantiators) {
+this.instantiators = instantiators;
+}
+
 /*
  * (non-Javadoc)
  * @see org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport#afterPropertiesSet()
@@ -127,13 +134,18 @@ public void afterPropertiesSet() {
 if (dataAccessStrategy == null) {
 
 SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(mappingContext);
-this.dataAccessStrategy = new DefaultDataAccessStrategy(sqlGeneratorSource, mappingContext, operations);
+this.dataAccessStrategy = new DefaultDataAccessStrategy(sqlGeneratorSource, mappingContext, operations,
+instantiators);
 }
 
 if (rowMapperMap == null) {
 this.rowMapperMap = RowMapperMap.EMPTY;
 }
 
+if (instantiators == null) {
+this.instantiators = new EntityInstantiators();
+}
+
 super.afterPropertiesSet();
 }
 }

src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java

@@ -16,7 +16,8 @@
 package org.springframework.data.jdbc.core;
 
 import static org.assertj.core.api.Assertions.*;
-import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.*;
 
 import lombok.RequiredArgsConstructor;
@@ -26,8 +27,8 @@
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.springframework.data.annotation.Id;
+import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
-import org.springframework.data.jdbc.core.mapping.NamingStrategy;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
 import org.springframework.jdbc.core.namedparam.SqlParameterSource;
 import org.springframework.jdbc.support.KeyHolder;
@@ -47,8 +48,12 @@ public class DefaultDataAccessStrategyUnitTests {
 HashMap<String, Object> additionalParameters = new HashMap<>();
 ArgumentCaptor<SqlParameterSource> paramSourceCaptor = ArgumentCaptor.forClass(SqlParameterSource.class);
 
-DefaultDataAccessStrategy accessStrategy = new DefaultDataAccessStrategy(new SqlGeneratorSource(context), context,
-jdbcOperations);
+DefaultDataAccessStrategy accessStrategy = new DefaultDataAccessStrategy( //
+new SqlGeneratorSource(context), //
+context, //
+jdbcOperations, //
+new EntityInstantiators() //
+);
 
 @Test // DATAJDBC-146
 public void additionalParameterForIdDoesNotLeadToDuplicateParameters() {

src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java

@@ -17,7 +17,8 @@
 
 import static java.util.Arrays.*;
 import static org.assertj.core.api.Assertions.*;
-import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.*;
 
 import lombok.RequiredArgsConstructor;
@@ -40,12 +41,12 @@
 import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.core.convert.support.GenericConversionService;
 import org.springframework.data.annotation.Id;
+import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.convert.Jsr310Converters;
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
 import org.springframework.data.jdbc.core.mapping.JdbcPersistentEntity;
 import org.springframework.data.jdbc.core.mapping.JdbcPersistentProperty;
 import org.springframework.data.jdbc.core.mapping.NamingStrategy;
-import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
 import org.springframework.util.Assert;
 
 /**
@@ -199,8 +200,12 @@ private <T> EntityRowMapper<T> createRowMapper(Class<T> type, NamingStrategy nam
 DefaultConversionService.addDefaultConverters(conversionService);
 Jsr310Converters.getConvertersToRegister().forEach(conversionService::addConverter);
 
-return new EntityRowMapper<>((JdbcPersistentEntity<T>) context.getRequiredPersistentEntity(type), context,
-accessStrategy);
+return new EntityRowMapper<>( //
+(JdbcPersistentEntity<T>) context.getRequiredPersistentEntity(type), //
+context, //
+new EntityInstantiators(), //
+accessStrategy //
+);
 }
 
 private static ResultSet mockResultSet(List<String> columns, Object... values) {
@@ -212,7 +217,7 @@ private static ResultSet mockResultSet(List<String> columns, Object... values) {
 "Number of values [%d] must be a multiple of the number of columns [%d]", //
 values.length, //
 columns.size() //
-) //
+) //
 );
 
 List<Map<String, Object>> result = convertValues(columns, values);