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

Fix FluentResolver when tagging two services with same class name #297

Merged
merged 3 commits into from
Mar 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
26 changes: 4 additions & 22 deletions Command/DebugCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ protected function configure()
InputOption::VALUE_IS_ARRAY | InputOption::VALUE_OPTIONAL,
sprintf('filter by a category (%s).', implode(', ', self::$categories))
)
->addOption(
'with-service-id',
null,
InputOption::VALUE_NONE,
'also display service id'
)
->setDescription('Display current GraphQL services (types, resolvers and mutations)');
}

Expand All @@ -72,53 +66,41 @@ protected function execute(InputInterface $input, OutputInterface $output)
}

$categories = empty($categoriesOption) ? self::$categories : $categoriesOption;
$withServiceId = $input->getOption('with-service-id');

$io = new SymfonyStyle($input, $output);
$tableHeaders = ['solution id', 'aliases'];
if ($withServiceId) {
$tableHeaders[] = 'service id';
}

foreach ($categories as $category) {
$io->title(sprintf('GraphQL %ss Services', ucfirst($category)));

/** @var FluentResolverInterface $resolver */
$resolver = $this->{$category.'Resolver'};
$this->renderTable($resolver, $tableHeaders, $io, $withServiceId);
$this->renderTable($resolver, $tableHeaders, $io);
}
}

/**
* @param FluentResolverInterface $resolver
* @param array $tableHeaders
* @param SymfonyStyle $io
* @param bool $withServiceId
*/
private function renderTable(FluentResolverInterface $resolver, array $tableHeaders, SymfonyStyle $io, $withServiceId)
private function renderTable(FluentResolverInterface $resolver, array $tableHeaders, SymfonyStyle $io)
{
$tableRows = [];
$solutionIDs = array_keys($resolver->getSolutions());
sort($solutionIDs);
foreach ($solutionIDs as $solutionID) {
$aliases = $resolver->getSolutionAliases($solutionID);
$options = $resolver->getSolutionOptions($solutionID);
$tableRows[$solutionID] = [$solutionID, self::serializeAliases($aliases, $options)];
if ($withServiceId) {
$tableRows[$solutionID][] = $options['id'];
}
$tableRows[$solutionID] = [$solutionID, self::serializeAliases($aliases)];
}
ksort($tableRows);
$io->table($tableHeaders, $tableRows);
$io->write("\n\n");
}

private static function serializeAliases(array $aliases, array $options)
private static function serializeAliases(array $aliases)
{
ksort($aliases);
$aliases = array_map(function ($alias) use ($options) {
return $alias.(isset($options['method']) ? ' (method: '.$options['method'].')' : '');
}, $aliases);

return implode("\n", $aliases);
}
Expand Down
2 changes: 1 addition & 1 deletion Config/TypeDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected function nameSection()
->ifTrue(function ($name) {
return !preg_match('/^[_a-z][_0-9a-z]*$/i', $name);
})
->thenInvalid('Invalid type name "%s". (see http://facebook.github.io/graphql/October2016/#Name)')
->thenInvalid('Invalid type name "%s". (see https://facebook.github.io/graphql/October2016/#Name)')
->end();

return $node;
Expand Down
5 changes: 2 additions & 3 deletions DependencyInjection/Compiler/TaggedServiceMappingPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@ private function getTaggedServiceMapping(ContainerBuilder $container, $tagName)
$isType = TypeTaggedServiceMappingPass::TAG_NAME === $tagName;

