Skip to content

Commit

Permalink
Issue #118: Query Creation Uses Attribute Name instead of Column Name
Browse files Browse the repository at this point in the history 
Regression from Spring v4.5
  • Loading branch information
@derjust
derjust committed Feb 2, 2018
1 parent 2ccb10b commit 398eed8
Show file tree
Hide file tree
Showing 14 changed files with 394 additions and 72 deletions.
40 changes: 21 additions & 19 deletions ...org/socialsignin/spring/data/dynamodb/repository/query/AbstractDynamoDBQueryCriteria.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ protected QueryRequest buildQueryRequest(String tableName, String theIndexName,
queryRequest.setIndexName(theIndexName);

if (isApplicableForGlobalSecondaryIndex()) {
List<String> allowedSortProperties = new ArrayList<String>();
List<String> allowedSortProperties = new ArrayList<>();

for (Entry<String, List<Condition>> singlePropertyCondition : propertyConditions.entrySet()) {
if (entityInformation.getGlobalSecondaryIndexNamesByPropertyName().keySet()
Expand All @@ -97,7 +97,7 @@ protected QueryRequest buildQueryRequest(String tableName, String theIndexName,
}
}

HashMap<String, Condition> keyConditions = new HashMap<String, Condition>();
HashMap<String, Condition> keyConditions = new HashMap<>();

if (hashKeyConditions != null && hashKeyConditions.size() > 0) {
for (Condition hashKeyCondition : hashKeyConditions) {
Expand Down Expand Up @@ -130,7 +130,7 @@ protected QueryRequest buildQueryRequest(String tableName, String theIndexName,

queryRequest.setKeyConditions(keyConditions);
queryRequest.setSelect(Select.ALL_PROJECTED_ATTRIBUTES);
applySortIfSpecified(queryRequest, new ArrayList<String>(new HashSet<String>(allowedSortProperties)));
applySortIfSpecified(queryRequest, new ArrayList<>(new HashSet<>(allowedSortProperties)));
}
return queryRequest;
}
Expand Down Expand Up @@ -223,11 +223,11 @@ protected List<Condition> getHashKeyConditions() {

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.attributeConditions = new LinkedMultiValueMap<>();
this.propertyConditions = new LinkedMultiValueMap<>();
this.hashKeyPropertyName = dynamoDBEntityInformation.getHashKeyPropertyName();
this.entityInformation = dynamoDBEntityInformation;
this.attributeNamesByPropertyName = new HashMap<String, String>();
this.attributeNamesByPropertyName = new HashMap<>();
// TODO consider adding the DynamoDBMapper table model to DynamoDBEntityInformation instead
this.tableModel = tableModel;
}
Expand Down Expand Up @@ -257,10 +257,10 @@ protected String getGlobalSecondaryIndexName() {
if (globalSecondaryIndexName == null && attributeConditions != null && !attributeConditions.isEmpty())
{
// Declare map of index names by attribute name which we will populate below - this will be used to determine which index to use if multiple indexes are applicable
Map<String,String[]> indexNamesByAttributeName = new HashMap<String,String[]>();
Map<String, String[]> indexNamesByAttributeName = new HashMap<>();

// Declare map of attribute lists by index name which we will populate below - this will be used to determine whether we have an exact match index for specified attribute conditions
MultiValueMap<String,String> attributeListsByIndexName = new LinkedMultiValueMap<String,String>();
MultiValueMap<String, String> attributeListsByIndexName = new LinkedMultiValueMap<>();

// Populate the above maps
for (Entry<String, String[]> indexNamesForPropertyNameEntry : entityInformation.getGlobalSecondaryIndexNamesByPropertyName().entrySet())
Expand All @@ -275,8 +275,8 @@ protected String getGlobalSecondaryIndexName() {
}

// Declare lists to store matching index names
List<String> exactMatchIndexNames = new ArrayList<String>();
List<String> partialMatchIndexNames = new ArrayList<String>();
List<String> exactMatchIndexNames = new ArrayList<>();
List<String> partialMatchIndexNames = new ArrayList<>();

// Populate matching index name lists - an index is either an exact match ( the index attributes match all the specified criteria exactly)
// or a partial match ( the properties for the specified criteria are contained within the property set for an index )
Expand Down Expand Up @@ -412,8 +412,7 @@ public Object getHashKeyPropertyValue() {
protected String getAttributeName(String propertyName) {
String attributeName = attributeNamesByPropertyName.get(propertyName);
if (attributeName == null) {
String overriddenName = entityInformation.getOverriddenAttributeName(propertyName);
attributeName = overriddenName != null ? overriddenName : propertyName;
attributeName = entityInformation.getOverriddenAttributeName(propertyName).orElse(propertyName);
attributeNamesByPropertyName.put(propertyName, attributeName);
}
return attributeName;
Expand Down Expand Up @@ -498,7 +497,11 @@ protected <V extends Object> Object getPropertyAttributeValue(final String prope
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);

String attributeName = getAttributeName(propertyName);
entityInformation.getOverriddenAttributeName(propertyName).orElse(propertyName);

DynamoDBMapperFieldModel<T,Object> fieldModel = tableModel.field(attributeName);
if (fieldModel != null) {
return fieldModel.convert(value);
}
Expand All @@ -515,7 +518,7 @@ protected <V> Condition createNoValueCondition(String propertyName, ComparisonOp
}

private List<String> getNumberListAsStringList(List<Number> numberList) {
List<String> list = new ArrayList<String>();
List<String> list = new ArrayList<>();
for (Number number : numberList) {
if (number != null) {
list.add(number.toString());
Expand All @@ -541,7 +544,7 @@ private List<String> getDateListAsStringList(List<Date> dateList) {

private List<String> getInstantListAsStringList(List<Instant> dateList) {
DynamoDBMarshaller<Instant> marshaller = new Instant2IsoDynamoDBMarshaller();
List<String> list = new ArrayList<String>();
List<String> list = new ArrayList<>();
for (Instant date : dateList) {
if (date != null) {
list.add(marshaller.marshall(date));
Expand All @@ -553,7 +556,7 @@ private List<String> getInstantListAsStringList(List<Instant> dateList) {
}

private List<String> getBooleanListAsStringList(List<Boolean> booleanList) {
List<String> list = new ArrayList<String>();
List<String> list = new ArrayList<>();
for (Boolean booleanValue : booleanList) {
if (booleanValue != null) {
list.add(booleanValue.booleanValue() ? "1" : "0");
Expand All @@ -567,9 +570,8 @@ private List<String> getBooleanListAsStringList(List<Boolean> booleanList) {
@SuppressWarnings("unchecked")
private <P> List<P> getAttributeValueAsList(Object attributeValue) {
boolean isIterable = ClassUtils.isAssignable(Iterable.class, attributeValue.getClass());
List<P> attributeValueAsList = null;
if (isIterable) {
attributeValueAsList = new ArrayList<P>();
List<P> attributeValueAsList = new ArrayList<>();
Iterable<P> iterable = (Iterable<P>) attributeValue;
for (P attributeValueElement : iterable) {
attributeValueAsList.add(attributeValueElement);
Expand Down Expand Up @@ -643,7 +645,7 @@ 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>();
List<AttributeValue> attributeValueList = new ArrayList<>();
Object attributeValue = !alreadyMarshalledIfRequired ? getPropertyAttributeValue(propertyName, o) : o;
if (ClassUtils.isAssignableValue(AttributeValue.class, attributeValue)) {
attributeValueList.add((AttributeValue) attributeValue);
Expand Down
29 changes: 15 additions & 14 deletions ...g/socialsignin/spring/data/dynamodb/repository/support/DynamoDBEntityMetadataSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

/**
* @author Michael Lavelle
Expand Down Expand Up @@ -221,64 +222,64 @@ public String getOverriddenAttributeName(Method method) {
}

@Override
public String getOverriddenAttributeName(final String propertyName) {
public Optional<String> getOverriddenAttributeName(final String propertyName) {

Method method = findMethod(propertyName);
if (method != null) {
if (method.getAnnotation(DynamoDBAttribute.class) != null
&& !StringUtils.isEmpty(method.getAnnotation(DynamoDBAttribute.class).attributeName())) {
return method.getAnnotation(DynamoDBAttribute.class).attributeName();
return Optional.of(method.getAnnotation(DynamoDBAttribute.class).attributeName());
}
if (method.getAnnotation(DynamoDBHashKey.class) != null
&& !StringUtils.isEmpty(method.getAnnotation(DynamoDBHashKey.class).attributeName())) {
return method.getAnnotation(DynamoDBHashKey.class).attributeName();
return Optional.of(method.getAnnotation(DynamoDBHashKey.class).attributeName());
}
if (method.getAnnotation(DynamoDBRangeKey.class) != null
&& !StringUtils.isEmpty(method.getAnnotation(DynamoDBRangeKey.class).attributeName())) {
return method.getAnnotation(DynamoDBRangeKey.class).attributeName();
return Optional.of(method.getAnnotation(DynamoDBRangeKey.class).attributeName());
}
if (method.getAnnotation(DynamoDBIndexRangeKey.class) != null
&& !StringUtils.isEmpty(method.getAnnotation(DynamoDBIndexRangeKey.class).attributeName())) {
return method.getAnnotation(DynamoDBIndexRangeKey.class).attributeName();
return Optional.of(method.getAnnotation(DynamoDBIndexRangeKey.class).attributeName());
}
if (method.getAnnotation(DynamoDBIndexHashKey.class) != null
&& !StringUtils.isEmpty(method.getAnnotation(DynamoDBIndexHashKey.class).attributeName())) {
return method.getAnnotation(DynamoDBIndexHashKey.class).attributeName();
return Optional.of(method.getAnnotation(DynamoDBIndexHashKey.class).attributeName());
}
if (method.getAnnotation(DynamoDBVersionAttribute.class) != null
&& !StringUtils.isEmpty(method.getAnnotation(DynamoDBVersionAttribute.class).attributeName())) {
return method.getAnnotation(DynamoDBVersionAttribute.class).attributeName();
return Optional.of(method.getAnnotation(DynamoDBVersionAttribute.class).attributeName());
}
}

Field field = findField(propertyName);
if (field != null) {
if (field.getAnnotation(DynamoDBAttribute.class) != null
&& !StringUtils.isEmpty(field.getAnnotation(DynamoDBAttribute.class).attributeName())) {
return field.getAnnotation(DynamoDBAttribute.class).attributeName();
return Optional.of(field.getAnnotation(DynamoDBAttribute.class).attributeName());
}
if (field.getAnnotation(DynamoDBHashKey.class) != null
&& !StringUtils.isEmpty(field.getAnnotation(DynamoDBHashKey.class).attributeName())) {
return field.getAnnotation(DynamoDBHashKey.class).attributeName();
return Optional.of(field.getAnnotation(DynamoDBHashKey.class).attributeName());
}
if (field.getAnnotation(DynamoDBRangeKey.class) != null
&& !StringUtils.isEmpty(field.getAnnotation(DynamoDBRangeKey.class).attributeName())) {
return field.getAnnotation(DynamoDBRangeKey.class).attributeName();
return Optional.of(field.getAnnotation(DynamoDBRangeKey.class).attributeName());
}
if (field.getAnnotation(DynamoDBIndexRangeKey.class) != null
&& !StringUtils.isEmpty(field.getAnnotation(DynamoDBIndexRangeKey.class).attributeName())) {
return field.getAnnotation(DynamoDBIndexRangeKey.class).attributeName();
return Optional.of(field.getAnnotation(DynamoDBIndexRangeKey.class).attributeName());
}
if (field.getAnnotation(DynamoDBIndexHashKey.class) != null
&& !StringUtils.isEmpty(field.getAnnotation(DynamoDBIndexHashKey.class).attributeName())) {
return field.getAnnotation(DynamoDBIndexHashKey.class).attributeName();
return Optional.of(field.getAnnotation(DynamoDBIndexHashKey.class).attributeName());
}
if (field.getAnnotation(DynamoDBVersionAttribute.class) != null
&& !StringUtils.isEmpty(field.getAnnotation(DynamoDBVersionAttribute.class).attributeName())) {
return field.getAnnotation(DynamoDBVersionAttribute.class).attributeName();
return Optional.of(field.getAnnotation(DynamoDBVersionAttribute.class).attributeName());
}
}
return null;
return Optional.empty();

}

Expand Down
11 changes: 6 additions & 5 deletions ...gnin/spring/data/dynamodb/repository/support/DynamoDBHashKeyExtractingEntityMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.springframework.data.repository.core.EntityMetadata;

import java.util.Map;
import java.util.Optional;

/**
* Obtains basic hash key-related metadata about a DynamoDBEntity, such as
Expand All @@ -30,15 +31,15 @@
*/
public interface DynamoDBHashKeyExtractingEntityMetadata<T> extends EntityMetadata<T> {

public String getOverriddenAttributeName(String propertyName);
Optional<String> getOverriddenAttributeName(String propertyName);

public DynamoDBMarshaller<?> getMarshallerForProperty(String propertyName);
DynamoDBMarshaller<?> getMarshallerForProperty(String propertyName);

public boolean isHashKeyProperty(String propertyName);
boolean isHashKeyProperty(String propertyName);

public String getHashKeyPropertyName();
String getHashKeyPropertyName();

public String getDynamoDBTableName();
String getDynamoDBTableName();

Map<String, String[]> getGlobalSecondaryIndexNamesByPropertyName();

Expand Down
3 changes: 2 additions & 1 deletion ...ng/data/dynamodb/repository/support/DynamoDBIdIsHashAndRangeKeyEntityInformationImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.springframework.data.repository.core.support.ReflectionEntityInformation;

import java.util.Map;
import java.util.Optional;
import java.util.Set;

/**
Expand Down Expand Up @@ -60,7 +61,7 @@ public Object getRangeKey(final ID id) {
}

@Override
public String getOverriddenAttributeName(String attributeName) {
public Optional<String> getOverriddenAttributeName(String attributeName) {
return metadata.getOverriddenAttributeName(attributeName);
}

Expand Down
3 changes: 2 additions & 1 deletion ...nin/spring/data/dynamodb/repository/support/DynamoDBIdIsHashKeyEntityInformationImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.springframework.util.Assert;

import java.util.Map;
import java.util.Optional;

/**
* Encapsulates minimal information needed to load DynamoDB entities.
Expand Down Expand Up @@ -65,7 +66,7 @@ public boolean isRangeKeyAware() {
}

@Override
public String getOverriddenAttributeName(String attributeName) {
public Optional<String> getOverriddenAttributeName(String attributeName) {
return metadata.getOverriddenAttributeName(attributeName);
}

Expand Down
8 changes: 7 additions & 1 deletion ...a/org/socialsignin/spring/data/dynamodb/repository/support/DynamoDBRepositoryFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,17 @@ public class DynamoDBRepositoryFactory extends RepositoryFactorySupport {
private static final Logger LOGGER = LoggerFactory.getLogger(DynamoDBRepositoryFactory.class);

static {
final String DEVELOPMENT = "DEVELOPMENT";

String awsSdkVersion = VersionInfoUtils.getVersion();
String springDataVersion = Version.class.getPackage().getImplementationVersion();

String thisSpecVersion = DynamoDBRepositoryFactory.class.getPackage().getSpecificationVersion();
String thisImplVersion = DynamoDBRepositoryFactory.class.getPackage().getImplementationVersion();
if (thisImplVersion == null || thisSpecVersion == null) {
thisSpecVersion = DEVELOPMENT;
thisImplVersion = DEVELOPMENT;
}

LOGGER.info("Spring Data DynamoDB Version: {} ({})", thisImplVersion, thisSpecVersion);
LOGGER.info("Spring Data Version: {}", springDataVersion);
Expand All @@ -57,7 +63,7 @@ public class DynamoDBRepositoryFactory extends RepositoryFactorySupport {
LOGGER.info("Platform Details: {} {}", System.getProperty("os.name"),
System.getProperty("os.version"));

if (!isCompatible(springDataVersion, thisSpecVersion)) {
if (!DEVELOPMENT.equals(thisImplVersion) && !isCompatible(springDataVersion, thisSpecVersion)) {
LOGGER.warn("This Spring Data DynamoDB implementation might not be compatible with the available Spring Data classes on the classpath!"
+ System.getProperty("line.separator") + "NoDefClassFoundExceptions or similar might occur!");
}
Expand Down

0 comments on commit 398eed8

Please sign in to comment.