diff --git a/v2/pkg/ast/ast_interface_type_definition.go b/v2/pkg/ast/ast_interface_type_definition.go index 50aa099dea..432556e93e 100644 --- a/v2/pkg/ast/ast_interface_type_definition.go +++ b/v2/pkg/ast/ast_interface_type_definition.go @@ -15,14 +15,15 @@ import ( // name: String // } type InterfaceTypeDefinition struct { - Description Description // optional, describes the interface - InterfaceLiteral position.Position // interface - Name ByteSliceReference // e.g. NamedEntity - ImplementsInterfaces TypeList // e.g implements Bar & Baz - HasDirectives bool - Directives DirectiveList // optional, e.g. @foo - HasFieldDefinitions bool - FieldsDefinition FieldDefinitionList // optional, e.g. { name: String } + Description Description // optional, describes the interface + InterfaceLiteral position.Position // interface + Name ByteSliceReference // e.g. NamedEntity + ImplementsInterfaces TypeList // e.g implements Bar & Baz + HasDirectives bool + Directives DirectiveList // optional, e.g. @foo + HasFieldDefinitions bool + FieldsDefinition FieldDefinitionList // optional, e.g. { name: String } + ImplementedByObjectDefinitions []int // list of ObjectTypeDefinition refs that implement this interface } func (d *Document) InterfaceTypeDefinitionNameBytes(ref int) ByteSlice { @@ -163,15 +164,21 @@ func (d *Document) InterfaceTypeDefinitionFieldWithName(ref int, fieldName []byt // InterfaceTypeDefinitionImplementedByObjectWithNames returns object type names implementing the interface. func (d *Document) InterfaceTypeDefinitionImplementedByObjectWithNames(interfaceDefRef int) (typeNames []string, ok bool) { - implementedByNodes := d.InterfaceTypeDefinitionImplementedByRootNodes(interfaceDefRef) - - typeNames = make([]string, 0, len(implementedByNodes)) - for _, implementedByNode := range implementedByNodes { - if implementedByNode.Kind != NodeKindObjectTypeDefinition { - continue + implementedByObjectRefs := d.InterfaceTypeDefinitions[interfaceDefRef].ImplementedByObjectDefinitions + if implementedByObjectRefs == nil { + // fallback for documents not precalculated (e.g., built programmatically) + implementors := d.InterfaceTypeDefinitionImplementedByRootNodes(interfaceDefRef) + typeNames = make([]string, 0, len(implementors)) + for _, n := range implementors { + if n.Kind == NodeKindObjectTypeDefinition { + typeNames = append(typeNames, d.ObjectTypeDefinitionNameString(n.Ref)) + } + } + } else { + typeNames = make([]string, 0, len(implementedByObjectRefs)) + for _, implementedByObjectRef := range d.InterfaceTypeDefinitions[interfaceDefRef].ImplementedByObjectDefinitions { + typeNames = append(typeNames, d.ObjectTypeDefinitionNameString(implementedByObjectRef)) } - - typeNames = append(typeNames, implementedByNode.NameString(d)) } if len(typeNames) > 0 { @@ -183,15 +190,21 @@ func (d *Document) InterfaceTypeDefinitionImplementedByObjectWithNames(interface } func (d *Document) InterfaceTypeDefinitionImplementedByObjectWithNamesAsBytes(interfaceDefRef int) (typeNames [][]byte, ok bool) { - implementedByNodes := d.InterfaceTypeDefinitionImplementedByRootNodes(interfaceDefRef) - - typeNames = make([][]byte, 0, len(implementedByNodes)) - for _, implementedByNode := range implementedByNodes { - if implementedByNode.Kind != NodeKindObjectTypeDefinition { - continue + implementedByObjectRefs := d.InterfaceTypeDefinitions[interfaceDefRef].ImplementedByObjectDefinitions + if implementedByObjectRefs == nil { + implementors := d.InterfaceTypeDefinitionImplementedByRootNodes(interfaceDefRef) + typeNames = make([][]byte, 0, len(implementors)) + // fallback for documents not precalculated (e.g., built programmatically) + for _, n := range implementors { + if n.Kind == NodeKindObjectTypeDefinition { + typeNames = append(typeNames, d.ObjectTypeDefinitionNameBytes(n.Ref)) + } + } + } else { + typeNames = make([][]byte, 0, len(implementedByObjectRefs)) + for _, implementedByObjectRef := range d.InterfaceTypeDefinitions[interfaceDefRef].ImplementedByObjectDefinitions { + typeNames = append(typeNames, d.ObjectTypeDefinitionNameBytes(implementedByObjectRef)) } - - typeNames = append(typeNames, implementedByNode.NameBytes(d)) } if len(typeNames) > 0 { @@ -201,3 +214,18 @@ func (d *Document) InterfaceTypeDefinitionImplementedByObjectWithNamesAsBytes(in return nil, false } + +func (d *Document) PopulateInterfaceTypeDefinitionImplementedByObjects() { + for ref := 0; ref < len(d.InterfaceTypeDefinitions); ref++ { + implementedByNodes := d.InterfaceTypeDefinitionImplementedByRootNodes(ref) + implementedByObjectRefs := make([]int, 0, len(implementedByNodes)) + for _, implementedByNode := range implementedByNodes { + if implementedByNode.Kind != NodeKindObjectTypeDefinition { + continue + } + + implementedByObjectRefs = append(implementedByObjectRefs, implementedByNode.Ref) + } + d.InterfaceTypeDefinitions[ref].ImplementedByObjectDefinitions = implementedByObjectRefs + } +} diff --git a/v2/pkg/astnormalization/abstract_field_normalizer.go b/v2/pkg/astnormalization/abstract_field_normalizer.go index 16c365adcc..5922f768ba 100644 --- a/v2/pkg/astnormalization/abstract_field_normalizer.go +++ b/v2/pkg/astnormalization/abstract_field_normalizer.go @@ -17,7 +17,7 @@ type AbstractFieldNormalizer struct { } func NewAbstractFieldNormalizer(operation *ast.Document, definition *ast.Document, fieldRef int) *AbstractFieldNormalizer { - walker := astvisitor.NewWalker(48) + walker := astvisitor.NewWalkerWithID(48, "AbstractFieldNormalizer") normalizer := &AbstractFieldNormalizer{ Walker: &walker, diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index c92018e22a..04631f8398 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -203,7 +203,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { // NOTE: normalization rules for variables rely on the fact that // we will visit only a single operation, so it is important to remove non-matching operations if o.options.removeNotMatchingOperationDefinitions { - removeNotMatchingOperationDefinitionsWalker := astvisitor.NewWalker(2) + removeNotMatchingOperationDefinitionsWalker := astvisitor.NewWalkerWithID(2, "RemoveNotMatchingOperationDefinitions") // This rule does not walk deep into ast, so a separate walk is not expensive, // but we could not mix this walk with other rules, because they need to go deep o.removeOperationDefinitionsVisitor = removeOperationDefinitions(&removeNotMatchingOperationDefinitionsWalker) @@ -214,11 +214,11 @@ func (o *OperationNormalizer) setupOperationWalkers() { }) } - directivesIncludeSkip := astvisitor.NewWalker(8) + directivesIncludeSkip := astvisitor.NewWalkerWithID(8, "DirectivesIncludeSkip") preventFragmentCycles(&directivesIncludeSkip) directiveIncludeSkipKeepNodes(&directivesIncludeSkip, o.options.ignoreSkipInclude) - cleanup := astvisitor.NewWalker(8) + cleanup := astvisitor.NewWalkerWithID(8, "Cleanup") deduplicateFields(&cleanup) if o.options.removeUnusedVariables { del := deleteUnusedVariables(&cleanup) @@ -235,7 +235,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { }) if o.options.inlineFragmentSpreads { - fragmentInline := astvisitor.NewWalker(8) + fragmentInline := astvisitor.NewWalkerWithID(8, "FragmentSpreadInline") fragmentSpreadInline(&fragmentInline) o.operationWalkers = append(o.operationWalkers, walkerStage{ name: "fragmentInline", @@ -244,7 +244,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { } if o.options.extractVariables { - extractVariablesWalker := astvisitor.NewWalker(8) + extractVariablesWalker := astvisitor.NewWalkerWithID(8, "ExtractVariables") extractVariables(&extractVariablesWalker) o.operationWalkers = append(o.operationWalkers, walkerStage{ name: "extractVariables", @@ -252,7 +252,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { }) } - other := astvisitor.NewWalker(8) + other := astvisitor.NewWalkerWithID(8, "Other") removeSelfAliasing(&other) inlineSelectionsFromInlineFragments(&other) o.operationWalkers = append(o.operationWalkers, walkerStage{ @@ -260,7 +260,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { walker: &other, }) - mergeInlineFragments := astvisitor.NewWalker(8) + mergeInlineFragments := astvisitor.NewWalkerWithID(8, "MergeInlineFragmentSelections") mergeInlineFragmentSelections(&mergeInlineFragments) o.operationWalkers = append(o.operationWalkers, walkerStage{ name: "mergeInlineFragmentSelections", @@ -268,7 +268,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { }) if o.options.removeFragmentDefinitions { - removeFragments := astvisitor.NewWalker(8) + removeFragments := astvisitor.NewWalkerWithID(8, "RemoveFragmentDefinitions") removeFragmentDefinitions(&removeFragments) o.operationWalkers = append(o.operationWalkers, walkerStage{ @@ -283,7 +283,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { }) if o.options.extractVariables { - variablesProcessing := astvisitor.NewWalker(8) + variablesProcessing := astvisitor.NewWalkerWithID(8, "VariablesProcessing") inputCoercionForList(&variablesProcessing) extractVariablesDefaultValue(&variablesProcessing) injectInputFieldDefaults(&variablesProcessing) @@ -356,21 +356,21 @@ type VariablesNormalizer struct { func NewVariablesNormalizer() *VariablesNormalizer { // delete unused modifying variables refs, // so it is safer to run it sequentially with the extraction - thirdDeleteUnused := astvisitor.NewWalker(8) + thirdDeleteUnused := astvisitor.NewWalkerWithID(8, "DeleteUnusedVariables") del := deleteUnusedVariables(&thirdDeleteUnused) // register variable usage detection on the first stage // and pass usage information to the deletion visitor // so it keeps variables that are defined but not used at all // ensuring that validation can still catch them - firstDetectUnused := astvisitor.NewWalker(8) + firstDetectUnused := astvisitor.NewWalkerWithID(8, "DetectVariableUsage") detectVariableUsage(&firstDetectUnused, del) - secondExtract := astvisitor.NewWalker(8) + secondExtract := astvisitor.NewWalkerWithID(8, "ExtractVariables") variablesExtractionVisitor := extractVariables(&secondExtract) extractVariablesDefaultValue(&secondExtract) - fourthCoerce := astvisitor.NewWalker(0) + fourthCoerce := astvisitor.NewWalkerWithID(0, "VariablesCoercion") inputCoercionForList(&fourthCoerce) return &VariablesNormalizer{ diff --git a/v2/pkg/astnormalization/variables_mapper.go b/v2/pkg/astnormalization/variables_mapper.go index f7afbbadc2..35d706892a 100644 --- a/v2/pkg/astnormalization/variables_mapper.go +++ b/v2/pkg/astnormalization/variables_mapper.go @@ -12,7 +12,7 @@ type VariablesMapper struct { } func NewVariablesMapper() *VariablesMapper { - walker := astvisitor.NewDefaultWalker() + walker := astvisitor.NewDefaultWalkerWithID("VariablesMapper") mapper := remapVariables(&walker) return &VariablesMapper{ diff --git a/v2/pkg/astparser/parser.go b/v2/pkg/astparser/parser.go index 0c57f43054..f019943534 100644 --- a/v2/pkg/astparser/parser.go +++ b/v2/pkg/astparser/parser.go @@ -91,6 +91,7 @@ func (p *Parser) Parse(document *ast.Document, report *operationreport.Report) { p.report = report p.tokenize() p.parse() + p.precalculate() } func (p *Parser) tokenize() { @@ -106,9 +107,18 @@ func (p *Parser) ParseWithLimits(limits TokenizerLimits, document *ast.Document, return stats, err } p.parse() + p.precalculate() return stats, nil } +func (p *Parser) precalculate() { + if p.report.HasErrors() { + return + } + + p.document.PopulateInterfaceTypeDefinitionImplementedByObjects() +} + func (p *Parser) parse() { for { key, literalReference := p.peekLiteral() diff --git a/v2/pkg/asttransform/typename_visitor.go b/v2/pkg/asttransform/typename_visitor.go index 5a058dfff6..22de134746 100644 --- a/v2/pkg/asttransform/typename_visitor.go +++ b/v2/pkg/asttransform/typename_visitor.go @@ -18,7 +18,7 @@ type TypeNameVisitor struct { } func NewTypeNameVisitor() *TypeNameVisitor { - walker := astvisitor.NewWalker(48) + walker := astvisitor.NewWalkerWithID(48, "TypeNameVisitor") visitor := &TypeNameVisitor{ Walker: &walker, diff --git a/v2/pkg/astvalidation/operation_validation.go b/v2/pkg/astvalidation/operation_validation.go index d6d1bff1b8..ec3f345fc2 100644 --- a/v2/pkg/astvalidation/operation_validation.go +++ b/v2/pkg/astvalidation/operation_validation.go @@ -30,7 +30,7 @@ func DefaultOperationValidator(options ...Option) *OperationValidator { opt(&opts) } validator := OperationValidator{ - walker: astvisitor.NewWalker(48), + walker: astvisitor.NewWalkerWithID(48, "OperationValidator"), } if opts.ApolloCompatibilityFlags.UseGraphQLValidationFailedStatus { @@ -64,7 +64,7 @@ func DefaultOperationValidator(options ...Option) *OperationValidator { func NewOperationValidator(rules []Rule) *OperationValidator { validator := OperationValidator{ - walker: astvisitor.NewWalker(48), + walker: astvisitor.NewWalkerWithID(48, "OperationValidator"), } for _, rule := range rules { diff --git a/v2/pkg/astvisitor/visitor.go b/v2/pkg/astvisitor/visitor.go index a2cbb102da..86f284c72e 100644 --- a/v2/pkg/astvisitor/visitor.go +++ b/v2/pkg/astvisitor/visitor.go @@ -63,6 +63,7 @@ func newSkipVisitors(skips []int, planner interface{}, allowedToVisit bool) Skip // Walker orchestrates the process of walking an AST and calling all registered callbacks // Always use NewWalker to instantiate a new Walker type Walker struct { + walkerID string // Ancestors is the slice of Nodes to the current Node in a callback // don't keep a reference to this slice, always copy it if you want to work with it after the callback returned Ancestors []ast.Node @@ -96,11 +97,21 @@ type Walker struct { OnExternalError func(err *operationreport.ExternalError) } +func NewWalkerWithID(ancestorSize int, id string) Walker { + return Walker{ + Ancestors: make([]ast.Node, 0, ancestorSize), + Path: make([]ast.PathItem, 0, 64), + TypeDefinitions: make([]ast.Node, 0, ancestorSize), + deferred: make([]func(), 0, 8), + walkerID: id, + } +} + // NewWalker returns a fully initialized Walker func NewWalker(ancestorSize int) Walker { return Walker{ Ancestors: make([]ast.Node, 0, ancestorSize), - Path: make([]ast.PathItem, 0, ancestorSize), + Path: make([]ast.PathItem, 0, 64), TypeDefinitions: make([]ast.Node, 0, ancestorSize), deferred: make([]func(), 0, 8), } @@ -111,6 +122,10 @@ func NewDefaultWalker() Walker { return NewWalker(8) } +func NewDefaultWalkerWithID(id string) Walker { + return NewWalkerWithID(8, id) +} + var ( walkerPool = sync.Pool{} ) @@ -1365,6 +1380,12 @@ func (w *Walker) SetVisitorFilter(filter VisitorFilter) { // Walk initiates the walker to start walking the AST from the top root Node func (w *Walker) Walk(document, definition *ast.Document, report *operationreport.Report) { + // uncomment to get walker labels in pprof profiles + // ctx := context.Background() + // labels := pprof.Labels("walker_id", w.walkerID) + // pprof.Do(ctx, labels, func(ctx context.Context) { + // }) + if report == nil { w.Report = &operationreport.Report{} } else { diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go index 3488d86341..7f41943c85 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go @@ -282,6 +282,7 @@ func TestGraphQLDataSource(t *testing.T) { Name: []byte("stringList"), Value: &resolve.Array{ Nullable: true, + Path: []string{"stringList"}, Item: &resolve.String{ Nullable: true, }, @@ -350,11 +351,6 @@ func TestGraphQLDataSource(t *testing.T) { }, }, }, - { - TypeName: "Query", - FieldName: "stringList", - DisableDefaultMapping: true, - }, { TypeName: "Query", FieldName: "nestedStringList", @@ -589,6 +585,7 @@ func TestGraphQLDataSource(t *testing.T) { Name: []byte("stringList"), Value: &resolve.Array{ Nullable: true, + Path: []string{"stringList"}, Item: &resolve.String{ Nullable: true, }, @@ -677,11 +674,6 @@ func TestGraphQLDataSource(t *testing.T) { }, }, }, - { - TypeName: "Query", - FieldName: "stringList", - DisableDefaultMapping: true, - }, { TypeName: "Query", FieldName: "nestedStringList", @@ -1699,6 +1691,7 @@ func TestGraphQLDataSource(t *testing.T) { Name: []byte("stringList"), Value: &resolve.Array{ Nullable: true, + Path: []string{"stringList"}, Item: &resolve.String{ Nullable: true, }, @@ -1767,11 +1760,6 @@ func TestGraphQLDataSource(t *testing.T) { }, }, }, - { - TypeName: "Query", - FieldName: "stringList", - DisableDefaultMapping: true, - }, { TypeName: "Query", FieldName: "nestedStringList", @@ -2528,6 +2516,7 @@ func TestGraphQLDataSource(t *testing.T) { { Name: []byte("addFriend"), Value: &resolve.Object{ + Path: []string{"addFriend"}, PossibleTypes: map[string]struct{}{ "Friend": {}, }, @@ -2589,9 +2578,8 @@ func TestGraphQLDataSource(t *testing.T) { }, Fields: []plan.FieldConfiguration{ { - TypeName: "Mutation", - FieldName: "addFriend", - DisableDefaultMapping: true, + TypeName: "Mutation", + FieldName: "addFriend", Arguments: []plan.ArgumentConfiguration{ { Name: "name", @@ -5006,12 +4994,12 @@ func TestGraphQLDataSource(t *testing.T) { TypeName: "Query", FieldNames: []string{"me", "user"}, }, - }, - ChildNodes: []plan.TypeField{ { TypeName: "User", FieldNames: []string{"id", "name", "username", "birthDate", "account", "metadata", "ssn"}, }, + }, + ChildNodes: []plan.TypeField{ { TypeName: "UserMetadata", FieldNames: []string{"name", "address", "description"}, @@ -5392,12 +5380,12 @@ func TestGraphQLDataSource(t *testing.T) { TypeName: "Query", FieldNames: []string{"me", "user"}, }, - }, - ChildNodes: []plan.TypeField{ { TypeName: "User", FieldNames: []string{"id", "name", "username", "birthDate", "account", "metadata", "ssn"}, }, + }, + ChildNodes: []plan.TypeField{ { TypeName: "UserMetadata", FieldNames: []string{"name", "address", "description"}, diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter.go b/v2/pkg/engine/plan/abstract_selection_rewriter.go index 8320d570d6..4d19e9b08f 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter.go @@ -692,7 +692,7 @@ func (v *AbstractFieldPathCollector) EnterField(ref int) { } func collectPath(operation *ast.Document, definition *ast.Document, fieldRef int, fieldToPath bool) (fieldRefPaths map[int]string, pathFieldRefs map[string][]int, err error) { - walker := astvisitor.NewWalker(4) + walker := astvisitor.NewWalkerWithID(4, "AbstractFieldPathCollector") c := &AbstractFieldPathCollector{ Walker: &walker, diff --git a/v2/pkg/engine/plan/analyze_plan_kind.go b/v2/pkg/engine/plan/analyze_plan_kind.go index 82ada1ca3c..9d949884af 100644 --- a/v2/pkg/engine/plan/analyze_plan_kind.go +++ b/v2/pkg/engine/plan/analyze_plan_kind.go @@ -7,7 +7,7 @@ import ( ) func AnalyzePlanKind(operation, definition *ast.Document, operationName string) (operationType ast.OperationType, streaming bool, error error) { - walker := astvisitor.NewWalker(48) + walker := astvisitor.NewWalkerWithID(48, "PlanKindVisitor") visitor := &planKindVisitor{ Walker: &walker, operationName: operationName, diff --git a/v2/pkg/engine/plan/configuration.go b/v2/pkg/engine/plan/configuration.go index eb6fadf3d8..dafcb021c5 100644 --- a/v2/pkg/engine/plan/configuration.go +++ b/v2/pkg/engine/plan/configuration.go @@ -115,7 +115,7 @@ type FieldConfiguration struct { TypeName string FieldName string // DisableDefaultMapping - instructs planner whether to use path mapping coming from Path field - DisableDefaultMapping bool + DisableDefaultMapping bool // TODO: has no effect as of now, remove? // Path - represents a json path to lookup for a field value in response json Path []string Arguments ArgumentsConfigurations diff --git a/v2/pkg/engine/plan/datasource_configuration.go b/v2/pkg/engine/plan/datasource_configuration.go index 24bfa115c4..f196a00bc8 100644 --- a/v2/pkg/engine/plan/datasource_configuration.go +++ b/v2/pkg/engine/plan/datasource_configuration.go @@ -88,10 +88,13 @@ func (d *DataSourceMetadata) Init() error { } func (d *DataSourceMetadata) InitKeys() error { + d.FederationMetaData.entityTypeNames = make(map[string]struct{}) + for i := 0; i < len(d.FederationMetaData.Keys); i++ { if err := d.FederationMetaData.Keys[i].parseSelectionSet(); err != nil { return err } + d.FederationMetaData.entityTypeNames[d.FederationMetaData.Keys[i].TypeName] = struct{}{} } return nil diff --git a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go index 9b31f7840b..b9d47ad3ba 100644 --- a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go @@ -21,6 +21,11 @@ type nodesCollector struct { maxConcurrency uint seenKeys map[SeenKeyPath]struct{} + fieldInfo map[int]fieldInfo + newFieldRefs map[int]struct{} + + dsVisitors []*collectNodesDSVisitor + dsVisitorsReports []*operationreport.Report } type DSKeyInfo struct { @@ -41,28 +46,80 @@ type SeenKeyPath struct { DSHash DSHash } -func (c *nodesCollector) CollectNodes() (nodes *NodeSuggestions, keys []DSKeyInfo) { +func (c *nodesCollector) CollectNodes() (keys []DSKeyInfo) { c.buildTree() if c.report.HasErrors() { - return nil, nil + return nil } + // reset keys to not preserve already collected keys from previous iterations + c.keys = c.keys[:0] + c.collectNodes() if c.report.HasErrors() { - return nil, nil + return nil } - return c.nodes, c.keys + return c.keys +} + +type nodeVisitTask struct { + fieldRef int + treeNodeData []int + treeNodeId uint +} + +func (c *nodesCollector) initVisitors() { + c.keys = make([]DSKeyInfo, 0, len(c.dataSources)) + c.dsVisitors = make([]*collectNodesDSVisitor, 0, len(c.dataSources)) + c.dsVisitorsReports = make([]*operationreport.Report, 0, len(c.dataSources)) + + // prepare visitors for each data source + for _, dataSource := range c.dataSources { + visitor := &collectNodesDSVisitor{ + operation: c.operation, + definition: c.definition, + nodes: c.nodes, + info: c.fieldInfo, + keys: make([]DSKeyInfo, 0, 2), + localSeenKeys: make(map[SeenKeyPath]struct{}), + localSuggestionLookup: make(map[int]struct{}), + globalSeenKeys: c.seenKeys, + } + visitor.dataSource = dataSource + visitor.notExternalKeyPaths = make(map[string]struct{}) + c.dsVisitors = append(c.dsVisitors, visitor) + c.dsVisitorsReports = append(c.dsVisitorsReports, operationreport.NewReport()) + } } func (c *nodesCollector) collectNodes() { + // collect fields to visit + nodesToVisit := make([]nodeVisitTask, 0, len(c.operation.Fields)) + for treeNodeID, treeNode := range TraverseBFS(c.nodes.responseTree) { + if treeNodeID == treeRootID { + continue + } + fieldRef := TreeNodeFieldRef(treeNodeID) - info := getFieldInfo(c.operation, c.definition) + if len(c.newFieldRefs) > 0 { + if _, ok := c.newFieldRefs[fieldRef]; !ok { + // skip field refs which were not added during the current iteration + continue + } + } + + task := nodeVisitTask{ + fieldRef: fieldRef, + treeNodeData: treeNode.GetData(), + treeNodeId: treeNodeID, + } + + nodesToVisit = append(nodesToVisit, task) + } wg := &sync.WaitGroup{} - wg.Add(len(c.dataSources)) - visitors := make([]*collectNodesVisitor, len(c.dataSources)) - reports := make([]*operationreport.Report, len(c.dataSources)) + wg.Add(len(c.dsVisitors)) // Create a semaphore if concurrency is limited var sem chan struct{} @@ -70,37 +127,19 @@ func (c *nodesCollector) collectNodes() { sem = make(chan struct{}, c.maxConcurrency) } - for i, dataSource := range c.dataSources { - walker := astvisitor.WalkerFromPool() - visitor := &collectNodesVisitor{ - operation: c.operation, - definition: c.definition, - walker: walker, - nodes: c.nodes, - info: info, - keys: make([]DSKeyInfo, 0, 2), - localSeenKeys: make(map[SeenKeyPath]struct{}), - globalSeenKeys: c.seenKeys, - } - walker.RegisterFieldVisitor(visitor) - visitor.dataSource = dataSource - visitor.notExternalKeyPaths = make(map[string]struct{}) - visitors[i] = visitor - report := operationreport.Report{} - reports[i] = &report - + for i, visitor := range c.dsVisitors { if sem != nil { sem <- struct{}{} } - go func(walker *astvisitor.Walker, report *operationreport.Report) { - defer wg.Done() + go func(visitor *collectNodesDSVisitor, report *operationreport.Report, nodesToVisit []nodeVisitTask) { + defer func() { + wg.Done() + }() defer func() { if sem != nil { <-sem } }() - defer walker.Release() - defer func() { // recover from panic and add it to the report if r := recover(); r != nil { @@ -108,12 +147,22 @@ func (c *nodesCollector) collectNodes() { } }() - walker.Walk(c.operation, c.definition, report) + // cleanup data from previous runs, but preserve indexes + visitor.reset() - }(walker, &report) + for _, node := range nodesToVisit { + if err := visitor.EnterField(node.fieldRef, node.treeNodeData, node.treeNodeId); err != nil { + report.AddInternalError(fmt.Errorf("data source %s: %v", visitor.dataSource.Name(), err)) + // stop processing on error + return + } + } + }(visitor, c.dsVisitorsReports[i], nodesToVisit) } + wg.Wait() - for _, report := range reports { + + for _, report := range c.dsVisitorsReports { if report.HasErrors() { for i := range report.ExternalErrors { c.report.AddExternalError(report.ExternalErrors[i]) @@ -125,11 +174,9 @@ func (c *nodesCollector) collectNodes() { } } - c.keys = make([]DSKeyInfo, 0, len(c.dataSources)) - // NOTE: collect nodes should never modify the tree, nodes or seen keys during the walk // it will be a data race - for _, visitor := range visitors { + for _, visitor := range c.dsVisitors { visitor.applySuggestions() c.keys = append(c.keys, visitor.keys...) @@ -141,13 +188,15 @@ func (c *nodesCollector) collectNodes() { } func (c *nodesCollector) buildTree() { - walker := astvisitor.NewWalker(32) + walker := astvisitor.NewWalkerWithID(32, "TreeBuilderVisitor") visitor := &treeBuilderVisitor{ operation: c.operation, definition: c.definition, walker: &walker, nodes: c.nodes, + fieldInfo: c.fieldInfo, } + walker.RegisterEnterDocumentVisitor(visitor) walker.RegisterFieldVisitor(visitor) walker.Walk(c.operation, c.definition, c.report) @@ -159,27 +208,30 @@ type treeBuilderVisitor struct { definition *ast.Document nodes *NodeSuggestions parentNodeIds []uint + fieldInfo map[int]fieldInfo } func (f *treeBuilderVisitor) EnterDocument(_, _ *ast.Document) { f.parentNodeIds = []uint{treeRootID} } -func (f *treeBuilderVisitor) EnterField(ref int) { - if f.nodes.IsFieldSeen(ref) { - currentNodeId := TreeNodeID(ref) +func (f *treeBuilderVisitor) EnterField(fieldRef int) { + if f.nodes.IsFieldSeen(fieldRef) { + currentNodeId := TreeNodeID(fieldRef) f.parentNodeIds = append(f.parentNodeIds, currentNodeId) return } - f.nodes.AddSeenField(ref) + f.nodes.AddSeenField(fieldRef) parentNodeId := f.currentParentID() - currentNodeId := TreeNodeID(ref) + currentNodeId := TreeNodeID(fieldRef) // we intentionally ignore the return values added, exists // because we do not recheck the same field refs, so all added nodes should be new and unique _, _ = f.nodes.responseTree.Add(currentNodeId, parentNodeId, nil) f.parentNodeIds = append(f.parentNodeIds, currentNodeId) + + f.collectFieldInfo(fieldRef) } func (f *treeBuilderVisitor) currentParentID() uint { @@ -195,14 +247,15 @@ func (f *treeBuilderVisitor) LeaveField(ref int) { } } -type collectNodesVisitor struct { - walker *astvisitor.Walker +type collectNodesDSVisitor struct { operation *ast.Document definition *ast.Document dataSource DataSource - localSuggestions []*NodeSuggestion - providesEntries []*NodeSuggestion + localSuggestions []*NodeSuggestion + localSuggestionLookup map[int]struct{} + + providesEntries []*NodeSuggestion nodes *NodeSuggestions @@ -215,7 +268,12 @@ type collectNodesVisitor struct { localSeenKeys map[SeenKeyPath]struct{} } -func (f *collectNodesVisitor) hasSuggestionForFieldOnCurrentDataSource(itemIds []int, ref int) (itemID int, ok bool) { +func (f *collectNodesDSVisitor) reset() { + f.localSuggestions = f.localSuggestions[:0] + f.keys = f.keys[:0] +} + +func (f *collectNodesDSVisitor) hasSuggestionForFieldOnCurrentDataSource(itemIds []int, ref int) (itemID int, ok bool) { idx := slices.IndexFunc(itemIds, func(i int) bool { suggestion := f.nodes.items[i] return suggestion.FieldRef == ref && suggestion.DataSourceHash == f.dataSource.Hash() @@ -228,19 +286,7 @@ func (f *collectNodesVisitor) hasSuggestionForFieldOnCurrentDataSource(itemIds [ return -1, false } -func (f *collectNodesVisitor) hasLocalSuggestion(ref int) (localItemID int, ok bool) { - idx := slices.IndexFunc(f.localSuggestions, func(suggestion *NodeSuggestion) bool { - return suggestion.FieldRef == ref - }) - - if idx != -1 { - return idx, true - } - - return -1, false -} - -func (f *collectNodesVisitor) hasProvidesConfiguration(typeName, fieldName string) (selectionSet string, ok bool) { +func (f *collectNodesDSVisitor) hasProvidesConfiguration(typeName, fieldName string) (selectionSet string, ok bool) { providesIdx := slices.IndexFunc(f.dataSource.FederationConfiguration().Provides, func(provide FederationFieldConfiguration) bool { return provide.TypeName == typeName && provide.FieldName == fieldName }) @@ -250,18 +296,18 @@ func (f *collectNodesVisitor) hasProvidesConfiguration(typeName, fieldName strin return f.dataSource.FederationConfiguration().Provides[providesIdx].SelectionSet, true } -func (f *collectNodesVisitor) isEntityInterface(typeName string) bool { +func (f *collectNodesDSVisitor) isEntityInterface(typeName string) bool { cfg := f.dataSource.FederationConfiguration() return cfg.HasEntityInterface(typeName) } -func (f *collectNodesVisitor) isInterfaceObject(typeName string) bool { +func (f *collectNodesDSVisitor) isInterfaceObject(typeName string) bool { cfg := f.dataSource.FederationConfiguration() return cfg.HasInterfaceObject(typeName) } // has disabled entity resolver -func (f *collectNodesVisitor) allKeysHasDisabledEntityResolver(typeName string) bool { +func (f *collectNodesDSVisitor) allKeysHasDisabledEntityResolver(typeName string) bool { keys := f.dataSource.FederationConfiguration().Keys if len(keys) == 0 { @@ -280,36 +326,34 @@ func (f *collectNodesVisitor) allKeysHasDisabledEntityResolver(typeName string) }) } -func (f *collectNodesVisitor) handleProvidesSuggestions(fieldRef int, typeName, fieldName, currentPath string) { +func (f *collectNodesDSVisitor) handleProvidesSuggestions(fieldRef int, typeName, fieldName, currentPath string, enclosingTypeDefinition ast.Node) error { if !f.operation.FieldHasSelections(fieldRef) { - return + return nil } providesSelectionSet, hasProvides := f.hasProvidesConfiguration(typeName, fieldName) if !hasProvides { - return + return nil } - if f.walker.EnclosingTypeDefinition.Kind != ast.NodeKindObjectTypeDefinition { - return + if enclosingTypeDefinition.Kind != ast.NodeKindObjectTypeDefinition { + return nil } - fieldDefRef, ok := f.definition.ObjectTypeDefinitionFieldWithName(f.walker.EnclosingTypeDefinition.Ref, f.operation.FieldNameBytes(fieldRef)) + fieldDefRef, ok := f.definition.ObjectTypeDefinitionFieldWithName(enclosingTypeDefinition.Ref, f.operation.FieldNameBytes(fieldRef)) if !ok { - return + return nil } fieldTypeName := f.definition.FieldDefinitionTypeNameString(fieldDefRef) providesFieldSet, report := providesFragment(fieldTypeName, providesSelectionSet, f.definition) if report.HasErrors() { - f.walker.StopWithInternalErr(fmt.Errorf("failed to parse provides fields for %s.%s at path %s: %v", typeName, fieldName, currentPath, report)) - return + return fmt.Errorf("failed to parse provides fields for %s.%s at path %s: %v", typeName, fieldName, currentPath, report) } selectionSetRef, ok := f.operation.FieldSelectionSet(fieldRef) if !ok { - f.walker.StopWithInternalErr(fmt.Errorf("failed to get selection set ref for %s.%s at path %s. Field with provides directive should have a selections", typeName, fieldName, currentPath)) - return + return fmt.Errorf("failed to get selection set ref for %s.%s at path %s. Field with provides directive should have a selections", typeName, fieldName, currentPath) } input := &providesInput{ @@ -323,19 +367,19 @@ func (f *collectNodesVisitor) handleProvidesSuggestions(fieldRef int, typeName, } providesSuggestions := providesSuggestions(input) if report.HasErrors() { - f.walker.StopWithInternalErr(fmt.Errorf("failed to get provides suggestions for %s.%s at path %s: %v", typeName, fieldName, currentPath, report)) - return + return fmt.Errorf("failed to get provides suggestions for %s.%s at path %s: %v", typeName, fieldName, currentPath, report) } f.providesEntries = append(f.providesEntries, providesSuggestions...) + return nil } -func (f *collectNodesVisitor) shouldAddUnionTypenameFieldSuggestion(info fieldInfo) bool { +func (f *collectNodesDSVisitor) shouldAddUnionTypenameFieldSuggestion(info fieldInfo) bool { if !info.isTypeName { return false } - if f.walker.EnclosingTypeDefinition.Kind != ast.NodeKindUnionTypeDefinition { + if info.enclosingTypeDefinition.Kind != ast.NodeKindUnionTypeDefinition { return false } @@ -355,48 +399,54 @@ func (f *collectNodesVisitor) shouldAddUnionTypenameFieldSuggestion(info fieldIn return node.Kind == ast.NodeKindUnionTypeDefinition } -func (f *collectNodesVisitor) isNotExternalKeyField(currentPath string) bool { +func (f *collectNodesDSVisitor) isNotExternalKeyField(currentPath string) bool { _, ok := f.notExternalKeyPaths[currentPath] return ok } -func (f *collectNodesVisitor) EnterField(fieldRef int) { +func (f *collectNodesDSVisitor) EnterField(fieldRef int, itemIds []int, treeNodeId uint) error { info, ok := f.info[fieldRef] if !ok { - return + return nil } - // add fields from provides directive on the current field - // it needs to be done each time we enter a field - // because we add provides suggestion only for a fields present in the query - TODO: we do not evaluate only fields present in a query anymore, so probably we could make it once, but currently provides is not cached anywhere - f.handleProvidesSuggestions(fieldRef, info.typeName, info.fieldName, info.currentPath) + if err := f.handleProvidesSuggestions(fieldRef, info.typeName, info.fieldName, info.currentPath, info.enclosingTypeDefinition); err != nil { + return err + } - // should be done after handling provides - f.collectKeysForPath(info.typeName, info.parentPath) + // For pubsub entities could also be a child node, so checking for only root nodes is not enough, so we check for entity keys existence + // when we have no keys, it is still expensive to create an index entry for a seen key path, + // so we skip check as a whole when there is no entity with such a name + if f.dataSource.HasEntity(info.typeName) { + // should be done after handling provides + if err := f.collectKeysForPath(info.typeName, info.parentPath); err != nil { + return err + } + } if info.possibleTypeNames != nil { - // We need to collect keys for all possible types of abstract type too + // We need to collect keys for all possible types of an abstract type too // because during initial planning we do not know yet if the abstract selection will be rewritten, // This means that in the unmodified query we could try to match abstract to concrete type, which won't match - // So we have to add possible choices for each of concrete types, to make this match possible + // So we have to add possible choices for each of the concrete types, to make this match possible for _, possibleTypeName := range info.possibleTypeNames { - f.collectKeysForPath(possibleTypeName, info.parentPath) + // for each of the possible typenames we also check if we have an entity + if f.dataSource.HasEntity(possibleTypeName) { + if err := f.collectKeysForPath(possibleTypeName, info.parentPath); err != nil { + return err + } + } } } - currentNodeId := TreeNodeID(fieldRef) - treeNode, _ := f.nodes.responseTree.Find(currentNodeId) - itemIds := treeNode.GetData() - - // TODO: use local seen fields - which survives between iterations - // this is the check for the global suggestions if _, ok := f.hasSuggestionForFieldOnCurrentDataSource(itemIds, fieldRef); ok { - return + return nil } // this is the check for the current collect nodes iterations suggestions - if _, ok := f.hasLocalSuggestion(fieldRef); ok { - return + // whether we already added a suggestion for the field with a typename + if _, hasLocalSuggestion := f.localSuggestionLookup[fieldRef]; hasLocalSuggestion { + return nil } isProvided := slices.ContainsFunc(f.providesEntries, func(suggestion *NodeSuggestion) bool { @@ -409,14 +459,15 @@ func (f *collectNodesVisitor) EnterField(fieldRef int) { // we will add a typename field to the interface object query in the datasource planner // at the same time we should allow to select a typename on the entity interface - return + return nil } + hasRootNodeWithTypename := f.dataSource.HasRootNodeWithTypename(info.typeName) // hasRootNode is true when: // - ds config has a root node for the field // - we have a root node with typename and the field is a __typename field // we no longer add a typename field for the root query nodes, as it is now handled by the planning visitor - hasRootNode := f.dataSource.HasRootNode(info.typeName, info.fieldName) || (info.isTypeName && f.dataSource.HasRootNodeWithTypename(info.typeName) && !IsMutationOrQueryRootType(info.typeName)) + hasRootNode := f.dataSource.HasRootNode(info.typeName, info.fieldName) || (info.isTypeName && hasRootNodeWithTypename && !IsMutationOrQueryRootType(info.typeName)) // hasChildNode is true when: // - ds config has a child node for the field @@ -460,18 +511,17 @@ func (f *collectNodesVisitor) EnterField(fieldRef int) { IsProvided: isProvided, IsLeaf: isLeaf, isTypeName: info.isTypeName, - treeNodeId: currentNodeId, + treeNodeId: treeNodeId, } f.localSuggestions = append(f.localSuggestions, &node) + f.localSuggestionLookup[fieldRef] = struct{}{} } -} - -func (f *collectNodesVisitor) LeaveField(ref int) { + return nil } -func (f *collectNodesVisitor) applySuggestions() { +func (f *collectNodesDSVisitor) applySuggestions() { for _, suggestion := range f.localSuggestions { f.nodes.addSuggestion(suggestion) itemId := len(f.nodes.items) - 1 @@ -490,6 +540,10 @@ func TreeNodeID(fieldRef int) uint { return uint(100 + fieldRef) } +func TreeNodeFieldRef(nodeID uint) int { + return int(nodeID - 100) +} + const ( queryTypeName = "Query" mutationTypeName = "Mutation" @@ -499,35 +553,16 @@ func IsMutationOrQueryRootType(typeName string) bool { return queryTypeName == typeName || mutationTypeName == typeName } -func getFieldInfo(operation, definition *ast.Document) map[int]fieldInfo { - walker := astvisitor.NewWalker(8) - visitor := &fieldInfoVisitor{ - walker: &walker, - operation: operation, - definition: definition, - infoCache: make(map[int]fieldInfo, len(operation.Fields)), - } - walker.RegisterEnterFieldVisitor(visitor) - report := &operationreport.Report{} - walker.Walk(operation, definition, report) - return visitor.infoCache -} - -type fieldInfoVisitor struct { - walker *astvisitor.Walker - operation, definition *ast.Document - infoCache map[int]fieldInfo -} - type fieldInfo struct { typeName, fieldName, fieldAliasOrName, parentPath, currentPath string onFragment, isTypeName bool parentPathWithoutFragment string possibleTypeNames []string currentPathWithoutFragments string + enclosingTypeDefinition ast.Node } -func (f *fieldInfoVisitor) EnterField(ref int) { +func (f *treeBuilderVisitor) collectFieldInfo(fieldRef int) { typeName := f.walker.EnclosingTypeDefinition.NameString(f.definition) var possibleTypes []string @@ -539,8 +574,8 @@ func (f *fieldInfoVisitor) EnterField(ref int) { default: } - fieldName := f.operation.FieldNameUnsafeString(ref) - fieldAliasOrName := f.operation.FieldAliasOrNameString(ref) + fieldName := f.operation.FieldNameUnsafeString(fieldRef) + fieldAliasOrName := f.operation.FieldAliasOrNameString(fieldRef) isTypeName := fieldName == typeNameField parentPath := f.walker.Path.DotDelimitedString() onFragment := f.walker.Path.EndsWithFragment() @@ -549,7 +584,7 @@ func (f *fieldInfoVisitor) EnterField(ref int) { currentPath := fmt.Sprintf("%s.%s", parentPath, fieldAliasOrName) currentPathWithoutFragments := fmt.Sprintf("%s.%s", parentPathWithoutFragment, fieldAliasOrName) - f.infoCache[ref] = fieldInfo{ + f.fieldInfo[fieldRef] = fieldInfo{ typeName: typeName, possibleTypeNames: possibleTypes, fieldName: fieldName, @@ -560,5 +595,6 @@ func (f *fieldInfoVisitor) EnterField(ref int) { parentPathWithoutFragment: parentPathWithoutFragment, currentPathWithoutFragments: currentPathWithoutFragments, isTypeName: isTypeName, + enclosingTypeDefinition: f.walker.EnclosingTypeDefinition, } } diff --git a/v2/pkg/engine/plan/datasource_filter_node_suggestions.go b/v2/pkg/engine/plan/datasource_filter_node_suggestions.go index 12857c143c..9722949cc4 100644 --- a/v2/pkg/engine/plan/datasource_filter_node_suggestions.go +++ b/v2/pkg/engine/plan/datasource_filter_node_suggestions.go @@ -3,8 +3,10 @@ package plan import ( "encoding/json" "fmt" + "iter" "github.com/kingledion/go-tools/tree" + "github.com/phf/go-queue/queue" ) const treeRootID = ^uint(0) @@ -95,6 +97,28 @@ type NodeSuggestions struct { responseTree tree.Tree[[]int] } +func TraverseBFS(data tree.Tree[[]int]) iter.Seq2[uint, tree.Node[[]int]] { + return func(yield func(uint, tree.Node[[]int]) bool) { + q := queue.New() + q.PushBack(data.Root()) + + for { + current := q.PopFront() + switch node := current.(type) { + case tree.Node[[]int]: + for _, child := range node.GetChildren() { + q.PushBack(child) + } + if !yield(node.GetID(), node) { + return + } + case nil: + return + } + } + } +} + func NewNodeSuggestions() *NodeSuggestions { return NewNodeSuggestionsWithSize(32) } diff --git a/v2/pkg/engine/plan/datasource_filter_visitor.go b/v2/pkg/engine/plan/datasource_filter_visitor.go index f0ba72c9b7..50a2ed2d1a 100644 --- a/v2/pkg/engine/plan/datasource_filter_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_visitor.go @@ -3,8 +3,6 @@ package plan import ( "slices" - "github.com/kingledion/go-tools/tree" - "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" ) @@ -24,20 +22,24 @@ type DataSourceFilter struct { secondaryRun bool fieldDependsOn map[int][]int + newFieldRefs map[int]struct{} dataSources []DataSource jumpsForPathForTypename map[KeyIndex]*DataSourceJumpsGraph dsHashesHavingKeys map[DSHash]struct{} - seenKeys map[SeenKeyPath]struct{} maxDataSourceCollectorsConcurrency uint + nodesCollector *nodesCollector } -func NewDataSourceFilter(operation, definition *ast.Document, report *operationreport.Report) *DataSourceFilter { +func NewDataSourceFilter(operation, definition *ast.Document, report *operationreport.Report, dataSources []DataSource, newFieldRefs map[int]struct{}) *DataSourceFilter { return &DataSourceFilter{ - operation: operation, - definition: definition, - report: report, + operation: operation, + definition: definition, + report: report, + dataSources: dataSources, + nodes: NewNodeSuggestions(), + newFieldRefs: newFieldRefs, } } @@ -51,22 +53,21 @@ func (f *DataSourceFilter) WithMaxDataSourceCollectorsConcurrency(maxConcurrency return f } -func (f *DataSourceFilter) FilterDataSources(dataSources []DataSource, existingNodes *NodeSuggestions, landedTo map[int]DSHash, fieldDependsOn map[int][]int) (used []DataSource, suggestions *NodeSuggestions) { +func (f *DataSourceFilter) FilterDataSources(landedTo map[int]DSHash, fieldDependsOn map[int][]int) (used []DataSource, suggestions *NodeSuggestions) { var dsInUse map[DSHash]struct{} f.fieldDependsOn = fieldDependsOn - f.dataSources = dataSources - suggestions, dsInUse = f.findBestDataSourceSet(existingNodes, landedTo) + suggestions, dsInUse = f.findBestDataSourceSet(landedTo) if f.report.HasErrors() { return } used = make([]DataSource, 0, len(dsInUse)) - for i := range dataSources { - _, inUse := dsInUse[dataSources[i].Hash()] + for i := range f.dataSources { + _, inUse := dsInUse[f.dataSources[i].Hash()] if inUse { - used = append(used, dataSources[i]) + used = append(used, f.dataSources[i]) } } @@ -74,8 +75,8 @@ func (f *DataSourceFilter) FilterDataSources(dataSources []DataSource, existingN return used, suggestions } -func (f *DataSourceFilter) findBestDataSourceSet(existingNodes *NodeSuggestions, landedTo map[int]DSHash) (*NodeSuggestions, map[DSHash]struct{}) { - f.collectNodes(f.dataSources, existingNodes) +func (f *DataSourceFilter) findBestDataSourceSet(landedTo map[int]DSHash) (*NodeSuggestions, map[DSHash]struct{}) { + f.collectNodes() if f.report.HasErrors() { return nil, nil } @@ -139,27 +140,24 @@ func (f *DataSourceFilter) applyLandedTo(landedTo map[int]DSHash) { } -func (f *DataSourceFilter) collectNodes(dataSources []DataSource, existingNodes *NodeSuggestions) { - if existingNodes == nil { - existingNodes = NewNodeSuggestions() - } - - if f.seenKeys == nil { - f.seenKeys = make(map[SeenKeyPath]struct{}) - } +func (f *DataSourceFilter) collectNodes() { + if f.nodesCollector == nil { + f.nodesCollector = &nodesCollector{ + operation: f.operation, + definition: f.definition, + dataSources: f.dataSources, + nodes: f.nodes, + report: f.report, + maxConcurrency: f.maxDataSourceCollectorsConcurrency, + seenKeys: make(map[SeenKeyPath]struct{}), + fieldInfo: make(map[int]fieldInfo), + newFieldRefs: f.newFieldRefs, + } - nodesCollector := &nodesCollector{ - operation: f.operation, - definition: f.definition, - dataSources: dataSources, - nodes: existingNodes, - report: f.report, - maxConcurrency: f.maxDataSourceCollectorsConcurrency, - seenKeys: f.seenKeys, + f.nodesCollector.initVisitors() } - var keysInfo []DSKeyInfo - f.nodes, keysInfo = nodesCollector.CollectNodes() + keysInfo := f.nodesCollector.CollectNodes() if f.dsHashesHavingKeys == nil { f.dsHashesHavingKeys = make(map[DSHash]struct{}) @@ -184,8 +182,8 @@ func (f *DataSourceFilter) collectNodes(dataSources []DataSource, existingNodes } usedDsHashes := make([]DSHash, 0, len(f.dsHashesHavingKeys)) - // iterate over datasources to have deterministic order - for _, ds := range dataSources { + // iterate over datasources to have a deterministic order + for _, ds := range f.dataSources { if _, ok := f.dsHashesHavingKeys[ds.Hash()]; ok { usedDsHashes = append(usedDsHashes, ds.Hash()) } @@ -383,14 +381,11 @@ func (f *DataSourceFilter) assignKeys(itemIdx int, parentNodeIndexes []int) { // On a second run in additional to all the checks from the first run // we select nodes which was not chosen by previous stages, so we just pick first available datasource func (f *DataSourceFilter) selectDuplicateNodes(secondPass bool) { - - treeNodes := f.nodes.responseTree.Traverse(tree.TraverseBreadthFirst) - - for treeNode := range treeNodes { + for id, treeNode := range TraverseBFS(f.nodes.responseTree) { // f.nodes.printNodes("nodes") // fmt.Println() - if treeNode.GetID() == treeRootID { + if id == treeRootID { continue } diff --git a/v2/pkg/engine/plan/datasource_filter_visitor_test.go b/v2/pkg/engine/plan/datasource_filter_visitor_test.go index dc01dfe4b5..24e691adab 100644 --- a/v2/pkg/engine/plan/datasource_filter_visitor_test.go +++ b/v2/pkg/engine/plan/datasource_filter_visitor_test.go @@ -940,8 +940,8 @@ func TestFindBestDataSourceSet(t *testing.T) { suggestions: newNodeSuggestions([]NodeSuggestion{ {TypeName: "Query", FieldName: "user", DataSourceHash: 11, Path: "query.user", ParentPath: "query", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage1: unique"}}, {TypeName: "User", FieldName: "id", DataSourceHash: 11, Path: "query.user.id", ParentPath: "query.user", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage2: node on the same source as selected parent"}}, - {TypeName: "User", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, {TypeName: "User", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, + {TypeName: "User", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, {TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, {TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, {TypeName: "NestedTwo", FieldName: "shared", DataSourceHash: 11, Path: "query.user.nested.nested.shared", ParentPath: "query.user.nested.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage2: node on the same source as selected parent"}}, @@ -958,10 +958,10 @@ func TestFindBestDataSourceSet(t *testing.T) { suggestions: newNodeSuggestions([]NodeSuggestion{ {TypeName: "Query", FieldName: "user", DataSourceHash: 11, Path: "query.user", ParentPath: "query", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage1: unique"}}, {TypeName: "User", FieldName: "id", DataSourceHash: 11, Path: "query.user.id", ParentPath: "query.user", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage2: node on the same source as selected parent"}}, - {TypeName: "User", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, {TypeName: "User", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, - {TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, + {TypeName: "User", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, {TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, + {TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage1: same source parent of unique node"}}, {TypeName: "NestedTwo", FieldName: "shared", DataSourceHash: 22, Path: "query.user.nested.nested.shared", ParentPath: "query.user.nested.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage2: node on the same source as selected parent"}}, {TypeName: "NestedTwo", FieldName: "uniqueOne", DataSourceHash: 11, Path: "query.user.nested.nested.uniqueOne", ParentPath: "query.user.nested.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage1: unique"}}, {TypeName: "NestedTwo", FieldName: "uniqueTwo", DataSourceHash: 22, Path: "query.user.nested.nested.uniqueTwo", ParentPath: "query.user.nested.nested", IsRootNode: false, Selected: true, SelectionReasons: []string{"stage1: unique"}}, @@ -1039,13 +1039,12 @@ func TestFindBestDataSourceSet(t *testing.T) { t.Fatal(report.Error()) } - dsFilter := NewDataSourceFilter(&operation, &definition, &report) + dsFilter := NewDataSourceFilter(&operation, &definition, &report, DataSources, nil) if report.HasErrors() { t.Fatal(report.Error()) } dsFilter.EnableSelectionReasons() - dsFilter.dataSources = DataSources - planned, _ := dsFilter.findBestDataSourceSet(nil, nil) + planned, _ := dsFilter.findBestDataSourceSet(nil) if report.HasErrors() { t.Fatal(report.Error()) } diff --git a/v2/pkg/engine/plan/federation_metadata.go b/v2/pkg/engine/plan/federation_metadata.go index 994ea9539f..ce1290eea6 100644 --- a/v2/pkg/engine/plan/federation_metadata.go +++ b/v2/pkg/engine/plan/federation_metadata.go @@ -13,6 +13,8 @@ type FederationMetaData struct { Provides FederationFieldConfigurations EntityInterfaces []EntityInterfaceConfiguration InterfaceObjects []EntityInterfaceConfiguration + + entityTypeNames map[string]struct{} } type FederationInfo interface { @@ -34,7 +36,8 @@ func (d *FederationMetaData) RequiredFieldsByKey(typeName string) []FederationFi } func (d *FederationMetaData) HasEntity(typeName string) bool { - return len(d.Keys.FilterByTypeAndResolvability(typeName, false)) > 0 + _, ok := d.entityTypeNames[typeName] + return ok } func (d *FederationMetaData) RequiredFieldsByRequires(typeName, fieldName string) (cfg FederationFieldConfiguration, exists bool) { diff --git a/v2/pkg/engine/plan/key_fields_visitor.go b/v2/pkg/engine/plan/key_fields_visitor.go index a2c3151799..1e018d56dd 100644 --- a/v2/pkg/engine/plan/key_fields_visitor.go +++ b/v2/pkg/engine/plan/key_fields_visitor.go @@ -87,7 +87,7 @@ type Entity { */ -func (f *collectNodesVisitor) collectKeysForPath(typeName, parentPath string) { +func (f *collectNodesDSVisitor) collectKeysForPath(typeName, parentPath string) error { indexKey := SeenKeyPath{ TypeName: typeName, Path: parentPath, @@ -95,12 +95,12 @@ func (f *collectNodesVisitor) collectKeysForPath(typeName, parentPath string) { } // global seen keys is used when we recollect nodes if _, ok := f.globalSeenKeys[indexKey]; ok { - return + return nil } // local seen fields is used when we have multipe fields on a path, and we visit it first time if _, ok := f.localSeenKeys[indexKey]; ok { // we already collected keys for this path - return + return nil } // WARNING: we are not writing to global map from go routine f.localSeenKeys[indexKey] = struct{}{} @@ -108,7 +108,7 @@ func (f *collectNodesVisitor) collectKeysForPath(typeName, parentPath string) { allKeys := f.dataSource.FederationConfiguration().Keys keys := allKeys.FilterByTypeAndResolvability(typeName, false) if len(keys) == 0 { - return + return nil } typeNameKeys := make([]KeyInfo, 0, len(keys)) @@ -130,8 +130,7 @@ func (f *collectNodesVisitor) collectKeysForPath(typeName, parentPath string) { keyPaths, hasExternalFields := getKeyPaths(input) if report.HasErrors() { - f.walker.StopWithInternalErr(report) - return + return report } target := !key.DisableEntityResolver @@ -168,10 +167,12 @@ func (f *collectNodesVisitor) collectKeysForPath(typeName, parentPath string) { Path: parentPath, Keys: typeNameKeys, }) + + return nil } func getKeyPaths(input *keyVisitorInput) (keyPaths []KeyInfoFieldPath, hasExternalFields bool) { - walker := astvisitor.NewWalker(48) + walker := astvisitor.NewWalkerWithID(48, "KeyInfoVisitor") visitor := &keyInfoVisitor{ walker: &walker, input: input, diff --git a/v2/pkg/engine/plan/key_fields_visitor_test.go b/v2/pkg/engine/plan/key_fields_visitor_test.go index 39a83f16ce..7cb91112ce 100644 --- a/v2/pkg/engine/plan/key_fields_visitor_test.go +++ b/v2/pkg/engine/plan/key_fields_visitor_test.go @@ -483,7 +483,7 @@ func TestCollectKeysForPath(t *testing.T) { t.Run(c.name, func(t *testing.T) { definition := unsafeparser.ParseGraphqlDocumentStringWithBaseSchema(c.definition) - collectNodesVisitor := &collectNodesVisitor{ + collectNodesVisitor := &collectNodesDSVisitor{ definition: &definition, dataSource: c.dataSource, providesEntries: c.providesEntries, @@ -492,9 +492,9 @@ func TestCollectKeysForPath(t *testing.T) { notExternalKeyPaths: make(map[string]struct{}), } - collectNodesVisitor.collectKeysForPath(c.typeName, c.parentPath) + assert.NoError(t, collectNodesVisitor.collectKeysForPath(c.typeName, c.parentPath)) // call it again to test the deduplication - collectNodesVisitor.collectKeysForPath(c.typeName, c.parentPath) + assert.NoError(t, collectNodesVisitor.collectKeysForPath(c.typeName, c.parentPath)) assert.Equal(t, len(c.expectKeys), len(collectNodesVisitor.keys)) assert.Equal(t, c.expectKeys, collectNodesVisitor.keys) diff --git a/v2/pkg/engine/plan/node_selection_builder.go b/v2/pkg/engine/plan/node_selection_builder.go index a9e48803e1..3ad400a5ec 100644 --- a/v2/pkg/engine/plan/node_selection_builder.go +++ b/v2/pkg/engine/plan/node_selection_builder.go @@ -50,10 +50,11 @@ type NodeSelectionResult struct { } func NewNodeSelectionBuilder(config *Configuration) *NodeSelectionBuilder { - nodeSelectionsWalker := astvisitor.NewWalker(48) + nodeSelectionsWalker := astvisitor.NewWalkerWithID(48, "NodeSelectionsWalker") nodeSelectionVisitor := &nodeSelectionVisitor{ walker: &nodeSelectionsWalker, addTypenameInNestedSelections: config.ValidateRequiredExternalFields, + newFieldRefs: make(map[int]struct{}), } nodeSelectionsWalker.RegisterEnterDocumentVisitor(nodeSelectionVisitor) @@ -61,7 +62,7 @@ func NewNodeSelectionBuilder(config *Configuration) *NodeSelectionBuilder { nodeSelectionsWalker.RegisterEnterOperationVisitor(nodeSelectionVisitor) nodeSelectionsWalker.RegisterSelectionSetVisitor(nodeSelectionVisitor) - nodeResolvableWalker := astvisitor.NewWalker(32) + nodeResolvableWalker := astvisitor.NewWalkerWithID(32, "NodeResolvableWalker") nodeResolvableVisitor := &nodesResolvableVisitor{ walker: &nodeResolvableWalker, } @@ -83,10 +84,11 @@ func (p *NodeSelectionBuilder) SetOperationName(name string) { func (p *NodeSelectionBuilder) ResetSkipFieldRefs() { p.nodeSelectionsVisitor.skipFieldsRefs = nil + p.nodeSelectionsVisitor.newFieldRefs = make(map[int]struct{}) } func (p *NodeSelectionBuilder) SelectNodes(operation, definition *ast.Document, report *operationreport.Report) (out *NodeSelectionResult) { - dsFilter := NewDataSourceFilter(operation, definition, report) + dsFilter := NewDataSourceFilter(operation, definition, report, p.config.DataSources, p.nodeSelectionsVisitor.newFieldRefs) if p.config.Debug.PrintNodeSuggestions { dsFilter.EnableSelectionReasons() @@ -101,7 +103,7 @@ func (p *NodeSelectionBuilder) SelectNodes(operation, definition *ast.Document, // set initial suggestions and used data sources p.nodeSelectionsVisitor.dataSources, p.nodeSelectionsVisitor.nodeSuggestions = - dsFilter.FilterDataSources(p.config.DataSources, nil, nil, nil) + dsFilter.FilterDataSources(nil, nil) if report.HasErrors() { return } @@ -136,7 +138,7 @@ func (p *NodeSelectionBuilder) SelectNodes(operation, definition *ast.Document, if p.nodeSelectionsVisitor.hasNewFields { // update suggestions for the new required fields p.nodeSelectionsVisitor.dataSources, p.nodeSelectionsVisitor.nodeSuggestions = - dsFilter.FilterDataSources(p.config.DataSources, p.nodeSelectionsVisitor.nodeSuggestions, p.nodeSelectionsVisitor.fieldLandedTo, p.nodeSelectionsVisitor.fieldRefDependsOn) + dsFilter.FilterDataSources(p.nodeSelectionsVisitor.fieldLandedTo, p.nodeSelectionsVisitor.fieldRefDependsOn) if report.HasErrors() { return } diff --git a/v2/pkg/engine/plan/node_selection_visitor.go b/v2/pkg/engine/plan/node_selection_visitor.go index d173e74ea7..0345af2ec1 100644 --- a/v2/pkg/engine/plan/node_selection_visitor.go +++ b/v2/pkg/engine/plan/node_selection_visitor.go @@ -50,6 +50,20 @@ type nodeSelectionVisitor struct { // addTypenameInNestedSelections controls forced addition of __typename to nested selection sets // used by "requires" keys, not only when fragments are present. addTypenameInNestedSelections bool + + newFieldRefs map[int]struct{} +} + +func (c *nodeSelectionVisitor) addSkipFieldRefs(fieldRefs ...int) { + c.skipFieldsRefs = append(c.skipFieldsRefs, fieldRefs...) + + c.addNewFieldRefs(fieldRefs...) +} + +func (c *nodeSelectionVisitor) addNewFieldRefs(fieldRefs ...int) { + for _, fieldRef := range fieldRefs { + c.newFieldRefs[fieldRef] = struct{}{} + } } type fieldDependencyKey struct { @@ -483,7 +497,7 @@ func (c *nodeSelectionVisitor) addFieldRequirementsToOperation(selectionSetRef i } c.resetVisitedAbstractChecksForModifiedFields(addFieldsResult.modifiedFieldRefs) - c.skipFieldsRefs = append(c.skipFieldsRefs, addFieldsResult.skipFieldRefs...) + c.addSkipFieldRefs(addFieldsResult.skipFieldRefs...) // add mapping for the field dependencies for _, requestedByFieldRef := range requirements.requestedByFieldRefs { fieldKey := fieldIndexKey{requestedByFieldRef, requirements.dsHash} @@ -573,7 +587,7 @@ func (c *nodeSelectionVisitor) addKeyRequirementsToOperation(selectionSetRef int // op, _ := astprinter.PrintStringIndentDebug(c.operation, " ") // fmt.Println("operation: ", op) - c.skipFieldsRefs = append(c.skipFieldsRefs, addFieldsResult.skipFieldRefs...) + c.addSkipFieldRefs(addFieldsResult.skipFieldRefs...) // setup deps between key chain items if currentFieldRefs != nil && previousJump != nil { @@ -656,7 +670,7 @@ func (c *nodeSelectionVisitor) rewriteSelectionSetHavingAbstractFragments(fieldR return } - c.skipFieldsRefs = append(c.skipFieldsRefs, rewriter.skipFieldRefs...) + c.addSkipFieldRefs(rewriter.skipFieldRefs...) c.hasNewFields = true c.rewrittenFieldRefs = append(c.rewrittenFieldRefs, fieldRef) @@ -699,4 +713,8 @@ func (c *nodeSelectionVisitor) updateFieldDependsOn(changedFieldRefs map[int][]i c.fieldRefDependsOn[key] = updatedFieldRefs } + + for _, newRefs := range changedFieldRefs { + c.addNewFieldRefs(newRefs...) + } } diff --git a/v2/pkg/engine/plan/path_builder.go b/v2/pkg/engine/plan/path_builder.go index e9d8e306f3..7483e1b8bd 100644 --- a/v2/pkg/engine/plan/path_builder.go +++ b/v2/pkg/engine/plan/path_builder.go @@ -17,7 +17,7 @@ type PathBuilder struct { } func NewPathBuilder(config *Configuration) *PathBuilder { - walker := astvisitor.NewWalker(48) + walker := astvisitor.NewWalkerWithID(48, "PathBuilderWalker") visitor := &pathBuilderVisitor{ walker: &walker, fieldConfigurations: config.Fields, diff --git a/v2/pkg/engine/plan/planner.go b/v2/pkg/engine/plan/planner.go index 25d6e89608..77f1de78ce 100644 --- a/v2/pkg/engine/plan/planner.go +++ b/v2/pkg/engine/plan/planner.go @@ -53,12 +53,12 @@ func NewPlanner(config Configuration) (*Planner, error) { config.EntityInterfaceNames = entityInterfaceNames // prepare operation walker handles internal normalization for planner - prepareOperationWalker := astvisitor.NewWalker(48) + prepareOperationWalker := astvisitor.NewWalkerWithID(48, "PrepareOperationWalker") astnormalization.InlineFragmentAddOnType(&prepareOperationWalker) // planning - planningWalker := astvisitor.NewWalker(48) + planningWalker := astvisitor.NewWalkerWithID(48, "PlanningWalker") planningVisitor := &Visitor{ Walker: &planningWalker, fieldConfigs: map[int]*FieldConfiguration{}, diff --git a/v2/pkg/engine/plan/provides_fields_visitor.go b/v2/pkg/engine/plan/provides_fields_visitor.go index bc3271fbb7..50cf19eed2 100644 --- a/v2/pkg/engine/plan/provides_fields_visitor.go +++ b/v2/pkg/engine/plan/provides_fields_visitor.go @@ -39,7 +39,7 @@ func (a *addTypenamesVisitor) EnterSelectionSet(ref int) { } func addTypenames(operation, definition *ast.Document, report *operationreport.Report) { - walker := astvisitor.NewWalker(32) + walker := astvisitor.NewWalkerWithID(32, "AddTypenamesVisitor") visitor := &addTypenamesVisitor{ walker: &walker, providesFieldSet: operation, @@ -66,7 +66,7 @@ func providesFragment(fieldTypeName string, providesSelectionSet string, definit } func providesSuggestions(input *providesInput) []*NodeSuggestion { - walker := astvisitor.NewWalker(48) + walker := astvisitor.NewWalkerWithID(48, "ProvidesVisitor") visitor := &providesVisitor{ walker: &walker, diff --git a/v2/pkg/engine/plan/required_fields_visitor.go b/v2/pkg/engine/plan/required_fields_visitor.go index b14b3116b7..4f3d479915 100644 --- a/v2/pkg/engine/plan/required_fields_visitor.go +++ b/v2/pkg/engine/plan/required_fields_visitor.go @@ -64,7 +64,7 @@ func addRequiredFields(config *addRequiredFieldsConfiguration) (out AddRequiredF return out, report } - walker := astvisitor.NewWalker(4) + walker := astvisitor.NewWalkerWithID(4, "RequiredFieldsVisitor") visitor := &requiredFieldsVisitor{ Walker: &walker, diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index ef8d094757..ebbd0d5c16 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -391,7 +391,7 @@ func (v *Visitor) EnterField(ref int) { v.currentField.Value = str } } else { - path := v.resolveFieldPath(ref) + path := []string{v.Operation.FieldAliasOrNameString(ref)} v.currentField.Value = v.resolveFieldValue(ref, fieldDefinitionTypeRef, true, path) } @@ -1020,6 +1020,7 @@ func (v *Visitor) EnterOperationDefinition(ref int) { } } +// TODO: cleanup - field alias override logic is disabled func (v *Visitor) resolveFieldPath(ref int) []string { typeName := v.Walker.EnclosingTypeDefinition.NameString(v.Definition) fieldName := v.Operation.FieldNameUnsafeString(ref) diff --git a/v2/pkg/variablesvalidation/variablesvalidation.go b/v2/pkg/variablesvalidation/variablesvalidation.go index 7f15a365b4..d9631739ee 100644 --- a/v2/pkg/variablesvalidation/variablesvalidation.go +++ b/v2/pkg/variablesvalidation/variablesvalidation.go @@ -78,7 +78,7 @@ type VariablesValidatorOptions struct { } func NewVariablesValidator(options VariablesValidatorOptions) *VariablesValidator { - walker := astvisitor.NewWalker(8) + walker := astvisitor.NewWalkerWithID(8, "VariablesValidator") visitor := &variablesVisitor{ walker: &walker, opts: options,