feat(aws-apigateway): expand RestApi support to models, parameters and validators by julienlepine · Pull Request #2960 · aws/aws-cdk

julienlepine · 2019-06-20T13:25:14Z

Fixes #905: "apigateway: "methodResponses" is missing from MethodOptions"
Fixes #1695: apigateway: missing support for models
Fixes #727: API Gateway: improve API for request parameters and responses
Fixes #723: API Gateway: missing features
Fixes #2957: RestApi to use logical id as a name for APIs instead of name of current construct
Adds support for JsonSchema in Model
Aligns Model to the PhysicalName convention.
No breaking change, documentation updated

Pull Request Checklist

Testing
- Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
- CLI change?: coordinate update of integration tests with team
- cdk-init template change?: coordinated update of integration tests with team
Docs
- jsdocs: All public APIs documented
- README: README and/or documentation topic updated
- Design: For significant features, design document added to design folder
Title and Description
- Change type: title prefixed with fix, feat and module name in parens, which will appear in changelog
- Title: use lower-case and doesn't end with a period
- Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
- Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
Sensitive Modules (requires 2 PR approvers)
- IAM Policy Document (in @aws-cdk/aws-iam)
- EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
- Grant APIs (only if not based on official documentation with a reference)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

…d validators Fixes #905: "apigateway: "methodResponses" is missing from MethodOptions" Fixes #1695: apigateway: missing support for models Fixes #727: API Gateway: improve API for request parameters and responses Fixes #723: API Gateway: missing features Fixes #2957: RestApi to use logical id as a name for APIs instrea of name of current construct

…epine/aws-cdk into julienlepine/apigw-request-model

julienlepine · 2019-06-21T18:44:03Z

re-aligned with the changes to have the PhysicalName as the default name of the construct, merged with the incoming changes.

NGL321 · 2019-06-21T21:13:42Z

Hi Julien! Thank you so much for contributing. We are working hard to stabilize the CDK APIs and tuning them to meet our consistency guidelines. While we work on getting the APIs aligned with our guidelines, we are pausing work on most community PRs starting today. Please continue to report issues and submit feature requests, of course. We expect to get back to work on community PRs within a few weeks.

packages/@aws-cdk/aws-apigateway/lib/json-schema.ts

packages/@aws-cdk/aws-apigateway/lib/method.ts

packages/@aws-cdk/aws-apigateway/lib/model.ts

packages/@aws-cdk/aws-apigateway/lib/requestvalidator.ts

julienlepine · 2019-06-24T20:24:05Z

Re merged with the latest master commit, and adressed PR comments.

julienlepine · 2019-06-25T13:57:36Z

Merged with 0.36

eladb

Looks good, getting there.

A few notes:

We can't introduce breaking changes any more to any of the APIs, so we'll have to figure out the right way to maintain compat.
Check out the new pattern we have for allowing physical names.

packages/@aws-cdk/aws-apigateway/lib/model.ts

eladb · 2019-06-25T14:12:13Z

packages/@aws-cdk/aws-apigateway/lib/model.ts

