DATAJDBC-137 - Polishing.

Moved the dependency to NamedParameterJdbcOperations out of JdbcMappingContext. This revealed that despite the DataAccessStrategy abstraction, there are a couple of places in the query execution subsystem that work with a plain NamedParameterJdbcOperations instance, so that we now have to carry that around all the way from the repository factory bean. Improving that is subject for further changes.

A bit of JavaDoc and generics polish here and there.

Author: odrotbohm

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

@@ -21,7 +21,8 @@
 import org.springframework.data.mapping.PropertyPath;
 
 /**
- * Abstraction for accesses to the database that should be implementable with a single SQL statement and relates to a single entity as opposed to {@link JdbcEntityOperations} which provides interactions related to complete aggregates.
+ * Abstraction for accesses to the database that should be implementable with a single SQL statement and relates to a
+ * single entity as opposed to {@link JdbcEntityOperations} which provides interactions related to complete aggregates.
  *
  * @author Jens Schauder
  * @since 1.0
@@ -68,5 +69,4 @@ public interface DataAccessStrategy {
 	<T> Iterable<T> findAllByProperty(Object rootId, JdbcPersistentProperty property);
 
 	<T> boolean existsById(Object id, Class<T> domainType);
-
 }

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

@@ -15,6 +15,9 @@
  */
 package org.springframework.data.jdbc.core;
 
+import lombok.NonNull;
+import lombok.RequiredArgsConstructor;
+
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -46,34 +49,27 @@
  * @author Jens Schauder
  * @since 1.0
  */
