Skip to content

Commit 9e204c0

Browse files
committed
rework handling of internal typename placeholder
1 parent c4be2e9 commit 9e204c0

3 files changed

Lines changed: 91 additions & 11 deletions

File tree

execution/engine/execution_engine_test.go

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1606,7 +1606,7 @@ func TestExecutionEngine_Execute(t *testing.T) {
16061606
expectedHost: "example.com",
16071607
expectedPath: "/",
16081608
expectedBody: "",
1609-
sendResponseBody: `{"data":{"__internal__typename_placeholder":"Query"}}`,
1609+
sendResponseBody: `doesn't matter, no fetch will be done, as query typename resolved by engine`,
16101610
sendStatusCode: 200,
16111611
}),
16121612
),
@@ -1644,6 +1644,82 @@ func TestExecutionEngine_Execute(t *testing.T) {
16441644
expectedResponse: `{"data":{}}`,
16451645
}))
16461646

1647+
t.Run("execute operation with all nested fields skipped", runWithoutError(ExecutionEngineTestCase{
1648+
schema: func(t *testing.T) *graphql.Schema {
1649+
t.Helper()
1650+
schema := `
1651+
type Query {
1652+
hero(name: String!): Hero!
1653+
}
1654+
1655+
type Hero {
1656+
name: String!
1657+
}
1658+
`
1659+
parseSchema, err := graphql.NewSchemaFromString(schema)
1660+
require.NoError(t, err)
1661+
return parseSchema
1662+
}(t),
1663+
operation: func(t *testing.T) graphql.Request {
1664+
return graphql.Request{
1665+
OperationName: "MyHero",
1666+
Variables: []byte(`{"heroName": "Luke"}`),
1667+
Query: `query MyHero($heroName: String!){
1668+
hero(name: $heroName) {
1669+
name @skip(if: true)
1670+
}
1671+
}`,
1672+
}
1673+
},
1674+
dataSources: []plan.DataSource{
1675+
mustGraphqlDataSourceConfiguration(t,
1676+
"id",
1677+
mustFactory(t,
1678+
testNetHttpClient(t, roundTripperTestCase{
1679+
expectedHost: "example.com",
1680+
expectedPath: "/",
1681+
expectedBody: "",
1682+
sendResponseBody: `{"data":{"hero":{"__typename":"Hero"}}}`,
1683+
sendStatusCode: 200,
1684+
}),
1685+
),
1686+
&plan.DataSourceMetadata{
1687+
RootNodes: []plan.TypeField{
1688+
{TypeName: "Query", FieldNames: []string{"hero"}},
1689+
},
1690+
ChildNodes: []plan.TypeField{
1691+
{TypeName: "Hero", FieldNames: []string{"name"}},
1692+
},
1693+
},
1694+
mustConfiguration(t, graphql_datasource.ConfigurationInput{
1695+
Fetch: &graphql_datasource.FetchConfiguration{
1696+
URL: "https://example.com/",
1697+
Method: "POST",
1698+
},
1699+
SchemaConfiguration: mustSchemaConfig(
1700+
t,
1701+
nil,
1702+
`type Query { hero(name: String!): Hero! } type Hero { name: String! }`,
1703+
),
1704+
}),
1705+
),
1706+
},
1707+
fields: []plan.FieldConfiguration{
1708+
{
1709+
TypeName: "Query",
1710+
FieldName: "hero",
1711+
Path: []string{"hero"},
1712+
Arguments: []plan.ArgumentConfiguration{
1713+
{
1714+
Name: "name",
1715+
SourceType: plan.FieldArgumentSource,
1716+
},
1717+
},
1718+
},
1719+
},
1720+
expectedResponse: `{"data":{"hero":{}}}`,
1721+
}))
1722+
16471723
t.Run("execute operation and apply input coercion for lists without variables", runWithoutError(ExecutionEngineTestCase{
16481724
schema: graphql.InputCoercionForListSchema(t),
16491725
operation: func(t *testing.T) graphql.Request {

v2/pkg/engine/plan/node_selection_visitor.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package plan
22

33
import (
4+
"bytes"
45
"errors"
56
"fmt"
67
"slices"
@@ -54,10 +55,13 @@ type nodeSelectionVisitor struct {
5455
newFieldRefs map[int]struct{} // newFieldRefs is a set of field refs which were added by the visitor or was modified by a rewrite
5556
}
5657

58+
func (c *nodeSelectionVisitor) addNewSkipFieldRefs(fieldRefs ...int) {
59+
c.addSkipFieldRefs(fieldRefs...)
60+
c.addNewFieldRefs(fieldRefs...)
61+
}
62+
5763
func (c *nodeSelectionVisitor) addSkipFieldRefs(fieldRefs ...int) {
5864
c.skipFieldsRefs = append(c.skipFieldsRefs, fieldRefs...)
59-
60-
c.addNewFieldRefs(fieldRefs...)
6165
}
6266

6367
func (c *nodeSelectionVisitor) addNewFieldRefs(fieldRefs ...int) {
@@ -255,6 +259,11 @@ func (c *nodeSelectionVisitor) handleEnterField(fieldRef int, handleRequires boo
255259
}
256260

257261
func (c *nodeSelectionVisitor) LeaveField(ref int) {
262+
if bytes.Equal(c.operation.FieldAliasOrNameBytes(ref), []byte("__internal__typename_placeholder")) {
263+
// we should skip such typename as it was added as a placeholder to keep query valid
264+
// when normalizaion removed all other selections from the selection set
265+
c.addSkipFieldRefs(ref)
266+
}
258267
}
259268

260269
func (c *nodeSelectionVisitor) handleFieldRequiredByRequires(fieldRef int, parentPath, typeName, fieldName, currentPath string, dsConfig DataSource) {
@@ -520,7 +529,7 @@ func (c *nodeSelectionVisitor) addFieldRequirementsToOperation(selectionSetRef i
520529
}
521530
c.resetVisitedAbstractChecksForModifiedFields(addFieldsResult.modifiedFieldRefs)
522531

523-
c.addSkipFieldRefs(addFieldsResult.skipFieldRefs...)
532+
c.addNewSkipFieldRefs(addFieldsResult.skipFieldRefs...)
524533
// add mapping for the field dependencies
525534
for _, requestedByFieldRef := range requirements.requestedByFieldRefs {
526535
fieldKey := fieldIndexKey{requestedByFieldRef, requirements.dsHash}
@@ -610,7 +619,7 @@ func (c *nodeSelectionVisitor) addKeyRequirementsToOperation(selectionSetRef int
610619
// op, _ := astprinter.PrintStringIndentDebug(c.operation, " ")
611620
// fmt.Println("operation: ", op)
612621

613-
c.addSkipFieldRefs(addFieldsResult.skipFieldRefs...)
622+
c.addNewSkipFieldRefs(addFieldsResult.skipFieldRefs...)
614623

615624
// setup deps between key chain items
616625
if currentFieldRefs != nil && previousJump != nil {
@@ -710,7 +719,7 @@ func (c *nodeSelectionVisitor) rewriteSelectionSetHavingAbstractFragments(fieldR
710719
return
711720
}
712721

713-
c.addSkipFieldRefs(rewriter.skipFieldRefs...)
722+
c.addNewSkipFieldRefs(rewriter.skipFieldRefs...)
714723
c.hasNewFields = true
715724
c.rewrittenFieldRefs = append(c.rewrittenFieldRefs, fieldRef)
716725
c.persistedRewrittenFieldRefs[fieldRef] = struct{}{}

v2/pkg/engine/plan/visitor.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -300,11 +300,6 @@ func (v *Visitor) EnterField(ref int) {
300300
fieldName := v.Operation.FieldNameBytes(ref)
301301
fieldAliasOrName := v.Operation.FieldAliasOrNameBytes(ref)
302302

303-
if bytes.Equal(fieldAliasOrName, []byte("__internal__typename_placeholder")) {
304-
// we should skip such typename as it was added as a placeholder to keep query valid
305-
return
306-
}
307-
308303
fieldDefinition, ok := v.Walker.FieldDefinition(ref)
309304
if !ok {
310305
return

0 commit comments

Comments
 (0)