From 3a24c6d0d4e557933664783a1e6db0bc3ae846b3 Mon Sep 17 00:00:00 2001
From: Oliver Drotbohm <ogierke@pivotal.io>
Date: Tue, 16 Jul 2019 21:09:03 +0200
Subject: [PATCH] =?UTF-8?q?DATAJPA-1535=20-=20Fix=20SpringJpaRepository.de?=
 =?UTF-8?q?lete(=E2=80=A6)=20for=20repositories=20working=20with=20mapped?=
 =?UTF-8?q?=20superclasses.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The new optimized implementation of SimpleJpaRepository.delete(…) used the Java type registered with the EntityInformation backing the repository, not the concrete type of the entity handed into the method. For mapped superclasses that's problematic as the JPA provider does not know which concrete type to actually look for. We now simply use the type of the actual instance to perform the by id lookup.
---
 .../support/SimpleJpaRepository.java          |  7 ++++--
 .../jpa/domain/sample/AbstractMappedType.java |  4 +++-
 .../MappedTypeRepositoryIntegrationTests.java | 22 +++++++++++++++++++
 3 files changed, 30 insertions(+), 3 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 81dd00f91..73483e89e 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
@@ -179,8 +179,9 @@ public class SimpleJpaRepository<T, ID> implements JpaRepositoryImplementation<T
 	 * (non-Javadoc)
 	 * @see org.springframework.data.repository.CrudRepository#delete(java.lang.Object)
 	 */
-	@Transactional
 	@Override
+	@Transactional
+	@SuppressWarnings("unchecked")
 	public void delete(T entity) {
 
 		Assert.notNull(entity, "Entity must not be null!");
@@ -189,11 +190,13 @@ public class SimpleJpaRepository<T, ID> implements JpaRepositoryImplementation<T
 			return;
 		}
 
-		T existing = em.find(entityInformation.getJavaType(), entityInformation.getId(entity));
+		T existing = (T) em.find(entity.getClass(), entityInformation.getId(entity));
+
 		// if the entity to be deleted doesn't exist, delete is a NOOP
 		if (existing == null) {
 			return;
 		}
+
 		em.remove(em.contains(entity) ? entity : em.merge(entity));
 	}
 
diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/AbstractMappedType.java b/src/test/java/org/springframework/data/jpa/domain/sample/AbstractMappedType.java
index cc1aa3e35..6c5b6b566 100644
--- a/src/test/java/org/springframework/data/jpa/domain/sample/AbstractMappedType.java
+++ b/src/test/java/org/springframework/data/jpa/domain/sample/AbstractMappedType.java
@@ -15,6 +15,8 @@
  */
 package org.springframework.data.jpa.domain.sample;
 
+import lombok.Getter;
+
 import javax.persistence.GeneratedValue;
 import javax.persistence.Id;
 import javax.persistence.MappedSuperclass;
@@ -27,7 +29,7 @@ import javax.persistence.Version;
 @MappedSuperclass
 public abstract class AbstractMappedType {
 
-	@Id @GeneratedValue Long id;
+	@Id @GeneratedValue @Getter Long id;
 	@Version Long version;
 	String attribute1;
 
diff --git a/src/test/java/org/springframework/data/jpa/repository/MappedTypeRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/MappedTypeRepositoryIntegrationTests.java
index 1c43a5bd0..79fb5797b 100644
--- a/src/test/java/org/springframework/data/jpa/repository/MappedTypeRepositoryIntegrationTests.java
+++ b/src/test/java/org/springframework/data/jpa/repository/MappedTypeRepositoryIntegrationTests.java
@@ -19,18 +19,23 @@ import static org.assertj.core.api.Assertions.*;
 
 import java.util.List;
 
+import javax.persistence.EntityManager;
+
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Sort;
+import org.springframework.data.jpa.domain.sample.AbstractMappedType;
 import org.springframework.data.jpa.domain.sample.ConcreteType1;
 import org.springframework.data.jpa.domain.sample.ConcreteType2;
 import org.springframework.data.jpa.repository.sample.ConcreteRepository1;
 import org.springframework.data.jpa.repository.sample.ConcreteRepository2;
 import org.springframework.data.jpa.repository.sample.MappedTypeRepository;
 import org.springframework.data.jpa.repository.sample.SampleConfig;
+import org.springframework.data.jpa.repository.support.JpaRepositoryFactory;
+import org.springframework.data.repository.CrudRepository;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import org.springframework.transaction.annotation.Transactional;
@@ -49,6 +54,8 @@ public class MappedTypeRepositoryIntegrationTests {
 	@Autowired ConcreteRepository1 concreteRepository1;
 	@Autowired ConcreteRepository2 concreteRepository2;
 
+	@Autowired EntityManager entityManager;
+
 	@Test // DATAJPA-170
 	public void supportForExpressionBasedQueryMethods() {
 
@@ -73,4 +80,19 @@ public class MappedTypeRepositoryIntegrationTests {
 
 		assertThat(page.getNumberOfElements()).isEqualTo(1);
 	}
+
+	@Test // DATAJPA-1535
+	@SuppressWarnings("unchecked")
+	public void deletesConcreteInstancesForRepositoryBoundToMappedSuperclass() {
+
+		JpaRepositoryFactory factory = new JpaRepositoryFactory(entityManager);
+		CustomMappedTypeRepository<AbstractMappedType> repository = factory.getRepository(CustomMappedTypeRepository.class);
+
+		ConcreteType1 entity = repository.save(new ConcreteType1());
+
+		assertThatCode(() -> repository.delete(entity)).doesNotThrowAnyException();
+		assertThat(concreteRepository1.findById(entity.getId())).isEmpty();
+	}
+
+	private interface CustomMappedTypeRepository<T extends AbstractMappedType> extends CrudRepository<T, Long> {}
 }