search

awslabs / aws-mobile-appsync-sdk-ios

iOS SDK for AWS AppSync.
https://awslabs.github.io/aws-mobile-appsync-sdk-ios/
Other
262 stars 130 forks source link

fix: deserialization of custom scalars #536

Closed jellybeansoup closed 2 years ago

jellybeansoup commented 2 years ago

Issue #, if available: None.

Description of changes: We use a number of custom scalars with AWS AppSync in our codebase, which are successfully parsed from the AppSync service, and get stored within the SQLite cache, but fail to be deserialised and thus cause cache reads involving these types to completely fail. This has primarily meant that we are unable to cache these calls at all, relying on live data for calls relying on our custom scalar types.

This is exactly the same as an issue raised on the Apollo library in 2018, and was resolved by returning the fieldJSONValue from the SQLiteSerialization.deserialize(fieldJSONValue:), which correctly returns the custom scalar value as-is (in this PR).

I've replicated the changes here, including the removal of the then defunct AWSAppSyncQueriesCacheError.invalidRecordValue case.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

atierian commented 2 years ago

Thanks for opening this PR (and the other one). We'll review and come back to you on this.

jellybeansoup commented 2 years ago

Let's verify integration tests are passing as expected (SDK owners can assist in running this if it's difficult for developer contributor to perform this)

I'd certainly appreciate it if someone could assist; I had trouble getting the appropriate AWS environment set up (the lambda instructions in particular), but I did verify that the other tests run and pass as expected.

lawmicha commented 2 years ago

Let's verify integration tests are passing as expected (SDK owners can assist in running this if it's difficult for developer contributor to perform this)

I'd certainly appreciate it if someone could assist; I had trouble getting the appropriate AWS environment set up (the lambda instructions in particular), but I did verify that the other tests run and pass as expected.

Ran the integration tests and they're all passing

jellybeansoup commented 2 years ago

Thanks for running the integration tests on my behalf, @lawmicha! I've cherry-picked the commit to return the removed error case as suggested, so it should be good for another look 😊