foreach ($taggedServices as $id => $tags) {
$className = $container->findDefinition($id)->getClass();
foreach ($tags as $attributes) {
$this->checkRequirements($id, $attributes);
$attributes = self::resolveAttributes($attributes, $id, !$isType);
$solutionID = $className;
$solutionID = $id;

if (!$isType && '__invoke' !== $attributes['method']) {
$solutionID = sprintf('%s::%s', $className, $attributes['method']);
$solutionID = sprintf('%s::%s', $id, $attributes['method']);
}

if (!isset($serviceMapping[$solutionID])) {
Expand Down
4 changes: 1 addition & 3 deletions Relay/Mutation/MutationFieldDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace Overblog\GraphQLBundle\Relay\Mutation;

use Overblog\GraphQLBundle\Definition\Builder\MappingInterface;
use Overblog\GraphQLBundle\GraphQL\Relay\Mutation\MutationFieldResolver;

final class MutationFieldDefinition implements MappingInterface
{
Expand All @@ -16,14 +15,13 @@ public function toMappingDefinition(array $config)
$mutateAndGetPayload = $this->cleanMutateAndGetPayload($config['mutateAndGetPayload']);
$payloadType = isset($config['payloadType']) && is_string($config['payloadType']) ? $config['payloadType'] : null;
$inputType = isset($config['inputType']) && is_string($config['inputType']) ? $config['inputType'].'!' : null;
$resolver = addslashes(MutationFieldResolver::class);

return [
'type' => $payloadType,
'args' => [
'input' => ['type' => $inputType],
],
'resolve' => "@=resolver('$resolver', [args, context, info, mutateAndGetPayloadCallback($mutateAndGetPayload)])",
'resolve' => "@=resolver('relay_mutation_field', [args, context, info, mutateAndGetPayloadCallback($mutateAndGetPayload)])",
];
}

Expand Down
4 changes: 1 addition & 3 deletions Relay/Node/GlobalIdFieldDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,18 @@
namespace Overblog\GraphQLBundle\Relay\Node;

use Overblog\GraphQLBundle\Definition\Builder\MappingInterface;
use Overblog\GraphQLBundle\GraphQL\Relay\Node\GlobalIdFieldResolver;

final class GlobalIdFieldDefinition implements MappingInterface
{
public function toMappingDefinition(array $config)
{
$typeName = isset($config['typeName']) && is_string($config['typeName']) ? var_export($config['typeName'], true) : 'null';
$idFetcher = isset($config['idFetcher']) && is_string($config['idFetcher']) ? $this->cleanIdFetcher($config['idFetcher']) : 'null';
$resolver = addslashes(GlobalIdFieldResolver::class);

return [
'description' => 'The ID of an object',
'type' => 'ID!',
'resolve' => "@=resolver('$resolver', [value, info, $idFetcher, $typeName])",
'resolve' => "@=resolver('relay_globalid_field', [value, info, $idFetcher, $typeName])",
];
}

Expand Down
4 changes: 1 addition & 3 deletions Relay/Node/NodeFieldDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace Overblog\GraphQLBundle\Relay\Node;

use Overblog\GraphQLBundle\Definition\Builder\MappingInterface;
use Overblog\GraphQLBundle\GraphQL\Relay\Node\NodeFieldResolver;

final class NodeFieldDefinition implements MappingInterface
{
Expand All @@ -15,15 +14,14 @@ public function toMappingDefinition(array $config)

$idFetcher = $this->cleanIdFetcher($config['idFetcher']);
$nodeInterfaceType = isset($config['nodeInterfaceType']) && is_string($config['nodeInterfaceType']) ? $config['nodeInterfaceType'] : null;
$resolver = addslashes(NodeFieldResolver::class);

return [
'description' => 'Fetches an object given its ID',
'type' => $nodeInterfaceType,
'args' => [
'id' => ['type' => 'ID!', 'description' => 'The ID of an object'],
],
'resolve' => "@=resolver('$resolver', [args, context, info, idFetcherCallback($idFetcher)])",
'resolve' => "@=resolver('relay_node_field', [args, context, info, idFetcherCallback($idFetcher)])",
];
}

Expand Down
5 changes: 1 addition & 4 deletions Relay/Node/PluralIdentifyingRootFieldDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace Overblog\GraphQLBundle\Relay\Node;

use Overblog\GraphQLBundle\Definition\Builder\MappingInterface;
use Overblog\GraphQLBundle\GraphQL\Relay\Node\PluralIdentifyingRootFieldResolver;

final class PluralIdentifyingRootFieldDefinition implements MappingInterface
{
Expand All @@ -26,14 +25,12 @@ public function toMappingDefinition(array $config)
}

$argName = $config['argName'];
$resolver = addslashes(PluralIdentifyingRootFieldResolver::class);

return [
'type' => "[${config['outputType']}]",
'args' => [$argName => ['type' => "[${config['inputType']}!]!"]],
'resolve' => sprintf(
"@=resolver('%s', [args['$argName'], context, info, resolveSingleInputCallback(%s)])",
$resolver,
"@=resolver('relay_plural_identifying_field', [args['$argName'], context, info, resolveSingleInputCallback(%s)])",
$this->cleanResolveSingleInput($config['resolveSingleInput'])
),
];
Expand Down
20 changes: 19 additions & 1 deletion Resolver/AbstractResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Overblog\GraphQLBundle\Resolver;

use Symfony\Component\HttpKernel\Kernel;

abstract class AbstractResolver implements FluentResolverInterface
{
/** @var array */
Expand All @@ -15,8 +17,17 @@ abstract class AbstractResolver implements FluentResolverInterface
/** @var array */
private $fullyLoadedSolutions = [];

/** @var bool */
private $ignoreCase = true;

public function __construct()
{
$this->ignoreCase = version_compare(Kernel::VERSION, '3.3.0') < 0;
}

public function addSolution($id, $solutionOrFactory, array $aliases = [], array $options = [])
{
$id = $this->cleanIdOrAlias($id);
$this->fullyLoadedSolutions[$id] = false;
$this->addAliases($id, $aliases);

Expand Down Expand Up @@ -105,7 +116,7 @@ private function loadSolution($id)
private function addAliases($id, $aliases)
{
foreach ($aliases as $alias) {
$this->aliases[$alias] = $id;
$this->aliases[$this->cleanIdOrAlias($alias)] = $id;
}
}

Expand All @@ -116,9 +127,16 @@ private static function isSolutionFactory($solutionOrFactory)

private function resolveAlias($alias)
{
$alias = $this->cleanIdOrAlias($alias);

return isset($this->aliases[$alias]) ? $this->aliases[$alias] : $alias;
}

private function cleanIdOrAlias($idOrAlias)
{
return $this->ignoreCase ? strtolower($idOrAlias) : $idOrAlias;
}

/**
* @return mixed[]
*/
Expand Down
18 changes: 4 additions & 14 deletions Resolver/TypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,10 @@
namespace Overblog\GraphQLBundle\Resolver;

use GraphQL\Type\Definition\Type;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\Cache\Adapter\ArrayAdapter;

class TypeResolver extends AbstractResolver
{
/** @var CacheItemPoolInterface */
private $cacheAdapter;

public function __construct(CacheItemPoolInterface $cacheAdapter = null)
{
$this->cacheAdapter = null !== $cacheAdapter ? $cacheAdapter : new ArrayAdapter(0, false);
}
private $cache = [];

/**
* @param string $alias
Expand All @@ -26,15 +18,13 @@ public function resolve($alias)
if (null === $alias) {
return;
}
$item = $this->cacheAdapter->getItem(md5($alias));

if (!$item->isHit()) {
if (!isset($this->cache[$alias])) {
$type = $this->string2Type($alias);
$item->set($type);
$this->cacheAdapter->save($item);
$this->cache[$alias] = $type;
}

return $item->get();
return $this->cache[$alias];
}

private function string2Type($alias)
Expand Down
2 changes: 0 additions & 2 deletions Resources/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ services:
overblog_graphql.type_resolver:
class: Overblog\GraphQLBundle\Resolver\TypeResolver
public: true
arguments:
-
tags:
- { name: overblog_graphql.global_variable, alias: typeResolver }

Expand Down
11 changes: 6 additions & 5 deletions Resources/doc/definitions/resolver.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ Resolvers can be define 2 different ways
or `Overblog\GraphQLBundle\Definition\Resolver\MutationInterface`)
in `src/*Bundle/GraphQL` or `app/GraphQL` and they will be auto discovered.
Auto map classes method are accessible by:
* the class method name (example: `AppBunble\GraphQL\CustomResolver::myMethod`)
* the FQCN for callable classes (example: `AppBunble\GraphQL\InvokeResolver` for a resolver implementing the `__invoke` method)
* double-colon (::) to separate service id (class name) and the method names
(example: `AppBunble\GraphQL\CustomResolver::myMethod`)
* for callable classes you can use the service id (example: `AppBunble\GraphQL\InvokeResolver` for a resolver implementing the `__invoke` method)
you can also alias a type by implementing `Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface`
which returns a map of method/alias. The service created will autowire the `__construct`
and `Symfony\Component\DependencyInjection\ContainerAwareInterface::setContainer` methods.
Expand Down Expand Up @@ -133,7 +134,7 @@ Resolvers can be define 2 different ways
```

**Note:**
* When using FQCN in yaml definition, backslash must be correctly quotes,
* When using service id as FQCN in yaml definition, backslashes must be correctly escaped,
here an example `'@=resolver("App\\GraphQL\\Resolver\\Greetings", [args['name']])'`.
* You can also see the more straight forward way using [resolver map](resolver-map.md)

Expand All @@ -151,7 +152,7 @@ Resolvers can be define 2 different ways
#class: App\GraphQL\Resolver\Greetings
tags:
- { name: overblog_graphql.resolver, method: sayHello, alias: say_hello } # add alias say_hello
- { name: overblog_graphql.resolver, method: sayHello } # add method full qualified name
- { name: overblog_graphql.resolver, method: sayHello } # add service id "App\GraphQL\Resolver\Greetings"
```

`SayHello` resolver can be access by using `App\GraphQL\Resolver\Greetings::sayHello` or
Expand All @@ -168,7 +169,7 @@ Resolvers can be define 2 different ways
- { name: overblog_graphql.resolver }
```

This way resolver can be accessed with FQCN `App\GraphQL\Resolver\Greetings`.
This way resolver can be accessed with service id `App\GraphQL\Resolver\Greetings`.

for mutation:

Expand Down
4 changes: 2 additions & 2 deletions Resources/doc/definitions/type-system/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Types can be define 3 different ways:

You can also declare PHP types (any subclass of `GraphQL\Type\Definition\Type`)
in `src/*Bundle/GraphQL` or `app/GraphQL`
they will be auto discover (thanks to auto mapping). Auto map classes are accessible by FQCN
they will be auto discover (thanks to auto mapping). Auto map classes are accessible by service id
(example: `AppBunble\GraphQL\Type\DateTimeType`), you can also alias a type by
implementing `Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface`
that returns an array of aliases.
Expand Down Expand Up @@ -102,7 +102,7 @@ Types can be define 3 different ways:
**Note:**
* Types are lazy loaded so when using Symfony DI `autoconfigure` or this bundle auto mapping, the
only access to type is FQCN (or aliases if implements the aliases interface).
* When using FQCN in yaml definition, backslash must be correctly quotes,
* When using service id as FQCN in yaml definition, backslashes must be correctly escaped,

3. **The service way**

Expand Down
5 changes: 1 addition & 4 deletions Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ public function __construct(ResolverResolver $resolver)
'message' => ['type' => Type::string()],
],
'resolve' => function ($root, $args) use ($resolver) {
return $resolver->resolve([
EchoResolver::class,
[$args['message']],
]);
return $resolver->resolve([EchoResolver::class, [$args['message']]]);
},
],
],
Expand Down
4 changes: 2 additions & 2 deletions Tests/Functional/App/config/autoMapping/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ overblog_graphql:
enabled: true
directories: ["%kernel.root_dir%/IsolatedResolver"]
schema:
query: "Overblog\\GraphQLBundle\\Tests\\Functional\\App\\GraphQL\\HelloWord\\Type\\QueryType"
mutation: "Overblog\\GraphQLBundle\\Tests\\Functional\\App\\GraphQL\\HelloWord\\Type\\MutationType"
query: "Query"
mutation: "Calc"
2 changes: 1 addition & 1 deletion Tests/Functional/App/config/node/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ imports:
- { resource: ../config.yml }

services:
overblog_graphql.test.resolver.node:
node_resolver:
class: Overblog\GraphQLBundle\Tests\Functional\App\Resolver\NodeResolver
arguments:
- "@overblog_graphql.type_resolver"
Expand Down
2 changes: 1 addition & 1 deletion Tests/Functional/App/config/node/mapping/node_type.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Node:
type: relay-node
config:
resolveType: '@=resolver("Overblog\\GraphQLBundle\\Tests\\Functional\\App\\Resolver\\NodeResolver::typeResolver", [value])'
resolveType: '@=resolver("node_resolver::typeResolver", [value])'
2 changes: 1 addition & 1 deletion Tests/Functional/App/config/plural/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ imports:
- { resource: ../config.yml }

services:
overblog_graphql.test.resolver.plural:
plural_resolver:
class: Overblog\GraphQLBundle\Tests\Functional\App\Resolver\PluralResolver
tags:
- { name: "overblog_graphql.resolver" }
Expand Down
Loading