DATAJDBC-318 - Adding a failing test.

Added failing tests for entities with references.

Code deduplication.
Documentation wording.
Formatting.

Original pull request: #209.

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/AbstractJdbcQuery.java

@@ -27,8 +27,8 @@ import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
- * A query to be executed based on a repository method, it's annotated SQL query and the arguments provided to the
- * method.
+ * Base class for queries based on a repository method. It holds the infrastructure for executing a query and knows how
+ * to execute a query based on the return type of the method. How to construct the query is left to subclasses.
  *
  * @author Jens Schauder
  * @author Kazuki Shimizu

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/JdbcQueryCreator.java

@@ -65,7 +65,7 @@ class JdbcQueryCreator extends RelationalQueryCreator<ParametrizedQuery> {
 	 * @param entityMetadata relational entity metadata, must not be {@literal null}.
 	 * @param accessor parameter metadata provider, must not be {@literal null}.
 	 */
-	public JdbcQueryCreator(PartTree tree, JdbcConverter converter, Dialect dialect,
+	JdbcQueryCreator(PartTree tree, JdbcConverter converter, Dialect dialect,
 			RelationalEntityMetadata<?> entityMetadata, RelationalParameterAccessor accessor) {
 		super(tree, accessor);
 

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/JdbcQueryExecution.java

@@ -19,7 +19,8 @@ import org.springframework.jdbc.core.namedparam.SqlParameterSource;
 import org.springframework.lang.Nullable;
 
 /**
- * Interface specifying a result execution strategy.
+ * Interface specifying a query execution strategy. Implementations encapsulate information how to actually execute the
+ * query and how to process the result in order to get the desired return type.
  * 
  * @author Mark Paluch
  * @since 2.0
@@ -28,11 +29,11 @@ import org.springframework.lang.Nullable;
 interface JdbcQueryExecution<T> {
 
 	/**
-	 * Execute the given {@code query}.
+	 * Execute the given {@code query} and {@code parameter} and transforms the result into a {@code T}.
 	 * 
-	 * @param query
-	 * @param parameter
-	 * @return
+	 * @param query the query to be executed. Must not be {@literal null}.
+	 * @param parameter the parameters to be bound to the query. Must not be {@literal null}.
+	 * @return the result of the query. Might be {@literal null}.
 	 */
 	@Nullable
 	T execute(String query, SqlParameterSource parameter);

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/ParametrizedQuery.java

@@ -18,7 +18,7 @@ package org.springframework.data.jdbc.repository.query;
 import org.springframework.jdbc.core.namedparam.SqlParameterSource;
 
 /**
- * Value object encapsulating a parametrized query containing named parameters and {@link SqlParameterSource}.
+ * Value object encapsulating a query containing named parameters and a{@link SqlParameterSource} to bind the parameters.
  * 
  * @author Mark Paluch
  * @since 2.0
@@ -28,16 +28,17 @@ class ParametrizedQuery {
 	private final String query;
 	private final SqlParameterSource parameterSource;
 
-	public ParametrizedQuery(String query, SqlParameterSource parameterSource) {
+	ParametrizedQuery(String query, SqlParameterSource parameterSource) {
+
 		this.query = query;
 		this.parameterSource = parameterSource;
 	}
 
-	public String getQuery() {
+	String getQuery() {
 		return query;
 	}
 
-	public SqlParameterSource getParameterSource() {
+	SqlParameterSource getParameterSource() {
 		return parameterSource;
 	}
 

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/PartTreeJdbcQuery.java

@@ -64,9 +64,11 @@ public class PartTreeJdbcQuery extends AbstractJdbcQuery {
 		this.converter = converter;
 
 		try {
+
 			this.tree = new PartTree(queryMethod.getName(), queryMethod.getEntityInformation().getJavaType());
 			JdbcQueryCreator.validate(this.tree, this.parameters);
 		} catch (RuntimeException e) {
+
 			throw new IllegalArgumentException(
 					String.format("Failed to create query for method %s! %s", queryMethod, e.getMessage()), e);
 		}

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java

@@ -69,7 +69,7 @@ class QueryMapper {
 	 * @param converter must not be {@literal null}.
 	 */
 	@SuppressWarnings({ "unchecked", "rawtypes" })
-	public QueryMapper(Dialect dialect, JdbcConverter converter) {
+	QueryMapper(Dialect dialect, JdbcConverter converter) {
 
 		Assert.notNull(dialect, "Dialect must not be null!");
 		Assert.notNull(converter, "JdbcConverter must not be null!");
@@ -80,13 +80,13 @@ class QueryMapper {
 	}
 
 	/**
-	 * Map the {@link Sort} object to apply field name mapping using {@link Class the type to read}.
+	 * Map the {@link Sort} object to apply field name mapping using {@link RelationalPersistentEntity the type to read}.
 	 *
 	 * @param sort must not be {@literal null}.
 	 * @param entity related {@link RelationalPersistentEntity}, can be {@literal null}.
 	 * @return
 	 */
-	public List<OrderByField> getMappedSort(Table table, Sort sort, @Nullable RelationalPersistentEntity<?> entity) {
+	List<OrderByField> getMappedSort(Table table, Sort sort, @Nullable RelationalPersistentEntity<?> entity) {
 
 		List<OrderByField> mappedOrder = new ArrayList<>();
 
@@ -102,13 +102,14 @@ class QueryMapper {
 	}
 
 	/**
-	 * Map the {@link Expression} object to apply field name mapping using {@link Class the type to read}.
+	 * Map the {@link Expression} object to apply field name mapping using {@link RelationalPersistentEntity the type to
+	 * read}.
 	 *
 	 * @param expression must not be {@literal null}.
 	 * @param entity related {@link RelationalPersistentEntity}, can be {@literal null}.
-	 * @return the mapped {@link Expression}.
+	 * @return the mapped {@link Expression}. Guaranteed to be not {@literal null}.
 	 */
-	public Expression getMappedObject(Expression expression, @Nullable RelationalPersistentEntity<?> entity) {
+	Expression getMappedObject(Expression expression, @Nullable RelationalPersistentEntity<?> entity) {
 
 		if (entity == null || expression instanceof AsteriskFromTable) {
 			return expression;
@@ -120,6 +121,8 @@ class QueryMapper {
 			Field field = createPropertyField(entity, column.getName());
 			Table table = column.getTable();
 
+			Assert.state(table != null, String.format("The column %s must have a table set.", column));
+
 			Column columnFromTable = table.column(field.getMappedColumnName());
 			return column instanceof Aliased ? columnFromTable.as(((Aliased) column).getAlias()) : columnFromTable;
 		}
@@ -144,7 +147,7 @@ class QueryMapper {
 	}
 
 	/**
-	 * Map a {@link CriteriaDefinition} object into {@link Condition} and consider value/{@code NULL} {@link Bindings}.
+	 * Map a {@link CriteriaDefinition} object into {@link Condition} and consider value/{@code NULL} bindings.
 	 *
 	 * @param parameterSource bind parameterSource object, must not be {@literal null}.
 	 * @param criteria criteria definition to map, must not be {@literal null}.
@@ -152,7 +155,7 @@ class QueryMapper {
 	 * @param entity related {@link RelationalPersistentEntity}, can be {@literal null}.
 	 * @return the mapped {@link Condition}.
 	 */
-	public Condition getMappedObject(MapSqlParameterSource parameterSource, CriteriaDefinition criteria, Table table,
+	Condition getMappedObject(MapSqlParameterSource parameterSource, CriteriaDefinition criteria, Table table,
 			@Nullable RelationalPersistentEntity<?> entity) {
 
 		Assert.notNull(parameterSource, "MapSqlParameterSource must not be null!");
@@ -349,12 +352,12 @@ class QueryMapper {
 
 			Pair<Object, Object> pair = (Pair<Object, Object>) value;
 
-			Object first = convertValue(pair.getFirst(),
-					typeInformation.getActualType() != null ? typeInformation.getRequiredActualType()
+			Object first = convertValue(pair.getFirst(), typeInformation.getActualType() != null //
+					? typeInformation.getRequiredActualType()
 					: ClassTypeInformation.OBJECT);
 
-			Object second = convertValue(pair.getSecond(),
-					typeInformation.getActualType() != null ? typeInformation.getRequiredActualType()
+			Object second = convertValue(pair.getSecond(), typeInformation.getActualType() != null //
+					? typeInformation.getRequiredActualType()
 					: ClassTypeInformation.OBJECT);
 
 			return Pair.of(first, second);
@@ -365,6 +368,7 @@ class QueryMapper {
 			List<Object> mapped = new ArrayList<>();
 
 			for (Object o : (Iterable<?>) value) {
+
 				mapped.add(convertValue(o, typeInformation.getActualType() != null ? typeInformation.getRequiredActualType()
 						: ClassTypeInformation.OBJECT));
 			}
@@ -498,10 +502,6 @@ class QueryMapper {
 		return entity == null ? new Field(key) : new MetadataBackedField(key, entity, mappingContext, converter);
 	}
 
-	Class<?> getTypeHint(@Nullable Object mappedValue, Class<?> propertyType) {
-		return propertyType;
-	}
-
 	int getTypeHint(@Nullable Object mappedValue, Class<?> propertyType, JdbcValue settableValue) {
 
 		if (mappedValue == null || propertyType.equals(Object.class)) {
@@ -560,7 +560,7 @@ class QueryMapper {
 		 *
 		 * @param name must not be {@literal null} or empty.
 		 */
-		public Field(SqlIdentifier name) {
+		Field(SqlIdentifier name) {
 
 			Assert.notNull(name, "Name must not be null!");
 			this.name = name;

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

@@ -94,11 +94,7 @@ class JdbcQueryLookupStrategy implements QueryLookupStrategy {
 		JdbcQueryMethod queryMethod = new JdbcQueryMethod(method, repositoryMetadata, projectionFactory, namedQueries,
 				context);
 
-		if (namedQueries.hasQuery(queryMethod.getNamedQueryName())) {
-
-			RowMapper<?> mapper = queryMethod.isModifyingQuery() ? null : createMapper(queryMethod);
-			return new StringBasedJdbcQuery(queryMethod, operations, mapper, converter);
-		} else if (queryMethod.hasAnnotatedQuery()) {
+		if (namedQueries.hasQuery(queryMethod.getNamedQueryName()) || queryMethod.hasAnnotatedQuery()) {
 
 			RowMapper<?> mapper = queryMethod.isModifyingQuery() ? null : createMapper(queryMethod);
 			return new StringBasedJdbcQuery(queryMethod, operations, mapper, converter);

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

@@ -19,7 +19,6 @@ import static org.assertj.core.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
 import lombok.AllArgsConstructor;
-import lombok.Data;
 
 import java.lang.reflect.Method;
 import java.util.Collection;
@@ -31,7 +30,6 @@ import java.util.Properties;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.junit.MockitoJUnitRunner;
-
 import org.springframework.data.annotation.Id;
 import org.springframework.data.jdbc.core.convert.BasicJdbcConverter;
 import org.springframework.data.jdbc.core.convert.JdbcConverter;
@@ -63,6 +61,26 @@ public class PartTreeJdbcQueryUnitTests {
 	JdbcMappingContext mappingContext = new JdbcMappingContext();
 	JdbcConverter converter = new BasicJdbcConverter(mappingContext, mock(RelationResolver.class));
 
+	@Test // DATAJDBC-318
+	public void selectContainsColumnsForOneToOneReference() throws Exception {
+
+		JdbcQueryMethod queryMethod = getQueryMethod("findAllByFirstName", String.class);
+		PartTreeJdbcQuery jdbcQuery = createQuery(queryMethod);
+		ParametrizedQuery query = jdbcQuery.createQuery(getAccessor(queryMethod, new Object[] { "John" }));
+
+		assertThat(query.getQuery()).contains("hated.\"name\" AS \"hated_name\"");
+	}
+
+	@Test // DATAJDBC-318
+	public void doesNotContainsColumnsForOneToManyReference() throws Exception{
+
+		JdbcQueryMethod queryMethod = getQueryMethod("findAllByFirstName", String.class);
+		PartTreeJdbcQuery jdbcQuery = createQuery(queryMethod);
+		ParametrizedQuery query = jdbcQuery.createQuery(getAccessor(queryMethod, new Object[] { "John" }));
+
+		assertThat(query.getQuery().toLowerCase()).doesNotContain("hobbies");
+	}
+
 	@Test // DATAJDBC-318
 	public void createsQueryToFindAllEntitiesByStringAttribute() throws Exception {
 
@@ -622,7 +640,6 @@ public class PartTreeJdbcQueryUnitTests {
 	}
 
 	@Table("users")
-	@Data
 	static class User {
 
 		@Id Long id;
@@ -633,12 +650,18 @@ public class PartTreeJdbcQueryUnitTests {
 		Boolean active;
 
 		@Embedded(prefix = "user_", onEmpty = Embedded.OnEmpty.USE_NULL) Address address;
+
+		List<Hobby> hobbies;
+		Hobby hated;
 	}
 
-	@Data
 	@AllArgsConstructor
 	static class Address {
 		String street;
 		String city;
 	}
+
+	static class Hobby {
+		String name;
+	}
 }

spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/RelationalQueryCreator.java

@@ -103,8 +103,10 @@ public abstract class RelationalQueryCreator<T> extends AbstractQueryCreator<T,
 
 		Iterable<Part> parts = () -> tree.stream().flatMap(Streamable::stream).iterator();
 		for (Part part : parts) {
+
 			int numberOfArguments = part.getNumberOfArguments();
 			for (int i = 0; i < numberOfArguments; i++) {
+
 				throwExceptionOnArgumentMismatch(part, parameters, argCount);
 				argCount++;
 			}
@@ -117,6 +119,7 @@ public abstract class RelationalQueryCreator<T> extends AbstractQueryCreator<T,
 		String property = part.getProperty().toDotPath();
 
 		if (!parameters.getBindableParameters().hasParameterAt(index)) {
+
 			String msgTemplate = "Query method expects at least %d arguments but only found %d. "
 					+ "This leaves an operator of type %s for property %s unbound.";
 			String formattedMsg = String.format(msgTemplate, index + 1, index, type.name(), property);
@@ -125,9 +128,11 @@ public abstract class RelationalQueryCreator<T> extends AbstractQueryCreator<T,
 
 		Parameter parameter = parameters.getBindableParameter(index);
 		if (expectsCollection(type) && !parameterIsCollectionLike(parameter)) {
+
 			String message = wrongParameterTypeMessage(property, type, "Collection", parameter);
 			throw new IllegalStateException(message);
 		} else if (!expectsCollection(type) && !parameterIsScalarLike(parameter)) {
+
 			String message = wrongParameterTypeMessage(property, type, "scalar", parameter);
 			throw new IllegalStateException(message);
 		}

src/main/asciidoc/jdbc.adoc

@@ -447,7 +447,7 @@ Thus, the method name results in a query expression of `SELECT … FROM person W
 <2> Use `Pageable` to pass offset and sorting parameters to the database.
 <3> Find a single entity for the given criteria.
 It completes with `IncorrectResultSizeDataAccessException` on non-unique results.
-<4> Unless <3>, the first entity is always emitted even if the query yields more result documents.
+<4> In contrast to <3>, the first entity is always emitted even if the query yields more result documents.
 <5> The `findByLastname` method shows a query for all people with the given last name.
 ====
 
@@ -541,7 +541,7 @@ The following table shows the keywords that are supported for query methods:
 | `active IS FALSE`
 |===
 
-NOTE: Query derivation is limited to properties that can be used in a `WHERE` clause without involving joins. 
+NOTE: Query derivation is limited to properties that can be used in a `WHERE` clause without using joins.
 
 [[jdbc.query-methods.strategies]]
 === Query Lookup Strategies
@@ -622,7 +622,7 @@ Iterating happens in the order of registration, so make sure to register more ge
 If applicable, wrapper types such as collections or `Optional` are unwrapped.
 Thus, a return type of `Optional<Person>` uses the `Person` type in the preceding process.
 
-NOTE: Using a custom `RowMapper` through `QueryMappingConfiguration`, `@Query(rowMapperClass=…)`, or a custom `ResultSetExtractor` disables Entity Callbacks and Lifecycle Events as these components are under full control of the result mapping and can issue their own events/callbacks if needed.
+NOTE: Using a custom `RowMapper` through `QueryMappingConfiguration`, `@Query(rowMapperClass=…)`, or a custom `ResultSetExtractor` disables Entity Callbacks and Lifecycle Events as the result mapping can issue its own events/callbacks if needed.
 
 [[jdbc.query-methods.at-query.modifying]]
 ==== Modifying Query