Polishing.

Added `@since` comments for new methods and classes.
General formatting and code style tweaking.
Github references for new tests added.
Fixes for integration tests with various databases:
- Not all stores support submillisecond precision for Instant.
- Count for exists query doesn't work for all databases, nor does `LEAST(COUNT(1), 1)`
- MariaDB defaults timestamp columns to the current time.
- Ordering was applied twice.
- DATETIME in SqlServer has a most peculiar preceision. We switch to DATETIME2.

Original pull request #1195
See #1192

Author: schauder

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

@@ -196,6 +196,7 @@ public interface JdbcAggregateOperations {
 	 * @param entityClass the entity type must not be {@literal null}.
 	 * @return exactly one result or {@link Optional#empty()} if no match found.
 	 * @throws org.springframework.dao.IncorrectResultSizeDataAccessException if more than one match found.
+	 * @since 3.0
 	 */
 	<T> Optional<T> selectOne(Query query, Class<T> entityClass);
 
@@ -204,18 +205,19 @@ public interface JdbcAggregateOperations {
 	 *
 	 * @param query must not be {@literal null}.
 	 * @param entityClass the entity type must not be {@literal null}.
-	 * @param sort the sorting that should be used on the result.
 	 * @return a non-null sorted list with all the matching results.
 	 * @throws org.springframework.dao.IncorrectResultSizeDataAccessException if more than one match found.
+	 * @since 3.0
 	 */
-	<T> Iterable<T> select(Query query, Class<T> entityClass, Sort sort);
+	<T> Iterable<T> select(Query query, Class<T> entityClass);
 
 	/**
 	 * Determine whether there are aggregates that match the {@link Query}
 	 *
 	 * @param query must not be {@literal null}.
 	 * @param entityClass the entity type must not be {@literal null}.
 	 * @return {@literal true} if the object exists.
+	 * @since 3.0
 	 */
 	<T> boolean exists(Query query, Class<T> entityClass);
 
@@ -225,6 +227,7 @@ public interface JdbcAggregateOperations {
 	 * @param query must not be {@literal null}.
 	 * @param entityClass the entity type must not be {@literal null}.
 	 * @return the number of instances stored in the database. Guaranteed to be not {@code null}.
+	 * @since 3.0
 	 */
 	<T> long count(Query query, Class<T> entityClass);
 
@@ -236,6 +239,7 @@ public interface JdbcAggregateOperations {
 	 * @param entityClass the entity type must not be {@literal null}.
 	 * @param pageable can be null.
 	 * @return a {@link Page} of entities matching the given {@link Example}.
+	 * @since 3.0
 	 */
 	<T> Page<T> select(Query query, Class<T> entityClass, Pageable pageable);
 }

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

@@ -74,7 +74,6 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations {
 
 	private final DataAccessStrategy accessStrategy;
 	private final AggregateChangeExecutor executor;
-
 	private final JdbcConverter converter;
 
 	private EntityCallbacks entityCallbacks = EntityCallbacks.create();
@@ -248,7 +247,7 @@ public <T> Optional<T> selectOne(Query query, Class<T> entityClass) {
 	}
 
 	@Override
-	public <T> Iterable<T> select(Query query, Class<T> entityClass, Sort sort) {
+	public <T> Iterable<T> select(Query query, Class<T> entityClass) {
 		return accessStrategy.select(query, entityClass);
 	}
 
@@ -264,16 +263,13 @@ public <T> long count(Query query, Class<T> entityClass) {
 
 	@Override
 	public <T> Page<T> select(Query query, Class<T> entityClass, Pageable pageable) {
+
 		Iterable<T> items = triggerAfterConvert(accessStrategy.select(query, entityClass, pageable));
 		List<T> content = StreamSupport.stream(items.spliterator(), false).collect(Collectors.toList());
 
 		return PageableExecutionUtils.getPage(content, pageable, () -> accessStrategy.count(query, entityClass));
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.data.jdbc.core.JdbcAggregateOperations#findAll(java.lang.Class)
-	 */
 	@Override
 	public <T> Iterable<T> findAll(Class<T> domainType) {
 

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

@@ -15,6 +15,8 @@
  */
 package org.springframework.data.jdbc.core.convert;
 
+import static java.lang.Boolean.*;
+
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -29,10 +31,8 @@
 import org.springframework.data.relational.core.query.Query;
 import org.springframework.data.relational.core.sql.LockMode;
 
-import static java.lang.Boolean.*;
-
 /**
- * Delegates each methods to the {@link DataAccessStrategy}s passed to the constructor in turn until the first that does
+ * Delegates each method to the {@link DataAccessStrategy}s passed to the constructor in turn until the first that does
  * not throw an exception.
  *
  * @author Jens Schauder

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

@@ -114,8 +114,8 @@ public interface DataAccessStrategy extends RelationResolver {
 	 * @param previousVersion The previous version assigned to the instance being saved.
 	 * @param <T> the type of the instance to save.
 	 * @return whether the update actually updated a row.
-	 * @throws OptimisticLockingFailureException if the update fails to update at least one row assuming the
-	 *           optimistic locking version check failed.
+	 * @throws OptimisticLockingFailureException if the update fails to update at least one row assuming the optimistic
+	 *           locking version check failed.
 	 * @since 2.0
 	 */
 	<T> boolean updateWithVersion(T instance, Class<T> domainType, Number previousVersion);
@@ -155,8 +155,8 @@ public interface DataAccessStrategy extends RelationResolver {
 	 * @param domainType the type of entity to be deleted. Implicitly determines the table to operate on. Must not be
 	 *          {@code null}.
 	 * @param previousVersion The previous version assigned to the instance being saved.
-	 * @throws OptimisticLockingFailureException if the update fails to update at least one row assuming the
-	 *           optimistic locking version check failed.
+	 * @throws OptimisticLockingFailureException if the update fails to update at least one row assuming the optimistic
+	 *           locking version check failed.
 	 * @since 2.0
 	 */
 	<T> void deleteWithVersion(Object id, Class<T> domainType, Number previousVersion);
@@ -292,6 +292,7 @@ Iterable<Object> findAllByPath(Identifier identifier,
 	 * @param probeType the type of entities. Must not be {@code null}.
 	 * @return exactly one result or {@link Optional#empty()} if no match found.
 	 * @throws org.springframework.dao.IncorrectResultSizeDataAccessException if more than one match found.
+	 * @since 3.0
 	 */
 	<T> Optional<T> selectOne(Query query, Class<T> probeType);
 
@@ -302,6 +303,7 @@ Iterable<Object> findAllByPath(Identifier identifier,
 	 * @param probeType the type of entities. Must not be {@code null}.
 	 * @return a non-null list with all the matching results.
 	 * @throws org.springframework.dao.IncorrectResultSizeDataAccessException if more than one match found.
+	 * @since 3.0
 	 */
 	<T> Iterable<T> select(Query query, Class<T> probeType);
 
@@ -314,6 +316,7 @@ Iterable<Object> findAllByPath(Identifier identifier,
 	 * @param pageable the pagination that should be applied. Must not be {@literal null}.
 	 * @return a non-null list with all the matching results.
 	 * @throws org.springframework.dao.IncorrectResultSizeDataAccessException if more than one match found.
+	 * @since 3.0
 	 */
 	<T> Iterable<T> select(Query query, Class<T> probeType, Pageable pageable);
 
@@ -323,6 +326,7 @@ Iterable<Object> findAllByPath(Identifier identifier,
 	 * @param query must not be {@literal null}.
 	 * @param probeType the type of entities. Must not be {@code null}.
 	 * @return {@literal true} if the object exists.
+	 * @since 3.0
 	 */
 	<T> boolean exists(Query query, Class<T> probeType);
 
@@ -332,6 +336,7 @@ Iterable<Object> findAllByPath(Identifier identifier,
 	 * @param probeType the probe type for which to count the elements. Must not be {@code null}.
 	 * @param query the query which elements have to match.
 	 * @return the count. Guaranteed to be not {@code null}.
+	 * @since 3.0
 	 */
 	<T> long count(Query query, Class<T> probeType);
 }

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

@@ -20,9 +20,7 @@
 import java.sql.ResultSet;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
-import java.util.function.Predicate;
 
 import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.dao.OptimisticLockingFailureException;
@@ -262,27 +260,24 @@ public long count(Class<?> domainType) {
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
 	public <T> T findById(Object id, Class<T> domainType) {
 
 		String findOneSql = sql(domainType).getFindOne();
 		SqlIdentifierParameterSource parameter = sqlParametersFactory.forQueryById(id, domainType, ID_SQL_PARAMETER);
 
 		try {
-			return operations.queryForObject(findOneSql, parameter, (RowMapper<T>) getEntityRowMapper(domainType));
+			return operations.queryForObject(findOneSql, parameter, getEntityRowMapper(domainType));
 		} catch (EmptyResultDataAccessException e) {
 			return null;
 		}
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
 	public <T> Iterable<T> findAll(Class<T> domainType) {
-		return operations.query(sql(domainType).getFindAll(), (RowMapper<T>) getEntityRowMapper(domainType));
+		return operations.query(sql(domainType).getFindAll(), getEntityRowMapper(domainType));
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
 	public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 
 		if (!ids.iterator().hasNext()) {
@@ -293,7 +288,7 @@ public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 
 		String findAllInListSql = sql(domainType).getFindAllInList();
 
-		return operations.query(findAllInListSql, parameterSource, (RowMapper<T>) getEntityRowMapper(domainType));
+		return operations.query(findAllInListSql, parameterSource, getEntityRowMapper(domainType));
 	}
 
 	@Override
@@ -330,73 +325,74 @@ public <T> boolean existsById(Object id, Class<T> domainType) {
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
 	public <T> Iterable<T> findAll(Class<T> domainType, Sort sort) {
-		return operations.query(sql(domainType).getFindAll(sort), (RowMapper<T>) getEntityRowMapper(domainType));
+		return operations.query(sql(domainType).getFindAll(sort), getEntityRowMapper(domainType));
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
 	public <T> Iterable<T> findAll(Class<T> domainType, Pageable pageable) {
-		return operations.query(sql(domainType).getFindAll(pageable), (RowMapper<T>) getEntityRowMapper(domainType));
+		return operations.query(sql(domainType).getFindAll(pageable), getEntityRowMapper(domainType));
 	}
 
 	@Override
 	public <T> Optional<T> selectOne(Query query, Class<T> probeType) {
+
 		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
 		String sqlQuery = sql(probeType).selectByQuery(query, parameterSource);
 
-		T foundObject;
 		try {
-			foundObject = operations.queryForObject(sqlQuery, parameterSource, (RowMapper<T>) getEntityRowMapper(probeType));
+			return Optional.ofNullable(
+					operations.queryForObject(sqlQuery, parameterSource, getEntityRowMapper(probeType)));
 		} catch (EmptyResultDataAccessException e) {
-			foundObject = null;
+			return Optional.empty();
 		}
-
-		return Optional.ofNullable(foundObject);
 	}
 
 	@Override
 	public <T> Iterable<T> select(Query query, Class<T> probeType) {
+
 		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
 		String sqlQuery = sql(probeType).selectByQuery(query, parameterSource);
 
-		return operations.query(sqlQuery, parameterSource, (RowMapper<T>) getEntityRowMapper(probeType));
+		return operations.query(sqlQuery, parameterSource, getEntityRowMapper(probeType));
 	}
 
 	@Override
 	public <T> Iterable<T> select(Query query, Class<T> probeType, Pageable pageable) {
+
 		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
 		String sqlQuery = sql(probeType).selectByQuery(query, parameterSource, pageable);
 
-		return operations.query(sqlQuery, parameterSource, (RowMapper<T>) getEntityRowMapper(probeType));
+		return operations.query(sqlQuery, parameterSource, getEntityRowMapper(probeType));
 	}
 
 	@Override
 	public <T> boolean exists(Query query, Class<T> probeType) {
-		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
 
+		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
 		String sqlQuery = sql(probeType).existsByQuery(query, parameterSource);
 
 		Boolean result = operations.queryForObject(sqlQuery, parameterSource, Boolean.class);
-		Assert.notNull(result, "The result of an exists query must not be null");
+
+		Assert.state(result != null, "The result of an exists query must not be null");
 
 		return result;
 	}
 
 	@Override
 	public <T> long count(Query query, Class<T> probeType) {
+
 		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
 		String sqlQuery = sql(probeType).countByQuery(query, parameterSource);
 
 		Long result = operations.queryForObject(sqlQuery, parameterSource, Long.class);
 
-		Assert.notNull(result, "The result of a count query must not be null.");
+		Assert.state(result != null, "The result of a count query must not be null.");
 
 		return result;
 	}
 
-	private EntityRowMapper<?> getEntityRowMapper(Class<?> domainType) {
+	private <T> EntityRowMapper<T> getEntityRowMapper(Class<T> domainType) {
 		return new EntityRowMapper<>(getRequiredPersistentEntity(domainType), converter);
 	}
 

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

@@ -16,6 +16,7 @@
 package org.springframework.data.jdbc.core.convert;
 
 import java.util.List;
+import java.util.Optional;
 
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
@@ -26,8 +27,6 @@
 import org.springframework.data.relational.core.sql.LockMode;
 import org.springframework.util.Assert;
 
-import java.util.Optional;
-
 /**
  * Delegates all method calls to an instance set after construction. This is useful for {@link DataAccessStrategy}s with
  * cyclic dependencies.