Skip to content

Commit

Permalink
Fix review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mcg-web committed Mar 5, 2018
1 parent 7c887f8 commit 04e48ae
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 31 deletions.
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
6 changes: 3 additions & 3 deletions Resources/doc/definitions/resolver.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ 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:
* double colon (::) to separate service id (class name) and the method names
(example: `AppBunble\GraphQL\CustomResolver::myMethod` or `appbunble\graphql\customresolver::myMethod` for Symfony < 3.3)
* 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`
Expand Down Expand Up @@ -134,7 +134,7 @@ Resolvers can be define 2 different ways
```

**Note:**
* When using service id as 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 Down
2 changes: 1 addition & 1 deletion Resources/doc/definitions/type-system/index.md
Original file line number Diff line number Diff line change
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 service id as 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
6 changes: 1 addition & 5 deletions Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface;
use Overblog\GraphQLBundle\Resolver\ResolverResolver;
use Overblog\GraphQLBundle\Tests\Functional\App\IsolatedResolver\EchoResolver;
use Symfony\Component\HttpKernel\Kernel;

final class QueryType extends ObjectType implements AliasedInterface
{
Expand All @@ -22,10 +21,7 @@ public function __construct(ResolverResolver $resolver)
'message' => ['type' => Type::string()],
],
'resolve' => function ($root, $args) use ($resolver) {
return $resolver->resolve([
version_compare(Kernel::VERSION, '3.3.0') < 0 ? strtolower(EchoResolver::class) : EchoResolver::class,
[$args['message']],
]);
return $resolver->resolve([EchoResolver::class, [$args['message']]]);
},
],
],
Expand Down
4 changes: 2 additions & 2 deletions UPGRADE-0.11.md
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ UPGRADE FROM 0.10 to 0.11
### Change fluent resolvers id

The use of class name as prefix of fluent resolver id remove the possibility to use same class as 2 different services.
see this [issue for more detail](https://github.com/overblog/GraphQLBundle/issues/296)
That the reason from 0.11 we using service id as prefix (like in Symfony 4.1)...
See issue [#296](https://github.com/overblog/GraphQLBundle/issues/296) for more detail
That's the reason why starting v0.11 we are using service id as prefix (like in Symfony 4.1)...

Example:
```yaml
Expand Down
4 changes: 1 addition & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@
},
"require": {
"php": ">=5.6",
"doctrine/doctrine-cache-bundle": "^1.2",
"overblog/graphql-php-generator": "^0.7.0",
"symfony/cache": "^3.1 || ^4.0",
"psr/log": "^1.0",
"symfony/config": "^3.1 || ^4.0",
"symfony/dependency-injection": "^3.1 || ^4.0",
"symfony/event-dispatcher": "^3.1 || ^4.0",
Expand All @@ -49,7 +48,6 @@
},
"require-dev": {
"phpunit/phpunit": "^5.7.26 || ^6.0",
"psr/log": "^1.0",
"react/promise": "^2.5",
"sensio/framework-extra-bundle": "^3.0",
"symfony/asset": "^3.1 || ^4.0",
Expand Down

0 comments on commit 04e48ae

Please sign in to comment.