From eeeb1881170a0c9987e6d7cd0968c49a91e56ed2 Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
Date: Tue, 9 Jun 2020 15:31:14 +0200
Subject: [PATCH] DATAJDBC-560 - Fix Criteria mapping when composing a group
 from top-level criteria.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Using Criteria.from(…) with multiple Criteria objects now uses properly AND combination along with group nesting to render a correct criteria. Previously, the INITIAL combinator in groups caused a mapping exception.
---
 .../data/jdbc/repository/query/QueryMapper.java    | 10 ++++++----
 .../repository/query/QueryMapperUnitTests.java     | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java
index b56b3bb1f..cd27c1fa3 100644
--- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java
+++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java
@@ -191,7 +191,7 @@ class QueryMapper {
 
 			Condition condition = getCondition(criterion, parameterSource, table, entity);
 			if (condition != null) {
-				result = combine(criterion, mapped, criterion.getCombinator(), condition);
+				result = combine(mapped, criterion.getCombinator(), condition);
 			}
 
 			if (result != null) {
@@ -221,7 +221,7 @@ class QueryMapper {
 
 			Condition condition = unroll(criterion, table, entity, parameterSource);
 
-			mapped = combine(criterion, mapped, combinator, condition);
+			mapped = combine(mapped, combinator, condition);
 		}
 
 		return mapped;
@@ -245,17 +245,19 @@ class QueryMapper {
 		return mapCondition(criteria, parameterSource, table, entity);
 	}
 
-	private Condition combine(CriteriaDefinition criteria, @Nullable Condition currentCondition,
+	private Condition combine(@Nullable Condition currentCondition,
 			CriteriaDefinition.Combinator combinator, Condition nextCondition) {
 
 		if (currentCondition == null) {
 			currentCondition = nextCondition;
+		} else if (combinator == CriteriaDefinition.Combinator.INITIAL) {
+			currentCondition = currentCondition.and(Conditions.nest(nextCondition));
 		} else if (combinator == CriteriaDefinition.Combinator.AND) {
 			currentCondition = currentCondition.and(nextCondition);
 		} else if (combinator == CriteriaDefinition.Combinator.OR) {
 			currentCondition = currentCondition.or(nextCondition);
 		} else {
-			throw new IllegalStateException("Combinator " + criteria.getCombinator() + " not supported");
+			throw new IllegalStateException("Combinator " + combinator + " not supported");
 		}
 
 		return currentCondition;
diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/QueryMapperUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/QueryMapperUnitTests.java
index 658e55dc6..3a55bd138 100644
--- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/QueryMapperUnitTests.java
+++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/QueryMapperUnitTests.java
@@ -126,6 +126,20 @@ public class QueryMapperUnitTests {
 				.hasToString("person.\"NAME\" = ?[:name] AND (person.\"NAME\" = ?[:name1] OR person.age < ?[:age])");
 	}
 
+	@Test // DATAJDBC-560
+	public void shouldMapFromConcat() {
+
+		Criteria criteria = Criteria.from(Criteria.where("name").is("Foo"), Criteria.where("name").is("Bar") //
+				.or("age").lessThan(49));
+
+		assertThat(criteria.isEmpty()).isFalse();
+
+		Condition condition = map(criteria);
+
+		assertThat(condition)
+				.hasToString("(person.\"NAME\" = ?[:name] AND (person.\"NAME\" = ?[:name1] OR person.age < ?[:age]))");
+	}
+
 	@Test // DATAJDBC-318
 	public void shouldMapSimpleCriteria() {