From 7d044c22f1c007cbb13caadbd756b1141be959de Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@vmware.com>
Date: Tue, 2 Feb 2021 11:56:02 +0100
Subject: [PATCH] Consider Specification order in composition.

We now consider the ordering of left-hand-side and right-hand-side arguments when composing specifications. The primary aspect is consistency so that predicates appear in the actual SQL query in the order they were combined. Most SQL databases tend to reorder the criteria according to the most useful query execution plan. Only special cases tend to follow deferred evaluation when using OR combination.

Resolves #2146.
---
 .../jpa/domain/SpecificationComposition.java  |  5 +--
 .../jpa/domain/SpecificationUnitTests.java    | 32 +++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/springframework/data/jpa/domain/SpecificationComposition.java b/src/main/java/org/springframework/data/jpa/domain/SpecificationComposition.java
index 48065af38..e2db0ae6d 100644
--- a/src/main/java/org/springframework/data/jpa/domain/SpecificationComposition.java
+++ b/src/main/java/org/springframework/data/jpa/domain/SpecificationComposition.java
@@ -30,6 +30,7 @@ import org.springframework.lang.Nullable;
  * @author Sebastian Staudt
  * @author Oliver Gierke
  * @author Jens Schauder
+ * @author Mark Paluch
  * @see Specification
  * @since 2.2
  */
@@ -44,8 +45,8 @@ class SpecificationComposition {
 
 		return (root, query, builder) -> {
 
-			Predicate otherPredicate = toPredicate(lhs, root, query, builder);
-			Predicate thisPredicate = toPredicate(rhs, root, query, builder);
+			Predicate thisPredicate = toPredicate(lhs, root, query, builder);
+			Predicate otherPredicate = toPredicate(rhs, root, query, builder);
 
 			if (thisPredicate == null) {
 				return otherPredicate;
diff --git a/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java b/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java
index 671889872..773bbc160 100644
--- a/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java
+++ b/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java
@@ -16,6 +16,7 @@
 package org.springframework.data.jpa.domain;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
 import static org.springframework.data.jpa.domain.Specification.*;
 import static org.springframework.data.jpa.domain.Specification.not;
 import static org.springframework.util.SerializationUtils.*;
@@ -40,6 +41,7 @@ import org.mockito.junit.MockitoJUnitRunner;
  * @author Thomas Darimont
  * @author Sebastian Staudt
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 @SuppressWarnings("serial")
 @RunWith(MockitoJUnitRunner.class)
@@ -142,6 +144,36 @@ public class SpecificationUnitTests implements Serializable {
 		assertThat(transferredSpecification).isNotNull();
 	}
 
+	@Test // #2146
+	public void andCombinesSpecificationsInOrder() {
+
+		Predicate firstPredicate = mock(Predicate.class);
+		Predicate secondPredicate = mock(Predicate.class);
+
+		Specification<Object> first = ((root1, query1, criteriaBuilder) -> firstPredicate);
+
+		Specification<Object> second = ((root1, query1, criteriaBuilder) -> secondPredicate);
+
+		first.and(second).toPredicate(root, query, builder);
+
+		verify(builder).and(firstPredicate, secondPredicate);
+	}
+
+	@Test // #2146
+	void orCombinesSpecificationsInOrder() {
+
+		Predicate firstPredicate = mock(Predicate.class);
+		Predicate secondPredicate = mock(Predicate.class);
+
+		Specification<Object> first = ((root1, query1, criteriaBuilder) -> firstPredicate);
+
+		Specification<Object> second = ((root1, query1, criteriaBuilder) -> secondPredicate);
+
+		first.or(second).toPredicate(root, query, builder);
+
+		verify(builder).or(firstPredicate, secondPredicate);
+	}
+
 	public class SerializableSpecification implements Serializable, Specification<Object> {
 
 		@Override