FIX Use static call not self call when rebuilding schema (#624)

silverstripe · Jan 29, 2025 · 78af3f9 · 78af3f9
1 parent 2617d57
commit 78af3f9
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 18 deletions.
diff --git a/src/Schema/Storage/AbstractTypeRegistry.php b/src/Schema/Storage/AbstractTypeRegistry.php
@@ -47,7 +47,7 @@ public static function get(string $typename)
         try {
             return static::fromCache($typename);
         } catch (Exception $e) {
-            if (!preg_match('/(Missing|Unknown) graphql/', $e->getMessage()) || !AbstractTypeRegistry::canRebuildOnMissing()) {
+            if (!preg_match('/(Missing|Unknown) graphql/', $e->getMessage()) || !static::canRebuildOnMissing()) {
                 throw $e;
             }
             // Try to rebuild the whole schema as fallback.
@@ -59,7 +59,7 @@ public static function get(string $typename)
             $schema = $builder->boot($key);
             try {
                 $builder->build($schema, true);
-                $path = AbstractTypeRegistry::getRebuildOnMissingPath();
+                $path = static::getRebuildOnMissingPath();
                 file_put_contents($path, time());
             } catch (EmptySchemaException $e) {
                 // noop

diff --git a/tests/Schema/AbstractTypeRegistryTest.php b/tests/Schema/AbstractTypeRegistryTest.php
@@ -2,15 +2,15 @@
 
 namespace SilverStripe\GraphQL\Tests\Schema;
 
-use GraphQL\Type\Definition\AbstractType;
+use Exception;
 use SilverStripe\Control\Controller;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\GraphQL\Schema\Storage\AbstractTypeRegistry;
 use SilverStripe\GraphQL\Controller as GraphQLController;
 use Symfony\Component\Filesystem\Filesystem;
 use ReflectionObject;
-use stdClass;
 use SilverStripe\Control\Session;
+use SilverStripe\GraphQL\Schema\Schema;
 
 class AbstractTypeRegistryTest extends SapphireTest
 {
@@ -61,20 +61,7 @@ public function testRebuildOnMissing(
         bool $expected
     ): void {
         list($registry, $canRebuildOnMissingMethod, $_, $getRebuildOnMissingFilename) = $this->getInstance();
-        $graphqlController = new GraphQLController('test');
-
-        // autobuild
-        $graphqlController->setAutobuildSchema($autobuild);
-
-        // controller
-        if ($controller) {
-            // Make it so that Controller::curr() returns a GraphQLController
-            $fakeSession = new Session([]);
-            $request = Controller::curr()->getRequest();
-            $request->setSession($fakeSession);
-            $graphqlController->setRequest($request);
-            $graphqlController->pushCurrent();
-        }
+        $this->prepGraphQLController($controller, $autobuild);
 
         // config
         AbstractTypeRegistry::config()->set('rebuild_on_missing_schema_file', $config);
@@ -138,6 +125,68 @@ public function provideRebuildOnMissing(): array
         ];
     }
 
+    /**
+     * This test checks no uncaught exceptions are thrown with a successful rebuild.
+     * This test is required separately from the others, because the other tests explicitly
+     * set some private methods to be accesible via reflection.
+     */
+    public function testGetRebuildOnMissing(): void
+    {
+        $registry = new class extends AbstractTypeRegistry
+        {
+            private static int $getAttempts = 0;
+
+            protected static function getSourceDirectory(): string
+            {
+                return AbstractTypeRegistryTest::SOURCE_DIRECTORY;
+            }
+
+            protected static function getSourceNamespace(): string
+            {
+                return '';
+            }
+
+            protected static function fromCache(string $typename): bool
+            {
+                if (static::$getAttempts === 0) {
+                    static::$getAttempts++;
+                    throw new Exception('Missing graphql file for ' . $typename);
+                }
+                return true;
+            }
+        };
+        $this->prepGraphQLController(true, true);
+        AbstractTypeRegistry::config()->set('rebuild_on_missing_schema_file', true);
+        Schema::config()->merge('schemas', [
+            '.graphql-generated' => [],
+        ]);
+
+        $registry::get('test');
+        // This test passes by not throwing any exceptions.
+        $this->expectNotToPerformAssertions();
+    }
+
+    /**
+     * Prepare a GraphQLController for AbstractTypeRegistry::canRebuildOnMissing() checks.
+     */
+    private function prepGraphQLController(bool $controller, bool $autobuild): void
+    {
+        $graphqlController = new GraphQLController('test');
+
+        // autobuild
+        $graphqlController->setAutobuildSchema($autobuild);
+
+        // controller
+        if ($controller) {
+            // Make it so that Controller::curr() returns a GraphQLController
+            $fakeSession = new Session([]);
+            $request = Controller::curr()->getRequest();
+            $request->setSession($fakeSession);
+            $graphqlController->setRequest($request);
+            $graphqlController->pushCurrent();
+        }
+    }
+
     private function getInstance()
     {
         $registry = new class extends AbstractTypeRegistry

diff --git a/tests/Schema/_AbstractTypeRegistryTest/schema.yml b/tests/Schema/_AbstractTypeRegistryTest/schema.yml
@@ -0,0 +1,12 @@
+types:
+  MyType:
+    fields:
+      field1: String
+      field2: Int
+queries:
+  readMyTypes:
+    type: '[MyType]'
+    resolver: [SilverStripe\GraphQL\Tests\Fake\IntegrationTestResolver, lotsOfMyTypes]
+    plugins:
+      paginate:
+        resolver: [SilverStripe\GraphQL\Tests\Fake\IntegrationTestResolver, testPaginate]