From b18b2f909a4374be80af038e10e0aa8f9adf308e Mon Sep 17 00:00:00 2001 From: Sami Mazouz Date: Fri, 3 Nov 2023 15:29:39 +0100 Subject: [PATCH 1/2] fix: handled API errors break preloaded content --- extensions/tags/src/Content/Tag.php | 13 +++- extensions/tags/src/Content/Tags.php | 14 +++- framework/core/src/Api/ApiServiceProvider.php | 12 ++-- framework/core/src/Api/Client.php | 23 ++++++- .../core/src/Api/ClientMiddlewarePipe.php | 68 +++++++++++++++++++ .../core/src/Forum/Content/Discussion.php | 18 +++-- framework/core/src/Forum/Content/Index.php | 9 ++- framework/core/src/Forum/Content/User.php | 15 ++-- framework/core/src/Frontend/Frontend.php | 4 +- 9 files changed, 140 insertions(+), 36 deletions(-) create mode 100644 framework/core/src/Api/ClientMiddlewarePipe.php diff --git a/extensions/tags/src/Content/Tag.php b/extensions/tags/src/Content/Tag.php index 01804fc909..0d2afa4267 100644 --- a/extensions/tags/src/Content/Tag.php +++ b/extensions/tags/src/Content/Tag.php @@ -96,8 +96,15 @@ protected function getApiDocument(Request $request, array $params): object protected function getTagsDocument(Request $request, string $slug): object { - return json_decode($this->api->withParentRequest($request)->withQueryParams([ - 'include' => 'children,children.parent,parent,parent.children.parent,state' - ])->get("/tags/$slug")->getBody()); + return json_decode( + $this->api + ->withoutErrorHandling() + ->withParentRequest($request) + ->withQueryParams([ + 'include' => 'children,children.parent,parent,parent.children.parent,state' + ]) + ->get("/tags/$slug") + ->getBody() + ); } } diff --git a/extensions/tags/src/Content/Tags.php b/extensions/tags/src/Content/Tags.php index b17c5da0bb..a62618495b 100644 --- a/extensions/tags/src/Content/Tags.php +++ b/extensions/tags/src/Content/Tags.php @@ -58,8 +58,16 @@ public function __invoke(Document $document, Request $request): Document protected function getTagsDocument(Request $request): array { - return json_decode($this->api->withParentRequest($request)->withQueryParams([ - 'include' => 'children,lastPostedDiscussion,parent' - ])->get('/tags')->getBody(), true); + return json_decode( + $this->api + ->withoutErrorHandling() + ->withParentRequest($request) + ->withQueryParams([ + 'include' => 'children,lastPostedDiscussion,parent' + ]) + ->get('/tags') + ->getBody(), + true + ); } } diff --git a/framework/core/src/Api/ApiServiceProvider.php b/framework/core/src/Api/ApiServiceProvider.php index acd7a8b636..4ced3c4a00 100644 --- a/framework/core/src/Api/ApiServiceProvider.php +++ b/framework/core/src/Api/ApiServiceProvider.php @@ -114,21 +114,17 @@ public function register(): void }); $this->container->singleton(Client::class, function ($container) { - $pipe = new MiddlewarePipe; - $exclude = $container->make('flarum.api_client.exclude_middleware'); $middlewareStack = array_filter($container->make('flarum.api.middleware'), function ($middlewareClass) use ($exclude) { return ! in_array($middlewareClass, $exclude); }); - foreach ($middlewareStack as $middleware) { - $pipe->pipe($container->make($middleware)); - } + $middlewareStack[] = HttpMiddleware\ExecuteRoute::class; - $pipe->pipe(new HttpMiddleware\ExecuteRoute()); - - return new Client($pipe); + return new Client( + new ClientMiddlewarePipe($container, $middlewareStack) + ); }); } diff --git a/framework/core/src/Api/Client.php b/framework/core/src/Api/Client.php index 3ef24084d8..2d57bfae04 100644 --- a/framework/core/src/Api/Client.php +++ b/framework/core/src/Api/Client.php @@ -23,9 +23,10 @@ class Client protected ?ServerRequestInterface $parent = null; protected array $queryParams = []; protected array $body = []; + protected bool $errorHandling = true; public function __construct( - protected MiddlewarePipeInterface $pipe + protected ClientMiddlewarePipe $pipe ) { } @@ -66,6 +67,22 @@ public function withBody(array $body): Client return $new; } + public function withoutErrorHandling(): Client + { + $new = clone $this; + $new->errorHandling = false; + + return $new; + } + + public function withErrorHandling(): Client + { + $new = clone $this; + $new->errorHandling = true; + + return $new; + } + public function get(string $path): ResponseInterface { return $this->send('GET', $path); @@ -114,6 +131,8 @@ public function send(string $method, string $path): ResponseInterface $request = RequestUtil::withActor($request, $this->actor); } - return $this->pipe->handle($request); + return $this->pipe + ->errorHandling($this->errorHandling) + ->handle($request); } } diff --git a/framework/core/src/Api/ClientMiddlewarePipe.php b/framework/core/src/Api/ClientMiddlewarePipe.php new file mode 100644 index 0000000000..9fbb6ff1c8 --- /dev/null +++ b/framework/core/src/Api/ClientMiddlewarePipe.php @@ -0,0 +1,68 @@ +middlewares = new Collection($middlewares); + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + return $this->getPipe()->process($request, $handler); + } + + public function handle(ServerRequestInterface $request): ResponseInterface + { + return $this->getPipe()->handle($request); + } + + public function pipe(MiddlewareInterface $middleware): void + { + $this->middlewares->push($middleware); + } + + protected function getPipe(): MiddlewarePipeInterface + { + if (isset($this->pipe)) { + return $this->pipe; + } + + $this->pipe = new MiddlewarePipe(); + + foreach ($this->middlewares as $middleware) { + $this->pipe->pipe($this->container->make($middleware)); + } + + return $this->pipe; + } + + public function errorHandling(bool $handleErrors): static + { + $errorHandler = 'flarum.api.error_handler'; + + if ($handleErrors && ! $this->middlewares->contains($errorHandler)) { + $this->middlewares = $this->middlewares->prepend($errorHandler); + } elseif (! $handleErrors && $this->middlewares->contains($errorHandler)) { + $this->middlewares = $this->middlewares->reject($errorHandler); + } + + return $this; + } +} diff --git a/framework/core/src/Forum/Content/Discussion.php b/framework/core/src/Forum/Content/Discussion.php index 5c49cadbfb..69019e4e08 100644 --- a/framework/core/src/Forum/Content/Discussion.php +++ b/framework/core/src/Forum/Content/Discussion.php @@ -95,16 +95,14 @@ public function __invoke(Document $document, Request $request): Document protected function getApiDocument(Request $request, string $id, array $params): object { $params['bySlug'] = true; - $response = $this->api - ->withParentRequest($request) - ->withQueryParams($params) - ->get("/discussions/$id"); - $statusCode = $response->getStatusCode(); - - if ($statusCode === 404) { - throw new RouteNotFoundException; - } - return json_decode($response->getBody()); + return json_decode( + $this->api + ->withoutErrorHandling() + ->withParentRequest($request) + ->withQueryParams($params) + ->get("/discussions/$id") + ->getBody() + ); } } diff --git a/framework/core/src/Forum/Content/Index.php b/framework/core/src/Forum/Content/Index.php index a8af226e68..6a7bc94aca 100644 --- a/framework/core/src/Forum/Content/Index.php +++ b/framework/core/src/Forum/Content/Index.php @@ -69,6 +69,13 @@ public function __invoke(Document $document, Request $request): Document */ protected function getApiDocument(Request $request, array $params): object { - return json_decode($this->api->withParentRequest($request)->withQueryParams($params)->get('/discussions')->getBody()); + return json_decode( + $this->api + ->withoutErrorHandling() + ->withParentRequest($request) + ->withQueryParams($params) + ->get('/discussions') + ->getBody() + ); } } diff --git a/framework/core/src/Forum/Content/User.php b/framework/core/src/Forum/Content/User.php index cbd46140a2..8ecee2b0c1 100644 --- a/framework/core/src/Forum/Content/User.php +++ b/framework/core/src/Forum/Content/User.php @@ -46,13 +46,12 @@ public function __invoke(Document $document, Request $request): Document */ protected function getApiDocument(Request $request, string $username): object { - $response = $this->api->withParentRequest($request)->withQueryParams(['bySlug' => true])->get("/users/$username"); - $statusCode = $response->getStatusCode(); - - if ($statusCode === 404) { - throw new ModelNotFoundException; - } - - return json_decode($response->getBody()); + return json_decode( + $this->api + ->withoutErrorHandling() + ->withParentRequest($request) + ->withQueryParams(['bySlug' => true]) + ->get("/users/$username")->getBody() + ); } } diff --git a/framework/core/src/Frontend/Frontend.php b/framework/core/src/Frontend/Frontend.php index 32079881c4..1f4be210fe 100644 --- a/framework/core/src/Frontend/Frontend.php +++ b/framework/core/src/Frontend/Frontend.php @@ -59,7 +59,9 @@ protected function populate(Document $document, Request $request): void private function getForumDocument(Request $request): array { return $this->getResponseBody( - $this->api->withParentRequest($request)->get('/') + $this->api->withoutErrorHandling() + ->withParentRequest($request) + ->get('/') ); } From 1dec3f8e08008984c705bdd3421edae2f9872bf9 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Fri, 3 Nov 2023 14:30:05 +0000 Subject: [PATCH 2/2] Apply fixes from StyleCI --- framework/core/src/Api/Client.php | 1 - framework/core/src/Api/ClientMiddlewarePipe.php | 7 +++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/framework/core/src/Api/Client.php b/framework/core/src/Api/Client.php index 2d57bfae04..d0f263480c 100644 --- a/framework/core/src/Api/Client.php +++ b/framework/core/src/Api/Client.php @@ -13,7 +13,6 @@ use Flarum\User\User; use Laminas\Diactoros\ServerRequestFactory; use Laminas\Diactoros\Uri; -use Laminas\Stratigility\MiddlewarePipeInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; diff --git a/framework/core/src/Api/ClientMiddlewarePipe.php b/framework/core/src/Api/ClientMiddlewarePipe.php index 9fbb6ff1c8..fcf1a09539 100644 --- a/framework/core/src/Api/ClientMiddlewarePipe.php +++ b/framework/core/src/Api/ClientMiddlewarePipe.php @@ -1,5 +1,12 @@