BREAKING: Introduce an interface for DynamoDB attribute

[jungseoklee]

Changes since v1:

• Redesign signature of add{Partition|Sort}Key methods which is a breaking change
• Rename AttributeProps to Attribute
• Rename KeyAttributeType to AttributeType

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

Summary

This patch introduces AttributeProps to represent DynamoDB attribute and deprecates add{Partition|Sort}Key(name, type) methods.

Why AttributeProps needed?

It is needed to represent the fact that both name and type are required when defining attributes for table and global or local secondary indexes. Whereas DynamoDB CloudFormation abstracts it through AttributeDefinition, CDK does not have a corresponding concept yet, which makes supporting GSI and LSI complicated and cumbersome.

Here is an example designing a GSI property. We can deal with partition and sort keys without an additional property. It is not concise though. How about handling NonKeyAttributes? NonKeyAttributes should be part of AttributeDefinitions, which means the GSI property should know their types as well. AttributeProps simplifies this matter.

// with AttributeProps
export interface GlobalSecondaryIndexProps {
    indexName: string;

    partitionKey: AttributeProps;

    sortKey?: AttributeProps;

    projectionType: ProjectionType;

    nonKeyAttributes?: AttributeProps[];

    readCapacity?: number;

    writeCapacity?: number;
}

// without AttributeProps
export interface GlobalSecondaryIndexProps {
    indexName: string;

    partitionKeyName: string;

    partitionKeyType: KeyAttributeType;  

    sortKeyName?: string;

    sortKeyType?: KeyAttributeType;

    projectionType: ProjectionType;

    nonKeyAttributes?: ???;

    readCapacity?: number;

    writeCapacity?: number;
}

Why add{Partition|Sort}Key(name, type) deprecated?

IMHO, it does not align with a way to define database schema. Both name and type always come together when defining partition and sort keys. In that sense, it would be natural to group them.

Test

OK: 33 assertions (287ms)

=============================== Coverage summary ===============================
Statements   : 97.5% ( 78/80 )
Branches     : 91.84% ( 45/49 )
Functions    : 100% ( 18/18 )
Lines        : 97.4% ( 75/77 )
================================================================================
Verifying integ.dynamodb.js against integ.dynamodb.expected.json... OK.

[rix0rrr]

I agree with the gist of this change, but there are some (naming) modifications I'd like to see done to the current proposal.

[rix0rrr]

No overloads please (does not translate to every jsii language).

Now is the time to break this API, if we're going to do so.

[jungseoklee]

I do not have a clear picture on backward compatibility rules. That is why I overloaded the method.

I would prefer breaking it if this is allowed. If this PR is a right direction to support GSI and LSI, the indexes will give users different API experience when defining partition and sort keys. It means that we will have API inconsistency which is not ideal.

[eladb]

We are pre-1.0, feel free to break. Just make sure you follow conventional-commits to indicate the break

[jungseoklee]

Thanks for comments! I will prepare the next revision.

[rix0rrr]

I think this might better be called Attribute. Props is usually reserved for the set of parameters we use to instantiate a new class, which this is not.

[jungseoklee]

I will rename it in the next revision.

[rix0rrr]

Does it make sense for this type to still be called KeyAttributeType, since you're proposing to use Attribute also for things like nonKeyAttributes? Should it not be called AttributeType instead?

On an unrelated note: does it make sense to default it to AttributeType.String?

[jungseoklee]

Does it make sense for this type to still be called KeyAttributeType, since you're proposing to use Attribute also for things like nonKeyAttributes?

You're right. AttributeType is an appropriate name here.

On an unrelated note: does it make sense to default it to AttributeType.String?

That makes sense for partition key in most cases, but sort key is a bit different. AttributeType.Number is also widely used for query efficiency.