+  public readonly modelName: string;
+
+  constructor(scope: Construct, id: string, props: ModelProps) {
+    super(scope, id);


Follow the new pattern to support physical names (see s3.Bucket for example).

packages/@aws-cdk/aws-apigateway/lib/requestvalidator.ts

Use PhysicalName constructs

julienlepine · 2019-06-25T16:36:22Z

Updated based on comments

eladb

Getting there! Super high quality contribution. Thanks so much for all the effort.

packages/@aws-cdk/aws-apigateway/lib/model.ts

eladb · 2019-06-26T07:20:13Z

packages/@aws-cdk/aws-apigateway/lib/model.ts

+export interface ModelOptions {
+  /**
+   * The content type for the model.
+   * @default None


Can you elaborate in the docs what does it mean not to provide a content type?

Also, the value in @default should be “-“ to indicate it’s not a “real” value

packages/@aws-cdk/aws-apigateway/lib/model.ts

packages/@aws-cdk/aws-apigateway/lib/json-schema.ts

eladb · 2019-06-26T07:29:45Z

packages/@aws-cdk/aws-apigateway/lib/requestvalidator.ts

+      physicalName: props.requestValidatorName,
+    });
+
+    const validatorProps: CfnRequestValidatorProps = {


This needs to be “this.physicalName”

Done for Model.ts, for RequestValidator the name is actually kind of useless per se, name is not a property of the CfnRequestValidator, and it's really an Id.

packages/@aws-cdk/aws-apigateway/lib/requestvalidator.ts

eladb · 2019-06-26T07:31:51Z

packages/@aws-cdk/aws-apigateway/test/test.restapi.ts


    test.done();
  },
+


What about request validation?

Completing migration to PhysicalName Moved JsonSchemaMapper to internal class in util.ts Removed dependency to IConstruct on IModel (and added linter exclusion in package.json)

eladb

Looks great. Please merge from master, update PR description and let me know when you are ready for merge.

packages/@aws-cdk/aws-apigateway/lib/model.ts

julienlepine · 2019-06-26T14:52:47Z

Merged and pushed

julienlepine · 2019-06-26T20:28:01Z

Ready to be merged to master.

eladb · 2019-06-26T21:38:18Z

Can you please update the PR description so we’ll use it for the merge commit?

julienlepine added 2 commits June 20, 2019 14:48

julienlepine requested a review from a team as a code owner June 20, 2019 13:25

julienlepine added 3 commits June 20, 2019 18:50

Merge branch 'julienlepine/apigw-request-model' of github.com:julienl…

32cdcc1

…epine/aws-cdk into julienlepine/apigw-request-model

Merge from upstream/master, and update from breaking changes.

9f68b9c

Merge with awslabs/aws-cdk master

c4b1ec7

Merge branch 'master' into julienlepine/apigw-request-model

52a10c4

eladb reviewed Jun 24, 2019

View reviewed changes

julienlepine added 2 commits June 24, 2019 22:21

Merge and address PR Review comments.

20a6367

Merge back from remote

373a9ba

Merge branch 'master' into julienlepine/apigw-request-model

c3e6267

eladb reviewed Jun 25, 2019

View reviewed changes

Move back to modelId

c5603b8

Use PhysicalName constructs

julienlepine and others added 2 commits June 25, 2019 18:36

Merge branch 'master' into julienlepine/apigw-request-model

a7264b6

Change modelName to modelId in ERROR_MODEL and EMPTY_MODEL

da87366

eladb suggested changes Jun 26, 2019

View reviewed changes

re-implement ErrorModel and EmptyModel classes.

31138b7

Completing migration to PhysicalName Moved JsonSchemaMapper to internal class in util.ts Removed dependency to IConstruct on IModel (and added linter exclusion in package.json)

eladb approved these changes Jun 26, 2019

View reviewed changes

packages/@aws-cdk/aws-apigateway/lib/model.ts Show resolved Hide resolved

Merge branch 'master' into julienlepine/apigw-request-model

7a8ecbb

julienlepine and others added 2 commits June 26, 2019 16:54

@deprecated to old entries

a036082

Merge branch 'master' into julienlepine/apigw-request-model

ff1cf4a

Merge branch 'master' into julienlepine/apigw-request-model

3d8e7fc

This was referenced Aug 22, 2019

[Snyk] Upgrade @aws-cdk/aws-ec2 from 0.18.1 to 0.39.0 udinachmany/trivia-api#3

Open

[Snyk] Upgrade @aws-cdk/aws-ecr from 0.18.1 to 0.39.0 udinachmany/trivia-api#4

Open

[Snyk] Upgrade @aws-cdk/aws-ecs from 0.18.1 to 0.39.0 udinachmany/trivia-api#5

Open

This was referenced Jan 20, 2020

[Snyk] Upgrade @aws-cdk/aws-route53 from 0.22.0 to 0.39.0 MechanicalRock/tech-radar#20

Closed

[Snyk] Upgrade @aws-cdk/aws-s3 from 0.22.0 to 0.39.0 MechanicalRock/tech-radar#21

Closed

NOUIY mentioned this pull request Oct 2, 2024

[Snyk] Upgrade @aws-cdk/aws-apigateway from 0.0.0 to 0.39.0 NOUIY/aws-solutions-constructs#132

Open

Conversation

julienlepine commented Jun 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Uh oh!

julienlepine commented Jun 21, 2019

Uh oh!

NGL321 commented Jun 21, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

julienlepine commented Jun 24, 2019

Uh oh!

julienlepine commented Jun 25, 2019

Uh oh!

eladb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

julienlepine commented Jun 25, 2019

Uh oh!

eladb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eladb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

julienlepine commented Jun 26, 2019

Uh oh!

julienlepine commented Jun 26, 2019

Uh oh!

eladb commented Jun 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

julienlepine commented Jun 20, 2019 •

edited

Loading