DATAJPA-1535 - When deleting an entity we no longer merge it if it doesn't exist in the database.

The merge happened to allow deleting of unmanaged entities.
As a side effect it triggered an insert for new (and thereby also unmanaged) entities.
By checking if the entity exists we avoid this now.

The check should NOT trigger an additional select since the select would have happened in the past anyway.

Original pull request: #378.

src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java

@@ -168,6 +168,19 @@ public class SimpleJpaRepository<T, ID> implements JpaRepositoryImplementation<T
 	public void delete(T entity) {
 
 		Assert.notNull(entity, "The entity must not be null!");
+
+		Object id = entityInformation.getId(entity);
+		if (id == null) {
+			return;
+		}
+
+		T existing = em.find(entityInformation.getJavaType(), id);
+
+		// 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));
 	}
 

src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java

@@ -174,6 +174,13 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi
 		super.findByEmptyCollectionOfStrings();
 	}
 
+	@Override
+	@Test
+	@Ignore
+	public void savingUserThrowsAnException() {
+		super.savingUserThrowsAnException();
+	}
+
 	private void assumeNotEclipseLink2_7_2plus() {
 
 		Assume.assumeFalse("Empty collections seem to be broken in EclipseLink 2.7.2+",

src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java

@@ -48,12 +48,11 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.dao.DataAccessException;
+import org.springframework.dao.DataIntegrityViolationException;
 import org.springframework.dao.IncorrectResultSizeDataAccessException;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.domain.Example;
 import org.springframework.data.domain.ExampleMatcher;
-import org.springframework.data.domain.ExampleMatcher.GenericPropertyMatcher;
-import org.springframework.data.domain.ExampleMatcher.StringMatcher;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.PageRequest;
@@ -62,6 +61,7 @@ import org.springframework.data.domain.Slice;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
 import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.domain.ExampleMatcher.*;
 import org.springframework.data.jpa.domain.Specification;
 import org.springframework.data.jpa.domain.sample.Address;
 import org.springframework.data.jpa.domain.sample.Role;
@@ -2211,6 +2211,17 @@ public class UserRepositoryTests {
 		assertThat(repository.findByEmailNativeAddressJdbcStyleParameter("gierke@synyx.de")).isEqualTo(firstUser);
 	}
 
+	@Test() // DATAJPA-1535
+	public void savingUserThrowsAnException() {
+		// if this test fails this means deleteNewInstanceSucceedsByDoingNothing() might actually save the user without the test failing, which would be a bad thing.
+		assertThatThrownBy(() -> repository.save(new User())).isInstanceOf(DataIntegrityViolationException.class);
+	}
+
+	@Test // DATAJPA-1535
+	public void deleteNewInstanceSucceedsByDoingNothing() {
+		repository.delete(new User());
+	}
+
 	private Page<User> executeSpecWithSort(Sort sort) {
 
 		flushTestUsers();

src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java

@@ -156,4 +156,33 @@ public class SimpleJpaRepositoryUnitTests {
 
 		verify(em).merge(attachedUser);
 	}
+
+	@Test // DATAJPA-1535
+	public void doNothingWhenNewInstanceGetsDeleted() {
+
+		User newUser = new User();
+		newUser.setId(null);
+
+		repo.delete(newUser);
+
+		verify(em, never()).find(any(Class.class), any(Object.class));
+		verify(em, never()).remove(newUser);
+		verify(em, never()).merge(newUser);
+	}
+
+	@Test // DATAJPA-1535
+	public void doNothingWhenNonExistantInstanceGetsDeleted() {
+
+		User newUser = new User();
+		newUser.setId(23);
+
+		when(information.isNew(newUser)).thenReturn(false);
+		when(em.find(User.class,23)).thenReturn(null);
+
+		repo.delete(newUser);
+
+		verify(em, never()).remove(newUser);
+		verify(em, never()).merge(newUser);
+	}
+
 }