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

fix: add modelgen support for granular read ops #537

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AaronZyLee
Copy link
Contributor

@AaronZyLee AaronZyLee commented Jan 31, 2023

Description of changes

Added granular modelgen support for Java
For the other libraries, the generated code has no problem and only library support is needed

Issue #, if available

aws-amplify/amplify-flutter#2526

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • Breaking changes to existing customers are released behind a feature flag or major version update
  • Changes are tested using sample applications for all relevant platforms (iOS/android/flutter/Javascript) that use the feature added/modified

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

@AaronZyLee AaronZyLee requested a review from a team as a code owner January 31, 2023 00:39
@codecov-commenter
Copy link

codecov-commenter commented Jan 31, 2023

Codecov Report

Merging #537 (a54ddaa) into main (c6c7b7a) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main     #537   +/-   ##
=======================================
  Coverage   85.69%   85.69%           
=======================================
  Files         148      148           
  Lines        7380     7380           
  Branches     1962     1962           
=======================================
  Hits         6324     6324           
  Misses        959      959           
  Partials       97       97           
Impacted Files Coverage Δ
...delgen-plugin/src/visitors/appsync-java-visitor.ts 94.80% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

list: 'ModelOperation.LIST',
sync: 'ModelOperation.SYNC',
listen: 'ModelOperation.LISTEN',
search: 'ModelOperation.SEARCH',
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we changing the behavior for only appsync-java-visitor or all other visitors as well? I assume this generateAuthRules method is common to all visitors? If that's the case, can we move this to a common visitor? Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for java visitor only. It maps the input operation to the library-defined enum value. If not added here, the code in java will be generated as empty string.
For the other code generators, these ops will be simply parsed and generated in the final code and do not need additional mapping. You can see this in the snapshot changes for other visitors

Copy link
Contributor Author

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

3 participants