aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.42k stars 2.12k forks source link

Amplify CLI Gen 2 - Multiple SortKeys combined into one input with GraphQL Generation #13408

Closed ChristopherGabba closed 4 months ago

ChristopherGabba commented 4 months ago

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

GraphQL API

Amplify Version

v6

Amplify Categories

api

Backend

Amplify Gen 2 (Preview)

Environment information

``` System: OS: macOS 14.4.1 CPU: (10) arm64 Apple M2 Pro Memory: 75.05 MB / 16.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 20.7.0 - /opt/homebrew/bin/node Yarn: 1.22.22 - /opt/homebrew/bin/yarn npm: 10.1.0 - /opt/homebrew/bin/npm Watchman: 2023.09.04.00 - /opt/homebrew/bin/watchman Browsers: Safari: 17.4.1 npmPackages: %name%: 0.1.0 @aws-amplify/backend: ^1.0.1 => 1.0.2 @aws-amplify/backend-cli: ^1.0.2 => 1.0.3 @aws-amplify/react-native: ^1.1.0 => 1.1.1 @aws-amplify/ui-react-native: ^2.2.0 => 2.2.2 @aws-appsync/utils: ^1.7.0 => 1.7.0 @babel/core: ^7.20.0 => 7.24.5 @babel/plugin-proposal-export-namespace-from: ^7.18.9 => 7.18.9 @babel/plugin-proposal-optional-chaining: ^7.0.0 => 7.21.0 @babel/plugin-transform-arrow-functions: ^7.0.0 => 7.24.1 @babel/plugin-transform-nullish-coalescing-operator: ^7.0.0 => 7.24.1 @babel/plugin-transform-shorthand-properties: ^7.0.0 => 7.24.1 @babel/plugin-transform-template-literals: ^7.0.0 => 7.24.1 @babel/preset-env: ^7.20.0 => 7.24.5 @babel/runtime: ^7.20.0 => 7.24.5 @config-plugins/ffmpeg-kit-react-native: ^8.0.0 => 8.0.0 @expo-google-fonts/m-plus-1p: ^0.2.3 => 0.2.3 @expo-google-fonts/montserrat: ^0.2.3 => 0.2.3 @expo/config-plugins: ~8.0.0 => 8.0.4 (7.9.2) @expo/metro-runtime: ~3.2.1 => 3.2.1 @gorhom/bottom-sheet: ^4.6.1 => 4.6.3 @react-native-async-storage/async-storage: ^1.23.1 => 1.23.1 @react-native-community/netinfo: 11.3.1 => 11.3.1 @react-navigation/bottom-tabs: ^6.5.20 => 6.5.20 @react-navigation/native: ^6.0.2 => 6.1.17 @react-navigation/native-stack: ^6.0.2 => 6.9.26 @sentry/react-native: ~5.22.0 => 5.22.2 @shopify/flash-list: 1.6.4 => 1.6.4 @types/i18n-js: 3.8.2 => 3.8.2 @types/jest: ^29.2.1 => 29.5.12 @types/lodash.filter: ^4.6.9 => 4.6.9 @types/react: ~18.2.14 => 18.2.79 (18.3.2) @types/react-test-renderer: ^18.0.0 => 18.3.0 @typescript-eslint/eslint-plugin: ^5.59.0 => 5.62.0 @typescript-eslint/parser: ^5.59.0 => 5.62.0 ContextAPIMixpanel: 0.0.1 HelloWorld: 0.0.1 MixpanelDemo: 0.0.1 SimpleMixpanel: 0.0.1 apisauce: 3.0.1 => 3.0.1 aws-amplify: ^6.3.1 => 6.3.2 aws-amplify/adapter-core: undefined () aws-amplify/analytics: undefined () aws-amplify/analytics/kinesis: undefined () aws-amplify/analytics/kinesis-firehose: undefined () aws-amplify/analytics/personalize: undefined () aws-amplify/analytics/pinpoint: undefined () aws-amplify/api: undefined () aws-amplify/api/server: undefined () aws-amplify/auth: undefined () aws-amplify/auth/cognito: undefined () aws-amplify/auth/cognito/server: undefined () aws-amplify/auth/enable-oauth-listener: undefined () aws-amplify/auth/server: undefined () aws-amplify/data: undefined () aws-amplify/data/server: undefined () aws-amplify/datastore: undefined () aws-amplify/in-app-messaging: undefined () aws-amplify/in-app-messaging/pinpoint: undefined () aws-amplify/push-notifications: undefined () aws-amplify/push-notifications/pinpoint: undefined () aws-amplify/storage: undefined () aws-amplify/storage/s3: undefined () aws-amplify/storage/s3/server: undefined () aws-amplify/storage/server: undefined () aws-amplify/utils: undefined () aws-cdk: ^2.141.0 => 2.142.1 aws-cdk-lib: ^2.141.0 => 2.142.1 babel-jest: ^29.2.1 => 29.7.0 cheerio: ^1.0.0-rc.12 => 1.0.0-rc.12 constructs: ^10.3.0 => 10.3.0 date-fns: ^2.30.0 => 2.30.0 esbuild: ^0.21.1 => 0.21.3 (0.20.2) eslint: 8.17.0 => 8.17.0 eslint-config-prettier: 8.5.0 => 8.5.0 eslint-config-standard: 17.0.0 => 17.0.0 eslint-plugin-import: 2.26.0 => 2.26.0 eslint-plugin-n: ^15.0.0 => 15.7.0 eslint-plugin-promise: 6.0.0 => 6.0.0 eslint-plugin-react: 7.30.0 => 7.30.0 eslint-plugin-react-native: 4.0.0 => 4.0.0 eslint-plugin-reactotron: ^0.1.2 => 0.1.4 ex: ^0.1.4 => 0.1.4 expo: ~51.0.6 => 51.0.8 expo-application: ~5.9.1 => 5.9.1 expo-av: ~14.0.4 => 14.0.5 expo-blur: ~13.0.2 => 13.0.2 expo-build-properties: ^0.12.1 => 0.12.1 expo-clipboard: ~6.0.3 => 6.0.3 expo-constants: ^16.0.1 => 16.0.1 expo-contacts: ~13.0.3 => 13.0.3 expo-dev-client: ~4.0.13 => 4.0.14 expo-device: ~6.0.2 => 6.0.2 expo-font: ~12.0.4 => 12.0.5 expo-haptics: ~13.0.1 => 13.0.1 expo-image: ~1.12.9 => 1.12.9 expo-image-picker: ~15.0.4 => 15.0.5 expo-linear-gradient: ~13.0.2 => 13.0.2 expo-linking: ~6.3.1 => 6.3.1 expo-localization: ~15.0.3 => 15.0.3 expo-notifications: ^0.28.1 => 0.28.3 expo-secure-store: ~13.0.1 => 13.0.1 expo-share-intent: ^2.0.0 => 2.0.0 expo-splash-screen: ^0.27.4 => 0.27.4 expo-status-bar: ~1.12.1 => 1.12.1 expo-store-review: ~7.0.2 => 7.0.2 expo-updates: ~0.25.12 => 0.25.14 expo-video-thumbnails: ~8.0.0 => 8.0.0 ffmpeg-kit-react-native: ^6.0.2 => 6.0.2 i18n-js: 3.9.2 => 3.9.2 jest: ^29.2.1 => 29.7.0 jest-expo: ~51.0.1 => 51.0.2 libphonenumber-js: ^1.11.1 => 1.11.1 (1.9.47) libphonenumber-js-core: undefined (1.0.0) libphonenumber-js-max: undefined (1.0.0) libphonenumber-js-min: undefined (1.0.0) libphonenumber-js-mobile: undefined (1.0.0) libphonenumber-js/build: undefined () libphonenumber-js/core: undefined () libphonenumber-js/max: undefined () libphonenumber-js/max/metadata: undefined () libphonenumber-js/min: undefined () libphonenumber-js/min/metadata: undefined () libphonenumber-js/mobile: undefined () libphonenumber-js/mobile/examples: undefined () libphonenumber-js/mobile/metadata: undefined () lodash: ^4.17.21 => 4.17.21 lodash.filter: ^4.6.0 => 4.6.0 lottie-react-native: 6.7.0 => 6.7.0 mixpanel-react-native: ^3.0.2 => 3.0.5 mixpanelexpo: 1.0.0 mobx: 6.10.2 => 6.10.2 mobx-react-lite: 4.0.5 => 4.0.5 mobx-state-tree: 5.3.0 => 5.3.0 patch-package: 6.4.7 => 6.4.7 postinstall-prepare: 1.0.1 => 1.0.1 prettier: 2.8.8 => 2.8.8 (2.3.2, 1.19.1) react: 18.2.0 => 18.2.0 react-dom: 18.2.0 => 18.2.0 react-native: 0.74.1 => 0.74.1 react-native-blurhash: ^2.0.2 => 2.0.2 react-native-compressor: ^1.8.24 => 1.8.24 react-native-context-menu-view: ^1.16.0 => 1.16.0 react-native-device-info: ^10.13.2 => 10.14.0 react-native-fs: ^2.20.0 => 2.20.0 react-native-gesture-handler: ~2.16.1 => 2.16.2 react-native-get-random-values: ^1.11.0 => 1.11.0 react-native-mime-types: ^2.5.0 => 2.5.0 react-native-mmkv: ^2.12.2 => 2.12.2 react-native-reanimated: ~3.10.1 => 3.10.1 react-native-safe-area-context: ^4.10.1 => 4.10.1 react-native-screens: 3.31.1 => 3.31.1 react-native-static-safe-area-insets: ^2.2.0 => 2.2.0 react-native-touchable-scale: ^2.2.0 => 2.2.0 react-native-url-polyfill: ^2.0.0 => 2.0.0 react-native-vision-camera: ^4.0.3 => 4.0.5 react-native-volume-manager: ^1.10.0 => 1.10.0 react-native-web: ~0.19.6 => 0.19.11 react-native-webview: 13.8.6 => 13.8.6 react-native-youtube-iframe: ^2.3.0 => 2.3.0 react-test-renderer: 18.2.0 => 18.2.0 reactotron-core-client: ^2.8.13 => 2.9.3 reactotron-mst: ^3.1.7 => 3.1.9 reactotron-react-js: ^3.3.11 => 3.3.14 reactotron-react-native: ^5.0.5 => 5.1.7 ts-jest: ^29.1.1 => 29.1.3 ts-node: ^10.9.2 => 10.9.2 tsx: ^4.9.4 => 4.10.5 typescript: ~5.3.3 => 5.3.3 (4.4.4, 4.9.5) uuid: ^9.0.1 => 9.0.1 (8.3.2, 3.3.2, 7.0.3) npmGlobalPackages: @aws-amplify/cli-internal: 12.12.0 @aws-amplify/cli: 12.11.0 @react-native-community/netinfo: 9.4.1 eas-cli: 9.0.8 expo-cli: 6.3.10 firebase-tools: 11.24.1 n: 9.1.0 node-gyp: 10.0.1 node: 20.6.0 npm: 10.7.0 pod-install: 0.2.2 react-native-spinkit: 1.5.1 typescript: 5.4.5 yarn: 1.22.22 ```

