Skip to content

Commit 5a7ef94

Browse files
committed
Properly handle null IDs
`null` arguments passed into the `find`-methods should not cause a NPE down the road but should be actively rejected. Solves #51
1 parent 8c3e139 commit 5a7ef94

File tree

2 files changed

+23
-40
lines changed

2 files changed

+23
-40
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import java.util.List;
3030
import java.util.Map;
3131

32-
public class DynamoDBTemplate implements DynamoDBOperations,ApplicationContextAware {
32+
public class DynamoDBTemplate implements DynamoDBOperations, ApplicationContextAware {
3333

3434
private final DynamoDBMapper dynamoDBMapper;
3535
private final AmazonDynamoDB amazonDynamoDB;

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

Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@
2222
import org.springframework.dao.EmptyResultDataAccessException;
2323
import org.springframework.util.Assert;
2424

25-
import java.util.ArrayList;
26-
import java.util.HashMap;
25+
import java.util.Collections;
2726
import java.util.List;
2827
import java.util.Map;
2928
import java.util.Optional;
29+
import java.util.concurrent.atomic.AtomicInteger;
30+
import java.util.stream.Collectors;
31+
import java.util.stream.StreamSupport;
3032

3133
/**
3234
* Default implementation of the
@@ -66,6 +68,9 @@ public SimpleDynamoDBCrudRepository(
6668

6769
@Override
6870
public Optional<T> findById(ID id) {
71+
72+
Assert.notNull(id, "The given id must not be null!");
73+
6974
T result;
7075
if (entityInformation.isRangeKeyAware()) {
7176
result = dynamoDBOperations.load(domainType,
@@ -84,51 +89,29 @@ public Optional<T> findById(ID id) {
8489

8590
@SuppressWarnings("unchecked")
8691
public List<T> findAllById(Iterable<ID> ids) {
87-
Map<Class<?>, List<KeyPair>> keyPairsMap = new HashMap<>();
88-
List<KeyPair> keyPairs = new ArrayList<>();
89-
for (ID id : ids) {
90-
if (entityInformation.isRangeKeyAware()) {
91-
keyPairs.add(new KeyPair().withHashKey(
92-
entityInformation.getHashKey(id)).withRangeKey(
93-
entityInformation.getRangeKey(id)));
94-
} else {
95-
keyPairs.add(new KeyPair().withHashKey(id));
96-
}
97-
}
98-
keyPairsMap.put(domainType, keyPairs);
99-
return (List<T>) dynamoDBOperations.batchLoad(keyPairsMap).get(dynamoDBOperations.getOverriddenTableName(domainType, entityInformation.getDynamoDBTableName()));
100-
}
10192

102-
protected T load(ID id) {
103-
if (entityInformation.isRangeKeyAware()) {
104-
return dynamoDBOperations.load(domainType,
105-
entityInformation.getHashKey(id),
106-
entityInformation.getRangeKey(id));
107-
} else {
108-
return dynamoDBOperations.load(domainType,
109-
entityInformation.getHashKey(id));
110-
}
111-
}
93+
Assert.notNull(ids, "The given ids must not be null!");
94+
95+
// Works only with non-parallel streams!
96+
AtomicInteger idx = new AtomicInteger();
97+
List<KeyPair> keyPairs = StreamSupport.stream(ids.spliterator(), false).map(id -> {
98+
99+
Assert.notNull(id, "The given id at position " + idx.getAndIncrement() + " must not be null!");
112100

113-
@SuppressWarnings("unchecked")
114-
protected List<T> loadBatch(Iterable<ID> ids) {
115-
Map<Class<?>, List<KeyPair>> keyPairsMap = new HashMap<Class<?>, List<KeyPair>>();
116-
List<KeyPair> keyPairs = new ArrayList<KeyPair>();
117-
for (ID id : ids) {
118101
if (entityInformation.isRangeKeyAware()) {
119-
keyPairs.add(new KeyPair().withHashKey(
102+
return new KeyPair().withHashKey(
120103
entityInformation.getHashKey(id)).withRangeKey(
121-
entityInformation.getRangeKey(id)));
104+
entityInformation.getRangeKey(id));
122105
} else {
123-
keyPairs.add(new KeyPair().withHashKey(id));
124-
106+
return new KeyPair().withHashKey(id);
125107
}
126-
}
127-
keyPairsMap.put(domainType, keyPairs);
128-
return (List<T>) dynamoDBOperations.batchLoad(keyPairsMap).get(domainType);
108+
}).collect(Collectors.toList());
109+
110+
Map<Class<?>, List<KeyPair>> keyPairsMap = Collections.singletonMap(domainType, keyPairs);
111+
return (List<T>)dynamoDBOperations.batchLoad(keyPairsMap)
112+
.get(dynamoDBOperations.getOverriddenTableName(domainType, entityInformation.getDynamoDBTableName()));
129113
}
130114

131-
132115
@Override
133116
public <S extends T> S save(S entity) {
134117

0 commit comments

Comments
 (0)