Merge branch 'master' into annotations

.travis.yml

@@ -37,7 +37,7 @@ jobs:
           env: SYMFONY_VERSION=4.0.*
         - php: 7.2
           env: SYMFONY_VERSION=4.1.*
-        - php: 7.2
+        - php: 7.3
           env: SYMFONY_VERSION=4.2.*
         - php: nightly
           env: COMPOSER_UPDATE_FLAGS=--ignore-platform-reqs

docs/definitions/expression-language.md

@@ -40,6 +40,42 @@ All definition config entries can use expression language but it must be explici
 
 [For more details on expression syntax](http://symfony.com/doc/current/components/expression_language/syntax.html)
 
+Private services
+----------------
+
+It is not possible to use private services with `service` or `serv` functions since this is equivalent to call
+`get` method on the container. Private services must be tag as global variable to be accessible.
+
+Yaml example:
+
+```yaml
+App\MyPrivateService:
+    public: false
+    tags:
+        - { name: overblog_graphql.global_variable, alias: my_private_service }
+```
+
+To use a vendor private services:
+
+```php
+<?php
+
+$vendorPrivateServiceDef = $container->findDefinition(\Vendor\PrivateService::class);
+$vendorPrivateServiceDef->addTag('overblog_graphql.global_variable', ['alias' => 'vendor_private_service']);
+```
+
+Usage:
+
+```yaml
+MyType:
+    type: object
+    config:
+        fields:
+            name:
+                type: String!
+                resolve: '@=my_private_service.formatName(value)'
+```
+
 Custom expression function
 --------------------------
 

src/Resolver/AccessResolver.php

@@ -30,22 +30,18 @@ public function resolve(callable $accessChecker, callable $resolveCallback, arra
             return $this->checkAccessForStrictMode($accessChecker, $resolveCallback, $resolveArgs);
         }
 
-        $result = \call_user_func_array($resolveCallback, $resolveArgs);
+        $resultOrPromise = \call_user_func_array($resolveCallback, $resolveArgs);
 
-        if ($result instanceof Promise) {
-            $result = $result->adoptedPromise;
-        }
-
-        if ($this->promiseAdapter->isThenable($result) || $result instanceof SyncPromise) {
-            return $this->promiseAdapter->then(
-                new Promise($result, $this->promiseAdapter),
+        if ($this->isThenable($resultOrPromise)) {
+            return $this->createPromise(
+                $resultOrPromise,
                 function ($result) use ($accessChecker, $resolveArgs) {
                     return $this->processFilter($result, $accessChecker, $resolveArgs);
                 }
             );
         }
 
-        return $this->processFilter($result, $accessChecker, $resolveArgs);
+        return $this->processFilter($resultOrPromise, $accessChecker, $resolveArgs);
     }
 
     private static function isMutationRootField(ResolveInfo $info): bool
@@ -55,12 +51,21 @@ private static function isMutationRootField(ResolveInfo $info): bool
 
     private function checkAccessForStrictMode(callable $accessChecker, callable $resolveCallback, array $resolveArgs = [])
     {
-        if (!$this->hasAccess($accessChecker, $resolveArgs)) {
-            $exceptionClassName = self::isMutationRootField($resolveArgs[3]) ? UserError::class : UserWarning::class;
-            throw new $exceptionClassName('Access denied to this field.');
-        }
+        $promiseOrHasAccess = $this->hasAccess($accessChecker, $resolveArgs);
+        $callback = function ($hasAccess) use ($resolveArgs, $resolveCallback) {
+            if (!$hasAccess) {
+                $exceptionClassName = self::isMutationRootField($resolveArgs[3]) ? UserError::class : UserWarning::class;
+                throw new $exceptionClassName('Access denied to this field.');
+            }
+
+            return \call_user_func_array($resolveCallback, $resolveArgs);
+        };
 
-        return \call_user_func_array($resolveCallback, $resolveArgs);
+        if ($this->isThenable($promiseOrHasAccess)) {
+            return $this->createPromise($promiseOrHasAccess, $callback);
+        } else {
+            return $callback($promiseOrHasAccess);
+        }
     }
 
     private function processFilter($result, $accessChecker, $resolveArgs)
@@ -88,11 +93,35 @@ function (Edge $edge) use ($accessChecker, $resolveArgs) {
         return $result;
     }
 
-    private function hasAccess(callable $accessChecker, array $resolveArgs = [], $object = null): bool
+    private function hasAccess(callable $accessChecker, array $resolveArgs = [], $object = null)
     {
         $resolveArgs[] = $object;
-        $access = (bool) \call_user_func_array($accessChecker, $resolveArgs);
+        $accessOrPromise = \call_user_func_array($accessChecker, $resolveArgs);
+
+        return $accessOrPromise;
+    }
+
+    private function isThenable($object): bool
+    {
+        $object = $this->extractAdoptedPromise($object);
+
+        return $this->promiseAdapter->isThenable($object) || $object instanceof SyncPromise;
+    }
 
-        return $access;
+    private function extractAdoptedPromise($object)
+    {
+        if ($object instanceof Promise) {
+            $object = $object->adoptedPromise;
+        }
+
+        return $object;
+    }
+
+    private function createPromise($promise, callable $onFulfilled = null): Promise
+    {
+        return $this->promiseAdapter->then(
+            new Promise($this->extractAdoptedPromise($promise), $this->promiseAdapter),
+            $onFulfilled
+        );
     }
 }

tests/Functional/App/Resolver/ConnectionResolver.php

@@ -82,4 +82,9 @@ public function resolveQuery()
 
         return $this->allUsers[0];
     }
