From 1f677bd166e5649995ea56b7c610020d829909eb 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     | 24 ++++++++++++--
 2 files changed, 51 insertions(+), 4 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 5ebcac88d..0fd17468f 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
@@ -18,6 +18,7 @@ package org.springframework.data.jpa.repository.query;
 import java.util.Collection;
 import java.util.Collections;
 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.query.ParametersParameterAccessor;
 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
@@ -54,8 +56,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;
 	}
@@ -325,4 +329,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 dc6864124..ab9ee8c69 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
@@ -17,10 +17,11 @@ package org.springframework.data.jpa.repository.query;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
-import static org.mockito.Matchers.*;
+import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.*;
 
 import java.util.Arrays;
+import java.util.Optional;
 
 import javax.persistence.EntityManager;
 import javax.persistence.Query;
@@ -147,15 +148,32 @@ public class JpaQueryExecutionUnitTests {
 		verify(jpaQuery, times(0)).createQuery((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;
 		}
 	}