From f3b913147aec835ea8ed0b916681f4bfb16d4704 Mon Sep 17 00:00:00 2001
From: Jens Schauder <jschauder@pivotal.io>
Date: Fri, 8 Dec 2017 14:15:02 +0100
Subject: [PATCH] DATAJPA-931 - Avoid unnecessary merging on save.

Checking if entity is already attached to entity manager before calling merge.

Fixed one test that was relying on the implicit flush triggered by the save.

See also: https://vladmihalcea.com/2016/07/19/jpa-persist-and-merge/
---
 .../support/SimpleJpaRepository.java          |  5 +++-
 .../support/AuditingEntityListenerTests.java  |  5 +++-
 .../support/SimpleJpaRepositoryUnitTests.java | 25 +++++++++++++++++++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java
index 63919bd11..6a9fd435e 100644
--- a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java
+++ b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java
@@ -67,6 +67,7 @@ import org.springframework.util.Assert;
  * @author Thomas Darimont
  * @author Mark Paluch
  * @author Christoph Strobl
+ * @author Jens Schauder
  * @param <T> the type of the entity to handle
  * @param <ID> the type of the entity's identifier
  */
@@ -488,9 +489,11 @@ public class SimpleJpaRepository<T, ID> implements JpaRepository<T, ID>, JpaSpec
 		if (entityInformation.isNew(entity)) {
 			em.persist(entity);
 			return entity;
-		} else {
+		} else if (!em.contains(entity)) {
 			return em.merge(entity);
 		}
+
+		return entity;
 	}
 
 	/*
diff --git a/src/test/java/org/springframework/data/jpa/domain/support/AuditingEntityListenerTests.java b/src/test/java/org/springframework/data/jpa/domain/support/AuditingEntityListenerTests.java
index ea07a4689..c660277b1 100644
--- a/src/test/java/org/springframework/data/jpa/domain/support/AuditingEntityListenerTests.java
+++ b/src/test/java/org/springframework/data/jpa/domain/support/AuditingEntityListenerTests.java
@@ -41,6 +41,7 @@ import org.springframework.transaction.annotation.Transactional;
  * Integration test for {@link AuditingEntityListener}.
  *
  * @author Oliver Gierke
+ * @author Jens Schauder
  */
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration("classpath:auditing/auditing-entity-listener.xml")
@@ -89,7 +90,9 @@ public class AuditingEntityListenerTests {
 		role.setName("ADMIN");
 
 		user.addRole(role);
-		repository.save(user);
+
+		repository.saveAndFlush(user);
+
 		role = user.getRoles().iterator().next();
 
 		assertDatesSet(user);
diff --git a/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java
index d15253752..4043943a8 100644
--- a/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java
+++ b/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java
@@ -44,6 +44,7 @@ import org.springframework.data.repository.CrudRepository;
  * @author Oliver Gierke
  * @author Thomas Darimont
  * @author Mark Paluch
+ * @author Jens Schauder
  */
 @RunWith(MockitoJUnitRunner.Silent.class)
 public class SimpleJpaRepositoryUnitTests {
@@ -131,4 +132,28 @@ public class SimpleJpaRepositoryUnitTests {
 
 		verify(em).find(User.class, id, singletonMap(EntityGraphType.LOAD.getKey(), (Object) entityGraph));
 	}
+
+	@Test // DATAJPA-931
+	public void mergeGetsCalledWhenDetached() {
+
+		User detachedUser = new User();
+
+		when(em.contains(detachedUser)).thenReturn(false);
+
+		repo.save(detachedUser);
+
+		verify(em).merge(detachedUser);
+	}
+
+	@Test // DATAJPA-931
+	public void mergeGetsNotCalledWhenAttached() {
+
+		User attachedUser = new User();
+
+		when(em.contains(attachedUser)).thenReturn(true);
+
+		repo.save(attachedUser);
+
+		verify(em, never()).merge(attachedUser);
+	}
 }