Skip to content

Commit

Permalink
Each API field definition is lazy loaded #10525
Browse files Browse the repository at this point in the history
Previously, we lazy-loaded the whole group of fields of QueryType and
MutationType at once. But each field were not lazy-loaded individually.
So a query with a single field, still loaded the schema for all possible
queries. Same for mutations.

Now we enforce lazy-loaded individual fields via PhpStan extra typing.
The measured speed improvement is actually extremely small, and possibly
even non-existent (!), even for our largest projects. Something like
1.01 ± 0.09 times faster than before.

But we still keep it, because the work is already done, and it
gives other minor advantages, such as:

- less indented code
- easier access to specific mutation via name instead of numerical index
- _maybe_ less memory usage (though I didn't measure it)
  • Loading branch information
PowerKiKi committed Jun 17, 2024
1 parent 4ec06dc commit b122d23
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 17 deletions.
37 changes: 22 additions & 15 deletions src/Api/Field/AclConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,31 @@
use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\Type;

/**
* @phpstan-import-type PermissiveFieldsConfig from FieldInterface
*/
abstract class AclConfiguration
{
public static function build(Acl $acl, EnumType $roleType): array
/**
* Return the single field configuration, including its name.
*
* @return PermissiveFieldsConfig
*/
public static function build(Acl $acl, EnumType $roleType): iterable
{
return
[
'name' => 'aclConfiguration',
'type' => Type::nonNull(Type::listOf(Type::nonNull(_types()->get(AclResourceConfigurationType::class)))),
'description' => 'User friendly configuration of the ACL',
'args' => [
'roles' => Type::nonNull(Type::listOf(Type::nonNull($roleType))),
],
'resolve' => function ($root, array $args) use ($acl): array {
$roles = $args['roles'];
$result = $acl->show(new MultipleRoles($roles));
yield 'aclConfiguration' => fn () => [
'name' => 'aclConfiguration',
'type' => Type::nonNull(Type::listOf(Type::nonNull(_types()->get(AclResourceConfigurationType::class)))),
'description' => 'User friendly configuration of the ACL',
'args' => [
'roles' => Type::nonNull(Type::listOf(Type::nonNull($roleType))),
],
'resolve' => function ($root, array $args) use ($acl): array {
$roles = $args['roles'];
$result = $acl->show(new MultipleRoles($roles));

return $result;
},
];
return $result;
},
];
}
}
32 changes: 31 additions & 1 deletion src/Api/Field/FieldInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,43 @@

namespace Ecodev\Felix\Api\Field;

use GraphQL\Language\AST\FieldDefinitionNode;
use GraphQL\Type\Definition\FieldDefinition;
use GraphQL\Type\Definition\ResolveInfo;
use Mezzio\Session\SessionInterface;

/**
* Represent a single field configuration.
*
* Below, we re-define some type coming from webonyx/graphql, because our `\GraphQL\Doctrine\Types::get()`
* is too poorly typed to match the correctness of webonyx/graphql. If we are able to fix our typing one day,
* then we should remove those overrides below.
*
* @phpstan-import-type FieldType from FieldDefinition
* @phpstan-import-type ComplexityFn from FieldDefinition
* @phpstan-import-type VisibilityFn from FieldDefinition
*
* @phpstan-type PermissiveArgumentListConfig iterable<mixed>
* @phpstan-type FieldResolverWithSessionInterface callable(mixed, array<string, mixed>, SessionInterface, ResolveInfo): mixed
* @phpstan-type PermissiveUnnamedFieldDefinitionConfig array{
* type: FieldType,
* resolve?: FieldResolverWithSessionInterface|null,
* args?: PermissiveArgumentListConfig|null,
* description?: string|null,
* visible?: VisibilityFn|bool,
* deprecationReason?: string|null,
* astNode?: FieldDefinitionNode|null,
* complexity?: ComplexityFn|null
* }
* @phpstan-type PermissiveDefinitionResolver callable(): PermissiveUnnamedFieldDefinitionConfig
* @phpstan-type PermissiveFieldsConfig iterable<string, PermissiveDefinitionResolver>
*/
interface FieldInterface
{
/**
* Return the single field configuration, including its name.
*
* @return PermissiveFieldsConfig
*/
public static function build(): array;
public static function build(): iterable;
}
26 changes: 25 additions & 1 deletion src/Utility.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use GraphQL\Doctrine\Definition\EntityID;
use ReflectionClass;

abstract class Utility
final class Utility
{
/**
* Returns the short class name of any object, eg: Application\Model\Calendar => Calendar.
Expand Down Expand Up @@ -124,4 +124,28 @@ public static function getCookieDomain(string $input): ?string

return $cookieDomain;
}

/**
* Concatenate all given iterables into a new iterator that yields key/value pairs exactly as if we iterated through each iterable sequentially.
*/
public static function concat(iterable ...$iterables): iterable
{
foreach ($iterables as $iterable) {
foreach ($iterable as $k => $v) {
yield $k => $v;
}
}
}

/**
* Return a new iterable with only key/value pairs that match the given keys.
*/
public static function filterByKeys(iterable $things, string ...$keyToKeep): iterable
{
foreach ($things as $k => $v) {
if (in_array($k, $keyToKeep, true)) {
yield $k => $v;
}
}
}
}
24 changes: 24 additions & 0 deletions tests/UtilityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace EcodevTests\Felix;

use ArrayIterator;
use Ecodev\Felix\Model\Model;
use Ecodev\Felix\Utility;
use EcodevTests\Felix\Blog\Model\User;
Expand Down Expand Up @@ -178,4 +179,27 @@ public function testGetCookieDomain(string $input, ?string $expected): void
$actual = Utility::getCookieDomain($input);
self::assertSame($expected, $actual);
}

public function testConcat(): void
{
$iterable = new ArrayIterator([1 => 'one', 'a' => 'overridden', 'c' => 'c']);
$actual = Utility::concat(['a' => 'a', 2 => 'two'], $iterable);

self::assertSame([
'a' => 'overridden',
2 => 'two',
1 => 'one',
'c' => 'c',
], iterator_to_array($actual));
}

public function testFilterByKeys(): void
{
$things = new ArrayIterator(['a' => 'a', 'b' => 'b', 'c' => 'c']);

self::assertSame(['a' => 'a'], iterator_to_array(Utility::filterByKeys($things, 'a')));
self::assertSame(['a' => 'a', 'c' => 'c'], iterator_to_array(Utility::filterByKeys($things, 'c', 'a')));
self::assertSame([], iterator_to_array(Utility::filterByKeys($things, 'unknown-key')));
self::assertSame([], iterator_to_array(Utility::filterByKeys($things)));
}
}

0 comments on commit b122d23

Please sign in to comment.