Merge pull request #120 from derjust/backport_issue_62

Backport issue #62
derjust · Jan 23, 2018 · a8e930f · a8e930f
2 parents a17a3f8 + e7f5773
commit a8e930f
Show file tree

Hide file tree

Showing 11 changed files with 94 additions and 36 deletions.
diff --git a/pom.xml b/pom.xml
@@ -517,6 +517,10 @@
             <name>Vito Limandibhrata</name>
             <url>https://github.com/vitolimandibhrata</url>
         </contributor>
+        <contributor>
+            <name>Alex Arana</name>
+            <url>https://github.com/alexarana</url>
+        </contributor>
     </contributors>
 
 

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -27,15 +27,23 @@
       <action dev="derjust" type="add" date="2017-12-17" >
         Update Mockito and resolve dependency clashes
       </action>
-      <action dev="Gaurav Rawat" type="fix" issue="91">>
+      <action dev="Gaurav Rawat" type="fix" issue="91">
         Fixed false assertion introduced implementing #91
       </action>
+      <action dev="Alex Arana" type="add" issue="62">
+        Support @DynamoDBTypeConverted instead of/in addition to @DynamoDBMarshalling for query methods
+      </action>
     </release>
     <release version="5.0.0" date="2017-11-27" description="Spring 5 release" >
       <action dev="derjust" type="add">
         Added Spring 5 / Spring-Data Kay support
       </action>
     </release>
+    <release version="4.5.4" date="2017-01-23" description="Backport of PR #79">
+      <action dev="Alex Arana" type="add" issue="62">
+        Support @DynamoDBTypeConverted instead of/in addition to @DynamoDBMarshalling for query methods
+      </action>
+    </release>
     <release version="4.5.3" date="2017-01-14" description="Backport of PR #108">
       <action dev="derjust" issue="108" type="fix" date="2018-01-14">
         Opened constructor and fixed NPE in case of missing DynamoDBConfig

diff --git a/src/main/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBOperations.java b/src/main/java/org/socialsignin/spring/data/dynamodb/core/DynamoDBOperations.java
@@ -3,6 +3,7 @@
 import java.util.List;
 import java.util.Map;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBQueryExpression;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBScanExpression;
 import com.amazonaws.services.dynamodbv2.datamodeling.KeyPair;
@@ -35,5 +36,11 @@ public interface DynamoDBOperations {
 
 	public <T> String getOverriddenTableName(Class<T> domainClass, String tableName);
 
-
+    /**
+     * Provides access to the DynamoDB mapper table model of the underlying domain type.
+     *
+     * @param domainClass A domain type
+     * @return Corresponding DynamoDB table model
+     */
+    <T> DynamoDBMapperTableModel<T> getTableModel(Class<T> domainClass);
 }
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
@@ -21,6 +21,7 @@
 import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperConfig;
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBQueryExpression;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBScanExpression;
 import com.amazonaws.services.dynamodbv2.datamodeling.KeyPair;
@@ -239,7 +240,15 @@ public <T> String getOverriddenTableName(Class<T> domainClass, String tableName)
 
 		return tableName;
 	}
