From 031cabf4606eac35580705eac2b90af3cf54dcb9 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 3d323ed3f..dd82eed6e 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.*;
@@ -42,6 +43,7 @@ import org.mockito.quality.Strictness;
  * @author Thomas Darimont
  * @author Sebastian Staudt
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 @SuppressWarnings("serial")
 @ExtendWith(MockitoExtension.class)
@@ -145,6 +147,36 @@ class SpecificationUnitTests implements Serializable {
 		assertThat(transferredSpecification).isNotNull();
 	}
 
+	@Test // #2146
+	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);
+	}
+
 	static class SerializableSpecification implements Serializable, Specification<Object> {
 
 		@Override