Query complexity is missing on fields included by fragments

[jacor84]

Description

QueryComplexity rule implements visitor in such a way that results in missing field definitions when non-inline fragments are used and defined after the operation (as it usually is). If you reverse the order and put fragments first, all works fine.

Reproduction path

A simple reproduction script is attaching a custom complexity closure to ExampleType fields. They are included using a fragment, and should result in a complexity error. However, no such error is triggered, because these field definitions are missing from fieldNodeAndDefs, and a default complexity function is used on them.

<?php

use GraphQL\GraphQL;
use GraphQL\Utils\BuildSchema;
use GraphQL\Validator\Rules\QueryComplexity;

require_once __DIR__ . '/vendor/autoload.php';

$exampleSchema = '
  type Query {
    example: ExampleType
  }

  type ExampleType {
    a: Int
    b: String
  }
';

$query = '
  query {
    ...exampleFragment
  }

  fragment exampleFragment on Query { # values from default complexity fn
    example { # 2 + 1
      a
      b
    }
  }
';

$typeConfigDecorator = function (array $typeConfig): array {
    if ($typeConfig['name'] !== 'ExampleType') {
        return $typeConfig;
    }
    $typeConfig['fields'] = static function () use ($typeConfig) {
        $fieldDefinitions = $typeConfig['fields']();
        foreach ($fieldDefinitions as $fieldName => &$fieldDefinition) {
            $fieldDefinition['complexity'] = function ($childComplexity) {
                return $childComplexity + 1000;
            };
        }

        return $fieldDefinitions;
    };

    return $typeConfig;
};

$schema = BuildSchema::build($exampleSchema, $typeConfigDecorator);

$queryComplexity = new QueryComplexity(100);

$result = GraphQL::executeQuery(
    $schema,
    $query,
    null,
    null,
    null,
    null,
    null,
    [
        $queryComplexity
    ]
);
echo json_encode($result->toArray(), JSON_PRETTY_PRINT);

Put it in an empty directory, call composer require webonyx/graphql-php and run php complexity.php.

[jacor84]

@spawnia Any thoughts on this? We were only able to fix the issue when we introduced deep changes to the way this class works, like only handling NodeKind::VARIABLE_DEFINITION and leave on NodeKind::OPERATION_DEFINITION, but with recursive selection set handling. I can't think of any way a visitor running sequentially could handle the task.

[spawnia]

For many complex implementations, we use graphql-js as a baseline. It has countless hours and contributions poured into it and is usually correct. It seems like we do not have that luxury here, but might consider https://github.com/slicknode/graphql-query-complexity for inspiration.

[kinimodmeyer]

If you reverse the order and put fragments first, all works fine

When i get it right then you can fix the mentioned issue at least with that easy workaround:

$parser = Parser::parse($input['query']);
$newDefinitions = [];

foreach ($parser->definitions as $def) {
    if ($def->kind === NodeKind::FRAGMENT_DEFINITION) {
        array_unshift($newDefinitions, $def);
    } else {
        array_push($newDefinitions, $def);
    }
}

$parser->definitions = $newDefinitions;
$rule = new QueryComplexity($maxQueryComplexity);
$parseErrors = DocumentValidator::validate(
    $schema,
    $parser,
    array_merge(DocumentValidator::defaultRules(), [$rule])
);

When that has no side effects, would that not be a option for the base-framework to put them always on top?

[spawnia]

First step towards fixing this is adding a failing test case to https://github.com/webonyx/graphql-php/blob/master/tests/Validator/QueryComplexityTest.php.