-
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public <T> DynamoDBMapperTableModel<T> getTableModel(Class<T> domainClass) {
+        return dynamoDBMapper.getTableModel(domainClass, dynamoDBMapperConfig);
+    }
+
 	protected <T> void maybeEmitEvent(DynamoDBMappingEvent<T> event) {
 		if (null != eventPublisher) {
 			eventPublisher.publishEvent(event);

diff --git a/.../org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQueryCreator.java b/.../org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQueryCreator.java
@@ -33,6 +33,7 @@
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel;
 import com.amazonaws.services.dynamodbv2.model.ComparisonOperator;
 
 /**
@@ -60,10 +61,10 @@ public AbstractDynamoDBQueryCreator(PartTree tree, ParameterAccessor parameterAc
 
 	@Override
 	protected DynamoDBQueryCriteria<T, ID> create(Part part, Iterator<Object> iterator) {
-
+        final DynamoDBMapperTableModel<T> tableModel = dynamoDBOperations.getTableModel(entityMetadata.getJavaType());
 		DynamoDBQueryCriteria<T, ID> criteria = entityMetadata.isRangeKeyAware() ? new DynamoDBEntityWithHashAndRangeKeyCriteria<T, ID>(
-				(DynamoDBIdIsHashAndRangeKeyEntityInformation<T, ID>) entityMetadata)
-				: new DynamoDBEntityWithHashKeyOnlyCriteria<T, ID>(entityMetadata);
+				(DynamoDBIdIsHashAndRangeKeyEntityInformation<T, ID>) entityMetadata, tableModel)
+				: new DynamoDBEntityWithHashKeyOnlyCriteria<T, ID>(entityMetadata, tableModel);
 		return addCriteria(criteria, part, iterator);
 	}
 

diff --git a/...org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQueryCriteria.java b/...org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQueryCriteria.java
@@ -40,6 +40,8 @@
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperFieldModel;
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMarshaller;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBQueryExpression;
 import com.amazonaws.services.dynamodbv2.model.AttributeValue;
@@ -56,6 +58,7 @@ public abstract class AbstractDynamoDBQueryCriteria<T, ID extends Serializable>
 	protected Class<T> clazz;
 	private DynamoDBEntityInformation<T, ID> entityInformation;
 	private Map<String, String> attributeNamesByPropertyName;
+	private final DynamoDBMapperTableModel<T> tableModel;
 	private String hashKeyPropertyName;
 
 	protected MultiValueMap<String, Condition> attributeConditions;
@@ -211,14 +214,15 @@ protected List<Condition> getHashKeyConditions() {
 		return hashKeyConditions;
 	}
 
-	public AbstractDynamoDBQueryCriteria(DynamoDBEntityInformation<T, ID> dynamoDBEntityInformation) {
+	public AbstractDynamoDBQueryCriteria(DynamoDBEntityInformation<T, ID> dynamoDBEntityInformation, final DynamoDBMapperTableModel<T> tableModel) {
 		this.clazz = dynamoDBEntityInformation.getJavaType();
 		this.attributeConditions = new LinkedMultiValueMap<String, Condition>();
 		this.propertyConditions = new LinkedMultiValueMap<String, Condition>();
 		this.hashKeyPropertyName = dynamoDBEntityInformation.getHashKeyPropertyName();
 		this.entityInformation = dynamoDBEntityInformation;
 		this.attributeNamesByPropertyName = new HashMap<String, String>();
-
+		// TODO consider adding the DynamoDBMapper table model to DynamoDBEntityInformation instead
+		this.tableModel = tableModel;
 	}
 
 	private String getFirstDeclaredIndexNameForAttribute(Map<String,String[]> indexNamesByAttributeName,List<String> indexNamesToCheck,String attributeName)
@@ -479,16 +483,22 @@ public DynamoDBQueryCriteria<T, ID> withCondition(String propertyName, Condition
 		return this;
 	}
 
-	@SuppressWarnings("unchecked")
-	protected <V> Object getPropertyAttributeValue(String propertyName, Object value) {
-		DynamoDBMarshaller<V> marshaller = (DynamoDBMarshaller<V>) entityInformation.getMarshallerForProperty(propertyName);
+    @SuppressWarnings({"deprecation", "unchecked"})
+    protected <V extends Object> Object getPropertyAttributeValue(final String propertyName, final V value) {
+        // TODO consider removing DynamoDBMarshaller code altogether as table model will handle accordingly
+        final DynamoDBMarshaller<V> marshaller = (DynamoDBMarshaller<V>) entityInformation.getMarshallerForProperty(propertyName);
 
-		if (marshaller != null) {
-			return marshaller.marshall((V) value);
-		} else {
-			return value;
-		}
-	}
+        if (marshaller != null) {
+            return marshaller.marshall(value);
+        } else if (tableModel != null) {  // purely here for testing as DynamoDBMapperTableModel cannot be mocked using Mockito
+            DynamoDBMapperFieldModel<T,Object> fieldModel = tableModel.field(propertyName);
+            if (fieldModel != null) {
+                return fieldModel.convert(value);
+            }
+        }
+
+        return value;
+    }
 
 	protected <V> Condition createNoValueCondition(String propertyName, ComparisonOperator comparisonOperator) {
 
@@ -626,16 +636,19 @@ protected Condition createSingleValueCondition(String propertyName, ComparisonOp
 		Assert.notNull(o, "Creating conditions on null property values not supported: please specify a value for '"
 				+ propertyName + "'");
 
+		List<AttributeValue> attributeValueList = new ArrayList<AttributeValue>();
 		Object attributeValue = !alreadyMarshalledIfRequired ? getPropertyAttributeValue(propertyName, o) : o;
+		if (ClassUtils.isAssignableValue(AttributeValue.class, attributeValue)) {
+		    attributeValueList.add((AttributeValue) attributeValue);
+		} else {
+		    boolean marshalled = !alreadyMarshalledIfRequired && attributeValue != o
+		        && !entityInformation.isCompositeHashAndRangeKeyProperty(propertyName);
 
-		boolean marshalled = !alreadyMarshalledIfRequired && attributeValue != o
-				&& !entityInformation.isCompositeHashAndRangeKeyProperty(propertyName);
+		    Class<?> targetPropertyType = marshalled ? String.class : propertyType;
+		    attributeValueList = addAttributeValue(attributeValueList, attributeValue, propertyName, targetPropertyType, true);
+		}
 
-		Class<?> targetPropertyType = marshalled ? String.class : propertyType;
-		List<AttributeValue> attributeValueList = new ArrayList<AttributeValue>();
-		attributeValueList = addAttributeValue(attributeValueList, attributeValue, propertyName, targetPropertyType, true);
 		return new Condition().withComparisonOperator(comparisonOperator).withAttributeValueList(attributeValueList);
-
 	}
 
 	protected Condition createCollectionCondition(String propertyName, ComparisonOperator comparisonOperator, Iterable<?> o,
@@ -647,12 +660,15 @@ protected Condition createCollectionCondition(String propertyName, ComparisonOpe
 		boolean marshalled = false;
 		for (Object object : o) {
 			Object attributeValue = getPropertyAttributeValue(propertyName, object);
-			if (attributeValue != null) {
-				marshalled = attributeValue != object && !entityInformation.isCompositeHashAndRangeKeyProperty(propertyName);
-			}
-			Class<?> targetPropertyType = marshalled ? String.class : propertyType;
-			attributeValueList = addAttributeValue(attributeValueList, attributeValue, propertyName, targetPropertyType, false);
-
+	        if (ClassUtils.isAssignableValue(AttributeValue.class, attributeValue)) {
+	            attributeValueList.add((AttributeValue) attributeValue);
+	        } else {
+	            if (attributeValue != null) {
+	                marshalled = attributeValue != object && !entityInformation.isCompositeHashAndRangeKeyProperty(propertyName);
+	            }
+	            Class<?> targetPropertyType = marshalled ? String.class : propertyType;
+	            attributeValueList = addAttributeValue(attributeValueList, attributeValue, propertyName, targetPropertyType, false);
+	        }
 		}
 
 		return new Condition().withComparisonOperator(comparisonOperator).withAttributeValueList(attributeValueList);

diff --git a/...gnin/spring/data/dynamodb/repository/query/DynamoDBEntityWithHashAndRangeKeyCriteria.java b/...gnin/spring/data/dynamodb/repository/query/DynamoDBEntityWithHashAndRangeKeyCriteria.java
@@ -38,6 +38,7 @@
 import org.socialsignin.spring.data.dynamodb.repository.support.DynamoDBIdIsHashAndRangeKeyEntityInformation;
 import org.springframework.util.Assert;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBQueryExpression;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBScanExpression;
 import com.amazonaws.services.dynamodbv2.model.ComparisonOperator;
@@ -67,8 +68,10 @@ protected boolean isRangeKeyProperty(String propertyName) {
 		return rangeKeyPropertyName.equals(propertyName);
 	}
 
-	public DynamoDBEntityWithHashAndRangeKeyCriteria(DynamoDBIdIsHashAndRangeKeyEntityInformation<T, ID> entityInformation) {
-		super(entityInformation);
+	public DynamoDBEntityWithHashAndRangeKeyCriteria(
+	        DynamoDBIdIsHashAndRangeKeyEntityInformation<T, ID> entityInformation, DynamoDBMapperTableModel<T> tableModel) {
+
+	    super(entityInformation, tableModel);
 		this.rangeKeyPropertyName = entityInformation.getRangeKeyPropertyName();
 		this.indexRangeKeyPropertyNames = entityInformation.getIndexRangeKeyPropertyNames();
 		if (indexRangeKeyPropertyNames == null) {

diff --git a/...alsignin/spring/data/dynamodb/repository/query/DynamoDBEntityWithHashKeyOnlyCriteria.java b/...alsignin/spring/data/dynamodb/repository/query/DynamoDBEntityWithHashKeyOnlyCriteria.java
@@ -29,6 +29,7 @@
 import org.socialsignin.spring.data.dynamodb.query.SingleEntityLoadByHashKeyQuery;
 import org.socialsignin.spring.data.dynamodb.repository.support.DynamoDBEntityInformation;
 
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBScanExpression;
 import com.amazonaws.services.dynamodbv2.model.ComparisonOperator;
 import com.amazonaws.services.dynamodbv2.model.Condition;
@@ -41,8 +42,8 @@ public class DynamoDBEntityWithHashKeyOnlyCriteria<T, ID extends Serializable> e
 
 	private DynamoDBEntityInformation<T, ID> entityInformation;
 
-	public DynamoDBEntityWithHashKeyOnlyCriteria(DynamoDBEntityInformation<T, ID> entityInformation) {
-		super(entityInformation);
+	public DynamoDBEntityWithHashKeyOnlyCriteria(DynamoDBEntityInformation<T, ID> entityInformation, DynamoDBMapperTableModel<T> tableModel) {
+		super(entityInformation, tableModel);
 		this.entityInformation = entityInformation;
 	}
 

diff --git a/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/FeedUser.java b/src/test/java/org/socialsignin/spring/data/dynamodb/domain/sample/FeedUser.java
@@ -9,8 +9,10 @@
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBHashKey;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBIndexHashKey;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBIndexRangeKey;
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperFieldModel.DynamoDBAttributeType;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBNativeBoolean;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBTable;
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBTyped;
 
 @DynamoDBTable(tableName = "feed_user")
 public class FeedUser {
@@ -29,7 +31,14 @@ public class FeedUser {
     private Date feedRegDate;
 
     @DynamoDBAttribute
-    @DynamoDBNativeBoolean
+    @DynamoDBTyped(DynamoDBAttributeType.N)  // For backwards compatibility with old versions, DynamoDBMapper booleans are
+                                             // serialized using the DynamoDB N type so this is not strictly required. This
+                                             // is here purely to test the DynamoDBMapper v2 conversion schemas using new
+                                             // non-deprecated annotations. NOTE: this introduces an important change in
+                                             // *internal* behaviour where @DynamoDBNativeBoolean will no longer be translated
+                                             // to DynamoDB type N. Instead, it would be correctly mapped to type BOOL which
+                                             // would cause this test to fail given that only scalar (B, N or S) types are
+                                             // allowed key types
     @DynamoDBIndexRangeKey(globalSecondaryIndexName = "idx_global_usrNo_feedOpenYn")
     private boolean feedOpenYn;
 

diff --git a/...ing/data/dynamodb/repository/query/DynamoDBEntityWithHashAndRangeKeyCriteriaUnitTest.java b/...ing/data/dynamodb/repository/query/DynamoDBEntityWithHashAndRangeKeyCriteriaUnitTest.java
@@ -22,7 +22,7 @@ public void setUp()
 	{
 		Mockito.when(entityInformation.getHashKeyPropertyName()).thenReturn("userName");
 		Mockito.when(entityInformation.getRangeKeyPropertyName()).thenReturn("playlistName");
-		criteria = new DynamoDBEntityWithHashAndRangeKeyCriteria<Playlist,String>(entityInformation);
+		criteria = new DynamoDBEntityWithHashAndRangeKeyCriteria<Playlist,String>(entityInformation, null);
 	}
 
 	@Test

diff --git a/.../spring/data/dynamodb/repository/query/DynamoDBEntityWithHashKeyOnlyCriteriaUnitTest.java b/.../spring/data/dynamodb/repository/query/DynamoDBEntityWithHashKeyOnlyCriteriaUnitTest.java
@@ -23,7 +23,7 @@ public class DynamoDBEntityWithHashKeyOnlyCriteriaUnitTest extends AbstractDynam
 	public void setUp()
 	{
 		Mockito.when(entityInformation.getHashKeyPropertyName()).thenReturn("id");
-		criteria = new DynamoDBEntityWithHashKeyOnlyCriteria<User,String>(entityInformation);
+		criteria = new DynamoDBEntityWithHashKeyOnlyCriteria<User,String>(entityInformation, null);
 	}
 
 	@Test