Implement 2-step introspection #5371
Conversation
✅ Deploy Preview for apollo-android-docs canceled.
|
| // be robust to errors: [] keys | ||
| ignoreUnknownKeys = true |
There was a problem hiding this comment.
What happens if a field errors (very unlikely to happen for an introspection query but 🤷 )? Should we output an error instead?
There was a problem hiding this comment.
If you get an error here, something is very wrong, so it would make sense to fail early I think.
| @Serializable | ||
| private class MIntrospection( | ||
| val data: MData | ||
| ) | ||
|
|
||
| @Serializable | ||
| private class MData( | ||
| @Suppress("PropertyName") | ||
| val __schema: MSchema | ||
| ) | ||
|
|
||
| @Serializable | ||
| private class MSchema( | ||
| val types: List<MType>, | ||
| ) | ||
|
|
||
| @Serializable | ||
| private class MType( | ||
| val name: String, | ||
| val fields: List<MField>?, | ||
| ) | ||
|
|
||
| @Serializable | ||
| private class MField( | ||
| val name: String, | ||
| val args: List<MInputValue>, | ||
| ) | ||
|
|
||
| @Serializable | ||
| private class MInputValue( | ||
| val name: String, | ||
| ) |
There was a problem hiding this comment.
For the meta introspection query, you could use Apollo and generate all of that automatically.
There was a problem hiding this comment.
Totally, I was in a "don't use Apollo" mindset, but it makes sense to use it here actually - the code will be simpler 🎉
| # Base introspection query that will be augmented according to the server capabilities. | ||
| # The capabilities are retrieved by executing the meta-introspection query next to this file. | ||
| # The fields that can be added or modified are shown as comments. |
There was a problem hiding this comment.
Can you add a comment ~what version of the spec this corresponds to?
| __schema { | ||
| types { | ||
| name | ||
| fields { | ||
| name | ||
| args { | ||
| name | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Download only the required types?
query MetaIntrospectionQuery {
__type(name: "__Schema") { ...typeDetails }
__type(name: "__Type") { ...typeDetails }
__type(name: "__Directive") { ...typeDetails }
__type(name: "__Field") { ...typeDetails }
__type(name: "__InputValue") { ...typeDetails }
}It's not so much about the performance improvements but the fact that it conveys better what we're going to do with this data?
There was a problem hiding this comment.
Didn't realize it was allowed! Will update 👍
| class SchemaDownloaderTests { | ||
| private lateinit var mockServer: MockServer | ||
| private lateinit var tempFile: File | ||
| private const val metaSchemaJune2018Response = """ |
There was a problem hiding this comment.
Nit: can those json files be moved to separate test fixture files so that we have less scrolling to do?
| mockServer.enqueueString(statusCode = 400) | ||
| mockServer.enqueueString(statusCode = 400) | ||
| mockServer.enqueueString(schemaString1) | ||
| fun `schema is downloaded correctly when server supports Draft spec as of 2023-11-15`() = runTest(before = { setUp() }, after = { tearDown() }) { |
There was a problem hiding this comment.
@oneOf is not in the draft yet so it's more like server supports @oneOf
There was a problem hiding this comment.
Looks great! 👍
One last comment about bikeshedding the name 😃 . I have mixed feelings about "meta introspection query" as "meta" is quite the loaded term. The only suggestion I have in mind is "step1 introspection query" which isn't great either so I'm fine keeping this "meta" for now. Up to you.
Agree meta wasn't great - I changed it to "pre-introspection query" 😁 LMKWYT |
| /** | ||
| * `__Type.inputFields`, `includeDeprecated` argument, introduced in [Draft](https://spec.graphql.org/draft/) | ||
| */ | ||
| TypeInputFieldsIncludeDeprecated, | ||
|
|
||
| /** | ||
| * `__Directive.args`, `includeDeprecated` argument, introduced in [Draft](https://spec.graphql.org/draft/) | ||
| */ | ||
| DirectiveArgsIncludeDeprecated, | ||
|
|
||
| /** | ||
| * `__Field.args`, `includeDeprecated` argument, introduced in [Draft](https://spec.graphql.org/draft/) | ||
| */ | ||
| FieldArgsIncludeDeprecated, | ||
|
|
||
| /** | ||
| * `__InputValue.isDeprecated`, introduced in [Draft](https://spec.graphql.org/draft/) | ||
| */ | ||
| InputValueIsDeprecated, | ||
|
|
||
| /** | ||
| * `__InputValue.deprecationReason`, introduced in [Draft](https://spec.graphql.org/draft/) | ||
| */ | ||
| InputValueDeprecatedReason, |
There was a problem hiding this comment.
I think these are all the same? Does it make sense to include one but not the others?
There was a problem hiding this comment.
More generally, the naming should be about "features" and not introspection names. Maybe SpecifiedBy, OneOf, DeprecatedInputValues, RepeatableDirectives, ...?
There was a problem hiding this comment.
In 830615e I've renamed the enum to GraphQLFeature, and made it less granular.
| val metaSchemaMockResponse = MockResponse().setBody(preIntrospectionResponse) | ||
| val schemaMockResponse = MockResponse().setBody(schemaString1) | ||
| mockServer.enqueue(metaSchemaMockResponse) | ||
| mockServer.enqueue(schemaMockResponse) |
There was a problem hiding this comment.
metaSchemaMockResponse -> preIntrospectionResponse
BoD
force-pushed
the
2-step-introspection
branch
from
85e52c8 to
69e7634
Compare
The Gradle plugin is no longer used for that part, but still present and used by the Apollo Platform API related operations (registry, persisted queries, field insights, telemetry).