feat: make location parse configurable on version 5.X

nuwave · Jan 17, 2024 · 2e52d8f · 2e52d8f
1 parent d325357
commit 2e52d8f
Show file tree

Hide file tree

Showing 8 changed files with 201 additions and 47 deletions.
diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml
@@ -9,16 +9,16 @@ env:
   REQUIRED_PHP_EXTENSIONS: mbstring, mysqli, pdo_mysql, redis
   LIGHTHOUSE_TEST_DB_USER: root
   LIGHTHOUSE_TEST_DB_PASSWORD: root
-  LIGHTHOUSE_TEST_DB_HOST: localhost
-  LIGHTHOUSE_TEST_DB_UNIX_SOCKET: /var/run/mysqld/mysqld.sock
+  LIGHTHOUSE_TEST_DB_HOST: 127.0.0.1
+  LIGHTHOUSE_TEST_DB_PORT: 33060
   LIGHTHOUSE_TEST_REDIS_HOST: 127.0.0.1
 
 # Using ubuntu-18.04 because it has MySQL 5.7.
 # TODO switch to MySQL 8 https://github.com/nuwave/lighthouse/issues/1784
 
 jobs:
   static-analysis:
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
 
     strategy:
       fail-fast: false
@@ -62,6 +62,8 @@ jobs:
             laravel-version: ^6
           - php-version: 8.1
             laravel-version: ^7
+          - php-version: 8.1
+            laravel-version: ^8
           - php-version: 8.2
             laravel-version: ^6
           - php-version: 8.2
@@ -95,7 +97,17 @@ jobs:
       - run: vendor/bin/phpstan
 
   tests:
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
+
+    services:
+      mysql:
+        image: mysql:5.7
+        env:
+          MYSQL_ROOT_PASSWORD: root
+          MYSQL_DATABASE: test
+        ports:
+          - 33060:3306
+        options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=10
 
     strategy:
       matrix:
@@ -191,14 +203,24 @@ jobs:
 
       - run: composer require illuminate/contracts:${{ matrix.laravel-version }} --no-interaction --prefer-dist --no-progress
 
-      - run: |
-          sudo systemctl start mysql.service
-          mysql --user=root --password=root --execute='CREATE DATABASE test;'
+      - name: Check MySQL version
+        run: mysql --host 127.0.0.1 --port ${{ job.services.mysql.ports['3306'] }} -uroot -proot -e "SELECT @@VERSION"
 
-      - run: vendor/bin/phpunit --colors=always --verbose
+      - name: Execute test
+        run: vendor/bin/phpunit --colors=always --verbose
 
   coverage:
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
+
+    services:
+      mysql:
+        image: mysql:5.7
+        env:
+          MYSQL_ROOT_PASSWORD: root
+          MYSQL_DATABASE: test
+        ports:
+          - 33060:3306
+        options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=10
 
     strategy:
       matrix:
@@ -235,17 +257,13 @@ jobs:
 
       - run: composer require illuminate/contracts:${{ matrix.laravel-version }} --no-interaction --prefer-dist --no-progress
 
-      - run: |
-          sudo systemctl start mysql.service
-          mysql --user=root --password=root --execute='CREATE DATABASE test;'
-
       - run: vendor/bin/phpunit --coverage-clover=coverage.xml
 
       - name: "Upload to Codecov"
         uses: codecov/codecov-action@v2
 
   benchmarks:
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
 
     strategy:
       matrix:

diff --git a/src/GraphQL.php b/src/GraphQL.php
@@ -216,7 +216,7 @@ public function parse(string $query): DocumentNode
         $cacheConfig = $this->configRepository->get('lighthouse.query_cache');
 
         if (! $cacheConfig['enable']) {
-            return Parser::parse($query);
+            return $this->parseQuery($query);
         }
 
         $cacheFactory = Container::getInstance()->make(CacheFactory::class);
@@ -227,8 +227,8 @@ public function parse(string $query): DocumentNode
         return $store->remember(
             'lighthouse:query:' . hash('sha256', $query),
             $cacheConfig['ttl'],
-            static function () use ($query): DocumentNode {
-                return Parser::parse($query);
+            function () use ($query): DocumentNode {
+                return $this->parseQuery($query);
             }
         );
     }
@@ -450,4 +450,11 @@ public function prepSchema(): Schema
     {
         return $this->schemaBuilder->schema();
     }
+
+    protected function parseQuery(string $query): DocumentNode
+    {
+        return Parser::parse($query, [
+            'noLocation' => ! $this->configRepository->get('lighthouse.parse_source_location'),
+        ]);
+    }
 }
diff --git a/src/Schema/Directives/ThrottleDirective.php b/src/Schema/Directives/ThrottleDirective.php
@@ -78,6 +78,7 @@ public function handleField(FieldValue $fieldValue, \Closure $next): FieldValue
             // @phpstan-ignore-next-line won't be executed on Laravel < 8
             $limiter = $this->limiter->limiter($name);
 
