Skip to content

Fixing n+1 relations problem #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 40 commits into from
Sep 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
60cdd3f
ignore PhpStorm IDE files
lorado Aug 5, 2018
9ec1416
improve guessing relations
lorado Aug 5, 2018
4c11459
fix errors
lorado Aug 6, 2018
718de93
define Paginable interface for identifying those transformers
lorado Aug 6, 2018
870de54
ListTransformer implements now Paginable interface
lorado Aug 6, 2018
bffe998
rework, how ListTransformer is identified
lorado Aug 6, 2018
f694792
extend DB structure for tests. Tests are still the same
lorado Aug 8, 2018
1051554
sync tables and model definitions
lorado Aug 8, 2018
53db424
rework how relations are identified (much cleaner and correct way)
lorado Aug 8, 2018
1453da4
add missing author parameter to PostDefinition
lorado Aug 8, 2018
171c290
define unit tests for eager loading relations
lorado Aug 8, 2018
035b470
copy-paste travis config
lorado Aug 8, 2018
13cdf22
delete unused imports
lorado Aug 8, 2018
a9beecc
refactor code, so phpmd don't blame on me (CouplingBetweenObjects)
lorado Aug 8, 2018
02cb7e9
calling function recursive make sense only, if relations are found
lorado Aug 8, 2018
88ceb91
save changes after php-cs-fixer
lorado Aug 8, 2018
9088301
remove unnecessary $app variable
lorado Aug 23, 2018
c9fe50e
define fiels selection depth as constant
lorado Aug 24, 2018
68901b2
move relations guessing to static in GraphQL class (will be reused)
lorado Aug 24, 2018
7703678
guess relations also for custom fields
lorado Aug 24, 2018
8bd4965
extend custom query, add test for eager loading in custom queries
lorado Aug 24, 2018
daf6927
add doc to test-query
lorado Aug 24, 2018
06097b4
remove array_filter(), as I see no reason in using it...
lorado Aug 24, 2018
c3bbc1a
extend readme
lorado Aug 24, 2018
55c3c00
fix code styling
lorado Aug 24, 2018
4af8f80
fetch fields ONLY, if returned type is an object
lorado Sep 3, 2018
83b94ea
add new test for fetching simple string from graphql
lorado Sep 3, 2018
fe7fd5e
add authorize check for fields, default is true. update readme
lorado Sep 3, 2018
31cb88b
add tests for authorize checking (test - unauthorized error gets thrown)
lorado Sep 3, 2018
e2a0261
fix definition of method
lorado Sep 3, 2018
dcc86aa
Merge branch 'master' into bug/26
cmizzi Sep 10, 2018
a92e22c
fix(transformers): fix bad merge
cmizzi Sep 10, 2018
12024dd
Merge branch 'master' into bug/26
lorado Sep 10, 2018
7d085f7
fix returned type in docs
lorado Sep 10, 2018
60928bb
adopt EloquentTransformer to fix n+1 problem (at least on read data)
lorado Sep 10, 2018
e227585
fix for linters
lorado Sep 10, 2018
4b52290
add readme for authorization
lorado Sep 10, 2018
5845e35
Merge remote-tracking branch 'origin/bug/26' into bug/26
lorado Sep 10, 2018
8a8eb19
remove unused reflect
lorado Sep 11, 2018
63f6abd
update readme
lorado Sep 11, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
/vendor
/.php_cs.cache
*.orig
/.idea
41 changes: 21 additions & 20 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -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
57 changes: 55 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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();
}
}
Expand All @@ -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.
Expand All @@ -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}
*
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions database/factories/Comment.php
Original file line number Diff line number Diff line change
@@ -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)
];
});
9 changes: 9 additions & 0 deletions database/factories/Country.php
Original file line number Diff line number Diff line change
@@ -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
];
});
9 changes: 9 additions & 0 deletions database/factories/Label.php
Original file line number Diff line number Diff line change
@@ -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
];
});
10 changes: 10 additions & 0 deletions database/factories/Phone.php
Original file line number Diff line number Diff line change
@@ -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
];
});
33 changes: 33 additions & 0 deletions database/migrations/2018_08_07_021200_create_phones_table.php
Original file line number Diff line number Diff line change
@@ -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');
}
}
Original file line number Diff line number Diff line change
@@ -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');
}
}
35 changes: 35 additions & 0 deletions database/migrations/2018_08_08_144100_create_comments_table.php
Original file line number Diff line number Diff line change
@@ -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');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;

/**
* CreateLabelsAndLabelablesTables
*
* @see Migration
*/
class CreateLabelsAndLabelablesTables extends Migration {
/**
* Run the migrations.
*
* @return void
*/
public function up() {
Schema::create('labels', function (Blueprint $table) {
$table->increments('id');
$table->text('name');
});
Schema::create('labelables', function (Blueprint $table) {
$table->unsignedInteger('label_id');
$table->unsignedInteger('labelable_id');
$table->string('labelable_type');
});
}

/**
* Reverse the migrations.
*
* @return void
*/
public function down() {
Schema::drop('labelables');
Schema::drop('labels');
}
}
Loading