Merge pull request #27 from lorado/bug/26

Fixing n+1 relations problem

Author: cmizzi

.gitignore

@@ -2,3 +2,4 @@
 /vendor
 /.php_cs.cache
 *.orig
+/.idea

.travis.yml

@@ -1,29 +1,30 @@
-language : php
-php : [7.1, 7.2]
-cache : { directories : [$COMPOSER_CACHE_DIR, $HOME/.composer/cache, vendor] }
-install : composer update --no-interaction --prefer-dist
+language      : php
+php           : [7.1, 7.2]
+cache         : { directories : [$COMPOSER_CACHE_DIR, $HOME/.composer/cache, vendor] }
+install       : composer update --no-interaction --prefer-dist
 notifications :
-    email : false
+  email : false
 
 stages :
-    - test
-    - lint
+  - test
+  - lint
 
 script :
-    - vendor/bin/phpunit
+  - vendor/bin/phpunit
 
 before_install :
-    - composer global require hirak/prestissimo --update-no-dev
+  - composer global require hirak/prestissimo --update-no-dev
 
 jobs :
-    include :
-        - stage : lint
-          php : 7.2
-          before_install :
-            - composer global require hirak/prestissimo --update-no-dev
-            - composer require phpmd/phpmd --no-update --prefer-dist
-            - composer require phpstan/phpstan --no-update --prefer-dist
-          script :
-              - vendor/bin/phpmd src text phpmd.xml
-              - vendor/bin/phpmd tests text phpmd.xml
-              - vendor/bin/phpstan analyse --autoload-file=_ide_helper.php --level 1 src
+  include :
+  - stage : lint
+    php   : 7.2
+    env   : TESTBENCH_VERSION=3.6.* LARAVEL_VERSION=5.6.*
+    before_install :
+    - composer global require hirak/prestissimo --update-no-dev
+    - composer require phpmd/phpmd --no-update --prefer-dist
+    - composer require phpstan/phpstan --no-update --prefer-dist
+    script :
+    - vendor/bin/phpmd src text phpmd.xml
+    - vendor/bin/phpmd tests text phpmd.xml
+    - vendor/bin/phpstan analyse --autoload-file=_ide_helper.php --level 1 src

README.md

