Skip to content

Commit

Permalink
Issue #108: Opened constructor and fixed NPE
Browse files Browse the repository at this point in the history
In some use cases it is required to pass in all three parameters into
`DynamoDBTemplate` - therefore made the internal constructor public (that
was used by all other constructors anyway)

Also passing in a combination of `null`/non-`null` parameters could lead
to NPEs down the road - ie. by calling `getOverrideTableName`.
The constructor now initalizes all fields with non-`null` objects and does
not relay on AWS constructors to pick some proper default configs.
  • Loading branch information
derjust committed Jan 14, 2018
1 parent fe3b629 commit d3e8f3d
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.util.Assert;

import java.util.List;
import java.util.Map;
Expand All @@ -36,36 +37,45 @@ public class DynamoDBTemplate implements DynamoDBOperations, ApplicationContextA
private final DynamoDBMapperConfig dynamoDBMapperConfig;
private ApplicationEventPublisher eventPublisher;

/** Convenient constructor to use the default {@link DynamoDBMapper#DynamoDBMapper(AmazonDynamoDB)} */
public DynamoDBTemplate(AmazonDynamoDB amazonDynamoDB, DynamoDBMapperConfig dynamoDBMapperConfig)
{
this(amazonDynamoDB, dynamoDBMapperConfig, null);
}

/** Convenient constructor to use the {@link DynamoDBMapperConfig.DEFAULT} */

public DynamoDBTemplate(AmazonDynamoDB amazonDynamoDB, DynamoDBMapper dynamoDBMapper)
{
this(amazonDynamoDB, null, dynamoDBMapper);
}

/** Convenient construcotr to thse the {@link DynamoDBMapperConfig.DEFAULT} and default {@link DynamoDBMapper#DynamoDBMapper(AmazonDynamoDB)} */
public DynamoDBTemplate(AmazonDynamoDB amazonDynamoDB)
{
this(amazonDynamoDB, null, null);
}

DynamoDBTemplate(AmazonDynamoDB amazonDynamoDB, DynamoDBMapperConfig dynamoDBMapperConfig, DynamoDBMapper dynamoDBMapper) {
this.amazonDynamoDB = amazonDynamoDB;
if (dynamoDBMapper == null && dynamoDBMapperConfig == null) {
this.dynamoDBMapperConfig = DynamoDBMapperConfig.DEFAULT;
this.dynamoDBMapper = new DynamoDBMapper(amazonDynamoDB);
// Mapper must be null as it could not have been constructed without a Config
assert dynamoDBMapper == null;
} else {
this.dynamoDBMapperConfig = dynamoDBMapperConfig;
if (dynamoDBMapper == null) {
this.dynamoDBMapper = new DynamoDBMapper(amazonDynamoDB, dynamoDBMapperConfig);
} else {
this.dynamoDBMapper = dynamoDBMapper;
}
}
/** Initializes a new {@code DynamoDBTemplate}.
* The following combinations are valid:
* @param amazonDynamoDB must not be {@code null}
* @param dynamoDBMapperConfig can be {@code null} - {@link DynamoDBMapperConfig.DEFAULT} is used if {@code null} is passed in
* @param dynamoDBMapper can be {@code null} - {@link DynamoDBMapper#DynamoDBMapper(AmazonDynamoDB, DynamoDBMapperConfig)} is used if {@code null} is passed in */
public DynamoDBTemplate(AmazonDynamoDB amazonDynamoDB, DynamoDBMapperConfig dynamoDBMapperConfig, DynamoDBMapper dynamoDBMapper) {
Assert.notNull(amazonDynamoDB, "amazonDynamoDB must not be null!");
this.amazonDynamoDB = amazonDynamoDB;

if (dynamoDBMapperConfig == null) {
this.dynamoDBMapperConfig = DynamoDBMapperConfig.DEFAULT;
} else {
this.dynamoDBMapperConfig = dynamoDBMapperConfig;
}

if (dynamoDBMapper == null) {
this.dynamoDBMapper = new DynamoDBMapper(amazonDynamoDB, dynamoDBMapperConfig);
} else {
this.dynamoDBMapper = dynamoDBMapper;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper;
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperConfig;
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperConfig.TableNameResolver;
import com.amazonaws.services.dynamodbv2.document.DynamoDB;

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.Mock;
import org.mockito.Mockito;
Expand All @@ -17,11 +21,13 @@
import java.util.ArrayList;
import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

@RunWith(MockitoJUnitRunner.class)
public class DynamoDBTemplateUnitTest {

@Rule
public ExpectedException expectedException = ExpectedException.none();
@Mock
private DynamoDBMapper dynamoDBMapper;
@Mock
Expand All @@ -35,9 +41,25 @@ public class DynamoDBTemplateUnitTest {
public void setUp() {
this.dynamoDBTemplate = new DynamoDBTemplate(dynamoDB, dynamoDBMapperConfig, dynamoDBMapper);
}

@Test
public void testConstructorMandatory() {
expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage("must not be null!");
new DynamoDBTemplate(null);
}

@Test
public void testPreconfiguredDynamoDBMapper() {
public void testConstructorOptionalAllNull() {
dynamoDBTemplate = new DynamoDBTemplate(dynamoDB, null, null);

// check that the defaults are properly initialized - #108
String userTableName = dynamoDBTemplate.getOverriddenTableName(User.class, "UserTable");
assertEquals("user", userTableName);
}

@Test
public void testConstructorOptionalPreconfiguredDynamoDBMapper() {
// Introduced constructor via #91 should not fail its assert
DynamoDBTemplate usePreconfiguredDynamoDBMapper = new DynamoDBTemplate(dynamoDB, dynamoDBMapper);

Expand Down

0 comments on commit d3e8f3d

Please sign in to comment.