From d75019bae975a16bed76b3dbfa68325e7d39814d Mon Sep 17 00:00:00 2001
From: Oliver Gierke <ogierke@pivotal.io>
Date: Wed, 17 Aug 2016 16:30:36 +0200
Subject: [PATCH] DATAJPA-951 - Prevent preemptive conversion to Optional in
 JpaQueryExecution.

JpaQueryExecution applies a ConversionService to convert between low-level types such as integers, longs and byte arrays. That ConversionService also preemptively converts objects into JDK 8's Optional in case the invoked method's return type is Optional.

We now explicitly remove the converter taking care of the latter to avoid Optionals to be created before the actual result conversion is applied, as it needs to see the raw value to create DTOs or interface based projections correctly.
---
 .../repository/query/JpaQueryExecution.java   | 31 ++++++++++++++++++-
 .../query/JpaQueryExecutionUnitTests.java     | 22 +++++++++++--
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java
index ada670440..6a5833df3 100644
--- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java
+++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java
@@ -17,6 +17,7 @@ package org.springframework.data.jpa.repository.query;
 
 import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
 
 import javax.persistence.EntityManager;
 import javax.persistence.NoResultException;
@@ -38,6 +39,7 @@ import org.springframework.data.repository.support.PageableExecutionUtils.TotalS
 import org.springframework.data.util.CloseableIterator;
 import org.springframework.data.util.StreamUtils;
 import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 
 /**
  * Set of classes to contain query execution strategies. Depending (mostly) on the return type of a
@@ -55,8 +57,10 @@ public abstract class JpaQueryExecution {
 	static {
 
 		ConfigurableConversionService conversionService = new DefaultConversionService();
-		conversionService.removeConvertible(Collection.class, Object.class);
+
 		conversionService.addConverter(JpaResultConverters.BlobToByteArrayConverter.INSTANCE);
+		conversionService.removeConvertible(Collection.class, Object.class);
+		potentiallyRemoveOptionalConverter(conversionService);
 
 		CONVERSION_SERVICE = conversionService;
 	}
@@ -319,4 +323,29 @@ public abstract class JpaQueryExecution {
 			return StreamUtils.createStreamFromIterator(iter);
 		}
 	}
+
+	/**
+	 * Removes the converter being able to convert any object into an {@link Optional} from the given
+	 * {@link ConversionService} in case we're running on Java 8.
+	 * 
+	 * @param conversionService must not be {@literal null}.
+	 */
+	public static void potentiallyRemoveOptionalConverter(ConfigurableConversionService conversionService) {
+
+		ClassLoader classLoader = JpaQueryExecution.class.getClassLoader();
+
+		if (ClassUtils.isPresent("java.util.Optional", classLoader)) {
+
+			try {
+
+				Class<?> optionalType = ClassUtils.forName("java.util.Optional", classLoader);
+				conversionService.removeConvertible(Object.class, optionalType);
+
+			} catch (ClassNotFoundException e) {
+				return;
+			} catch (LinkageError e) {
+				return;
+			}
+		}
+	}
 }
diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryExecutionUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryExecutionUnitTests.java
index 25f3da18e..44f71d31d 100644
--- a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryExecutionUnitTests.java
+++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryExecutionUnitTests.java
@@ -22,6 +22,7 @@ import static org.mockito.Mockito.*;
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Optional;
 
 import javax.persistence.EntityManager;
 import javax.persistence.Query;
@@ -220,15 +221,32 @@ public class JpaQueryExecutionUnitTests {
 		verify(jpaQuery).createCountQuery((Object[]) any());
 	}
 
-	public static void sampleMethod(Pageable pageable) {
+	/**
+	 * @see DATAJPA-951
+	 */
+	@Test
+	public void doesNotPreemtivelyWrapResultIntoOptional() throws Exception {
 
+		doReturn(method).when(jpaQuery).getQueryMethod();
+		doReturn(Optional.class).when(method).getReturnType();
+
+		StubQueryExecution execution = new StubQueryExecution() {
+			protected Object doExecute(AbstractJpaQuery query, Object[] values) {
+				return "result";
+			}
+		};
+
+		Object result = execution.execute(jpaQuery, new Object[0]);
+
+		assertThat(result, is(instanceOf(String.class)));
 	}
 
+	public static void sampleMethod(Pageable pageable) {}
+
 	static class StubQueryExecution extends JpaQueryExecution {
 
 		@Override
 		protected Object doExecute(AbstractJpaQuery query, Object[] values) {
-
 			return null;
 		}
 	}