-
Notifications
You must be signed in to change notification settings - Fork 57
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
feature(defer): Partial and Incremental Parsing #217
feature(defer): Partial and Incremental Parsing #217
Conversation
✅ Deploy Preview for eclectic-pie-88a2ba canceled.
|
712589c
to
9d54318
Compare
…and-incremental-execution # Conflicts: # Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift
Get specific about what values are expected rather than simply .success
@AnthonyMDev I think this PR is finally ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just reviewed the production code, and had a few very minor comments. I'm going to review the tests shortly as well!
) | ||
} | ||
|
||
func execute< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like we could really still just use the one execution function above if we just change it from needed RootSelectionSet
to just SelectionSet
. It doesn't look like there is any other actual deviation in functionality between these two functions. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I've made a note that we should chat about it this week and if there are changes I'll make a separate PR for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests LGTM! Just a couple minor questions.
Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplateTests.swift
Show resolved
Hide resolved
Co-authored-by: Anthony Miller <[email protected]>
9e7af06
into
feature/defer-execution-networking
Closes apollographql/apollo-ios#3277
Closes apollographql/apollo-ios#3145
Closes apollographql/apollo-ios#3147
To recap the progress so far:
main
has some defer changes merged for codegen but they are disabledfeature/defer-execution-networking
is where I'm collecting the rest of the changes that will enable preview releases.This PR brings parsing of the initial response (partial) and subsequent responses (incremental) to the defer implementation. The changeset includes changes to the templates and rendering but mostly are focused on the execution and networking layers. Details below:
hasDeferredFragments
from the Operation definition. This was only used for the request HTTP headers but that behaviour has changed too and operations now get adeferredFragments
property to help with incremental execution.Deferrable
conformance was moved to theInlineFragment
declaration to match the definition ofFragment
. This allows us to clean up some of the inline fragment code generation.IncrementalJSONResponseParsingInterceptor
is a new interceptor built to handle incremental payloads and combining them with previously parsed results..deferred
selection set is encountered because those fields will not be present in the response data, but the deferred fragment is marked for future collection. Incremental execution will then pick up the deferred fragments fields for collection when the relevant response is received.GraphQLExecutor
now has two entry points for execution; one used for the initial partial response and the other used during incremental execution parsing. They both converge in subsequent execution calls but the difference is in the generic constraints of the function and which selection set they operate on -RootSelectionSet
orSelectionSet
.GraphQLResponse
has been broken up into a base struct namedAnyGraphQLResponse
which provides common logic betweenGraphQLResponse
, used for partial responses, andIncrementalGraphQLResponse
which is used for incremental response parsing.DataDictMapper
was broken out ofGraphQLSelectionSetMapper
and handles the conversion of JSON data into theDataDict
format.GraphQLResult
now has the ability to merge in data from anIncrementalGraphQLResult
. This is used during incremental response parsing and is required because of the way the interceptors require a single 'complete' result to be passed into the call toproceedAsync
. The partial and incremental results are built upon each other during each incremental response storing the 'completed' response in the incremental interceptor which is unique to each operation request.PathEntry
fromGraphQLError
was broken out into it's own type for reuse in the incremental execution.@defer
. This will come in a later PR.There are no tests included with this PR yet, they're still coming.Last of the tests outstanding is for incremental execution.