+
+    public function resolvePromiseFullFilled($value)
+    {
+        return $this->promiseAdapter->createFulfilled($value);
+    }
 }

tests/Functional/App/config/access/mapping/access.types.yml

@@ -38,6 +38,10 @@ User:
                 access: "@=hasRole('ROLE_ADMIN')"
                 resolve: ['ROLE_USER']
             isEnabled:
+                # access as a promise
+                access: "@=resolver('promise', [args['hasAccess']])"
+                args:
+                    hasAccess: {type: Boolean!}
                 type: Boolean
                 resolve: true
             friends:

tests/Functional/App/config/connection/services.yml

@@ -8,3 +8,4 @@ services:
             - { name: "overblog_graphql.resolver", alias: "node", method: "resolveNode" }
             - { name: "overblog_graphql.resolver", alias: "query", method: "resolveQuery" }
             - { name: "overblog_graphql.resolver", alias: "connection", method: "resolveConnection" }
+            - { name: "overblog_graphql.resolver", alias: "promise", method: "resolvePromiseFullFilled" }

tests/Functional/Security/AccessTest.php

@@ -17,7 +17,7 @@ class AccessTest extends TestCase
 
     private $userRolesQuery = 'query { user { roles } }';
 
-    private $userIsEnabledQuery = 'query { user { isEnabled } }';
+    private $userIsEnabledQuery = 'query ($hasAccess: Boolean = true) { user { isEnabled(hasAccess: $hasAccess) } }';
 
     private $userFriendsQuery = <<<'EOF'
 query {
@@ -68,6 +68,44 @@ public function testCustomClassLoaderNotRegister(): void
         $this->assertResponse($this->userNameQuery, [], static::ANONYMOUS_USER, 'access');
     }
 
+    public function testNotAuthenticatedUserAccessAsPromisedFulfilledTrue(): void
+    {
+        $this->assertResponse(
+            $this->userIsEnabledQuery,
+            ['data' => ['user' => ['isEnabled' => true]]],
+            static::ANONYMOUS_USER,
+            'access'
+        );
+    }
+
+    public function testNotAuthenticatedUserAccessAsPromisedFulfilledFalse(): void
+    {
+        $this->assertResponse(
+            $this->userIsEnabledQuery,
+            [
+                'data' => [
+                    'user' => [
+                        'isEnabled' => null,
+                    ],
+                ],
+                'extensions' => [
+                    'warnings' => [
+                        [
+                            'message' => 'Access denied to this field.',
+                            'category' => 'user',
+                            'locations' => [['line' => 1, 'column' => 45]],
+                            'path' => ['user', 'isEnabled'],
+                        ],
+                    ],
+                ],
+            ],
+            static::ANONYMOUS_USER,
+            'access',
+            '',
+            ['hasAccess' => false]
+        );
+    }
+
     public function testNotAuthenticatedUserAccessToUserName(): void
     {
         $expected = [