Skip to content
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

Throw partial errors when failing to delete, forceDelete or restore a model #1420

Merged
merged 19 commits into from
Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,17 @@ You can find and compare releases at the [GitHub release page](https://github.co

- Publish config file with tag `lighthouse-config` and default schema with tag `lighthouse-schema`
instead of the previously used tags `config` and `schema` https://github.com/nuwave/lighthouse/issues/1489
- Throw partial errors when failing to delete, forceDelete or restore a model https://github.com/nuwave/lighthouse/pull/1420
- Add `\Nuwave\Lighthouse\Execution\ErrorPool` to allow collection of partial errors https://github.com/nuwave/lighthouse/pull/1420

### Fixed

- Ensure the `@count` directive works properly with polymorphic relations https://github.com/nuwave/lighthouse/pull/1466

### Deprecated

- Deprecate `\Nuwave\Lighthouse\Execution\ErrorBuffer` in favor of `\Nuwave\Lighthouse\Execution\ErrorPool` https://github.com/nuwave/lighthouse/pull/1420

## 4.15.0

### Added
Expand Down
16 changes: 16 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,19 @@ If you need to revert to the old behavior of using `fill()`, you can change your
- 'force_fill' => true,
+ 'force_fill' => false,
```

### Replace `ErrorBuffer` with `ErrorPool`

Collecting partial errors is now done through the singleton `\Nuwave\Lighthouse\Execution\ErrorPool`
instead of `\Nuwave\Lighthouse\Execution\ErrorBuffer`:

```php
try {
// Something that might fail but still allows for a partial result
} catch (\Throwable $error) {
$errorPool = app(\Nuwave\Lighthouse\Execution\ErrorPool::class);
$errorPool->record($error);
}

return $result;
```
17 changes: 15 additions & 2 deletions docs/master/digging-deeper/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,21 @@ class ExtensionErrorHandler implements ErrorHandler
}
```

## Collecting Errors
## Partial Errors

As a GraphQL query may return a partial result, you may not always want to abort
execution immediately after an error occurred. You can use the [`\Nuwave\Lighthouse\Execution\ErrorBuffer`](https://github.com/nuwave/lighthouse/blob/master/src/Execution/ErrorBuffer.php)
execution immediately after an error occurred.

Use the [`ErrorPool`](https://github.com/nuwave/lighthouse/blob/master/src/Execution/ErrorPool.php)
when you want to collect multiple errors before returning a result.

```php
try {
// Something that might fail but still allows for a partial result
} catch (\Throwable $error) {
$errorPool = app(\Nuwave\Lighthouse\Execution\ErrorPool::class);
$errorPool->record($error);
}

return $result;
```
26 changes: 13 additions & 13 deletions docs/master/security/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ to `sanctum` and register Sanctum's `EnsureFrontendRequestsAreStateful` as first
// ... other middleware

\Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful::class,
]
],
],
'guard' => 'sanctum',
```
Expand Down Expand Up @@ -131,24 +131,24 @@ Add the following middleware to `config/lighthouse.php`:
\Illuminate\Cookie\Middleware\EncryptCookies::class,
\Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
\Illuminate\Session\Middleware\StartSession::class,

// Or this one when using Laravel Sanctum:
\Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful::class
\Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful::class,

// ... other middleware
]
],
],
```

The `login` and `logout` might be defined and implement like this:

```graphql
type Mutation {
"Log in to a new session and get the user."
login(email: String!, password: String!): User!
"Log in to a new session and get the user."
login(email: String!, password: String!): User!

"Log out from the current session, showing the user one last time."
logout: User @guard
"Log out from the current session, showing the user one last time."
logout: User @guard
}
```

