Issue 177

pom.xml

@@ -787,6 +787,10 @@
             <name>boothen</name>
             <url>https://github.com/boothen</url>
         </contributor>
+        <contributor>
+            <name>Alex Simkin</name>
+            <url>https://github.com/SimY4</url>
+        </contributor>
     </contributors>
 
 

src/changes/changes.xml

@@ -61,6 +61,9 @@
       <action dev="boothen" issue="15" type="fix" date="2018-06-12">
         Spring Data Rest uses a PersistentEntityResourceAssembler that requires the DynamoDBMappingContext to be exposed as a Spring Bean.
       </action>
+      <action dev="SimY4" issue="177" type="fix" date="2018-07-04">
+        Fixed NPE when deleting nonexistent entity
+      </action>
     </release>
     <release version="5.0.2" date="2018-03-05" description="Maintenance release">
       <action dev="vitolimandibhrata" issue="40" type="add" date="2017-01-07">

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

@@ -0,0 +1,27 @@
+/**
+ * Copyright © 2018 spring-data-dynamodb (https://github.com/derjust/spring-data-dynamodb)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.socialsignin.spring.data.dynamodb.exception;
+
+import org.springframework.dao.DataAccessException;
+
+@SuppressWarnings("serial")
+public class BatchDeleteException extends DataAccessException {
+
+	public BatchDeleteException(String msg, Throwable cause) {
+		super(msg, cause);
+	}
+
+}

src/main/java/org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQuery.java

@@ -15,8 +15,11 @@
  */
 package org.socialsignin.spring.data.dynamodb.repository.query;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper;
 import org.socialsignin.spring.data.dynamodb.core.DynamoDBOperations;
+import org.socialsignin.spring.data.dynamodb.exception.BatchDeleteException;
 import org.socialsignin.spring.data.dynamodb.query.Query;
+import org.socialsignin.spring.data.dynamodb.utils.ExceptionHandler;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.Pageable;
@@ -35,7 +38,7 @@
  * @author Michael Lavelle
  * @author Sebastian Just
  */
