Skip to content
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
10 changes: 5 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ matrix:
- php: 7.0
env: SYMFONY_VERSION=3.2.*
- php: 7.1
env: SYMFONY_VERSION=3.3.* SCRUTINIZER=true
env: SYMFONY_VERSION=3.3.* TEST_COVERAGE=true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool 👍

- php: 7.1
env: GRAPHQLPHP_VERSION=0.10.0
- php: hhvm
Expand All @@ -31,17 +31,17 @@ cache:
- $HOME/.php_cs.cache

before_install:
- if [[ ${SCRUTINIZER} != true && ${TRAVIS_PHP_VERSION} != "hhvm" ]]; then phpenv config-rm xdebug.ini || true; fi
- if [ ${TEST_COVERAGE} != true ]; then phpenv config-rm xdebug.ini || true; fi
- composer selfupdate
- if [ $SYMFONY_VERSION ]; then composer require "symfony/symfony:${SYMFONY_VERSION}" "symfony/framework-bundle:${SYMFONY_VERSION}" --dev --no-update; fi;
- if [ $GRAPHQLPHP_VERSION ]; then composer require "webonyx/graphql-php:${GRAPHQLPHP_VERSION}" --dev --no-update; fi;

install: composer update --prefer-source --no-interaction --optimize-autoloader

script:
- bin/phpunit --debug $( if [ $SCRUTINIZER = true ]; then echo "-d xdebug.max_nesting_level=1000 --coverage-clover=build/logs/clover.xml"; fi; )
- bin/phpunit --debug $( if [ $TEST_COVERAGE = true ]; then echo "-d xdebug.max_nesting_level=1000 --coverage-clover=build/logs/clover.xml"; fi; )
- if [ ${TRAVIS_PHP_VERSION} == "7.0" ]; then bin/php-cs-fixer fix --diff --dry-run -v; fi;

after_script:
- if [ ${SCRUTINIZER} = true ]; then wget https://scrutinizer-ci.com/ocular.phar && php ocular.phar code-coverage:upload --format=php-clover build/logs/clover.xml; fi
- if [ ${SCRUTINIZER} = true ]; then composer require "satooshi/php-coveralls:^1.0" && travis_retry php bin/coveralls -v; fi
- if [ ${TEST_COVERAGE} = true ]; then wget https://scrutinizer-ci.com/ocular.phar && travis_retry php ocular.phar code-coverage:upload --format=php-clover build/logs/clover.xml; fi
- if [ ${TEST_COVERAGE} = true ]; then composer require "satooshi/php-coveralls:^1.0" && travis_retry php bin/coveralls -v; fi
20 changes: 20 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
Contributing
============

Running tests
--------------

