DATAJPA-1535 - Polishing.

Use isNew(…) check in delete(T) instead of just relying on Id nullability. Formatting.

Original pull request: #378.

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

@@ -172,13 +172,11 @@ public class SimpleJpaRepository<T, ID extends Serializable>
 
 		Assert.notNull(entity, "The entity must not be null!");
 
-		Object id = entityInformation.getId(entity);
-		if (id == null) {
+		if (entityInformation.isNew(entity)) {
 			return;
 		}
 
-		T existing = em.find(entityInformation.getJavaType(), id);
-
+		T existing = em.find(entityInformation.getJavaType(), entityInformation.getId(entity));
 		// if the entity to be deleted doesn't exist, delete is a NOOP
 		if (existing == null) {
 			return;

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

@@ -22,14 +22,16 @@ import static org.hamcrest.Matchers.*;
 import javax.persistence.Query;
 
 import org.junit.Assume;
+import org.junit.Ignore;
 import org.junit.Test;
+
 import org.springframework.data.jpa.repository.sample.UserRepository;
 import org.springframework.data.util.Version;
 import org.springframework.test.context.ContextConfiguration;
 
 /**
  * Testcase to run {@link UserRepository} integration tests on top of EclipseLink.
- * 
+ *
  * @author Oliver Gierke
  * @author Thomas Darimont
  * @author Jens Schauder
@@ -174,12 +176,13 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi
 		super.findByEmptyCollectionOfStrings();
 	}
 
+	/**
+	 * Ignores the test for EclipseLink.
+	 */
 	@Override
 	@Test
 	@Ignore
-	public void savingUserThrowsAnException() {
-		super.savingUserThrowsAnException();
-	}
+	public void savingUserThrowsAnException() {}
 
 	private void assumeNotEclipseLink2_7_2plus() {
 

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

@@ -41,7 +41,7 @@ import org.springframework.test.context.ContextConfiguration;
 
 /**
  * Testcase to run {@link UserRepository} integration tests on top of OpenJPA.
- * 
+ *
  * @author Oliver Gierke
  * @author Jens Schauder
  */
@@ -133,4 +133,12 @@ public class OpenJpaNamespaceUserRepositoryTests extends NamespaceUserRepository
 	 */
 	@Override
 	public void supportsProjectionsWithNativeQueries() {}
+
+	/**
+	 * Ignores the test for OpenJPA.
+	 */
+	@Override
+	@Test
+	@Ignore
+	public void savingUserThrowsAnException() {}
 }

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

@@ -89,7 +89,7 @@ import com.google.common.base.Optional;
  * well as Hibernate configuration to execute tests.
  * <p>
  * To test further persistence providers subclass this class and provide a custom provider configuration.
- * 
+ *
  * @author Oliver Gierke
  * @author Kevin Raymond
  * @author Thomas Darimont
@@ -324,7 +324,7 @@ public class UserRepositoryTests {
 
 	/**
 	 * Tests, that searching by the email address of the reference user returns exactly that instance.
-	 * 
+	 *
 	 * @throws Exception
 	 */
 	@Test
@@ -352,7 +352,7 @@ public class UserRepositoryTests {
 
 	/**
 	 * Tests that all users get deleted by triggering {@link UserRepository#deleteAll()}.
-	 * 
+	 *
 	 * @throws Exception
 	 */
 	@Test
@@ -2257,10 +2257,11 @@ public class UserRepositoryTests {
 		assertThat(map.get(new Object()), is(nullValue()));
 	}
 
-	@Test() // DATAJPA-1535
+	@Test(expected = DataIntegrityViolationException.class) // 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);
+		// if this test fails this means deleteNewInstanceSucceedsByDoingNothing() might actually save the user without the
+		// test failing, which would be a bad thing.
+		repository.save(new User());
 	}
 
 	@Test // DATAJPA-1535

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

@@ -138,6 +138,8 @@ public class SimpleJpaRepositoryUnitTests {
 		User newUser = new User();
 		newUser.setId(null);
 
+		when(information.isNew(newUser)).thenReturn(true);
+
 		repo.delete(newUser);
 
 		verify(em, never()).find(any(Class.class), any(Object.class));
@@ -146,7 +148,7 @@ public class SimpleJpaRepositoryUnitTests {
 	}
 
 	@Test // DATAJPA-1535
-	public void doNothingWhenNonExistantInstanceGetsDeleted() {
+	public void doNothingWhenNonExistentInstanceGetsDeleted() {
 
 		User newUser = new User();
 		newUser.setId(23);