Skip to content

Commit e8ea29e

Browse files
Damienbelingherimcg-webBenMorelVincz
authored
Add annotation reader cache and fix doctrine typeGuesser (#867)
* Bump master to 1.0-dev * Fix master branch badges * Fix typo * Typo fix * Update AnnotationParser.php Update AnnotationParser.php Update AnnotationParser.php * Fix resolveTypeFromDoctrineType() * Fix resolveTypeFromDoctrineType * fix return getAnnotationReader * fix single quote * change version symfony/cache * remove symfony/cache from composer * add check symfony/cache is installed and improve doc * add check symfony/cache is installed and improve doc * Update index.md * Update composer.json * Test for annotation dependencies package * Fix ci.yml * Make doctrine/orm & doctrine/annotations optionnal * Skip validator test if no doctrine/annotations * revert branch 1.0 -> 0.14 * revert version 1 -> 0.14 * Fix ci & composer Co-authored-by: Jeremiah VALERIE <[email protected]> Co-authored-by: Benjamin Morel <[email protected]> Co-authored-by: Jeremiah VALERIE <[email protected]> Co-authored-by: Vincent <[email protected]> Co-authored-by: Vincent <[email protected]>
1 parent f303a03 commit e8ea29e

File tree

14 files changed

+148
-51
lines changed

14 files changed

+148
-51
lines changed

.github/workflows/ci.yml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ jobs:
3131
- php-version: '8.0'
3232
symfony-version: '5.3'
3333
dependencies: 'lowest'
34-
remove-dependencies: '--dev symfony/validator'
34+
remove-dependencies: '--dev symfony/validator doctrine/orm doctrine/annotations'
3535
- php-version: '8.0'
3636
symfony-version: '5.3'
3737
dependencies: 'lowest'
@@ -77,7 +77,6 @@ jobs:
7777
run: |
7878
composer global require php-coveralls/php-coveralls
7979
php-coveralls --coverage_clover=build/logs/clover.xml -v
80-
8180
coding-standard:
8281
runs-on: ubuntu-20.04
8382
name: Coding Standard
@@ -130,4 +129,4 @@ jobs:
130129
uses: ramsey/[email protected]
131130

132131
- name: "Run static-analysis"
133-
run: composer static-analysis
132+
run: composer static-analysis

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ Talks and slides to help you start
9696

9797
* GraphQL in Symfony *by Bernd Alter* - [Twitter](https://twitter.com/bazoo0815)
9898
- [Talk about GraphQL and its implementation with Symfony (26.04.2017)](https://www.slideshare.net/berndalter7/graphql-in-symfony) `English`
99-
* GraphQL is right in front of us, let's to it! *by Renato Mendes Figueiredo* - [Twitter](https://twitter.com/renatomefi), [GitHub](https://github.com/renatomefi)
99+
* GraphQL is right in front of us, let's do it! *by Renato Mendes Figueiredo* - [Twitter](https://twitter.com/renatomefi), [GitHub](https://github.com/renatomefi)
100100
- [Slides at http://talks.mefi.in/graphql-scotphp17](http://talks.mefi.in/graphql-scotphp17/) `English`
101101
- [Video at SymfonyCamp UA 2017](https://www.youtube.com/watch?v=jyoYlnCPNgk) `English`
102102
- [Video at DPC 2017](https://www.youtube.com/watch?v=E7MjoCOGSSY) `English`

composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"type": "symfony-bundle",
44
"license": "MIT",
55
"description": "This bundle provides tools to build a GraphQL server in your Symfony App.",
6-
"keywords": ["GraphQL", "Relay"],
6+
"keywords": ["GraphQL","Relay"],
77
"authors": [
88
{
99
"name": "Overblog",
@@ -83,7 +83,7 @@
8383
"static-analysis": [
8484
"phpstan analyse --ansi --memory-limit=1G"
8585
],
86-
"bench": [
86+
"bench": [
8787
"test -f phpbench.phar || wget https://github.com/phpbench/phpbench/releases/download/1.0.0-alpha7/phpbench.phar -O phpbench.phar",
8888
"@php phpbench.phar run -l dots --ansi -vvv --report='generator: \"table\", cols: [\"benchmark\", \"subject\", \"params\", \"best\", \"mean\", \"mode\", \"worst\", \"diff\"], break: [\"benchmark\"], sort: {mean: \"asc\"}'"
8989
],

docs/annotations/index.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
In order to use annotations or attributes, you need to configure the mapping:
44

5-
To use annotations, use the `annotation` mapping type.
5+
To use annotations, You must install `symfony/cache` and `doctrine/annotation` and use the `annotation` mapping type.
6+
7+
68
```yaml
79
# config/packages/graphql.yaml
810
overblog_graphql:
@@ -13,7 +15,6 @@ overblog_graphql:
1315
dir: "%kernel.project_dir%/src/GraphQL"
1416
suffix: ~
1517
```
16-
1718
To use attributes, use the `attribute` mapping type.
1819

1920
```yaml
@@ -215,6 +216,7 @@ In this example, the type `String!` will be auto-guessed from the type hint of t
215216
### @Field type auto-guessing from Doctrine ORM Annotations
216217

217218
Based on other Doctrine annotations on your fields, the corresponding GraphQL type can sometimes be guessed automatically.
219+
In order to activate this guesser, you must install `doctrine/orm` package.
218220

219221
The type can be auto-guessed from the following annotations:
220222

docs/error-handling/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ Custom error handling / formatting
159159
-----------------------------------
160160

161161
This can also be done by using events.
162-
* First totally disabled default errors handler:
162+
* First totally disable default errors handler:
163163
```yaml
164164
overblog_graphql:
165165
errors_handler: false

docs/index.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,15 @@ overblog_graphql_multiple_endpoint:
102102
prefix: /graphql
103103
```
104104
105+
106+
Optionnal features depencies
107+
------------
108+
109+
- To use the Validator features, you must also install `symfony/validator` and `doctrine/annotations`
110+
- To use the annotations, you must also install `doctrine/annotations`
111+
- To use the annotations doctrine type guesser, you must also install `doctrine/orm`
112+
113+
105114
Composer autoloader configuration (optional)
106115
------------
107116

src/Config/Parser/AnnotationParser.php

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,22 @@
66

77
use Doctrine\Common\Annotations\AnnotationReader;
88
use Doctrine\Common\Annotations\AnnotationRegistry;
9+
use Doctrine\Common\Annotations\PsrCachedReader;
10+
use Doctrine\Common\Annotations\Reader;
911
use Overblog\GraphQLBundle\Config\Parser\MetadataParser\MetadataParser;
1012
use ReflectionClass;
1113
use ReflectionMethod;
1214
use ReflectionProperty;
1315
use Reflector;
14-
use RuntimeException;
16+
use Symfony\Component\Cache\Adapter\ApcuAdapter;
17+
use Symfony\Component\Cache\Adapter\PhpFilesAdapter;
18+
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
1519

1620
class AnnotationParser extends MetadataParser
1721
{
1822
public const METADATA_FORMAT = '@%s';
1923

20-
protected static ?AnnotationReader $annotationReader = null;
24+
protected static Reader $annotationReader;
2125

2226
protected static function getMetadatas(Reflector $reflector): array
2327
{
@@ -32,18 +36,29 @@ protected static function getMetadatas(Reflector $reflector): array
3236
return [];
3337
}
3438

35-
protected static function getAnnotationReader(): AnnotationReader
39+
public static function getAnnotationReader(): Reader
3640
{
37-
if (null === self::$annotationReader) {
38-
if (!class_exists(AnnotationReader::class) ||
39-
!class_exists(AnnotationRegistry::class)) {
40-
// @codeCoverageIgnoreStart
41-
throw new RuntimeException('In order to use graphql annotation, you need to require doctrine annotations');
42-
// @codeCoverageIgnoreEnd
41+
if (!isset(self::$annotationReader)) {
42+
if (!class_exists(AnnotationReader::class)) {
43+
throw new ServiceNotFoundException("In order to use annotations, you need to install 'doctrine/annotations' first. See: 'https://www.doctrine-project.org/projects/annotations.html'");
4344
}
45+
if (!class_exists(ApcuAdapter::class)) {
46+
throw new ServiceNotFoundException("In order to use annotations, you need to install 'symfony/cache' first. See: 'https://symfony.com/doc/current/components/cache.html'");
47+
}
48+
49+
if (class_exists(AnnotationRegistry::class)) {
50+
AnnotationRegistry::registerLoader('class_exists');
51+
}
52+
$cacheKey = md5(__DIR__);
53+
// @codeCoverageIgnoreStart
54+
if (extension_loaded('apcu') && apcu_enabled()) {
55+
$annotationCache = new ApcuAdapter($cacheKey);
56+
} else {
57+
$annotationCache = new PhpFilesAdapter($cacheKey);
58+
}
59+
// @codeCoverageIgnoreEnd
4460

45-
AnnotationRegistry::registerLoader('class_exists');
46-
self::$annotationReader = new AnnotationReader();
61+
self::$annotationReader = new PsrCachedReader(new AnnotationReader(), $annotationCache, true);
4762
}
4863

4964
return self::$annotationReader;

src/Config/Parser/MetadataParser/TypeGuesser/DoctrineTypeGuesser.php

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,22 @@
44

55
namespace Overblog\GraphQLBundle\Config\Parser\MetadataParser\TypeGuesser;
66

7-
use Doctrine\Common\Annotations\AnnotationReader;
8-
use Doctrine\Common\Annotations\AnnotationRegistry;
97
use Doctrine\ORM\Mapping\Annotation as MappingAnnotation;
108
use Doctrine\ORM\Mapping\Column;
119
use Doctrine\ORM\Mapping\JoinColumn;
1210
use Doctrine\ORM\Mapping\ManyToMany;
1311
use Doctrine\ORM\Mapping\ManyToOne;
1412
use Doctrine\ORM\Mapping\OneToMany;
1513
use Doctrine\ORM\Mapping\OneToOne;
14+
use Overblog\GraphQLBundle\Config\Parser\AnnotationParser;
1615
use Overblog\GraphQLBundle\Config\Parser\MetadataParser\ClassesTypesMap;
1716
use ReflectionClass;
1817
use ReflectionMethod;
1918
use ReflectionProperty;
2019
use Reflector;
21-
use RuntimeException;
2220

2321
class DoctrineTypeGuesser extends TypeGuesser
2422
{
25-
protected ?AnnotationReader $annotationReader = null;
2623
protected array $doctrineMapping = [];
2724

2825
public function __construct(ClassesTypesMap $map, array $doctrineMapping = [])
@@ -46,14 +43,19 @@ public function supports(Reflector $reflector): bool
4643
*/
4744
public function guessType(ReflectionClass $reflectionClass, Reflector $reflector, array $filterGraphQLTypes = []): ?string
4845
{
46+
if (!class_exists(Column::class)) {
47+
throw new TypeGuessingException(sprintf('You must install doctrine/orm package to use this type guesser.'));
48+
}
49+
4950
if (!$reflector instanceof ReflectionProperty) {
5051
throw new TypeGuessingException('Doctrine type guesser only apply to properties.');
5152
}
53+
5254
/** @var Column|null $columnAnnotation */
5355
$columnAnnotation = $this->getAnnotation($reflector, Column::class);
5456

5557
if (null !== $columnAnnotation) {
56-
$type = $this->resolveTypeFromDoctrineType($columnAnnotation->type ?? 'string');
58+
$type = $this->resolveTypeFromDoctrineType($columnAnnotation->type ?: 'string');
5759
$nullable = $columnAnnotation->nullable;
5860
if ($type) {
5961
return $nullable ? $type : sprintf('%s!', $type);
@@ -100,7 +102,7 @@ public function guessType(ReflectionClass $reflectionClass, Reflector $reflector
100102

101103
private function getAnnotation(Reflector $reflector, string $annotationClass): ?MappingAnnotation
102104
{
103-
$reader = $this->getAnnotationReader();
105+
$reader = AnnotationParser::getAnnotationReader();
104106
$annotations = [];
105107
switch (true) {
106108
case $reflector instanceof ReflectionClass: $annotations = $reader->getClassAnnotations($reflector); break;
@@ -117,21 +119,6 @@ private function getAnnotation(Reflector $reflector, string $annotationClass): ?
117119
return null;
118120
}
119121

120-
private function getAnnotationReader(): AnnotationReader
121-
{
122-
if (null === $this->annotationReader) {
123-
if (!class_exists(AnnotationReader::class) ||
124-
!class_exists(AnnotationRegistry::class)) {
125-
throw new RuntimeException('In order to use graphql annotation/attributes, you need to require doctrine annotations');
126-
}
127-
128-
AnnotationRegistry::registerLoader('class_exists');
129-
$this->annotationReader = new AnnotationReader();
130-
}
131-
132-
return $this->annotationReader;
133-
}
134-
135122
/**
136123
* Resolve a FQN from classname and namespace.
137124
*

tests/Config/Parser/AnnotationParserTest.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,22 @@
66

77
use Overblog\GraphQLBundle\Config\Parser\AnnotationParser;
88
use SplFileInfo;
9+
use Symfony\Component\DependencyInjection\ContainerBuilder;
10+
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
11+
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
912

1013
class AnnotationParserTest extends MetadataParserTest
1114
{
15+
public function setUp(): void
16+
{
17+
if ('testNoDoctrineAnnotations' !== $this->getName()) {
18+
if (!self::isDoctrineAnnotationInstalled()) {
19+
$this->markTestSkipped('doctrine/annotations are not installed. Skipping annotation parser tests.');
20+
}
21+
parent::setUp();
22+
}
23+
}
24+
1225
public function parser(string $method, ...$args)
1326
{
1427
return AnnotationParser::$method(...$args);
@@ -19,6 +32,21 @@ public function formatMetadata(string $metadata): string
1932
return sprintf('@%s', $metadata);
2033
}
2134

35+
public function testNoDoctrineAnnotations(): void
36+
{
37+
if (self::isDoctrineAnnotationInstalled()) {
38+
$this->markTestSkipped('doctrine/annotations are installed');
39+
}
40+
41+
try {
42+
$containerBuilder = $this->getMockBuilder(ContainerBuilder::class)->disableOriginalConstructor()->getMock();
43+
AnnotationParser::parse(new SplFileInfo(__DIR__.'/fixtures/annotations/Type/Animal.php'), $containerBuilder);
44+
} catch (InvalidArgumentException $e) {
45+
$this->assertInstanceOf(ServiceNotFoundException::class, $e->getPrevious());
46+
$this->assertMatchesRegularExpression('/doctrine\/annotations/', $e->getPrevious()->getMessage());
47+
}
48+
}
49+
2250
public function testLegacyNestedAnnotations(): void
2351
{
2452
$this->config = self::cleanConfig($this->parser('parse', new SplFileInfo(__DIR__.'/fixtures/annotations/Deprecated/DeprecatedNestedAnnotations.php'), $this->containerBuilder, ['doctrine' => ['types_mapping' => []]]));

tests/Config/Parser/MetadataParser/TypeGuesser/DoctrineTypeGuesserTest.php

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Overblog\GraphQLBundle\Tests\Config\Parser;
66

7+
use Doctrine\ORM\Mapping\Column;
78
use Exception;
89
use Overblog\GraphQLBundle\Config\Parser\MetadataParser\ClassesTypesMap;
910
use Overblog\GraphQLBundle\Config\Parser\MetadataParser\TypeGuesser\DoctrineTypeGuesser;
@@ -15,24 +16,47 @@ class DoctrineTypeGuesserTest extends TestCase
1516
// @phpstan-ignore-next-line
1617
protected $property;
1718

19+
public static function isDoctrineInstalled(): bool
20+
{
21+
return class_exists(Column::class);
22+
}
23+
1824
public function testGuessError(): void
1925
{
26+
if (!self::isDoctrineInstalled()) {
27+
$this->markTestSkipped('Doctrine ORM is not installed');
28+
}
29+
2030
$refClass = new ReflectionClass(__CLASS__);
21-
$docBlockGuesser = new DoctrineTypeGuesser(new ClassesTypesMap());
31+
$doctrineGuesser = new DoctrineTypeGuesser(new ClassesTypesMap());
2232

2333
try {
2434
// @phpstan-ignore-next-line
25-
$docBlockGuesser->guessType($refClass, $refClass);
35+
$doctrineGuesser->guessType($refClass, $refClass);
2636
} catch (Exception $e) {
2737
$this->assertInstanceOf(TypeGuessingException::class, $e);
2838
$this->assertStringContainsString('Doctrine type guesser only apply to properties.', $e->getMessage());
2939
}
3040

3141
try {
32-
$docBlockGuesser->guessType($refClass, $refClass->getProperty('property'));
42+
$doctrineGuesser->guessType($refClass, $refClass->getProperty('property'));
3343
} catch (Exception $e) {
3444
$this->assertInstanceOf(TypeGuessingException::class, $e);
3545
$this->assertStringContainsString('No Doctrine ORM annotation found.', $e->getMessage());
3646
}
3747
}
48+
49+
public function testDoctrineOrmNotInstalled(): void
50+
{
51+
if (self::isDoctrineInstalled()) {
52+
$this->markTestSkipped('Doctrine ORM is installed');
53+
}
54+
55+
$this->expectException(TypeGuessingException::class);
56+
$this->expectExceptionMessageMatches('/^You must install doctrine\/orm/');
57+
58+
$refClass = new ReflectionClass(__CLASS__);
59+
$doctrineGuesser = new DoctrineTypeGuesser(new ClassesTypesMap());
60+
$doctrineGuesser->guessType($refClass, $refClass->getProperty('property'));
61+
}
3862
}

0 commit comments

Comments
 (0)