Skip to content

Commit bcf547d

Browse files
devsergiydevsergiy
authored
fix: fix collecting representation for fetches scoped to concrete types (#1200)
<!-- Important: Before developing new features, please open an issue to discuss your ideas with the maintainers. This ensures project alignment and helps avoid unnecessary work for you. Thank you for your contribution! Please provide a detailed description below and ensure you've met all the requirements. Squashed commit messages must follow the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) standard to facilitate changelog generation. Please ensure your PR title follows the Conventional Commits specification, using the appropriate type (e.g., feat, fix, docs) and scope. Examples of good PR titles: - 💥feat!: change implementation in an non-backward compatible way - ✨feat(auth): add support for OAuth2 login - 🐞fix(router): add support for custom metrics - 📚docs(README): update installation instructions - 🧹chore(deps): bump dependencies to latest versions --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved support for nested fetches in federated GraphQL queries, allowing selective fetching of fields based on specific types within interfaces or unions. * **Bug Fixes** * Fixed handling and merging of type information for fetch paths, ensuring correct data resolution across nested and polymorphic fields. * Corrected typographical errors in query planning internals to enhance stability. * **Tests** * Added new tests verifying nested fetch behavior on different types and ensuring type conditions are respected in federated schemas. * Expanded federation test coverage for interface and union type handling with nested fetches. * **Refactor** * Enhanced tracking of selection sets with associated type metadata to improve query planning accuracy. * Unified and simplified JSON item selection logic by consolidating multiple methods into a single, type-aware method. * Improved deduplication logic by merging fetch paths and type information instead of removing duplicates outright. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Checklist - [ ] I have discussed my proposed changes in an issue and have received approval to proceed. - [ ] I have followed the coding standards of the project. - [ ] Tests or benchmarks have been added or updated. <!-- Please add any additional information or context regarding your changes here. -->
1 parent e9b9f19 commit bcf547d

File tree

9 files changed

+807
-93
lines changed

9 files changed

+807
-93
lines changed

‎execution/engine/execution_engine_test.go‎

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4224,6 +4224,214 @@ func TestExecutionEngine_Execute(t *testing.T) {
42244224
))
42254225
})
42264226
})
4227+
4228+
t.Run("execute operation with nested fetch on one of the types", func(t *testing.T) {
4229+
4230+
definition := `
4231+
type User implements Node {
4232+
id: ID!
4233+
title: String!
4234+
some: User!
4235+
}
4236+
4237+
type Admin implements Node {
4238+
id: ID!
4239+
title: String!
4240+
some: User!
4241+
}
4242+
4243+
interface Node {
4244+
id: ID!
4245+
title: String!
4246+
some: User!
4247+
}
4248+
4249+
type Query {
4250+
accounts: [Node!]!
4251+
}`
4252+
4253+
firstSubgraphSDL := `
4254+
type User implements Node @key(fields: "id") {
4255+
id: ID!
4256+
title: String! @external
4257+
some: User!
4258+
}
4259+
4260+
type Admin implements Node @key(fields: "id") {
4261+
id: ID!
4262+
title: String! @external
4263+
some: User!
4264+
}
4265+
4266+
interface Node {
4267+
id: ID!
4268+
title: String!
4269+
some: User!
4270+
}
4271+
4272+
type Query {
4273+
accounts: [Node!]!
4274+
}
4275+
`
4276+
secondSubgraphSDL := `
4277+
type User @key(fields: "id") {
4278+
id: ID!
4279+
name: String!
4280+
title: String!
4281+
}
4282+
4283+
type Admin @key(fields: "id") {
4284+
id: ID!
4285+
adminName: String!
4286+
title: String!
4287+
}
4288+
`
4289+
4290+
datasources := []plan.DataSource{
4291+
mustGraphqlDataSourceConfiguration(t,
4292+
"id-1",
4293+
mustFactory(t,
4294+
testNetHttpClient(t, roundTripperTestCase{
4295+
expectedHost: "first",
4296+
expectedPath: "/",
4297+
expectedBody: `{"query":"{accounts {__typename ... on User {some {__typename id}} ... on Admin {some {__typename id}}}}"}`,
4298+
sendResponseBody: `{"data":{"accounts":[{"__typename":"User","some":{"__typename":"User","id":"1"}},{"__typename":"Admin","some":{"__typename":"User","id":"2"}},{"__typename":"User","some":{"__typename":"User","id":"3"}}]}}`,
4299+
sendStatusCode: 200,
4300+
}),
4301+
),
4302+
&plan.DataSourceMetadata{
4303+
RootNodes: []plan.TypeField{
4304+
{
4305+
TypeName: "Query",
4306+
FieldNames: []string{"accounts"},
4307+
},
4308+
{
4309+
TypeName: "User",
4310+
FieldNames: []string{"id", "some"},
4311+
},
4312+
{
4313+
TypeName: "Admin",
4314+
FieldNames: []string{"id", "some"},
4315+
},
4316+
},
4317+
ChildNodes: []plan.TypeField{
4318+
{
4319+
TypeName: "Node",
4320+
FieldNames: []string{"id", "title", "some"},
4321+
},
4322+
},
4323+
FederationMetaData: plan.FederationMetaData{
4324+
Keys: plan.FederationFieldConfigurations{
4325+
{
4326+
TypeName: "User",
4327+
SelectionSet: "id",
4328+
},
4329+
{
4330+
TypeName: "Admin",
4331+
SelectionSet: "id",
4332+
},
4333+
},
4334+
},
4335+
},
4336+
mustConfiguration(t, graphql_datasource.ConfigurationInput{
4337+
Fetch: &graphql_datasource.FetchConfiguration{
4338+
URL: "https://first/",
4339+
Method: "POST",
4340+
},
4341+
SchemaConfiguration: mustSchemaConfig(
4342+
t,
4343+
&graphql_datasource.FederationConfiguration{
4344+
Enabled: true,
4345+
ServiceSDL: firstSubgraphSDL,
4346+
},
4347+
firstSubgraphSDL,
4348+
),
4349+
}),
4350+
),
4351+
mustGraphqlDataSourceConfiguration(t,
4352+
"id-2",
4353+
mustFactory(t,
4354+
testNetHttpClient(t, roundTripperTestCase{
4355+
expectedHost: "second",
4356+
expectedPath: "/",
4357+
expectedBody: `{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on User {__typename title}}}","variables":{"representations":[{"__typename":"User","id":"1"},{"__typename":"User","id":"3"}]}}`,
4358+
sendResponseBody: `{"data":{"_entities":[{"__typename":"User","title":"User1"},{"__typename":"User","title":"User3"}]}}`,
4359+
sendStatusCode: 200,
4360+
}),
4361+
),
4362+
&plan.DataSourceMetadata{
4363+
RootNodes: []plan.TypeField{
4364+
{
4365+
TypeName: "User",
4366+
FieldNames: []string{"id", "name", "title"},
4367+
},
4368+
{
4369+
TypeName: "Admin",
4370+
FieldNames: []string{"id", "adminName", "title"},
4371+
},
4372+
},
4373+
FederationMetaData: plan.FederationMetaData{
4374+
Keys: plan.FederationFieldConfigurations{
4375+
{
4376+
TypeName: "User",
4377+
SelectionSet: "id",
4378+
},
4379+
{
4380+
TypeName: "Admin",
4381+
SelectionSet: "id",
4382+
},
4383+
},
4384+
},
4385+
},
4386+
mustConfiguration(t, graphql_datasource.ConfigurationInput{
4387+
Fetch: &graphql_datasource.FetchConfiguration{
4388+
URL: "https://second/",
4389+
Method: "POST",
4390+
},
4391+
SchemaConfiguration: mustSchemaConfig(
4392+
t,
4393+
&graphql_datasource.FederationConfiguration{
4394+
Enabled: true,
4395+
ServiceSDL: secondSubgraphSDL,
4396+
},
4397+
secondSubgraphSDL,
4398+
),
4399+
}),
4400+
),
4401+
}
4402+
4403+
t.Run("run", runWithoutError(ExecutionEngineTestCase{
4404+
schema: func(t *testing.T) *graphql.Schema {
4405+
t.Helper()
4406+
parseSchema, err := graphql.NewSchemaFromString(definition)
4407+
require.NoError(t, err)
4408+
return parseSchema
4409+
}(t),
4410+
operation: func(t *testing.T) graphql.Request {
4411+
return graphql.Request{
4412+
OperationName: "Accounts",
4413+
Query: `
4414+
query Accounts {
4415+
accounts {
4416+
... on User {
4417+
some {
4418+
title
4419+
}
4420+
}
4421+
... on Admin {
4422+
some {
4423+
__typename
4424+
id
4425+
}
4426+
}
4427+
}
4428+
}`,
4429+
}
4430+
},
4431+
dataSources: datasources,
4432+
expectedResponse: `{"data":{"accounts":[{"some":{"title":"User1"}},{"some":{"__typename":"User","id":"2"}},{"some":{"title":"User3"}}]}}`,
4433+
}))
4434+
})
42274435
}
42284436

42294437
func testNetHttpClient(t *testing.T, testCase roundTripperTestCase) *http.Client {

0 commit comments

Comments
 (0)