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

[DRAFT] feat(v1): lazy loading custom selection set POC - implementation only #2711

Closed
wants to merge 1 commit into from

Conversation

lawmicha
Copy link
Member

Issue #

Description

General Checklist

  • Added new tests to cover change, if needed
  • Build succeeds with all target using Swift Package Manager
  • All unit tests pass
  • All integration tests pass
  • Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)
  • Documentation update for the change if required
  • PR title conforms to conventional commit style
  • New or updated tests include Given When Then inline code documentation and are named accordingly testThing_condition_expectation()
  • If breaking change, documentation/changelog update with migration instructions

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

Comment on lines 173 to +177
authType: AWSAuthorizationType? = nil) -> GraphQLRequest<MutationSyncResult> {

var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelSchema: modelSchema,
operationType: .subscription)
operationType: .subscription,
primaryKeysOnly: true)
Copy link
Member Author

Choose a reason for hiding this comment

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

The subscription API in GraphQLRequest+AnyModelWithSync will pass in primaryKeysOnly true initially. This is reverted back to false if it detects that the models do not have the lazy loading enabled. When lazy loading is enabled, then the selection set will be reduced to just the primary keys in the nested model, thus supporting mutation events sourced from other platforms.

While testing Flutter with this backport, DataStore.start() will establish the subscriptions. The first step is to make sure the subscription's selection sets are reduced as expected.

Comment on lines +93 to +120
// The Swift DataStore plugin has a dependency on using `__typename` from the response data.
// For example, DataStore will use the type `MutationSync<AnyModel>` to decode the model with sync metadata by
// pulling out the modelName from `__typename` and use that to decode to the actual model type.
// The selection set created by other platforms such as JS and Android does not contain the `__typename`
// in the mutation request, which is one of the reasons for the decoding error when the mutation is sent from
// Studio/JS/Android.
//
// This code injects the typename field at runtime for response payloads specifically used by DataStore,
// `MutationSync<AnyModel>`, so that that the JS/Android sourced subscription events decode successfully.
//
// Since we are injecting the typename into the response payload, do we still need to request it from the service?
// Yes, because mutations sourced from the new iOS clients sent to old iOS clients should be able to decode
// successfully. We're enabling new use cases (successful decoding of Studio/JS/Android sourced mutations) but
// should not break iOS upgrade use cases without a major version bump. iOS users that haven't upgraded to the
// latest version of the developer's app will continue to work because the the mutation request sent from the
// latest library continues to have the typename field.
private func shouldAddTypename(to graphQLData: JSONValue) -> JSONValue? {
if let modelName = modelName,
request.responseType == MutationSync<AnyModel>.self,
case var .object(modelJSON) = graphQLData,
// No need to replace existing response payloads that have it already
modelJSON["__typename"] == nil {
modelJSON["__typename"] = .string(modelName)
return JSONValue.object(modelJSON)
} else {
return nil
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the second piece of logic that makes cross platform events successful, see in-line comment

@lawmicha lawmicha changed the title feat(v1): lazy loading custom selection set POC - implementation only [DRAFT] feat(v1): lazy loading custom selection set POC - implementation only Feb 6, 2023
@HuiSF
Copy link

HuiSF commented Mar 29, 2023

From a few quick test runs, the custom selection set change eliminates the subscription GraphQL error described in this issue: aws-amplify/amplify-flutter#663 . A create mutation originated from an Android device, can correctly trigger a onCreate event with correct payload delivered to the iOS App.

image

I'm looking forward to finalizing this draft PR so we can resolve the linked issue for amplify-flutter developers and developers who develop Apps communicate between iOS and Android.

@lawmicha
Copy link
Member Author

lawmicha commented Nov 6, 2023

Closing for #2944

@lawmicha lawmicha closed this Nov 6, 2023
@lawmicha lawmicha deleted the v1-data-dev-preview branch November 6, 2023 13:09
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

2 participants