Server: disable query batching by default; allow array as server config

src/Server/Helper.php

@@ -28,7 +28,7 @@ class Helper
 {
     /**
      * Executes GraphQL operation with given server configuration and returns execution result
-     * (or promise when promise adapter is different from SyncPromiseAdapter)
+     * (or promise when promise adapter is different than SyncPromiseAdapter)
      *
      * @param ServerConfig $config
      * @param OperationParams $op
@@ -61,7 +61,7 @@ class Helper
         $result = [];
 
         foreach ($operations as $operation) {
-            $result[] = $this->promiseToExecuteOperation($promiseAdapter, $config, $operation);
+            $result[] = $this->promiseToExecuteOperation($promiseAdapter, $config, $operation, true);
         }
 
         $result = $promiseAdapter->all($result);
@@ -77,20 +77,28 @@ class Helper
      * @param PromiseAdapter $promiseAdapter
      * @param ServerConfig $config
      * @param OperationParams $op
+     * @param bool $isBatch
      * @return Promise
      */
-    private function promiseToExecuteOperation(PromiseAdapter $promiseAdapter, ServerConfig $config, OperationParams $op)
+    private function promiseToExecuteOperation(PromiseAdapter $promiseAdapter, ServerConfig $config, OperationParams $op, $isBatch = false)
     {
         try {
+            if ($isBatch && !$config->getQueryBatching()) {
+                throw new RequestError("Batched queries are not supported by this server");
+            }
+
             $errors = $this->validateOperationParams($op);
 
             if (!empty($errors)) {
+                $errors = Utils::map($errors, function(RequestError $err) {
+                    return Error::createLocatedError($err, null, null);
+                });
                 return $promiseAdapter->createFulfilled(
                     new ExecutionResult(null, $errors)
                 );
             }
 
-            $doc = $op->queryId ? static::loadPersistedQuery($config, $op) : $op->query;
+            $doc = $op->queryId ? $this->loadPersistedQuery($config, $op) : $op->query;
 
             if (!$doc instanceof DocumentNode) {
                 $doc = Parser::parse($doc);
@@ -154,8 +162,7 @@ class Helper
      * @param ServerConfig $config
      * @param OperationParams $op
      * @return mixed
-     * @throws Error
+     * @throws RequestError
-     * @throws InvariantViolation
      */
     private function loadPersistedQuery(ServerConfig $config, OperationParams $op)
     {
@@ -251,7 +258,7 @@ class Helper
      *
      * @param callable|null $readRawBodyFn
      * @return OperationParams|OperationParams[]
-     * @throws Error
+     * @throws RequestError
      */
     public function parseHttpRequest(callable $readRawBodyFn = null)
     {
@@ -359,6 +366,10 @@ class Helper
         }
     }
 
+    /**
+     * @param $result
+     * @param $exitWhenDone
+     */
     private function doSendResponse($result, $exitWhenDone)
     {
         $httpStatus = $this->resolveHttpStatus($result);
@@ -382,7 +393,7 @@ class Helper
      * @param array $bodyParams
      * @param array $queryParams
      * @return OperationParams|OperationParams[]
-     * @throws Error
+     * @throws RequestError
      */
     public function parseRequestParams($method, array $bodyParams, array $queryParams)
     {

src/Server/ServerConfig.php

@@ -11,9 +11,17 @@ class ServerConfig
     /**
      * @return static
      */
-    public static function create()
+    public static function create(array $config = [])
     {
-        return new static();
+        $instance = new static();
+        foreach ($config as $key => $value) {
+            $method = 'set' . ucfirst($key);
+            if (!method_exists($instance, $method)) {
+                throw new InvariantViolation("Unknown server config option \"$key\"");
+            }
+            $instance->$method($value);
+        }
+        return $instance;
     }
 
     /**
@@ -41,6 +49,11 @@ class ServerConfig
      */
     private $debug = false;
 
+    /**
+     * @var bool
+     */
+    private $queryBatching = false;
+
     /**
      * @var array|callable
      */
@@ -173,7 +186,7 @@ class ServerConfig
     {
         if (!is_callable($validationRules) && !is_array($validationRules) && $validationRules !== null) {
             throw new InvariantViolation(
-                __METHOD__ . ' expects array of validation rules or callable returning such array, but got: %s' .
+                'Server config expects array of validation rules or callable returning such array, but got ' .
                 Utils::printSafe($validationRules)
             );
         }
@@ -253,4 +266,24 @@ class ServerConfig
         $this->debug = (bool) $set;
         return $this;
     }
+
+    /**
+     * @return bool
+     */
+    public function getQueryBatching()
+    {
+        return $this->queryBatching;
+    }
+
+    /**
+     * Allow batching queries
+     *
+     * @param bool $enableBatching
+     * @return ServerConfig
+     */
+    public function setQueryBatching($enableBatching)
+    {
+        $this->queryBatching = (bool) $enableBatching;
+        return $this;
+    }
 }

src/Server/StandardServer.php

@@ -43,8 +43,15 @@ class StandardServer
      * StandardServer constructor.
      * @param ServerConfig $config
      */
-    protected function __construct(ServerConfig $config)
+    protected function __construct($config)
     {
+        if (is_array($config)) {
+            $config = ServerConfig::create($config);
+        }
+        if (!$config instanceof ServerConfig) {
+            throw new InvariantViolation("");
+        }
+
         $this->config = $config;
         $this->helper = new Helper();
     }

tests/Server/QueryExecutionTest.php

@@ -10,6 +10,7 @@ use GraphQL\Language\Parser;
 use GraphQL\Schema;
 use GraphQL\Server\Helper;
 use GraphQL\Server\OperationParams;
+use GraphQL\Server\RequestError;
 use GraphQL\Server\ServerConfig;
 use GraphQL\Type\Definition\ObjectType;
 use GraphQL\Type\Definition\Type;
@@ -102,7 +103,8 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
             ])
         ]);
 
-        $this->config = ServerConfig::create()->setSchema($schema);
+        $this->config = ServerConfig::create()
+            ->setSchema($schema);
     }
 
     public function testSimpleQueryExecution()