Describe the bug

If you define a schema with a secondaryIndex that contains two sortKeys and then you run: npx ampx generate graphql-client-code --statement-max-depth 3 --out app/services/api/AWS

It generates graphql code for you to use. However, the sortKeys are merged into a single input for some reason.

  User: a
    .model({
      id: a.id().required(),
      birthdate: a.string().required(),
      firstName: a.string().required(),
      lastName: a.string().required(),
      username: a.string().required(),
      searchTerm: a.string().required(),
      sentFriendships: a.hasMany("Friendship", "senderId"),
      receivedFriendships: a.hasMany("Friendship", "receiverId"),
    })
    .secondaryIndexes((index) => [
      index("searchTerm").queryField("listUsersBySearchTerm").sortKeys(["firstName", "lastName"]), // two sort keys
    ])
    .authorization((allow) => [allow.publicApiKey()]),
// Using new method works correctly
const response = await client.models.User.listUsersBySearchTerm({
    searchTerm: "ABCD"
    firstName: { eq: "Chris" },
    lastName: { eq: "Gabba" },
})
// using graphql generated method they are combined for some reason.
const response = await client.graphql({
  query: listUsersBySearchTerm,
  variables: {
     input: {
        searchTerm: "ABCD",
        firstNameLastName: //. ?????? these should not be combined into a single string
     }
  }
})

Also another issue: The generatedGraphql client.graphql does not allow the user to set a "ne" filter on the sortKey but the new method using client.models.User.listUsersBySearchTerm will let me us ne as a filter.

Expected behavior

There should be separate inputs generated for each sortKey.

Reproduction steps

See steps above.

Code Snippet

// Put your code below this line.

Log output

``` // Put your logs below this line ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

iPhone 12 Physical

Mobile Operating System

iOS 17

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

See above.

chrisbonifacio commented 4 months ago

Hi @ChristopherGabba, long time no see! 😃 thanks for raising this issue.

I was able to reproduce the sort keys being combined into one field on the listBy query and the missing ne filter:

image

The client.models API does split up the sort keys into separate fields and includes the ne filter for both fields:

image image

Interestingly, this seems to have been a fixed behavior in the client.models API as well. An older version of @aws-amplify/backend@1.0.0 had the same issue:

image

Marking this as a bug for the team to investigate further

chrisbonifacio commented 4 months ago

Apparently the sort keys are also combined on the AppSync console, the firstName and lastName fields are just nested within the operators:

image

Same is true on the client.graphql API:

image

So the client.models API might actually be exposing more operators than it should.

iartemiev commented 4 months ago

What you're seeing in the client-side API client is expected behavior. The sort key is combined intentionally because that's how the GraphQL index query arguments are generated. We simply match those input shapes in the client.

One thing to keep in mind here is how the sort key is actually evaluated at runtime. Due to how DynamoDB applies sort key predicates, it's impossible to evaluate each field separately. The operands you provide are combined into a single string and DDB compares the combined operand against the values in the table lexicographically. See this response for more detail: https://github.com/aws-amplify/amplify-category-api/issues/802#issuecomment-1944423269

chrisbonifacio commented 4 months ago

Piggybacking off of @iartemiev 's comment. The original issue described is expected behavior so I will close this issue.

We'll track the separate issue of the mismatch between the GSI operator typing on the data client and the graphql input here:

https://github.com/aws-amplify/amplify-js/issues/13444

ChristopherGabba commented 4 months ago

Great, thank you both. You may just need to update the documentation with an example using multiple sort keys so that this is clear to the developers when they do use this option. Maybe that's already there and I just missed it... Thanks again though.