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

Add ApiGatewayExportsNullabilityExceptionIntegration #2056

Merged
merged 8 commits into from
Mar 23, 2023

Conversation

syall
Copy link
Contributor

@syall syall commented Mar 17, 2023

For changes to files under the /codegen/aws-models folder, and manual edits to autogenerated code (e.g. /service/s3/api.go) please create an Issue instead of a PR for those type of changes.

N/A.


If the PR addresses an existing bug or feature, please reference it here.

N/A.


To help speed up the process and reduce the time to merge please ensure that Allow edits by maintainers is checked before submitting your PR. This will allow the project maintainers to make minor adjustments or improvements to the submitted PR, allow us to reduce the roundtrip time for merging your request.


Overview

Due to internal model fixes for API Gateway (APIGW) exports, certain shapes
and members that target those shapes that used to have defaults became
nullable. In order to not break the existing Go v2 client API nullability,
this customization adds back default values for affected shapes:

  • Root Boolean shapes: false
  • Root Number shapes: 0
  • Snapshotted Members shapes: inherit defaults

The class of services affected are APIGW services and APIGW services that
migrated to Smithy, seen in APIGW_NULLABILITY_EXCEPTION_SERVICES.

A "snapshot" of root-level and member shapes are captured in
APIGW_exports_nullability_exceptions.json, a mapping of service shape IDs
to a list of affected root-level and member shapes.

Cases:

  • Snapshotted Root level:
    • If a snapshotted root level shape doesn't exist, an exception will be
      thrown.
    • Otherwise, a default will be patched if a default doesn't already exist
  • Snapshotted Member level:
    • If a snapshotted member level shape doesn't exist, an exception will be
      thrown.
    • Otherwise, a default will be patched if a default doesn't already exist
  • Nonsnapshotted Member level:
    • All nonsnapshotted member level shape that target a snapshotted root level
      will be identified and throw an error.
    • This is prevent breaking changes if a nonsnapshotted member changes target
      from a snapshotted root shape to a nonsnapshotted root shape, as the SDK
      has no prior info on either shape.

Also removes serviceid.smithy which was not used anywhere.

Testing

Surprisingly, Go v2 had NO codegen tests at all. This PR sets up the first codegen test.

  1. CI passes
  2. Add ApiGatewayExportsNullabilityExceptionIntegrationTest
  3. No diffs for code generation with existing service models when running make generate
  4. No diffs for code generation when stripping @default traits from the affected service models:
    • Strip @default traits from affected service models using the crusher.js script
    • Run make generate and confirm no codegen changes (empty commit)
    • Revert stripping @default traits

@syall syall force-pushed the apigw-exports-nullability-breakfix branch 2 times, most recently from 67cc313 to c48348f Compare March 20, 2023 23:48
@syall syall changed the title Implement ApiGatewayExportsNullabilityExceptionIntegration Add ApiGatewayExportsNullabilityExceptionIntegration Mar 20, 2023
@syall syall marked this pull request as ready for review March 21, 2023 00:01
@syall syall requested a review from a team as a code owner March 21, 2023 00:01
@syall syall requested a review from mtdowling March 21, 2023 00:05
@syall syall force-pushed the apigw-exports-nullability-breakfix branch from c48348f to e58b408 Compare March 21, 2023 01:16
@syall syall force-pushed the apigw-exports-nullability-breakfix branch 2 times, most recently from a2a4056 to 3c53a8d Compare March 22, 2023 20:45
@syall syall force-pushed the apigw-exports-nullability-breakfix branch from 3c53a8d to a3a9240 Compare March 22, 2023 23:43
Due to internal model fixes for API Gateway (APIGW) exports, certain shapes
and members that target those shapes that used to have defaults became
nullable. In order to not break the existing Go v2 client API nullability,
this customization adds back default values for affected shapes:

- Root Boolean shapes: false
- Root Number shapes: 0
- Snapshotted Members shapes: inherit defaults

The class of services affected are APIGW services and APIGW services that
migrated to Smithy, seen in `APIGW_NULLABILITY_EXCEPTION_SERVICES`.

A "snapshot" of root-level and member shapes are captured in
`APIGW_exports_nullability_exceptions.json`, a mapping of service shape IDs
to a list of affected root-level and member shapes.

Cases:
- Snapshotted Root level:
  - If a snapshotted root level shape doesn't exist, an exception will be
    thrown.
  - Otherwise, a default will be patched if a default doesn't already exist
- Snapshotted Member level:
  - If a snapshotted member level shape doesn't exist, an exception will be
    thrown.
  - Otherwise, a default will be patched if a default doesn't already exist
- Nonsnapshotted Member level:
  - All nonsnapshotted member level shape that target a snapshotted root level
    will be identified and throw an error.
  - This is prevent breaking changes if a nonsnapshotted member changes target
    from a snapshotted root shape to a nonsnapshotted root shape, as the SDK
    has no prior info on either shape.
@syall syall force-pushed the apigw-exports-nullability-breakfix branch from a3a9240 to a3e2e7c Compare March 23, 2023 21:02
@syall syall requested review from mtdowling and removed request for mtdowling March 23, 2023 21:12
@syall syall requested a review from aajtodd March 23, 2023 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants