Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup Input default value and Fix #1171 #1172

Merged
merged 7 commits into from
Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix #1169
When using attributes or annotations, we can try to auto resolve the types associated with interfaces.
  • Loading branch information
Vincz committed Mar 29, 2024
commit f1dd087e77eee7493ea9216c7ae58317c0631e40
8 changes: 5 additions & 3 deletions docs/annotations/annotations-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -591,13 +591,15 @@ class Hero {

This annotation is used on _class_ to define a GraphQL interface.

Required attributes:
Optional attributes:

- **resolveType** : An expression to resolve the types
- **name** : The GraphQL name of the interface (default to the class name without namespace)

Optional attributes:
If the `resolveType` attribute is not set, the service `overblog_graphql.interface_type_resolver` will be used to try to resolve the type automatically based on types implementing the interface and their associated class.
The system will register a map of interfaces with the list of types and their associated class name implementing the interface (the parameter is named `overblog_graphql_types.interfaces_map` in the container) and use it to resolve the type from the value (the first type where the class `instanceof` operator returns true will be used).

- **name** : The GraphQL name of the interface (default to the class name without namespace)
```php
Vincz marked this conversation as resolved.
Show resolved Hide resolved

## @Scalar

Expand Down
4 changes: 2 additions & 2 deletions src/Annotation/TypeInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ final class TypeInterface extends Annotation
/**
* Resolver type for interface.
*/
public string $resolveType;
public ?string $resolveType;

/**
* Interface name.
Expand All @@ -31,7 +31,7 @@ final class TypeInterface extends Annotation
* @param string $resolveType The express resolve type
* @param string|null $name The GraphQL name of the interface
*/
public function __construct(string $resolveType, ?string $name = null)
public function __construct(?string $resolveType = null, ?string $name = null)
{
$this->resolveType = $resolveType;
$this->name = $name;
Expand Down
24 changes: 23 additions & 1 deletion src/Config/Parser/MetadataParser/ClassesTypesMap.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
*/
private array $classesMap = [];

/**
* @var array<string, array{class: string, type: string}>
*/
private array $interfacesMap = [];

public function hasType(string $gqlType): bool
{
return isset($this->classesMap[$gqlType]);
Expand Down Expand Up @@ -72,8 +77,25 @@
return $classNames;
}

public function toArray(): array
public function classesToArray(): array
{
return $this->classesMap;
}

/**
* Add a type and its associated class to the interfaces map
*/
public function addInterfaceType(string $interfaceType, string $graphqlType, string $className): void
{
if (!isset($this->interfacesMap[$interfaceType])) {
$this->interfacesMap[$interfaceType] = [];

Check failure on line 91 in src/Config/Parser/MetadataParser/ClassesTypesMap.php

View workflow job for this annotation

GitHub Actions / Static analysis

Property Overblog\GraphQLBundle\Config\Parser\MetadataParser\ClassesTypesMap::$interfacesMap (array<string, array{class: string, type: string}>) does not accept non-empty-array<string, array{}|array{class: string, type: string}>.
}

$this->interfacesMap[$interfaceType][$className] = $graphqlType;

Check failure on line 94 in src/Config/Parser/MetadataParser/ClassesTypesMap.php

View workflow job for this annotation

GitHub Actions / Static analysis

Property Overblog\GraphQLBundle\Config\Parser\MetadataParser\ClassesTypesMap::$interfacesMap (array<string, array{class: string, type: string}>) does not accept non-empty-array<string, non-empty-array<string, string>>.
}

public function interfacesToArray(): array
{
return $this->interfacesMap;
}
}
39 changes: 34 additions & 5 deletions src/Config/Parser/MetadataParser/MetadataParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@
return self::processFile($file, $container, $configs, false);
}

public static function finalize(ContainerBuilder $container): void
{
$parameter = 'overblog_graphql_types.interfaces_map';
$value = $container->hasParameter($parameter) ? $container->getParameter($parameter) : [];
foreach (self::$map->interfacesToArray() as $interface => $types) {
if (!isset($value[$interface])) {

Check failure on line 96 in src/Config/Parser/MetadataParser/MetadataParser.php

View workflow job for this annotation

GitHub Actions / Static analysis

Cannot access offset (int|string) on array|bool|float|int|string.
$value[$interface] = [];

Check failure on line 97 in src/Config/Parser/MetadataParser/MetadataParser.php

View workflow job for this annotation

GitHub Actions / Static analysis

Cannot access offset (int|string) on array|bool|float|int|string.
}
foreach ($types as $className => $typeName) {
$value[$interface][$className] = $typeName;
}
}

$container->setParameter('overblog_graphql_types.interfaces_map', $value);
}

/**
* @internal
*/
Expand Down Expand Up @@ -134,7 +150,7 @@
}
}

return $preProcess ? self::$map->toArray() : $gqlTypes;
return $preProcess ? self::$map->classesToArray() : $gqlTypes;
} catch (ReflectionException $e) {
return $gqlTypes;
} catch (\InvalidArgumentException $e) {
Expand Down Expand Up @@ -189,6 +205,11 @@

array_unshift($gqlConfiguration['config']['builders'], ['builder' => 'relay-connection', 'builderConfig' => ['edgeType' => $edgeType]]);
}

$interfaces = $gqlConfiguration['config']['interfaces'] ?? [];
foreach ($interfaces as $interface) {
self::$map->addInterfaceType($interface, $gqlName, $reflectionClass->getName());
}
}
break;

Expand Down Expand Up @@ -224,7 +245,10 @@
case $classMetadata instanceof Metadata\TypeInterface:
$gqlType = self::GQL_INTERFACE;
if (!$preProcess) {
$gqlConfiguration = self::typeInterfaceMetadataToGQLConfiguration($reflectionClass, $classMetadata);
if (!$gqlName) {

Check failure on line 248 in src/Config/Parser/MetadataParser/MetadataParser.php

View workflow job for this annotation

GitHub Actions / Static analysis

Negated boolean expression is always true.
$gqlName = !empty($classMetadata->name) ? $classMetadata->name : $reflectionClass->getShortName();
}
$gqlConfiguration = self::typeInterfaceMetadataToGQLConfiguration($reflectionClass, $classMetadata, $gqlName);
}
break;

Expand Down Expand Up @@ -380,7 +404,7 @@
*
* @return array{type: 'interface', config: array}
*/
private static function typeInterfaceMetadataToGQLConfiguration(ReflectionClass $reflectionClass, Metadata\TypeInterface $interfaceAnnotation): array
private static function typeInterfaceMetadataToGQLConfiguration(ReflectionClass $reflectionClass, Metadata\TypeInterface $interfaceAnnotation, string $gqlName): array
{
$interfaceConfiguration = [];

Expand All @@ -390,7 +414,12 @@
$interfaceConfiguration['fields'] = array_merge($fieldsFromProperties, $fieldsFromMethods);
$interfaceConfiguration = self::getDescriptionConfiguration(static::getMetadatas($reflectionClass)) + $interfaceConfiguration;

$interfaceConfiguration['resolveType'] = self::formatExpression($interfaceAnnotation->resolveType);
if (isset($interfaceAnnotation->resolveType)) {
$interfaceConfiguration['resolveType'] = self::formatExpression($interfaceAnnotation->resolveType);
} else {
// Try to use default interface resolver type
$interfaceConfiguration['resolveType'] = self::formatExpression(sprintf("service('overblog_graphql.interface_type_resolver').resolveType('%s', value)", $gqlName));
}

return ['type' => 'interface', 'config' => $interfaceConfiguration];
}
Expand Down Expand Up @@ -687,7 +716,7 @@

if ($fieldMetadata instanceof InputField && null !== $fieldMetadata->defaultValue) {
$fieldConfiguration['defaultValue'] = $fieldMetadata->defaultValue;
} elseif ($reflector->hasDefaultValue()) {
} elseif ($reflector->hasDefaultValue() && null !== $reflector->getDefaultValue()) {
$fieldConfiguration['defaultValue'] = $reflector->getDefaultValue();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Controller/ProfilerController.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function __invoke(Request $request, string $token): Response

$tokens = array_map(function ($tokenData) {
$profile = $this->profiler->loadProfile($tokenData['token']);
if (!$profile->hasCollector('graphql')) {
if (!$profile || !$profile->hasCollector('graphql')) {
return false;
}
$tokenData['graphql'] = $profile->getCollector('graphql');
Expand Down
5 changes: 5 additions & 0 deletions src/DependencyInjection/Compiler/ConfigParserPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ private function getConfigs(ContainerBuilder $container): array
$config = $container->getParameterBag()->resolveValue($container->getParameter('overblog_graphql.config'));
$container->getParameterBag()->remove('overblog_graphql.config');
$container->setParameter($this->getAlias().'.classes_map', []);
$container->setParameter($this->getAlias().'.interfaces_map', []);

$typesMappings = $this->mappingConfig($config, $container);
// reset treated files
$this->treatedFiles = [];
Expand All @@ -117,6 +119,9 @@ private function getConfigs(ContainerBuilder $container): array
// flatten config is a requirement to support inheritance
$flattenTypeConfig = array_merge(...$typeConfigs);

AnnotationParser::finalize($container);
AttributeParser::finalize($container);

return $flattenTypeConfig;
}

Expand Down
39 changes: 39 additions & 0 deletions src/Resolver/InterfaceTypeResolver.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

declare(strict_types=1);

namespace Overblog\GraphQLBundle\Resolver;

class InterfaceTypeResolver
{
private TypeResolver $typeResolver;
private array $interfacesMap;

public function __construct(TypeResolver $typeResolver, array $interfacesMap = [])
{
$this->typeResolver = $typeResolver;
$this->interfacesMap = $interfacesMap;
}

public function resolveType(string $interfaceType, mixed $value)

Check failure on line 18 in src/Resolver/InterfaceTypeResolver.php

View workflow job for this annotation

GitHub Actions / Static analysis

Method Overblog\GraphQLBundle\Resolver\InterfaceTypeResolver::resolveType() has no return type specified.
{
if (!isset($this->interfacesMap[$interfaceType])) {
throw new UnresolvableException(sprintf('Default interface type resolver was unable to find interface with name "%s"', $interfaceType));
}

$gqlType = null;
$types = $this->interfacesMap[$interfaceType];
foreach ($types as $className => $type) {
if ($value instanceof $className) {
$gqlType = $type;
break;
}
}

if (null === $gqlType) {
throw new UnresolvableException(sprintf('Default interface type resolver with interface "%s" did not find a matching instance in: %s', $interfaceType, implode(', ', array_keys($types))));
}

return $this->typeResolver->resolve($gqlType);
}
}
1 change: 1 addition & 0 deletions src/Resources/config/aliases.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ services:
overblog_graphql.request_parser: '@Overblog\GraphQLBundle\Request\Parser'
overblog_graphql.request_batch_parser: '@Overblog\GraphQLBundle\Request\BatchParser'
overblog_graphql.arguments_transformer: '@Overblog\GraphQLBundle\Transformer\ArgumentsTransformer'
overblog_graphql.interface_type_resolver: '@Overblog\GraphQLBundle\Resolver\InterfaceTypeResolver'

overblog_graphql.schema_builder:
alias: 'Overblog\GraphQLBundle\Definition\Builder\SchemaBuilder'
Expand Down
49 changes: 27 additions & 22 deletions src/Resources/config/services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ services:
Overblog\GraphQLBundle\Resolver\FieldResolver: ~

Overblog\GraphQLBundle\Definition\GraphQLServices:
tags: ['container.service_locator']
tags: ["container.service_locator"]

Overblog\GraphQLBundle\Request\Executor:
arguments:
- "@overblog_graphql.executor"
- "@overblog_graphql.promise_adapter"
- "@event_dispatcher"
- '@overblog_graphql.default_field_resolver'
- "@overblog_graphql.default_field_resolver"
calls:
- ["setMaxQueryComplexity", ["%overblog_graphql.query_max_complexity%"]]
- ["setMaxQueryDepth", ["%overblog_graphql.query_max_depth%"]]
Expand All @@ -34,14 +34,14 @@ services:

Overblog\GraphQLBundle\Resolver\TypeResolver:
calls:
- ['setDispatcher', ['@event_dispatcher']]
- ["setDispatcher", ["@event_dispatcher"]]
tags:
- { name: overblog_graphql.service, alias: typeResolver }

Overblog\GraphQLBundle\Transformer\ArgumentsTransformer:
arguments:
- '@?validator'
- '%overblog_graphql_types.classes_map%'
- "@?validator"
- "%overblog_graphql_types.classes_map%"

Overblog\GraphQLBundle\Resolver\QueryResolver:
tags:
Expand All @@ -51,31 +51,36 @@ services:
tags:
- { name: overblog_graphql.service, alias: mutationResolver }

Overblog\GraphQLBundle\Resolver\InterfaceTypeResolver:
arguments:
- '@Overblog\GraphQLBundle\Resolver\TypeResolver'
- "%overblog_graphql_types.interfaces_map%"

Overblog\GraphQLBundle\Resolver\AccessResolver:
arguments:
- '@overblog_graphql.promise_adapter'
- "@overblog_graphql.promise_adapter"

Overblog\GraphQLBundle\ExpressionLanguage\ExpressionLanguage:
arguments:
- '@?overblog_graphql.cache_expression_language_parser'
- "@?overblog_graphql.cache_expression_language_parser"

Overblog\GraphQLBundle\Generator\TypeGenerator:
arguments:
- '%overblog_graphql_types.config%'
- "%overblog_graphql_types.config%"
- '@Overblog\GraphQLBundle\Generator\TypeBuilder'
- '@Symfony\Contracts\EventDispatcher\EventDispatcherInterface'
- !service
class: Overblog\GraphQLBundle\Generator\TypeGeneratorOptions
arguments:
- '%overblog_graphql.class_namespace%'
- '%overblog_graphql.cache_dir%'
- '%overblog_graphql.use_classloader_listener%'
- '%kernel.cache_dir%'
- '%overblog_graphql.cache_dir_permissions%'
class: Overblog\GraphQLBundle\Generator\TypeGeneratorOptions
arguments:
- "%overblog_graphql.class_namespace%"
- "%overblog_graphql.cache_dir%"
- "%overblog_graphql.use_classloader_listener%"
- "%kernel.cache_dir%"
- "%overblog_graphql.cache_dir_permissions%"

Overblog\GraphQLBundle\Definition\ArgumentFactory:
arguments:
- '%overblog_graphql.argument_class%'
- "%overblog_graphql.argument_class%"
tags:
- { name: overblog_graphql.service, alias: argumentFactory }

Expand All @@ -90,7 +95,7 @@ services:

Overblog\GraphQLBundle\Definition\ConfigProcessor:
arguments:
- !tagged_iterator 'overblog_graphql.definition_config_processor'
- !tagged_iterator "overblog_graphql.definition_config_processor"

GraphQL\Executor\Promise\PromiseAdapter: "@overblog_graphql.promise_adapter"

Expand All @@ -100,7 +105,7 @@ services:

Overblog\GraphQLBundle\Security\Security:
arguments:
- '@?security.helper'
- "@?security.helper"
tags:
- { name: overblog_graphql.service, alias: security, public: false }

Expand All @@ -111,12 +116,12 @@ services:
Overblog\GraphQLBundle\Generator\TypeBuilder:
arguments:
- '@Overblog\GraphQLBundle\Generator\Converter\ExpressionConverter'
- '%overblog_graphql.class_namespace%'
- "%overblog_graphql.class_namespace%"

Overblog\GraphQLBundle\Validator\InputValidatorFactory:
arguments:
- '@?validator.validator_factory'
- '@?validator'
- '@?translator.default'
- "@?validator.validator_factory"
- "@?validator"
- "@?translator.default"
tags:
- { name: overblog_graphql.service, alias: input_validator_factory, public: false }
6 changes: 5 additions & 1 deletion tests/Config/Parser/MetadataParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ public function testInterfaces(): void
'description' => 'The armored interface',
'resolveType' => '@=query(\'character_type\', [value])',
]);

$this->expect('Biped', 'interface', [
'resolveType' => "@=service('overblog_graphql.interface_type_resolver').resolveType('Biped', value)",
]);
}

public function testEnum(): void
Expand Down Expand Up @@ -304,7 +308,7 @@ public function testUnionAutoguessed(): void
public function testInterfaceAutoguessed(): void
{
$this->expect('Mandalorian', 'object', [
'interfaces' => ['Character', 'WithArmor'],
'interfaces' => ['Biped', 'Character', 'WithArmor'],
'fields' => [
'name' => ['type' => 'String!', 'description' => 'The name of the character'],
'friends' => ['type' => '[Character]', 'description' => 'The friends of the character', 'resolve' => "@=query('App\\MyResolver::getFriends')"],
Expand Down
15 changes: 15 additions & 0 deletions tests/Config/Parser/fixtures/annotations/Type/Biped.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace Overblog\GraphQLBundle\Tests\Config\Parser\fixtures\annotations\Type;

use Overblog\GraphQLBundle\Annotation as GQL;

/**
* @GQL\TypeInterface
*/
#[GQL\TypeInterface]
interface Biped
{
}
Loading
Loading