Skip to content

Commit

Permalink
Create field args mapper and cache args resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
@Warxcell
Warxcell authored Oct 24, 2024
1 parent c4df6e5 commit 9563fc5
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 18 deletions.
7 changes: 5 additions & 2 deletions docs/class-reference.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1341,8 +1341,9 @@ const CLASS_MAP = [

Implements the "Evaluating requests" section of the GraphQL specification.

@phpstan-type ArgsMapper callable(array<string, mixed>, FieldDefinition, FieldNode, mixed): mixed
@phpstan-type FieldResolver callable(mixed, array<string, mixed>, mixed, ResolveInfo): mixed
@phpstan-type ImplementationFactory callable(PromiseAdapter, Schema, DocumentNode, mixed, mixed, array<mixed>, ?string, callable): ExecutorImplementation
@phpstan-type ImplementationFactory callable(PromiseAdapter, Schema, DocumentNode, mixed, mixed, array<mixed>, ?string, callable, callable): ExecutorImplementation

@see \GraphQL\Tests\Executor\ExecutorTest

Expand Down Expand Up @@ -1388,6 +1389,7 @@ static function execute(
* @param array<string, mixed>|null $variableValues
*
* @phpstan-param FieldResolver|null $fieldResolver
* @phpstan-param ArgsMapper|null $argsMapper
*
* @api
*/
Expand All @@ -1399,7 +1401,8 @@ static function promiseToExecute(
$contextValue = null,
?array $variableValues = null,
?string $operationName = null,
?callable $fieldResolver = null
?callable $fieldResolver = null,
?callable $argsMapper = null
): GraphQL\Executor\Promise\Promise
```

Expand Down
1 change: 1 addition & 0 deletions docs/type-definitions/object-types.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ This example uses **inline** style for Object Type definitions, but you can also
| interfaces | `array` or `callable` | List of interfaces implemented by this type or callable returning such a list. See [Interface Types](interfaces.md) for details. See also the section on [Circular types](#recurring-and-circular-types) for an explanation of when to use callable for this option. |
| isTypeOf | `callable` | **function ($value, $context, [ResolveInfo](../class-reference.md#graphqltypedefinitionresolveinfo) $info): bool**<br> Expected to return **true** if **$value** qualifies for this type (see section about [Abstract Type Resolution](interfaces.md#interface-role-in-data-fetching) for explanation). |
| resolveField | `callable` | **function ($value, array $args, $context, [ResolveInfo](../class-reference.md#graphqltypedefinitionresolveinfo) $info): mixed**<br> Given the **$value** of this type, it is expected to return value for a field defined in **$info->fieldName**. A good place to define a type-specific strategy for field resolution. See section on [Data Fetching](../data-fetching.md) for details. |
| argsMapper | `callable` | **function (array $args, FieldDefinition, FieldNode): mixed**<br> Called once, when Executor resolves arguments for given field. Could be used to validate args and/or to map them to DTO/Object. |
| visible | `bool` or `callable` | Defaults to `true`. The given callable receives no arguments and is expected to return a `bool`, it is called once when the field may be accessed. The field is treated as if it were not defined at all when this is `false`. |

### Field configuration options
Expand Down
10 changes: 10 additions & 0 deletions src/Executor/ExecutionContext.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* and the fragments defined in the query document.
*
* @phpstan-import-type FieldResolver from Executor
* @phpstan-import-type ArgsMapper from Executor
*/
class ExecutionContext
{
Expand All @@ -41,6 +42,13 @@ class ExecutionContext
*/
public $fieldResolver;

/**
* @var callable
*
* @phpstan-var ArgsMapper
*/
public $argsMapper;

/** @var array<int, Error> */
public array $errors;

Expand All @@ -64,6 +72,7 @@ public function __construct(
array $variableValues,
array $errors,
callable $fieldResolver,
callable $argsMapper,
PromiseAdapter $promiseAdapter
) {
$this->schema = $schema;
Expand All @@ -74,6 +83,7 @@ public function __construct(
$this->variableValues = $variableValues;
$this->errors = $errors;
$this->fieldResolver = $fieldResolver;
$this->argsMapper = $argsMapper;
$this->promiseAdapter = $promiseAdapter;
}

Expand Down
37 changes: 34 additions & 3 deletions src/Executor/Executor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,18 @@
use GraphQL\Executor\Promise\Promise;
use GraphQL\Executor\Promise\PromiseAdapter;
use GraphQL\Language\AST\DocumentNode;
use GraphQL\Language\AST\FieldNode;
use GraphQL\Type\Definition\FieldDefinition;
use GraphQL\Type\Definition\ResolveInfo;
use GraphQL\Type\Schema;
use GraphQL\Utils\Utils;

/**
* Implements the "Evaluating requests" section of the GraphQL specification.
*
* @phpstan-type ArgsMapper callable(array<string, mixed>, FieldDefinition, FieldNode, mixed): mixed
* @phpstan-type FieldResolver callable(mixed, array<string, mixed>, mixed, ResolveInfo): mixed
* @phpstan-type ImplementationFactory callable(PromiseAdapter, Schema, DocumentNode, mixed, mixed, array<mixed>, ?string, callable): ExecutorImplementation
* @phpstan-type ImplementationFactory callable(PromiseAdapter, Schema, DocumentNode, mixed, mixed, array<mixed>, ?string, callable, callable): ExecutorImplementation

This comment has been minimized.

Sign in to view

Copy link
@andrew-demb

andrew-demb Oct 24, 2024

Contributor

Adding the required parameter is a BC break, isn't it?

Our tests are broken after upgrading to the 15.16

@Warxcell @spawnia are you OK with PR that tries to make this parameter optional?

This comment has been minimized.

Sign in to view

Copy link
@spawnia

spawnia Oct 24, 2024

Collaborator

Please elaborate on how this causes your tests to fail.

This comment has been minimized.

Sign in to view

Copy link
@andrew-demb

andrew-demb Oct 24, 2024

Contributor

@spawnia
We have a custom implementation factory like this:

use GraphQL\Executor\Executor;
use GraphQL\Executor\Promise\PromiseAdapter;
use GraphQL\Language\AST\DocumentNode;
use GraphQL\Language\AST\FieldNode;
use GraphQL\Language\AST\OperationDefinitionNode;
use GraphQL\Type\Schema;
use FooBar\PrometheusMetricRepositoryInterface;
use Psr\Log\LoggerInterface;

/**
 * Custom factory over Webonyx for implement readable tracing and grouping for batch query and prefetch methods
 *
 * @internal
 */
class PrometheusReferenceExecutorWrapperFactory
{
    public function __construct(
        private readonly PrometheusMetricRepositoryInterface $prometheusMetricRepository,
        private readonly LoggerInterface $logger,
    ) {
    }

    public static function registerInWebonyx(self $factory): void
    {
        Executor::setImplementationFactory($factory->asWebonyxFactory(Executor::getImplementationFactory()));
    }

    public function asWebonyxFactory(callable $prevFactory): callable
    {
        /**
         * This function should respect implicit contract from webonyx {@see \GraphQL\Executor\Executor::promiseToExecute()}
         */
        return function (
            PromiseAdapter $promiseAdapter,
            Schema $schema,
            DocumentNode $documentNode,
            $rootValue,
            $contextValue,
            $variableValues,
            ?string $operationName,
            callable $fieldResolver,
        ) use ($prevFactory): PrometheusReferenceExecutorWrapper {
            $prevExecutor = $prevFactory(
                $promiseAdapter,
                $schema,
                $documentNode,
                $rootValue,
                $contextValue,
                $variableValues,
                $operationName,
                $fieldResolver,
            );

            [$type, $name] = $this->getOperationData($documentNode);

            return new PrometheusReferenceExecutorWrapper(
                $this->prometheusMetricRepository,
                $this->logger,
                $prevExecutor,
                $name,
                $type,
            );
        };
    }

    /**
     * @return array{0: string, 1: string} - [type, name]
     */
    private function getOperationData(DocumentNode $node): array
    {
        $type = null;
        $name = null;

        foreach ($node->definitions as $definition) {
            if (false === $definition instanceof OperationDefinitionNode) {
                continue;
            }

            $type ??= $definition->operation;

            foreach ($definition->selectionSet->selections as $selection) {
                if (false === $selection instanceof FieldNode) {
                    continue;
                }

                $name = $selection->name->value;

                if ('' !== $name) {
                    break 2;
                }
            }
        }

        return [
            $type ?? 'query',
            $name ?? 'undefined',
        ];
    }
}

Our tests are broken since 15.16.0 because of error:

Too few arguments to function GraphQL\Executor\ReferenceExecutor::create(), 8 passed in /.../vendor/foobar/foobar/src/PrometheusReferenceExecutorWrapperFactory.php on line 49 and exactly 9 expected

This comment has been minimized.

Sign in to view

Copy link
@andrew-demb

andrew-demb Oct 25, 2024

Contributor

For history - there's a PR that fixes BC #1622
*
* @see \GraphQL\Tests\Executor\ExecutorTest
*/
Expand All @@ -28,6 +31,13 @@ class Executor
*/
private static $defaultFieldResolver = [self::class, 'defaultFieldResolver'];

/**
* @var callable
*
* @phpstan-var ArgsMapper
*/
private static $defaultArgsMapper = [self::class, 'defaultArgsMapper'];

private static ?PromiseAdapter $defaultPromiseAdapter;

/**
Expand All @@ -53,6 +63,12 @@ public static function setDefaultFieldResolver(callable $fieldResolver): void
self::$defaultFieldResolver = $fieldResolver;
}

/** @phpstan-param ArgsMapper $argsMapper */
public static function setDefaultArgsMapper(callable $argsMapper): void
{
self::$defaultArgsMapper = $argsMapper;
}

public static function getPromiseAdapter(): PromiseAdapter
{
return self::$defaultPromiseAdapter ??= new SyncPromiseAdapter();
Expand Down Expand Up @@ -132,6 +148,7 @@ public static function execute(
* @param array<string, mixed>|null $variableValues
*
* @phpstan-param FieldResolver|null $fieldResolver
* @phpstan-param ArgsMapper|null $argsMapper
*
* @api
*/
Expand All @@ -143,7 +160,8 @@ public static function promiseToExecute(
$contextValue = null,
?array $variableValues = null,
?string $operationName = null,
?callable $fieldResolver = null
?callable $fieldResolver = null,
?callable $argsMapper = null
): Promise {
$executor = (self::$implementationFactory)(
$promiseAdapter,
Expand All @@ -153,7 +171,8 @@ public static function promiseToExecute(
$contextValue,
$variableValues ?? [],
$operationName,
$fieldResolver ?? self::$defaultFieldResolver
$fieldResolver ?? self::$defaultFieldResolver,
$argsMapper ?? self::$defaultArgsMapper,
);

return $executor->doExecute();
Expand All @@ -179,4 +198,16 @@ public static function defaultFieldResolver($objectLikeValue, array $args, $cont
? $property($objectLikeValue, $args, $contextValue, $info)
: $property;
}

/**
* @template T of array<string, mixed>
*
* @param T $args
*
* @return T
*/
public static function defaultArgsMapper(array $args): array
{
return $args;
}
}
38 changes: 27 additions & 11 deletions src/Executor/ReferenceExecutor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
/**
* @phpstan-import-type FieldResolver from Executor
* @phpstan-import-type Path from ResolveInfo
* @phpstan-import-type ArgsMapper from Executor
*
* @phpstan-type Fields \ArrayObject<string, \ArrayObject<int, FieldNode>>
*/
Expand All @@ -60,6 +61,9 @@ class ReferenceExecutor implements ExecutorImplementation
*/
protected \SplObjectStorage $subFieldCache;

/** @var \SplObjectStorage<FieldDefinition, \SplObjectStorage<FieldNode, mixed>> */
protected \SplObjectStorage $fieldArgsCache;

protected function __construct(ExecutionContext $context)
{
if (! isset(static::$UNDEFINED)) {
Expand All @@ -68,6 +72,7 @@ protected function __construct(ExecutionContext $context)

$this->exeContext = $context;
$this->subFieldCache = new \SplObjectStorage();
$this->fieldArgsCache = new \SplObjectStorage();
}

/**
Expand All @@ -76,6 +81,7 @@ protected function __construct(ExecutionContext $context)
* @param array<string, mixed> $variableValues
*
* @phpstan-param FieldResolver $fieldResolver
* @phpstan-param ArgsMapper $argsMapper
*
* @throws \Exception
*/
Expand All @@ -87,7 +93,8 @@ public static function create(
$contextValue,
array $variableValues,
?string $operationName,
callable $fieldResolver
callable $fieldResolver,
callable $argsMapper
): ExecutorImplementation {
$exeContext = static::buildExecutionContext(
$schema,
Expand All @@ -97,7 +104,8 @@ public static function create(
$variableValues,
$operationName,
$fieldResolver,
$promiseAdapter
$argsMapper,
$promiseAdapter,
);

if (\is_array($exeContext)) {
Expand Down Expand Up @@ -141,6 +149,7 @@ protected static function buildExecutionContext(
array $rawVariableValues,
?string $operationName,
callable $fieldResolver,
callable $argsMapper,
PromiseAdapter $promiseAdapter
) {
/** @var array<int, Error> $errors */
Expand Down Expand Up @@ -217,6 +226,7 @@ protected static function buildExecutionContext(
$variableValues,
$errors,
$fieldResolver,
$argsMapper,
$promiseAdapter
);
}
Expand Down Expand Up @@ -640,20 +650,22 @@ protected function resolveField(
$exeContext->variableValues,
$unaliasedPath
);
if ($fieldDef->resolveFn !== null) {
$resolveFn = $fieldDef->resolveFn;
} elseif ($parentType->resolveFieldFn !== null) {
$resolveFn = $parentType->resolveFieldFn;
} else {
$resolveFn = $this->exeContext->fieldResolver;
}

$resolveFn = $fieldDef->resolveFn
?? $parentType->resolveFieldFn
?? $this->exeContext->fieldResolver;

$argsMapper = $fieldDef->argsMapper
?? $parentType->argsMapper
?? $this->exeContext->argsMapper;

// Get the resolve function, regardless of if its result is normal
// or abrupt (error).
$result = $this->resolveFieldValueOrError(
$fieldDef,
$fieldNode,
$resolveFn,
$argsMapper,
$rootValue,
$info,
$contextValue
Expand Down Expand Up @@ -721,18 +733,22 @@ protected function resolveFieldValueOrError(
FieldDefinition $fieldDef,
FieldNode $fieldNode,
callable $resolveFn,
callable $argsMapper,
$rootValue,
ResolveInfo $info,
$contextValue
) {
try {
// Build a map of arguments from the field.arguments AST, using the
// variables scope to fulfill any variable references.
$args = Values::getArgumentValues(
/** @phpstan-ignore-next-line ignored because no way to tell phpstan what are generics of SplObjectStorage without assign it to var first */
$this->fieldArgsCache[$fieldDef] ??= new \SplObjectStorage();

$args = $this->fieldArgsCache[$fieldDef][$fieldNode] ??= $argsMapper(Values::getArgumentValues(
$fieldDef,
$fieldNode,
$this->exeContext->variableValues
);
), $fieldDef, $fieldNode, $contextValue);

return $resolveFn($rootValue, $args, $contextValue, $info);
} catch (\Throwable $error) {
Expand Down
13 changes: 13 additions & 0 deletions src/Type/Definition/FieldDefinition.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* @see Executor
*
* @phpstan-import-type FieldResolver from Executor
* @phpstan-import-type ArgsMapper from Executor
* @phpstan-import-type ArgumentListConfig from Argument
*
* @phpstan-type FieldType (Type&OutputType)|callable(): (Type&OutputType)
Expand All @@ -22,6 +23,7 @@
* type: FieldType,
* resolve?: FieldResolver|null,
* args?: ArgumentListConfig|null,
* argsMapper?: ArgsMapper|null,
* description?: string|null,
* visible?: VisibilityFn|bool,
* deprecationReason?: string|null,
Expand All @@ -32,6 +34,7 @@
* type: FieldType,
* resolve?: FieldResolver|null,
* args?: ArgumentListConfig|null,
* argsMapper?: ArgsMapper|null,
* description?: string|null,
* visible?: VisibilityFn|bool,
* deprecationReason?: string|null,
Expand All @@ -56,6 +59,15 @@ class FieldDefinition
/** @var array<int, Argument> */
public array $args;

/**
* Callback to transform args to value object.
*
* @var callable|null
*
* @phpstan-var ArgsMapper|null
*/
public $argsMapper;

/**
* Callback for resolving field value given parent value.
*
Expand Down Expand Up @@ -103,6 +115,7 @@ public function __construct(array $config)
$this->args = isset($config['args'])
? Argument::listFromConfig($config['args'])
: [];
$this->argsMapper = $config['argsMapper'] ?? null;
$this->description = $config['description'] ?? null;
$this->visible = $config['visible'] ?? true;
$this->deprecationReason = $config['deprecationReason'] ?? null;
Expand Down
10 changes: 10 additions & 0 deletions src/Type/Definition/ObjectType.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@
* ]);
*
* @phpstan-import-type FieldResolver from Executor
* @phpstan-import-type ArgsMapper from Executor
*
* @phpstan-type InterfaceTypeReference InterfaceType|callable(): InterfaceType
* @phpstan-type ObjectConfig array{
* name?: string|null,
* description?: string|null,
* resolveField?: FieldResolver|null,
* argsMapper?: ArgsMapper|null,
* fields: (callable(): iterable<mixed>)|iterable<mixed>,
* interfaces?: iterable<InterfaceTypeReference>|callable(): iterable<InterfaceTypeReference>,
* isTypeOf?: (callable(mixed $objectValue, mixed $context, ResolveInfo $resolveInfo): (bool|Deferred|null))|null,
Expand All @@ -81,6 +83,13 @@ class ObjectType extends Type implements OutputType, CompositeType, NullableType
*/
public $resolveFieldFn;

/**
* @var callable|null
*
* @phpstan-var ArgsMapper|null
*/
public $argsMapper;

/** @phpstan-var ObjectConfig */
public array $config;

Expand All @@ -94,6 +103,7 @@ public function __construct(array $config)
$this->name = $config['name'] ?? $this->inferName();
$this->description = $config['description'] ?? null;
$this->resolveFieldFn = $config['resolveField'] ?? null;
$this->argsMapper = $config['argsMapper'] ?? null;
$this->astNode = $config['astNode'] ?? null;
$this->extensionASTNodes = $config['extensionASTNodes'] ?? [];

Expand Down
4 changes: 3 additions & 1 deletion src/Utils/SchemaExtender.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ protected function extendFieldMap(Type $type): array
'type' => $this->extendType($field->getType()),
'args' => $this->extendArgs($field->args),
'resolve' => $field->resolveFn,
'argsMapper' => $field->argsMapper,
'astNode' => $field->astNode,
];
}
Expand Down Expand Up @@ -537,7 +538,8 @@ protected function extendObjectType(ObjectType $type): ObjectType
'interfaces' => fn (): array => $this->extendImplementedInterfaces($type),
'fields' => fn (): array => $this->extendFieldMap($type),
'isTypeOf' => [$type, 'isTypeOf'],
'resolveField' => $type->resolveFieldFn ?? null,
'resolveField' => $type->resolveFieldFn,
'argsMapper' => $type->argsMapper,
'astNode' => $type->astNode,
'extensionASTNodes' => $extensionASTNodes,
]);
Expand Down
Loading

0 comments on commit 9563fc5

Please sign in to comment.