Expand All @@ -161,8 +161,8 @@ class Login
*/
public function __invoke($_, array $args): User
{
// Plain Laravel: Auth::guard()
// Laravel Sanctum: Auth::guard(config('sanctum.guard', 'web'))
// Plain Laravel: Auth::guard()
// Laravel Sanctum: Auth::guard(config('sanctum.guard', 'web'))
$guard = ?;

if( ! $guard->attempt($args)) {
Expand Down Expand Up @@ -190,8 +190,8 @@ class Logout
*/
public function __invoke($_, array $args): ?User
{
// Plain Laravel: Auth::guard()
// Laravel Sanctum: Auth::guard(config('sanctum.guard', 'web'))
// Plain Laravel: Auth::guard()
// Laravel Sanctum: Auth::guard(config('sanctum.guard', 'web'))
$guard = ?;

/** @var \App\Models\User|null $user */
Expand Down
2 changes: 1 addition & 1 deletion docs/master/security/validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ You can customize the messages for the given rules by implementing the `messages
## Customize Query Validation Rules

By default, Lighthouse enables all default query validation rules from `webonyx/graphql-php`.
This covers fundamental checks, e.g. queried fields match the schema, variables have values of the correct type.
This covers fundamental checks, e.g. queried fields match the schema, variables have values of the correct type.

If you want to add custom rules or change which ones are used, you can bind a custom implementation
of the interface `\Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules` through a service provider.
Expand Down
4 changes: 4 additions & 0 deletions src/Execution/ErrorBuffer.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
use Closure;
use Nuwave\Lighthouse\Exceptions\GenericException;

/**
* @deprecated in favor of
* @see \Nuwave\Lighthouse\Execution\ErrorPool
*/
class ErrorBuffer
{
/**
Expand Down
53 changes: 53 additions & 0 deletions src/Execution/ErrorPool.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

namespace Nuwave\Lighthouse\Execution;

use GraphQL\Error\Error;
use Throwable;

class ErrorPool
{
/**
* The buffered errors.
*
* @var array<\Throwable>
*/
protected $throwables = [];

/**
* Stores an error that will be added to the result.
*/
public function record(Throwable $throwable): void
{
$this->throwables [] = $throwable;
}

/**
* @return array<\GraphQL\Error\Error>
*/
public function errors(): array
{
return array_map(
function (Throwable $throwable): Error {
if ($throwable instanceof Error) {
return $throwable;
}

return new Error(
$throwable->getMessage(),
null,
null,
null,
null,
$throwable
);
},
$this->throwables
);
}

public function clear(): void
{
$this->throwables = [];
}
}
15 changes: 14 additions & 1 deletion src/GraphQL.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Nuwave\Lighthouse\Events\ManipulateResult;
use Nuwave\Lighthouse\Events\StartExecution;
use Nuwave\Lighthouse\Execution\DataLoader\BatchLoader;
use Nuwave\Lighthouse\Execution\ErrorPool;
use Nuwave\Lighthouse\Execution\GraphQLRequest;
use Nuwave\Lighthouse\Schema\AST\ASTBuilder;
use Nuwave\Lighthouse\Schema\AST\DocumentAST;
Expand Down Expand Up @@ -53,7 +54,12 @@ class GraphQL
protected $createsContext;

/**
* @var ProvidesValidationRules
* @var \Nuwave\Lighthouse\Execution\ErrorPool
*/
protected $errorPool;

/**
* @var \Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules
*/
protected $providesValidationRules;

Expand All @@ -63,13 +69,15 @@ public function __construct(
EventDispatcher $eventDispatcher,
ASTBuilder $astBuilder,
CreatesContext $createsContext,
ErrorPool $errorPool,
ProvidesValidationRules $providesValidationRules
) {
$this->schemaBuilder = $schemaBuilder;
$this->pipeline = $pipeline;
$this->eventDispatcher = $eventDispatcher;
$this->astBuilder = $astBuilder;
$this->createsContext = $createsContext;
$this->errorPool = $errorPool;
$this->providesValidationRules = $providesValidationRules;
}

Expand Down Expand Up @@ -159,6 +167,10 @@ public function executeQuery(
}
}

foreach ($this->errorPool->errors() as $error) {
$result->errors [] = $error;
}

$result->setErrorsHandler(
function (array $errors, callable $formatter): array {
// User defined error handlers, implementing \Nuwave\Lighthouse\Execution\ErrorHandler
Expand Down Expand Up @@ -209,6 +221,7 @@ public function prepSchema(): Schema
protected function cleanUp(): void
{
BatchLoader::forgetInstances();
$this->errorPool->clear();
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/LighthouseServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Nuwave\Lighthouse\Console\UnionCommand;
use Nuwave\Lighthouse\Console\ValidateSchemaCommand;
use Nuwave\Lighthouse\Execution\ContextFactory;
use Nuwave\Lighthouse\Execution\ErrorPool;
use Nuwave\Lighthouse\Execution\GraphQLRequest;
use Nuwave\Lighthouse\Execution\GraphQLValidator;
use Nuwave\Lighthouse\Execution\LighthouseRequest;
Expand Down Expand Up @@ -111,6 +112,7 @@ public function register(): void
$this->app->singleton(DirectiveFactory::class);
$this->app->singleton(NodeRegistry::class);
$this->app->singleton(TypeRegistry::class);
$this->app->singleton(ErrorPool::class);
$this->app->singleton(CreatesContext::class, ContextFactory::class);
$this->app->singleton(CanStreamResponse::class, ResponseStream::class);

Expand Down
4 changes: 2 additions & 2 deletions src/Schema/Directives/DeleteDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ protected function find(string $modelClass, $idOrIds)
return $modelClass::find($idOrIds);
}

protected function modifyExistence(Model $model): void
protected function modifyExistence(Model $model): bool
{
$model->delete();
return (bool) $model->delete();
}

/**
Expand Down
38 changes: 29 additions & 9 deletions src/Schema/Directives/ModifyModelExistenceDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

namespace Nuwave\Lighthouse\Schema\Directives;

use GraphQL\Error\Error;
use GraphQL\Language\AST\FieldDefinitionNode;
use GraphQL\Language\AST\ListTypeNode;
use GraphQL\Language\AST\NonNullTypeNode;
use GraphQL\Language\AST\ObjectTypeDefinitionNode;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Database\Eloquent\Model;
use Nuwave\Lighthouse\Exceptions\DefinitionException;
use Nuwave\Lighthouse\Execution\ErrorPool;
use Nuwave\Lighthouse\Schema\AST\DocumentAST;
use Nuwave\Lighthouse\Schema\Values\FieldValue;
use Nuwave\Lighthouse\Support\Contracts\DefinedDirective;
Expand All @@ -19,20 +21,26 @@
abstract class ModifyModelExistenceDirective extends BaseDirective implements FieldResolver, FieldManipulator, DefinedDirective
{
/**
* The GlobalId resolver.
*
* @var \Nuwave\Lighthouse\Support\Contracts\GlobalId
*/
protected $globalId;

public function __construct(GlobalId $globalId)
/**
* @var \Nuwave\Lighthouse\Execution\ErrorPool
*/
protected $errorPool;

public function __construct(GlobalId $globalId, ErrorPool $errorPool)
{
$this->globalId = $globalId;
$this->errorPool = $errorPool;
}

public static function couldNotModify(Model $user): string
{
return 'Could not modify model '.get_class($user).' with ID '.$user->getKey().'.';
}

/**
* Resolve the field directive.
*/
public function resolveField(FieldValue $fieldValue): FieldValue
{
return $fieldValue->setResolver(
Expand All @@ -54,11 +62,21 @@ function ($root, array $args) {
return;
}

$modifyModelExistence = function (Model $model): void {
if (! $this->modifyExistence($model)) {
$this->errorPool->record(
new Error(
self::couldNotModify($model)
)
);
}
};

if ($modelOrModels instanceof Model) {
$this->modifyExistence($modelOrModels);
$modifyModelExistence($modelOrModels);
} elseif ($modelOrModels instanceof Collection) {
foreach ($modelOrModels as $model) {
$this->modifyExistence($model);
$modifyModelExistence($model);
}
}

Expand Down Expand Up @@ -137,6 +155,8 @@ abstract protected function find(string $modelClass, $idOrIds);

/**
* Bring a model in or out of existence.
*
* The return value indicates if the operation was successful.
*/
abstract protected function modifyExistence(Model $model): void;
abstract protected function modifyExistence(Model $model): bool;
}
4 changes: 2 additions & 2 deletions src/SoftDeletes/ForceDeleteDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ protected function find(string $modelClass, $idOrIds)
return $modelClass::withTrashed()->find($idOrIds);
}

protected function modifyExistence(Model $model): void
protected function modifyExistence(Model $model): bool
{
/** @var \Illuminate\Database\Eloquent\Model&\Illuminate\Database\Eloquent\SoftDeletes $model */
$model->forceDelete();
return (bool) $model->forceDelete();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/SoftDeletes/RestoreDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ protected function find(string $modelClass, $idOrIds)
return $modelClass::withTrashed()->find($idOrIds);
}

protected function modifyExistence(Model $model): void
protected function modifyExistence(Model $model): bool
{
/** @var \Illuminate\Database\Eloquent\Model&\Illuminate\Database\Eloquent\SoftDeletes $model */
$model->restore();
return (bool) $model->restore();
}

/**
Expand Down
Loading