From 66f2d93c5e946e2040950b24980da7baa0e06750 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 12 Mar 2024 22:24:06 +0100 Subject: [PATCH] feat(Middleware): introduce InjectionMiddleware, with Table support first - no more handcrafted Table instantiation and error handling on Controllers - switching to Attributes Signed-off-by: Arthur Schiwon --- .../AEnvironmentAwareOCSController.php | 17 +++ lib/Controller/ApiTablesController.php | 91 +++++++--------- lib/Middleware/Attribute/RequireTable.php | 19 ++++ lib/Middleware/InjectionMiddleware.php | 102 ++++++++++++++++++ 4 files changed, 174 insertions(+), 55 deletions(-) create mode 100644 lib/Controller/AEnvironmentAwareOCSController.php create mode 100644 lib/Middleware/Attribute/RequireTable.php create mode 100644 lib/Middleware/InjectionMiddleware.php diff --git a/lib/Controller/AEnvironmentAwareOCSController.php b/lib/Controller/AEnvironmentAwareOCSController.php new file mode 100644 index 000000000..f270d47c7 --- /dev/null +++ b/lib/Controller/AEnvironmentAwareOCSController.php @@ -0,0 +1,17 @@ +table = $table; + } + + public function getTable(): ?Table { + return $this->table; + } +} diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index 6748d109f..feaf8f65f 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -6,9 +6,11 @@ use OCA\Tables\Errors\InternalError; use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; +use OCA\Tables\Middleware\Attribute\RequireTable; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\TableService; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\DataResponse; use OCP\IL10N; use OCP\IRequest; @@ -17,7 +19,7 @@ /** * @psalm-import-type TablesTable from ResponseDefinitions */ -class ApiTablesController extends AOCSController { +class ApiTablesController extends AEnvironmentAwareOCSController { private TableService $service; public function __construct( @@ -33,12 +35,11 @@ public function __construct( /** * [api v2] Returns all Tables * - * @NoAdminRequired - * * @return DataResponse|DataResponse * * 200: Tables returned */ + #[NoAdminRequired] public function index(): DataResponse { try { return new DataResponse($this->service->formatTables($this->service->findAll($this->userId))); @@ -50,32 +51,21 @@ public function index(): DataResponse { /** * [api v2] Get a table object * - * @NoAdminRequired - * - * @param int $id Table ID * @return DataResponse|DataResponse * * 200: Table returned * 403: No permissions * 404: Not found */ - public function show(int $id): DataResponse { - try { - return new DataResponse($this->service->find($id)->jsonSerialize()); - } catch (PermissionError $e) { - return $this->handlePermissionError($e); - } catch (InternalError $e) { - return $this->handleError($e); - } catch (NotFoundError $e) { - return $this->handleNotFoundError($e); - } + #[NoAdminRequired] + #[RequireTable(enhance: true)] + public function show(): DataResponse { + return new DataResponse($this->getTable()->jsonSerialize()); } /** * [api v2] Create a new table and return it * - * @NoAdminRequired - * * @param string $title Title of the table * @param string|null $emoji Emoji for the table * @param string $template Template to use if wanted @@ -84,6 +74,7 @@ public function show(int $id): DataResponse { * * 200: Tables returned */ + #[NoAdminRequired] public function create(string $title, ?string $emoji, string $template = 'custom'): DataResponse { try { return new DataResponse($this->service->create($title, $template, $emoji)->jsonSerialize()); @@ -95,9 +86,6 @@ public function create(string $title, ?string $emoji, string $template = 'custom /** * [api v2] Update tables properties * - * @NoAdminRequired - * - * @param int $id Table ID * @param string|null $title New table title * @param string|null $emoji New table emoji * @param bool $archived whether the table is archived @@ -106,41 +94,36 @@ public function create(string $title, ?string $emoji, string $template = 'custom * 200: Tables returned * 403: No permissions * 404: Not found + * + * @throws InternalError + * @throws NotFoundError + * @throws PermissionError */ - public function update(int $id, ?string $title = null, ?string $emoji = null, ?bool $archived = null): DataResponse { - try { - return new DataResponse($this->service->update($id, $title, $emoji, $archived, $this->userId)->jsonSerialize()); - } catch (PermissionError $e) { - return $this->handlePermissionError($e); - } catch (InternalError $e) { - return $this->handleError($e); - } catch (NotFoundError $e) { - return $this->handleNotFoundError($e); - } + #[NoAdminRequired] + #[RequireTable(enhance: true)] + public function update(?string $title = null, ?string $emoji = null, ?bool $archived = null): DataResponse { + // TODO: service class to accept Table instead of ID + return new DataResponse($this->service->update($this->getTable()->getId(), $title, $emoji, $archived, $this->userId)->jsonSerialize()); } /** * [api v2] Delete a table * - * @NoAdminRequired - * - * @param int $id Table ID * @return DataResponse|DataResponse * * 200: Deleted table returned * 403: No permissions * 404: Not found + * + * @throws InternalError + * @throws NotFoundError + * @throws PermissionError */ - public function destroy(int $id): DataResponse { - try { - return new DataResponse($this->service->delete($id)->jsonSerialize()); - } catch (PermissionError $e) { - return $this->handlePermissionError($e); - } catch (InternalError $e) { - return $this->handleError($e); - } catch (NotFoundError $e) { - return $this->handleNotFoundError($e); - } + #[NoAdminRequired] + #[RequireTable(enhance: true)] + public function destroy(): DataResponse { + // TODO: service class to accept Table instead of ID + return new DataResponse($this->service->delete($this->getTable()->getId())->jsonSerialize()); } /** @@ -150,7 +133,6 @@ public function destroy(int $id): DataResponse { * * @NoAdminRequired * - * @param int $id Table ID * @param string $newOwnerUserId New user ID * * @return DataResponse|DataResponse @@ -158,16 +140,15 @@ public function destroy(int $id): DataResponse { * 200: Ownership changed * 403: No permissions * 404: Not found + * + * @throws InternalError + * @throws NotFoundError + * @throws PermissionError */ - public function transfer(int $id, string $newOwnerUserId): DataResponse { - try { - return new DataResponse($this->service->setOwner($id, $newOwnerUserId)->jsonSerialize()); - } catch (PermissionError $e) { - return $this->handlePermissionError($e); - } catch (InternalError $e) { - return $this->handleError($e); - } catch (NotFoundError $e) { - return $this->handleNotFoundError($e); - } + #[NoAdminRequired] + #[RequireTable(enhance: true)] + public function transfer(string $newOwnerUserId): DataResponse { + // TODO: service class to accept Table instead of ID + return new DataResponse($this->service->setOwner($this->getTable()->getId(), $newOwnerUserId)->jsonSerialize()); } } diff --git a/lib/Middleware/Attribute/RequireTable.php b/lib/Middleware/Attribute/RequireTable.php new file mode 100644 index 000000000..8aacd0509 --- /dev/null +++ b/lib/Middleware/Attribute/RequireTable.php @@ -0,0 +1,19 @@ +enhance; + } +} diff --git a/lib/Middleware/InjectionMiddleware.php b/lib/Middleware/InjectionMiddleware.php new file mode 100644 index 000000000..610fa015b --- /dev/null +++ b/lib/Middleware/InjectionMiddleware.php @@ -0,0 +1,102 @@ +getAttributes(RequireTable::class); + if (!empty($attributes)) { + /** @var ReflectionAttribute $attribute */ + $attribute = current($attributes); + /** @var RequireTable $requireTableAttribute */ + $requireTableAttribute = $attribute->newInstance(); + $this->getTable($controller, $requireTableAttribute->enhance()); + } + } + + public function afterException($controller, $methodName, $exception): Response { + if ($exception instanceof InvalidArgumentException) { + throw new OCSException($exception->getMessage(), Http::STATUS_BAD_REQUEST, $exception); + } + + $loggerOptions = [ + 'errorCode' => $exception->getCode(), + 'errorMessage' => $exception->getMessage(), + 'exception' => $exception, + ]; + + if ($exception instanceof NotFoundError) { + $this->logger->warning('A not found error occurred: [{errorCode}] {errorMessage}', $loggerOptions); + return new DataResponse(['message' => $this->l->t('A not found error occurred. More details can be found in the logs. Please reach out to your administration.')], Http::STATUS_NOT_FOUND); + } + if ($exception instanceof InternalError) { + $this->logger->warning('An internal error or exception occurred: [{errorCode}] {errorMessage}', $loggerOptions); + return new DataResponse(['message' => $this->l->t('An unexpected error occurred. More details can be found in the logs. Please reach out to your administration.')], Http::STATUS_INTERNAL_SERVER_ERROR); + } + if ($exception instanceof PermissionError) { + $this->logger->warning('A permission error occurred: [{errorCode}] {errorMessage}', $loggerOptions); + return new DataResponse(['message' => $this->l->t('A permission error occurred. More details can be found in the logs. Please reach out to your administration.')], Http::STATUS_FORBIDDEN); + } + + $this->logger->warning('A unknown error occurred: [{errorCode}] {errorMessage}', $loggerOptions); + return new RedirectResponse($this->urlGenerator->linkToDefaultPageUrl()); + } + + /** + * @throws NotFoundError + * @throws PermissionError + * @throws InternalError + */ + protected function getTable(AEnvironmentAwareOCSController $controller, bool $enhance): void { + $tableId = $this->request->getParam('id'); + if ($tableId === null) { + throw new InvalidArgumentException('Missing table ID.'); + } + + $userId = $this->userSession->getUser()?->getUID(); + + $controller->setTable($this->tableService->find($tableId, !$enhance, $userId)); + } +}