Fix top level hydration #572
Conversation
| .filterNot { | ||
| it.fieldName == TypeNameMetaFieldDef.name | ||
| } | ||
| .firstOrNull() ?: topLevelFields.first(), |
There was a problem hiding this comment.
This gets the first non __typename field, to maintain the old behavior.
(Refer to above comment).
| @@ -343,9 +343,16 @@ internal class NextgenEngine( | |||
| serviceContext = executionContext.getContextForService(service).await(), | |||
| serviceExecutionContext = serviceExecutionContext, | |||
| hydrationDetails = executionHydrationDetails, | |||
| executableNormalizedField = transformedQuery, | |||
| // Prefer non __typename field first, otherwise we just get first | |||
| executableNormalizedField = topLevelFields | |||
There was a problem hiding this comment.
So this ServiceExecutionParameters.executableNormalizedField actually needs to be deprecated and removed but we are keeping it for now for compatibility.
It's currently used to extract the cloud ID for routing.
The reason why we went from ExecutableNormalizedField to List<ExecutableNormalizedField> is because the root level hydration transform injects a __typename field.
{
batch_hydration__myIssues__myIssueKeys: myIssueKeys
__typename__batch_hydration__myIssues: __typename
}You could argue that we don't need a __typename for the operation types, but it's not a big deal to just keep it in.
Plus in the future we may end up grouping more root level fields into one execution, so this sort of lays the ground work for that.
| } | ||
|
|
||
| private fun onlyTopLevelTypenameField(executableNormalizedField: ExecutableNormalizedField): Boolean { | ||
| if (executableNormalizedField.fieldName == TypeNameMetaFieldDef.name) { | ||
| private fun isOnlyTopLevelFieldTypename(topLevelFields: List<ExecutableNormalizedField>): Boolean { |
There was a problem hiding this comment.
This code here was modified a bit.
Context: we wanted to add code that would shortcut query execution IF there was only one field and it was __typename
Note: this code is protected by a FF hints.shortCircuitEmptyQuery(service) and it's mostly off because it hasn't been fully rolled out.
|
|
||
| val service = definitionNamesToService[overallType.name] | ||
| ?: coordinatesToService[fieldCoordinates] | ||
| ?: error("Unable to determine service for $fieldCoordinates") |
There was a problem hiding this comment.
Main fix is here.
Before we would do type name -> service.
This doesn't work for operation types.
So we now fall back to field name -> service.
gnawf
force-pushed
the
fix-top-level-hydration
branch
from
c7c7813
to
7de5eb6
Compare
So the bug was that we couldn't find the service that owned a hydrated field, because we looked at the ownership of the parent type of the hydrated field, which for root level hydrations was the operation type.
Then on top of that, we didn't handle multple root level fields.
This PR fixes both.