API Client: Use new error handling mechanism

This commit is contained in:
Franz Liedke
2019-08-09 23:57:33 +02:00
committed by Daniël Klabbers
parent 6cf3c1088d
commit ddfb2c1ec1
10 changed files with 33 additions and 68 deletions

View File

@ -10,11 +10,14 @@
namespace Flarum\Api; namespace Flarum\Api;
use Exception; use Exception;
use Flarum\Foundation\ErrorHandling\JsonApiRenderer;
use Flarum\Foundation\ErrorHandling\Registry;
use Flarum\User\User; use Flarum\User\User;
use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Container\Container;
use InvalidArgumentException; use InvalidArgumentException;
use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ResponseInterface;
use Psr\Http\Server\RequestHandlerInterface; use Psr\Http\Server\RequestHandlerInterface;
use Throwable;
use Zend\Diactoros\ServerRequestFactory; use Zend\Diactoros\ServerRequestFactory;
class Client class Client
@ -25,18 +28,18 @@ class Client
protected $container; protected $container;
/** /**
* @var ErrorHandler * @var Registry
*/ */
protected $errorHandler; protected $registry;
/** /**
* @param Container $container * @param Container $container
* @param ErrorHandler $errorHandler * @param Registry $registry
*/ */
public function __construct(Container $container, ErrorHandler $errorHandler = null) public function __construct(Container $container, Registry $registry)
{ {
$this->container = $container; $this->container = $container;
$this->errorHandler = $errorHandler; $this->registry = $registry;
} }
/** /**
@ -67,23 +70,14 @@ class Client
try { try {
return $controller->handle($request); return $controller->handle($request);
} catch (Exception $e) { } catch (Throwable $e) {
if (! $this->errorHandler) { $error = $this->registry->handle($e);
if ($error->shouldBeReported()) {
throw $e; throw $e;
} }
return $this->errorHandler->handle($e); return (new JsonApiRenderer)->format($error, $request);
} }
} }
/**
* @param ErrorHandler $errorHandler
* @return Client
*/
public function setErrorHandler(?ErrorHandler $errorHandler): self
{
$this->errorHandler = $errorHandler;
return $this;
}
} }

View File

@ -68,8 +68,6 @@ class Frontend
{ {
$actor = $request->getAttribute('actor'); $actor = $request->getAttribute('actor');
$this->api->setErrorHandler(null);
return $this->getResponseBody( return $this->getResponseBody(
$this->api->send(ShowForumController::class, $actor) $this->api->send(ShowForumController::class, $actor)
); );

View File

@ -15,7 +15,6 @@ use Flarum\Api\Client;
use Flarum\Api\Controller\CreateGroupController; use Flarum\Api\Controller\CreateGroupController;
use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\RetrievesAuthorizedUsers;
use Flarum\Tests\integration\TestCase; use Flarum\Tests\integration\TestCase;
use Flarum\User\Exception\PermissionDeniedException;
use Flarum\User\Guest; use Flarum\User\Guest;
use Flarum\User\User; use Flarum\User\User;
use Illuminate\Support\Str; use Illuminate\Support\Str;
@ -61,11 +60,10 @@ class AuthenticateWithApiKeyTest extends TestCase
{ {
/** @var Client $api */ /** @var Client $api */
$api = $this->app()->getContainer()->make(Client::class); $api = $this->app()->getContainer()->make(Client::class);
$api->setErrorHandler(null);
$this->expectException(PermissionDeniedException::class); $response = $api->send(CreateGroupController::class, new Guest);
$api->send(CreateGroupController::class, new Guest); $this->assertEquals(403, $response->getStatusCode());
} }
/** /**

View File

@ -51,8 +51,6 @@ abstract class ApiControllerTestCase extends TestCase
/** @var Client $api */ /** @var Client $api */
$api = $this->app()->getContainer()->make(Client::class); $api = $this->app()->getContainer()->make(Client::class);
$api->setErrorHandler(null);
return $api->send($controller, $actor ?? new Guest, $queryParams, $body); return $api->send($controller, $actor ?? new Guest, $queryParams, $body);
} }
} }

