Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
284 changes: 206 additions & 78 deletions packages/amplify-graphql-api-construct/.jsii

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,18 @@ exports[`Directive Definitions IndexDirective 1`] = `
Object {
"defaults": Object {},
"definition": "
directive @index(name: String, sortKeyFields: [String], queryField: String) repeatable on FIELD_DEFINITION
directive @index(name: String, sortKeyFields: [String], queryField: String, projection: ProjectionInput) repeatable on FIELD_DEFINITION

input ProjectionInput {
type: ProjectionType!
nonKeyAttributes: [String]
}

enum ProjectionType {
ALL
KEYS_ONLY
INCLUDE
}
",
"name": "index",
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,18 @@ import { Directive } from './directive';

const name = 'index';
const definition = /* GraphQL */ `
directive @${name}(name: String, sortKeyFields: [String], queryField: String) repeatable on FIELD_DEFINITION
directive @${name}(name: String, sortKeyFields: [String], queryField: String, projection: ProjectionInput) repeatable on FIELD_DEFINITION

input ProjectionInput {
type: ProjectionType!
nonKeyAttributes: [String]
}

enum ProjectionType {
ALL
KEYS_ONLY
INCLUDE
}
`;
const defaults = {};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1607,3 +1607,98 @@ describe('auth', () => {
expect(out.resolvers['Query.testsByDescription.postAuth.1.res.vtl']).toMatchSnapshot();
});
});

test('@index with KEYS_ONLY projection creates GSI with correct projection type', () => {
const inputSchema = `
type Product @model {
id: ID!
name: String! @index(name: "byName", queryField: "productsByName", projection: { type: KEYS_ONLY })
category: String!
price: Float!
}`;
const out = testTransform({
schema: inputSchema,
transformers: [new ModelTransformer(), new IndexTransformer()],
});
const stack = out.stacks.Product;

AssertionTemplate.fromJSON(stack).hasResourceProperties('AWS::DynamoDB::Table', {
GlobalSecondaryIndexes: [
{
IndexName: 'byName',
Projection: {
ProjectionType: 'KEYS_ONLY',
},
},
],
});
});

test('@index with INCLUDE projection creates GSI with nonKeyAttributes', () => {
const inputSchema = `
type Product @model {
id: ID!
name: String!
category: String! @index(name: "byCategory", queryField: "productsByCategory", projection: { type: INCLUDE, nonKeyAttributes: ["name", "price"] })
price: Float!
inStock: Boolean!
}`;
const out = testTransform({
schema: inputSchema,
transformers: [new ModelTransformer(), new IndexTransformer()],
});
const stack = out.stacks.Product;

AssertionTemplate.fromJSON(stack).hasResourceProperties('AWS::DynamoDB::Table', {
GlobalSecondaryIndexes: [
{
IndexName: 'byCategory',
Projection: {
ProjectionType: 'INCLUDE',
NonKeyAttributes: ['name', 'price'],
},
},
],
});
});

test('@index with ALL projection creates GSI with ALL projection type', () => {
const inputSchema = `
type Product @model {
id: ID!
name: String!
category: String! @index(name: "byCategory", queryField: "productsByCategory", projection: { type: ALL })
price: Float!
}`;
const out = testTransform({
schema: inputSchema,
transformers: [new ModelTransformer(), new IndexTransformer()],
});
const stack = out.stacks.Product;

AssertionTemplate.fromJSON(stack).hasResourceProperties('AWS::DynamoDB::Table', {
GlobalSecondaryIndexes: [
{
IndexName: 'byCategory',
Projection: {
ProjectionType: 'ALL',
},
},
],
});
});

test('@index throws error when INCLUDE projection has no nonKeyAttributes', () => {
const inputSchema = `
type Product @model {
id: ID!
category: String! @index(name: "byCategory", projection: { type: INCLUDE })
}`;

expect(() =>
testTransform({
schema: inputSchema,
transformers: [new ModelTransformer(), new IndexTransformer()],
}),
).toThrow("@index 'byCategory': nonKeyAttributes must be specified when projection type is INCLUDE");
});
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ export class IndexTransformer extends TransformerPluginBase {
object: parent as ObjectTypeDefinitionNode,
field: definition,
directive,
} as IndexDirectiveConfiguration,
} as Required<IndexDirectiveConfiguration>,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Quick question - just trying to understand why we're setting this as Required<> when IndexDirectiveConfiguration properties are defined as optional. Maybe some comments could be added to explain this decision in the file

generateGetArgumentsInput(context.transformParameters),
);
) as Required<IndexDirectiveConfiguration>;

/**
* Impute Optional Fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ export const validateSortDirectionInput = (config: PrimaryKeyDirectiveConfigurat
* appendSecondaryIndex
*/
export const appendSecondaryIndex = (config: IndexDirectiveConfiguration, ctx: TransformerContextProvider): void => {
const { name, object, primaryKeyField } = config;
const { name, object, primaryKeyField, projection } = config;
if (isSqlModel(ctx, object.name.value)) {
return;
}
Expand All @@ -382,11 +382,19 @@ export const appendSecondaryIndex = (config: IndexDirectiveConfiguration, ctx: T
const partitionKeyType = attrDefs.find((attr) => attr.attributeName === partitionKeyName)?.attributeType ?? 'S';
const sortKeyType = sortKeyName ? attrDefs.find((attr) => attr.attributeName === sortKeyName)?.attributeType ?? 'S' : undefined;

const projectionType = projection?.type || 'ALL';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to check if this is intended.

js allows any false values with the || operator. That is, if we had an undefined value, then it would default to ALL (as we want). However, empty or invalid strings, or null values would also end up defaulting to ALL from my understanding. Do we want to add checks for correct types?

const nonKeyAttributes = projection?.nonKeyAttributes || [];

if (projectionType === 'INCLUDE' && (!nonKeyAttributes || nonKeyAttributes.length === 0)) {
throw new Error(`@index '${name}': nonKeyAttributes must be specified when projection type is INCLUDE`);
}

if (!ctx.transformParameters.secondaryKeyAsGSI && primaryKeyPartitionKeyName === partitionKeyName) {
// Create an LSI.
table.addLocalSecondaryIndex({
indexName: name,
projectionType: 'ALL',
projectionType,
...(projectionType === 'INCLUDE' ? { nonKeyAttributes } : {}),
sortKey: sortKeyName
? {
name: sortKeyName,
Expand All @@ -398,7 +406,8 @@ export const appendSecondaryIndex = (config: IndexDirectiveConfiguration, ctx: T
// Create a GSI.
table.addGlobalSecondaryIndex({
indexName: name,
projectionType: 'ALL',
projectionType,
...(projectionType === 'INCLUDE' ? { nonKeyAttributes } : {}),
partitionKey: {
name: partitionKeyName,
type: partitionKeyType,
Expand All @@ -419,7 +428,7 @@ export const appendSecondaryIndex = (config: IndexDirectiveConfiguration, ctx: T
const newIndex = {
indexName: name,
keySchema,
projection: { projectionType: 'ALL' },
projection: { projectionType, ...(projectionType === 'INCLUDE' ? { nonKeyAttributes } : {}) },
provisionedThroughput: cdk.Fn.conditionIf(ResourceConstants.CONDITIONS.ShouldUsePayPerRequestBilling, cdk.Fn.ref('AWS::NoValue'), {
ReadCapacityUnits: cdk.Fn.ref(ResourceConstants.PARAMETERS.DynamoDBModelTableReadIOPS),
WriteCapacityUnits: cdk.Fn.ref(ResourceConstants.PARAMETERS.DynamoDBModelTableWriteIOPS),
Expand Down
4 changes: 4 additions & 0 deletions packages/amplify-graphql-index-transformer/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ export type IndexDirectiveConfiguration = PrimaryKeyDirectiveConfiguration & {
name: string | null;
queryField: string | null;
primaryKeyField: FieldDefinitionNode;
projection?: {
type: 'ALL' | 'KEYS_ONLY' | 'INCLUDE';
nonKeyAttributes?: string[];
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ const usePascalCaseForObjectKeys = (obj: { [key: string]: any }): { [key: string
const value = obj[key];

if (Array.isArray(value)) {
result[capitalizedKey] = value.map((v) => usePascalCaseForObjectKeys(v));
result[capitalizedKey] = value.map((v) => (typeof v === 'object' && v !== null ? usePascalCaseForObjectKeys(v) : v));
} else if (typeof value === 'object' && value !== null) {
// If the value is an object, recursively capitalize its keys
result[capitalizedKey] = usePascalCaseForObjectKeys(value);
Expand Down
Loading