Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Commit

Permalink
Merge 2632ee8 into 57b7964
Browse files Browse the repository at this point in the history
  • Loading branch information
msgroi committed Apr 2, 2019
2 parents 57b7964 + 2632ee8 commit abcba29
Show file tree
Hide file tree
Showing 10 changed files with 345 additions and 189 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
import com.salesforce.dynamodbv2.mt.mappers.index.DynamoSecondaryIndex.DynamoSecondaryIndexType;
import com.salesforce.dynamodbv2.mt.mappers.metadata.PrimaryKey;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;

/**
* TODO: write Javadoc.
Expand All @@ -42,7 +44,7 @@ public static CreateTableRequestBuilder builder() {

public CreateTableRequest build() {
setDefaults();
return createTableRequest;
return createTableRequest.clone();
}

private void setDefaults() {
Expand Down Expand Up @@ -95,6 +97,18 @@ public CreateTableRequestBuilder withTableKeySchema(String hashKeyField,
return this;
}

/**
* Adds <code>AttributeDefinition</code>s.
*
* @param attributeDefinitions attribute definitions to add
* @return CreateTableRequestBuilder
*/
public CreateTableRequestBuilder withAttributeDefinitions(AttributeDefinition... attributeDefinitions) {
this.createTableRequest.setAttributeDefinitions(Optional.ofNullable(attributeDefinitions)
.map(Arrays::asList).orElse(null));
return this;
}

/**
* TODO: write Javadoc.
*
Expand Down Expand Up @@ -162,6 +176,35 @@ public CreateTableRequestBuilder withStreamSpecification(StreamSpecification str
return this;
}

public CreateTableRequestBuilder withKeySchema(KeySchemaElement... keySchema) {
this.createTableRequest.withKeySchema(Optional.ofNullable(keySchema).map(Arrays::asList).orElse(null));
return this;
}

/**
* Set the list of global secondary indices.
*
* @param globalSecondaryIndexes global secondary indices to add
* @return CreateTableRequestBuilder
*/
public CreateTableRequestBuilder withGlobalSecondaryIndexes(GlobalSecondaryIndex... globalSecondaryIndexes) {
this.createTableRequest.setGlobalSecondaryIndexes(Optional.ofNullable(globalSecondaryIndexes)
.map(Arrays::asList).orElse(null));
return this;
}

/**
* Sets the list of local secondary indices.
*
* @param localSecondaryIndexes local secondary indices to add
* @return CreateTableRequestBuilder
*/
public CreateTableRequestBuilder withLocalSecondaryIndexes(LocalSecondaryIndex... localSecondaryIndexes) {
this.createTableRequest.setLocalSecondaryIndexes(Optional.ofNullable(localSecondaryIndexes)
.map(Arrays::asList).orElse(null));
return this;
}