@@ -285,6 +287,42 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($expected, $result->toArray());
     }
 
+    public function testBatchedQueriesAreDisabledByDefault()
+    {
+        $batch = [
+            [
+                'query' => '{invalid}'
+            ],
+            [
+                'query' => '{f1,fieldWithException}'
+            ]
+        ];
+
+        $result = $this->executeBatchedQuery($batch);
+
+        $expected = [
+            [
+                'errors' => [
+                    [
+                        'message' => 'Batched queries are not supported by this server',
+                        'category' => 'request'
+                    ]
+                ]
+            ],
+            [
+                'errors' => [
+                    [
+                        'message' => 'Batched queries are not supported by this server',
+                        'category' => 'request'
+                    ]
+                ]
+            ],
+        ];
+
+        $this->assertEquals($expected[0], $result[0]->toArray());
+        $this->assertEquals($expected[1], $result[1]->toArray());
+    }
+
     public function testMutationsAreNotAllowedInReadonlyMode()
     {
         $mutation = 'mutation { a }';
@@ -416,6 +454,8 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
 
     public function testExecutesBatchedQueries()
     {
+        $this->config->setQueryBatching(true);
+
         $batch = [
             [
                 'query' => '{invalid}'
@@ -479,6 +519,7 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
         $calls = [];
 
         $this->config
+            ->setQueryBatching(true)
             ->setRootValue('1')
             ->setContext([
                 'buffer' => function($num) use (&$calls) {
@@ -525,6 +566,27 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($expected[2], $result[2]->toArray());
     }
 
+    public function testValidatesParamsBeforeExecution()
+    {
+        $op = OperationParams::create(['queryBad' => '{f1}']);
+        $helper = new Helper();
+        $result = $helper->executeOperation($this->config, $op);
+        $this->assertInstanceOf(ExecutionResult::class, $result);
+
+        $this->assertEquals(null, $result->data);
+        $this->assertCount(1, $result->errors);
+
+        $this->assertEquals(
+            'GraphQL Request must include at least one of those two parameters: "query" or "queryId"',
+            $result->errors[0]->getMessage()
+        );
+
+        $this->assertInstanceOf(
+            RequestError::class,
+            $result->errors[0]->getPrevious()
+        );
+    }
+
     private function executePersistedQuery($queryId, $variables = null)
     {
         $op = OperationParams::create(['queryId' => $queryId, 'variables' => $variables]);

tests/Server/ServerConfigTest.php

@@ -1,11 +1,12 @@
 <?php
 namespace GraphQL\Tests\Server;
 
-use GraphQL\Error\FormattedError;
+use GraphQL\Error\InvariantViolation;
 use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter;
-use GraphQL\Schema;
+use GraphQL\Type\Schema;
 use GraphQL\Server\ServerConfig;
 use GraphQL\Type\Definition\ObjectType;
+use GraphQL\Type\Definition\Type;
 
 class ServerConfigTest extends \PHPUnit_Framework_TestCase
 {
@@ -21,6 +22,7 @@ class ServerConfigTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(null, $config->getDefaultFieldResolver());
         $this->assertEquals(null, $config->getPersistentQueryLoader());
         $this->assertEquals(false, $config->getDebug());
+        $this->assertEquals(false, $config->getQueryBatching());
     }
 
     public function testAllowsSettingSchema()
@@ -141,4 +143,62 @@ class ServerConfigTest extends \PHPUnit_Framework_TestCase
         $config->setDebug(false);
         $this->assertSame(false, $config->getDebug());
     }
+
+    public function testAcceptsArray()
+    {
+        $arr = [
+            'schema' => new \GraphQL\Type\Schema([
+                'query' => new ObjectType(['name' => 't', 'fields' => ['a' => Type::string()]])
+            ]),
+            'context' => new \stdClass(),
+            'rootValue' => new \stdClass(),
+            'errorFormatter' => function() {},
+            'promiseAdapter' => new SyncPromiseAdapter(),
+            'validationRules' => [function() {}],
+            'defaultFieldResolver' => function() {},
+            'persistentQueryLoader' => function() {},
+            'debug' => true,
+            'queryBatching' => true,
+        ];
+
+        $config = ServerConfig::create($arr);
+
+        $this->assertSame($arr['schema'], $config->getSchema());
+        $this->assertSame($arr['context'], $config->getContext());
+        $this->assertSame($arr['rootValue'], $config->getRootValue());
+        $this->assertSame($arr['errorFormatter'], $config->getErrorFormatter());
+        $this->assertSame($arr['promiseAdapter'], $config->getPromiseAdapter());
+        $this->assertSame($arr['validationRules'], $config->getValidationRules());
+        $this->assertSame($arr['defaultFieldResolver'], $config->getDefaultFieldResolver());
+        $this->assertSame($arr['persistentQueryLoader'], $config->getPersistentQueryLoader());
+        $this->assertSame(true, $config->getDebug());
+        $this->assertSame(true, $config->getQueryBatching());
+    }
+
+    public function testThrowsOnInvalidArrayKey()
+    {
+        $arr = [
+            'missingKey' => 'value'
+        ];
+
+        $this->setExpectedException(
+            InvariantViolation::class,
+            'Unknown server config option "missingKey"'
+        );
+
+        ServerConfig::create($arr);
+    }
+
+    public function testInvalidValidationRules()
+    {
+        $rules = new \stdClass();
+        $config = ServerConfig::create();
+
+        $this->setExpectedException(
+            InvariantViolation::class,
+            'Server config expects array of validation rules or callable returning such array, but got instance of stdClass'
+        );
+
+        $config->setValidationRules($rules);
+    }
 }