@@ -33,8 +33,10 @@ php artisan vendor:publish --provider="StudioNet\GraphQL\ServiceProvider"
 - [Definition](#definition)
 - [Query](#query)
 - [Mutation](#mutation)
+- [Require authorization](#require-authorization)
 - [Self documentation](#self-documentation)
 - [Examples](#examples)
+- [N+1 Problem](#n+1-problem)
 
 ### Definition
 
@@ -50,6 +52,7 @@ use StudioNet\GraphQL\Definition\Type;
 use StudioNet\GraphQL\Support\Definition\EloquentDefinition;
 use StudioNet\GraphQL\Filter\EqualsOrContainsFilter;
 use App\User;
+use Auth;
 
 /**
  * Specify user GraphQL definition
@@ -184,21 +187,38 @@ namespace App\GraphQL\Query;
 
 use StudioNet\GraphQL\Support\Definition\Query;
 use Illuminate\Support\Facades\Auth;
+use App\User;
+use Auth;
 
 class Viewer extends Query {
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function authorize(array $args) {
+		// check, that user is not a guest
+		return !Auth::guest();
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */
 	public function getRelatedType() {
 		return \GraphQL::type('user');
 	}
+	
+	/**
+	 * {@inheritdoc}
+	 */
+	public function getSource() {
+		return User::class;
+	}
 
 	/**
 	 * Return logged user
 	 *
-	 * @return \App\User|null
+	 * @return User|null
 	 */
-	public function getResolver() {
+	public function getResolver($opts) {
 		return Auth::user();
 	}
 }
@@ -222,6 +242,15 @@ return [
 ];
 ```
 
+`getResolver()` receives an array-argument with followed item:
+
+- `root` 1st argument given by webonyx library - `GraphQL\Executor\Executor::resolveOrError()`
+- `args` 2nd argument given by webonyx library
+- `context` 3rd argument given by webonyx library
+- `info` 4th argument given by webonyx library
+- `fields` array of fields, that were fetched from query. Limited by depth in `StudioNet\GraphQL\GraphQL::FIELD_SELECTION_DEPTH`
+- `with` array of relations, that could/should be eager loaded. **NOTICE:** Finding this relations happens ONLY, if `getSource()` is defined - this method should return a class name of a associated root-type in query. If `getSource()` is not defined, then `with` will be always empty.
+
 ### Mutation
 
 Mutation are used to update or create data.
@@ -236,6 +265,14 @@ use StudioNet\GraphQL\Definition\Type;
 use App\User;
 
 class Profile extends Mutation {
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function authorize(array $args) {
+		// check, that user is not a guest
+		return !Auth::guest();
+	}
+
 	/**
 	 * {@inheritDoc}
 	 *
@@ -294,6 +331,14 @@ return [
 ];
 ```
 
+### Require authorization
+
+Currently you have a possibility to protect your own queries and mutations. You have to implement `authorize()` method in your query/mutation, that return a boolean, that indicates, if requested query/mutation has to be executed. If method return `false`, an `UNAUTHORIZED` GraphQL-Error will be thrown.
+
+Usage examples are in query and mutation above.
+
+Protection of definition transformers are currently not implemented, but may be will in the future. By now you have to define your query/mutation yourself, and protect it then with logic in `authorize()`.
+
 ### Self documentation
 
 A documentation generator is implemented with the package. By default, you can access it by navigate to `/doc/graphql`. You can change this behavior within the configuration file. The built-in documentation is implemented from [this repository](https://github.com/mhallin/graphql-docs).
@@ -560,6 +605,14 @@ post-save (which can be useful for eloquent relational models) :
 	}
 ```
 
+### N+1 Problem
+
+The common question is, if graphql library solves n+1 problem. This occures, when graphql resolves relation. Often entities are fetched without relations, and when graphql query needs to fetch relation, for each fetched entity relation would be fetched from SQL separately. So instead of executing 2 SQL queries, you will get N+1 queries, where N is the count of results of root entity. In that example you would query only one relation. If you query more relations, then it becomes N^2+1 problem.
+
+To solve it, Eloquent has already options to eager load relations. Transformers in this library use eager loading, depends on what you query.
+
+Currently this smart detection works perfect only on View and List Transformers. Other transformers will be reworked soon.
+
 ## Contribution
 
 If you want participate to the project, thank you ! In order to work properly,

database/factories/Comment.php

@@ -0,0 +1,9 @@
+<?php
+use Faker\Generator;
+use StudioNet\GraphQL\Tests\Entity;
+
+$factory->define(Entity\Comment::class, function (Generator $faker) {
+	return [
+		'body' => $faker->text(100)
+	];
+});

database/factories/Country.php

@@ -0,0 +1,9 @@
+<?php
+use Faker\Generator;
+use StudioNet\GraphQL\Tests\Entity;
+
+$factory->define(Entity\Country::class, function (Generator $faker) {
+	return [
+		'name' => $faker->country
+	];
+});

database/factories/Label.php

@@ -0,0 +1,9 @@
+<?php
+use Faker\Generator;
+use StudioNet\GraphQL\Tests\Entity;
+
+$factory->define(Entity\Label::class, function (Generator $faker) {
+	return [
+		'name' => $faker->word
+	];
+});

database/factories/Phone.php

@@ -0,0 +1,10 @@
+<?php
+use Faker\Generator;
+use StudioNet\GraphQL\Tests\Entity;
+
+$factory->define(Entity\Phone::class, function (Generator $faker) {
+	return [
+		'label' => $faker->word,
+		'number' => $faker->phoneNumber
+	];
+});

database/migrations/2018_08_07_021200_create_phones_table.php

@@ -0,0 +1,33 @@
+<?php
+use Illuminate\Database\Migrations\Migration;
+
+/**
+ * CreatePhonesTable
+ *
+ * @see Migration
+ */
+class CreatePhonesTable extends Migration {
+	/**
+	 * Run the migrations.
+	 *
+	 * @return void
+	 */
+	public function up() {
+		Schema::create('phones', function ($table) {
+			$table->increments('id');
+
+			$table->string('label');
+			$table->string('number');
+			$table->unsignedInteger('user_id');
+		});
+	}
+
+	/**
+	 * Reverse the migrations.
+	 *
+	 * @return void
+	 */
+	public function down() {
+		Schema::drop('phones');
+	}
+}

database/migrations/2018_08_07_022900_create_countries_table_and_extend_users_table.php

@@ -0,0 +1,39 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+
+/**
+ * CreateCountriesTable
+ *
+ * @see Migration
+ */
+class CreateCountriesTableAndExtendUsersTable extends Migration {
+	/**
+	 * Run the migrations.
+	 *
+	 * @return void
+	 */
+	public function up() {
+		Schema::create('countries', function (Blueprint $table) {
+			$table->increments('id');
+			$table->string('name');
+		});
+
+		Schema::table('users', function (Blueprint $table) {
+			$table->unsignedInteger('country_id')->nullable();
+		});
+	}
+
+	/**
+	 * Reverse the migrations.
+	 *
+	 * @return void
+	 */
+	public function down() {
+		Schema::table('users', function (Blueprint $table) {
+			$table->dropColumn('country_id');
+		});
+		Schema::drop('phones');
+	}
+}

database/migrations/2018_08_08_144100_create_comments_table.php

@@ -0,0 +1,35 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+
+/**
+ * CreateCommentsTable
+ *
+ * @see Migration
+ */
+class CreateCommentsTable extends Migration {
+	/**
+	 * Run the migrations.
+	 *
+	 * @return void
+	 */
+	public function up() {
+		Schema::create('comments', function (Blueprint $table) {
+			$table->increments('id');
+			$table->text('body');
+			$table->unsignedInteger('commentable_id');
+			$table->string('commentable_type');
+			$table->timestamps();
+		});
+	}
+
+	/**
+	 * Reverse the migrations.
+	 *
+	 * @return void
+	 */
+	public function down() {
+		Schema::drop('comments');
+	}
+}