private void addAttributeDefinition(String field, ScalarAttributeType fieldType) {
if (createTableRequest.getAttributeDefinitions() == null) {
createTableRequest.setAttributeDefinitions(new ArrayList<>());
Expand All @@ -186,4 +229,5 @@ private List<KeySchemaElement> buildKeySchema(PrimaryKey primaryKey) {
public CreateTableRequest getCreateTableRequest() {
return createTableRequest;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
import static com.amazonaws.util.CollectionUtils.isNullOrEmpty;

import com.amazonaws.services.dynamodbv2.model.AttributeValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;

import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -26,11 +29,13 @@
class ItemMapper {

private final FieldMapper fieldMapper;
private final TableMapping tableMapping;
private final Map<String, List<FieldMapping>> virtualToPhysicalFieldMappings;
private final Map<String, List<FieldMapping>> physicalToVirtualFieldMappings;

ItemMapper(TableMapping tableMapping, FieldMapper fieldMapper) {
ItemMapper(FieldMapper fieldMapper, Map<String, List<FieldMapping>> virtualToPhysicalFieldMappings) {
this.fieldMapper = fieldMapper;
this.tableMapping = tableMapping;
this.virtualToPhysicalFieldMappings = virtualToPhysicalFieldMappings;
this.physicalToVirtualFieldMappings = invertMapping(virtualToPhysicalFieldMappings);
}

/*
Expand All @@ -42,8 +47,6 @@ class ItemMapper {
*/
Map<String, AttributeValue> apply(Map<String, AttributeValue> unqualifiedItem) {
Map<String, AttributeValue> qualifiedItem = new HashMap<>();
Map<String, List<FieldMapping>> virtualToPhysicalFieldMappings =
tableMapping.getAllVirtualToPhysicalFieldMappings();
unqualifiedItem.forEach((field, attribute) -> {
List<FieldMapping> fieldMappings = virtualToPhysicalFieldMappings.get(field);
if (!isNullOrEmpty(fieldMappings)) {
Expand All @@ -70,8 +73,6 @@ Map<String, AttributeValue> reverse(Map<String, AttributeValue> qualifiedItem) {
return null;
}
Map<String, AttributeValue> unqualifiedItem = new HashMap<>();
Map<String, List<FieldMapping>> physicalToVirtualFieldMappings =
tableMapping.getAllPhysicalToVirtualFieldMappings();
qualifiedItem.forEach((field, attribute) -> {
List<FieldMapping> fieldMappings = physicalToVirtualFieldMappings.get((field));
if (fieldMappings != null && !fieldMappings.isEmpty()) {
Expand All @@ -86,4 +87,20 @@ Map<String, AttributeValue> reverse(Map<String, AttributeValue> qualifiedItem) {
return unqualifiedItem;
}

@VisibleForTesting
protected static Map<String, List<FieldMapping>> invertMapping(
Map<String, List<FieldMapping>> mapping) {
Map<String, List<FieldMapping>> fieldMappings = new HashMap<>();
mapping.values().stream()
.flatMap(Collection::stream)
.forEach(fieldMapping -> fieldMappings.put(fieldMapping.getTarget().getName(),
ImmutableList.of(new FieldMapping(fieldMapping.getTarget(),
fieldMapping.getSource(),
fieldMapping.getVirtualIndexName(),
fieldMapping.getPhysicalIndexName(),
fieldMapping.getIndexType(),
fieldMapping.isContextAware()))));
return fieldMappings;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ public GetItemResult getItem(GetItemRequest getItemRequest) {
getItemRequest.withTableName(tableMapping.getPhysicalTable().getTableName());

// map key
getItemRequest.setKey(tableMapping.getItemMapper().apply(getItemRequest.getKey()));
getItemRequest.setKey(tableMapping.getKeyMapper().apply(getItemRequest.getKey()));

// get
GetItemResult getItemResult = getAmazonDynamoDb().getItem(getItemRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.amazonaws.services.dynamodbv2.model.ResourceNotFoundException;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.salesforce.dynamodbv2.mt.context.MtAmazonDynamoDbContextProvider;
import com.salesforce.dynamodbv2.mt.mappers.MappingException;
import com.salesforce.dynamodbv2.mt.mappers.index.DynamoSecondaryIndex;
Expand All @@ -28,8 +27,8 @@
import com.salesforce.dynamodbv2.mt.mappers.metadata.PrimaryKey;
import com.salesforce.dynamodbv2.mt.mappers.sharedtable.CreateTableRequestFactory;
import com.salesforce.dynamodbv2.mt.mappers.sharedtable.impl.FieldMapping.Field;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -48,10 +47,10 @@ class TableMapping {
private DynamoTableDescription physicalTable;
private final DynamoSecondaryIndexMapper secondaryIndexMapper;
private final Map<String, List<FieldMapping>> virtualToPhysicalMappings;
private final Map<String, List<FieldMapping>> physicalToVirtualMappings;
private final Map<DynamoSecondaryIndex, List<FieldMapping>> secondaryIndexFieldMappings;

private final ItemMapper itemMapper;
private final ItemMapper keyMapper;
private final QueryAndScanMapper queryAndScanMapper;
private final ConditionMapper conditionMapper;

Expand All @@ -67,12 +66,18 @@ class TableMapping {
this.secondaryIndexFieldMappings =
buildIndexPrimaryKeyFieldMappings(virtualTable, physicalTable, secondaryIndexMapper);
this.virtualToPhysicalMappings = buildAllVirtualToPhysicalFieldMappings(virtualTable);
this.physicalToVirtualMappings = buildAllPhysicalToVirtualFieldMappings(virtualToPhysicalMappings);
validateVirtualPhysicalCompatibility();
validateMapping();
FieldMapper fieldMapper = new FieldMapper(mtContext,
virtualTable.getTableName(),
new FieldPrefixFunction(delimiter));
itemMapper = new ItemMapper(this, fieldMapper);
itemMapper = new ItemMapper(
fieldMapper,
virtualToPhysicalMappings
);
keyMapper = new ItemMapper(
fieldMapper,
buildVirtualToPhysicalKeyFieldMappings()
);
queryAndScanMapper = new QueryAndScanMapper(this, fieldMapper);
conditionMapper = new ConditionMapper(this, fieldMapper);
}
Expand All @@ -89,6 +94,10 @@ ItemMapper getItemMapper() {
return itemMapper;
}

ItemMapper getKeyMapper() {
return keyMapper;
}

QueryAndScanMapper getQueryAndScanMapper() {
return queryAndScanMapper;
}
Expand Down Expand Up @@ -140,13 +149,6 @@ private static Map<String, FieldMapping> dedupeFieldMappings(Map<String, List<Fi
));
}

/*
* Returns a mapping of physical to virtual fields.
*/
Map<String, List<FieldMapping>> getAllPhysicalToVirtualFieldMappings() {
return physicalToVirtualMappings;
}

/*
* Returns a mapping of primary key fields for a specific secondary index, virtual to physical.
*/
Expand Down Expand Up @@ -194,25 +196,16 @@ private DynamoTableDescription lookupPhysicalTable(DynamoTableDescription virtua

private Map<String, List<FieldMapping>> buildAllVirtualToPhysicalFieldMappings(
DynamoTableDescription virtualTable) {
Map<String, List<FieldMapping>> fieldMappings = new HashMap<>();
getTablePrimaryKeyFieldMappings().forEach(fieldMapping -> addFieldMapping(fieldMappings, fieldMapping));
Map<String, List<FieldMapping>> fieldMappings = new HashMap<>(buildVirtualToPhysicalKeyFieldMappings());
virtualTable.getSis().forEach(virtualSi -> getIndexPrimaryKeyFieldMappings(virtualSi)
.forEach(fieldMapping -> addFieldMapping(fieldMappings, fieldMapping)));
return fieldMappings;
}

private Map<String, List<FieldMapping>> buildAllPhysicalToVirtualFieldMappings(
Map<String, List<FieldMapping>> virtualToPhysicalMappings) {
@VisibleForTesting
Map<String, List<FieldMapping>> buildVirtualToPhysicalKeyFieldMappings() {
Map<String, List<FieldMapping>> fieldMappings = new HashMap<>();
virtualToPhysicalMappings.values().stream()
.flatMap(Collection::stream)
.forEach(fieldMapping -> fieldMappings.put(fieldMapping.getTarget().getName(),
ImmutableList.of(new FieldMapping(fieldMapping.getTarget(),
fieldMapping.getSource(),
fieldMapping.getVirtualIndexName(),
fieldMapping.getPhysicalIndexName(),
fieldMapping.getIndexType(),
fieldMapping.isContextAware()))));
getTablePrimaryKeyFieldMappings().forEach(fieldMapping -> addFieldMapping(fieldMappings, fieldMapping));
return fieldMappings;
}

Expand Down Expand Up @@ -257,12 +250,21 @@ private Map<DynamoSecondaryIndex, List<FieldMapping>> buildIndexPrimaryKeyFieldM
/*
* Helper method for adding a single FieldMapping object to the existing list of FieldMapping objects.
*/
private void addFieldMapping(Map<String, List<FieldMapping>> fieldMappings, FieldMapping fieldMappingToAdd) {
private static void addFieldMapping(Map<String, List<FieldMapping>> fieldMappings, FieldMapping fieldMappingToAdd) {
String key = fieldMappingToAdd.getSource().getName();
List<FieldMapping> fieldMapping = fieldMappings.computeIfAbsent(key, k -> new ArrayList<>());
fieldMapping.add(fieldMappingToAdd);
}

/*
* Validates the table mapping.
*/
private void validateMapping() {
checkArgument(virtualTable.getGsis().size() <= 1, "no more than one GSI is supported");
checkArgument(virtualTable.getLsis().size() <= 1, "no more than one LSI is supported");
validateVirtualPhysicalCompatibility();
}

/*
* Validate that the key schema elements match between the table's virtual and physical primary key as
* well as indexes.
Expand Down Expand Up @@ -319,9 +321,6 @@ private void validateLsiMappings(DynamoTableDescription virtualTable,
try {
DynamoSecondaryIndex physicalLsi = secondaryIndexMapper.lookupPhysicalSecondaryIndex(virtualLsi,
physicalTable);
checkArgument(!usedPhysicalLsis.containsKey(physicalLsi),
"two virtual LSIs (one:" + usedPhysicalLsis.get(physicalLsi) + ", two:"
+ virtualLsi + "), mapped to one physical LSI: " + physicalLsi);
usedPhysicalLsis.put(physicalLsi, virtualLsi);
} catch (MappingException e) {
throw new IllegalArgumentException("failure mapping virtual to physical " + virtualLsi.getType() + ": "
Expand Down
20 changes: 20 additions & 0 deletions src/test/java/com/salesforce/dynamodbv2/GetTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@
import static com.amazonaws.services.dynamodbv2.model.ScalarAttributeType.S;
import static com.salesforce.dynamodbv2.testsupport.DefaultTestSetup.TABLE1;
import static com.salesforce.dynamodbv2.testsupport.DefaultTestSetup.TABLE3;
import static com.salesforce.dynamodbv2.testsupport.DefaultTestSetup.TABLE5;
import static com.salesforce.dynamodbv2.testsupport.TestSupport.HASH_KEY_VALUE;
import static com.salesforce.dynamodbv2.testsupport.TestSupport.RANGE_KEY_S_VALUE;
import static com.salesforce.dynamodbv2.testsupport.TestSupport.SOME_FIELD_VALUE;
import static com.salesforce.dynamodbv2.testsupport.TestSupport.getItem;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertEquals;

import com.salesforce.dynamodbv2.testsupport.ArgumentBuilder.TestArgument;
import com.salesforce.dynamodbv2.testsupport.DefaultArgumentProvider;
import com.salesforce.dynamodbv2.testsupport.ItemBuilder;

import java.util.Optional;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;
Expand Down Expand Up @@ -53,4 +56,21 @@ void getHkRkTable(TestArgument testArgument) {
.build())));
}

@ParameterizedTest(name = "{arguments}")
@ArgumentsSource(DefaultArgumentProvider.class)
void getHkRk_TableWithGsiHkSameAsTableRk(TestArgument testArgument) {
String table = TABLE5;
testArgument.forEachOrgContext(
org -> assertEquals(
ItemBuilder.builder(testArgument.getHashKeyAttrType(), HASH_KEY_VALUE)
.someField(S, SOME_FIELD_VALUE + table + org)
.rangeKey(S, RANGE_KEY_S_VALUE)
.build(),
getItem(testArgument.getAmazonDynamoDb(),
table,
HASH_KEY_VALUE,
testArgument.getHashKeyAttrType(),
Optional.of(RANGE_KEY_S_VALUE))));
}

}
22 changes: 22 additions & 0 deletions src/test/java/com/salesforce/dynamodbv2/QueryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import static com.salesforce.dynamodbv2.testsupport.DefaultTestSetup.TABLE1;
import static com.salesforce.dynamodbv2.testsupport.DefaultTestSetup.TABLE3;
import static com.salesforce.dynamodbv2.testsupport.DefaultTestSetup.TABLE4;
import static com.salesforce.dynamodbv2.testsupport.DefaultTestSetup.TABLE5;
import static com.salesforce.dynamodbv2.testsupport.ItemBuilder.HASH_KEY_FIELD;
import static com.salesforce.dynamodbv2.testsupport.ItemBuilder.INDEX_FIELD;
import static com.salesforce.dynamodbv2.testsupport.ItemBuilder.RANGE_KEY_FIELD;
Expand Down Expand Up @@ -49,6 +50,7 @@
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;

Expand Down Expand Up @@ -391,6 +393,26 @@ void queryGsi(TestArgument testArgument) {
});
}

@ParameterizedTest(name = "{arguments}")
@ArgumentsSource(DefaultArgumentProvider.class)
void queryGsi_TableWithGsiHkSameAsTableRk(TestArgument testArgument) {
testArgument.forEachOrgContext(org -> {
String table = TABLE5;
List<Map<String, AttributeValue>> items = testArgument.getAmazonDynamoDb().query(
new QueryRequest().withTableName(table).withKeyConditionExpression("#name = :value")
.withExpressionAttributeNames(ImmutableMap.of("#name", RANGE_KEY_FIELD))
.withExpressionAttributeValues(ImmutableMap.of(":value",
createStringAttribute(RANGE_KEY_OTHER_S_VALUE)))
.withIndexName("testgsi_table_rk_as_index_hk")).getItems();
assertEquals(1, items.size());
assertThat(items.get(0), is(ItemBuilder.builder(testArgument.getHashKeyAttrType(), HASH_KEY_VALUE)
.someField(S, SOME_OTHER_FIELD_VALUE + table + org)
.rangeKey(S, RANGE_KEY_OTHER_S_VALUE)
.indexField(S, INDEX_FIELD_VALUE)
.build()));
});
}

@ParameterizedTest(name = "{arguments}")
@ArgumentsSource(DefaultArgumentProvider.class)
void queryLsi(TestArgument testArgument) {
Expand Down

0 comments on commit abcba29

Please sign in to comment.