Issue #109: deleteById used the Optional from findById and didn't unwrap the actual value

Author: derjust

src/changes/changes.xml

@@ -16,9 +16,12 @@
       <action dev="derjust" type="add" date="2017-12-17" >
         Update Mockito and resolve dependency clashes
       </action>
-      <action dev="Gaurav Rawat" type="fix" issue="91">>
+      <action dev="Gaurav Rawat" type="fix" issue="91">
         Fixed false assertion introduced implementing #91
       </action>
+      <action dev="derjust" type="fix" issue="109">
+        Fix incorrect passing of an Optional into the delete method
+      </action>
     </release>
     <release version="5.0.0" date="2017-11-27" description="Spring 5 release" >
       <action dev="derjust" type="add">

src/main/java/org/socialsignin/spring/data/dynamodb/repository/support/SimpleDynamoDBCrudRepository.java

@@ -168,11 +168,14 @@ public void deleteById(ID id) {
 		Assert.notNull(id, "The given id must not be null!");
 
 		Optional<T> entity = findById(id);
-		if (!entity.isPresent()) {
+
+		if (entity.isPresent()) {
+			dynamoDBOperations.delete(entity.get());
+
+		} else {
 			throw new EmptyResultDataAccessException(String.format(
 					"No %s entity with id %s exists!", domainType, id), 1);
 		}
-		dynamoDBOperations.delete(entity);
 	}
 
 	@Override

src/test/java/org/socialsignin/spring/data/dynamodb/repository/support/SimpleDynamoDBCrudRepositoryUnitTest.java

@@ -15,9 +15,11 @@
  */
 package org.socialsignin.spring.data.dynamodb.repository.support;
 
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
@@ -28,8 +30,11 @@
 import org.springframework.dao.EmptyResultDataAccessException;
 
 import java.util.Optional;
+import java.util.Random;
 
 import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 
@@ -87,16 +92,32 @@ public void setUp() {
 		when(entityWithCompositeIdInformation.getRangeKey(testPlaylistId)).thenReturn("playlist1");
 		when(entityWithCompositeIdInformation.isRangeKeyAware()).thenReturn(true);
 
-		repoForEntityWithOnlyHashKey = new SimpleDynamoDBCrudRepository<User, Long>(entityWithSimpleIdInformation,
+		repoForEntityWithOnlyHashKey = new SimpleDynamoDBCrudRepository<>(entityWithSimpleIdInformation,
 				dynamoDBOperations,mockEnableScanPermissions);
-		repoForEntityWithHashAndRangeKey = new SimpleDynamoDBCrudRepository<Playlist, PlaylistId>(
+		repoForEntityWithHashAndRangeKey = new SimpleDynamoDBCrudRepository<>(
 				entityWithCompositeIdInformation, dynamoDBOperations,mockEnableScanPermissions);
 
 		when(dynamoDBOperations.load(User.class, 1l)).thenReturn(testUser);
 		when(dynamoDBOperations.load(Playlist.class, "michael", "playlist1")).thenReturn(testPlaylist);
 
 	}
 
+	@Test
+	public void deleteById() {
+		final long id = new Random().nextLong();
+		User testResult = new User();
+		testResult.setId(Long.toString(id));
+
+		when(entityWithSimpleIdInformation.getHashKey(id)).thenReturn(id);
+		when(dynamoDBOperations.load(User.class, id)).thenReturn(testResult);
+
+		repoForEntityWithOnlyHashKey.deleteById(id);
+
+		ArgumentCaptor<User> captor = ArgumentCaptor.forClass(User.class);
+		Mockito.verify(dynamoDBOperations).delete(captor.capture());
+		Assert.assertEquals(Long.toString(id), captor.getValue().getId());
+	}
+
 	/**
 	 * @see DATAJPA-177
 	 */