Install [phpunit](https://phpunit.de/manual/current/en/installation.html).

In the bundle directory:

```bash
bin/phpunit
```

Giving some love to PHP CS
---------------------------

```bash
bin/php-cs-fixer fix ./
```
6 changes: 3 additions & 3 deletions Config/Parser/YmlParser.php → Config/Parser/YamlParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\Finder\SplFileInfo;
use Symfony\Component\Yaml\Exception\ParseException;
use Symfony\Component\Yaml\Parser as YamlParser;
use Symfony\Component\Yaml\Parser;

class YmlParser implements ParserInterface
class YamlParser implements ParserInterface
{
private static $yamlParser;

Expand All @@ -31,7 +31,7 @@ class YmlParser implements ParserInterface
public static function parse(SplFileInfo $file, ContainerBuilder $container)
{
if (null === self::$yamlParser) {
self::$yamlParser = new YamlParser();
self::$yamlParser = new Parser();
}

try {
Expand Down
12 changes: 11 additions & 1 deletion DependencyInjection/Compiler/ConfigTypesPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,22 @@ class ConfigTypesPass implements CompilerPassInterface
public function process(ContainerBuilder $container)
{
$config = $container->getParameter('overblog_graphql_types.config');
$generatedClasses = $container->get('overblog_graphql.cache_compiler')->compile($this->processConfig($config));
$generatedClasses = $container->get('overblog_graphql.cache_compiler')->compile(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea to extract $container->get('overblog_graphql.cache_compiler') and check its type just for safety and the IDE will also type hint it nicely

$this->processConfig($config),
$container->getParameter('overblog_graphql.use_classloader_listener')
);

foreach ($generatedClasses as $class => $file) {
if (!class_exists($class)) {
throw new \RuntimeException(sprintf(
'Type class %s not found. If you are using your own classLoader verify the path and the namespace please.',
json_encode($class))
);
}
$aliases = call_user_func($class.'::getAliases');
$this->setTypeServiceDefinition($container, $class, $aliases);
}
$container->getParameterBag()->remove('overblog_graphql_types.config');
}

private function setTypeServiceDefinition(ContainerBuilder $container, $class, array $aliases)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ protected function getTagName()
return 'overblog_graphql.mutation';
}

protected function getParameterName()
{
return 'overblog_graphql.mutations_mapping';
}

protected function getResolverServiceID()
{
return 'overblog_graphql.mutation_resolver';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ protected function getTagName()
return 'overblog_graphql.resolver';
}

protected function getParameterName()
{
return 'overblog_graphql.resolvers_mapping';
}

protected function checkRequirements($id, array $tag)
{
parent::checkRequirements($id, $tag);
Expand Down
3 changes: 0 additions & 3 deletions DependencyInjection/Compiler/TaggedServiceMappingPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ private function getTaggedServiceMapping(ContainerBuilder $container, $tagName)
public function process(ContainerBuilder $container)
{
$mapping = $this->getTaggedServiceMapping($container, $this->getTagName());
$container->setParameter($this->getParameterName(), $mapping);
$resolverDefinition = $container->findDefinition($this->getResolverServiceID());

foreach ($mapping as $name => $options) {
Expand Down Expand Up @@ -101,6 +100,4 @@ protected function checkRequirements($id, array $tag)
abstract protected function getTagName();

abstract protected function getResolverServiceID();

abstract protected function getParameterName();
}
5 changes: 0 additions & 5 deletions DependencyInjection/Compiler/TypeTaggedServiceMappingPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ protected function getTagName()
return 'overblog_graphql.type';
}

protected function getParameterName()
{
return 'overblog_graphql.types_mapping';
}

protected function getResolverServiceID()
{
return 'overblog_graphql.type_resolver';
Expand Down
29 changes: 24 additions & 5 deletions DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,26 @@
use GraphQL\Validator\Rules\QueryComplexity;
use GraphQL\Validator\Rules\QueryDepth;
use Overblog\GraphQLBundle\Error\ErrorHandler;
use Overblog\GraphQLBundle\Resolver\Resolver;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;

class Configuration implements ConfigurationInterface
{
private $debug;

private $cacheDir;

/**
* Constructor.
*
* @param bool $debug Whether to use the debug mode
* @param bool $debug Whether to use the debug mode
* @param null|string $cacheDir
*/
public function __construct($debug)
public function __construct($debug, $cacheDir = null)
{
$this->debug = (bool) $debug;
$this->cacheDir = $cacheDir;
}

public function getConfigTreeBuilder()
Expand All @@ -46,6 +51,10 @@ public function getConfigTreeBuilder()
->addDefaultsIfNotSet()
->children()
->scalarNode('internal_error_message')->defaultNull()->end()
->variableNode('default_resolver')->defaultValue([Resolver::class, 'defaultResolveFn'])->end()
->scalarNode('class_namespace')->defaultValue('Overblog\\GraphQLBundle\\__DEFINITIONS__')->end()
->scalarNode('cache_dir')->defaultValue($this->cacheDir.'/overblog/graphql-bundle/__definitions__')->end()
->booleanNode('use_classloader_listener')->defaultTrue()->end()
->booleanNode('show_debug_info')->defaultFalse()->end()
->booleanNode('config_validation')->defaultValue($this->debug)->end()
->arrayNode('schema')
Expand Down Expand Up @@ -89,8 +98,18 @@ public function getConfigTreeBuilder()
->arrayNode('types')
->prototype('array')
->addDefaultsIfNotSet()
->beforeNormalization()
->ifTrue(function ($v) {
return isset($v['type']) && $v['type'] === 'yml';
})
->then(function ($v) {
$v['type'] = 'yaml';

return $v;
})
->end()
->children()
->enumNode('type')->isRequired()->values(['yml', 'xml'])->end()
->enumNode('type')->isRequired()->values(['yaml', 'xml'])->end()
->scalarNode('dir')->defaultNull()->end()
->end()
->end()
Expand Down Expand Up @@ -167,8 +186,8 @@ public function getConfigTreeBuilder()
->arrayNode('versions')
->addDefaultsIfNotSet()
->children()
->scalarNode('graphiql')->defaultValue('0.9')->end()
->scalarNode('react')->defaultValue('15.4')->end()
->scalarNode('graphiql')->defaultValue('0.11')->end()
->scalarNode('react')->defaultValue('15.6')->end()
->scalarNode('fetch')->defaultValue('2.0')->end()
->enumNode('relay')->values(['modern', 'classic'])->defaultValue('classic')->end()
->end()
Expand Down
30 changes: 27 additions & 3 deletions DependencyInjection/OverblogGraphQLExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use GraphQL\Type\Schema;
use Overblog\GraphQLBundle\Config\TypeWithOutputFieldsDefinition;
use Overblog\GraphQLBundle\EventListener\ClassLoaderListener;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
Expand Down Expand Up @@ -45,7 +46,8 @@ public function load(array $configs, ContainerBuilder $container)
$this->setConfigBuilders($config, $container);
$this->setVersions($config, $container);
$this->setShowDebug($config, $container);
$this->setAutoMappingParameters($config, $container);
$this->setDefinitionParameters($config, $container);
$this->setClassLoaderListener($config, $container);

$container->setParameter($this->getAlias().'.resources_dir', realpath(__DIR__.'/../Resources'));
}
Expand All @@ -68,13 +70,35 @@ public function getAlias()

public function getConfiguration(array $config, ContainerBuilder $container)
{
return new Configuration($container->getParameter('kernel.debug'));
return new Configuration(
$container->getParameter('kernel.debug'),
$container->hasParameter('kernel.cache_dir') ? $container->getParameter('kernel.cache_dir') : null
);
}

private function setAutoMappingParameters(array $config, ContainerBuilder $container)
private function setClassLoaderListener(array $config, ContainerBuilder $container)
{
$container->setParameter($this->getAlias().'.use_classloader_listener', $config['definitions']['use_classloader_listener']);
if ($config['definitions']['use_classloader_listener']) {
$definition = $container->setDefinition(
$this->getAlias().'.event_listener.classloader_listener',
new Definition(ClassLoaderListener::class)
);
$definition->setArguments([new Reference($this->getAlias().'.cache_compiler')]);
$definition->addTag('kernel.event_listener', ['event' => 'kernel.request', 'method' => 'load', 'priority' => 255]);
$definition->addTag('kernel.event_listener', ['event' => 'console.command', 'method' => 'load', 'priority' => 255]);
}
}

private function setDefinitionParameters(array $config, ContainerBuilder $container)
{
// auto mapping
$container->setParameter($this->getAlias().'.auto_mapping.enabled', $config['definitions']['auto_mapping']['enabled']);
$container->setParameter($this->getAlias().'.auto_mapping.directories', $config['definitions']['auto_mapping']['directories']);
// generator and config
$container->setParameter($this->getAlias().'.default_resolver', $config['definitions']['default_resolver']);
$container->setParameter($this->getAlias().'.class_namespace', $config['definitions']['class_namespace']);
$container->setParameter($this->getAlias().'.cache_dir', $config['definitions']['cache_dir']);
}

private function setBatchingMethod(array $config, ContainerBuilder $container)
Expand Down
6 changes: 4 additions & 2 deletions DependencyInjection/OverblogGraphQLTypesExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@

class OverblogGraphQLTypesExtension extends Extension
{
private static $configTypes = ['yml', 'xml'];
private static $configTypes = ['yaml', 'xml'];

private static $typeExtensions = ['yaml' => '{yaml,yml}', 'xml' => 'xml'];

public function load(array $configs, ContainerBuilder $container)
{
Expand Down Expand Up @@ -112,7 +114,7 @@ private function detectFilesByType(ContainerBuilder $container, $path, $type = n

foreach ($types as $type) {
try {
$finder->files()->in($path)->name('*.types.'.$type);
$finder->files()->in($path)->name('*.types.'.self::$typeExtensions[$type]);
} catch (\InvalidArgumentException $e) {
continue;
}
Expand Down
21 changes: 10 additions & 11 deletions Executor/Executor.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

namespace Overblog\GraphQLBundle\Executor;

use GraphQL\Executor\ExecutionResult;
use GraphQL\Executor\Promise\Promise;
use GraphQL\Executor\Promise\PromiseAdapter;
use GraphQL\GraphQL;
use GraphQL\Type\Schema;
Expand All @@ -25,14 +23,7 @@ class Executor implements ExecutorInterface
private $promiseAdapter;

/**
* @param Schema $schema
* @param string $requestString
* @param null|array $rootValue
* @param null|array $contextValue
* @param null|array $variableValues
* @param null|string $operationName
*
* @return ExecutionResult|Promise
* {@inheritdoc}
*/
public function execute(Schema $schema, $requestString, $rootValue = null, $contextValue = null, $variableValues = null, $operationName = null)
{
Expand All @@ -49,10 +40,18 @@ public function execute(Schema $schema, $requestString, $rootValue = null, $cont
}

/**
* @param PromiseAdapter|null $promiseAdapter
* {@inheritdoc}
*/
public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null)
{
$this->promiseAdapter = $promiseAdapter;
}

/**
* {@inheritdoc}
*/
public function setDefaultFieldResolver(callable $fn)
{
call_user_func_array(sprintf('\%s::setDefaultFieldResolver', GraphQL::class), func_get_args());
}
}
5 changes: 5 additions & 0 deletions Executor/ExecutorInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,9 @@ public function execute(Schema $schema, $requestString, $rootValue = null, $cont
* @param PromiseAdapter|null $promiseAdapter
*/
public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null);

/**
* @param callable $fn
*/
public function setDefaultFieldResolver(callable $fn);
}
20 changes: 13 additions & 7 deletions Generator/TypeGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,13 @@ public function compile(array $configs, $loadClasses = true)

$classes = $this->generateClasses($configs, $cacheDir, true);

$content = "<?php\nreturn ".var_export($classes, true).';';
// replaced hard-coding absolute path by __DIR__ (see https://github.com/overblog/GraphQLBundle/issues/167)
$content = str_replace(' => \''.$cacheDir, ' => __DIR__ . \'', $content);
if ($loadClasses) {
$content = "<?php\nreturn ".var_export($classes, true).';';
// replaced hard-coding absolute path by __DIR__ (see https://github.com/overblog/GraphQLBundle/issues/167)
$content = str_replace(' => \''.$cacheDir, ' => __DIR__ . \'', $content);

file_put_contents($this->getClassesMap(), $content);
file_put_contents($this->getClassesMap(), $content);

if ($loadClasses) {
$this->loadClasses(true);
}

Expand All @@ -216,8 +216,14 @@ public function loadClasses($forceReload = false)
{
if (!self::$classMapLoaded || $forceReload) {
$classes = require $this->getClassesMap();

$mapClassLoader = new ClassLoader();
/** @var ClassLoader $mapClassLoader */
static $mapClassLoader = null;
if (null === $mapClassLoader) {
$mapClassLoader = new ClassLoader();
$mapClassLoader->setClassMapAuthoritative(true);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to enter this else statement? $mapClassLoader seems to be always null to me or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it is null on init because $mapClassLoader is declare as static so after it init it always has the same instance of ClassLoader.

$mapClassLoader->unregister();
}
$mapClassLoader->addClassMap($classes);
$mapClassLoader->register();

Expand Down
Loading