Merge pull request #104 from derjust/issue_68

Issue #68: Batch save should return the failed entities

Author: derjust

src/main/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBOperations.java

@@ -1,5 +1,6 @@
 package org.socialsignin.spring.data.dynamodb.core;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper.FailedBatch;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBQueryExpression;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBScanExpression;
@@ -27,12 +28,10 @@ public interface DynamoDBOperations {
 	Map<String, List<Object>> batchLoad(Map<Class<?>, List<KeyPair>> itemsToGet);
 
 	void save(Object entity);
-	void batchSave(List<?> entities);
-	void batchSave(Iterable<?> entities);
+	List<FailedBatch> batchSave(Iterable<?> entities);
 
 	void delete(Object entity);
-	void batchDelete(List<?> entities);
-    void batchDelete(Iterable<?> entities);
+    List<FailedBatch> batchDelete(Iterable<?> entities);
 
 	<T> String getOverriddenTableName(Class<T> domainClass, String tableName);
 

src/main/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplate.java

@@ -2,6 +2,7 @@
 
 import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper;
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper.FailedBatch;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperConfig;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBQueryExpression;
@@ -25,7 +26,6 @@
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.context.ApplicationEventPublisher;
 
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -99,7 +99,7 @@ public <T> T load(Class<T> domainClass, Object hashKey, Object rangeKey) {
 		T entity =  dynamoDBMapper.load(domainClass, hashKey,rangeKey);
 		if (entity != null)
 		{
-			maybeEmitEvent(new AfterLoadEvent<Object>(entity));
+			maybeEmitEvent(new AfterLoadEvent<>(entity));
 		}
 		return entity;
 	}
@@ -109,7 +109,7 @@ public <T> T load(Class<T> domainClass, Object hashKey) {
 		T entity =  dynamoDBMapper.load(domainClass, hashKey);
 		if (entity != null)
 		{
-			maybeEmitEvent(new AfterLoadEvent<Object>(entity));
+			maybeEmitEvent(new AfterLoadEvent<>(entity));
 		}
 		return entity;
 	}
@@ -137,58 +137,38 @@ public Map<String, List<Object>> batchLoad(Map<Class<?>, List<KeyPair>> itemsToG
 
 	@Override
 	public void save(Object entity) {
-		maybeEmitEvent(new BeforeSaveEvent<Object>(entity));
+		maybeEmitEvent(new BeforeSaveEvent<>(entity));
 		dynamoDBMapper.save(entity);
-		maybeEmitEvent(new AfterSaveEvent<Object>(entity));
+		maybeEmitEvent(new AfterSaveEvent<>(entity));
 
 	}
-
-	@Override
-	@Deprecated
-	public void batchSave(List<?> entities) {
-		Iterable<?> iterableEntities = entities;
-		batchSave(iterableEntities);
-	}
 
 	@Override
-        public void batchSave(Iterable<?> entities) {
-	        Iterator<?> iteratorBefore = entities.iterator();
-	        while( iteratorBefore.hasNext() ){
-	            maybeEmitEvent(new BeforeSaveEvent<Object>(iteratorBefore.next()));
-	        }
-                dynamoDBMapper.batchSave(entities);
-                Iterator<?> iteratorAfter = entities.iterator();
-                while( iteratorAfter.hasNext() ){
-                    maybeEmitEvent(new BeforeSaveEvent<Object>(iteratorAfter.next()));
-                }
-        }
+	public List<FailedBatch> batchSave(Iterable<?> entities) {
+		entities.forEach(it -> maybeEmitEvent(new BeforeSaveEvent<>(it)));
+
+		List<FailedBatch> result = dynamoDBMapper.batchSave(entities);
+
+		entities.forEach(it -> maybeEmitEvent(new AfterSaveEvent<>(it)));
+        return result;
+    }
 
 	@Override
 	public void delete(Object entity) {
-		maybeEmitEvent(new BeforeDeleteEvent<Object>(entity));
+		maybeEmitEvent(new BeforeDeleteEvent<>(entity));
 		dynamoDBMapper.delete(entity);
-		maybeEmitEvent(new AfterDeleteEvent<Object>(entity));
+		maybeEmitEvent(new AfterDeleteEvent<>(entity));
 
 	}
-
-	@Override
-	@Deprecated
-	public void batchDelete(List<?> entities) {
-	        Iterable<?> iterableEntities = entities;
-	        batchDelete(iterableEntities);
-	}
 
 	@Override
-	public void batchDelete(Iterable<?> entities) {
-	    Iterator<?> iteratorBefore = entities.iterator();
-            while( iteratorBefore.hasNext() ){
-                maybeEmitEvent(new BeforeDeleteEvent<Object>(iteratorBefore.next()));
-            }
-	    dynamoDBMapper.batchDelete(entities);
-	    Iterator<?> iteratorAfter = entities.iterator();
-            while( iteratorAfter.hasNext() ){
-                maybeEmitEvent(new AfterDeleteEvent<Object>(iteratorAfter.next()));
-            }
+	public List<FailedBatch> batchDelete(Iterable<?> entities) {
+		entities.forEach(it -> maybeEmitEvent(new BeforeDeleteEvent<>(it)));
+		
+	    List<FailedBatch> result = dynamoDBMapper.batchDelete(entities);
+
+		entities.forEach(it -> maybeEmitEvent(new AfterDeleteEvent<>(it)));
+		return result;
 	}
 
 	@Override

src/main/java/org/socialsignin/spring/data/dynamodb/exception/BatchWriteException.java

@@ -0,0 +1,12 @@
+package org.socialsignin.spring.data.dynamodb.exception;
+
+import org.springframework.dao.DataAccessException;
+
+@SuppressWarnings("serial")
+public class BatchWriteException extends DataAccessException {
+
+	public BatchWriteException(String msg, Throwable cause) {
+		super(msg, cause);
+	}
+
+}

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

@@ -15,17 +15,23 @@
  */
 package org.socialsignin.spring.data.dynamodb.repository.support;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper.FailedBatch;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBScanExpression;
 import com.amazonaws.services.dynamodbv2.datamodeling.KeyPair;
+
+import org.socialsignin.spring.data.dynamodb.exception.BatchWriteException;
 import org.socialsignin.spring.data.dynamodb.core.DynamoDBOperations;
 import org.socialsignin.spring.data.dynamodb.repository.DynamoDBCrudRepository;
+import org.springframework.dao.DataAccessException;
 import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.util.Assert;
 
 import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Queue;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
@@ -34,8 +40,6 @@
  * Default implementation of the
  * {@link org.springframework.data.repository.CrudRepository} interface.
  * 
- * @author Michael Lavelle
- * 
  * @param <T>
  *            the type of the entity to handle
  * @param <ID>
@@ -63,7 +67,6 @@ public SimpleDynamoDBCrudRepository(
 		this.dynamoDBOperations = dynamoDBOperations;
 		this.domainType = entityInformation.getJavaType();
 		this.enableScanPermissions = enableScanPermissions;
-
 	}
 
 	@Override
@@ -119,10 +122,35 @@ public <S extends T> S save(S entity) {
 		return entity;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 * 
+	 * @throws BatchWriteException in case of an error during saving
+	 */
 	@Override
-	public <S extends T> Iterable<S> saveAll(Iterable<S> entities) {
-		dynamoDBOperations.batchSave(entities);
-		return entities;
+	public <S extends T> Iterable<S> saveAll(Iterable<S> entities) throws BatchWriteException, IllegalArgumentException {
+		
+		Assert.notNull(entities, "The given Iterable of entities not be null!");
+		List<FailedBatch> failedBatches = dynamoDBOperations.batchSave(entities);
+		
+		if (failedBatches.isEmpty()) {
+			// Happy path
+			return entities;
+		} else {
+			// Error handling:
+			Queue<Exception> allExceptions = failedBatches.stream()
+					.map(it ->it.getException())
+					.collect(Collectors.toCollection(LinkedList::new));
+
+			// The first exception is hopefully the cause
+			Exception cause = allExceptions.poll();
+			DataAccessException e = new BatchWriteException("Saving of entities failed!", cause);
+			// and all other exceptions are 'just' follow-up exceptions
+			allExceptions.stream()
+				.forEach(e::addSuppressed);
+			
+			throw e;
+		}
 	}
 
 	@Override

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

@@ -15,9 +15,13 @@
  */
 package org.socialsignin.spring.data.dynamodb.repository.support;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper.FailedBatch;
+
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
@@ -27,13 +31,17 @@
 import org.socialsignin.spring.data.dynamodb.domain.sample.Playlist;
 import org.socialsignin.spring.data.dynamodb.domain.sample.PlaylistId;
 import org.socialsignin.spring.data.dynamodb.domain.sample.User;
+import org.socialsignin.spring.data.dynamodb.exception.BatchWriteException;
 import org.springframework.dao.EmptyResultDataAccessException;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Optional;
 import java.util.Random;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyIterable;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -50,6 +58,9 @@ public class SimpleDynamoDBCrudRepositoryUnitTest {
 
 	SimpleDynamoDBCrudRepository<Playlist, PlaylistId> repoForEntityWithHashAndRangeKey;
 
+	@Rule
+	public ExpectedException expectedException = ExpectedException.none();
+
 	@Mock
 	DynamoDBOperations dynamoDBOperations;
 
@@ -153,4 +164,38 @@ public void throwsExceptionIfEntityWithHashAndRangeKeyToDeleteDoesNotExist() {
 
 		repoForEntityWithHashAndRangeKey.deleteById(playlistId);
 	}
+
+	@Test
+	public void testBatchSave() {
+
+		List<User> entities = new ArrayList<>();
+		entities.add(new User());
+		entities.add(new User());
+		when(dynamoDBOperations.batchSave(anyIterable())).thenReturn(Collections.emptyList());
+
+		repoForEntityWithOnlyHashKey.saveAll(entities);
+
+		verify(dynamoDBOperations).batchSave(anyIterable());
+	}
+
+	@Test
+	public void testBatchSaveFailure() {
+		List<FailedBatch> failures = new ArrayList<>();
+		FailedBatch e1 = new FailedBatch();
+		e1.setException(new Exception("First exception"));
+		failures.add(e1);
+		FailedBatch e2 = new FailedBatch();
+		e2.setException(new Exception("Followup exception"));
+		failures.add(e2);
+
+		List<User> entities = new ArrayList<>();
+		entities.add(new User());
+		entities.add(new User());
+		when(dynamoDBOperations.batchSave(anyIterable())).thenReturn(failures);
+
+		expectedException.expect(BatchWriteException.class);
+		expectedException.expectMessage("Saving of entities failed!; nested exception is java.lang.Exception: First exception");
+
+		repoForEntityWithOnlyHashKey.saveAll(entities);
+	}
 }