From 8c24c4008f83a814ea872231e52a5c53ffd7d8e8 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Mon, 24 Jul 2023 16:30:32 +0300 Subject: [PATCH] ocs controllers response types fixes (#16) --- lib/Controller/AppConfigController.php | 22 ++--- lib/Controller/ExAppController.php | 8 +- lib/Controller/OCSApiController.php | 112 ++++------------------ lib/Controller/PreferencesController.php | 23 ++--- lib/Service/ExFilesActionsMenuService.php | 1 + 5 files changed, 41 insertions(+), 125 deletions(-) diff --git a/lib/Controller/AppConfigController.php b/lib/Controller/AppConfigController.php index b5a77e5d..b9fc8875 100644 --- a/lib/Controller/AppConfigController.php +++ b/lib/Controller/AppConfigController.php @@ -40,7 +40,6 @@ use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; -use OCP\AppFramework\Http\Response; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; @@ -66,22 +65,21 @@ public function __construct( * * @param string $configKey * @param mixed $configValue - * @param string $format * * @throws OCSBadRequestException - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function setAppConfigValue(string $configKey, mixed $configValue, string $format = 'json'): Response { + public function setAppConfigValue(string $configKey, mixed $configValue): DataResponse { if ($configKey === '') { throw new OCSBadRequestException('Config key cannot be empty'); } $appId = $this->request->getHeader('EX-APP-ID'); $result = $this->exAppConfigService->setAppConfigValue($appId, $configKey, $configValue); if ($result instanceof ExAppConfig) { - return $this->buildResponse(new DataResponse(1, Http::STATUS_OK), $format); + return new DataResponse(1, Http::STATUS_OK); } throw new OCSBadRequestException('Error setting app config value'); } @@ -91,17 +89,16 @@ public function setAppConfigValue(string $configKey, mixed $configValue, string * @NoCSRFRequired * * @param array $configKeys - * @param string $format * - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function getAppConfigValues(array $configKeys, string $format = 'json'): Response { + public function getAppConfigValues(array $configKeys): DataResponse { $appId = $this->request->getHeader('EX-APP-ID'); $result = $this->exAppConfigService->getAppConfigValues($appId, $configKeys); - return $this->buildResponse(new DataResponse($result, !empty($result) ? Http::STATUS_OK : Http::STATUS_NOT_FOUND), $format); + return new DataResponse($result, Http::STATUS_OK); } /** @@ -109,16 +106,15 @@ public function getAppConfigValues(array $configKeys, string $format = 'json'): * @NoCSRFRequired * * @param array $configKeys - * @param string $format * * @throws OCSBadRequestException * @throws OCSNotFoundException - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function deleteAppConfigValues(array $configKeys, string $format = 'json'): Response { + public function deleteAppConfigValues(array $configKeys): DataResponse { $appId = $this->request->getHeader('EX-APP-ID'); $result = $this->exAppConfigService->deleteAppConfigValues($configKeys, $appId); if ($result === -1) { @@ -127,6 +123,6 @@ public function deleteAppConfigValues(array $configKeys, string $format = 'json' if ($result === 0) { throw new OCSNotFoundException('No appconfig_ex values deleted'); } - return $this->buildResponse(new DataResponse($result, Http::STATUS_OK), $format); + return new DataResponse($result, Http::STATUS_OK); } } diff --git a/lib/Controller/ExAppController.php b/lib/Controller/ExAppController.php index 15ab360b..4aea0f4f 100644 --- a/lib/Controller/ExAppController.php +++ b/lib/Controller/ExAppController.php @@ -37,7 +37,6 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\DataResponse; -use OCP\AppFramework\Http\Response; use OCP\AppFramework\OCSController; use OCP\IRequest; @@ -57,12 +56,11 @@ public function __construct( * @NoCSRFRequired * * @param bool $extended - * @param string $format * - * @return Response + * @return DataResponse */ #[NoCSRFRequired] - public function getExApps(bool $extended = false, string $format = 'json'): Response { - return $this->buildResponse(new DataResponse($this->service->getExAppsList($extended), Http::STATUS_OK), $format); + public function getExApps(bool $extended = false): DataResponse { + return new DataResponse($this->service->getExAppsList($extended), Http::STATUS_OK); } } diff --git a/lib/Controller/OCSApiController.php b/lib/Controller/OCSApiController.php index 95bc6555..8161fd42 100644 --- a/lib/Controller/OCSApiController.php +++ b/lib/Controller/OCSApiController.php @@ -43,7 +43,6 @@ use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; -use OCP\AppFramework\Http\Response; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; @@ -87,20 +86,15 @@ public function __construct( * * @param int $level * @param string $message - * @param string $format * * @throws OCSBadRequestException - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoAdminRequired] #[NoCSRFRequired] - public function log( - int $level, - string $message, - string $format = 'json', - ): Response { + public function log(int $level, string $message): DataResponse { try { $appId = $this->request->getHeader('EX-APP-ID'); $exApp = $this->service->getExApp($appId); @@ -116,97 +110,31 @@ public function log( $this->logger->log($level, $message, [ 'app' => $appId, ]); - return $this->buildResponse(new DataResponse(1, Http::STATUS_OK), $format); + return new DataResponse(); } catch (\Psr\Log\InvalidArgumentException) { $this->logger->error('Invalid log level'); throw new OCSBadRequestException('Invalid log level'); } } - /** - * @NoCSRFRequired - * - * @param string $appId - * @param array $appData - * @param string $format - * - * @return Response - */ - #[NoCSRFRequired] - public function registerExternalApp(string $appId, array $appData, string $format = 'json'): Response { - // TODO: Sync logic with OCC command - $result = $this->service->registerExApp($appId, $appData); - return $this->buildResponse(new DataResponse([ - 'success' => $result !== null, - 'registeredExApp' => $result, - ], Http::STATUS_OK), $format); - } - - /** - * @NoCSRFRequired - * - * @param string $appId - * @param string $format - * - * @return Response - */ - #[NoCSRFRequired] - public function unregisterExternalApp(string $appId, string $format = 'json'): Response { - // TODO: Sync logic with OCC command - $deletedExApp = $this->service->unregisterExApp($appId); - if ($deletedExApp === null) { - return $this->buildResponse(new DataResponse([ - 'success' => false, - 'error' => $this->l->t('ExApp not found'), - ], Http::STATUS_NOT_FOUND), $format); - } - return $this->buildResponse(new DataResponse([ - 'success' => $deletedExApp->getAppid() === $appId, - 'deletedExApp' => $deletedExApp, - ], Http::STATUS_OK), $format); - } - - /** - * @NoAdminRequired - * @NoCSRFRequired - * - * @param string $appId - * @param string $format - * - * @return Response - */ - #[NoAdminRequired] - #[NoCSRFRequired] - public function getAppStatus(string $appId, string $format = 'json'): Response { - $appStatus = $this->service->getAppStatus($appId); - return $this->buildResponse(new DataResponse([ - 'success' => $appStatus !== null, - 'appStatus' => [ - 'appId' => $appId, - 'status' => $appStatus, - ], - ], Http::STATUS_OK), $format); - } - /** * @PublicPage * @NoCSRFRequired * * @param array $fileActionMenuParams [name, display_name, mime, permissions, order, icon, icon_class, action_handler] - * @param string $format * - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function registerFileActionMenu(array $fileActionMenuParams, string $format = 'json'): Response { + public function registerFileActionMenu(array $fileActionMenuParams): DataResponse { $appId = $this->request->getHeader('EX-APP-ID'); $registeredFileActionMenu = $this->exFilesActionsMenuService->registerFileActionMenu($appId, $fileActionMenuParams); - return $this->buildResponse(new DataResponse([ + return new DataResponse([ 'success' => $registeredFileActionMenu !== null, 'registeredFileActionMenu' => $registeredFileActionMenu, - ], Http::STATUS_OK), $format); + ], Http::STATUS_OK); } /** @@ -238,18 +166,17 @@ public function unregisterFileActionMenu(string $fileActionMenuName): DataRespon * @param string $actionName * @param array $actionFile * @param string $actionHandler - * @param string $format * - * @return Response + * @return DataResponse */ #[NoAdminRequired] #[NoCSRFRequired] - public function handleFileAction(string $appId, string $actionName, array $actionFile, string $actionHandler, string $format = 'json'): Response { + public function handleFileAction(string $appId, string $actionName, array $actionFile, string $actionHandler): DataResponse { $result = $this->exFilesActionsMenuService->handleFileAction($this->userId, $appId, $actionName, $actionHandler, $actionFile); - return $this->buildResponse(new DataResponse([ + return new DataResponse([ 'success' => $result, 'handleFileActionSent' => $result, - ], Http::STATUS_OK), $format); + ], Http::STATUS_OK); } /** @@ -271,7 +198,7 @@ public function loadFileActionIcon(string $appId, string $exFileActionName): Dat Http::STATUS_OK, ['Content-Type' => $icon['headers']['Content-Type'][0] ?? 'image/svg+xml'] ); - $response->cacheFor(Application::ICON_CACHE_TTL, false, true); + $response->cacheFor(ExFilesActionsMenuService::ICON_CACHE_TTL, false, true); return $response; } return new DataDisplayResponse('', 400); @@ -281,15 +208,13 @@ public function loadFileActionIcon(string $appId, string $exFileActionName): Dat * @PublicPage * @NoCSRFRequired * - * @param string $format - * - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function getExAppUsers(string $format = 'json'): Response { - return $this->buildResponse(new DataResponse($this->service->getNCUsersList(), Http::STATUS_OK), $format); + public function getExAppUsers(): DataResponse { + return new DataResponse($this->service->getNCUsersList(), Http::STATUS_OK); } /** @@ -299,19 +224,18 @@ public function getExAppUsers(string $format = 'json'): Response { * @param string $apiRoute * @param int $scopeGroup * @param string $name - * @param string $format * * @throws OCSBadRequestException - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function registerApiScope(string $apiRoute, int $scopeGroup, string $name, string $format): Response { + public function registerApiScope(string $apiRoute, int $scopeGroup, string $name): DataResponse { $apiScope = $this->exAppApiScopeService->registerApiScope($apiRoute, $scopeGroup, $name); if ($apiScope === null) { throw new OCSBadRequestException('Failed to register API scope'); } - return $this->buildResponse(new DataResponse(1, Http::STATUS_OK), $format); + return new DataResponse(1, Http::STATUS_OK); } } diff --git a/lib/Controller/PreferencesController.php b/lib/Controller/PreferencesController.php index 7b6cf26d..912f70de 100644 --- a/lib/Controller/PreferencesController.php +++ b/lib/Controller/PreferencesController.php @@ -40,7 +40,6 @@ use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; -use OCP\AppFramework\Http\Response; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; @@ -70,15 +69,14 @@ public function __construct( * * @param string $configKey * @param mixed $configValue - * @param string $format * * @throws OCSBadRequestException - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function setUserConfigValue(string $configKey, mixed $configValue, string $format = 'json'): Response { + public function setUserConfigValue(string $configKey, mixed $configValue): DataResponse { if ($configKey === '') { throw new OCSBadRequestException('Config key cannot be empty'); } @@ -86,7 +84,7 @@ public function setUserConfigValue(string $configKey, mixed $configValue, string $appId = $this->request->getHeader('EX-APP-ID'); $result = $this->exAppPreferenceService->setUserConfigValue($userId, $appId, $configKey, $configValue); if ($result instanceof ExAppPreference) { - return $this->buildResponse(new DataResponse(1, Http::STATUS_OK), $format); + return new DataResponse(1, Http::STATUS_OK); } throw new OCSBadRequestException('Failed to set user config value'); } @@ -96,18 +94,17 @@ public function setUserConfigValue(string $configKey, mixed $configValue, string * @NoCSRFRequired * * @param array $configKeys - * @param string $format * - * @return Response + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function getUserConfigValues(array $configKeys, string $format = 'json'): Response { + public function getUserConfigValues(array $configKeys): DataResponse { $userId = $this->userSession->getUser()->getUID(); $appId = $this->request->getHeader('EX-APP-ID'); $result = $this->exAppPreferenceService->getUserConfigValues($userId, $appId, $configKeys); - return $this->buildResponse(new DataResponse($result, !empty($result) ? Http::STATUS_OK : Http::STATUS_NOT_FOUND), $format); + return new DataResponse($result, Http::STATUS_OK); } /** @@ -115,15 +112,15 @@ public function getUserConfigValues(array $configKeys, string $format = 'json'): * @NoCSRFRequired * * @param array $configKeys - * @param string $format * * @throws OCSBadRequestException - * @return Response + * @throws OCSNotFoundException + * @return DataResponse */ #[AppEcosystemAuth] #[PublicPage] #[NoCSRFRequired] - public function deleteUserConfigValues(array $configKeys, string $format = 'json'): Response { + public function deleteUserConfigValues(array $configKeys): DataResponse { $userId = $this->userSession->getUser()->getUID(); $appId = $this->request->getHeader('EX-APP-ID'); $result = $this->exAppPreferenceService->deleteUserConfigValues($configKeys, $userId, $appId); @@ -133,6 +130,6 @@ public function deleteUserConfigValues(array $configKeys, string $format = 'json if ($result === 0) { throw new OCSNotFoundException('No preferences_ex values deleted'); } - return $this->buildResponse(new DataResponse($result, Http::STATUS_OK), $format); + return new DataResponse($result, Http::STATUS_OK); } } diff --git a/lib/Service/ExFilesActionsMenuService.php b/lib/Service/ExFilesActionsMenuService.php index 055c125b..c5e2e36d 100644 --- a/lib/Service/ExFilesActionsMenuService.php +++ b/lib/Service/ExFilesActionsMenuService.php @@ -48,6 +48,7 @@ use Psr\Log\LoggerInterface; class ExFilesActionsMenuService { + public const ICON_CACHE_TTL = 60 * 60 * 24; // 1 day private ICache $cache; private ExFilesActionsMenuMapper $mapper; private LoggerInterface $logger;