From 3a17e8033b182012395707b3bd500d1b43018053 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Tue, 19 Mar 2024 10:03:59 +0800 Subject: [PATCH] Eliminate unnecessary merge() if entity is non-versioned Fix GH-3401 --- .../support/SimpleJpaRepository.java | 8 ++++++- .../support/JpaRepositoryTests.java | 21 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index 31abc19187..67e437917c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -195,7 +195,13 @@ public void delete(T entity) { return; } - entityManager.remove(entityManager.contains(entity) ? entity : entityManager.merge(entity)); + if (entityInformation.getVersionAttribute().isPresent()) { + // call merge() to raise ObjectOptimisticLockingFailureException if entity is stale + entityManager.remove(entityManager.contains(entity) ? entity : entityManager.merge(entity)); + } + else { + entityManager.remove(existing); + } } @Override diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java index cf4878d6fa..a67167833c 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java @@ -16,6 +16,9 @@ package org.springframework.data.jpa.repository.support; import static org.assertj.core.api.Assertions.*; +import static org.mockito.BDDMockito.then; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import jakarta.persistence.EntityManager; import jakarta.persistence.OptimisticLockException; @@ -32,6 +35,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.jpa.domain.sample.PersistableWithIdClass; import org.springframework.data.jpa.domain.sample.PersistableWithIdClassPK; @@ -65,6 +69,8 @@ class JpaRepositoryTests { @PersistenceContext EntityManager em; + private EntityManager spiedEntityManager; + private JpaRepository repository; private CrudRepository idClassRepository; private JpaRepository versionedUserRepository; @@ -72,7 +78,8 @@ class JpaRepositoryTests { @BeforeEach void setUp() { - repository = new JpaRepositoryFactory(em).getRepository(SampleEntityRepository.class); + spiedEntityManager = Mockito.spy(em); + repository = new JpaRepositoryFactory(spiedEntityManager).getRepository(SampleEntityRepository.class); idClassRepository = new JpaRepositoryFactory(em).getRepository(SampleWithIdClassRepository.class); versionedUserRepository = new JpaRepositoryFactory(em).getRepository(VersionedUserRepository.class); jdbcOperations = new NamedParameterJdbcTemplate(dataSource); @@ -219,6 +226,18 @@ void deleteDirtyManagedVersionedEntityShouldRaiseOptimisticLockException() { jdbcOperations.update("delete from VersionedUser", Map.of()); } + @Test //GH-3401 + void deleteNonVersionedEntityShouldNotInvokeMerge() { + SampleEntity entity = new SampleEntity("one", "eins"); + repository.save(entity); + repository.flush(); + em.detach(entity); + + reset(spiedEntityManager); + repository.delete(entity); + then(spiedEntityManager).should(never()).merge(entity); + } + private interface SampleEntityRepository extends JpaRepository { }