+            // @phpstan-ignore-next-line won't be executed on Laravel < 8
             $limiterResponse = $limiter($this->request);
             // @phpstan-ignore-next-line won't be executed on Laravel < 8
             if ($limiterResponse instanceof Unlimited) {

diff --git a/src/Schema/Types/Scalars/DateScalar.php b/src/Schema/Types/Scalars/DateScalar.php
@@ -77,6 +77,7 @@ protected function tryParsingDate($value, string $exceptionClass): IlluminateCar
                     || CarbonImmutable::class === get_class($value)
                 )
             ) {
+                // @phpstan-ignore-next-line We need to type hint $value into one of the two types or static analysis will throw
                 assert($value instanceof CarbonCarbon || $value instanceof CarbonImmutable);
 
                 $carbon = IlluminateCarbon::create(

diff --git a/src/lighthouse.php b/src/lighthouse.php
@@ -28,11 +28,11 @@
          * make sure to return spec-compliant responses in case an error is thrown.
          */
         'middleware' => [
-            \Nuwave\Lighthouse\Support\Http\Middleware\AcceptJson::class,
+            Nuwave\Lighthouse\Support\Http\Middleware\AcceptJson::class,
 
             // Logs in a user if they are authenticated. In contrast to Laravel's 'auth'
             // middleware, this delegates auth and permission checks to the field level.
-            \Nuwave\Lighthouse\Support\Http\Middleware\AttemptAuthentication::class,
+            Nuwave\Lighthouse\Support\Http\Middleware\AttemptAuthentication::class,
 
             // Logs every incoming GraphQL query.
             // \Nuwave\Lighthouse\Support\Http\Middleware\LogGraphQLQueries::class,
@@ -152,12 +152,25 @@
          */
         'ttl' => env(
             'LIGHTHOUSE_QUERY_CACHE_TTL',
-            \Nuwave\Lighthouse\Support\AppVersion::atLeast(5.8)
+            Nuwave\Lighthouse\Support\AppVersion::atLeast(5.8)
                 ? 24 * 60 * 60 // 1 day in seconds
                 : 24 * 60 // 1 day in minutes
         ),
     ],
 
+    /*
+    |--------------------------------------------------------------------------
+    | Parse source location
+    |--------------------------------------------------------------------------
+    |
+    | Should the source location be included in the AST nodes resulting from query parsing?
+    | Setting this to `false` improves performance, but omits the key `locations` from errors,
+    | see https://spec.graphql.org/October2021/#sec-Errors.Error-result-format.
+    |
+    */
+
+    'parse_source_location' => true,
+
     /*
     |--------------------------------------------------------------------------
     | Namespaces
@@ -192,11 +205,11 @@
     */
 
     'security' => [
-        'max_query_complexity' => \GraphQL\Validator\Rules\QueryComplexity::DISABLED,
-        'max_query_depth' => \GraphQL\Validator\Rules\QueryDepth::DISABLED,
+        'max_query_complexity' => GraphQL\Validator\Rules\QueryComplexity::DISABLED,
+        'max_query_depth' => GraphQL\Validator\Rules\QueryDepth::DISABLED,
         'disable_introspection' => (bool) env('LIGHTHOUSE_SECURITY_DISABLE_INTROSPECTION', false)
-            ? \GraphQL\Validator\Rules\DisableIntrospection::ENABLED
-            : \GraphQL\Validator\Rules\DisableIntrospection::DISABLED,
+            ? GraphQL\Validator\Rules\DisableIntrospection::ENABLED
+            : GraphQL\Validator\Rules\DisableIntrospection::DISABLED,
     ],
 
     /*
@@ -251,7 +264,7 @@
     |
     */
 
-    'debug' => env('LIGHTHOUSE_DEBUG', \GraphQL\Error\DebugFlag::INCLUDE_DEBUG_MESSAGE | \GraphQL\Error\DebugFlag::INCLUDE_TRACE),
+    'debug' => env('LIGHTHOUSE_DEBUG', GraphQL\Error\DebugFlag::INCLUDE_DEBUG_MESSAGE | GraphQL\Error\DebugFlag::INCLUDE_TRACE),
 
     /*
     |--------------------------------------------------------------------------
@@ -265,11 +278,11 @@
     */
 
     'error_handlers' => [
-        \Nuwave\Lighthouse\Execution\AuthenticationErrorHandler::class,
-        \Nuwave\Lighthouse\Execution\AuthorizationErrorHandler::class,
-        \Nuwave\Lighthouse\Execution\ValidationErrorHandler::class,
-        \Nuwave\Lighthouse\Execution\ExtensionErrorHandler::class,
-        \Nuwave\Lighthouse\Execution\ReportingErrorHandler::class,
+        Nuwave\Lighthouse\Execution\AuthenticationErrorHandler::class,
+        Nuwave\Lighthouse\Execution\AuthorizationErrorHandler::class,
+        Nuwave\Lighthouse\Execution\ValidationErrorHandler::class,
+        Nuwave\Lighthouse\Execution\ExtensionErrorHandler::class,
+        Nuwave\Lighthouse\Execution\ReportingErrorHandler::class,
     ],
 
     /*
@@ -284,14 +297,14 @@
     */
 
     'field_middleware' => [
-        \Nuwave\Lighthouse\Schema\Directives\TrimDirective::class,
-        \Nuwave\Lighthouse\Schema\Directives\ConvertEmptyStringsToNullDirective::class,
-        \Nuwave\Lighthouse\Schema\Directives\SanitizeDirective::class,
-        \Nuwave\Lighthouse\Validation\ValidateDirective::class,
-        \Nuwave\Lighthouse\Schema\Directives\TransformArgsDirective::class,
-        \Nuwave\Lighthouse\Schema\Directives\SpreadDirective::class,
-        \Nuwave\Lighthouse\Schema\Directives\RenameArgsDirective::class,
-        \Nuwave\Lighthouse\Schema\Directives\DropArgsDirective::class,
+        Nuwave\Lighthouse\Schema\Directives\TrimDirective::class,
+        Nuwave\Lighthouse\Schema\Directives\ConvertEmptyStringsToNullDirective::class,
+        Nuwave\Lighthouse\Schema\Directives\SanitizeDirective::class,
+        Nuwave\Lighthouse\Validation\ValidateDirective::class,
+        Nuwave\Lighthouse\Schema\Directives\TransformArgsDirective::class,
+        Nuwave\Lighthouse\Schema\Directives\SpreadDirective::class,
+        Nuwave\Lighthouse\Schema\Directives\RenameArgsDirective::class,
+        Nuwave\Lighthouse\Schema\Directives\DropArgsDirective::class,
     ],
 
     /*
@@ -453,13 +466,13 @@
             ],
             'pusher' => [
                 'driver' => 'pusher',
-                'routes' => \Nuwave\Lighthouse\Subscriptions\SubscriptionRouter::class . '@pusher',
+                'routes' => Nuwave\Lighthouse\Subscriptions\SubscriptionRouter::class . '@pusher',
                 'connection' => 'pusher',
             ],
             'echo' => [
                 'driver' => 'echo',
                 'connection' => env('LIGHTHOUSE_SUBSCRIPTION_REDIS_CONNECTION', 'default'),
-                'routes' => \Nuwave\Lighthouse\Subscriptions\SubscriptionRouter::class . '@echoRoutes',
+                'routes' => Nuwave\Lighthouse\Subscriptions\SubscriptionRouter::class . '@echoRoutes',
             ],
         ],
 

diff --git a/tests/Integration/ErrorTest.php b/tests/Integration/ErrorTest.php
@@ -4,7 +4,6 @@
 
 use GraphQL\Error\DebugFlag;
 use GraphQL\Error\Error;
-use GraphQL\Error\FormattedError;
 use Illuminate\Contracts\Config\Repository as ConfigRepository;
 use Tests\TestCase;
 
@@ -41,6 +40,81 @@ public function testRejectsInvalidQuery(): void
         );
     }
 
+    public function testReturnsFullGraphQLError(): void
+    {
+        $message = 'some error';
+        $this->mockResolver(static function () use ($message): Error {
+            return new Error($message);
+        });
+
+        $this->schema = /** @lang GraphQL */ '
+        type Query {
+            foo: ID @mock
+        }
+        ';
+
+        $this
+            ->graphQL(/** @lang GraphQL */'
+            {
+                foo
+            }
+            ')
+            ->assertStatus(200)
+            ->assertJson([
+                'data' => [
+                    'foo' => null,
+                ],
+                'errors' => [
+                    [
+                        'message' => $message,
+                        'path' => ['foo'],
+                        'locations' => [
+                            [
+                                'line' => 3,
+                                'column' => 17,
+                            ],
+                        ],
+                    ],
+                ],
+            ]);
+    }
+
+    public function testReturnsGraphQLErrorWithoutLocationNode(): void
+    {
+        $config = $this->app->make(ConfigRepository::class);
+        $config->set('lighthouse.parse_source_location', false);
+
+        $message = 'some error';
+        $this->mockResolver(static function () use ($message): Error {
+            return new Error($message);
+        });
+
+        $this->schema = /** @lang GraphQL */ '
+        type Query {
+            foo: ID @mock
+        }
+        ';
+
+        $this
+            ->graphQL(/** @lang GraphQL */ '
+            {
+                foo
+            }
+            ')
+            ->assertStatus(200)
+            ->assertJson([
+                'data' => [
+                    'foo' => null,
+                ],
+                'errors' => [
+                    [
+                        'message' => $message,
+                        'path' => ['foo'],
+                    ],
+                ],
+            ]);
+    }
+
     public function testIgnoresInvalidJSONVariables(): void
     {
         $this
@@ -237,3 +311,4 @@ public function testAssertGraphQLErrorNonClientSafe(): void
             ->assertGraphQLError($exception);
     }
 }
+