-public abstract class AbstractDynamoDBQuery<T, ID> implements RepositoryQuery {
+public abstract class AbstractDynamoDBQuery<T, ID> implements RepositoryQuery, ExceptionHandler {
 
 	protected final DynamoDBOperations dynamoDBOperations;
 	private final DynamoDBQueryMethod<T, ID> method;
@@ -243,10 +246,14 @@ && getResultsRestrictionIfApplicable().intValue() <= results.size())
 	class DeleteExecution implements QueryExecution<T, ID> {
 
 		@Override
-		public Object execute(AbstractDynamoDBQuery<T, ID> dynamoDBQuery, Object[] values) {
-			T entity = dynamoDBQuery.doCreateQueryWithPermissions(values).getSingleResult();
-			dynamoDBOperations.delete(entity);
-			return entity;
+		public Object execute(AbstractDynamoDBQuery<T, ID> dynamoDBQuery, Object[] values) throws BatchDeleteException {
+			List<T> entities = dynamoDBQuery.doCreateQueryWithPermissions(values).getResultList();
+			List<DynamoDBMapper.FailedBatch> failedBatches = dynamoDBOperations.batchDelete(entities);
+			if (failedBatches.isEmpty()) {
+				return entities;
+			} else {
+				throw repackageToException(failedBatches, BatchDeleteException.class);
+			}
 		}
 	}
 

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

@@ -21,17 +21,15 @@
 import org.socialsignin.spring.data.dynamodb.core.DynamoDBOperations;
 import org.socialsignin.spring.data.dynamodb.exception.BatchWriteException;
 import org.socialsignin.spring.data.dynamodb.repository.DynamoDBCrudRepository;
+import org.socialsignin.spring.data.dynamodb.utils.ExceptionHandler;
 import org.socialsignin.spring.data.dynamodb.utils.SortHandler;
-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;
@@ -45,7 +43,11 @@
  * @param <ID>
  *            the type of the entity's identifier
  */
-public class SimpleDynamoDBCrudRepository<T, ID> implements DynamoDBCrudRepository<T, ID>, SortHandler {
+public class SimpleDynamoDBCrudRepository<T, ID>
+		implements
+			DynamoDBCrudRepository<T, ID>,
+			SortHandler,
+			ExceptionHandler {
 
 	protected DynamoDBEntityInformation<T, ID> entityInformation;
 
@@ -132,16 +134,7 @@ public <S extends T> Iterable<S> saveAll(Iterable<S> entities)
 			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;
+			throw repackageToException(failedBatches, BatchWriteException.class);
 		}
 	}
 

src/main/java/org/socialsignin/spring/data/dynamodb/utils/ExceptionHandler.java

@@ -0,0 +1,51 @@
+/**
+ * Copyright © 2018 spring-data-dynamodb (https://github.com/derjust/spring-data-dynamodb)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.socialsignin.spring.data.dynamodb.utils;
+
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper;
+import org.socialsignin.spring.data.dynamodb.exception.BatchWriteException;
+import org.springframework.dao.DataAccessException;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Queue;
+import java.util.stream.Collectors;
+
+public interface ExceptionHandler {
+
+	default <T extends DataAccessException> T repackageToException(List<DynamoDBMapper.FailedBatch> failedBatches,
+			Class<T> targetType) {
+		// 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();
+		try {
+			Constructor<T> ctor = targetType.getConstructor(String.class, Throwable.class);
+			T e = ctor.newInstance("Processing of entities failed!", cause);
+			// and all other exceptions are 'just' follow-up exceptions
+			allExceptions.stream().forEach(e::addSuppressed);
+			return e;
+		} catch (NoSuchMethodException | InstantiationException | IllegalAccessException
+				| InvocationTargetException e) {
+			assert false; // we should never end up here
+			throw new RuntimeException("Could not repackage '" + failedBatches + "' to " + targetType, e);
+		}
+	}
+}

src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/CRUDOperationsIT.java

@@ -16,14 +16,18 @@
 package org.socialsignin.spring.data.dynamodb.domain.sample;
 
 import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
+import com.amazonaws.services.dynamodbv2.document.Expected;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.socialsignin.spring.data.dynamodb.repository.config.EnableDynamoDBRepositories;
 import org.socialsignin.spring.data.dynamodb.utils.DynamoDBLocalResource;
 import org.socialsignin.spring.data.dynamodb.utils.TableCreationListener;
 import org.socialsignin.spring.data.dynamodb.utils.TableCreationListener.DynamoDBCreateTable;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.TestExecutionListeners;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -50,6 +54,9 @@
 @DynamoDBCreateTable(entityClasses = {User.class})
 public class CRUDOperationsIT {
 
+	@Rule
+	public ExpectedException expectedException = ExpectedException.none();
+
 	@Configuration
 	@EnableDynamoDBRepositories(basePackages = "org.socialsignin.spring.data.dynamodb.domain.sample")
 	public static class TestAppConfig {
@@ -157,4 +164,24 @@ public void testDelete() {
 		assertFalse("User should have been deleted!", actualUser.isPresent());
 	}
 
+	@Test
+	public void testDeleteNonExistent() {
+
+		expectedException.expect(EmptyResultDataAccessException.class);
+		// Delete specific
+		userRepository.deleteById("non-existent");
+	}
+
+	@Test
+	public void testDeleteNonExistentIdWithCondition() {
+		// Delete conditional
+		userRepository.deleteByIdAndName("non-existent", "non-existent");
+	}
+
+	@Test
+	public void testDeleteNonExistingGsiWithConditoin() {
+		// Delete via GSI
+		userRepository.deleteByPostCodeAndNumberOfPlaylists("non-existing", 23);
+
+	}
 }

src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/UserRepository.java

@@ -54,4 +54,5 @@ public interface UserRepository extends CrudRepository<User, String> {
 	@EnableScan
 	User findByNameAndLeaveDate(String name, Instant leaveDate);
 
+	void deleteByPostCodeAndNumberOfPlaylists(String postCode, Integer numberOfPlaylists);
 }

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

@@ -265,8 +265,8 @@ public void testBatchSaveFailure() {
 		when(dynamoDBOperations.batchSave(anyIterable())).thenReturn(failures);
 
 		expectedException.expect(BatchWriteException.class);
-		expectedException
-				.expectMessage("Saving of entities failed!; nested exception is java.lang.Exception: First exception");
+		expectedException.expectMessage(
+				"Processing of entities failed!; nested exception is java.lang.Exception: First exception");
 
 		repoForEntityWithOnlyHashKey.saveAll(entities);
 	}

src/test/java/org/socialsignin/spring/data/dynamodb/utils/ExceptionHandlerTest.java

@@ -0,0 +1,59 @@
+/**
+ * Copyright © 2018 spring-data-dynamodb (https://github.com/derjust/spring-data-dynamodb)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.socialsignin.spring.data.dynamodb.utils;
+
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper;
+import org.junit.Test;
+import org.socialsignin.spring.data.dynamodb.exception.BatchWriteException;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class ExceptionHandlerTest {
+
+	private ExceptionHandler underTest = new ExceptionHandler() {
+	};
+
+	@Test
+	public void testEmpty() {
+		underTest.repackageToException(Collections.emptyList(), BatchWriteException.class);
+
+		assertTrue(true);
+	}
+
+	@Test
+	public void testSimple() {
+		List<DynamoDBMapper.FailedBatch> failedBatches = new ArrayList<>();
+		DynamoDBMapper.FailedBatch fb1 = new DynamoDBMapper.FailedBatch();
+		fb1.setException(new Exception("Test Exception"));
+		failedBatches.add(fb1);
+		DynamoDBMapper.FailedBatch fb2 = new DynamoDBMapper.FailedBatch();
+		fb2.setException(new Exception("Followup Exception"));
+		failedBatches.add(fb2);
+
+		BatchWriteException actual = underTest.repackageToException(failedBatches, BatchWriteException.class);
+
+		assertEquals("Processing of entities failed!; nested exception is java.lang.Exception: Test Exception",
+				actual.getMessage());
+
+		assertEquals(1, actual.getSuppressed().length);
+		assertEquals("Followup Exception", actual.getSuppressed()[0].getMessage());
+	}
+}