From d3e8f3d895d270f0f86ad29d5b3ca75452a243ae Mon Sep 17 00:00:00 2001 From: Sebastian J Date: Sat, 13 Jan 2018 19:32:28 -0500 Subject: [PATCH] Issue #108: Opened constructor and fixed NPE 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. --- .../data/dynamodb/core/DynamoDBTemplate.java | 40 ++++++++++++------- .../core/DynamoDBTemplateUnitTest.java | 26 +++++++++++- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplate.java b/src/main/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplate.java index a629f9dd..fb143c35 100644 --- a/src/main/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplate.java +++ b/src/main/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplate.java @@ -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; @@ -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 diff --git a/src/test/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplateUnitTest.java b/src/test/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplateUnitTest.java index e0f7ab17..1cabfd19 100644 --- a/src/test/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplateUnitTest.java +++ b/src/test/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBTemplateUnitTest.java @@ -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; @@ -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 @@ -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);