View File

@ -13,7 +13,6 @@ use Flarum\Api\Controller\CreateDiscussionController;
use Flarum\Discussion\Discussion; use Flarum\Discussion\Discussion;
use Flarum\User\User; use Flarum\User\User;
use Illuminate\Support\Arr; use Illuminate\Support\Arr;
use Illuminate\Validation\ValidationException;
class CreateDiscussionControllerTest extends ApiControllerTestCase class CreateDiscussionControllerTest extends ApiControllerTestCase
{ {
@ -70,11 +69,9 @@ class CreateDiscussionControllerTest extends ApiControllerTestCase
$this->actor = User::find(1); $this->actor = User::find(1);
$data = Arr::except($this->data, 'content'); $data = Arr::except($this->data, 'content');
$response = $this->callWith($data);
$this->expectException(ValidationException::class); $this->assertEquals(422, $response->getStatusCode());
$this->expectExceptionMessage('The given data was invalid.');
$this->callWith($data);
} }
/** /**
@ -85,10 +82,8 @@ class CreateDiscussionControllerTest extends ApiControllerTestCase
$this->actor = User::find(1); $this->actor = User::find(1);
$data = Arr::except($this->data, 'title'); $data = Arr::except($this->data, 'title');
$response = $this->callWith($data);
$this->expectException(ValidationException::class); $this->assertEquals(422, $response->getStatusCode());
$this->expectExceptionMessage('The given data was invalid.');
$this->callWith($data);
} }
} }

View File

@ -11,11 +11,9 @@ namespace Flarum\Tests\integration\api\Controller;
use Flarum\Api\Controller\CreateGroupController; use Flarum\Api\Controller\CreateGroupController;
use Flarum\Group\Group; use Flarum\Group\Group;
use Flarum\User\Exception\PermissionDeniedException;
use Flarum\User\User; use Flarum\User\User;
use Illuminate\Support\Arr; use Illuminate\Support\Arr;
use Illuminate\Support\Str; use Illuminate\Support\Str;
use Illuminate\Validation\ValidationException;
class CreateGroupControllerTest extends ApiControllerTestCase class CreateGroupControllerTest extends ApiControllerTestCase
{ {
@ -53,10 +51,7 @@ class CreateGroupControllerTest extends ApiControllerTestCase
{ {
$this->actor = User::find(1); $this->actor = User::find(1);
$this->expectException(ValidationException::class); $this->assertEquals(422, $this->callWith()->getStatusCode());
$this->expectExceptionMessage('The given data was invalid.');
$this->callWith();
} }
/** /**
@ -87,8 +82,6 @@ class CreateGroupControllerTest extends ApiControllerTestCase
{ {
$this->actor = User::find(2); $this->actor = User::find(2);
$this->expectException(PermissionDeniedException::class); $this->assertEquals(403, $this->callWith($this->data)->getStatusCode());
$this->callWith($this->data);
} }
} }

View File

@ -11,10 +11,8 @@ namespace Flarum\Tests\integration\api\Controller;
use Flarum\Api\Controller\CreateUserController; use Flarum\Api\Controller\CreateUserController;
use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Settings\SettingsRepositoryInterface;
use Flarum\User\Exception\PermissionDeniedException;
use Flarum\User\User; use Flarum\User\User;
use Illuminate\Support\Arr; use Illuminate\Support\Arr;
use Illuminate\Validation\ValidationException;
class CreateUserControllerTest extends ApiControllerTestCase class CreateUserControllerTest extends ApiControllerTestCase
{ {
@ -51,10 +49,9 @@ class CreateUserControllerTest extends ApiControllerTestCase
*/ */
public function cannot_create_user_without_data() public function cannot_create_user_without_data()
{ {
$this->expectException(ValidationException::class); $response = $this->callWith();
$this->expectExceptionMessage('The given data was invalid.');
$this->callWith(); $this->assertEquals(422, $response->getStatusCode());
} }
/** /**
@ -104,12 +101,9 @@ class CreateUserControllerTest extends ApiControllerTestCase
$settings = app(SettingsRepositoryInterface::class); $settings = app(SettingsRepositoryInterface::class);
$settings->set('allow_sign_up', false); $settings->set('allow_sign_up', false);
$this->expectException(PermissionDeniedException::class); $response = $this->callWith($this->data);
$this->assertEquals(403, $response->getStatusCode());
try {
$this->callWith($this->data);
} finally {
$settings->set('allow_sign_up', true); $settings->set('allow_sign_up', true);
} }
} }
}

View File

@ -10,7 +10,6 @@
namespace Flarum\Tests\integration\api\Controller; namespace Flarum\Tests\integration\api\Controller;
use Flarum\Api\Controller\ListNotificationsController; use Flarum\Api\Controller\ListNotificationsController;
use Flarum\User\Exception\PermissionDeniedException;
use Flarum\User\User; use Flarum\User\User;
class ListNotificationsControllerTest extends ApiControllerTestCase class ListNotificationsControllerTest extends ApiControllerTestCase
@ -33,9 +32,9 @@ class ListNotificationsControllerTest extends ApiControllerTestCase
*/ */
public function disallows_index_for_guest() public function disallows_index_for_guest()
{ {
$this->expectException(PermissionDeniedException::class); $response = $this->callWith();
$this->callWith(); $this->assertEquals(403, $response->getStatusCode());
} }
/** /**

View File

@ -10,7 +10,6 @@
namespace Flarum\Tests\integration\api\Controller; namespace Flarum\Tests\integration\api\Controller;
use Flarum\Api\Controller\ListUsersController; use Flarum\Api\Controller\ListUsersController;
use Flarum\User\Exception\PermissionDeniedException;
use Flarum\User\User; use Flarum\User\User;
class ListUsersControllerTest extends ApiControllerTestCase class ListUsersControllerTest extends ApiControllerTestCase
@ -39,9 +38,9 @@ class ListUsersControllerTest extends ApiControllerTestCase
*/ */
public function disallows_index_for_guest() public function disallows_index_for_guest()
{ {
$this->expectException(PermissionDeniedException::class); $response = $this->callWith();
$this->callWith(); $this->assertEquals(403, $response->getStatusCode());
} }
/** /**

View File

@ -13,7 +13,6 @@ use Carbon\Carbon;
use Flarum\Api\Controller\ShowDiscussionController; use Flarum\Api\Controller\ShowDiscussionController;
use Flarum\Discussion\Discussion; use Flarum\Discussion\Discussion;
use Flarum\User\User; use Flarum\User\User;
use Illuminate\Database\Eloquent\ModelNotFoundException;
class ShowDiscussionControllerTest extends ApiControllerTestCase class ShowDiscussionControllerTest extends ApiControllerTestCase
{ {
@ -71,11 +70,9 @@ class ShowDiscussionControllerTest extends ApiControllerTestCase
*/ */
public function guest_cannot_see_empty_discussion() public function guest_cannot_see_empty_discussion()
{ {
$this->expectException(ModelNotFoundException::class);
$response = $this->callWith([], ['id' => 1]); $response = $this->callWith([], ['id' => 1]);
$this->assertEquals(200, $response->getStatusCode()); $this->assertEquals(404, $response->getStatusCode());
} }
/** /**
@ -93,8 +90,8 @@ class ShowDiscussionControllerTest extends ApiControllerTestCase
*/ */
public function guests_cannot_see_private_discussion() public function guests_cannot_see_private_discussion()
{ {
$this->expectException(ModelNotFoundException::class); $response = $this->callWith([], ['id' => 3]);
$this->callWith([], ['id' => 3]); $this->assertEquals(404, $response->getStatusCode());
} }
} }