-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(appsync): add caching config to AppSync resolvers #17815
Changes from 10 commits
9071e72
b76bc5f
9beecff
9a99d8b
63fad2e
e3d3c11
f53ac90
b413402
5be68e4
735a7c3
7f0130f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import { Duration } from '@aws-cdk/core'; | ||
|
||
/** | ||
* CachingConfig for AppSync resolvers | ||
*/ | ||
export interface CachingConfig { | ||
/** | ||
* The caching keys for a resolver that has caching enabled. | ||
* Valid values are entries from the $context.arguments, $context.source, and $context.identity maps. | ||
* | ||
* @default - No caching keys | ||
*/ | ||
readonly cachingKeys?: string[]; | ||
|
||
/** | ||
* The TTL in seconds for a resolver that has caching enabled. | ||
* Valid values are between 1 and 3600 seconds. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thing with this one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added validation + unit tests for ttl! |
||
* | ||
* @default - No TTL | ||
*/ | ||
readonly ttl?: Duration; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
export const CONTEXT_ARGUMENTS_CACHING_KEY = '$context.arguments'; | ||
export const CONTEXT_SOURCE_CACHING_KEY = '$context.source'; | ||
export const CONTEXT_IDENTITY_CACHING_KEY = '$context.identity'; | ||
export const BASE_CACHING_KEYS = [CONTEXT_ARGUMENTS_CACHING_KEY, CONTEXT_SOURCE_CACHING_KEY, CONTEXT_IDENTITY_CACHING_KEY]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
import * as path from 'path'; | ||
import { Template } from '@aws-cdk/assertions'; | ||
import * as lambda from '@aws-cdk/aws-lambda'; | ||
import * as cdk from '@aws-cdk/core'; | ||
import { Duration } from '@aws-cdk/core'; | ||
import * as appsync from '../lib'; | ||
|
||
let stack: cdk.Stack; | ||
let api: appsync.GraphqlApi; | ||
|
||
beforeEach(() => { | ||
// GIVEN | ||
stack = new cdk.Stack(); | ||
api = new appsync.GraphqlApi(stack, 'api', { | ||
name: 'api', | ||
schema: appsync.Schema.fromAsset(path.join(__dirname, 'appsync.lambda.graphql')), | ||
}); | ||
}); | ||
|
||
describe('Lambda caching config', () => { | ||
// GIVEN | ||
let func: lambda.Function; | ||
|
||
beforeEach(() => { | ||
func = new lambda.Function(stack, 'func', { | ||
code: lambda.Code.fromAsset(path.join(__dirname, 'verify/lambda-tutorial')), | ||
handler: 'lambda-tutorial.handler', | ||
runtime: lambda.Runtime.NODEJS_12_X, | ||
}); | ||
}); | ||
|
||
test('Lambda resolver contains caching config with caching key and TTL', () => { | ||
// WHEN | ||
const lambdaDS = api.addLambdaDataSource('LambdaDS', func); | ||
|
||
lambdaDS.createResolver({ | ||
typeName: 'Query', | ||
fieldName: 'allPosts', | ||
cachingConfig: { | ||
cachingKeys: ['$context.arguments', '$context.source', '$context.identity'], | ||
ttl: Duration.seconds(300), | ||
}, | ||
}); | ||
|
||
// THEN | ||
Template.fromStack(stack).hasResourceProperties('AWS::AppSync::Resolver', { | ||
TypeName: 'Query', | ||
FieldName: 'allPosts', | ||
CachingConfig: { | ||
CachingKeys: ['$context.arguments', '$context.source', '$context.identity'], | ||
Ttl: 300, | ||
}, | ||
}); | ||
}); | ||
|
||
test('Lambda resolver throws error when caching config with TTL is less than 1 second', () => { | ||
// WHEN | ||
const ttlInSconds = 0; | ||
const lambdaDS = api.addLambdaDataSource('LambdaDS', func); | ||
|
||
// THEN | ||
expect(() => { | ||
lambdaDS.createResolver({ | ||
typeName: 'Query', | ||
fieldName: 'allPosts', | ||
cachingConfig: { | ||
cachingKeys: ['$context.identity'], | ||
ttl: Duration.seconds(0), | ||
}, | ||
}); | ||
}).toThrowError(`Caching config TTL must be between 1 and 3600 seconds. Received: ${ttlInSconds}`); | ||
}); | ||
|
||
test('Lambda resolver throws error when caching config with TTL is greater than 3600 seconds', () => { | ||
// WHEN | ||
const ttlInSconds = 4200; | ||
const lambdaDS = api.addLambdaDataSource('LambdaDS', func); | ||
|
||
// THEN | ||
expect(() => { | ||
lambdaDS.createResolver({ | ||
typeName: 'Query', | ||
fieldName: 'allPosts', | ||
cachingConfig: { | ||
cachingKeys: ['$context.identity'], | ||
ttl: Duration.seconds(ttlInSconds), | ||
}, | ||
}); | ||
}).toThrowError(`Caching config TTL must be between 1 and 3600 seconds. Received: ${ttlInSconds}`); | ||
}); | ||
|
||
test('Lambda resolver throws error when caching config has invalid caching keys', () => { | ||
// WHEN | ||
const invalidCachingKeys = ['$context.metadata']; | ||
const lambdaDS = api.addLambdaDataSource('LambdaDS', func); | ||
|
||
// THEN | ||
expect(() => { | ||
lambdaDS.createResolver({ | ||
typeName: 'Query', | ||
fieldName: 'allPosts', | ||
cachingConfig: { | ||
cachingKeys: invalidCachingKeys, | ||
ttl: Duration.seconds(300), | ||
}, | ||
}); | ||
}).toThrowError(`Caching config keys must begin with $context.arguments, $context.source or $context.identity. Received: ${invalidCachingKeys}`); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please also add validation for this constraint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright - added validation for caching keys + unit test - added some constants for $context.source, $context.arguments and $context.identity to help validate that caching keys are prefixed by these keys. Technically, there's some VTL syntax that should be adhered to but I think that may be a bit much for CDK to take on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! One last thing: Before validating you should check whether the value is encoded as a token, using the
Token.isUnresolved()
static method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah cool - I'll make this change!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright sweet - done! Checked to see if the caching key is resolved first before validating.