From 845db0635c11d6389e53dcceac61d350e7e70c94 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm 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 | 5 +++- .../jpa/domain/sample/AbstractMappedType.java | 4 +++- .../MappedTypeRepositoryIntegrationTests.java | 24 ++++++++++++++++++- 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 b74ba6a4c..ccea9c4f1 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 @@ -167,7 +167,9 @@ public class SimpleJpaRepository * (non-Javadoc) * @see org.springframework.data.repository.CrudRepository#delete(java.lang.Object) */ + @Override @Transactional + @SuppressWarnings("unchecked") public void delete(T entity) { Assert.notNull(entity, "The entity must not be null!"); @@ -176,7 +178,8 @@ public class SimpleJpaRepository 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; 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 91753a445..bc2b6bba7 100644 --- a/src/test/java/org/springframework/data/jpa/repository/MappedTypeRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/MappedTypeRepositoryIntegrationTests.java @@ -20,25 +20,30 @@ import static org.junit.Assert.*; 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; /** * Integration tests for {@link MappedTypeRepository}. - * + * * @author Thomas Darimont */ @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(), is(1)); } + + @Test // DATAJPA-1535 + @SuppressWarnings("unchecked") + public void deletesConcreteInstancesForRepositoryBoundToMappedSuperclass() { + + JpaRepositoryFactory factory = new JpaRepositoryFactory(entityManager); + CustomMappedTypeRepository repository = factory.getRepository(CustomMappedTypeRepository.class); + + ConcreteType1 entity = repository.save(new ConcreteType1()); + + repository.delete(entity); + assertThat(concreteRepository1.findOne(entity.getId()), is(nullValue())); + } + + private interface CustomMappedTypeRepository extends CrudRepository {} }