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

Improved pipeline handling #296

Merged
merged 7 commits into from Jan 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 16 additions & 8 deletions README.md
Expand Up @@ -125,8 +125,11 @@ custom:
- dataSource: # data source name
type: # type name in schema (e.g. Query, Mutation, Subscription)
field: getUserInfo
request: # request mapping template name
response: # response mapping template name
# kind: UNIT (default, not required) or PIPELINE (required for pipeline resolvers)
functions: # array of functions if kind === 'PIPELINE'
- # function name
request: # request mapping template name | defaults to {type}.{field}.request.vtl
response: # response mapping template name | defaults to {type}.{field}.response.vtl
# When caching is enaled with `PER_RESOLVER_CACHING`,
# the caching options of the resolver.
# Disabled by default.
Expand All @@ -143,6 +146,11 @@ custom:
ttl: 1000 # override the ttl for this resolver. (default comes from global config)

- ${file({fileLocation}.yml)} # link to a file with arrays of mapping templates
functionConfigurations:
- name: # function name
dataSource: # data source name
request: # request mapping template name | defaults to {name}.request.vtl
response: # reponse mapping template name | defaults to {name}.response.vtl
dataSources:
- type: AMAZON_DYNAMODB
name: # data source name
Expand Down Expand Up @@ -276,21 +284,21 @@ custom:
mappingTemplates:
- type: Query
field: testPipelineQuery
request: './mapping-templates/before.vtl' # the pipeline's "before" mapping template
response: './mapping-templates/after.vtl' # the pipeline's "after" mapping template
request: './mapping-templates/before.vtl' # the pipeline's "before" mapping template, defaults to {type}.{field).request.vtl
response: './mapping-templates/after.vtl' # the pipeline's "after" mapping template, defaults to {type}.{field}.response.vtl
kind: PIPELINE
functions:
- authorizeFunction
- fetchDataFunction
functionConfigurations:
- dataSource: graphqlLambda
name: 'authorizeFunction'
request: './mapping-templates/authorize-request.vtl'
response: './mapping-templates/common-response.vtl'
request: './mapping-templates/authorize-request.vtl' # defaults to {name}.request.vtl
response: './mapping-templates/common-response.vtl' # defaults to {name}.response.vtl
- dataSource: dataTable
name: 'fetchDataFunction'
request: './mapping-templates/fetchData.vtl'
response: './mapping-templates/common-response.vtl'
request: './mapping-templates/fetchData.vtl' # defaults to {name}.request.vtl
response: './mapping-templates/common-response.vtl' # defaults to {name}.response.vtl
```

## ▶️ Usage
Expand Down
4 changes: 2 additions & 2 deletions src/index.js
Expand Up @@ -825,11 +825,11 @@ class ServerlessAppsyncPlugin {
return flattenedFunctionConfigurationResources.reduce((acc, tpl) => {
const reqTemplPath = path.join(
functionConfigLocation,
tpl.request || `${tpl.type}.${tpl.field}.request.vtl`,
tpl.request || `${tpl.name}.request.vtl`,
);
const respTemplPath = path.join(
functionConfigLocation,
tpl.response || `${tpl.type}.${tpl.field}.response.vtl`,
tpl.response || `${tpl.name}.response.vtl`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@markvp I understand you point.
Thinking about this now, I guess that the previous implementation did not really make sense
However, this should still be considered a breaking change.
Can we add backward compatibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that functions don't have a type or a field, I'm not sure how anyone could have used this functionality in the past? I certainly couldn't get it to work.
Given that, I don't really see this is a breaking change as much as it is a fix of unusable code (no offence!)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@markvp After giving it a second thought and having a deeper analysis on this, I think you are right.
I got that those fields are indeed not present in pipeline definitions, but I thought that someone might have used them anyway (as a hack) in order to auto-generate the filename. But I realize that it would make no sense, since you could just place the filename directly instead.

I'll accept this change 👍

);
const requestTemplate = fs.readFileSync(reqTemplPath, 'utf8');
const responseTemplate = fs.readFileSync(respTemplPath, 'utf8');
Expand Down