Skip to content

Commit

Permalink
Sergiy/eng 5282 fix requires planning (#823)
Browse files Browse the repository at this point in the history
  • Loading branch information
devsergiy committed Jun 12, 2024
2 parents b3aa353 + fadc4db commit dc45868
Show file tree
Hide file tree
Showing 21 changed files with 1,357 additions and 444 deletions.
12 changes: 12 additions & 0 deletions execution/engine/execution_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,18 @@ func TestExecutionEngine_Execute(t *testing.T) {
},
},
ChildNodes: []plan.TypeField{
{
TypeName: "Human",
FieldNames: []string{"name", "height", "friends"},
},
{
TypeName: "Droid",
FieldNames: []string{"name", "primaryFunction", "friends"},
},
{
TypeName: "Starship",
FieldNames: []string{"name", "length"},
},
{
TypeName: "Character",
FieldNames: []string{"name"},
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

121 changes: 82 additions & 39 deletions v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,9 @@ func TestIntrospectionDataSourcePlanning(t *testing.T) {
Fetch: &resolve.ParallelFetch{
Fetches: []resolve.Fetch{
&resolve.SingleFetch{
FetchID: 1,
FetchDependencies: resolve.FetchDependencies{
FetchID: 1,
},
DataSourceIdentifier: dataSourceIdentifier,
FetchConfiguration: resolve.FetchConfiguration{
Input: `{"request_type":3,"on_type_name":"$$0$$","include_deprecated":$$1$$}`,
Expand All @@ -397,7 +399,9 @@ func TestIntrospectionDataSourcePlanning(t *testing.T) {
},
},
&resolve.SingleFetch{
FetchID: 2,
FetchDependencies: resolve.FetchDependencies{
FetchID: 2,
},
DataSourceIdentifier: dataSourceIdentifier,
FetchConfiguration: resolve.FetchConfiguration{
Input: `{"request_type":4,"on_type_name":"$$0$$","include_deprecated":$$1$$}`,
Expand Down
8 changes: 7 additions & 1 deletion v2/pkg/engine/plan/abstract_selection_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,13 @@ func (r *fieldSelectionRewriter) interfaceFieldSelectionNeedsRewrite(selectionSe
// when we do not have fragments
if !selectionSetInfo.hasInlineFragmentsOnInterfaces && !selectionSetInfo.hasInlineFragmentsOnObjects {
// check that all types implementing the interface have a root node with the requested fields
return !r.allEntitiesHaveFieldsAsRootNode(entityNames, selectionSetInfo.fields)
if !r.allEntitiesHaveFieldsAsRootNode(entityNames, selectionSetInfo.fields) {
return true
}

return slices.ContainsFunc(entityNames, func(entityName string) bool {
return r.hasRequiresConfigurationForField(entityName, selectionSetInfo.fields)
})
}

if selectionSetInfo.hasInlineFragmentsOnObjects {
Expand Down
21 changes: 17 additions & 4 deletions v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,6 @@ func (r *fieldSelectionRewriter) interfaceFragmentNeedCleanup(inlineFragment inl
return true
}

if !inlineFragment.selectionSetInfo.hasInlineFragmentsOnInterfaces && !inlineFragment.selectionSetInfo.hasInlineFragmentsOnObjects {
return false
}

// if interface fragment has inline fragments on objects
// check that object type is present within parent selection valid types - e.g. members of union or parent interface
// check each fragment for the presence of other interface fragments
Expand All @@ -212,10 +208,15 @@ func (r *fieldSelectionRewriter) interfaceFragmentNeedCleanup(inlineFragment inl
}

if inlineFragment.selectionSetInfo.hasFields {
// NOTE: maybe we need to filter this typenames by parentSelectionValidTypes?
for _, typeName := range inlineFragment.typeNamesImplementingInterfaceInCurrentDS {
if !r.typeHasAllFieldLocal(typeName, inlineFragment.selectionSetInfo.fields) {
return true
}

if r.hasRequiresConfigurationForField(typeName, inlineFragment.selectionSetInfo.fields) {
return true
}
}
}

Expand All @@ -232,6 +233,18 @@ func (r *fieldSelectionRewriter) typeHasAllFieldLocal(typeName string, fields []
return true
}

func (r *fieldSelectionRewriter) hasRequiresConfigurationForField(typeName string, fields []fieldSelection) bool {
return slices.ContainsFunc(r.dsConfiguration.FederationConfiguration().Requires, func(cfg FederationFieldConfiguration) bool {
if cfg.TypeName != typeName {
return false
}

return slices.ContainsFunc(fields, func(fieldSelection fieldSelection) bool {
return cfg.FieldName == fieldSelection.fieldName
})
})
}

func (r *fieldSelectionRewriter) allFragmentTypesImplementsInterfaceTypes(inlineFragments []inlineFragmentSelection, interfaceTypes []string) bool {
for _, inlineFragment := range inlineFragments {
if !slices.Contains(interfaceTypes, inlineFragment.typeName) {
Expand Down
180 changes: 176 additions & 4 deletions v2/pkg/engine/plan/abstract_selection_rewriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,60 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
}`,
shouldRewrite: false,
},
{
name: "all fields local but one of the fields has requires directive. query without fragments",
definition: definition,
upstreamDefinition: definition,
dsConfiguration: dsb().
RootNode("Query", "iface").
RootNode("User", "id", "name").
RootNode("Admin", "id", "name").
KeysMetadata(FederationFieldConfigurations{
{
TypeName: "User",
SelectionSet: "id",
},
{
TypeName: "Admin",
SelectionSet: "id",
},
}).
WithMetadata(func(md *FederationMetaData) {
md.Requires = []FederationFieldConfiguration{
{
TypeName: "User",
FieldName: "name",
SelectionSet: "any",
},
}
}).
DS(),
operation: `
query {
iface {
name
}
}`,

expectedOperation: `
query {
iface {
... on Admin {
name
}
... on ImplementsNodeNotInUnion {
name
}
... on Moderator {
name
}
... on User {
name
}
}
}`,
shouldRewrite: true,
},
{
name: "all fields local. query has user fragment",
definition: definition,
Expand Down Expand Up @@ -1315,15 +1369,44 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
shouldRewrite: true,
},
{
name: "Union with interface fragment: no entity fragments, all fields local",
definition: definition,
upstreamDefinition: definition,
name: "Union with interface fragment: no entity fragments, all fields local",
definition: definition,
upstreamDefinition: `
interface Node {
id: ID!
name: String!
}
type User implements Node {
id: ID!
name: String!
isUser: Boolean!
}
type Admin implements Node {
id: ID!
name: String!
}
type Moderator implements Node {
id: ID!
name: String!
isModerator: Boolean!
}
union Account = User | Admin | Moderator
type Query {
iface: Node!
accounts: [Account!]!
}
`,
dsConfiguration: dsb().
RootNode("Query", "iface", "accounts").
RootNode("User", "id", "name", "isUser").
RootNode("Admin", "id", "name").
RootNode("Moderator", "id", "name", "isModerator").
RootNode("Node", "id", "name").
ChildNode("Node", "id", "name").
KeysMetadata(FederationFieldConfigurations{
{
TypeName: "User",
Expand Down Expand Up @@ -1358,6 +1441,95 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
}`,
shouldRewrite: false,
},
{
name: "Union with interface fragment: no entity fragments, all fields local - but one of the fields has requires",
definition: definition,
upstreamDefinition: `
interface Node {
id: ID!
name: String!
}
type User implements Node {
id: ID!
name: String!
isUser: Boolean!
}
type Admin implements Node {
id: ID!
name: String! @requires(fields: "importantNote")
importantNote: String! @external
}
type Moderator implements Node {
id: ID!
name: String!
isModerator: Boolean!
}
union Account = User | Admin | Moderator
type Query {
iface: Node!
accounts: [Account!]!
}
`,
dsConfiguration: dsb().
RootNode("Query", "iface", "accounts").
RootNode("User", "id", "name", "isUser").
RootNode("Admin", "id", "name").
RootNode("Moderator", "id", "name", "isModerator").
ChildNode("Node", "id", "name").
KeysMetadata(FederationFieldConfigurations{
{
TypeName: "User",
SelectionSet: "id",
},
{
TypeName: "Admin",
SelectionSet: "id",
},
{
TypeName: "Moderator",
SelectionSet: "id",
},
}).
WithMetadata(func(meta *FederationMetaData) {
meta.Requires = []FederationFieldConfiguration{
{
TypeName: "Admin",
FieldName: "name",
SelectionSet: "importantNote",
},
}
}).
DS(),
fieldName: "accounts",
operation: `
query {
accounts {
... on Node {
name
}
}
}`,
expectedOperation: `
query {
accounts {
... on Admin {
name
}
... on Moderator {
name
}
... on User {
name
}
}
}`,
shouldRewrite: true,
},
{
name: "Union with interface fragment: no entity fragments, user.name is local, admin.name and moderator.name is external",
definition: definition,
Expand Down
11 changes: 6 additions & 5 deletions v2/pkg/engine/plan/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ type Configuration struct {
}

type DebugConfiguration struct {
PrintOperationTransformations bool
PrintOperationEnableASTRefs bool
PrintPlanningPaths bool
PrintQueryPlans bool
PrintNodeSuggestions bool
PrintOperationTransformations bool
PrintOperationEnableASTRefs bool
PrintPlanningPaths bool
PrintQueryPlans bool
PrintNodeSuggestions bool
EnableNodeSuggestionsSelectionReasons bool

ConfigurationVisitor bool
PlanningVisitor bool
Expand Down
Loading

0 comments on commit dc45868

Please sign in to comment.