Refactor TypeGenerator

* Move generator options into a separate class
* Update UPGRADE.md

Author: murtukov

UPGRADE.md

@@ -43,25 +43,30 @@ $connectionBuilder = new ConnectionBuilder(
 
 ### Change arguments of `TypeGenerator` class
 
-The `Overblog\GraphQLBundle\Generator\TypeGenerator` service is used internally for GraphQL types compilation. If you 
+The `Overblog\GraphQLBundle\Generator\TypeGenerator` service is used internally for compilation of GraphQL types. If you 
 overrode the service definition, please take into account the new constructor signature:
 
-```diff
+```php
 public function __construct(
-    string $classNamespace,
--   array $skeletonDirs,
+   TypeBuilder $typeBuilder,
+   EventDispatcherInterface $eventDispatcher,
+   TypeGeneratorOptions $options
+)
+```
+`TypeBuilder` here is a new service `Overblog\GraphQLBundle\Generator\TypeBuilder`, which is also used internally.
+The rest of the arguments were moved into the separate class `Overblog\GraphQLBundle\Generator\TypeGeneratorOptions` 
+with the following constructor signature:
+
+```php
+public function __construct(
+    string $namespace,
     ?string $cacheDir,
-    array $configs,
-+   TypeBuilder $typeBuilder
-+   EventDispatcherInterface $eventDispatcher
+    array $types,
     bool $useClassMap = true,
--   callable $configProcessor = null,
-    ?string $baseCacheDir = null,
+    ?string $cacheBaseDir = null,
     ?int $cacheDirMask = null
-) {
+)
 ```
-`TypeBuilder` here is a new service `Overblog\GraphQLBundle\Generator\TypeBuilder`, which is also used internally.
-
 ### Add magic `__get` method to `ArgumentInterface` implementors
 
 The interface `Overblog\GraphQLBundle\Definition\ArgumentInterface` as well as implementing it class 

src/CacheWarmer/CompileCacheWarmer.php

@@ -19,32 +19,33 @@ public function __construct(TypeGenerator $typeGenerator, bool $compiled = true)
     }
 
     /**
-     * {@inheritdoc}
+     * @return bool
      */
     public function isOptional()
     {
         return !$this->compiled;
     }
 
     /**
-     * {@inheritdoc}
-     *
      * @param string $cacheDir
      *
-     * @return string[]
+     * @return array
      */
     public function warmUp($cacheDir)
     {
         if ($this->compiled) {
-            // use warm up cache dir if type generator cache dir not already explicitly declare
-            $baseCacheDir = $this->typeGenerator->getBaseCacheDir();
-            if (null === $this->typeGenerator->getCacheDir(false)) {
-                $this->typeGenerator->setBaseCacheDir($cacheDir);
+            // use warm up cache dir if type generator cache dir not already explicitly declared
+            $options = $this->typeGenerator->options;
+            $cacheBaseDir = $options->cacheBaseDir;
+
+            if (null === $options->cacheDir) {
+                $options->cacheBaseDir = $cacheDir;
             }
+
             $this->typeGenerator->compile(TypeGenerator::MODE_WRITE | TypeGenerator::MODE_OVERRIDE);
 
-            if (null !== $baseCacheDir) {
-                $this->typeGenerator->setBaseCacheDir($baseCacheDir);
+            if (null !== $cacheBaseDir) {
+                $options->cacheBaseDir = $cacheBaseDir;
             }
         }
 

src/Generator/TypeGenerator.php

@@ -24,74 +24,23 @@ class TypeGenerator
     public const MODE_MAPPING_ONLY = 2;
     public const MODE_WRITE = 4;
     public const MODE_OVERRIDE = 8;
-
     public const GRAPHQL_SERVICES = 'services';
 
     private static bool $classMapLoaded = false;
-    private ?string $cacheDir;
-    protected int $cacheDirMask;
-    private array $configs;
-    private bool $useClassMap;
-    private ?string $baseCacheDir;
-    private string $classNamespace;
+    public TypeGeneratorOptions $options;
     private TypeBuilder $typeBuilder;
-    private EventDispatcherInterface $eventDispatcher;
-
-    public function __construct(
-        string $classNamespace,
-        ?string $cacheDir,
-        array $configs,
-        TypeBuilder $typeBuilder,
-        EventDispatcherInterface $eventDispatcher,
-        bool $useClassMap = true,
-        ?string $baseCacheDir = null,
-        ?int $cacheDirMask = null
-    ) {
-        $this->cacheDir = $cacheDir;
-        $this->configs = $configs;
-        $this->useClassMap = $useClassMap;
-        $this->baseCacheDir = $baseCacheDir;
-        $this->typeBuilder = $typeBuilder;
-        $this->eventDispatcher = $eventDispatcher;
-        $this->classNamespace = $classNamespace;
-
-        if (null === $cacheDirMask) {
-            // Apply permission 0777 for default cache dir otherwise apply 0775.
-            $cacheDirMask = null === $cacheDir ? 0777 : 0775;
-        }
-
-        $this->cacheDirMask = $cacheDirMask;
-    }
+    private EventDispatcherInterface $dispatcher;
 
-    public function getBaseCacheDir(): ?string
+    public function __construct(TypeBuilder $typeBuilder, EventDispatcherInterface $dispatcher, TypeGeneratorOptions $options)
     {
-        return $this->baseCacheDir;
-    }
-
-    public function setBaseCacheDir(string $baseCacheDir): void
-    {
-        $this->baseCacheDir = $baseCacheDir;
-    }
-
-    public function getCacheDir(bool $useDefault = true): ?string
-    {
-        if ($useDefault) {
-            return $this->cacheDir ?: $this->baseCacheDir.'/overblog/graphql-bundle/__definitions__';
-        } else {
-            return $this->cacheDir;
-        }
-    }
-
-    public function setCacheDir(?string $cacheDir): self
-    {
-        $this->cacheDir = $cacheDir;
-
-        return $this;
+        $this->typeBuilder = $typeBuilder;
+        $this->dispatcher = $dispatcher;
+        $this->options = $options;
     }
 
     public function compile(int $mode): array
     {
-        $cacheDir = $this->getCacheDir();
+        $cacheDir = $this->options->getCacheDirOrDefault();
         $writeMode = $mode & self::MODE_WRITE;
 
         // Configure write mode
@@ -101,19 +50,19 @@ public function compile(int $mode): array
         }
 
         // Process configs
-        $configs = Processor::process($this->configs);
+        $types = Processor::process($this->options->types);
 
         // Generate classes
         $classes = [];
-        foreach ($configs as $name => $config) {
+        foreach ($types as $name => $config) {
             $config['config']['name'] ??= $name;
             $config['config']['class_name'] = $config['class_name'];
             $classMap = $this->generateClass($config, $cacheDir, $mode);
             $classes = array_merge($classes, $classMap);
         }
 
         // Create class map file
-        if ($writeMode && $this->useClassMap && count($classes) > 0) {
+        if ($writeMode && $this->options->useClassMap && count($classes) > 0) {
             $content = "<?php\nreturn ".var_export($classes, true).';';
 
             // replaced hard-coded absolute paths by __DIR__
@@ -125,7 +74,7 @@ public function compile(int $mode): array
             $this->loadClasses(true);
         }
 
-        $this->eventDispatcher->dispatch(new SchemaCompiledEvent());
+        $this->dispatcher->dispatch(new SchemaCompiledEvent());
 
         return $classes;
     }
@@ -145,12 +94,14 @@ public function generateClass(array $config, ?string $outputDirectory, int $mode
             }
         }
 
-        return ["$this->classNamespace\\$className" => $path];
+        $namespace = $this->options->namespace;
+
+        return ["$namespace\\$className" => $path];
     }
 
     public function loadClasses(bool $forceReload = false): void
     {
-        if ($this->useClassMap && (!self::$classMapLoaded || $forceReload)) {
+        if ($this->options->useClassMap && (!self::$classMapLoaded || $forceReload)) {
             $classMapFile = $this->getClassesMap();
             $classes = file_exists($classMapFile) ? require $classMapFile : [];
 
@@ -173,6 +124,6 @@ public function loadClasses(bool $forceReload = false): void
 
     private function getClassesMap(): string
     {
-        return $this->getCacheDir().'/__classes.map';
+        return $this->options->getCacheDirOrDefault().'/__classes.map';
     }
 }

src/Generator/TypeGeneratorOptions.php

@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Overblog\GraphQLBundle\Generator;
+
+class TypeGeneratorOptions
+{
+    public string $namespace;
+    public ?string $cacheDir;
+    public array $types;
+    public int $cacheDirMask;
+    public bool $useClassMap = true;
+    public ?string $cacheBaseDir;
+
+    public function __construct(
+        string $namespace,
+        ?string $cacheDir,
+        array $types,
+        bool $useClassMap = true,
+        ?string $cacheBaseDir = null,
+        ?int $cacheDirMask = null
+    ) {
+        $this->namespace = $namespace;
+        $this->cacheDir = $cacheDir;
+        $this->types = $types;
+        $this->useClassMap = $useClassMap;
+        $this->cacheBaseDir = $cacheBaseDir;
+
+        if (null === $cacheDirMask) {
+            // Apply permission 0777 for default cache dir otherwise apply 0775.
+            $cacheDirMask = null === $cacheDir ? 0777 : 0775;
+        }
+
+        $this->cacheDirMask = $cacheDirMask;
+    }
+
+    public function getCacheDirOrDefault(): string
+    {
+        return $this->cacheDir ?? $this->cacheBaseDir.'/overblog/graphql-bundle/__definitions__';
+    }
+}

src/Resources/config/services.yaml

@@ -35,14 +35,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:
@@ -54,22 +54,25 @@ services:
 
     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.class_namespace%"
-            - "%overblog_graphql.cache_dir%"
-            - "%overblog_graphql_types.config%"
             - '@Overblog\GraphQLBundle\Generator\TypeBuilder'
             - '@Symfony\Contracts\EventDispatcher\EventDispatcherInterface'
-            - "%overblog_graphql.use_classloader_listener%"
-            - "%kernel.cache_dir%"
-            - "%overblog_graphql.cache_dir_permissions%"
+            - !service
+                class: Overblog\GraphQLBundle\Generator\TypeGeneratorOptions
+                arguments:
+                    - '%overblog_graphql.class_namespace%'
+                    - '%overblog_graphql.cache_dir%'
+                    - '%overblog_graphql_types.config%'
+                    - '%overblog_graphql.use_classloader_listener%'
+                    - '%kernel.cache_dir%'
+                    - '%overblog_graphql.cache_dir_permissions%'
 
     Overblog\GraphQLBundle\Definition\ArgumentFactory:
         arguments:

tests/ExpressionLanguage/ExpressionFunction/Security/IsGrantedTest.php

@@ -5,8 +5,8 @@
 namespace Overblog\GraphQLBundle\Tests\ExpressionLanguage\ExpressionFunction\Security;
 
 use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\Security\IsGranted;
+use Overblog\GraphQLBundle\Generator\TypeGenerator;
 use Overblog\GraphQLBundle\Tests\ExpressionLanguage\TestCase;
-use Overblog\GraphQLBundle\Tests\Generator\TypeGenerator;
 
 class IsGrantedTest extends TestCase
 {

tests/Functional/Command/CompileCommandTest.php

@@ -31,8 +31,9 @@ public function setUp(): void
         $this->typesMapping = static::$kernel->getContainer()->get('overblog_graphql.cache_compiler')
             ->compile(TypeGenerator::MODE_MAPPING_ONLY);
 
-        // @phpstan-ignore-next-line
-        $this->cacheDir = static::$kernel->getContainer()->get('overblog_graphql.cache_compiler')->getCacheDir();
+        /** @var TypeGenerator $generator */
+        $generator = static::$kernel->getContainer()->get('overblog_graphql.cache_compiler');
+        $this->cacheDir = $generator->options->getCacheDirOrDefault();
         $this->commandTester = new CommandTester($command);
     }
 
@@ -48,7 +49,7 @@ public function testGeneration(): void
         $this->commandTester->execute([]);
         $this->assertSame(0, $this->commandTester->getStatusCode());
         $this->assertSame($this->displayExpected(), $this->commandTester->getDisplay());
-        foreach ($this->typesMapping as $class => $path) {
+        foreach ($this->typesMapping as $path) {
             $this->assertFileExists($path);
         }
     }