Skip to content
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

feat(appsync): remove addResolver from AppsyncResolver and lazy template evaluation #275

Merged
merged 7 commits into from
Jun 20, 2022
112 changes: 52 additions & 60 deletions src/appsync.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as appsync from "@aws-cdk/aws-appsync-alpha";
import { aws_appsync, Lazy } from "aws-cdk-lib";
import type { AppSyncResolverEvent } from "aws-lambda";
import { Construct } from "constructs";
import {
Expand Down Expand Up @@ -32,7 +33,12 @@ import {
import { findDeepIntegration, IntegrationImpl } from "./integration";
import { Literal } from "./literal";
import { FunctionlessNode } from "./node";
import { AnyFunction, isInTopLevelScope, singletonConstruct } from "./util";
import {
AnyFunction,
isInTopLevelScope,
memoize,
singletonConstruct,
} from "./util";
import { visitEachChild } from "./visit";
import { VTL } from "./vtl";

Expand Down Expand Up @@ -131,6 +137,18 @@ export class AppsyncVTL extends VTL {
}
}

export interface AppsyncResolverProps<
Arguments extends ResolverArguments,
Result,
Source = undefined
> extends Pick<
appsync.BaseResolverProps,
"typeName" | "fieldName" | "cachingConfig"
> {
readonly api: appsync.GraphqlApi;
readonly resolve: ResolverFunction<Arguments, Result, Source>;
}

/**
* An AWS AppSync Resolver Function derived from TypeScript syntax.
*
Expand Down Expand Up @@ -170,7 +188,7 @@ export class AppsyncResolver<
Arguments extends ResolverArguments,
Result,
Source = undefined
> {
> extends Construct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't we just make the StepFunction not a Construct/Resource? Maybe AppSync should wrap and not be too? What is the value of it itself being a construct? Will we ever want to call an app sync resolver from lambda?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure we would ever call a resolver directly. I don't think it's even possible? We can execute graphql queries but not individual resolvers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess if there is value in this being a construct, it is fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm also fine with moving it to resource. Keep things consistent.

/**
* This static property identifies this class as an AppsyncResolver to the TypeScript plugin.
*/
Expand All @@ -180,62 +198,36 @@ export class AppsyncResolver<
ResolverFunction<Arguments, Result, Source>
>;

constructor(fn: ResolverFunction<Arguments, Result, Source>) {
this.decl = validateFunctionDecl(fn, "AppsyncResolver");
}
public readonly resource;
public readonly resolvers;

/**
* Generate and add an AWS Appsync Resolver to an AWS Appsync GraphQL API.
*
* ```ts
* import * as appsync from "@aws-cdk/aws-appsync-alpha";
*
* const api = new appsync.GraphQLApi(..);
*
* ```ts
* const getPerson = new AppsyncResolver<{id: string}, Person | undefined>(
* ($context, id) => {
* const person = table.get({
* key: {
* id: $util.toDynamoDB(id)
* }
* });
* return person;
* });
* ```
*
* getPerson.createResolver(api, {
* typeName: "Query",
* fieldName: "getPerson"
* });
* ```
*
* @param api the AWS Appsync API to add this Resolver to
* @param options typeName, fieldName and cachingConfig for this Resolver.
* @returns a reference to the generated {@link appsync.Resolver}.
*/
public addResolver(
api: appsync.GraphqlApi,
options: Pick<
appsync.BaseResolverProps,
"typeName" | "fieldName" | "cachingConfig"
>
): SynthesizedAppsyncResolver {
const fields = this.getResolvableFields(api);

return new SynthesizedAppsyncResolver(
api,
`${options.typeName}${options.fieldName}Resolver`,
{
...options,
api,
templates: fields.templates,
dataSource: fields.dataSource,
requestMappingTemplate: fields.requestMappingTemplate,
responseMappingTemplate: fields.responseMappingTemplate,
pipelineConfig: fields.pipelineConfig,
}
);
constructor(
scope: Construct,
id: string,
props: AppsyncResolverProps<Arguments, Result, Source>
sam-goodwin marked this conversation as resolved.
Show resolved Hide resolved
) {
sam-goodwin marked this conversation as resolved.
Show resolved Hide resolved
super(scope, id);
this.decl = validateFunctionDecl(props.resolve, "AppsyncResolver");

this.resolvers = memoize(() => this.synthResolvers(props.api));

this.resource = new aws_appsync.CfnResolver(this, "Resource", {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why CfnResolver and not he L2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because the L2 doesn't expose IResolvable which I need to support lazy evaluation. Without lazy evaluation, users are forced to order things eagerly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense, I think we'll need to do this in other places.

apiId: props.api.apiId,
typeName: props.typeName,
fieldName: props.fieldName,
requestMappingTemplate: Lazy.string({
produce: () => this.resolvers().requestMappingTemplate.renderTemplate(),
}),
pipelineConfig: {
functions: Lazy.list({
produce: () => this.resolvers().templates,
}),
},
responseMappingTemplate: Lazy.string({
produce: () =>
this.resolvers().responseMappingTemplate.renderTemplate(),
}),
});
}

/**
Expand Down Expand Up @@ -279,11 +271,11 @@ export class AppsyncResolver<
appsync.ResolvableFieldOptions,
| "returnType"
| keyof ReturnType<
AppsyncResolver<Arguments, Result, Source>["getResolvableFields"]
AppsyncResolver<Arguments, Result, Source>["synthResolvers"]
>
>
): appsync.ResolvableField {
const fields = this.getResolvableFields(api);
const fields = this.synthResolvers(api);

return new appsync.ResolvableField({
...options,
Expand All @@ -295,7 +287,7 @@ export class AppsyncResolver<
});
}

private getResolvableFields(api: appsync.GraphqlApi) {
private synthResolvers(api: appsync.GraphqlApi) {
const resolverCount = countResolvers(this.decl);

const [pipelineConfig, responseMappingTemplate, innerTemplates] =
Expand Down
39 changes: 35 additions & 4 deletions src/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,45 @@ export function compile(
}

function visitAppsyncResolver(call: ts.NewExpression): ts.Node {
if (call.arguments?.length === 1) {
const impl = call.arguments[0];
if (ts.isFunctionExpression(impl) || ts.isArrowFunction(impl)) {
if (call.arguments?.length === 3) {
const [scope, id, props] = call.arguments;
if (ts.isObjectLiteralExpression(props)) {
sam-goodwin marked this conversation as resolved.
Show resolved Hide resolved
return ts.factory.updateNewExpression(
call,
call.expression,
call.typeArguments,
[errorBoundary(() => toFunction("FunctionDecl", impl, 1))]
[
scope,
id,
ts.factory.updateObjectLiteralExpression(
props,
props.properties.map((prop) => {
if (ts.isPropertyAssignment(prop)) {
if (
(ts.isIdentifier(prop.name) &&
prop.name.text === "resolve") ||
(ts.isStringLiteral(prop.name) &&
prop.name.text === "resolve")
) {
sam-goodwin marked this conversation as resolved.
Show resolved Hide resolved
const impl = prop.initializer;
if (
ts.isFunctionExpression(impl) ||
ts.isArrowFunction(impl)
) {
sam-goodwin marked this conversation as resolved.
Show resolved Hide resolved
return ts.factory.updatePropertyAssignment(
prop,
prop.name,
errorBoundary(() =>
sam-goodwin marked this conversation as resolved.
Show resolved Hide resolved
toFunction("FunctionDecl", impl, 1)
)
);
}
}
}
return prop;
})
),
]
);
}
}
Expand Down
18 changes: 18 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,24 @@ import { FunctionlessNode } from "./node";

export type AnyFunction = (...args: any[]) => any;

/**
* Create a memoized function.
*
* @param f the function that produces the value
* @returns a function that computes a value on demand at most once.
*/
export function memoize<T>(f: () => T): () => T {
let isComputed = false;
let t: T;
return () => {
if (!isComputed) {
t = f();
isComputed = true;
}
return t!;
};
}

sam-goodwin marked this conversation as resolved.
Show resolved Hide resolved
export function isInTopLevelScope(expr: FunctionlessNode): boolean {
if (expr.parent === undefined) {
return true;
Expand Down
28 changes: 2 additions & 26 deletions test-app/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,8 @@ const api = new appsync.GraphqlApi(stack, "Api", {
});

// @ts-ignore
const peopleDb = new PeopleDatabase(stack, "PeopleDB");

peopleDb.getPerson.addResolver(api, {
typeName: "Query",
fieldName: "getPerson",
});

peopleDb.addPerson.addResolver(api, {
typeName: "Mutation",
fieldName: "addPerson",
});

// add a duplicate addPerson API to test duplicates
peopleDb.addPerson.addResolver(api, {
typeName: "Mutation",
fieldName: "addPerson2",
});

peopleDb.updateName.addResolver(api, {
typeName: "Mutation",
fieldName: "updateName",
});

peopleDb.deletePerson.addResolver(api, {
typeName: "Mutation",
fieldName: "deletePerson",
const peopleDb = new PeopleDatabase(stack, "PeopleDB", {
api,
});

interface MyEventDetails {
Expand Down