diff --git a/lib/graphql/analysis/ast/query_complexity.rb b/lib/graphql/analysis/ast/query_complexity.rb index 5c55e2dabf..83ccf52f04 100644 --- a/lib/graphql/analysis/ast/query_complexity.rb +++ b/lib/graphql/analysis/ast/query_complexity.rb @@ -44,6 +44,10 @@ def initialize(parent_type, field_definition, query, response_path) def own_complexity(child_complexity) @field_definition.calculate_complexity(query: @query, nodes: @nodes, child_complexity: child_complexity) end + + def composite? + !empty? + end end def on_enter_field(node, parent, visitor) @@ -145,35 +149,38 @@ def merged_max_complexity(query, inner_selections) # Add up the total cost for each unique field name's coalesced selections unique_field_keys.each_key.reduce(0) do |total, field_key| - composite_scopes = nil - field_cost = 0 - - # Collect composite selection scopes for further aggregation, - # leaf selections report their costs directly. - inner_selections.each do |inner_selection| - child_scope = inner_selection[field_key] - next unless child_scope - - # Empty child scopes are leaf nodes with zero child complexity. - if child_scope.empty? - field_cost = child_scope.own_complexity(0) - field_complexity(child_scope, max_complexity: field_cost, child_complexity: nil) + # Collect all child scopes for this field key; + # all keys come with at least one scope. + child_scopes = inner_selections.filter_map { _1[field_key] } + + # Compute maximum possible cost of child selections; + # composites merge their maximums, while leaf scopes are always zero. + # FieldsWillMerge validation assures all scopes are uniformly composite or leaf. + maximum_children_cost = if child_scopes.any?(&:composite?) + merged_max_complexity_for_scopes(query, child_scopes) + else + 0 + end + + # Identify the maximum cost and scope among possibilities + maximum_cost = 0 + maximum_scope = child_scopes.reduce(child_scopes.last) do |max_scope, possible_scope| + scope_cost = possible_scope.own_complexity(maximum_children_cost) + if scope_cost > maximum_cost + maximum_cost = scope_cost + possible_scope else - composite_scopes ||= [] - composite_scopes << child_scope + max_scope end end - if composite_scopes - child_complexity = merged_max_complexity_for_scopes(query, composite_scopes) - - # This is the last composite scope visited; assume it's representative (for backwards compatibility). - # Note: it would be more correct to score each composite scope and use the maximum possibility. - field_cost = composite_scopes.last.own_complexity(child_complexity) - field_complexity(composite_scopes.last, max_complexity: field_cost, child_complexity: child_complexity) - end + field_complexity( + maximum_scope, + max_complexity: maximum_cost, + child_complexity: maximum_children_cost, + ) - total + field_cost + total + maximum_cost end end end diff --git a/spec/graphql/analysis/ast/query_complexity_spec.rb b/spec/graphql/analysis/ast/query_complexity_spec.rb index ca5c66ad88..817c4d1b78 100644 --- a/spec/graphql/analysis/ast/query_complexity_spec.rb +++ b/spec/graphql/analysis/ast/query_complexity_spec.rb @@ -642,10 +642,109 @@ def field_complexity(scoped_type_complexity, max_complexity:, child_complexity:) field_complexities = reduce_result.first assert_equal({ - ['cheese', 'id'] => { max_complexity: 1, child_complexity: nil }, - ['cheese', 'flavor'] => { max_complexity: 1, child_complexity: nil }, + ['cheese', 'id'] => { max_complexity: 1, child_complexity: 0 }, + ['cheese', 'flavor'] => { max_complexity: 1, child_complexity: 0 }, ['cheese'] => { max_complexity: 3, child_complexity: 2 }, }, field_complexities) end end + + describe "maximum of possible scopes regardless of selection order" do + class MaxOfPossibleScopes < GraphQL::Schema + class Cheese < GraphQL::Schema::Object + field :kind, String + end + + module Producer + include GraphQL::Schema::Interface + field :cheese, Cheese, complexity: 5 + field :name, String, complexity: 5 + end + + class Farm < GraphQL::Schema::Object + implements Producer + field :cheese, Cheese, complexity: 10 + field :name, String, complexity: 10 + end + + class Entity < GraphQL::Schema::Union + possible_types Farm + end + + class Query < GraphQL::Schema::Object + field :entity, Entity + end + + def self.resolve_type + Farm + end + + def self.cost(query_string) + GraphQL::Analysis::AST.analyze_query( + GraphQL::Query.new(self, query_string), + [GraphQL::Analysis::AST::QueryComplexity], + ).first + end + + query(Query) + orphan_types(Producer) + end + + it "uses maximum of merged composite fields, regardless of selection order" do + a = MaxOfPossibleScopes.cost(%| + { + entity { + ...on Producer { cheese { kind } } + ...on Farm { cheese { kind } } + } + } + |) + + b = MaxOfPossibleScopes.cost(%| + { + entity { + ...on Farm { cheese { kind } } + ...on Producer { cheese { kind } } + } + } + |) + + assert_equal 0, a - b + end + + it "uses maximum of merged leaf fields, regardless of selection order" do + a = MaxOfPossibleScopes.cost(%| + { + entity { + ...on Producer { name } + ...on Farm { name } + } + } + |) + + b = MaxOfPossibleScopes.cost(%| + { + entity { + ...on Farm { name } + ...on Producer { name } + } + } + |) + + assert_equal 0, a - b + end + + it "invalid mismatched scope types will still compute without error" do + cost = MaxOfPossibleScopes.cost(%| + { + entity { + ...on Farm { cheese { kind } } + ...on Producer { cheese: name } + } + } + |) + + assert_equal 12, cost + end + end end