diff --git a/pom.xml b/pom.xml index 92218270..601e696a 100755 --- a/pom.xml +++ b/pom.xml @@ -787,6 +787,10 @@ boothen https://github.com/boothen + + Alex Simkin + https://github.com/SimY4 + diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a89f0851..ee892c69 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -61,6 +61,9 @@ Spring Data Rest uses a PersistentEntityResourceAssembler that requires the DynamoDBMappingContext to be exposed as a Spring Bean. + + Fixed NPE when deleting nonexistent entity + diff --git a/src/main/java/org/socialsignin/spring/data/dynamodb/exception/BatchDeleteException.java b/src/main/java/org/socialsignin/spring/data/dynamodb/exception/BatchDeleteException.java new file mode 100644 index 00000000..6006b1fb --- /dev/null +++ b/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); + } + +} diff --git a/src/main/java/org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQuery.java b/src/main/java/org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQuery.java index 7e171330..25e80f8f 100644 --- a/src/main/java/org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQuery.java +++ b/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 implements RepositoryQuery { +public abstract class AbstractDynamoDBQuery implements RepositoryQuery, ExceptionHandler { protected final DynamoDBOperations dynamoDBOperations; private final DynamoDBQueryMethod method; @@ -243,10 +246,14 @@ && getResultsRestrictionIfApplicable().intValue() <= results.size()) class DeleteExecution implements QueryExecution { @Override - public Object execute(AbstractDynamoDBQuery dynamoDBQuery, Object[] values) { - T entity = dynamoDBQuery.doCreateQueryWithPermissions(values).getSingleResult(); - dynamoDBOperations.delete(entity); - return entity; + public Object execute(AbstractDynamoDBQuery dynamoDBQuery, Object[] values) throws BatchDeleteException { + List entities = dynamoDBQuery.doCreateQueryWithPermissions(values).getResultList(); + List failedBatches = dynamoDBOperations.batchDelete(entities); + if (failedBatches.isEmpty()) { + return entities; + } else { + throw repackageToException(failedBatches, BatchDeleteException.class); + } } } diff --git a/src/main/java/org/socialsignin/spring/data/dynamodb/repository/support/SimpleDynamoDBCrudRepository.java b/src/main/java/org/socialsignin/spring/data/dynamodb/repository/support/SimpleDynamoDBCrudRepository.java index a15c01b8..e5ce5b46 100644 --- a/src/main/java/org/socialsignin/spring/data/dynamodb/repository/support/SimpleDynamoDBCrudRepository.java +++ b/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 * the type of the entity's identifier */ -public class SimpleDynamoDBCrudRepository implements DynamoDBCrudRepository, SortHandler { +public class SimpleDynamoDBCrudRepository + implements + DynamoDBCrudRepository, + SortHandler, + ExceptionHandler { protected DynamoDBEntityInformation entityInformation; @@ -132,16 +134,7 @@ public Iterable saveAll(Iterable entities) return entities; } else { // Error handling: - Queue 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); } } diff --git a/src/main/java/org/socialsignin/spring/data/dynamodb/utils/ExceptionHandler.java b/src/main/java/org/socialsignin/spring/data/dynamodb/utils/ExceptionHandler.java new file mode 100644 index 00000000..9fe6ae88 --- /dev/null +++ b/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 repackageToException(List failedBatches, + Class targetType) { + // Error handling: + Queue 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 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); + } + } +} diff --git a/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/CRUDOperationsIT.java b/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/CRUDOperationsIT.java index a68c8044..8f28e010 100644 --- a/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/CRUDOperationsIT.java +++ b/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/CRUDOperationsIT.java @@ -16,7 +16,10 @@ 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; @@ -24,6 +27,7 @@ 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); + + } } diff --git a/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/UserRepository.java b/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/UserRepository.java index c0b1876c..576f5e79 100644 --- a/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/UserRepository.java +++ b/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/UserRepository.java @@ -54,4 +54,5 @@ public interface UserRepository extends CrudRepository { @EnableScan User findByNameAndLeaveDate(String name, Instant leaveDate); + void deleteByPostCodeAndNumberOfPlaylists(String postCode, Integer numberOfPlaylists); } diff --git a/src/test/java/org/socialsignin/spring/data/dynamodb/repository/support/SimpleDynamoDBCrudRepositoryTest.java b/src/test/java/org/socialsignin/spring/data/dynamodb/repository/support/SimpleDynamoDBCrudRepositoryTest.java index 4c507a47..6ea18ad1 100644 --- a/src/test/java/org/socialsignin/spring/data/dynamodb/repository/support/SimpleDynamoDBCrudRepositoryTest.java +++ b/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); } diff --git a/src/test/java/org/socialsignin/spring/data/dynamodb/utils/ExceptionHandlerTest.java b/src/test/java/org/socialsignin/spring/data/dynamodb/utils/ExceptionHandlerTest.java new file mode 100644 index 00000000..287ba8b4 --- /dev/null +++ b/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 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()); + } +}