Skip to content

Commit

Permalink
fix(appsync): unstable IDs on resolvers and functions
Browse files Browse the repository at this point in the history
Fixes an issue that would cause unexpected resource replacement for
appsync resolvers and functions because of construct nesting and ID
generation.

Changes `createResolver` and `createFunction` methods on `GraphQlApi`
and `DataSource` constructs to require explicitly passing an ID.
Additionally changes the scope of the constructs created in
`createResolver` and `createFunction` on the `DataSource` construct to
be `this.api` instead of `this`. This allows users to change the data
sources of resolvers and functions while keeping the IDs stable and
avoiding resource replacement.

This helps to avoid the `only one resolver per field` error that occurs
when deleting a resolver on a field, and adding a new one within the
same deployment.

BREAKING CHANGE: `DataSource.createResolver`,
`DataSource.createFunction`, and `GraphQlApi.createResolver` now require
2 arguments instead of 1.

Fixes: #13269
  • Loading branch information
MrArnoldPalmer committed Dec 13, 2022
1 parent 6e4b4ea commit da3d9bf
Show file tree
Hide file tree
Showing 43 changed files with 2,816 additions and 2,311 deletions.
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-appsync/lib/data-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ export abstract class BaseDataSource extends Construct {
/**
* creates a new resolver for this datasource and API using the given properties
*/
public createResolver(props: BaseResolverProps): Resolver {
return new Resolver(this, `${props.typeName}${props.fieldName}Resolver`, {
public createResolver(id: string, props: BaseResolverProps): Resolver {
return new Resolver(this.api, id, {
api: this.api,
dataSource: this,
...props,
Expand All @@ -143,8 +143,8 @@ export abstract class BaseDataSource extends Construct {
/**
* creates a new appsync function for this datasource and API using the given properties
*/
public createFunction(props: BaseAppsyncFunctionProps): AppsyncFunction {
return new AppsyncFunction(this, `${props.name}Function`, {
public createFunction(id: string, props: BaseAppsyncFunctionProps): AppsyncFunction {
return new AppsyncFunction(this.api, id, {
api: this.api,
dataSource: this,
...props,
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-appsync/lib/graphqlapi-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export interface IGraphqlApi extends IResource {
/**
* creates a new resolver for this datasource and API using the given properties
*/
createResolver(props: ExtendedResolverProps): Resolver;
createResolver(id: string, props: ExtendedResolverProps): Resolver;

/**
* Add schema dependency if not imported
Expand Down Expand Up @@ -285,8 +285,8 @@ export abstract class GraphqlApiBase extends Resource implements IGraphqlApi {
/**
* creates a new resolver for this datasource and API using the given properties
*/
public createResolver(props: ExtendedResolverProps): Resolver {
return new Resolver(this, `${props.typeName}${props.fieldName}Resolver`, {
public createResolver(id: string, props: ExtendedResolverProps): Resolver {
return new Resolver(this, id, {
api: this,
...props,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('Lambda caching config', () => {
// WHEN
const lambdaDS = api.addLambdaDataSource('LambdaDS', func);

lambdaDS.createResolver({
lambdaDS.createResolver('QueryAllPosts', {
typeName: 'Query',
fieldName: 'allPosts',
});
Expand All @@ -50,7 +50,7 @@ describe('Lambda caching config', () => {
// WHEN
const lambdaDS = api.addLambdaDataSource('LambdaDS', func);

lambdaDS.createResolver({
lambdaDS.createResolver('QueryAllPosts', {
typeName: 'Query',
fieldName: 'allPosts',
cachingConfig: {
Expand All @@ -77,7 +77,7 @@ describe('Lambda caching config', () => {

// THEN
expect(() => {
lambdaDS.createResolver({
lambdaDS.createResolver('QueryAllPosts', {
typeName: 'Query',
fieldName: 'allPosts',
cachingConfig: {
Expand All @@ -95,7 +95,7 @@ describe('Lambda caching config', () => {

// THEN
expect(() => {
lambdaDS.createResolver({
lambdaDS.createResolver('QueryAllPosts', {
typeName: 'Query',
fieldName: 'allPosts',
cachingConfig: {
Expand All @@ -113,7 +113,7 @@ describe('Lambda caching config', () => {

// THEN
expect(() => {
lambdaDS.createResolver({
lambdaDS.createResolver('QueryAllPosts', {
typeName: 'Query',
fieldName: 'allPosts',
cachingConfig: {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-appsync/test/appsync-lambda.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe('Lambda Data Source configuration', () => {
description: 'custom description',
});

ds.createResolver({
ds.createResolver('TestField', {
typeName: 'test',
fieldName: 'field',
});
Expand Down Expand Up @@ -164,4 +164,4 @@ describe('adding lambda data source from imported api', () => {
ApiId: { 'Fn::GetAtt': ['baseApiCDA4D43A', 'ApiId'] },
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Lambda Mapping Templates', () => {
// WHEN
const lambdaDS = api.addLambdaDataSource('LambdaDS', func);

lambdaDS.createResolver({
lambdaDS.createResolver('QueryAllPosts', {
typeName: 'Query',
fieldName: 'allPosts',
requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(),
Expand All @@ -52,7 +52,7 @@ describe('Lambda Mapping Templates', () => {
// WHEN
const lambdaDS = api.addLambdaDataSource('LambdaDS', func);

lambdaDS.createResolver({
lambdaDS.createResolver('PostRelatedPosts', {
typeName: 'Post',
fieldName: 'relatedPosts',
requestMappingTemplate: appsync.MappingTemplate.lambdaRequest('$util.toJson($ctx)', 'BatchInvoke'),
Expand All @@ -67,4 +67,4 @@ describe('Lambda Mapping Templates', () => {
MaxBatchSize: 10,
});
});
});
});
21 changes: 11 additions & 10 deletions packages/@aws-cdk/aws-appsync/test/appsync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ beforeEach(() => {
test('appsync should configure pipeline when pipelineConfig has contents', () => {
// WHEN
const ds = api.addNoneDataSource('none');
const test1 = ds.createFunction({
const test1 = ds.createFunction('Test1Function', {
name: 'test1',
});
const test2 = ds.createFunction({
const test2 = ds.createFunction('Test2Function', {
name: 'test2',
});
api.createResolver({
api.createResolver('TestTest2', {
typeName: 'test',
fieldName: 'test2',
pipelineConfig: [test1, test2],
Expand All @@ -38,8 +38,8 @@ test('appsync should configure pipeline when pipelineConfig has contents', () =>
Kind: 'PIPELINE',
PipelineConfig: {
Functions: [
{ 'Fn::GetAtt': ['apinonetest1FunctionEF63046F', 'FunctionId'] },
{ 'Fn::GetAtt': ['apinonetest2Function615111D0', 'FunctionId'] },
{ 'Fn::GetAtt': ['apiTest1Function793605E9', 'FunctionId'] },
{ 'Fn::GetAtt': ['apiTest2FunctionB704A7AD', 'FunctionId'] },
],
},
});
Expand All @@ -48,16 +48,17 @@ test('appsync should configure pipeline when pipelineConfig has contents', () =>
test('appsync should error when creating pipeline resolver with data source', () => {
// WHEN
const ds = api.addNoneDataSource('none');
const test1 = ds.createFunction({
const test1 = ds.createFunction('Test1Function', {
name: 'test1',
});
const test2 = ds.createFunction({
const test2 = ds.createFunction('Test2Function', {
name: 'test2',
});

// THEN
expect(() => {
ds.createResolver({
api.createResolver('TestTest2', {
dataSource: ds,
typeName: 'test',
fieldName: 'test2',
pipelineConfig: [test1, test2],
Expand All @@ -83,7 +84,7 @@ test('appsync should configure resolver as unit when pipelineConfig is empty', (

test('appsync should configure resolver as unit when pipelineConfig is empty array', () => {
// WHEN
api.createResolver({
api.createResolver('TestTest2', {
typeName: 'test',
fieldName: 'test2',
pipelineConfig: [],
Expand Down Expand Up @@ -237,4 +238,4 @@ test('log retention should not appear when no retention time is specified', () =

// THEN
Template.fromStack(stack).resourceCountIs('Custom::LogRetention', 0);
});
});
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-appsync/test/integ.api-import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ const testTable = new db.Table(stack, 'TestTable', {

const testDS = api.addDynamoDbDataSource('ds', testTable);

testDS.createResolver({
testDS.createResolver('QueryGetTests', {
typeName: 'Query',
fieldName: 'getTests',
requestMappingTemplate: appsync.MappingTemplate.dynamoDbScanTable(),
responseMappingTemplate: appsync.MappingTemplate.dynamoDbResultList(),
});

testDS.createResolver({
testDS.createResolver('MutationAddTest', {
typeName: 'Mutation',
fieldName: 'addTest',
requestMappingTemplate: appsync.MappingTemplate.dynamoDbPutItem(appsync.PrimaryKey.partition('id').auto(), appsync.Values.projecting('test')),
Expand All @@ -64,7 +64,7 @@ const api2 = appsync.GraphqlApi.fromGraphqlApiAttributes(stack, 'api2', {

const none = api2.addNoneDataSource('none');

const func = none.createFunction({
const func = none.createFunction('PipelineFunction', {
name: 'pipeline_function',
requestMappingTemplate: appsync.MappingTemplate.fromString(JSON.stringify({
version: '2017-02-28',
Expand All @@ -87,4 +87,4 @@ new appsync.Resolver(stack, 'pipeline_resolver', {
})),
});

app.synth();
app.synth();
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-appsync/test/integ.appsync-lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,40 +47,40 @@ const requestPayload = (field: string, { withArgs = false, withSource = false })
};
const responseMappingTemplate = appsync.MappingTemplate.lambdaResult();

lambdaDS.createResolver({
lambdaDS.createResolver('QueryGetPost', {
typeName: 'Query',
fieldName: 'getPost',
requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('getPost', { withArgs: true })),
responseMappingTemplate,
});

lambdaDS.createResolver({
lambdaDS.createResolver('QueryAllPosts', {
typeName: 'Query',
fieldName: 'allPosts',
requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('allPosts', {})),
responseMappingTemplate,
});

lambdaDS.createResolver({
lambdaDS.createResolver('MutationAddPost', {
typeName: 'Mutation',
fieldName: 'addPost',
requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('addPost', { withArgs: true })),
responseMappingTemplate,
});

lambdaDS.createResolver({
lambdaDS.createResolver('PostRelatedPosts', {
typeName: 'Post',
fieldName: 'relatedPosts',
requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('relatedPosts', { withSource: true }), 'BatchInvoke'),
responseMappingTemplate,
});

lambdaDS.createResolver({
lambdaDS.createResolver('PostRelatedPostsMaxBatchSize', {
typeName: 'Post',
fieldName: 'relatedPostsMaxBatchSize',
requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('relatedPostsMaxBatchSize', { withSource: true }), 'BatchInvoke'),
responseMappingTemplate,
maxBatchSize: 2,
});

app.synth();
app.synth();
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "21.0.0",
"version": "22.0.0",
"files": {
"b0462850439179659920597f4327262b24073af4f4969622163b0a295fce1dda": {
"de7d932209c6d07c0ba0e631387676246a8182018b2b7423dc18b52baec3e984": {
"source": {
"path": "aws-appsync-integ.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "b0462850439179659920597f4327262b24073af4f4969622163b0a295fce1dda.json",
"objectKey": "de7d932209c6d07c0ba0e631387676246a8182018b2b7423dc18b52baec3e984.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
}
}
},
"ApitestDataSourceQuerygetTestsResolverA3BBB672": {
"ApiQueryGetTestsF8C40170": {
"Type": "AWS::AppSync::Resolver",
"Properties": {
"ApiId": {
Expand All @@ -142,7 +142,7 @@
"ApitestDataSource96AE54D5"
]
},
"ApitestDataSourceMutationaddTestResolver36203D6B": {
"ApiMutationAddTestBF148084": {
"Type": "AWS::AppSync::Resolver",
"Properties": {
"ApiId": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"21.0.0"}
{"version":"22.0.0"}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "21.0.0",
"version": "22.0.0",
"testCases": {
"integ.auth-apikey": {
"stacks": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "21.0.0",
"version": "22.0.0",
"artifacts": {
"aws-appsync-integ.assets": {
"type": "cdk:asset-manifest",
Expand All @@ -17,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/b0462850439179659920597f4327262b24073af4f4969622163b0a295fce1dda.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/de7d932209c6d07c0ba0e631387676246a8182018b2b7423dc18b52baec3e984.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -69,16 +69,16 @@
"data": "ApitestDataSource96AE54D5"
}
],
"/aws-appsync-integ/Api/testDataSource/QuerygetTestsResolver/Resource": [
"/aws-appsync-integ/Api/QueryGetTests/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "ApitestDataSourceQuerygetTestsResolverA3BBB672"
"data": "ApiQueryGetTestsF8C40170"
}
],
"/aws-appsync-integ/Api/testDataSource/MutationaddTestResolver/Resource": [
"/aws-appsync-integ/Api/MutationAddTest/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "ApitestDataSourceMutationaddTestResolver36203D6B"
"data": "ApiMutationAddTestBF148084"
}
],
"/aws-appsync-integ/TestTable/Resource": [
Expand All @@ -98,6 +98,24 @@
"type": "aws:cdk:logicalId",
"data": "CheckBootstrapVersion"
}
],
"ApitestDataSourceQuerygetTestsResolverA3BBB672": [
{
"type": "aws:cdk:logicalId",
"data": "ApitestDataSourceQuerygetTestsResolverA3BBB672",
"trace": [
"!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
]
}
],
"ApitestDataSourceMutationaddTestResolver36203D6B": [
{
"type": "aws:cdk:logicalId",
"data": "ApitestDataSourceMutationaddTestResolver36203D6B",
"trace": [
"!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
]
}
]
},
"displayName": "aws-appsync-integ"
Expand Down
Loading

0 comments on commit da3d9bf

Please sign in to comment.