+@RequiredArgsConstructor
 public class DefaultDataAccessStrategy implements DataAccessStrategy {
 
 	private static final String ENTITY_NEW_AFTER_INSERT = "Entity [%s] still 'new' after insert. Please set either"
 			+ " the id property in a BeforeInsert event handler, or ensure the database creates a value and your "
 			+ "JDBC driver returns it.";
 
-	private final SqlGeneratorSource sqlGeneratorSource;
-	private final NamedParameterJdbcOperations operations;
-	private final JdbcMappingContext context;
-	private final DataAccessStrategy accessStrategy;
-
-	public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, JdbcMappingContext context,
-			DataAccessStrategy accessStrategy) {
-
-		this.sqlGeneratorSource = sqlGeneratorSource;
-		this.operations = context.getTemplate();
-		this.context = context;
-		this.accessStrategy = accessStrategy;
-	}
+	private final @NonNull SqlGeneratorSource sqlGeneratorSource;
+	private final @NonNull JdbcMappingContext context;
+	private final @NonNull DataAccessStrategy accessStrategy;
+	private final @NonNull NamedParameterJdbcOperations operations;
 
 	/**
 	 * 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) {
+	public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, JdbcMappingContext context,
+			NamedParameterJdbcOperations operations) {
 
 		this.sqlGeneratorSource = sqlGeneratorSource;
-		this.operations = context.getTemplate();
+		this.operations = operations;
 		this.context = context;
 		this.accessStrategy = this;
 	}
@@ -110,9 +106,12 @@ public <T> void insert(T instance, Class<T> domainType, Map<String, Object> addi
 		if (idProperty != null && idValue == null && entityInformation.isNew(instance)) {
 			throw new IllegalStateException(String.format(ENTITY_NEW_AFTER_INSERT, persistentEntity));
 		}
-
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#update(java.lang.Object, java.lang.Class)
+	 */
 	@Override
 	public <S> void update(S instance, Class<S> domainType) {
 
@@ -121,6 +120,10 @@ public <S> void update(S instance, Class<S> domainType) {
 		operations.update(sql(domainType).getUpdate(), getPropertyMap(instance, persistentEntity));
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#delete(java.lang.Object, java.lang.Class)
+	 */
 	@Override
 	public void delete(Object id, Class<?> domainType) {
 
@@ -130,6 +133,10 @@ public void delete(Object id, Class<?> domainType) {
 		operations.update(deleteByIdSql, parameter);
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#delete(java.lang.Object, org.springframework.data.mapping.PropertyPath)
+	 */
 	@Override
 	public void delete(Object rootId, PropertyPath propertyPath) {
 
@@ -145,39 +152,63 @@ public void delete(Object rootId, PropertyPath propertyPath) {
 		operations.update(format, parameters);
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteAll(java.lang.Class)
+	 */
 	@Override
 	public <T> void deleteAll(Class<T> domainType) {
 		operations.getJdbcOperations().update(sql(domainType).createDeleteAllSql(null));
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteAll(org.springframework.data.mapping.PropertyPath)
+	 */
 	@Override
 	public <T> void deleteAll(PropertyPath propertyPath) {
 		operations.getJdbcOperations().update(sql(propertyPath.getOwningType().getType()).createDeleteAllSql(propertyPath));
 	}
 
-	@SuppressWarnings("ConstantConditions")
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#count(java.lang.Class)
+	 */
 	@Override
 	public long count(Class<?> domainType) {
 		return operations.getJdbcOperations().queryForObject(sql(domainType).getCount(), Long.class);
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, java.lang.Class)
+	 */
 	@Override
 	public <T> T findById(Object id, Class<T> domainType) {
 
 		String findOneSql = sql(domainType).getFindOne();
 		MapSqlParameterSource parameter = createIdParameterSource(id, domainType);
+
 		try {
 			return operations.queryForObject(findOneSql, parameter, getEntityRowMapper(domainType));
 		} catch (EmptyResultDataAccessException e) {
 			return null;
 		}
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAll(java.lang.Class)
+	 */
 	@Override
 	public <T> Iterable<T> findAll(Class<T> domainType) {
 		return operations.query(sql(domainType).getFindAll(), getEntityRowMapper(domainType));
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAllById(java.lang.Iterable, java.lang.Class)
+	 */
 	@Override
 	public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 
@@ -194,15 +225,19 @@ public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 		return operations.query(findAllInListSql, parameter, getEntityRowMapper(domainType));
 	}
 
-	@SuppressWarnings("unchecked")
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAllByProperty(java.lang.Object, org.springframework.data.jdbc.mapping.model.JdbcPersistentProperty)
+	 */
 	@Override
+	@SuppressWarnings("unchecked")
 	public <T> Iterable<T> findAllByProperty(Object rootId, JdbcPersistentProperty property) {
 
 		Assert.notNull(rootId, "rootId must not be null.");
 
 		Class<?> actualType = property.getActualType();
-		String findAllByProperty = sql(actualType).getFindAllByProperty(property.getReverseColumnName(),
-				property.getKeyColumn(), property.isOrdered());
+		String findAllByProperty = sql(actualType) //
+				.getFindAllByProperty(property.getReverseColumnName(), property.getKeyColumn(), property.isOrdered());
 
 		MapSqlParameterSource parameter = new MapSqlParameterSource(property.getReverseColumnName(), rootId);
 
@@ -211,11 +246,16 @@ public <T> Iterable<T> findAllByProperty(Object rootId, JdbcPersistentProperty p
 				: getEntityRowMapper(actualType));
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#existsById(java.lang.Object, java.lang.Class)
+	 */
 	@Override
 	public <T> boolean existsById(Object id, Class<T> domainType) {
 
 		String existsSql = sql(domainType).getExists();
 		MapSqlParameterSource parameter = createIdParameterSource(id, domainType);
+
 		return operations.queryForObject(existsSql, parameter, Boolean.class);
 	}
 

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

@@ -20,6 +20,7 @@
 
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.Map;
 
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.converter.Converter;
@@ -35,8 +36,6 @@
 import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
 import org.springframework.data.mapping.model.ParameterValueProvider;
 import org.springframework.jdbc.core.RowMapper;
-import org.springframework.lang.Nullable;
-import org.springframework.util.Assert;
 
 /**
  * Maps a ResultSet to an entity of type {@code T}, including entities referenced.
@@ -47,7 +46,7 @@
  */
 public class EntityRowMapper<T> implements RowMapper<T> {
 
-	private static final Converter ITERABLE_OF_ENTRY_TO_MAP_CONVERTER = new IterableOfEntryToMapConverter();
+	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();
@@ -101,14 +100,12 @@ private T createInstance(ResultSet rs) {
 		return instantiator.createInstance(entity, new ResultSetParameterValueProvider(rs, entity, conversions, ""));
 	}
 
-
 	/**
 	 * Read a single value or a complete Entity from the {@link ResultSet} passed as an argument.
 	 *
 	 * @param resultSet the {@link ResultSet} to extract the value from. Must not be {@code null}.
 	 * @param property the {@link JdbcPersistentProperty} for which the value is intended. Must not be {@code null}.
 	 * @param prefix to be used for all column names accessed by this method. Must not be {@code null}.
-	 *
 	 * @return the value read from the {@link ResultSet}. May be {@code null}.
 	 */
 	private Object readFrom(ResultSet resultSet, JdbcPersistentProperty property, String prefix) {

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

@@ -15,27 +15,27 @@
  */
 package org.springframework.data.jdbc.core;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.converter.ConditionalConverter;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Map.Entry;
-
 /**
  * A converter for creating a {@link Map} from an {@link Iterable<Map.Entry>}.
  *
  * @author Jens Schauder
  * @since 1.0
  */
-class IterableOfEntryToMapConverter implements ConditionalConverter, Converter<Iterable, Map> {
+class IterableOfEntryToMapConverter implements ConditionalConverter, Converter<Iterable<?>, Map<?, ?>> {
 
 	@Nullable
 	@Override
-	public Map convert(Iterable source) {
+	public Map<?, ?> convert(Iterable<?> source) {
 
 		Map result = new HashMap();
 

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

@@ -40,5 +40,4 @@ public interface JdbcEntityOperations {
 	<T> Iterable<T> findAll(Class<T> domainType);
 
 	<T> boolean existsById(Object id, Class<T> domainType);
-
 }

src/main/java/org/springframework/data/jdbc/mapping/model/ConversionCustomizer.java

@@ -23,5 +23,7 @@
  */
 public interface ConversionCustomizer {
 
+	public static ConversionCustomizer NONE = __ -> {};
+
 	void customize(GenericConversionService conversions);
 }

src/main/java/org/springframework/data/jdbc/mapping/model/JdbcMappingContext.java

@@ -37,14 +37,15 @@
 import org.springframework.data.mapping.model.Property;
 import org.springframework.data.mapping.model.SimpleTypeHolder;
 import org.springframework.data.util.TypeInformation;
-import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
+import org.springframework.util.Assert;
 
 /**
  * {@link MappingContext} implementation for JDBC.
  *
  * @author Jens Schauder
  * @author Greg Turnquist
  * @author Kazuki Shimizu
+ * @author Oliver Gierke
  * @since 1.0
  */
 public class JdbcMappingContext extends AbstractMappingContext<JdbcPersistentEntity<?>, JdbcPersistentProperty> {
@@ -56,24 +57,37 @@ public class JdbcMappingContext extends AbstractMappingContext<JdbcPersistentEnt
 	));
 
 	@Getter private final NamingStrategy namingStrategy;
-	@Getter private final NamedParameterJdbcOperations template;
 	@Getter private SimpleTypeHolder simpleTypeHolder;
 	private GenericConversionService conversions = getDefaultConversionService();
 
-	public JdbcMappingContext(NamingStrategy namingStrategy, NamedParameterJdbcOperations template,
-			ConversionCustomizer customizer) {
+	/**
+	 * Creates a new {@link JdbcMappingContext}.
+	 */
+	public JdbcMappingContext() {
+		this(NamingStrategy.INSTANCE, ConversionCustomizer.NONE);
+	}
+
+	public JdbcMappingContext(NamingStrategy namingStrategy) {
+		this(namingStrategy, ConversionCustomizer.NONE);
+	}
+
+	/**
+	 * Creates a new {@link JdbcMappingContext} using the given {@link NamingStrategy} and {@link ConversionCustomizer}.
+	 * 
+	 * @param namingStrategy must not be {@literal null}.
+	 * @param customizer must not be {@literal null}.
+	 */
+	public JdbcMappingContext(NamingStrategy namingStrategy, ConversionCustomizer customizer) {
+
+		Assert.notNull(namingStrategy, "NamingStrategy must not be null!");
+		Assert.notNull(customizer, "ConversionCustomizer must not be null!");
 
 		this.namingStrategy = namingStrategy;
-		this.template = template;
 
 		customizer.customize(conversions);
 		setSimpleTypeHolder(new SimpleTypeHolder(CUSTOM_SIMPLE_TYPES, true));
 	}
 
-	public JdbcMappingContext(NamedParameterJdbcOperations template) {
-		this(NamingStrategy.INSTANCE, template, __ -> {});
-	}
-
 	@Override
 	public void setSimpleTypeHolder(SimpleTypeHolder simpleTypes) {
 		super.setSimpleTypeHolder(simpleTypes);