From 4121990c3e6dae6a3fa1483f345ebd96d1e98b41 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sun, 28 Jan 2024 12:25:40 +0100 Subject: [PATCH] refactor(settings): Create a UserSettingsService and load all settings via initial state Signed-off-by: Marcel Klehr --- appinfo/routes.php | 15 +- lib/Controller/SettingsController.php | 230 ++------------------------ lib/Controller/WebViewController.php | 62 ++----- lib/Service/UserSettingsService.php | 95 +++++++++++ src/components/Settings.vue | 12 +- src/store/actions.js | 7 +- 6 files changed, 133 insertions(+), 288 deletions(-) create mode 100644 lib/Service/UserSettingsService.php diff --git a/appinfo/routes.php b/appinfo/routes.php index d507f43c3d..f002c9e99f 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -128,19 +128,8 @@ ['name' => 'folders#delete_share', 'url' => '/public/rest/v2/share/{shareId}', 'verb' => 'DELETE'], //Settings - ['name' => 'settings#set_sorting', 'url' => '/settings/sorting', 'verb' => 'POST'], - ['name' => 'settings#get_sorting', 'url' => '/settings/sorting', 'verb' => 'GET'], - ['name' => 'settings#set_view_mode', 'url' => '/settings/viewMode', 'verb' => 'POST'], - ['name' => 'settings#get_view_mode', 'url' => '/settings/viewMode', 'verb' => 'GET'], - ['name' => 'settings#set_archive_path', 'url' => '/settings/archivePath', 'verb' => 'POST'], - ['name' => 'settings#get_archive_path', 'url' => '/settings/archivePath', 'verb' => 'GET'], - ['name' => 'settings#set_backup_path', 'url' => '/settings/backupPath', 'verb' => 'POST'], - ['name' => 'settings#get_backup_path', 'url' => '/settings/backupPath', 'verb' => 'GET'], - ['name' => 'settings#set_backup_enabled', 'url' => '/settings/backupEnabled', 'verb' => 'POST'], - ['name' => 'settings#get_backup_enabled', 'url' => '/settings/backupEnabled', 'verb' => 'GET'], - ['name' => 'settings#set_whatsnew', 'url' => '/settings/hasSeenWhatsnew', 'verb' => 'POST'], - ['name' => 'settings#get_whatsnew', 'url' => '/settings/hasSeenWhatsnew', 'verb' => 'GET'], - ['name' => 'settings#get_limit', 'url' => '/settings/limit', 'verb' => 'GET'], + ['name' => 'settings#set_setting', 'url' => '/settings/{key}', 'verb' => 'POST'], + ['name' => 'settings#get_setting', 'url' => '/settings/{key}', 'verb' => 'GET'], # public link web view ['name' => 'web_view#link', 'url' => '/public/{token}', 'verb' => 'GET'], diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 95d0790fa1..c87fee0806 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -7,248 +7,46 @@ namespace OCA\Bookmarks\Controller; -use Exception; +use OCA\Bookmarks\Service\UserSettingsService; use OCP\AppFramework\ApiController; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; -use OCP\IConfig; -use OCP\IL10N; use OCP\IRequest; class SettingsController extends ApiController { - /** @var IConfig */ - private $config; - - /** @var string */ - private $userId; - /** - * @var IL10N - */ - private $l; /** * @param string $appName * @param IRequest $request - * @param string $userId - * @param IConfig $config - * @param IL10N $l + * @param UserSettingsService $userSettingsService */ public function __construct( - $appName, $request, $userId, IConfig $config, IL10N $l + $appName, $request, + private UserSettingsService $userSettingsService, ) { parent::__construct($appName, $request); - $this->config = $config; - $this->userId = $userId; - $this->l = $l; } - private function getSetting(string $key, string $name, $default): JSONResponse { + public function getSetting(string $key): JSONResponse { try { - $userValue = $this->config->getUserValue( - $this->userId, - $this->appName, - $key, - $default - ); - } catch (Exception $e) { - return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); + $value = $this->userSettingsService->get($key); + } catch (\UnexpectedValueException) { + return new JSONResponse([], Http::STATUS_BAD_REQUEST); } - return new JSONResponse([$name => $userValue], Http::STATUS_OK); + return new JSONResponse([$key => $value], Http::STATUS_OK); } - private function setSetting(string $key, string $value): JSONResponse { + public function setSetting(string $key, string $value): JSONResponse { try { - $this->config->setUserValue( - $this->userId, - $this->appName, - $key, - $value - ); - } catch (Exception $e) { - return new JSONResponse(['status' => 'error'], Http::STATUS_INTERNAL_SERVER_ERROR); - } - - return new JSONResponse(['status' => 'success'], Http::STATUS_OK); - } - - /** - * get sorting option config value - * - * @return JSONResponse - * - * @NoAdminRequired - */ - public function getSorting(): JSONResponse { - return $this->getSetting('sorting', 'sorting', 'lastmodified'); - } - - /** - * set sorting option config value - * - * @param string $sorting - * @return JSONResponse - * - * @NoAdminRequired - */ - public function setSorting($sorting = ""): JSONResponse { - $legalArguments = ['title', 'added', 'clickcount', 'lastmodified', 'index', 'url']; - if (!in_array($sorting, $legalArguments)) { + $this->userSettingsService->set($key, $value); + } catch (\ValueError) { return new JSONResponse(['status' => 'error'], Http::STATUS_BAD_REQUEST); - } - return $this->setSetting( - 'sorting', - $sorting - ); - } - - /** - * get view mode option config value - * - * @return JSONResponse - * - * @NoAdminRequired - */ - public function getViewMode(): JSONResponse { - return $this->getSetting('viewMode', 'viewMode', 'grid'); - } - - /** - * set sorting option config value - * - * @param string $viewMode - * @return JSONResponse - * - * @NoAdminRequired - */ - public function setViewMode($viewMode = ""): JSONResponse { - $legalArguments = ['grid', 'list']; - if (!in_array($viewMode, $legalArguments)) { + } catch (\UnexpectedValueException) { return new JSONResponse(['status' => 'error'], Http::STATUS_BAD_REQUEST); } - return $this->setSetting( - 'viewMode', - $viewMode - ); - } - - /** - * get per-user bookmarks limit - * - * @return JSONResponse - * - * @NoAdminRequired - */ - public function getLimit(): JSONResponse { - $limit = (int)$this->config->getAppValue('bookmarks', 'performance.maxBookmarksperAccount', 0); - return new JSONResponse(['limit' => $limit], Http::STATUS_OK); - } - - /** - * get user-defined archive path - * - * @return JSONResponse - * - * @NoAdminRequired - */ - public function getArchivePath(): JSONResponse { - return $this->getSetting( - 'archive.filePath', - 'archivePath', - $this->l->t('Bookmarks') - ); - } - - /** - * set user-defined archive path - * - * @param string $archivePath - * @return JSONResponse - * - * @NoAdminRequired - */ - public function setArchivePath(string $archivePath): JSONResponse { - return $this->setSetting('archive.filePath', $archivePath); - } - - /** - * get user-defined archive path - * - * @return JSONResponse - * - * @NoAdminRequired - */ - public function getBackupEnabled(): JSONResponse { - return $this->getSetting( - 'backup.enabled', - 'backupEnabled', - (string) false - ); - } - - /** - * set user-defined backup path - * - * @param string $backupEnabled - * @return JSONResponse - * - * @NoAdminRequired - */ - public function setBackupEnabled(bool $backupEnabled): JSONResponse { - return $this->setSetting('backup.enabled', (string) $backupEnabled); - } - /** - * get user-defined archive path - * - * @return JSONResponse - * - * @NoAdminRequired - */ - public function getBackupPath(): JSONResponse { - return $this->getSetting( - 'backup.filePath', - 'backupPath', - $this->l->t('Bookmarks Backups') - ); - } - - /** - * set user-defined backup path - * - * @param string $backupPath - * @return JSONResponse - * - * @NoAdminRequired - */ - public function setBackupPath(string $backupPath): JSONResponse { - return $this->setSetting('backup.filePath', $backupPath); - } - - /** - * get hasSeenWhatsnew - * - * @return JSONResponse - * - * @NoAdminRequired - */ - public function getWhatsnew(): JSONResponse { - return $this->getSetting( - 'hasSeenWhatsnew', - 'hasSeenWhatsnew', - '0' - ); + return new JSONResponse(['status' => 'success'], Http::STATUS_OK); } - /** - * set hasSeenWhatsnew - * - * @param string $hasSeenWhatsnew - * @return JSONResponse - * - * @NoAdminRequired - */ - public function setWhatsnew(string $hasSeenWhatsnew): JSONResponse { - return $this->setSetting('hasSeenWhatsnew', $hasSeenWhatsnew); - } } diff --git a/lib/Controller/WebViewController.php b/lib/Controller/WebViewController.php index 2f646f2c0b..5a58fa2f8b 100644 --- a/lib/Controller/WebViewController.php +++ b/lib/Controller/WebViewController.php @@ -13,6 +13,7 @@ use OCA\Bookmarks\Db\FolderMapper; use OCA\Bookmarks\Db\PublicFolder; use OCA\Bookmarks\Db\PublicFolderMapper; +use OCA\Bookmarks\Service\UserSettingsService; use OCP\AppFramework\Controller; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -31,41 +32,6 @@ class WebViewController extends Controller { /** @var string */ private $userId; - /** - * @var IL10N - */ - private $l; - - /** - * @var PublicFolderMapper - */ - private $publicFolderMapper; - - /** - * @var IUserManager - */ - private $userManager; - /** - * @var FolderMapper - */ - private $folderMapper; - /** - * @var IURLGenerator - */ - private $urlGenerator; - /** - * @var \OCP\IInitialStateService - */ - private $initialState; - /** - * @var InternalFoldersController - */ - private $folderController; - /** - * @var IConfig - */ - private $config; - /** * WebViewController constructor. @@ -82,17 +48,20 @@ class WebViewController extends Controller { * @param InternalFoldersController $folderController * @param IConfig $config */ - public function __construct($appName, $request, $userId, IL10N $l, PublicFolderMapper $publicFolderMapper, IUserManager $userManager, FolderMapper $folderMapper, IURLGenerator $urlGenerator, \OCP\IInitialStateService $initialState, \OCA\Bookmarks\Controller\InternalFoldersController $folderController, IConfig $config) { + public function __construct( + $appName, + IRequest $request, + string $userId, + private IL10N $l, + private PublicFolderMapper $publicFolderMapper, + private IUserManager $userManager, + private FolderMapper $folderMapper, + private IURLGenerator $urlGenerator, + private \OCP\IInitialStateService $initialState, + private \OCA\Bookmarks\Controller\InternalFoldersController $folderController, + private UserSettingsService $userSettingsService) { parent::__construct($appName, $request); $this->userId = $userId; - $this->l = $l; - $this->publicFolderMapper = $publicFolderMapper; - $this->userManager = $userManager; - $this->folderMapper = $folderMapper; - $this->urlGenerator = $urlGenerator; - $this->initialState = $initialState; - $this->folderController = $folderController; - $this->config = $config; } /** @@ -115,10 +84,7 @@ public function index(): AugmentedTemplateResponse { // Provide complete folder hierarchy $this->initialState->provideInitialState($this->appName, 'folders', $this->folderController->getFolders()->getData()['data']); - $settings = []; - foreach (['sorting', 'viewMode', 'hasSeenWhatsnew'] as $setting) { - $settings[$setting] = $this->config->getUserValue($this->userId, $this->appName, $setting); - } + $settings = $this->userSettingsService->toArray(); $this->initialState->provideInitialState($this->appName, 'settings', $settings); return $res; diff --git a/lib/Service/UserSettingsService.php b/lib/Service/UserSettingsService.php new file mode 100644 index 0000000000..635d9342f0 --- /dev/null +++ b/lib/Service/UserSettingsService.php @@ -0,0 +1,95 @@ +config->getAppValue('bookmarks', 'performance.maxBookmarksperAccount', 0); + } + if ($key === 'archive.filePath') { + $default = $this->l->t('Bookmarks'); + } + if ($key === 'backup.enabled') { + $default = (string) false; + } + if ($key === 'backup.filePath') { + $default = $this->l->t('Bookmarks Backups'); + } + return $this->config->getUserValue( + $this->userId, + $this->appName, + $key, + $default + ); + } + + /** + * @param string $key + * @param string $value + * @return void + * @throws \ValueError|\UnexpectedValueException + */ + public function set(string $key, string $value): void { + if (!in_array($key, self::KEYS)) { + throw new \UnexpectedValueException(); + } + if ($key === 'viewMode' && !in_array($value, ['grid', 'list'], true)) { + throw new \ValueError(); + } + if ($key === 'sorting' && !in_array($value, ['title', 'added', 'clickcount', 'lastmodified', 'index', 'url'], true)) { + throw new \ValueError(); + } + $this->config->setUserValue( + $this->userId, + $this->appName, + $key, + $value + ); + } + + /** + * @return array + */ + public function toArray(): array { + $array = []; + foreach(self::KEYS as $key) { + $array[$key] = $this->get($key); + } + $array['limit'] = $this->get('limit'); + return $array; + } +} diff --git a/src/components/Settings.vue b/src/components/Settings.vue index ba5448acb7..1afb297c12 100644 --- a/src/components/Settings.vue +++ b/src/components/Settings.vue @@ -111,13 +111,13 @@ export default { return `javascript:(function(){var a=window,b=document,c=encodeURIComponent,e=c(document.title),d=a.open('${bookmarkletUrl}?url='+c(b.location)+'&title='+e${queryStringExtension},'bkmk_popup','left='+((a.screenX||a.screenLeft)+10)+',top='+((a.screenY||a.screenTop)+10)+',height=650px,width=550px,resizable=1,alwaysRaised=1');a.setTimeout(function(){d.focus()},300);})();` }, archivePath() { - return this.$store.state.settings.archivePath + return this.$store.state.settings['archive.filePath'] }, backupPath() { - return this.$store.state.settings.backupPath + return this.$store.state.settings['backup.filePath'] }, backupEnabled() { - return Boolean(this.$store.state.settings.backupEnabled) + return Boolean(this.$store.state.settings['backup.enabled']) }, }, mounted() { @@ -150,7 +150,7 @@ export default { async onChangeArchivePath(e) { const path = await this.archivePathPicker.pick() await this.$store.dispatch(actions.SET_SETTING, { - key: 'archivePath', + key: 'archive.filePath', value: path, }) }, @@ -160,13 +160,13 @@ export default { } const path = await this.backupPathPicker.pick() await this.$store.dispatch(actions.SET_SETTING, { - key: 'backupPath', + key: 'backup.filePath', value: path, }) }, async onChangeBackupEnabled(e) { await this.$store.dispatch(actions.SET_SETTING, { - key: 'backupEnabled', + key: 'backup.enabled', value: !this.backupEnabled, }) }, diff --git a/src/store/actions.js b/src/store/actions.js index d63a25e824..35fcbef127 100644 --- a/src/store/actions.js +++ b/src/store/actions.js @@ -1092,13 +1092,13 @@ export default { } return axios .post(url(state, `/settings/${key}`), { - [key]: value, + value: String(value), }) .catch(err => { console.error(err) commit( mutations.SET_ERROR, - AppGlobal.methods.t('bookmarks', 'Failed to change setting') + AppGlobal.methods.t('bookmarks', 'Failed to store setting') ) throw err }) @@ -1150,9 +1150,6 @@ export default { } await commit(mutations.SET_SETTING, { key, value }) } - ['archivePath', 'backupPath', 'backupEnabled', 'limit'].forEach(key => - dispatch(actions.LOAD_SETTING, key) - ) }, [actions.LOAD_SHARES]({ commit, dispatch, state }) {