Merge pull request #845 from murtukov/refactoring/decouple_type_generator_props

Refactor `TypeGenerator`

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,
+   array $typeConfigs,
+   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
     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

@@ -27,24 +27,24 @@ public function isOptional()
     }
 
     /**
-     * {@inheritdoc}
-     *
      * @param string $cacheDir
      *
      * @return string[]
      */
     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
+            $cacheBaseDir = $this->typeGenerator->getCacheBaseDir();
+
+            if (null === $this->typeGenerator->getCacheDir()) {
+                $this->typeGenerator->setCacheBaseDir($cacheDir);
             }
+
             $this->typeGenerator->compile(TypeGenerator::MODE_WRITE | TypeGenerator::MODE_OVERRIDE);
 
-            if (null !== $baseCacheDir) {
-                $this->typeGenerator->setBaseCacheDir($baseCacheDir);
+            if (null !== $cacheBaseDir) {
+                $this->typeGenerator->setCacheBaseDir($cacheBaseDir);
             }
         }
 

src/Generator/TypeGenerator.php

@@ -24,74 +24,29 @@ 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;
+    private array $typeConfigs;
+    private TypeGeneratorOptions $options;
     private TypeBuilder $typeBuilder;
-    private EventDispatcherInterface $eventDispatcher;
+    private EventDispatcherInterface $dispatcher;
 
     public function __construct(
-        string $classNamespace,
-        ?string $cacheDir,
-        array $configs,
+        array $typeConfigs,
         TypeBuilder $typeBuilder,
-        EventDispatcherInterface $eventDispatcher,
-        bool $useClassMap = true,
-        ?string $baseCacheDir = null,
-        ?int $cacheDirMask = null
+        EventDispatcherInterface $dispatcher,
+        TypeGeneratorOptions $options
     ) {
-        $this->cacheDir = $cacheDir;
-        $this->configs = $configs;
-        $this->useClassMap = $useClassMap;
-        $this->baseCacheDir = $baseCacheDir;
+        $this->typeConfigs = $typeConfigs;
         $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;
-    }
-
-    public function getBaseCacheDir(): ?string
-    {
-        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->dispatcher = $dispatcher;
+        $this->options = $options;
     }
 
     public function compile(int $mode): array
     {
-        $cacheDir = $this->getCacheDir();
+        $cacheDir = $this->getCacheDirOrDefault();
         $writeMode = $mode & self::MODE_WRITE;
 
         // Configure write mode
@@ -100,20 +55,17 @@ public function compile(int $mode): array
             $fs->remove($cacheDir);
         }
 
-        // Process configs
-        $configs = Processor::process($this->configs);
-
         // Generate classes
         $classes = [];
-        foreach ($configs as $name => $config) {
+        foreach (Processor::process($this->typeConfigs) 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 +77,7 @@ public function compile(int $mode): array
             $this->loadClasses(true);
         }
 
-        $this->eventDispatcher->dispatch(new SchemaCompiledEvent());
+        $this->dispatcher->dispatch(new SchemaCompiledEvent());
 
         return $classes;
     }
@@ -145,12 +97,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 : [];
 
@@ -171,8 +125,33 @@ public function loadClasses(bool $forceReload = false): void
         }
     }
 
+    public function getCacheDir(): ?string
+    {
+        return $this->options->cacheDir;
+    }
+
+    public function getCacheDirMask(): int
+    {
+        return $this->options->cacheDirMask;
+    }
+
+    public function getCacheBaseDir(): ?string
+    {
+        return $this->options->cacheBaseDir;
+    }
+
+    public function setCacheBaseDir(string $dir): void
+    {
+        $this->options->cacheBaseDir = $dir;
+    }
+
+    public function getCacheDirOrDefault(): string
+    {
+        return $this->options->cacheDir ?? $this->options->cacheBaseDir.'/overblog/graphql-bundle/__definitions__';
+    }
+
     private function getClassesMap(): string
     {
-        return $this->getCacheDir().'/__classes.map';
+        return $this->getCacheDirOrDefault().'/__classes.map';
     }
 }

src/Generator/TypeGeneratorOptions.php

@@ -0,0 +1,54 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Overblog\GraphQLBundle\Generator;
+
+class TypeGeneratorOptions
+{
+    /**
+     * PSR-4 namespace for generated GraphQL classes.
+     */
+    public string $namespace;
+
+    /**
+     * Relative path to a directory for generated GraphQL classes.
+     * Equals `null` unless explicitly set by user.
+     */
+    public ?string $cacheDir;
+
+    /**
+     * Permission bitmask for the directory of generated classes.
+     */
+    public int $cacheDirMask;
+
+    /**
+     * Whether a class map should be generated.
+     */
+    public bool $useClassMap = true;
+
+    /**
+     * Base directory for generated classes.
+     */
+    public ?string $cacheBaseDir;
+
+    public function __construct(
+        string $namespace,
+        ?string $cacheDir,
+        bool $useClassMap = true,
+        ?string $cacheBaseDir = null,
+        ?int $cacheDirMask = null
+    ) {
+        $this->namespace = $namespace;
+        $this->cacheDir = $cacheDir;
+        $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;
+    }
+}

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_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.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->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);
         }
     }